linux-kernel - Re: [PATCH v3 3/6] gpio: max77620: Don't set of

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 9 Jul 2020 00:44:34 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 3/6] gpio: max77620: Don't set of_node

08.07.2020 23:57, Andy Shevchenko пишет:
> 
> 
> On Wednesday, July 8, 2020, Dmitry Osipenko <digetx@...il.com
> <mailto:digetx@...il.com>> wrote:
> 
>     The gpiochip_add_data() takes care of setting the of_node to the
>     parent's
>     device of_node, hence there is no need to do it manually in the driver's
>     code. This patch corrects the parent's device pointer and removes the
>     unnecessary setting of the of_node.
> 
> 
> I gave a second look and I think my suggestion is wrong. Here is an
> interesting propagation of the parent device node to its grand son,
> leaving son’s one untouched. Original code has intentions to do that way.

The [1] says that gpio_chip.parent should point at the "device providing
the GPIOs". That's the pdev->dev.parent in the case of this driver.
MAX77620 is an MFD PMIC device that has virtual sub-devices like GPIO
controller, PINCTRL and RTC. The MFD is the parent device that provides
the GPIOs [2].

[1]
https://elixir.bootlin.com/linux/v5.8-rc3/source/include/linux/gpio/driver.h#L276

[2]
https://elixir.bootlin.com/linux/v5.8-rc3/source/arch/arm64/boot/dts/nvidia/tegra210-p2180.dtsi#L48

I think the old code was wrong and this patch is correct, please correct
me if I'm missing something.

>     Suggested-by: Andy Shevchenko <andy.shevchenko@...il.com
>     <mailto:andy.shevchenko@...il.com>>
>     Signed-off-by: Dmitry Osipenko <digetx@...il.com
>     <mailto:digetx@...il.com>>
>     ---
>      drivers/gpio/gpio-max77620.c | 5 +----
>      1 file changed, 1 insertion(+), 4 deletions(-)
> 
>     diff --git a/drivers/gpio/gpio-max77620.c b/drivers/gpio/gpio-max77620.c
>     index 7f7e8d4bf0d3..39d431da2dbc 100644
>     --- a/drivers/gpio/gpio-max77620.c
>     +++ b/drivers/gpio/gpio-max77620.c
>     @@ -279,7 +279,7 @@ static int max77620_gpio_probe(struct
>     platform_device *pdev)
>             mgpio->dev = &pdev->dev;
> 
>             mgpio->gpio_chip.label = pdev->name;
>     -       mgpio->gpio_chip.parent = &pdev->dev;
>     +       mgpio->gpio_chip.parent = pdev->dev.parent;
>             mgpio->gpio_chip.direction_input = max77620_gpio_dir_input;
>             mgpio->gpio_chip.get = max77620_gpio_get;
>             mgpio->gpio_chip.direction_output = max77620_gpio_dir_output;
>     @@ -288,9 +288,6 @@ static int max77620_gpio_probe(struct
>     platform_device *pdev)
>             mgpio->gpio_chip.ngpio = MAX77620_GPIO_NR;
>             mgpio->gpio_chip.can_sleep = 1;
>             mgpio->gpio_chip.base = -1;
>     -#ifdef CONFIG_OF_GPIO
>     -       mgpio->gpio_chip.of_node = pdev->dev.parent->of_node;
>     -#endif
> 
>             platform_set_drvdata(pdev, mgpio);
>      
>     -- 
>     2.26.0
> 
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>