linux-kernel - Re: [PATCH v1 09/11] regulator: bd9576: switch to using devm_fwnode_gpiod

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Mon, 5 Sep 2022 16:19:27 +0300
From:   Matti Vaittinen <mazziesaccount@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Felipe Balbi <balbi@...nel.org>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Daniel Vetter <daniel@...ll.ch>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Marc Zyngier <maz@...nel.org>,
        Richard Weinberger <richard@....at>,
        David Airlie <airlied@...ux.ie>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Alyssa Rosenzweig <alyssa@...enzweig.io>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Rob Herring <robh@...nel.org>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Pali Rohár <pali@...nel.org>,
        LINUXWATCHDOG <linux-watchdog@...r.kernel.org>,
        USB <linux-usb@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        linux-pci <linux-pci@...r.kernel.org>,
        linux-tegra <linux-tegra@...r.kernel.org>,
        "open list:MEMORY TECHNOLOGY..." <linux-mtd@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-stm32@...md-mailman.stormreply.com,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v1 09/11] regulator: bd9576: switch to using
 devm_fwnode_gpiod_get()

On 9/5/22 13:40, Andy Shevchenko wrote:
> On Mon, Sep 5, 2022 at 9:33 AM Dmitry Torokhov
> <dmitry.torokhov@...il.com> wrote:
>>
>> I would like to stop exporting OF-specific devm_gpiod_get_from_of_node()
>> so that gpiolib can be cleaned a bit, so let's switch to the generic
>> fwnode property API.
>>
>> While at it switch the rest of the calls to read properties in
>> bd957x_probe() to the generic device property API as well.
> 
> With or without below addressed,
> Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
> 
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
>>
>> diff --git a/drivers/regulator/bd9576-regulator.c b/drivers/regulator/bd9576-regulator.c
>> index aa42da4d141e..393c8693b327 100644
>> --- a/drivers/regulator/bd9576-regulator.c
>> +++ b/drivers/regulator/bd9576-regulator.c
>> @@ -12,6 +12,7 @@
>>   #include <linux/module.h>
>>   #include <linux/of.h>
>>   #include <linux/platform_device.h>
>> +#include <linux/property.h>
>>   #include <linux/regulator/driver.h>
>>   #include <linux/regulator/machine.h>
>>   #include <linux/regulator/of_regulator.h>
>> @@ -939,8 +940,8 @@ static int bd957x_probe(struct platform_device *pdev)
>>          }
>>
>>          ic_data->regmap = regmap;
>> -       vout_mode = of_property_read_bool(pdev->dev.parent->of_node,
>> -                                        "rohm,vout1-en-low");
>> +       vout_mode = device_property_read_bool(pdev->dev.parent,
>> +                                             "rohm,vout1-en-low");
> 
> They all using parent device and you may make code neater by adding
> 
>    struct device *parent = pdev->dev.parent;

This is a matter of personal preference. I prefer seeing 
pdev->dev.parent - as it is more obvious (to me) what the 'pdev' is than 
what 'parent' would be.

I'd use the local variable only when it shortens at least one of the 
lines so that we avoid splitting it. After that being said - I'm not 
going to argue over this change either if one who is improving the 
driver wants to use the "helper" variable here.

Best Regards
	-- Matti


-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~