[PATCH v1 09/11] regulator: bd9576: switch to using devm_fwnode_gpiod_get()

Dmitry Torokhov posted 11 patches 3 years, 7 months ago
[PATCH v1 09/11] regulator: bd9576: switch to using devm_fwnode_gpiod_get()
Posted by Dmitry Torokhov 3 years, 7 months ago
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.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.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");
 	if (vout_mode) {
 		struct gpio_desc *en;
 
@@ -948,10 +949,10 @@ static int bd957x_probe(struct platform_device *pdev)
 
 		/* VOUT1 enable state judged by VOUT1_EN pin */
 		/* See if we have GPIO defined */
-		en = devm_gpiod_get_from_of_node(&pdev->dev,
-						 pdev->dev.parent->of_node,
-						 "rohm,vout1-en-gpios", 0,
-						 GPIOD_OUT_LOW, "vout1-en");
+		en = devm_fwnode_gpiod_get(&pdev->dev,
+					   dev_fwnode(pdev->dev.parent),
+					   "rohm,vout1-en", GPIOD_OUT_LOW,
+					   "vout1-en");
 		if (!IS_ERR(en)) {
 			/* VOUT1_OPS gpio ctrl */
 			/*
@@ -986,8 +987,8 @@ static int bd957x_probe(struct platform_device *pdev)
 	 * like DDR voltage selection.
 	 */
 	platform_set_drvdata(pdev, ic_data);
-	ddr_sel =  of_property_read_bool(pdev->dev.parent->of_node,
-					 "rohm,ddr-sel-low");
+	ddr_sel = device_property_read_bool(pdev->dev.parent,
+					    "rohm,ddr-sel-low");
 	if (ddr_sel)
 		ic_data->regulator_data[2].desc.fixed_uV = 1350000;
 	else

-- 
b4 0.10.0-dev-fc921
Re: [PATCH v1 09/11] regulator: bd9576: switch to using devm_fwnode_gpiod_get()
Posted by Andy Shevchenko 3 years, 7 months ago
On Mon, Sep 5, 2022 at 9:33 AM Dmitry Torokhov
<dmitry.torokhov@gmail.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@gmail.com>

> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.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;

at the definition block of the probe function.

>         if (vout_mode) {
>                 struct gpio_desc *en;
>
> @@ -948,10 +949,10 @@ static int bd957x_probe(struct platform_device *pdev)
>
>                 /* VOUT1 enable state judged by VOUT1_EN pin */
>                 /* See if we have GPIO defined */
> -               en = devm_gpiod_get_from_of_node(&pdev->dev,
> -                                                pdev->dev.parent->of_node,
> -                                                "rohm,vout1-en-gpios", 0,
> -                                                GPIOD_OUT_LOW, "vout1-en");
> +               en = devm_fwnode_gpiod_get(&pdev->dev,
> +                                          dev_fwnode(pdev->dev.parent),
> +                                          "rohm,vout1-en", GPIOD_OUT_LOW,
> +                                          "vout1-en");
>                 if (!IS_ERR(en)) {
>                         /* VOUT1_OPS gpio ctrl */
>                         /*
> @@ -986,8 +987,8 @@ static int bd957x_probe(struct platform_device *pdev)
>          * like DDR voltage selection.
>          */
>         platform_set_drvdata(pdev, ic_data);
> -       ddr_sel =  of_property_read_bool(pdev->dev.parent->of_node,
> -                                        "rohm,ddr-sel-low");
> +       ddr_sel = device_property_read_bool(pdev->dev.parent,
> +                                           "rohm,ddr-sel-low");
>         if (ddr_sel)
>                 ic_data->regulator_data[2].desc.fixed_uV = 1350000;
>         else
>
> --
> b4 0.10.0-dev-fc921



-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 09/11] regulator: bd9576: switch to using devm_fwnode_gpiod_get()
Posted by Matti Vaittinen 3 years, 7 months ago
On 9/5/22 13:40, Andy Shevchenko wrote:
> On Mon, Sep 5, 2022 at 9:33 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.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@gmail.com>
> 
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.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! ~~
Re: [PATCH v1 09/11] regulator: bd9576: switch to using devm_fwnode_gpiod_get()
Posted by Andy Shevchenko 3 years, 7 months ago
On Mon, Sep 5, 2022 at 4:19 PM Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> On 9/5/22 13:40, Andy Shevchenko wrote:
> > On Mon, Sep 5, 2022 at 9:33 AM Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:

...

> >> +       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.

And I believe the quoted one is exactly the case of what you are saying above.

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH v1 09/11] regulator: bd9576: switch to using devm_fwnode_gpiod_get()
Posted by Matti Vaittinen 3 years, 7 months ago
On 9/5/22 09:31, Dmitry Torokhov 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.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>


Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

Thanks!

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

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