Re: [PATCH v2 1/2] dt-bindings: imx335: Mention reset-gpio polarity

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Umang,

On Mon, Jul 29, 2024 at 05:36:11PM +0530, Umang Jain wrote:
> On 29/07/24 4:40 pm, Laurent Pinchart wrote:
> > On Mon, Jul 29, 2024 at 04:34:36PM +0530, Umang Jain wrote:
> >> Mention the reset-gpio polarity in the device tree bindings.
> >> It is GPIO_ACTIVE_LOW according to the datasheet.
> >>
> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> >> ---
> >>   Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 2 ++
> >>   1 file changed, 2 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> >> index 106c36ee966d..fb4c9d42ed1c 100644
> >> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> >> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml
> >> @@ -92,6 +92,8 @@ examples:
> >>               ovdd-supply = <&camera_vddo_1v8>;
> >>               dvdd-supply = <&camera_vddd_1v2>;
> >>   
> >> +            reset-gpios = <&gpio 50 GPIO_ACTIVE_LOW>;
> >> +
> > I think it's good to include this in the example, but it doesn't match
> > the commit message. I was expecting to see a change to the binding
> > rules, not to the example.
> 
> Currently the binding already states reset-gpio as
> 
> ```
>    reset-gpios:
>      description: Reference to the GPIO connected to the XCLR pin, if any.
>      maxItems: 1
> ```
> 
> Pardon my limited knowledge here, do you mean something like :
> 
> ```
>    reset-gpios:
>      description: Reference to the GPIO connected to the XCLR pin 
> (active LOW), if any.
>      maxItems: 1
> ```
> 
> or something else?

No, I meant updating the commit message to something like:

dt-bindings: media: imx335: Add reset-gpios to the DT example

It's easy to get the polarity of GPIOs in the device tree wrong, as
shown by a recently fixed bug in the imx335 driver. To lower the chance
of future mistakes, especially in new bindings that would take the
imx335 binding as a starting point, add the reset-gpios property to the
DT example. This showcases the correct polarity of the XCLR signal for
Sony sensors in the most common case of the signal not being inverted on
the board.

> >>               port {
> >>                   imx335: endpoint {
> >>                       remote-endpoint = <&cam>;

-- 
Regards,

Laurent Pinchart