| Message ID | 1449047368-5768-5-git-send-email-wangkefeng.wang@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 2015/12/3 23:56, Lorenzo Pieralisi wrote: > On Thu, Dec 03, 2015 at 09:01:11PM +0800, Kefeng Wang wrote: >> Hi Graeme, Arnd, and Lorenzo, >> >> Firstly, we absolutely agree with the point which use AML to do some "special" >> initialisation and configuration. > > Good. > >> SAS and NIC driver were accepted by linux in hisilicon hip05 chip, and the drivers >> reset the control by syscon, we want to use "_RST" method, which is introduced by >> ACPI 6.0 spec in "7.3.25 _RST (Device Reset)", is it reasonable and standard for us? > > Can you point me at the drivers you are referring to please ? SAS： https://lkml.org/lkml/2015/11/17/572 [PATCH v5 19/32] scsi: hisi_sas: add v1 HW initialisation code static int reset_hw_v1_hw(struct hisi_hba *hisi_hba); HNS： drivers/net/ethernet/hisilicon/hns_mdio.c static int hns_mdio_reset(struct mii_bus *bus)； > >> But here is a scene, we can not find a suitable way to support ACPI. There is no >> independent memory region in some module(the driver not upstreamed), that is, >> when write and read the module's register, we must r/w by syscon. Any advice? > > What do you mean ? You mean that the reset control is a piece of HW > that is shared between multiple "components" ? What's your concern > about AML code driving those registers here ? This is unrelated to reset control. I mean we have some driver(not upstreamed), like LLC(last level cache), when access the register of llc, we need help through syscon, because the llc has no independent registers region , steps of Read and Write register in those drivers is shown below, 1) get the syscon base; 2) configure and choose the module which need to be accessed; 3) R/W the value from the syscon, that is, get/set the value from/to llc; Every read and write the register in those drivers, we must through syscon. That is why we need syscon to support ACPI. Thanks, Kefeng > > Thanks, > Lorenzo > >> >> Thanks, >> Kefeng >> >> On 2015/12/3 18:41, Graeme Gregory wrote: >>> On Wed, Dec 02, 2015 at 11:44:51AM +0100, Arnd Bergmann wrote: >>>> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote: >>>>> This enables syscon with ACPI support. >>>>> syscon_regmap_lookup_by_dev_property() function was added. With helper >>>>> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used >>>>> in both DT and ACPI. >>>>> >>>>> The device driver can obtain syscon using _DSD method in DSDT, an example >>>>> is shown below. >>>>> >>>>> Device(CTL0) { >>>>> Name(_HID, "HISI0061") >>>>> Name(_CRS, ResourceTemplate() { >>>>> Memory32Fixed(ReadWrite, 0x80000000, 0x10000) >>>>> }) >>>>> } >>>>> >>>>> Device(DEV0) { >>>>> Name(_HID, "HISI00B1") >>>>> Name(_CRS, ResourceTemplate() { >>>>> Memory32Fixed(ReadWrite, 0x8c030000, 0x10000) >>>>> Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 } >>>>> }) >>>>> >>>>> Name (_DSD, Package () { >>>>> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), >>>>> Package () { >>>>> Package () {"syscon",Package() {\_SB.CTL0} } >>>>> } >>>>> }) >>>>> } >>>>> >>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >>>> >>>> This sounds like a bad idea: >>>> >>>> syscon is basically a hack to let us access register that the SoC designer >>>> couldn't fit in anywhere sane. We need something like this with devicetree >>>> because we decided not to have any interpreted bytecode to do this behind >>>> our back. >>>> >>>> With ACPI, the same thing is done with AML, which is actually nicer than >>>> syscon (once you have to deal with all the problems introduced by AML). >>>> >>>> Use that instead. >>>> >>> >>> I have to agree with Arnd here, this is specifically why it was chosen >>> to use ACPI on machines to move all these "hacks" to AML. >>> >>> This leaves your driver being generic and any "special" initialisation >>> can be supplied by the OEM through the ACPI tables. >>> >>> Graeme >>> >>> >>> . >>> >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Arnd On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote: > On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote: >> This enables syscon with ACPI support. >> syscon_regmap_lookup_by_dev_property() function was added. With helper >> device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used >> in both DT and ACPI. >> >> The device driver can obtain syscon using _DSD method in DSDT, an example >> is shown below. >> >> Device(CTL0) { >> Name(_HID, "HISI0061") >> Name(_CRS, ResourceTemplate() { >> Memory32Fixed(ReadWrite, 0x80000000, 0x10000) >> }) >> } >> >> Device(DEV0) { >> Name(_HID, "HISI00B1") >> Name(_CRS, ResourceTemplate() { >> Memory32Fixed(ReadWrite, 0x8c030000, 0x10000) >> Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 } >> }) >> >> Name (_DSD, Package () { >> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), >> Package () { >> Package () {"syscon",Package() {\_SB.CTL0} } >> } >> }) >> } >> >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > > This sounds like a bad idea: > > syscon is basically a hack to let us access register that the SoC designer > couldn't fit in anywhere sane. We need something like this with devicetree > because we decided not to have any interpreted bytecode to do this behind > our back. > > With ACPI, the same thing is done with AML, which is actually nicer than > syscon (once you have to deal with all the problems introduced by AML). > > Use that instead. > Would you mind clarifying AML method, still not understand. Is is means realize the operation in uefi/bootloader and call some interface from kernel? The issue here is we want to access some common registers, which we do not want to ioremap for many times in each driver. In hisi platforms, we have many common registers shared by many components. Thanks. >> + pdev = acpi_dev_find_plat_dev(adev); >> + if (!pdev) >> + return ERR_PTR(-ENODEV); >> + syscon = platform_get_drvdata(pdev); > > This still requires the syscon device to be probed first, which is > something we shouldn't really need any more. > > A lot of syscon users depend on the regmap to be available before > device drivers are loaded, so we have changed the code to just look up > the device_node that is already present and ignore the device. > > Once clps711x has been converted to DT, I think we can rip out the > syscon_regmap_lookup_by_pdevname() interface and separate the syscon > regmap handling from the device handling that we only really need > for debugfs. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/11/2015 06:51 PM, Arnd Bergmann wrote: > On Friday 11 December 2015 18:35:17 Zhangfei Gao wrote: >> On Wed, Dec 2, 2015 at 6:44 PM, Arnd Bergmann <arnd@arndb.de> wrote: >>> On Wednesday 02 December 2015 17:09:28 Kefeng Wang wrote: >>> >>> This sounds like a bad idea: >>> >>> syscon is basically a hack to let us access register that the SoC designer >>> couldn't fit in anywhere sane. We need something like this with devicetree >>> because we decided not to have any interpreted bytecode to do this behind >>> our back. >>> >>> With ACPI, the same thing is done with AML, which is actually nicer than >>> syscon (once you have to deal with all the problems introduced by AML). >>> >>> Use that instead. >>> >> >> Would you mind clarifying AML method, still not understand. >> Is is means realize the operation in uefi/bootloader and call some >> interface from kernel? >> >> The issue here is we want to access some common registers, >> which we do not want to ioremap for many times in each driver. >> >> In hisi platforms, we have many common registers shared by many components. > > Sorry, you'd have to ask someone who is more familiar with ACPI. > > Basically, the idea of ACPI as I understand it is that you have a > interpreted bytecode language that is provided by the BIOS as tables > and executed in the context of the kernel. This byte code has access > to stuff like GPIO, PCI config space, MMIO registers, IPMI or the > embedded controller on a PC. This way, you can hide all the platform > specific details behind a function call from a generic device driver, > and the BIOS writer can provide the specific implementation for any > registers that are shared across components. > > That's about all I know, but the ACPI specifications are available > publicly, please have a look there for more details. Thanks Arnd rise this up, I agree we need to abstract the syscon inside UEFI. For ACPI 6.0, there are method such as _ON, _OFF and _RST (reset) to the device's power resources, I think it can be extended to syscon to reset the device with access syscon in ACPI method, those method will be implemented in UEFI which the kernel driver don't need to care about the detail. We are going to for that implementation for D02, and if problem we meet, we will go back for this discussion. Thanks Hanjun -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mfd/syscon.c b/drivers/mfd/syscon.c index 176bf0f..6f23418 100644 --- a/drivers/mfd/syscon.c +++ b/drivers/mfd/syscon.c @@ -12,6 +12,7 @@ * (at your option) any later version. */ +#include <linux/acpi.h> #include <linux/err.h> #include <linux/io.h> #include <linux/module.h> @@ -21,6 +22,7 @@ #include <linux/of_platform.h> #include <linux/platform_data/syscon.h> #include <linux/platform_device.h> +#include <linux/property.h> #include <linux/regmap.h> #include <linux/mfd/syscon.h> #include <linux/slab.h> @@ -174,6 +176,47 @@ struct regmap *syscon_regmap_lookup_by_phandle(struct device_node *np, } EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_phandle); +struct regmap *syscon_regmap_lookup_by_dev_property(struct device *dev, + const char *propname) +{ + struct fwnode_handle *fwnode; + struct regmap *regmap = NULL; + + fwnode = device_get_reference_node(dev, propname, 0); + + if (IS_ENABLED(CONFIG_OF) && dev->of_node) { + struct device_node *syscon_np; + if (propname) + syscon_np = to_of_node(fwnode); + else + syscon_np = dev->of_node; + if (!syscon_np) + return ERR_PTR(-ENODEV); + regmap = syscon_node_to_regmap(syscon_np); + of_node_put(syscon_np); + } else if (IS_ENABLED(CONFIG_ACPI)) { + struct platform_device *pdev; + struct acpi_device *adev; + struct syscon *syscon; + + adev = to_acpi_device_node(fwnode); + if (!adev) + return ERR_PTR(-ENODEV); + + pdev = acpi_dev_find_plat_dev(adev); + if (!pdev) + return ERR_PTR(-ENODEV); + + syscon = platform_get_drvdata(pdev); + if (!syscon) + return ERR_PTR(-ENODEV); + + regmap = syscon->regmap; + } + return regmap; +} +EXPORT_SYMBOL_GPL(syscon_regmap_lookup_by_dev_property); + static int syscon_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -216,9 +259,16 @@ static const struct platform_device_id syscon_ids[] = { { } }; +static const struct acpi_device_id syscon_acpi_match[] = { + { "HISI0061", 0 }, /* better to use generic ACPI ID */ + { } +}; +MODULE_DEVICE_TABLE(acpi, syscon_acpi_match); + static struct platform_driver syscon_driver = { .driver = { .name = "syscon", + .acpi_match_table = ACPI_PTR(syscon_acpi_match), }, .probe = syscon_probe, .id_table = syscon_ids, diff --git a/include/linux/mfd/syscon.h b/include/linux/mfd/syscon.h index 75e543b..db79ca2 100644 --- a/include/linux/mfd/syscon.h +++ b/include/linux/mfd/syscon.h @@ -17,6 +17,7 @@ #include <linux/err.h> +struct device; struct device_node; #ifdef CONFIG_MFD_SYSCON @@ -26,6 +27,8 @@ extern struct regmap *syscon_regmap_lookup_by_pdevname(const char *s); extern struct regmap *syscon_regmap_lookup_by_phandle( struct device_node *np, const char *property); +extern struct regmap *syscon_regmap_lookup_by_dev_property(struct device *dev, + const char *propname); #else static inline struct regmap *syscon_node_to_regmap(struct device_node *np) { @@ -48,6 +51,11 @@ static inline struct regmap *syscon_regmap_lookup_by_phandle( { return ERR_PTR(-ENOSYS); } +static inline struct regmap *syscon_regmap_lookup_by_dev_property(struct device *dev, + const char *propname); +{ + return ERR_PTR(-ENOSYS); +} #endif #endif /* __LINUX_MFD_SYSCON_H__ */
This enables syscon with ACPI support. syscon_regmap_lookup_by_dev_property() function was added. With helper device_get_reference_node() and acpi_dev_find_plat_dev(), it can be used in both DT and ACPI. The device driver can obtain syscon using _DSD method in DSDT, an example is shown below. Device(CTL0) { Name(_HID, "HISI0061") Name(_CRS, ResourceTemplate() { Memory32Fixed(ReadWrite, 0x80000000, 0x10000) }) } Device(DEV0) { Name(_HID, "HISI00B1") Name(_CRS, ResourceTemplate() { Memory32Fixed(ReadWrite, 0x8c030000, 0x10000) Interrupt(ResourceConsumer, Level, ActiveHigh, Exclusive){ 192 } }) Name (_DSD, Package () { ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"), Package () { Package () {"syscon",Package() {\_SB.CTL0} } } }) } Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- drivers/mfd/syscon.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++ include/linux/mfd/syscon.h | 8 ++++++++ 2 files changed, 58 insertions(+) -- 1.7.12.4 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html