Re: [EXT] Re: [PATCH 1/5] dt-bindings: media: i2c: Add bindings for OX05B1S sensor driver

[Mirela Rabulea <mirela.rabulea@nxp.com>]

Hi Laurent,

thanks for feedback.

On 29.10.2024 13:57, Laurent Pinchart wrote:
>>> +
>>> +  orientation: true
>>> +  rotation: true
>> I think you can drop both of these too.
> Aren't they needed given that the binding ends with
>
> additionalProperties: false
>
> ?
I added orientation & rotation properties in order to support 
orientation and rotation controls, libcamera warns about those (optional 
requirements last time I checked).
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
> The device requires a clock, shouldn't the clocks property be required ?

I intentionally left the clock optional, because NXP has a converter 
board which supports both ox05b1s and os08a20 sensor, and the converter 
board has an oscillator, and we are using that, not the SOC clock.

Here is how the module looks like for os08a20 for imx8mp:

https://docs.nxp.com/bundle/AN13712/page/topics/os08a20_sensor_module.html

There's a newer revision for the converter board for imx95, sorry but I 
do not have a link for that.

For imx8mp, we used in the past the clock from the SOC, then switched to 
the external clock (from the converter board).

I think Omnivision has their own module.

So, I thought leaving the clock as optional allows for more flexibility.

>
>>> +  - port
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/gpio/gpio.h>
>>> +
>>> +    i2c {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +
>>> +        ox05b1s: ox05b1s@36 {
>>> +            compatible = "ovti,ox05b1s";
>>> +            reg = <0x36>;
>>> +            reset-gpios = <&i2c3_gpio_expander_20 2 GPIO_ACTIVE_LOW>;
> This isn't specified in the bindings. Does the example validate ?

Apparently yes, I mean dt_binding_check passed:

$ rm Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.example.dtb

$ make dt_binding_check DT_CHECKER_FLAGS=-m 
DT_SCHEMA_FILES=ovti,ox05b1s.yaml
   DTC [C] 
Documentation/devicetree/bindings/media/i2c/ovti,ox05b1s.example.dtb

I have dtschema-2024.10.dev6+g12c3cd5.


The "reset-gpios" is described in this binding, as the GPIO connected to 
the XSHUTDOWN pin.

The <&i2c3_gpio_expander_20 2 GPIO_ACTIVE_LOW> is what works for imx95 
("nxp,pcal6408"), for imx8mp this works:

reset-gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;


Thanks,

Mirela