A set of ad-hoc cleanups with making driver to be used in e.g.,
ACPI envionment.
Andy Shevchenko (5):
w1: gpio: Make use of device properties
w1: gpio: Switch to use dev_err_probe()
w1: gpio: Use sizeof(*pointer) instead of sizeof(type)
w1: gpio: Remove duplicate NULL checks
w1: gpio: Don't use "proxy" headers
drivers/w1/masters/w1-gpio.c | 62 +++++++++++++++---------------------
1 file changed, 25 insertions(+), 37 deletions(-)
--
2.43.0.rc1.1.gbec44491f096
gpiod_set_value() is NULL-aware, no need to check that in the caller.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/w1/masters/w1-gpio.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 3881f2eaed2f..8fd9fedd8c56 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -117,8 +117,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
if (err)
return dev_err_probe(dev, err, "w1_add_master device failed\n");
- if (ddata->pullup_gpiod)
- gpiod_set_value(ddata->pullup_gpiod, 1);
+ gpiod_set_value(ddata->pullup_gpiod, 1);
platform_set_drvdata(pdev, master);
@@ -130,8 +129,7 @@ static void w1_gpio_remove(struct platform_device *pdev)
struct w1_bus_master *master = platform_get_drvdata(pdev);
struct w1_gpio_ddata *ddata = master->data;
- if (ddata->pullup_gpiod)
- gpiod_set_value(ddata->pullup_gpiod, 0);
+ gpiod_set_value(ddata->pullup_gpiod, 0);
w1_remove_master_device(master);
}
--
2.43.0.rc1.1.gbec44491f096
It is preferred to use sizeof(*pointer) instead of sizeof(type).
The type of the variable can change and one needs not change
the former (unlike the latter). No functional change intended.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/w1/masters/w1-gpio.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index a5ac34b32f54..3881f2eaed2f 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -85,8 +85,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
if (device_property_present(dev, "linux,open-drain"))
gflags = GPIOD_OUT_LOW;
- master = devm_kzalloc(dev, sizeof(struct w1_bus_master),
- GFP_KERNEL);
+ master = devm_kzalloc(dev, sizeof(*master), GFP_KERNEL);
if (!master)
return -ENOMEM;
--
2.43.0.rc1.1.gbec44491f096
Switch to use dev_err_probe() to simplify the error path and
unify a message template.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/w1/masters/w1-gpio.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 8ea53c757c99..a5ac34b32f54 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -91,18 +91,14 @@ static int w1_gpio_probe(struct platform_device *pdev)
return -ENOMEM;
ddata->gpiod = devm_gpiod_get_index(dev, NULL, 0, gflags);
- if (IS_ERR(ddata->gpiod)) {
- dev_err(dev, "gpio_request (pin) failed\n");
- return PTR_ERR(ddata->gpiod);
- }
+ if (IS_ERR(ddata->gpiod))
+ return dev_err_probe(dev, PTR_ERR(ddata->gpiod), "gpio_request (pin) failed\n");
ddata->pullup_gpiod =
devm_gpiod_get_index_optional(dev, NULL, 1, GPIOD_OUT_LOW);
- if (IS_ERR(ddata->pullup_gpiod)) {
- dev_err(dev, "gpio_request_one "
- "(ext_pullup_enable_pin) failed\n");
- return PTR_ERR(ddata->pullup_gpiod);
- }
+ if (IS_ERR(ddata->pullup_gpiod))
+ return dev_err_probe(dev, PTR_ERR(ddata->pullup_gpiod),
+ "gpio_request (ext_pullup_enable_pin) failed\n");
master->data = ddata;
master->read_bit = w1_gpio_read_bit;
@@ -119,10 +115,8 @@ static int w1_gpio_probe(struct platform_device *pdev)
master->set_pullup = w1_gpio_set_pullup;
err = w1_add_master_device(master);
- if (err) {
- dev_err(dev, "w1_add_master device failed\n");
- return err;
- }
+ if (err)
+ return dev_err_probe(dev, err, "w1_add_master device failed\n");
if (ddata->pullup_gpiod)
gpiod_set_value(ddata->pullup_gpiod, 1);
--
2.43.0.rc1.1.gbec44491f096
Hello Andy,
I wonder about your choice of recipients. I would have added Krzysztof
to To and me at most to Cc:.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Thu, Mar 07, 2024 at 04:35:48PM +0200, Andy Shevchenko wrote:
> Switch to use dev_err_probe() to simplify the error path and
> unify a message template.
This also has the upside that the error code is included in the message
and EPROBE_DEFER is correctly handled.
Acked-by: Uwe Kleine-K?nig <[email protected]>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Thu, Mar 07, 2024 at 04:35:49PM +0200, Andy Shevchenko wrote:
> It is preferred to use sizeof(*pointer) instead of sizeof(type).
> The type of the variable can change and one needs not change
> the former (unlike the latter). No functional change intended.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Acked-by: Uwe Kleine-K?nig <[email protected]>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Thu, Mar 07, 2024 at 05:38:54PM +0100, Uwe Kleine-K?nig wrote:
> Hello Andy,
>
> I wonder about your choice of recipients. I would have added Krzysztof
> to To and me at most to Cc:.
It's automatically generated using get_maintainers.pl.
See details in the source of the script [1] I'm using.
[1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
--
With Best Regards,
Andy Shevchenko
On Thu, Mar 07, 2024 at 04:35:50PM +0200, Andy Shevchenko wrote:
> gpiod_set_value() is NULL-aware, no need to check that in the caller.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/w1/masters/w1-gpio.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
> index 3881f2eaed2f..8fd9fedd8c56 100644
> --- a/drivers/w1/masters/w1-gpio.c
> +++ b/drivers/w1/masters/w1-gpio.c
> @@ -117,8 +117,7 @@ static int w1_gpio_probe(struct platform_device *pdev)
> if (err)
> return dev_err_probe(dev, err, "w1_add_master device failed\n");
>
> - if (ddata->pullup_gpiod)
> - gpiod_set_value(ddata->pullup_gpiod, 1);
> + gpiod_set_value(ddata->pullup_gpiod, 1);
>
> platform_set_drvdata(pdev, master);
>
> @@ -130,8 +129,7 @@ static void w1_gpio_remove(struct platform_device *pdev)
> struct w1_bus_master *master = platform_get_drvdata(pdev);
> struct w1_gpio_ddata *ddata = master->data;
>
> - if (ddata->pullup_gpiod)
> - gpiod_set_value(ddata->pullup_gpiod, 0);
> + gpiod_set_value(ddata->pullup_gpiod, 0);
This also has the advantage to not have to know that NULL is the dummy
value returned by the gpiod_get_optional() family of functions.
Acked-by: Uwe Kleine-K?nig <[email protected]>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello Andy,
On Thu, Mar 07, 2024 at 06:43:56PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 07, 2024 at 05:38:54PM +0100, Uwe Kleine-K?nig wrote:
> > I wonder about your choice of recipients. I would have added Krzysztof
> > to To and me at most to Cc:.
>
> It's automatically generated using get_maintainers.pl.
> See details in the source of the script [1] I'm using.
>
> [1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
Getting something wrong automatically isn't an excuse for getting it
wrong :-)
That scripts has:
to=$(git show -$count "$COMMIT" | scripts/get_maintainer.pl $OPTS --no-m --no-r)
cc=$(git show -$count "$COMMIT" | scripts/get_maintainer.pl $OPTS --no-l)
I recommend to swap the values for to and cc here to make sure you have
the maintainer in $to and the relevant lists in $cc.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Thu, Mar 07, 2024 at 06:12:46PM +0100, Uwe Kleine-K?nig wrote:
> On Thu, Mar 07, 2024 at 06:43:56PM +0200, Andy Shevchenko wrote:
> > On Thu, Mar 07, 2024 at 05:38:54PM +0100, Uwe Kleine-K?nig wrote:
..
> > > I wonder about your choice of recipients. I would have added Krzysztof
> > > to To and me at most to Cc:.
> >
> > It's automatically generated using get_maintainers.pl.
> > See details in the source of the script [1] I'm using.
> >
> > [1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
>
> Getting something wrong automatically isn't an excuse for getting it
> wrong :-)
I'm not sure why you think it's wrong. You worked on the code lately and Git
heuristics considered that over threshold of 67%.
> That scripts has:
>
> to=$(git show -$count "$COMMIT" | scripts/get_maintainer.pl $OPTS --no-m --no-r)
> cc=$(git show -$count "$COMMIT" | scripts/get_maintainer.pl $OPTS --no-l)
>
> I recommend to swap the values for to and cc here to make sure you have
> the maintainer in $to and the relevant lists in $cc.
Hmm... I don't remember why I put it this way.
Btw, you are the first one for the entire life cycle of that script (3 years?)
who complains about such details... So, patches are welcome! :-)
--
With Best Regards,
Andy Shevchenko
Update header inclusions to follow IWYU (Include What You Use)
principle.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/w1/masters/w1-gpio.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/w1/masters/w1-gpio.c b/drivers/w1/masters/w1-gpio.c
index 8fd9fedd8c56..a39fa8bf866a 100644
--- a/drivers/w1/masters/w1-gpio.c
+++ b/drivers/w1/masters/w1-gpio.c
@@ -5,15 +5,15 @@
* Copyright (C) 2007 Ville Syrjala <[email protected]>
*/
-#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/property.h>
-#include <linux/slab.h>
-#include <linux/gpio/consumer.h>
-#include <linux/err.h>
-#include <linux/delay.h>
+#include <linux/types.h>
#include <linux/w1.h>
--
2.43.0.rc1.1.gbec44491f096
Hello Andy,
On Thu, Mar 07, 2024 at 07:57:01PM +0200, Andy Shevchenko wrote:
> On Thu, Mar 07, 2024 at 06:12:46PM +0100, Uwe Kleine-K?nig wrote:
> > On Thu, Mar 07, 2024 at 06:43:56PM +0200, Andy Shevchenko wrote:
> > > On Thu, Mar 07, 2024 at 05:38:54PM +0100, Uwe Kleine-K?nig wrote:
>
> ...
>
> > > > I wonder about your choice of recipients. I would have added Krzysztof
> > > > to To and me at most to Cc:.
> > >
> > > It's automatically generated using get_maintainers.pl.
> > > See details in the source of the script [1] I'm using.
> > >
> > > [1]: https://github.com/andy-shev/home-bin-tools/blob/master/ge2maintainer.sh
> >
> > Getting something wrong automatically isn't an excuse for getting it
> > wrong :-)
>
> I'm not sure why you think it's wrong. You worked on the code lately and Git
> heuristics considered that over threshold of 67%.
When I send a patch I send it "to" the maintainers, because it's them
who I want an action from. This also matches the semantic of M: in
MAINTAINERS which requests to use these contaces in "To:".
> > That scripts has:
> >
> > to=$(git show -$count "$COMMIT" | scripts/get_maintainer.pl $OPTS --no-m --no-r)
> > cc=$(git show -$count "$COMMIT" | scripts/get_maintainer.pl $OPTS --no-l)
> >
> > I recommend to swap the values for to and cc here to make sure you have
> > the maintainer in $to and the relevant lists in $cc.
Thinking again, this is wrong. I'd recommend:
to=$(git show -$count "$COMMIT" | scripts/get_maintainer.pl $OPTS --no-r --no-l)
cc=$(git show -$count "$COMMIT" | scripts/get_maintainer.pl $OPTS --no-m)
> Btw, you are the first one for the entire life cycle of that script (3 years?)
> who complains about such details... So, patches are welcome! :-)
I won't do more than the above hint here.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On 07/03/2024 17:38, Uwe Kleine-König wrote:
> Hello Andy,
>
> I wonder about your choice of recipients. I would have added Krzysztof
> to To and me at most to Cc:.
That's indeed odd (and reminds me Qualcomm proposing wrapper over
get_maintainers.pl which on purpose put the maintainers in "To:" and
lists in "Cc:") and I imagine people having filters depending on To: and
Cc: distinction, but for the record: I don't care. People put it usually
wrong, so my filters just choose (To Or Cc).
Anyway all the patches will wait till the end of the merge window.
Best regards,
Krzysztof
On Fri, Mar 08, 2024 at 09:34:47AM +0100, Krzysztof Kozlowski wrote:
> On 07/03/2024 17:38, Uwe Kleine-K?nig wrote:
> > Hello Andy,
> >
> > I wonder about your choice of recipients. I would have added Krzysztof
> > to To and me at most to Cc:.
>
> That's indeed odd (and reminds me Qualcomm proposing wrapper over
> get_maintainers.pl which on purpose put the maintainers in "To:" and
> lists in "Cc:") and I imagine people having filters depending on To: and
> Cc: distinction, but for the record: I don't care. People put it usually
> wrong, so my filters just choose (To Or Cc).
>
> Anyway all the patches will wait till the end of the merge window.
Fine by me, thank you!
--
With Best Regards,
Andy Shevchenko
On Thu, 07 Mar 2024 16:35:46 +0200, Andy Shevchenko wrote:
> A set of ad-hoc cleanups with making driver to be used in e.g.,
> ACPI envionment.
>
> Andy Shevchenko (5):
> w1: gpio: Make use of device properties
> w1: gpio: Switch to use dev_err_probe()
> w1: gpio: Use sizeof(*pointer) instead of sizeof(type)
> w1: gpio: Remove duplicate NULL checks
> w1: gpio: Don't use "proxy" headers
>
> [...]
Applied, thanks!
[1/5] w1: gpio: Make use of device properties
https://git.kernel.org/krzk/linux-w1/c/8b39a723ef1fa3737e11832ca11183bbaeda2498
[2/5] w1: gpio: Switch to use dev_err_probe()
https://git.kernel.org/krzk/linux-w1/c/9e085c045868a6a727b3bd0fc7840ccc9e04d3a3
[3/5] w1: gpio: Use sizeof(*pointer) instead of sizeof(type)
https://git.kernel.org/krzk/linux-w1/c/ef2b810e1152d77686032e7dc064ff89b4350b00
[4/5] w1: gpio: Remove duplicate NULL checks
https://git.kernel.org/krzk/linux-w1/c/540d3f15c0aa2baf7e9b48a4e516391c179daab2
[5/5] w1: gpio: Don't use "proxy" headers
https://git.kernel.org/krzk/linux-w1/c/cde37a5bdb0ed2c4c7b86ef688e5fdb697525a57
Best regards,
--
Krzysztof Kozlowski <[email protected]>