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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7cee3358-bf8c-4ae5-a688-12ff18d4b7e0@nxp.com>
Date: Wed, 30 Oct 2024 08:02:44 +0200
From: Mirela Rabulea <mirela.rabulea@....com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 Bryan O'Donoghue <bryan.odonoghue@...aro.org>
Cc: mchehab@...nel.org, sakari.ailus@...ux.intel.com,
 hverkuil-cisco@...all.nl, laurentiu.palcu@....com, robert.chiras@....com,
 linux-media@...r.kernel.org, linux-kernel@...r.kernel.org, LnxRevLi@....com,
 kieran.bingham@...asonboard.com, hdegoede@...hat.com,
 dave.stevenson@...pberrypi.com, mike.rudenko@...il.com,
 alain.volmat@...s.st.com, julien.vuillaumier@....com, alice.yuan@....com
Subject: Re: [EXT] Re: [PATCH 1/5] dt-bindings: media: i2c: Add bindings for
 OX05B1S sensor driver

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