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

[Basavaraj Natikar <bnatikar@xxxxxxx>]

On 5/25/2022 10:15 PM, Andy Shevchenko wrote:

> On Wed, May 25, 2022 at 03:12:05PM +0530, Basavaraj Natikar wrote:
>> On 5/24/2022 10:09 PM, Andy Shevchenko wrote:
>>> 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".
>>>
>> Idea of adding _DSD property looks good, we can add easily _DSD
>> "pinctrl-iomux-present" u8 property to return index or instance
>> value directly. 
> Better to name them all, it will be more consistent and robust.

Thanks. Sounds good, will make this change.

Thanks,
Basavaraj