[PATCH] gpio: Add support for gpio-line-names reading

[Michal Simek]

Hi,

On 28. 07. 20 20:58, Simon Glass wrote:
> On Tue, 21 Jul 2020 at 07:15, Michal Simek <michal.simek at xilinx.com> wrote:
>>
>> The commit 2bd261dd1712 ("gpio: search for gpio label if gpio is not found
>> through bank name") introduced the option to search gpio via labels (gpio
>> hog). This patch is just follow up on this and fills pin name based on
>> gpio-line-names DT property.
>>
>> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
>> ---
>>
>>  arch/sandbox/dts/test.dts  |  2 ++
>>  drivers/gpio/gpio-uclass.c | 31 +++++++++++++++++++++++++++++++
>>  test/dm/gpio.c             |  6 ++++++
>>  3 files changed, 39 insertions(+)
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 

I have run CI loop with this patch added and I see one issue regarding
this.
Issue is visible when dm_test_gpio_get_dir_flags() is called.

Specifically
	ut_asserteq(6, gpio_request_list_by_name(dev, "test3-gpios", desc_list,
						 ARRAY_SIZE(desc_list), 0));

This function requests gpios and when request passes it assigns name to
gpio_dev_priv->name[] array.
This is done in dm_gpio_request()

Before assignment there is this code which check if name is already
assigned or not.
	uc_priv = dev_get_uclass_priv(dev);
	if (uc_priv->name[desc->offset])
		return -EBUSY;

That means that we are using name as indicator if gpio is used or not.

This logic is also applied in get_function() where you can see
	if (skip_unused && !uc_priv->name[offset])
		return GPIOF_UNUSED;

Where assigned name is just indicator of if device is used or not.

This doesn't sound right to me.
And that's just open a question how Heiko's patch should work. Because
you look for a name and you get that pin but you can't request is
because this request IMHO has to failed because gpio core thinks that it
is already in used and you shouldn't touch that pin.
The whole code which calls dm_gpio_lookup_name for bank name should be
fine because it is different entry in struct gpio_dev_priv.

Definitely this patch needs to be dropped and changed because it can't
work like this and the question is Heiko's patch should still stay in
tree or should be reverted.

Maybe we should create another entry in char **label; in struct
gpio_dev_priv where these labels should be saved and look for instead of
just name which is used for assignment.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200813/5e897059/attachment.sig>