| Message ID | 20250319161117.1780-1-sergio@pereznus.es (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v3,1/2] dt-bindings: iio: light: bh1750: Add reset-gpios property | expand |
On 19/03/2025 17:11, Sergio Perez wrote: > Some BH1750 sensors require a hardware reset via GPIO before they can > be properly detected on the I2C bus. Add a new reset-gpios property > to the binding to support this functionality. > > The reset-gpios property allows specifying a GPIO that will be toggled > during driver initialization to reset the sensor. > > Signed-off-by: Sergio Perez <sergio@pereznus.es> > --- > Documentation/devicetree/bindings/iio/light/bh1750.yaml | 5 +++++ > 1 file changed, 5 insertions(+) You just sent v3, while v4 was already on the lists, without improving and without responding to review. NAK. You keep repeating the same mistakes: not reading and responding feedback and it is getting tiresome. Best regards, Krzysztof
El 19/03/2025 a las 20:12, Krzysztof Kozlowski escribió: > On 19/03/2025 17:11, Sergio Perez wrote: >> Some BH1750 sensors require a hardware reset via GPIO before they can >> be properly detected on the I2C bus. Add a new reset-gpios property >> to the binding to support this functionality. >> >> The reset-gpios property allows specifying a GPIO that will be toggled >> during driver initialization to reset the sensor. >> >> Signed-off-by: Sergio Perez <sergio@pereznus.es> >> --- >> Documentation/devicetree/bindings/iio/light/bh1750.yaml | 5 +++++ >> 1 file changed, 5 insertions(+) > You just sent v3, while v4 was already on the lists, without improving > and without responding to review. > > NAK. > > You keep repeating the same mistakes: not reading and responding > feedback and it is getting tiresome. I apologize for the confusion with patch versions. You're right that I sent v3 after v4 was already on the list. I was trying to follow your exact instructions from: "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 *" Regarding the binding I've modified for next v5 the YAML description to remove "active low" to avoid confusion and modified the example to: examples: - | i2c { #address-cells = <1>; #size-cells = <0>; light-sensor@23 { compatible = "rohm,bh1750"; reg = <0x23>; reset-gpios = <&gpio2 17 GPIO_ACTIVE_HIGH>; }; }; That is the original version and is the configuration running on my machine. > Best regards, > Krzysztof
On Wed, Mar 19, 2025 at 11:38:09PM +0100, Sergio Pérez wrote: > > El 19/03/2025 a las 20:12, Krzysztof Kozlowski escribió: > > On 19/03/2025 17:11, Sergio Perez wrote: > > > Some BH1750 sensors require a hardware reset via GPIO before they can > > > be properly detected on the I2C bus. Add a new reset-gpios property > > > to the binding to support this functionality. > > > > > > The reset-gpios property allows specifying a GPIO that will be toggled > > > during driver initialization to reset the sensor. > > > > > > Signed-off-by: Sergio Perez <sergio@pereznus.es> > > > --- > > > Documentation/devicetree/bindings/iio/light/bh1750.yaml | 5 +++++ > > > 1 file changed, 5 insertions(+) > > You just sent v3, while v4 was already on the lists, without improving > > and without responding to review. > > > > NAK. > > > > You keep repeating the same mistakes: not reading and responding > > feedback and it is getting tiresome. > I apologize for the confusion with patch versions. You're right that I sent > v3 > after v4 was already on the list. I was trying to follow your exact > instructions from: > "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 *" v3 stands for version of the patch, so my instruction shuld be here adjusted. > > Regarding the binding I've modified for next v5 the YAML description to > remove "active low" to avoid confusion and modified the example to: So the signal is not active low? Are you really sure? Looking at BH1750FVI there is no reset signal in the first place... unless you mean this is DVI, but the description should then mention it. If this is DVI, then it is active low. Best regards, Krzysztof
El 20/03/2025 a las 9:52, Krzysztof Kozlowski escribió: > On Wed, Mar 19, 2025 at 11:38:09PM +0100, Sergio Pérez wrote: >> El 19/03/2025 a las 20:12, Krzysztof Kozlowski escribió: >>> On 19/03/2025 17:11, Sergio Perez wrote: >>>> Some BH1750 sensors require a hardware reset via GPIO before they can >>>> be properly detected on the I2C bus. Add a new reset-gpios property >>>> to the binding to support this functionality. >>>> >>>> The reset-gpios property allows specifying a GPIO that will be toggled >>>> during driver initialization to reset the sensor. >>>> >>>> Signed-off-by: Sergio Perez <sergio@pereznus.es> >>>> --- >>>> Documentation/devicetree/bindings/iio/light/bh1750.yaml | 5 +++++ >>>> 1 file changed, 5 insertions(+) >>> You just sent v3, while v4 was already on the lists, without improving >>> and without responding to review. >>> >>> NAK. >>> >>> You keep repeating the same mistakes: not reading and responding >>> feedback and it is getting tiresome. >> I apologize for the confusion with patch versions. You're right that I sent >> v3 >> after v4 was already on the list. I was trying to follow your exact >> instructions from: >> "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 *" > v3 stands for version of the patch, so my instruction shuld be here > adjusted. > >> Regarding the binding I've modified for next v5 the YAML description to >> remove "active low" to avoid confusion and modified the example to: > So the signal is not active low? Are you really sure? > > Looking at BH1750FVI there is no reset signal in the first place... > unless you mean this is DVI, but the description should then mention it. > > If this is DVI, then it is active low. I apologize for the confusion. You're completely right, and I misunderstood how the GPIO flags work in the kernel. I've now corrected my implementation to properly handle the active-low reset pin. Changes for v5: 1. In the binding YAML: - Updated description: "GPIO connected to the DVI reset pin (active low)" - Changed example to use GPIO_ACTIVE_LOW flag: reset-gpios = <&gpio2 17 GPIO_ACTIVE_LOW>; 2. In the driver code: - Corrected the reset sequence to properly handle active-low: ``` /* Perform reset sequence: active-deactive */ gpiod_set_value_cansleep(data->reset_gpio, 1); // Active reset (pin low) fsleep(BH1750_RESET_DELAY_US); gpiod_set_value_cansleep(data->reset_gpio, 0); // Deactivate reset (pin high) fsleep(BH1750_RESET_DELAY_US); ``` - Fixed indentation issues With these changes, the reset sequence correctly follows the datasheet requirements: pull the DVI pin low to reset, wait, then pull it high to resume normal operation. Thank you for your patience and guidance on this. > Best regards, > Krzysztof >
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>; }; };
Some BH1750 sensors require a hardware reset via GPIO before they can be properly detected on the I2C bus. Add a new reset-gpios property to the binding to support this functionality. The reset-gpios property allows specifying a GPIO that will be toggled during driver initialization to reset the sensor. Signed-off-by: Sergio Perez <sergio@pereznus.es> --- Documentation/devicetree/bindings/iio/light/bh1750.yaml | 5 +++++ 1 file changed, 5 insertions(+)