| Message ID | 20250316145514.627-1-sergio@pereznus.es (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: light: bh1750: Add hardware reset support via GPIO | expand |
On 16/03/2025 15:55, Sergio Perez wrote: > Some BH1750 sensors require a hardware reset before they can be > detected on the I2C bus. This patch adds support for an optional > reset GPIO that can be specified in the device tree. > > The reset sequence pulls the GPIO low and then high before > initializing the sensor, which enables proper detection with > tools like i2cdetect. > > Update the devicetree binding documentation to include the new > reset-gpios property with examples. > > Signed-off-by: Sergio Perez <sergio@pereznus.es> Please run scripts/checkpatch.pl and fix reported warnings. After that, run also `scripts/checkpatch.pl --strict` and (probably) fix more warnings. Some warnings can be ignored, especially from --strict run, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. <form letter> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time. Please kindly resend and include all necessary To/Cc entries. </form letter> > --- > .../devicetree/bindings/iio/light/bh1750.yaml | 20 +++- > drivers/iio/light/bh1750.c | 113 ++++++++++++------ ... and please go through your patch and see what happened there. > 2 files changed, 95 insertions(+), 38 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml > index 1a88b3c253d5..d53b221eb84b 100644 > --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml > +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml > @@ -11,6 +11,9 @@ maintainers: > > description: | > Ambient light sensor with an i2c interface. > + > + Some BH1750 sensors require a hardware reset before being properly detected > + on the I2C bus. This can be done using the optional reset-gpios property. > > properties: > compatible: > @@ -23,6 +26,10 @@ properties: > > reg: > maxItems: 1 > + > + reset-gpios: > + description: GPIO connected to the sensor's reset line (active low) > + maxItems: 1 > > required: > - compatible > @@ -41,5 +48,16 @@ examples: > reg = <0x23>; > }; > }; > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + light-sensor@23 { > + compatible = "rohm,bh1750"; > + reg = <0x23>; > + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; > + }; > + }; > > -... > +... > \ No newline at end of file You have unrelated changed all over the place. Best regards, Krzysztof
On Sun, 16 Mar 2025 15:55:13 +0100 Sergio Perez <sergio@pereznus.es> wrote: > Some BH1750 sensors require a hardware reset before they can be > detected on the I2C bus. This patch adds support for an optional > reset GPIO that can be specified in the device tree. > > The reset sequence pulls the GPIO low and then high before > initializing the sensor, which enables proper detection with > tools like i2cdetect. > > Update the devicetree binding documentation to include the new > reset-gpios property with examples. > > Signed-off-by: Sergio Perez <sergio@pereznus.es> > --- > .../devicetree/bindings/iio/light/bh1750.yaml | 20 +++- > drivers/iio/light/bh1750.c | 113 ++++++++++++------ > 2 files changed, 95 insertions(+), 38 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml > index 1a88b3c253d5..d53b221eb84b 100644 > --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml > +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml > @@ -11,6 +11,9 @@ maintainers: > > description: | > Ambient light sensor with an i2c interface. > + > + Some BH1750 sensors require a hardware reset before being properly detected > + on the I2C bus. This can be done using the optional reset-gpios property. I don't think this detail belongs here. In general we are going to reset them all if we have the GPIO. > > properties: > compatible: > @@ -23,6 +26,10 @@ properties: > > reg: > maxItems: 1 > + > + reset-gpios: > + description: GPIO connected to the sensor's reset line (active low) > + maxItems: 1 > > required: > - compatible > @@ -41,5 +48,16 @@ examples: > reg = <0x23>; > }; > }; > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + light-sensor@23 { > + compatible = "rohm,bh1750"; > + reg = <0x23>; > + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; Add the GPIO to the existing example rather than having a new one. > + }; > + }; > > -... > +... > \ No newline at end of file > diff --git a/drivers/iio/light/bh1750.c b/drivers/iio/light/bh1750.c > index 4b869fa9e5b1..53d64b70c03f 100644 > --- a/drivers/iio/light/bh1750.c > +++ b/drivers/iio/light/bh1750.c > @@ -22,11 +22,16 @@ > #include <linux/iio/iio.h> > #include <linux/iio/sysfs.h> > #include <linux/module.h> > +#include <linux/gpio/consumer.h> > +#include <linux/of.h> As already pointed out, there is a lot of accidental stuff in here. I'll review again once that is sorted out. For now I'll ignore it. If I weren't on a train and bored, I'd probably just have waited for v2. > > -#define BH1750_POWER_DOWN 0x00 > -#define BH1750_ONE_TIME_H_RES_MODE 0x20 /* auto-mode for BH1721 */ > -#define BH1750_CHANGE_INT_TIME_H_BIT 0x40 > -#define BH1750_CHANGE_INT_TIME_L_BIT 0x60 > +#define BH1750_POWER_DOWN 0x00 > +#define BH1750_ONE_TIME_H_RES_MODE 0x20 /* auto-mode for BH1721 */ > +#define BH1750_CHANGE_INT_TIME_H_BIT 0x40 > +#define BH1750_CHANGE_INT_TIME_L_BIT 0x60 > + > +/* Define the reset delay time in microseconds */ > +#define BH1750_RESET_DELAY_US 10000 /* 10ms */ > > enum { > BH1710, > @@ -40,6 +45,7 @@ struct bh1750_data { > struct mutex lock; > const struct bh1750_chip_info *chip_info; > u16 mtreg; > + struct gpio_desc *reset_gpio; > }; > > struct bh1750_chip_info { > @@ -62,11 +68,26 @@ struct bh1750_chip_info { > > +static int bh1750_reset(struct bh1750_data *data) > +{ > + if (!data->reset_gpio) No need to check outside and in here. > + return 0; /* No GPIO configured for reset, continue */ > + > + /* Perform reset sequence: low-high */ > + gpiod_set_value_cansleep(data->reset_gpio, 0); > + usleep_range(BH1750_RESET_DELAY_US, BH1750_RESET_DELAY_US + 1000); fsleep for cases like this where is approximately but greater than X usecs. > + gpiod_set_value_cansleep(data->reset_gpio, 1); > + usleep_range(BH1750_RESET_DELAY_US, BH1750_RESET_DELAY_US + 1000); fsleep > + > + dev_info(&data->client->dev, "BH1750 reset completed via GPIO\n"); Too noisy. dev_dbg at most for something like this. > + return 0; > +} > @@ -248,6 +266,19 @@ static int bh1750_probe(struct i2c_client *client) > data->client = client; > data->chip_info = &bh1750_chip_info_tbl[id->driver_data]; > > + data->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH); > + if (IS_ERR(data->reset_gpio)) { > + ret = PTR_ERR(data->reset_gpio); > + dev_err(&client->dev, "Failed to get reset GPIO: %d\n", ret); > + return ret; Use return dev_err_probe(). In general good to have because of pretty printing errors messages etc, but in this case you might get a deferral request and that call adds a bunch of debug info for probe deferal. > + } > + > + if (data->reset_gpio) { > + ret = bh1750_reset(data); There isn't a lot going on in that function, so I'd pull all the code down here and not bother with a function at all. > + if (ret < 0) > + return ret; > + } > +
On 17/03/2025 12:58, Jonathan Cameron wrote: >> + reset-gpios: >> + description: GPIO connected to the sensor's reset line (active low) >> + maxItems: 1 >> >> required: >> - compatible >> @@ -41,5 +48,16 @@ examples: >> reg = <0x23>; >> }; >> }; >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + light-sensor@23 { >> + compatible = "rohm,bh1750"; >> + reg = <0x23>; >> + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; > Add the GPIO to the existing example rather than having a new one. ... and it is weird to see you claim here is ACTIVE_HIGH but in description you say active low. It's possible, but unlikely. More likely this is just wrong, so use proper flag. Best regards, Krzysztof
On 18/03/2025 14:16, Sergio Perez Alegre wrote:
> El 17 mar. 2025, a las 8:24, Krzysztof Kozlowski <krzk@kernel.org> escribieron: On 16/03/2025 15:55, Sergio Perez wrote: Some BH1750 sensors require a hardware reset before they can be detected on the I2C bus. This patch adds support for an optional reset GPIO that can be specified in the device tree. The reset sequence pulls the GPIO low and then high before initializing the sensor, which enables proper detection with tools like i2cdetect. Update the devicetree binding documentation to include the new reset-gpios property with examples. Signed-off-by: Sergio Perez <sergio@pereznus.es> Please run scripts/checkpatch.pl and fix reported warnings. After that, run also `scripts/checkpatch.pl --strict` and (probably) fix more warnings. Some warnings can be ignored, especially from --strict run, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. <form letter> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time. Please kindly resend and include all necessary To/Cc entries. </form letter> Sorry, I had run the scripts/get_maintainer.pl tool and got fewer recipients than necessary. I have redone everything in a clean installation and now I have obtained more recipients. Any fixes I make in the patch I send to this same thread or should I send it with git send-mail? I say this because perhaps I have done it incorrectly and possibly created 3 versions, I apologize. My latest version (v3) includes all the suggestions mentioned but due to my ignorance of the procedure I thought they should be sent to the list again as before. --- .../devicetree/bindings/iio/light/bh1750.yaml | 20 +++- drivers/iio/light/bh1750.c | 113 ++++++++++++------ ... and please go through your patch and see what happened there. 2 files changed, 95 insertions(+), 38 deletions(-) diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml index 1a88b3c253d5..d53b221eb84b 100644 --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml @@ -11,6 +11,9 @@ maintainers: description: | Ambient light sensor with an i2c interface. + + Some BH1750 sensors require a hardware reset before being properly detected + on the I2C bus. This can be done using the optional reset-gpios property. properties: compatible: @@ -23,6 +26,10 @@ properties: reg: maxItems: 1 + + reset-gpios: + description: GPIO connected to the sensor's reset line (active low) + maxItems: 1 required: - compatible @@ -41,5 +48,16 @@ examples: reg = <0x23>; }; }; + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + light-sensor@23 { + compatible = "rohm,bh1750"; + reg = <0x23>; + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; + }; + }; -... +... \ No newline at end of file You have unrelated changed all over the place. Best regards, Krzysztof
Hm? No clue what this means, sorry, unreadable.
Best regards,
Krzysztof
Hello, El 17/03/2025 a las 8:24, Krzysztof Kozlowski escribió: > On 16/03/2025 15:55, Sergio Perez wrote: >> Some BH1750 sensors require a hardware reset before they can be >> detected on the I2C bus. This patch adds support for an optional >> reset GPIO that can be specified in the device tree. >> >> The reset sequence pulls the GPIO low and then high before >> initializing the sensor, which enables proper detection with >> tools like i2cdetect. >> >> Update the devicetree binding documentation to include the new >> reset-gpios property with examples. >> >> Signed-off-by: Sergio Perez <sergio@pereznus.es> > Please run scripts/checkpatch.pl and fix reported warnings. After that, > run also `scripts/checkpatch.pl --strict` and (probably) fix more > warnings. Some warnings can be ignored, especially from --strict run, > but the code here looks like it needs a fix. Feel free to get in touch > if the warning is not clear. > > <form letter> > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. > > Tools like b4 or scripts/get_maintainer.pl provide you proper list of > people, so fix your workflow. Tools might also fail if you work on some > ancient tree (don't, instead use mainline) or work on fork of kernel > (don't, instead use mainline). Just use b4 and everything should be > fine, although remember about `b4 prep --auto-to-cc` if you added new > patches to the patchset. > > You missed at least devicetree list (maybe more), so this won't be > tested by automated tooling. Performing review on untested code might be > a waste of time. > > Please kindly resend and include all necessary To/Cc entries. > </form letter> > Sorry, I had run the scripts/get_maintainer.pl tool and got fewer recipients than necessary. I have redone everything in a clean installation and now I have obtained more recipients. Any fixes I make in the patch I send to this same thread or should I send it with git send-mail? I say this because perhaps I have done it incorrectly and possibly created 3 versions, I apologize. My latest version (v3) includes all the suggestions mentioned but due to my ignorance of the procedure I thought they should be sent to the list again as before. Can I delete v2 and v3 and keep only the first version? >> --- >> .../devicetree/bindings/iio/light/bh1750.yaml | 20 +++- >> drivers/iio/light/bh1750.c | 113 ++++++++++++------ > > ... and please go through your patch and see what happened there. >> 2 files changed, 95 insertions(+), 38 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml >> index 1a88b3c253d5..d53b221eb84b 100644 >> --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml >> +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml >> @@ -11,6 +11,9 @@ maintainers: >> >> description: | >> Ambient light sensor with an i2c interface. >> + >> + Some BH1750 sensors require a hardware reset before being properly detected >> + on the I2C bus. This can be done using the optional reset-gpios property. >> >> properties: >> compatible: >> @@ -23,6 +26,10 @@ properties: >> >> reg: >> maxItems: 1 >> + >> + reset-gpios: >> + description: GPIO connected to the sensor's reset line (active low) >> + maxItems: 1 >> >> required: >> - compatible >> @@ -41,5 +48,16 @@ examples: >> reg = <0x23>; >> }; >> }; >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + light-sensor@23 { >> + compatible = "rohm,bh1750"; >> + reg = <0x23>; >> + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; >> + }; >> + }; >> >> -... >> +... >> \ No newline at end of file > You have unrelated changed all over the place. > > > Best regards, > Krzysztof Yes, in the patch I have prepared I have solved this problem, it only adds the exact lines and does not modify anything else.
El 17/03/2025 a las 12:58, Jonathan Cameron escribió: > On Sun, 16 Mar 2025 15:55:13 +0100 > Sergio Perez <sergio@pereznus.es> wrote: > >> Some BH1750 sensors require a hardware reset before they can be >> detected on the I2C bus. This patch adds support for an optional >> reset GPIO that can be specified in the device tree. >> >> The reset sequence pulls the GPIO low and then high before >> initializing the sensor, which enables proper detection with >> tools like i2cdetect. >> >> Update the devicetree binding documentation to include the new >> reset-gpios property with examples. >> >> Signed-off-by: Sergio Perez <sergio@pereznus.es> >> --- >> .../devicetree/bindings/iio/light/bh1750.yaml | 20 +++- >> drivers/iio/light/bh1750.c | 113 ++++++++++++------ >> 2 files changed, 95 insertions(+), 38 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml >> index 1a88b3c253d5..d53b221eb84b 100644 >> --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml >> +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml >> @@ -11,6 +11,9 @@ maintainers: >> >> description: | >> Ambient light sensor with an i2c interface. >> + >> + Some BH1750 sensors require a hardware reset before being properly detected >> + on the I2C bus. This can be done using the optional reset-gpios property. > I don't think this detail belongs here. In general we are going to reset > them all if we have the GPIO. > >> >> properties: >> compatible: >> @@ -23,6 +26,10 @@ properties: >> >> reg: >> maxItems: 1 >> + >> + reset-gpios: >> + description: GPIO connected to the sensor's reset line (active low) >> + maxItems: 1 >> >> required: >> - compatible >> @@ -41,5 +48,16 @@ examples: >> reg = <0x23>; >> }; >> }; >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + light-sensor@23 { >> + compatible = "rohm,bh1750"; >> + reg = <0x23>; >> + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; > Add the GPIO to the existing example rather than having a new one. Yes, I am going to change it. >> + }; >> + }; >> >> -... >> +... >> \ No newline at end of file >> diff --git a/drivers/iio/light/bh1750.c b/drivers/iio/light/bh1750.c >> index 4b869fa9e5b1..53d64b70c03f 100644 >> --- a/drivers/iio/light/bh1750.c >> +++ b/drivers/iio/light/bh1750.c >> @@ -22,11 +22,16 @@ >> #include <linux/iio/iio.h> >> #include <linux/iio/sysfs.h> >> #include <linux/module.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/of.h> > As already pointed out, there is a lot of accidental stuff in here. > > I'll review again once that is sorted out. For now I'll ignore it. > If I weren't on a train and bored, I'd probably just have waited for v2. > > >> >> -#define BH1750_POWER_DOWN 0x00 >> -#define BH1750_ONE_TIME_H_RES_MODE 0x20 /* auto-mode for BH1721 */ >> -#define BH1750_CHANGE_INT_TIME_H_BIT 0x40 >> -#define BH1750_CHANGE_INT_TIME_L_BIT 0x60 >> +#define BH1750_POWER_DOWN 0x00 >> +#define BH1750_ONE_TIME_H_RES_MODE 0x20 /* auto-mode for BH1721 */ >> +#define BH1750_CHANGE_INT_TIME_H_BIT 0x40 >> +#define BH1750_CHANGE_INT_TIME_L_BIT 0x60 >> + >> +/* Define the reset delay time in microseconds */ >> +#define BH1750_RESET_DELAY_US 10000 /* 10ms */ >> >> enum { >> BH1710, >> @@ -40,6 +45,7 @@ struct bh1750_data { >> struct mutex lock; >> const struct bh1750_chip_info *chip_info; >> u16 mtreg; >> + struct gpio_desc *reset_gpio; >> }; >> >> struct bh1750_chip_info { >> @@ -62,11 +68,26 @@ struct bh1750_chip_info { >> >> +static int bh1750_reset(struct bh1750_data *data) >> +{ >> + if (!data->reset_gpio) > No need to check outside and in here. > >> + return 0; /* No GPIO configured for reset, continue */ >> + >> + /* Perform reset sequence: low-high */ >> + gpiod_set_value_cansleep(data->reset_gpio, 0); >> + usleep_range(BH1750_RESET_DELAY_US, BH1750_RESET_DELAY_US + 1000); > fsleep for cases like this where is approximately but greater than X usecs. > >> + gpiod_set_value_cansleep(data->reset_gpio, 1); >> + usleep_range(BH1750_RESET_DELAY_US, BH1750_RESET_DELAY_US + 1000); > fsleep >> + >> + dev_info(&data->client->dev, "BH1750 reset completed via GPIO\n"); > Too noisy. dev_dbg at most for something like this. > >> + return 0; >> +} > >> @@ -248,6 +266,19 @@ static int bh1750_probe(struct i2c_client *client) >> data->client = client; >> data->chip_info = &bh1750_chip_info_tbl[id->driver_data]; >> >> + data->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH); >> + if (IS_ERR(data->reset_gpio)) { >> + ret = PTR_ERR(data->reset_gpio); >> + dev_err(&client->dev, "Failed to get reset GPIO: %d\n", ret); >> + return ret; > Use return dev_err_probe(). In general good to have because of pretty printing > errors messages etc, but in this case you might get a deferral request and > that call adds a bunch of debug info for probe deferal. > >> + } >> + >> + if (data->reset_gpio) { >> + ret = bh1750_reset(data); > There isn't a lot going on in that function, so I'd pull all the code down > here and not bother with a function at all. >> + if (ret < 0) >> + return ret; >> + } >> + Thanks for the suggestions, I rewrote the code with all these points, I passed the tests again and everything seems correct. From 2493b60b924fad787098ab2e3f1b9ba9a9a649c1 Mon Sep 17 00:00:00 2001 From: Sergio Perez <sergio@pereznus.es> Date: Tue, 18 Mar 2025 01:12:05 +0100 Subject: [PATCH] iio: light: bh1750: Add hardware reset support via GPIO Some BH1750 sensors require a hardware reset before they can be detected on the I2C bus. This patch adds support for an optional reset GPIO that can be specified in the device tree. The reset sequence pulls the GPIO low and then high before initializing the sensor, which enables proper detection with tools like i2cdetect. Update the devicetree binding documentation to include the new reset-gpios property with examples. Signed-off-by: Sergio Perez <sergio@pereznus.es> --- .../devicetree/bindings/iio/light/bh1750.yaml | 5 +++++ drivers/iio/light/bh1750.c | 22 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml index 1a88b3c253d5..f7a8dcd7d2a1 100644 --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml @@ -24,6 +24,10 @@ properties: reg: maxItems: 1 + reset-gpios: + description: GPIO connected to the sensor's reset line (active low) + maxItems: 1 + required: - compatible - reg @@ -39,6 +43,7 @@ examples: light-sensor@23 { compatible = "rohm,bh1750"; reg = <0x23>; + reset-gpios = <&gpio2 17 0>; }; }; diff --git a/drivers/iio/light/bh1750.c b/drivers/iio/light/bh1750.c index 4b869fa9e5b1..b88ce92acbc6 100644 --- a/drivers/iio/light/bh1750.c +++ b/drivers/iio/light/bh1750.c @@ -22,12 +22,16 @@ #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> #include <linux/module.h> +#include <linux/gpio/consumer.h> #define BH1750_POWER_DOWN 0x00 #define BH1750_ONE_TIME_H_RES_MODE 0x20 /* auto-mode for BH1721 */ #define BH1750_CHANGE_INT_TIME_H_BIT 0x40 #define BH1750_CHANGE_INT_TIME_L_BIT 0x60 +/* Define the reset delay time in microseconds */ +#define BH1750_RESET_DELAY_US 10000 /* 10ms */ + enum { BH1710, BH1721, @@ -40,6 +44,7 @@ struct bh1750_data { struct mutex lock; const struct bh1750_chip_info *chip_info; u16 mtreg; + struct gpio_desc *reset_gpio; }; struct bh1750_chip_info { @@ -248,6 +253,23 @@ static int bh1750_probe(struct i2c_client *client) data->client = client; data->chip_info = &bh1750_chip_info_tbl[id->driver_data]; + /* Get reset GPIO from device tree */ + data->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(data->reset_gpio)) + return dev_err_probe(&client->dev, PTR_ERR(data->reset_gpio), + "Failed to get reset GPIO\n"); + + /* Perform hardware reset if GPIO is provided */ + if (data->reset_gpio) { + /* Perform reset sequence: low-high */ + gpiod_set_value_cansleep(data->reset_gpio, 0); + fsleep(BH1750_RESET_DELAY_US); + gpiod_set_value_cansleep(data->reset_gpio, 1); + fsleep(BH1750_RESET_DELAY_US); + + dev_dbg(&client->dev, "BH1750 reset completed via GPIO\n"); + } + usec = data->chip_info->mtreg_to_usec * data->chip_info->mtreg_default; ret = bh1750_change_int_time(data, usec); if (ret < 0)
On 18/03/2025 15:16, Sergio Pérez wrote: > Hello, > > El 17/03/2025 a las 8:24, Krzysztof Kozlowski escribió: >> On 16/03/2025 15:55, Sergio Perez wrote: >>> Some BH1750 sensors require a hardware reset before they can be >>> detected on the I2C bus. This patch adds support for an optional >>> reset GPIO that can be specified in the device tree. >>> >>> The reset sequence pulls the GPIO low and then high before >>> initializing the sensor, which enables proper detection with >>> tools like i2cdetect. >>> >>> Update the devicetree binding documentation to include the new >>> reset-gpios property with examples. >>> >>> Signed-off-by: Sergio Perez <sergio@pereznus.es> >> Please run scripts/checkpatch.pl and fix reported warnings. After that, >> run also `scripts/checkpatch.pl --strict` and (probably) fix more >> warnings. Some warnings can be ignored, especially from --strict run, >> but the code here looks like it needs a fix. Feel free to get in touch >> if the warning is not clear. You keep ignoring paragraphs. Did you read this? >> >> <form letter> >> Please use scripts/get_maintainers.pl to get a list of necessary people >> and lists to CC. It might happen, that command when run on an older >> kernel, gives you outdated entries. Therefore please be sure you base >> your patches on recent Linux kernel. >> >> Tools like b4 or scripts/get_maintainer.pl provide you proper list of >> people, so fix your workflow. Tools might also fail if you work on some >> ancient tree (don't, instead use mainline) or work on fork of kernel >> (don't, instead use mainline). Just use b4 and everything should be >> fine, although remember about `b4 prep --auto-to-cc` if you added new >> patches to the patchset. >> >> You missed at least devicetree list (maybe more), so this won't be >> tested by automated tooling. Performing review on untested code might be >> a waste of time. >> >> Please kindly resend and include all necessary To/Cc entries. >> </form letter> >> > > Sorry, I had run the scripts/get_maintainer.pl tool and got fewer > recipients than necessary. I have redone everything in a clean > installation and now I have obtained more recipients. Please work on latest mainline tree, not some old clones. The cleanness of tree does not matter here. > > Any fixes I make in the patch I send to this same thread or should I > send it with git send-mail? I say this because perhaps I have done it > incorrectly and possibly created 3 versions, I apologize. My latest > version (v3) includes all the suggestions mentioned but due to my Not sure, maybe some, but not all. You still did not acknowledge the first feedback I repeated here and v3 makes the same mistake. > ignorance of the procedure I thought they should be sent to the list > again as before. Can I delete v2 and v3 and keep only the first version? You cannot delete things sent to people. That's why you should check things prior sending. Everything you send is archives and available for everyone, publicly. > >>> --- >>> .../devicetree/bindings/iio/light/bh1750.yaml | 20 +++- >>> drivers/iio/light/bh1750.c | 113 ++++++++++++------ >> >> ... and please go through your patch and see what happened there. >>> 2 files changed, 95 insertions(+), 38 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml >>> index 1a88b3c253d5..d53b221eb84b 100644 >>> --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml >>> +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml >>> @@ -11,6 +11,9 @@ maintainers: >>> >>> description: | >>> Ambient light sensor with an i2c interface. >>> + >>> + Some BH1750 sensors require a hardware reset before being properly detected >>> + on the I2C bus. This can be done using the optional reset-gpios property. >>> >>> properties: >>> compatible: >>> @@ -23,6 +26,10 @@ properties: >>> >>> reg: >>> maxItems: 1 >>> + >>> + reset-gpios: >>> + description: GPIO connected to the sensor's reset line (active low) >>> + maxItems: 1 >>> >>> required: >>> - compatible >>> @@ -41,5 +48,16 @@ examples: >>> reg = <0x23>; >>> }; >>> }; >>> + - | >>> + i2c { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + light-sensor@23 { >>> + compatible = "rohm,bh1750"; >>> + reg = <0x23>; >>> + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; >>> + }; >>> + }; >>> >>> -... >>> +... >>> \ No newline at end of file >> You have unrelated changed all over the place. >> >> >> Best regards, >> Krzysztof > > Yes, in the patch I have prepared I have solved this problem, it only > adds the exact lines and does not modify anything else. OK, so this one is solved, what about all the rest? > Best regards, Krzysztof
El 18/03/2025 a las 16:16, Krzysztof Kozlowski escribió: > On 18/03/2025 15:16, Sergio Pérez wrote: >> Hello, >> >> El 17/03/2025 a las 8:24, Krzysztof Kozlowski escribió: >>> On 16/03/2025 15:55, Sergio Perez wrote: >>>> Some BH1750 sensors require a hardware reset before they can be >>>> detected on the I2C bus. This patch adds support for an optional >>>> reset GPIO that can be specified in the device tree. >>>> >>>> The reset sequence pulls the GPIO low and then high before >>>> initializing the sensor, which enables proper detection with >>>> tools like i2cdetect. >>>> >>>> Update the devicetree binding documentation to include the new >>>> reset-gpios property with examples. >>>> >>>> Signed-off-by: Sergio Perez <sergio@pereznus.es> >>> Please run scripts/checkpatch.pl and fix reported warnings. After that, >>> run also `scripts/checkpatch.pl --strict` and (probably) fix more >>> warnings. Some warnings can be ignored, especially from --strict run, >>> but the code here looks like it needs a fix. Feel free to get in touch >>> if the warning is not clear. > You keep ignoring paragraphs. Did you read this? I pass this check several times and every time I do any step to make sure I am well. scripts/checkpatch.pl -f drivers/iio/light/bh1750.c total: 0 errors, 0 warnings, 354 lines checked drivers/iio/light/bh1750.c has no obvious style problems and is ready for submission. (patches) sergio@ultron:/mnt/c/Users/sergio/source/pr/linux$ scripts/checkpatch.pl --strict -f drivers/iio/light/bh1750.c and: scripts/checkpatch.pl --strict -f drivers/iio/light/bh1750.c CHECK: struct mutex definition without comment #44: FILE: drivers/iio/light/bh1750.c:44: + struct mutex lock; CHECK: Alignment should match open parenthesis #194: FILE: drivers/iio/light/bh1750.c:194: +static ssize_t bh1750_show_int_time_available(struct device *dev, + struct device_attribute *attr, char *buf) total: 0 errors, 0 warnings, 2 checks, 354 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. drivers/iio/light/bh1750.c has style problems, please review. NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. The two CHECKS on lines 44 and 194 are not part of my contribution, but I can solve them if you want. Do I solve them too? I thought that I should not modify anything that exists. > >>> <form letter> >>> Please use scripts/get_maintainers.pl to get a list of necessary people >>> and lists to CC. It might happen, that command when run on an older >>> kernel, gives you outdated entries. Therefore please be sure you base >>> your patches on recent Linux kernel. >>> >>> Tools like b4 or scripts/get_maintainer.pl provide you proper list of >>> people, so fix your workflow. Tools might also fail if you work on some >>> ancient tree (don't, instead use mainline) or work on fork of kernel >>> (don't, instead use mainline). Just use b4 and everything should be >>> fine, although remember about `b4 prep --auto-to-cc` if you added new >>> patches to the patchset. >>> >>> You missed at least devicetree list (maybe more), so this won't be >>> tested by automated tooling. Performing review on untested code might be >>> a waste of time. >>> >>> Please kindly resend and include all necessary To/Cc entries. >>> </form letter> >>> >> Sorry, I had run the scripts/get_maintainer.pl tool and got fewer >> recipients than necessary. I have redone everything in a clean >> installation and now I have obtained more recipients. > Please work on latest mainline tree, not some old clones. The cleanness > of tree does not matter here. I get the latest versión doing: git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git I checked versión doing: $ make kernelversion 6.14.0-rc7 The list I got is where I'm sending the patch, 9 recipients. The same ones to whom I have sent this email. > >> Any fixes I make in the patch I send to this same thread or should I >> send it with git send-mail? I say this because perhaps I have done it >> incorrectly and possibly created 3 versions, I apologize. My latest >> version (v3) includes all the suggestions mentioned but due to my > Not sure, maybe some, but not all. You still did not acknowledge the > first feedback I repeated here and v3 makes the same mistake. > > >> ignorance of the procedure I thought they should be sent to the list >> again as before. Can I delete v2 and v3 and keep only the first version? > You cannot delete things sent to people. That's why you should check > things prior sending. Everything you send is archives and available for > everyone, publicly. > >>>> --- >>>> .../devicetree/bindings/iio/light/bh1750.yaml | 20 +++- >>>> drivers/iio/light/bh1750.c | 113 ++++++++++++------ >>> ... and please go through your patch and see what happened there. >>>> 2 files changed, 95 insertions(+), 38 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml >>>> index 1a88b3c253d5..d53b221eb84b 100644 >>>> --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml >>>> +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml >>>> @@ -11,6 +11,9 @@ maintainers: >>>> >>>> description: | >>>> Ambient light sensor with an i2c interface. >>>> + >>>> + Some BH1750 sensors require a hardware reset before being properly detected >>>> + on the I2C bus. This can be done using the optional reset-gpios property. >>>> >>>> properties: >>>> compatible: >>>> @@ -23,6 +26,10 @@ properties: >>>> >>>> reg: >>>> maxItems: 1 >>>> + >>>> + reset-gpios: >>>> + description: GPIO connected to the sensor's reset line (active low) >>>> + maxItems: 1 >>>> >>>> required: >>>> - compatible >>>> @@ -41,5 +48,16 @@ examples: >>>> reg = <0x23>; >>>> }; >>>> }; >>>> + - | >>>> + i2c { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + light-sensor@23 { >>>> + compatible = "rohm,bh1750"; >>>> + reg = <0x23>; >>>> + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; >>>> + }; >>>> + }; >>>> >>>> -... >>>> +... >>>> \ No newline at end of file >>> You have unrelated changed all over the place. >>> >>> >>> Best regards, >>> Krzysztof >> Yes, in the patch I have prepared I have solved this problem, it only >> adds the exact lines and does not modify anything else. > OK, so this one is solved, what about all the rest? I think I have everything right, just tell me if you prefer me to solve the 2 strict checks of the existing code and I will do it. > > > Best regards, > Krzysztof
On 18/03/2025 17:06, Sergio Pérez wrote: > > El 18/03/2025 a las 16:16, Krzysztof Kozlowski escribió: >> On 18/03/2025 15:16, Sergio Pérez wrote: >>> Hello, >>> >>> El 17/03/2025 a las 8:24, Krzysztof Kozlowski escribió: >>>> On 16/03/2025 15:55, Sergio Perez wrote: >>>>> Some BH1750 sensors require a hardware reset before they can be >>>>> detected on the I2C bus. This patch adds support for an optional >>>>> reset GPIO that can be specified in the device tree. >>>>> >>>>> The reset sequence pulls the GPIO low and then high before >>>>> initializing the sensor, which enables proper detection with >>>>> tools like i2cdetect. >>>>> >>>>> Update the devicetree binding documentation to include the new >>>>> reset-gpios property with examples. >>>>> >>>>> Signed-off-by: Sergio Perez <sergio@pereznus.es> >>>> Please run scripts/checkpatch.pl and fix reported warnings. After that, >>>> run also `scripts/checkpatch.pl --strict` and (probably) fix more >>>> warnings. Some warnings can be ignored, especially from --strict run, >>>> but the code here looks like it needs a fix. Feel free to get in touch >>>> if the warning is not clear. >> You keep ignoring paragraphs. Did you read this? > > I pass this check several times and every time I do any step to make > sure I am well. > > scripts/checkpatch.pl -f drivers/iio/light/bh1750.c > total: 0 errors, 0 warnings, 354 lines checked That's not how you run checkpatch. Read the submitting patches. Just like the name tells you, check the patch, you run it on the patch. Best regards, Krzysztof
On 18/03/2025 17:21, Krzysztof Kozlowski wrote: > On 18/03/2025 17:06, Sergio Pérez wrote: >> >> El 18/03/2025 a las 16:16, Krzysztof Kozlowski escribió: >>> On 18/03/2025 15:16, Sergio Pérez wrote: >>>> Hello, >>>> >>>> El 17/03/2025 a las 8:24, Krzysztof Kozlowski escribió: >>>>> On 16/03/2025 15:55, Sergio Perez wrote: >>>>>> Some BH1750 sensors require a hardware reset before they can be >>>>>> detected on the I2C bus. This patch adds support for an optional >>>>>> reset GPIO that can be specified in the device tree. >>>>>> >>>>>> The reset sequence pulls the GPIO low and then high before >>>>>> initializing the sensor, which enables proper detection with >>>>>> tools like i2cdetect. >>>>>> >>>>>> Update the devicetree binding documentation to include the new >>>>>> reset-gpios property with examples. >>>>>> >>>>>> Signed-off-by: Sergio Perez <sergio@pereznus.es> >>>>> Please run scripts/checkpatch.pl and fix reported warnings. After that, >>>>> run also `scripts/checkpatch.pl --strict` and (probably) fix more >>>>> warnings. Some warnings can be ignored, especially from --strict run, >>>>> but the code here looks like it needs a fix. Feel free to get in touch >>>>> if the warning is not clear. >>> You keep ignoring paragraphs. Did you read this? >> >> I pass this check several times and every time I do any step to make >> sure I am well. >> >> scripts/checkpatch.pl -f drivers/iio/light/bh1750.c >> total: 0 errors, 0 warnings, 354 lines checked > > > That's not how you run checkpatch. Read the submitting patches. Just > like the name tells you, check the patch, you run it on the patch. BTW, I wonder which guideline told you to run it on the file? Because checkpatch description and submitting patches tell about running it on the patches, so I wonder where did you get suggestion to run it like that? Best regards, Krzysztof
El 18/03/2025 a las 17:23, Krzysztof Kozlowski escribió: > On 18/03/2025 17:21, Krzysztof Kozlowski wrote: >> On 18/03/2025 17:06, Sergio Pérez wrote: >>> El 18/03/2025 a las 16:16, Krzysztof Kozlowski escribió: >>>> On 18/03/2025 15:16, Sergio Pérez wrote: >>>>> Hello, >>>>> >>>>> El 17/03/2025 a las 8:24, Krzysztof Kozlowski escribió: >>>>>> On 16/03/2025 15:55, Sergio Perez wrote: >>>>>>> Some BH1750 sensors require a hardware reset before they can be >>>>>>> detected on the I2C bus. This patch adds support for an optional >>>>>>> reset GPIO that can be specified in the device tree. >>>>>>> >>>>>>> The reset sequence pulls the GPIO low and then high before >>>>>>> initializing the sensor, which enables proper detection with >>>>>>> tools like i2cdetect. >>>>>>> >>>>>>> Update the devicetree binding documentation to include the new >>>>>>> reset-gpios property with examples. >>>>>>> >>>>>>> Signed-off-by: Sergio Perez <sergio@pereznus.es> >>>>>> Please run scripts/checkpatch.pl and fix reported warnings. After that, >>>>>> run also `scripts/checkpatch.pl --strict` and (probably) fix more >>>>>> warnings. Some warnings can be ignored, especially from --strict run, >>>>>> but the code here looks like it needs a fix. Feel free to get in touch >>>>>> if the warning is not clear. >>>> You keep ignoring paragraphs. Did you read this? >>> I pass this check several times and every time I do any step to make >>> sure I am well. >>> >>> scripts/checkpatch.pl -f drivers/iio/light/bh1750.c >>> total: 0 errors, 0 warnings, 354 lines checked >> >> That's not how you run checkpatch. Read the submitting patches. Just >> like the name tells you, check the patch, you run it on the patch. > BTW, I wonder which guideline told you to run it on the file? Because > checkpatch description and submitting patches tell about running it on > the patches, so I wonder where did you get suggestion to run it like that? You're absolutely right. I misunderstood how to use checkpatch.pl and was incorrectly running it on the source files instead of the patch file. Thank you for pointing this out. $ scripts/checkpatch.pl --strict -f 0001-iio-light-bh1750-Add-hardware-reset-support-via-GPIO.patch total: 0 errors, 0 warnings, 0 checks, 102 lines checked 0001-iio-light-bh1750-Add-hardware-reset-support-via-GPIO.patch has no obvious style problems and is ready for submission. I've now run the tool correctly on my patch file and have fixed the identified issues: - Removed trailing whitespace - Fixed lines exceeding 79 characters - Fixed the inconsistency between the description and example for reset-gpios - Modified the existing example instead of adding a new one - Ensured proper line endings and formatting - Used proper get_maintainers.pl to include all recipients Thank you for your patience as I learn the proper kernel contribution process. > > Best regards, > Krzysztof From 0b0f8f0f7988eccb6b4ed42984c5bde092b1d5fe Mon Sep 17 00:00:00 2001 From: Sergio Perez <sergio@pereznus.es> Date: Tue, 18 Mar 2025 01:12:05 +0100 Subject: [PATCH] iio: light: bh1750: Add hardware reset support via GPIO Some BH1750 sensors require a hardware reset before they can be detected on the I2C bus. This patch adds support for an optional reset GPIO that can be specified in the device tree. The reset sequence pulls the GPIO low and then high before initializing the sensor, which enables proper detection with tools like i2cdetect. Update the devicetree binding documentation to include the new reset-gpios property with examples. Signed-off-by: Sergio Perez <sergio@pereznus.es> --- .../devicetree/bindings/iio/light/bh1750.yaml | 5 ++++ drivers/iio/light/bh1750.c | 23 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml index 1a88b3c253d5..f7a8dcd7d2a1 100644 --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml @@ -24,6 +24,10 @@ properties: reg: maxItems: 1 + reset-gpios: + description: GPIO connected to the sensor's reset line (active low) + maxItems: 1 + required: - compatible - reg @@ -39,6 +43,7 @@ examples: light-sensor@23 { compatible = "rohm,bh1750"; reg = <0x23>; + reset-gpios = <&gpio2 17 0>; }; }; diff --git a/drivers/iio/light/bh1750.c b/drivers/iio/light/bh1750.c index 4b869fa9e5b1..cfcce0c73aa2 100644 --- a/drivers/iio/light/bh1750.c +++ b/drivers/iio/light/bh1750.c @@ -22,12 +22,16 @@ #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> #include <linux/module.h> +#include <linux/gpio/consumer.h> #define BH1750_POWER_DOWN 0x00 #define BH1750_ONE_TIME_H_RES_MODE 0x20 /* auto-mode for BH1721 */ #define BH1750_CHANGE_INT_TIME_H_BIT 0x40 #define BH1750_CHANGE_INT_TIME_L_BIT 0x60 +/* Define the reset delay time in microseconds */ +#define BH1750_RESET_DELAY_US 10000 /* 10ms */ + enum { BH1710, BH1721, @@ -40,6 +44,7 @@ struct bh1750_data { struct mutex lock; const struct bh1750_chip_info *chip_info; u16 mtreg; + struct gpio_desc *reset_gpio; }; struct bh1750_chip_info { @@ -248,6 +253,24 @@ static int bh1750_probe(struct i2c_client *client) data->client = client; data->chip_info = &bh1750_chip_info_tbl[id->driver_data]; + /* Get reset GPIO from device tree */ + data->reset_gpio = devm_gpiod_get_optional(&client->dev, + "reset", GPIOD_OUT_HIGH); + if (IS_ERR(data->reset_gpio)) + return dev_err_probe(&client->dev, PTR_ERR(data->reset_gpio), + "Failed to get reset GPIO\n"); + + /* Perform hardware reset if GPIO is provided */ + if (data->reset_gpio) { + /* Perform reset sequence: low-high */ + gpiod_set_value_cansleep(data->reset_gpio, 0); + fsleep(BH1750_RESET_DELAY_US); + gpiod_set_value_cansleep(data->reset_gpio, 1); + fsleep(BH1750_RESET_DELAY_US); + + dev_dbg(&client->dev, "BH1750 reset completed via GPIO\n"); + } + usec = data->chip_info->mtreg_to_usec * data->chip_info->mtreg_default; ret = bh1750_change_int_time(data, usec); if (ret < 0) -- 2.43.0
On 18/03/2025 18:26, Sergio Pérez wrote: > > El 18/03/2025 a las 17:23, Krzysztof Kozlowski escribió: >> On 18/03/2025 17:21, Krzysztof Kozlowski wrote: >>> On 18/03/2025 17:06, Sergio Pérez wrote: >>>> El 18/03/2025 a las 16:16, Krzysztof Kozlowski escribió: >>>>> On 18/03/2025 15:16, Sergio Pérez wrote: >>>>>> Hello, >>>>>> >>>>>> El 17/03/2025 a las 8:24, Krzysztof Kozlowski escribió: >>>>>>> On 16/03/2025 15:55, Sergio Perez wrote: >>>>>>>> Some BH1750 sensors require a hardware reset before they can be >>>>>>>> detected on the I2C bus. This patch adds support for an optional >>>>>>>> reset GPIO that can be specified in the device tree. >>>>>>>> >>>>>>>> The reset sequence pulls the GPIO low and then high before >>>>>>>> initializing the sensor, which enables proper detection with >>>>>>>> tools like i2cdetect. >>>>>>>> >>>>>>>> Update the devicetree binding documentation to include the new >>>>>>>> reset-gpios property with examples. >>>>>>>> >>>>>>>> Signed-off-by: Sergio Perez <sergio@pereznus.es> >>>>>>> Please run scripts/checkpatch.pl and fix reported warnings. After that, >>>>>>> run also `scripts/checkpatch.pl --strict` and (probably) fix more >>>>>>> warnings. Some warnings can be ignored, especially from --strict run, >>>>>>> but the code here looks like it needs a fix. Feel free to get in touch >>>>>>> if the warning is not clear. >>>>> You keep ignoring paragraphs. Did you read this? >>>> I pass this check several times and every time I do any step to make >>>> sure I am well. >>>> >>>> scripts/checkpatch.pl -f drivers/iio/light/bh1750.c >>>> total: 0 errors, 0 warnings, 354 lines checked >>> >>> That's not how you run checkpatch. Read the submitting patches. Just >>> like the name tells you, check the patch, you run it on the patch. >> BTW, I wonder which guideline told you to run it on the file? Because >> checkpatch description and submitting patches tell about running it on >> the patches, so I wonder where did you get suggestion to run it like that? > You're absolutely right. I misunderstood how to use checkpatch.pl and > was incorrectly running it on the source files instead of the patch > file. Thank you for pointing this out. > > $ scripts/checkpatch.pl --strict -f No '-f'. Don't use such argument. Almost never. Read the help: " -f Treat FILE as a regular source file. This option must be used when running checkpatch on source files in the kernel." so why do you want a patch file to be a regular source file? How would it ever work? > 0001-iio-light-bh1750-Add-hardware-reset-support-via-GPIO.patch > total: 0 errors, 0 warnings, 0 checks, 102 lines checked > > 0001-iio-light-bh1750-Add-hardware-reset-support-via-GPIO.patch has no > obvious style problems and is ready for submission. You have clear examples how to run it inside: https://docs.kernel.org/dev-tools/checkpatch.html "./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,BRACES" So: checkpatch.pl mypatch > > I've now run the tool correctly on my patch file and have fixed the > identified issues: > - Removed trailing whitespace > - Fixed lines exceeding 79 characters > - Fixed the inconsistency between the description and example for > reset-gpios > - Modified the existing example instead of adding a new one > - Ensured proper line endings and formatting > - Used proper get_maintainers.pl to include all recipients > Please read the guides carefully. The process is extremely simple as: git add ... git commit --signed-off git format-patch -v3 -2 scripts/chekpatch.pl v3* scripts/get_maintainers.pl --no-git-fallback v3* git send-email * (or just use my git_send_email for last two) (or just use b4 for last four) The burden of reading the contributing guides is on you. We documented all this on purpose, so we will not have to repeat this on every email. [1] https://github.com/search?q=repo%3Akrzk%2Ftools%20git_send_email&type=code Best regards, Krzysztof
El 18/03/2025 a las 18:37, Krzysztof Kozlowski escribió: > On 18/03/2025 18:26, Sergio Pérez wrote: >> El 18/03/2025 a las 17:23, Krzysztof Kozlowski escribió: >>> On 18/03/2025 17:21, Krzysztof Kozlowski wrote: >>>> On 18/03/2025 17:06, Sergio Pérez wrote: >>>>> El 18/03/2025 a las 16:16, Krzysztof Kozlowski escribió: >>>>>> On 18/03/2025 15:16, Sergio Pérez wrote: >>>>>>> Hello, >>>>>>> >>>>>>> El 17/03/2025 a las 8:24, Krzysztof Kozlowski escribió: >>>>>>>> On 16/03/2025 15:55, Sergio Perez wrote: >>>>>>>>> Some BH1750 sensors require a hardware reset before they can be >>>>>>>>> detected on the I2C bus. This patch adds support for an optional >>>>>>>>> reset GPIO that can be specified in the device tree. >>>>>>>>> >>>>>>>>> The reset sequence pulls the GPIO low and then high before >>>>>>>>> initializing the sensor, which enables proper detection with >>>>>>>>> tools like i2cdetect. >>>>>>>>> >>>>>>>>> Update the devicetree binding documentation to include the new >>>>>>>>> reset-gpios property with examples. >>>>>>>>> >>>>>>>>> Signed-off-by: Sergio Perez <sergio@pereznus.es> >>>>>>>> Please run scripts/checkpatch.pl and fix reported warnings. After that, >>>>>>>> run also `scripts/checkpatch.pl --strict` and (probably) fix more >>>>>>>> warnings. Some warnings can be ignored, especially from --strict run, >>>>>>>> but the code here looks like it needs a fix. Feel free to get in touch >>>>>>>> if the warning is not clear. >>>>>> You keep ignoring paragraphs. Did you read this? >>>>> I pass this check several times and every time I do any step to make >>>>> sure I am well. >>>>> >>>>> scripts/checkpatch.pl -f drivers/iio/light/bh1750.c >>>>> total: 0 errors, 0 warnings, 354 lines checked >>>> That's not how you run checkpatch. Read the submitting patches. Just >>>> like the name tells you, check the patch, you run it on the patch. >>> BTW, I wonder which guideline told you to run it on the file? Because >>> checkpatch description and submitting patches tell about running it on >>> the patches, so I wonder where did you get suggestion to run it like that? >> You're absolutely right. I misunderstood how to use checkpatch.pl and >> was incorrectly running it on the source files instead of the patch >> file. Thank you for pointing this out. >> >> $ scripts/checkpatch.pl --strict -f > No '-f'. Don't use such argument. Almost never. Read the help: > > " -f > Treat FILE as a regular source file. This option must be used when > running checkpatch on source files in the kernel." > > so why do you want a patch file to be a regular source file? How would > it ever work? > >> 0001-iio-light-bh1750-Add-hardware-reset-support-via-GPIO.patch >> total: 0 errors, 0 warnings, 0 checks, 102 lines checked >> >> 0001-iio-light-bh1750-Add-hardware-reset-support-via-GPIO.patch has no >> obvious style problems and is ready for submission. > You have clear examples how to run it inside: > > https://docs.kernel.org/dev-tools/checkpatch.html > > "./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,BRACES" > > So: > checkpatch.pl mypatch > >> I've now run the tool correctly on my patch file and have fixed the >> identified issues: >> - Removed trailing whitespace >> - Fixed lines exceeding 79 characters >> - Fixed the inconsistency between the description and example for >> reset-gpios >> - Modified the existing example instead of adding a new one >> - Ensured proper line endings and formatting >> - Used proper get_maintainers.pl to include all recipients >> > Please read the guides carefully. The process is extremely simple as: > > git add ... > git commit --signed-off > git format-patch -v3 -2 > scripts/chekpatch.pl v3* > scripts/get_maintainers.pl --no-git-fallback v3* > git send-email * > (or just use my git_send_email for last two) > (or just use b4 for last four) > > The burden of reading the contributing guides is on you. We documented > all this on purpose, so we will not have to repeat this on every email. Thank you very much, I have followed the instructions with total precision and carefully read all the documentation. I have submitted both patches as v4 version. Apologies for the inconvenience. > > [1] > https://github.com/search?q=repo%3Akrzk%2Ftools%20git_send_email&type=code > > Best regards, > Krzysztof
On 18/03/2025 18:37, Krzysztof Kozlowski wrote: >> >> I've now run the tool correctly on my patch file and have fixed the >> identified issues: >> - Removed trailing whitespace >> - Fixed lines exceeding 79 characters >> - Fixed the inconsistency between the description and example for >> reset-gpios >> - Modified the existing example instead of adding a new one >> - Ensured proper line endings and formatting >> - Used proper get_maintainers.pl to include all recipients >> > > Please read the guides carefully. The process is extremely simple as: > > git add ... > git commit --signed-off > git format-patch -v3 -2 > scripts/chekpatch.pl v3* > scripts/get_maintainers.pl --no-git-fallback v3* > git send-email * Please read this again. I gave you detailed instruction which you still decided not to follow. The instructions are quite precise on purpose, because other method leads to wrong patchset - broken that or other way. > (or just use my git_send_email for last two) > (or just use b4 for last four) > > The burden of reading the contributing guides is on you. We documented > all this on purpose, so we will not have to repeat this on every email. > > > [1] > https://github.com/search?q=repo%3Akrzk%2Ftools%20git_send_email&type=code > > Best regards, > Krzysztof Best regards, Krzysztof
El 19/03/2025 a las 9:46, Krzysztof Kozlowski escribió: > On 18/03/2025 18:37, Krzysztof Kozlowski wrote: >>> I've now run the tool correctly on my patch file and have fixed the >>> identified issues: >>> - Removed trailing whitespace >>> - Fixed lines exceeding 79 characters >>> - Fixed the inconsistency between the description and example for >>> reset-gpios >>> - Modified the existing example instead of adding a new one >>> - Ensured proper line endings and formatting >>> - Used proper get_maintainers.pl to include all recipients >>> >> Please read the guides carefully. The process is extremely simple as: >> >> git add ... >> git commit --signed-off >> git format-patch -v3 -2 >> scripts/chekpatch.pl v3* >> scripts/get_maintainers.pl --no-git-fallback v3* >> git send-email * > Please read this again. I gave you detailed instruction which you still > decided not to follow. The instructions are quite precise on purpose, > because other method leads to wrong patchset - broken that or other way. > I transcribe exactly the commands I have executed: $ git add Documentation/devicetree/bindings/iio/light/bh1750.yaml $ git commit --signed-off $ git add drivers/iio/light/bh1750.c $ git commit --signed-off $ git format-patch -v3 -2 $ scripts/checkpatch.pl v3* --------------------------------------------------------------- v3-0001-dt-bindings-iio-light-bh1750-Add-reset-gpios-prop.patch --------------------------------------------------------------- total: 0 errors, 0 warnings, 17 lines checked v3-0001-dt-bindings-iio-light-bh1750-Add-reset-gpios-prop.patch has no obvious style problems and is ready for submission. --------------------------------------------------------------- v3-0002-iio-light-bh1750-Add-hardware-reset-support-via-G.patch --------------------------------------------------------------- total: 0 errors, 0 warnings, 47 lines checked v3-0002-iio-light-bh1750-Add-hardware-reset-support-via-G.patch has no obvious style problems and is ready for submission. $ scripts/get_maintainer.pl --no-git-fallback v3* Tomasz Duszynski <tduszyns@gmail.com> (maintainer:ROHM BH1750 AMBIENT LIGHT SENSOR DRIVER,in file) Jonathan Cameron <jic23@kernel.org> (maintainer:IIO SUBSYSTEM AND DRIVERS) Lars-Peter Clausen <lars@metafoo.de> (reviewer:IIO SUBSYSTEM AND DRIVERS) Rob Herring <robh@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) Krzysztof Kozlowski <krzk+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) Conor Dooley <conor+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) linux-iio@vger.kernel.org (open list:IIO SUBSYSTEM AND DRIVERS) devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) linux-kernel@vger.kernel.org (open list) $ git_send_email v3* v3-0001-dt-bindings-iio-light-bh1750-Add-reset-gpios-prop.patch v3-0002-iio-light-bh1750-Add-hardware-reset-support-via-G.patch (mbox) Adding cc: Sergio Perez <sergio@pereznus.es> from line 'From: Sergio Perez <sergio@pereznus.es>' (body) Adding cc: Sergio Perez <sergio@pereznus.es> from line 'Signed-off-by: Sergio Perez <sergio@pereznus.es>' From: Sergio Perez <sergio@pereznus.es> To: Tomasz Duszynski <tduszyns@gmail.com>, Jonathan Cameron <jic23@kernel.org>, Lars-Peter Clausen <lars@metafoo.de>, Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley <conor+dt@kernel.org>, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Sergio Perez <sergio@pereznus.es> Subject: [PATCH v3 1/2] dt-bindings: iio: light: bh1750: Add reset-gpios property Date: Wed, 19 Mar 2025 17:11:16 +0100 Message-ID: <20250319161117.1780-1-sergio@pereznus.es> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit The Cc list above has been expanded by additional addresses found in the patch commit message. By default send-email prompts before sending whenever this occurs. This behavior is controlled by the sendemail.confirm configuration setting. For additional information, run 'git send-email --help'. To retain the current behavior, but squelch this message, run 'git config --global sendemail.confirm auto'. Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y OK. Log says: Server: smtp.mail.me.com MAIL FROM:<sergio@pereznus.es> RCPT TO:<tduszyns@gmail.com> RCPT TO:<jic23@kernel.org> RCPT TO:<lars@metafoo.de> RCPT TO:<robh@kernel.org> RCPT TO:<krzk+dt@kernel.org> RCPT TO:<conor+dt@kernel.org> RCPT TO:<linux-iio@vger.kernel.org> RCPT TO:<devicetree@vger.kernel.org> RCPT TO:<linux-kernel@vger.kernel.org> RCPT TO:<sergio@pereznus.es> From: Sergio Perez <sergio@pereznus.es> To: Tomasz Duszynski <tduszyns@gmail.com>, Jonathan Cameron <jic23@kernel.org>, Lars-Peter Clausen <lars@metafoo.de>, Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley <conor+dt@kernel.org>, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Sergio Perez <sergio@pereznus.es> Subject: [PATCH v3 1/2] dt-bindings: iio: light: bh1750: Add reset-gpios property Date: Wed, 19 Mar 2025 17:11:16 +0100 Message-ID: <20250319161117.1780-1-sergio@pereznus.es> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Result: 250 (mbox) Adding cc: Sergio Perez <sergio@pereznus.es> from line 'From: Sergio Perez <sergio@pereznus.es>' (body) Adding cc: Sergio Perez <sergio@pereznus.es> from line 'Signed-off-by: Sergio Perez <sergio@pereznus.es>' From: Sergio Perez <sergio@pereznus.es> To: Tomasz Duszynski <tduszyns@gmail.com>, Jonathan Cameron <jic23@kernel.org>, Lars-Peter Clausen <lars@metafoo.de>, Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley <conor+dt@kernel.org>, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Sergio Perez <sergio@pereznus.es> Subject: [PATCH v3 2/2] iio: light: bh1750: Add hardware reset support via GPIO Date: Wed, 19 Mar 2025 17:11:17 +0100 Message-ID: <20250319161117.1780-2-sergio@pereznus.es> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250319161117.1780-1-sergio@pereznus.es> References: <20250319161117.1780-1-sergio@pereznus.es> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y OK. Log says: Server: smtp.mail.me.com MAIL FROM:<sergio@pereznus.es> RCPT TO:<tduszyns@gmail.com> RCPT TO:<jic23@kernel.org> RCPT TO:<lars@metafoo.de> RCPT TO:<robh@kernel.org> RCPT TO:<krzk+dt@kernel.org> RCPT TO:<conor+dt@kernel.org> RCPT TO:<linux-iio@vger.kernel.org> RCPT TO:<devicetree@vger.kernel.org> RCPT TO:<linux-kernel@vger.kernel.org> RCPT TO:<sergio@pereznus.es> From: Sergio Perez <sergio@pereznus.es> To: Tomasz Duszynski <tduszyns@gmail.com>, Jonathan Cameron <jic23@kernel.org>, Lars-Peter Clausen <lars@metafoo.de>, Rob Herring <robh@kernel.org>, Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley <conor+dt@kernel.org>, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Sergio Perez <sergio@pereznus.es> Subject: [PATCH v3 2/2] iio: light: bh1750: Add hardware reset support via GPIO Date: Wed, 19 Mar 2025 17:11:17 +0100 Message-ID: <20250319161117.1780-2-sergio@pereznus.es> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250319161117.1780-1-sergio@pereznus.es> References: <20250319161117.1780-1-sergio@pereznus.es> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Result: 250 >> (or just use my git_send_email for last two) >> (or just use b4 for last four) >> >> The burden of reading the contributing guides is on you. We documented >> all this on purpose, so we will not have to repeat this on every email. >> >> >> [1] >> https://github.com/search?q=repo%3Akrzk%2Ftools%20git_send_email&type=code >> >> Best regards, >> Krzysztof > > Best regards, > Krzysztof
On 18/03/2025 20:51, Sergio Pérez wrote: >> Please read the guides carefully. The process is extremely simple as: >> >> git add ... >> git commit --signed-off >> git format-patch -v3 -2 >> scripts/chekpatch.pl v3* >> scripts/get_maintainers.pl --no-git-fallback v3* >> git send-email * >> (or just use my git_send_email for last two) >> (or just use b4 for last four) >> >> The burden of reading the contributing guides is on you. We documented >> all this on purpose, so we will not have to repeat this on every email. > Thank you very much, I have followed the instructions with total > precision and carefully read all the documentation. I have submitted > both patches as v4 version. > Apologies for the inconvenience. No, you did not send proper v4. If you followed the process the v4 would be properly threaded, but it was not. What commands EXACTLY did you use? Best regards, Krzysztof
On 19/03/2025 17:26, Sergio Pérez wrote: > > El 19/03/2025 a las 9:46, Krzysztof Kozlowski escribió: >> On 18/03/2025 18:37, Krzysztof Kozlowski wrote: >>>> I've now run the tool correctly on my patch file and have fixed the >>>> identified issues: >>>> - Removed trailing whitespace >>>> - Fixed lines exceeding 79 characters >>>> - Fixed the inconsistency between the description and example for >>>> reset-gpios >>>> - Modified the existing example instead of adding a new one >>>> - Ensured proper line endings and formatting >>>> - Used proper get_maintainers.pl to include all recipients >>>> >>> Please read the guides carefully. The process is extremely simple as: >>> >>> git add ... >>> git commit --signed-off >>> git format-patch -v3 -2 >>> scripts/chekpatch.pl v3* >>> scripts/get_maintainers.pl --no-git-fallback v3* >>> git send-email * >> Please read this again. I gave you detailed instruction which you still >> decided not to follow. The instructions are quite precise on purpose, >> because other method leads to wrong patchset - broken that or other way. >> > I transcribe exactly the commands I have executed: > > $ git add Documentation/devicetree/bindings/iio/light/bh1750.yaml > > $ git commit --signed-off > > $ git add drivers/iio/light/bh1750.c > > $ git commit --signed-off > > $ git format-patch -v3 -2 So v3 or v4 or v5? After this email (dated 18th March), you sent v4, two hours later not following mentioned process. And then yesterday you sent v3 again, why this is supposed to be v5. Read the help of these commands, instead of copying them blindly without thinking. Every version is v3? Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/iio/light/bh1750.yaml b/Documentation/devicetree/bindings/iio/light/bh1750.yaml index 1a88b3c253d5..d53b221eb84b 100644 --- a/Documentation/devicetree/bindings/iio/light/bh1750.yaml +++ b/Documentation/devicetree/bindings/iio/light/bh1750.yaml @@ -11,6 +11,9 @@ maintainers: description: | Ambient light sensor with an i2c interface. + + Some BH1750 sensors require a hardware reset before being properly detected + on the I2C bus. This can be done using the optional reset-gpios property. properties: compatible: @@ -23,6 +26,10 @@ properties: reg: maxItems: 1 + + reset-gpios: + description: GPIO connected to the sensor's reset line (active low) + maxItems: 1 required: - compatible @@ -41,5 +48,16 @@ examples: reg = <0x23>; }; }; + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + light-sensor@23 { + compatible = "rohm,bh1750"; + reg = <0x23>; + reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; + }; + }; -... +... \ No newline at end of file diff --git a/drivers/iio/light/bh1750.c b/drivers/iio/light/bh1750.c index 4b869fa9e5b1..53d64b70c03f 100644 --- a/drivers/iio/light/bh1750.c +++ b/drivers/iio/light/bh1750.c @@ -22,11 +22,16 @@ #include <linux/iio/iio.h> #include <linux/iio/sysfs.h> #include <linux/module.h> +#include <linux/gpio/consumer.h> +#include <linux/of.h> -#define BH1750_POWER_DOWN 0x00 -#define BH1750_ONE_TIME_H_RES_MODE 0x20 /* auto-mode for BH1721 */ -#define BH1750_CHANGE_INT_TIME_H_BIT 0x40 -#define BH1750_CHANGE_INT_TIME_L_BIT 0x60 +#define BH1750_POWER_DOWN 0x00 +#define BH1750_ONE_TIME_H_RES_MODE 0x20 /* auto-mode for BH1721 */ +#define BH1750_CHANGE_INT_TIME_H_BIT 0x40 +#define BH1750_CHANGE_INT_TIME_L_BIT 0x60 + +/* Define the reset delay time in microseconds */ +#define BH1750_RESET_DELAY_US 10000 /* 10ms */ enum { BH1710, @@ -40,6 +45,7 @@ struct bh1750_data { struct mutex lock; const struct bh1750_chip_info *chip_info; u16 mtreg; + struct gpio_desc *reset_gpio; }; struct bh1750_chip_info { @@ -62,11 +68,26 @@ struct bh1750_chip_info { }; static const struct bh1750_chip_info bh1750_chip_info_tbl[] = { - [BH1710] = { 140, 1022, 300, 400, 250000000, 2, 0x001F, 0x03E0 }, - [BH1721] = { 140, 1020, 300, 400, 250000000, 2, 0x0010, 0x03E0 }, - [BH1750] = { 31, 254, 69, 1740, 57500000, 1, 0x001F, 0x00E0 }, + [BH1710] = {140, 1022, 300, 400, 250000000, 2, 0x001F, 0x03E0}, + [BH1721] = {140, 1020, 300, 400, 250000000, 2, 0x0010, 0x03E0}, + [BH1750] = {31, 254, 69, 1740, 57500000, 1, 0x001F, 0x00E0}, }; +static int bh1750_reset(struct bh1750_data *data) +{ + if (!data->reset_gpio) + return 0; /* No GPIO configured for reset, continue */ + + /* Perform reset sequence: low-high */ + gpiod_set_value_cansleep(data->reset_gpio, 0); + usleep_range(BH1750_RESET_DELAY_US, BH1750_RESET_DELAY_US + 1000); + gpiod_set_value_cansleep(data->reset_gpio, 1); + usleep_range(BH1750_RESET_DELAY_US, BH1750_RESET_DELAY_US + 1000); + + dev_info(&data->client->dev, "BH1750 reset completed via GPIO\n"); + return 0; +} + static int bh1750_change_int_time(struct bh1750_data *data, int usec) { int ret; @@ -87,13 +108,13 @@ static int bh1750_change_int_time(struct bh1750_data *data, int usec) regval = (val & chip_info->int_time_high_mask) >> 5; ret = i2c_smbus_write_byte(data->client, - BH1750_CHANGE_INT_TIME_H_BIT | regval); + BH1750_CHANGE_INT_TIME_H_BIT | regval); if (ret < 0) return ret; regval = val & chip_info->int_time_low_mask; ret = i2c_smbus_write_byte(data->client, - BH1750_CHANGE_INT_TIME_L_BIT | regval); + BH1750_CHANGE_INT_TIME_L_BIT | regval); if (ret < 0) return ret; @@ -129,8 +150,8 @@ static int bh1750_read(struct bh1750_data *data, int *val) } static int bh1750_read_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int *val, int *val2, long mask) + struct iio_chan_spec const *chan, + int *val, int *val2, long mask) { int ret, tmp; struct bh1750_data *data = iio_priv(indio_dev); @@ -165,8 +186,8 @@ static int bh1750_read_raw(struct iio_dev *indio_dev, } static int bh1750_write_raw(struct iio_dev *indio_dev, - struct iio_chan_spec const *chan, - int val, int val2, long mask) + struct iio_chan_spec const *chan, + int val, int val2, long mask) { int ret; struct bh1750_data *data = iio_priv(indio_dev); @@ -186,7 +207,7 @@ static int bh1750_write_raw(struct iio_dev *indio_dev, } static ssize_t bh1750_show_int_time_available(struct device *dev, - struct device_attribute *attr, char *buf) + struct device_attribute *attr, char *buf) { int i; size_t len = 0; @@ -195,7 +216,7 @@ static ssize_t bh1750_show_int_time_available(struct device *dev, for (i = chip_info->mtreg_min; i <= chip_info->mtreg_max; i += chip_info->inc) len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06d ", - chip_info->mtreg_to_usec * i); + chip_info->mtreg_to_usec * i); buf[len - 1] = '\n'; @@ -220,13 +241,10 @@ static const struct iio_info bh1750_info = { }; static const struct iio_chan_spec bh1750_channels[] = { - { - .type = IIO_LIGHT, - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | - BIT(IIO_CHAN_INFO_SCALE) | - BIT(IIO_CHAN_INFO_INT_TIME) - } -}; + {.type = IIO_LIGHT, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_INT_TIME)}}; static int bh1750_probe(struct i2c_client *client) { @@ -236,7 +254,7 @@ static int bh1750_probe(struct i2c_client *client) struct iio_dev *indio_dev; if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C | - I2C_FUNC_SMBUS_WRITE_BYTE)) + I2C_FUNC_SMBUS_WRITE_BYTE)) return -EOPNOTSUPP; indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); @@ -248,6 +266,19 @@ static int bh1750_probe(struct i2c_client *client) data->client = client; data->chip_info = &bh1750_chip_info_tbl[id->driver_data]; + data->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset", GPIOD_OUT_HIGH); + if (IS_ERR(data->reset_gpio)) { + ret = PTR_ERR(data->reset_gpio); + dev_err(&client->dev, "Failed to get reset GPIO: %d\n", ret); + return ret; + } + + if (data->reset_gpio) { + ret = bh1750_reset(data); + if (ret < 0) + return ret; + } + usec = data->chip_info->mtreg_to_usec * data->chip_info->mtreg_default; ret = bh1750_change_int_time(data, usec); if (ret < 0) @@ -295,23 +326,31 @@ static int bh1750_suspend(struct device *dev) static DEFINE_SIMPLE_DEV_PM_OPS(bh1750_pm_ops, bh1750_suspend, NULL); static const struct i2c_device_id bh1750_id[] = { - { "bh1710", BH1710 }, - { "bh1715", BH1750 }, - { "bh1721", BH1721 }, - { "bh1750", BH1750 }, - { "bh1751", BH1750 }, - { } -}; + {"bh1710", BH1710}, + {"bh1715", BH1750}, + {"bh1721", BH1721}, + {"bh1750", BH1750}, + {"bh1751", BH1750}, + {}}; MODULE_DEVICE_TABLE(i2c, bh1750_id); static const struct of_device_id bh1750_of_match[] = { - { .compatible = "rohm,bh1710", }, - { .compatible = "rohm,bh1715", }, - { .compatible = "rohm,bh1721", }, - { .compatible = "rohm,bh1750", }, - { .compatible = "rohm,bh1751", }, - { } -}; + { + .compatible = "rohm,bh1710", + }, + { + .compatible = "rohm,bh1715", + }, + { + .compatible = "rohm,bh1721", + }, + { + .compatible = "rohm,bh1750", + }, + { + .compatible = "rohm,bh1751", + }, + {}}; MODULE_DEVICE_TABLE(of, bh1750_of_match); static struct i2c_driver bh1750_driver = {
Some BH1750 sensors require a hardware reset before they can be detected on the I2C bus. This patch adds support for an optional reset GPIO that can be specified in the device tree. The reset sequence pulls the GPIO low and then high before initializing the sensor, which enables proper detection with tools like i2cdetect. Update the devicetree binding documentation to include the new reset-gpios property with examples. Signed-off-by: Sergio Perez <sergio@pereznus.es> --- .../devicetree/bindings/iio/light/bh1750.yaml | 20 +++- drivers/iio/light/bh1750.c | 113 ++++++++++++------ 2 files changed, 95 insertions(+), 38 deletions(-)