Re: [PATCH v2 2/3] pinctrl: amd: Get and update IOMUX details

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, May 24, 2022 at 07:20:34PM +0530, Basavaraj Natikar wrote:
> On 5/24/2022 6:04 PM, Mika Westerberg wrote:
> > On Tue, May 24, 2022 at 05:54:47PM +0530, Basavaraj Natikar wrote:
> >> There is no CRS method defined for IOMX/0xFED80D00 in ACPI namespace // IOMX Address Base 
> >> Hence I added additional code to get IOMX memory region.
> >>
> >> since _CRS method is used to get GPIO pin base for AMDI0030 in
> >> pinctrl-amd as below, same is not available for IOMX
> >>  
> >>   Device (GPIO)
> >>         {
> >>             Name (_HID, "AMDI0030")  // _HID: Hardware ID
> >>             Name (_CID, "AMDI0030")  // _CID: Compatible ID
> >>             Name (_UID, Zero)  // _UID: Unique ID
> >>             Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
> >>             {
> >>                 Name (RBUF, ResourceTemplate ()
> >>                 {
> >>                     Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, )
> >>                     {
> >>                         0x00000007,
> >>                     }
> >>                     Memory32Fixed (ReadWrite,
> >>                         0xFED81500,         // Address Base
> >>                         0x00000400,         // Address Length
> >>                         )
> > Is there something preventing you to add it here like below?
> >
> >                      Memory32Fixed (ReadWrite, 0xFED80D00 0x00000400)
> 
> yes few system has different entries already defined like below      
>   Device (GPIO)
>         {
>             Name (_HID, "AMDI0030")  // _HID: Hardware ID
>             Name (_CID, "AMDI0030")  // _CID: Compatible ID
>             Name (_UID, Zero)  // _UID: Unique ID
>             Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>             {
>                 Name (RBUF, ResourceTemplate ()
>                 {
>                     Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, )
>                     {
>                         0x00000007,
>                     }
>                     Memory32Fixed (ReadWrite,
>                         0xFED81500,         // Address Base
>                         0x00000400,         // Address Length
>                         )
>                 })
>                 Return (RBUF) /* \_SB_.GPIO._CRS.RBUF */
>             }
> 
> 
>         Device (GPIO)
>         {
>             Name (_HID, "AMDI0030")  // _HID: Hardware ID
>             Name (_CID, "AMDI0030")  // _CID: Compatible ID
>             Name (_UID, Zero)  // _UID: Unique ID
>             Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
>             {
>                 Name (RBUF, ResourceTemplate ()
>                 {
>                     Interrupt (ResourceConsumer, Level, ActiveLow, Shared, ,, )
>                     {
>                         0x00000007,
>                     }
>                     Memory32Fixed (ReadWrite,
>                         0xFED81500,         // Address Base
>                         0x00000400,         // Address Length
>                         )
>                     Memory32Fixed (ReadWrite,
>                         0xFED81200,         // Address Base
>                         0x00000100,         // Address Length
>                         )
>                 })
>                 Return (RBUF) /* \_SB_.GPIO._CRS.RBUF */
>             }
> 
> if we add or in future some entries added. 
> is there way to map particular entry for IOMUX? 

Straightforward way is to add it always to the end and add _DSD boolean
property ("amd,pinctrl-iomux-present") and act accordingly. More flexible and
less error prone is to name all resources with DSD property and find resource
by name: "amd,pinctrl-resource-names" = "bank0", "bank1", "iomux".


-- 
With Best Regards,
Andy Shevchenko