Re: [PATCH v3 1/2] dt-bindings: iio: light: bh1750: Add reset-gpios property

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxxx>
---
   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





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux