On 24.7.2018 21:39, Stefan Herbrechtsmeier wrote: > Am 24.07.2018 um 10:37 schrieb Michal Simek: >> On 23.7.2018 20:29, Stefan Herbrechtsmeier wrote: >>> 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.si...@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@ff0a00000: input: 0 [ ] >>>> gpio@ff0a00001: input: 0 [ ] >>>> gpio@ff0a00002: input: 0 [ ] >>>> gpio@ff0a00003: input: 0 [ ] >>>> gpio@ff0a00004: input: 0 [ ] >>>> gpio@ff0a00005: input: 0 [ ] >>>> gpio@ff0a00006: input: 0 [ ] >>>> gpio@ff0a00007: input: 0 [ ] >>>> gpio@ff0a00008: input: 0 [ ] >>>> gpio@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? > > Consistent solution between what?
all drivers. Name should be composed consistently among all drivers. It means drivers shouldn't add additional "_" in driver code for example. > >> 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. > > Normally the bank name is an alphabetic character or string. Maybe we > could add the device name to the gpio_lookup_name function and add an > additional optional device name parameter to the gpio command. > >> 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. > > Isn't the uc-priv->name used for the label of the request? Last time when I looked at it and I didn't see this name listed anywhere in output. >> Then in gpio status -a can be >> >> Device gpio@a0001000: >> Bank: >> ... >> >> but not sure how gpio commands will work to address exact pin from >> prompt. Because this is normally working >> gpio toggle gpio@a00010001 > > With an optional device name this would be: > gpio toggle gpio@a0001000 1 > > Alternative the gpio command could support the requested labels: > gpio toggle second-gpio I am open to see this solution. Feel free to invest your time support this but I have no time for that. Thanks, Michal _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
