diff mbox

[RFC,4/4] mfd: syscon: add ACPI support

Message ID 1449047368-5768-5-git-send-email-wangkefeng.wang@huawei.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

Kefeng Wang Dec. 2, 2015, 9:09 a.m. UTC
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(+)

Comments

Arnd Bergmann Dec. 2, 2015, 10:44 a.m. UTC | #1
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.

> +               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.

> @@ -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,

We certainly don't want to list specific devices, so the other problem
aside, we wouldn't merge it until there is an official ACPI way to
specify "random crap register areas" that does not depend on a vendor
specific ID.

	Arnd
--
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
Graeme Gregory Dec. 3, 2015, 10:41 a.m. UTC | #2
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

--
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
Kefeng Wang Dec. 3, 2015, 1:01 p.m. UTC | #3
Hi Graeme, Arnd, and Lorenzo,

Firstly, we absolutely agree with the point which use AML to do some "special"
initialisation and configuration.

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?

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?

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
> 
> 
> .
> 

--
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
Lorenzo Pieralisi Dec. 3, 2015, 3:56 p.m. UTC | #4
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 ?

> 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 ?

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
Kefeng Wang Dec. 7, 2015, 6:15 a.m. UTC | #5
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
Arnd Bergmann Dec. 7, 2015, 8:47 a.m. UTC | #6
On Monday 07 December 2015 14:15:37 Kefeng Wang wrote:
> 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)?

It seems that there is some commonality in here that there is more than
one device reset in this system control unit.

I'd suggest moving this out into a proper reset driver that of course
then has to be based on syscon for DT based systems so it doesn't conflict
with the other random stuff in the syscon space, and for backwards
compatibiltiy with old kernels that only know about the syscon based reset
you have currently implemented.

If there are additional devices that also use the same syscon node for
reset, they should of course only use the device_reset() method.

> >> 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.

last level cache is something that should go through architecture code,
it has no business in syscon anyway. What do you control with this anyway?
AFAIK, ARMv8 has architected instructions to control caches and doesn't
need to talk to a cache controller the way we used to do on v6 and older v7
based systems.

	Arnd
--
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
Zhangfei Gao Dec. 11, 2015, 10:35 a.m. UTC | #7
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
Arnd Bergmann Dec. 11, 2015, 10:51 a.m. UTC | #8
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.

	Arnd
--
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
Graeme Gregory Dec. 11, 2015, 10:59 a.m. UTC | #9
On Fri, 11 Dec 2015, at 10:51 AM, 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.
> 

Basically in the DSDT you would create a common accessor method for the
register/registers in question. This function takes care of the register
in the multiple user cases.

each _RST() method would then call this function to set the bits they
required.

So in effect it is no different to what you would do in Linux, just done
in AML instead.

Graeme

--
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
Hanjun Guo Dec. 11, 2015, 12:23 p.m. UTC | #10
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 mbox

Patch

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__ */