[U-Boot] [RFC PATCH] gpio: zynq: Setup bank_name to dev->name

[Michal Simek]

On 23.7.2018 20:29, Stefan Herbrechtsmeier wrote:
> Hi Michal,
> 
> 
> Am 23.07.2018 um 11:08 schrieb Michal Simek:
>> On 20.7.2018 21:31, Stefan Herbrechtsmeier wrote:
>>> Am 12.07.2018 um 16:04 schrieb Michal Simek:
>>>> There should be proper bank name setup to distiguish between different
>>>> gpio drivers. Use dev->name for it.
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>>>> ---
>>>>
>>>>    drivers/gpio/zynq_gpio.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/gpio/zynq_gpio.c b/drivers/gpio/zynq_gpio.c
>>>> index 26f69b1a713f..f793ee5754a8 100644
>>>> --- a/drivers/gpio/zynq_gpio.c
>>>> +++ b/drivers/gpio/zynq_gpio.c
>>>> @@ -337,6 +337,8 @@ static int zynq_gpio_probe(struct udevice *dev)
>>>>        struct zynq_gpio_privdata *priv = dev_get_priv(dev);
>>>>        struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
>>>>    +    uc_priv->bank_name = dev->name;
>>>> +
>>>>        if (priv->p_data)
>>>>            uc_priv->gpio_count = priv->p_data->ngpio;
>>>>    
>>> Does this not lead to ugly names because the gpio number is append to
>>> the bank_name? Have you check the "gpio status -a" output?
>> Yes I was checking it. Names are composed together but also just numbers
>> works as before.
>>
>> gpio at ff0a00000: input: 0 [ ]
>> gpio at ff0a00001: input: 0 [ ]
>> gpio at ff0a00002: input: 0 [ ]
>> gpio at ff0a00003: input: 0 [ ]
>> gpio at ff0a00004: input: 0 [ ]
>> gpio at ff0a00005: input: 0 [ ]
>> gpio at ff0a00006: input: 0 [ ]
>> gpio at ff0a00007: input: 0 [ ]
>> gpio at ff0a00008: input: 0 [ ]
>> gpio at ff0a00009: input: 0 [ ]
> 
> Do you think that this are meaningful names? It isn't possible to
> separate the device and pin number as well as it mix hex and decimal
> numbers.
> 
>> If you know better way how to setup a bank name please let me know but I
>> need to distinguish ps gpio from pl one and for pl we need to know the
>> address.
> 
> I know the use case.
> 
> A lot of drivers use the bank_name from the device tree, some drivers
> append an underscore to the bank name and others add the req_seq of the
> device to an alphabetic character.
> 
>>> Other drivers use the gpio-bank-name from the device tree.
>> I can't see this property inside Linux kernel. If this has been reviewed
>> by dt guys please let me know.
> 
> This property is only used by u-boot. I think it isn't needed by the
> Linux kernel.

I am happy to use consistent solution but what's that?
Mixing name with hex and int is not nice but adding "_" or something
else is just a pain in driver code. If this is done in core I am fine
with that but adding this code to all drivers don't look like generic
solution at all.
Using additional u-boot property is not good too.

I have mentioned in "gpio: xilinx: Convert driver to DM"
(sha1:10441ec9224d0d269dc512819a32c0785a6338d3)
that uc-priv->name is completely unused. Maybe this should be dev->name
and bank_name should be really used for banks.
Then in gpio status -a can be

Device gpio at a0001000:
Bank:
...

but not sure how gpio commands will work to address exact pin from
prompt. Because this is normally working
gpio toggle gpio at a00010001

Thanks,
Michal