From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Tue, 24 Jul 2018 10:37:00 +0200 Subject: [U-Boot] [RFC PATCH] gpio: zynq: Setup bank_name to dev->name In-Reply-To: <589cfccf-dc79-c84f-f147-36a2ac7b4aee@herbrechtsmeier.net> References: <3a5a2fbe9d0aad4fdbbbf197c39dc0f973e5045e.1531404282.git.michal.simek@xilinx.com> <589cfccf-dc79-c84f-f147-36a2ac7b4aee@herbrechtsmeier.net> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de 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 >>>> --- >>>> >>>>    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