[PATCH v3 0/4] Add QRB2210 RB1 vision mezzanine/kit support

[PATCH v3 0/4] Add QRB2210 RB1 vision mezzanine/kit support

[Loic Poulain]

This series enables initial camera functionality on QRB2210 RB1 with
Vision Mezzanine.It includes device tree updates for camera-related
components and a fix for the OV9282 sensor driver.

- Correct the reset GPIO logic to ensure proper reset behavior.
- Adds pinctrl configuration for the four camera master clocks (mclks).
- Adds the PM8008 camera PMIC node, which camera power management.
- Introduces an overlay enabling the Vision Mezzanine board with OV9282
  camera sensor support.

Changes in V2:
* Move mclk pinctrls to soc dtsi
* Ensure backward compatibility for ov9282 reset logic
* dts cleanup

Changes in V1:
* Use correct polarity for ov9282 pin
* Fix ov9282 reset pin logic
* Remove always-on from pm8008 regulators

Loic Poulain (4):
  media: i2c: ov9282: Fix reset-gpio logical state
  arm64: dts: qcom: qcm2290: Add pin configuration for mclks
  arm64: dts: qcom: qrb2210-rb1: Add PM8008 node
  arm64: dts: qcom: qrb2210-rb1: Add overlay for vision mezzanine

 arch/arm64/boot/dts/qcom/Makefile             |  5 ++
 arch/arm64/boot/dts/qcom/agatti.dtsi          | 28 +++++++
 .../qcom/qrb2210-rb1-vision-mezzanine.dtso    | 65 ++++++++++++++++
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts      | 75 +++++++++++++++++++
 drivers/media/i2c/ov9282.c                    | 26 ++++++-
 5 files changed, 195 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso

-- 
2.34.1

[PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Loic Poulain]

Ensure reset state is low in the power-on state and high in the
power-off state (assert reset). Note that the polarity is abstracted
by the GPIO subsystem, so the logic level reflects the intended reset
behavior.

To maintain backward compatibility with DTS files that use an incorrect
flag, we implement a mechanism similar to:
  commit 738455858a2d ("ASoC: codecs: wsa881x: Use proper shutdown GPIO polarity")

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/media/i2c/ov9282.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index a9f6176e9729..e79b326cdd94 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -160,6 +160,7 @@ struct ov9282_mode {
  * @sd: V4L2 sub-device
  * @pad: Media pad. Only one pad supported
  * @reset_gpio: Sensor reset gpio
+ * @reset_gpio_val: Logical value to reset the sensor
  * @inclk: Sensor input clock
  * @supplies: Regulator supplies for the sensor
  * @ctrl_handler: V4L2 control handler
@@ -180,6 +181,7 @@ struct ov9282 {
 	struct v4l2_subdev sd;
 	struct media_pad pad;
 	struct gpio_desc *reset_gpio;
+	unsigned int reset_gpio_val;
 	struct clk *inclk;
 	struct regulator_bulk_data supplies[OV9282_NUM_SUPPLIES];
 	struct v4l2_ctrl_handler ctrl_handler;
@@ -1127,13 +1129,29 @@ static int ov9282_parse_hw_config(struct ov9282 *ov9282)
 
 	/* Request optional reset pin */
 	ov9282->reset_gpio = devm_gpiod_get_optional(ov9282->dev, "reset",
-						     GPIOD_OUT_LOW);
+						     GPIOD_OUT_HIGH);
 	if (IS_ERR(ov9282->reset_gpio)) {
 		dev_err(ov9282->dev, "failed to get reset gpio %ld",
 			PTR_ERR(ov9282->reset_gpio));
 		return PTR_ERR(ov9282->reset_gpio);
 	}
 
+	/*
+	 * Backwards compatibility work-around.
+	 *
+	 * The reset GPIO is active-low, but the driver has always used the
+	 * gpiod API with inverted logic. As a result, the DTS had to
+	 * incorrectly mark the GPIO as active-high to compensate for this
+	 * behavior. Changing the flag in the driver now would break backward
+	 * compatibility with existing DTS configurations. To address this,
+	 * we add a simple value inversion so the driver works with both old
+	 * and new DTS.
+	 */
+	ov9282->reset_gpio_val = gpiod_is_active_low(ov9282->reset_gpio);
+	if (!ov9282->reset_gpio_val)
+		dev_warn(ov9282->dev, "Using ACTIVE_HIGH for reset GPIO. Your DTB might be outdated\n");
+	gpiod_set_value_cansleep(ov9282->reset_gpio, ov9282->reset_gpio_val);
+
 	/* Get sensor input clock */
 	ov9282->inclk = devm_v4l2_sensor_clk_get(ov9282->dev, NULL);
 	if (IS_ERR(ov9282->inclk))
@@ -1237,7 +1255,7 @@ static int ov9282_power_on(struct device *dev)
 
 	usleep_range(400, 600);
 
-	gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
+	gpiod_set_value_cansleep(ov9282->reset_gpio, !ov9282->reset_gpio_val);
 
 	ret = clk_prepare_enable(ov9282->inclk);
 	if (ret) {
@@ -1260,7 +1278,7 @@ static int ov9282_power_on(struct device *dev)
 error_clk:
 	clk_disable_unprepare(ov9282->inclk);
 error_reset:
-	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
+	gpiod_set_value_cansleep(ov9282->reset_gpio, ov9282->reset_gpio_val);
 
 	regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies);
 
@@ -1278,7 +1296,7 @@ static int ov9282_power_off(struct device *dev)
 	struct v4l2_subdev *sd = dev_get_drvdata(dev);
 	struct ov9282 *ov9282 = to_ov9282(sd);
 
-	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
+	gpiod_set_value_cansleep(ov9282->reset_gpio, ov9282->reset_gpio_val);
 
 	clk_disable_unprepare(ov9282->inclk);
 
-- 
2.34.1

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Sakari Ailus]

Hi Loic,

On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
> Ensure reset state is low in the power-on state and high in the
> power-off state (assert reset). Note that the polarity is abstracted
> by the GPIO subsystem, so the logic level reflects the intended reset
> behavior.

That's an interesting approach to fix DTS gone systematically wrong.

I was thinking of the drivers that have this issue, too, but I would have
introduced a new GPIO under a different name (many sensors use "enable",
too). Any thoughts?

Cc Laurent.

> 
> To maintain backward compatibility with DTS files that use an incorrect
> flag, we implement a mechanism similar to:
>   commit 738455858a2d ("ASoC: codecs: wsa881x: Use proper shutdown GPIO polarity")
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/media/i2c/ov9282.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index a9f6176e9729..e79b326cdd94 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -160,6 +160,7 @@ struct ov9282_mode {
>   * @sd: V4L2 sub-device
>   * @pad: Media pad. Only one pad supported
>   * @reset_gpio: Sensor reset gpio
> + * @reset_gpio_val: Logical value to reset the sensor
>   * @inclk: Sensor input clock
>   * @supplies: Regulator supplies for the sensor
>   * @ctrl_handler: V4L2 control handler
> @@ -180,6 +181,7 @@ struct ov9282 {
>  	struct v4l2_subdev sd;
>  	struct media_pad pad;
>  	struct gpio_desc *reset_gpio;
> +	unsigned int reset_gpio_val;
>  	struct clk *inclk;
>  	struct regulator_bulk_data supplies[OV9282_NUM_SUPPLIES];
>  	struct v4l2_ctrl_handler ctrl_handler;
> @@ -1127,13 +1129,29 @@ static int ov9282_parse_hw_config(struct ov9282 *ov9282)
>  
>  	/* Request optional reset pin */
>  	ov9282->reset_gpio = devm_gpiod_get_optional(ov9282->dev, "reset",
> -						     GPIOD_OUT_LOW);
> +						     GPIOD_OUT_HIGH);
>  	if (IS_ERR(ov9282->reset_gpio)) {
>  		dev_err(ov9282->dev, "failed to get reset gpio %ld",
>  			PTR_ERR(ov9282->reset_gpio));
>  		return PTR_ERR(ov9282->reset_gpio);
>  	}
>  
> +	/*
> +	 * Backwards compatibility work-around.
> +	 *
> +	 * The reset GPIO is active-low, but the driver has always used the
> +	 * gpiod API with inverted logic. As a result, the DTS had to
> +	 * incorrectly mark the GPIO as active-high to compensate for this
> +	 * behavior. Changing the flag in the driver now would break backward
> +	 * compatibility with existing DTS configurations. To address this,
> +	 * we add a simple value inversion so the driver works with both old
> +	 * and new DTS.
> +	 */
> +	ov9282->reset_gpio_val = gpiod_is_active_low(ov9282->reset_gpio);
> +	if (!ov9282->reset_gpio_val)
> +		dev_warn(ov9282->dev, "Using ACTIVE_HIGH for reset GPIO. Your DTB might be outdated\n");
> +	gpiod_set_value_cansleep(ov9282->reset_gpio, ov9282->reset_gpio_val);
> +
>  	/* Get sensor input clock */
>  	ov9282->inclk = devm_v4l2_sensor_clk_get(ov9282->dev, NULL);
>  	if (IS_ERR(ov9282->inclk))
> @@ -1237,7 +1255,7 @@ static int ov9282_power_on(struct device *dev)
>  
>  	usleep_range(400, 600);
>  
> -	gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
> +	gpiod_set_value_cansleep(ov9282->reset_gpio, !ov9282->reset_gpio_val);
>  
>  	ret = clk_prepare_enable(ov9282->inclk);
>  	if (ret) {
> @@ -1260,7 +1278,7 @@ static int ov9282_power_on(struct device *dev)
>  error_clk:
>  	clk_disable_unprepare(ov9282->inclk);
>  error_reset:
> -	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
> +	gpiod_set_value_cansleep(ov9282->reset_gpio, ov9282->reset_gpio_val);
>  
>  	regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies);
>  
> @@ -1278,7 +1296,7 @@ static int ov9282_power_off(struct device *dev)
>  	struct v4l2_subdev *sd = dev_get_drvdata(dev);
>  	struct ov9282 *ov9282 = to_ov9282(sd);
>  
> -	gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
> +	gpiod_set_value_cansleep(ov9282->reset_gpio, ov9282->reset_gpio_val);
>  
>  	clk_disable_unprepare(ov9282->inclk);
>  

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Loic Poulain]

Hi Laurent,

On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Loic,
>
> On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
> > Ensure reset state is low in the power-on state and high in the
> > power-off state (assert reset). Note that the polarity is abstracted
> > by the GPIO subsystem, so the logic level reflects the intended reset
> > behavior.
>
> That's an interesting approach to fix DTS gone systematically wrong.
>
> I was thinking of the drivers that have this issue, too, but I would have
> introduced a new GPIO under a different name (many sensors use "enable",
> too). Any thoughts?
>
> Cc Laurent.

Do you have any feedback on this change?

>
> >
> > To maintain backward compatibility with DTS files that use an incorrect
> > flag, we implement a mechanism similar to:
> >   commit 738455858a2d ("ASoC: codecs: wsa881x: Use proper shutdown GPIO polarity")
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  drivers/media/i2c/ov9282.c | 26 ++++++++++++++++++++++----
> >  1 file changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index a9f6176e9729..e79b326cdd94 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -160,6 +160,7 @@ struct ov9282_mode {
> >   * @sd: V4L2 sub-device
> >   * @pad: Media pad. Only one pad supported
> >   * @reset_gpio: Sensor reset gpio
> > + * @reset_gpio_val: Logical value to reset the sensor
> >   * @inclk: Sensor input clock
> >   * @supplies: Regulator supplies for the sensor
> >   * @ctrl_handler: V4L2 control handler
> > @@ -180,6 +181,7 @@ struct ov9282 {
> >       struct v4l2_subdev sd;
> >       struct media_pad pad;
> >       struct gpio_desc *reset_gpio;
> > +     unsigned int reset_gpio_val;
> >       struct clk *inclk;
> >       struct regulator_bulk_data supplies[OV9282_NUM_SUPPLIES];
> >       struct v4l2_ctrl_handler ctrl_handler;
> > @@ -1127,13 +1129,29 @@ static int ov9282_parse_hw_config(struct ov9282 *ov9282)
> >
> >       /* Request optional reset pin */
> >       ov9282->reset_gpio = devm_gpiod_get_optional(ov9282->dev, "reset",
> > -                                                  GPIOD_OUT_LOW);
> > +                                                  GPIOD_OUT_HIGH);
> >       if (IS_ERR(ov9282->reset_gpio)) {
> >               dev_err(ov9282->dev, "failed to get reset gpio %ld",
> >                       PTR_ERR(ov9282->reset_gpio));
> >               return PTR_ERR(ov9282->reset_gpio);
> >       }
> >
> > +     /*
> > +      * Backwards compatibility work-around.
> > +      *
> > +      * The reset GPIO is active-low, but the driver has always used the
> > +      * gpiod API with inverted logic. As a result, the DTS had to
> > +      * incorrectly mark the GPIO as active-high to compensate for this
> > +      * behavior. Changing the flag in the driver now would break backward
> > +      * compatibility with existing DTS configurations. To address this,
> > +      * we add a simple value inversion so the driver works with both old
> > +      * and new DTS.
> > +      */
> > +     ov9282->reset_gpio_val = gpiod_is_active_low(ov9282->reset_gpio);
> > +     if (!ov9282->reset_gpio_val)
> > +             dev_warn(ov9282->dev, "Using ACTIVE_HIGH for reset GPIO. Your DTB might be outdated\n");
> > +     gpiod_set_value_cansleep(ov9282->reset_gpio, ov9282->reset_gpio_val);
> > +
> >       /* Get sensor input clock */
> >       ov9282->inclk = devm_v4l2_sensor_clk_get(ov9282->dev, NULL);
> >       if (IS_ERR(ov9282->inclk))
> > @@ -1237,7 +1255,7 @@ static int ov9282_power_on(struct device *dev)
> >
> >       usleep_range(400, 600);
> >
> > -     gpiod_set_value_cansleep(ov9282->reset_gpio, 1);
> > +     gpiod_set_value_cansleep(ov9282->reset_gpio, !ov9282->reset_gpio_val);
> >
> >       ret = clk_prepare_enable(ov9282->inclk);
> >       if (ret) {
> > @@ -1260,7 +1278,7 @@ static int ov9282_power_on(struct device *dev)
> >  error_clk:
> >       clk_disable_unprepare(ov9282->inclk);
> >  error_reset:
> > -     gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
> > +     gpiod_set_value_cansleep(ov9282->reset_gpio, ov9282->reset_gpio_val);
> >
> >       regulator_bulk_disable(OV9282_NUM_SUPPLIES, ov9282->supplies);
> >
> > @@ -1278,7 +1296,7 @@ static int ov9282_power_off(struct device *dev)
> >       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> >       struct ov9282 *ov9282 = to_ov9282(sd);
> >
> > -     gpiod_set_value_cansleep(ov9282->reset_gpio, 0);
> > +     gpiod_set_value_cansleep(ov9282->reset_gpio, ov9282->reset_gpio_val);
> >
> >       clk_disable_unprepare(ov9282->inclk);
> >
>
> --
> Kind regards,
>
> Sakari Ailus

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Krzysztof Kozlowski]

On 03/12/2025 11:00, Loic Poulain wrote:
> Hi Laurent,
> 
> On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
>>
>> Hi Loic,
>>
>> On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
>>> Ensure reset state is low in the power-on state and high in the
>>> power-off state (assert reset). Note that the polarity is abstracted
>>> by the GPIO subsystem, so the logic level reflects the intended reset
>>> behavior.
>>
>> That's an interesting approach to fix DTS gone systematically wrong.
>>
>> I was thinking of the drivers that have this issue, too, but I would have
>> introduced a new GPIO under a different name (many sensors use "enable",
>> too). Any thoughts?
>>
>> Cc Laurent.
> 
> Do you have any feedback on this change?
> 
>>
>>>
>>> To maintain backward compatibility with DTS files that use an incorrect
>>> flag, we implement a mechanism similar to:
>>>   commit 738455858a2d ("ASoC: codecs: wsa881x: Use proper shutdown GPIO polarity")
>>>
>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>> ---
>>>  drivers/media/i2c/ov9282.c | 26 ++++++++++++++++++++++----
>>>  1 file changed, 22 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
>>> index a9f6176e9729..e79b326cdd94 100644
>>> --- a/drivers/media/i2c/ov9282.c
>>> +++ b/drivers/media/i2c/ov9282.c
>>> @@ -160,6 +160,7 @@ struct ov9282_mode {
>>>   * @sd: V4L2 sub-device
>>>   * @pad: Media pad. Only one pad supported
>>>   * @reset_gpio: Sensor reset gpio
>>> + * @reset_gpio_val: Logical value to reset the sensor
>>>   * @inclk: Sensor input clock
>>>   * @supplies: Regulator supplies for the sensor
>>>   * @ctrl_handler: V4L2 control handler
>>> @@ -180,6 +181,7 @@ struct ov9282 {
>>>       struct v4l2_subdev sd;
>>>       struct media_pad pad;
>>>       struct gpio_desc *reset_gpio;
>>> +     unsigned int reset_gpio_val;
>>>       struct clk *inclk;
>>>       struct regulator_bulk_data supplies[OV9282_NUM_SUPPLIES];
>>>       struct v4l2_ctrl_handler ctrl_handler;
>>> @@ -1127,13 +1129,29 @@ static int ov9282_parse_hw_config(struct ov9282 *ov9282)
>>>
>>>       /* Request optional reset pin */
>>>       ov9282->reset_gpio = devm_gpiod_get_optional(ov9282->dev, "reset",
>>> -                                                  GPIOD_OUT_LOW);
>>> +                                                  GPIOD_OUT_HIGH);
>>>       if (IS_ERR(ov9282->reset_gpio)) {
>>>               dev_err(ov9282->dev, "failed to get reset gpio %ld",
>>>                       PTR_ERR(ov9282->reset_gpio));
>>>               return PTR_ERR(ov9282->reset_gpio);
>>>       }
>>>
>>> +     /*
>>> +      * Backwards compatibility work-around.
>>> +      *
>>> +      * The reset GPIO is active-low, but the driver has always used the
>>> +      * gpiod API with inverted logic. As a result, the DTS had to
>>> +      * incorrectly mark the GPIO as active-high to compensate for this
>>> +      * behavior. Changing the flag in the driver now would break backward
>>> +      * compatibility with existing DTS configurations. To address this,
>>> +      * we add a simple value inversion so the driver works with both old
>>> +      * and new DTS.

This obviously is not true - driver will fail to work with some of old DTS.

I am surprised that it is second approach last weeks duplicating the
same problem and claiming the same - the change is backwards compatible,
while it is not - instead of taking my old explanation from WSA drivers
and admit the actual case of broken DTS.

With proper description I could agree, but with incorrect claims - no,
it's wrong.

Best regards,
Krzysztof

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Krzysztof Kozlowski]

On 03/12/2025 11:00, Loic Poulain wrote:
> Hi Laurent,
> 
> On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
>>
>> Hi Loic,
>>
>> On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
>>> Ensure reset state is low in the power-on state and high in the
>>> power-off state (assert reset). Note that the polarity is abstracted
>>> by the GPIO subsystem, so the logic level reflects the intended reset
>>> behavior.
>>
>> That's an interesting approach to fix DTS gone systematically wrong.
>>
>> I was thinking of the drivers that have this issue, too, but I would have
>> introduced a new GPIO under a different name (many sensors use "enable",
>> too). Any thoughts?
>>
>> Cc Laurent.
> 
> Do you have any feedback on this change?
> 
>>
>>>
>>> To maintain backward compatibility with DTS files that use an incorrect
>>> flag, we implement a mechanism similar to:
>>>   commit 738455858a2d ("ASoC: codecs: wsa881x: Use proper shutdown GPIO polarity")

Heh, so you even found my commit which exactly points which cases are
broken, but you:

...


>>> +     /*
>>> +      * Backwards compatibility work-around.
>>> +      *
>>> +      * The reset GPIO is active-low, but the driver has always used the
>>> +      * gpiod API with inverted logic. As a result, the DTS had to
>>> +      * incorrectly mark the GPIO as active-high to compensate for this
>>> +      * behavior. Changing the flag in the driver now would break backward
>>> +      * compatibility with existing DTS configurations. To address this,
>>> +      * we add a simple value inversion so the driver works with both old
>>> +      * and new DTS.

claim everything is working fine. Please read my commit.


Best regards,
Krzysztof

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Loic Poulain]

Hi Sakari,

On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Loic,
>
> On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
> > Ensure reset state is low in the power-on state and high in the
> > power-off state (assert reset). Note that the polarity is abstracted
> > by the GPIO subsystem, so the logic level reflects the intended reset
> > behavior.
>
> That's an interesting approach to fix DTS gone systematically wrong.
>
> I was thinking of the drivers that have this issue, too, but I would have
> introduced a new GPIO under a different name (many sensors use "enable",
> too). Any thoughts?

Apologies for missing your point earlier. We can’t really name it
enable, as it performs the opposite function and that would be
confusing in the device tree description. A property like reset2 would
be more accurate, but I suspect such a binding wouldn’t be acceptable
from a device tree/bindings perspective.

Regards,
Loic

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Sakari Ailus]

Hi Loic,

On Mon, Dec 15, 2025 at 10:35:15AM +0100, Loic Poulain wrote:
> Hi Sakari,
> 
> On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
> >
> > Hi Loic,
> >
> > On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
> > > Ensure reset state is low in the power-on state and high in the
> > > power-off state (assert reset). Note that the polarity is abstracted
> > > by the GPIO subsystem, so the logic level reflects the intended reset
> > > behavior.
> >
> > That's an interesting approach to fix DTS gone systematically wrong.
> >
> > I was thinking of the drivers that have this issue, too, but I would have
> > introduced a new GPIO under a different name (many sensors use "enable",
> > too). Any thoughts?
> 
> Apologies for missing your point earlier. We can’t really name it
> enable, as it performs the opposite function and that would be
> confusing in the device tree description. A property like reset2 would
> be more accurate, but I suspect such a binding wouldn’t be acceptable
> from a device tree/bindings perspective.

Many sensor datasheets document a pin called "xshutdown" or alike. That's
not exactly "reset" or "enable" but it can be mapped to either and this can
be seen in the existing bindings. The polarity is effectively the opposite,
yes, but does that matter?

-- 
Regards,

Sakari Ailus

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Loic Poulain]

On Mon, Dec 15, 2025 at 10:40 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> Hi Loic,
>
> On Mon, Dec 15, 2025 at 10:35:15AM +0100, Loic Poulain wrote:
> > Hi Sakari,
> >
> > On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> > >
> > > Hi Loic,
> > >
> > > On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
> > > > Ensure reset state is low in the power-on state and high in the
> > > > power-off state (assert reset). Note that the polarity is abstracted
> > > > by the GPIO subsystem, so the logic level reflects the intended reset
> > > > behavior.
> > >
> > > That's an interesting approach to fix DTS gone systematically wrong.
> > >
> > > I was thinking of the drivers that have this issue, too, but I would have
> > > introduced a new GPIO under a different name (many sensors use "enable",
> > > too). Any thoughts?
> >
> > Apologies for missing your point earlier. We can’t really name it
> > enable, as it performs the opposite function and that would be
> > confusing in the device tree description. A property like reset2 would
> > be more accurate, but I suspect such a binding wouldn’t be acceptable
> > from a device tree/bindings perspective.
>
> Many sensor datasheets document a pin called "xshutdown" or alike. That's
> not exactly "reset" or "enable" but it can be mapped to either and this can
> be seen in the existing bindings. The polarity is effectively the opposite,
> yes, but does that matter?

I assume naming a pin 'xshutdown' or 'xreset' indicates that its
polarity is inverted at the driver level, the driver interprets the
shutdown or reset function as being active when the logical level is 0
(low), as they actually incorrectly do for the 'reset' gpio.

From the driver’s perspective, this naming convention is acceptable;
however, it causes the devicetree description to slightly diverge from
the datasheet and leaves the reset property effectively inverted (and
therefore incorrect).

Honestly, in this specific case, the simplest solution would be to fix
the driver, since there is currently no upstream devicetree using this
sensor. That would technically break backward compatibility for any
out-of-tree DTS (if they exist), but those would have been incorrect
in the first place.

But yes, this seems like a good opportunity to discuss and define a
more general approach that can be applied to other drivers with
similar polarity or naming issues.

Krzysztof, any thoughts?

Regards,
Loic

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Laurent Pinchart]

On Mon, Dec 15, 2025 at 11:19:51AM +0100, Loic Poulain wrote:
> On Mon, Dec 15, 2025 at 10:40 AM Sakari Ailus wrote:
> > On Mon, Dec 15, 2025 at 10:35:15AM +0100, Loic Poulain wrote:
> > > On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus wrote:
> > > > On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
> > > > > Ensure reset state is low in the power-on state and high in the
> > > > > power-off state (assert reset). Note that the polarity is abstracted
> > > > > by the GPIO subsystem, so the logic level reflects the intended reset
> > > > > behavior.
> > > >
> > > > That's an interesting approach to fix DTS gone systematically wrong.
> > > >
> > > > I was thinking of the drivers that have this issue, too, but I would have
> > > > introduced a new GPIO under a different name (many sensors use "enable",
> > > > too). Any thoughts?
> > >
> > > Apologies for missing your point earlier. We can’t really name it
> > > enable, as it performs the opposite function and that would be
> > > confusing in the device tree description. A property like reset2 would
> > > be more accurate, but I suspect such a binding wouldn’t be acceptable
> > > from a device tree/bindings perspective.
> >
> > Many sensor datasheets document a pin called "xshutdown" or alike. That's
> > not exactly "reset" or "enable" but it can be mapped to either and this can
> > be seen in the existing bindings. The polarity is effectively the opposite,
> > yes, but does that matter?
> 
> I assume naming a pin 'xshutdown' or 'xreset' indicates that its
> polarity is inverted at the driver level, the driver interprets the
> shutdown or reset function as being active when the logical level is 0
> (low), as they actually incorrectly do for the 'reset' gpio.
> 
> From the driver’s perspective, this naming convention is acceptable;
> however, it causes the devicetree description to slightly diverge from
> the datasheet and leaves the reset property effectively inverted (and
> therefore incorrect).
> 
> Honestly, in this specific case, the simplest solution would be to fix
> the driver, since there is currently no upstream devicetree using this
> sensor. That would technically break backward compatibility for any
> out-of-tree DTS (if they exist), but those would have been incorrect
> in the first place.

I would either fix the driver, or update the DT bindings to indicate the
polarity should be inverted due to a historical mistake.

I don't think this patch is right. The polarity in DT is meant to
describe board-level inversion of the GPIO, so you can't consider that
ACTIVE_HIGH is a DT bug and print a warning.

> But yes, this seems like a good opportunity to discuss and define a
> more general approach that can be applied to other drivers with
> similar polarity or naming issues.
> 
> Krzysztof, any thoughts?

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Krzysztof Kozlowski]

On 15/12/2025 11:19, Loic Poulain wrote:
> On Mon, Dec 15, 2025 at 10:40 AM Sakari Ailus
> <sakari.ailus@linux.intel.com> wrote:
>>
>> Hi Loic,
>>
>> On Mon, Dec 15, 2025 at 10:35:15AM +0100, Loic Poulain wrote:
>>> Hi Sakari,
>>>
>>> On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
>>> <sakari.ailus@linux.intel.com> wrote:
>>>>
>>>> Hi Loic,
>>>>
>>>> On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
>>>>> Ensure reset state is low in the power-on state and high in the
>>>>> power-off state (assert reset). Note that the polarity is abstracted
>>>>> by the GPIO subsystem, so the logic level reflects the intended reset
>>>>> behavior.
>>>>
>>>> That's an interesting approach to fix DTS gone systematically wrong.
>>>>
>>>> I was thinking of the drivers that have this issue, too, but I would have
>>>> introduced a new GPIO under a different name (many sensors use "enable",
>>>> too). Any thoughts?
>>>
>>> Apologies for missing your point earlier. We can’t really name it
>>> enable, as it performs the opposite function and that would be
>>> confusing in the device tree description. A property like reset2 would
>>> be more accurate, but I suspect such a binding wouldn’t be acceptable
>>> from a device tree/bindings perspective.
>>
>> Many sensor datasheets document a pin called "xshutdown" or alike. That's
>> not exactly "reset" or "enable" but it can be mapped to either and this can
>> be seen in the existing bindings. The polarity is effectively the opposite,
>> yes, but does that matter?
> 
> I assume naming a pin 'xshutdown' or 'xreset' indicates that its
> polarity is inverted at the driver level, the driver interprets the
> shutdown or reset function as being active when the logical level is 0
> (low), as they actually incorrectly do for the 'reset' gpio.
> 
> From the driver’s perspective, this naming convention is acceptable;
> however, it causes the devicetree description to slightly diverge from
> the datasheet and leaves the reset property effectively inverted (and
> therefore incorrect).
> 
> Honestly, in this specific case, the simplest solution would be to fix
> the driver, since there is currently no upstream devicetree using this
> sensor. That would technically break backward compatibility for any
> out-of-tree DTS (if they exist), but those would have been incorrect
> in the first place.
> 
> But yes, this seems like a good opportunity to discuss and define a
> more general approach that can be applied to other drivers with
> similar polarity or naming issues.
> 
> Krzysztof, any thoughts?

You need to first CC me. You sent it to the special bulk email
address... Anyway, please be specific about the question.

I responded to earlier message that your claims in your comment in this
patch are clearly wrong, but what it is surprising me, it's second
approach this month people completely ignore existing and new DTS. Other
was MT7530 where author also claim all is fine, but actually both old
and new DTS were broken. Same here.



Best regards,
Krzysztof

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Loic Poulain]

Hi Krzysztof,

On Tue, Dec 30, 2025 at 2:54 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 15/12/2025 11:19, Loic Poulain wrote:
> > On Mon, Dec 15, 2025 at 10:40 AM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:
> >>
> >> Hi Loic,
> >>
> >> On Mon, Dec 15, 2025 at 10:35:15AM +0100, Loic Poulain wrote:
> >>> Hi Sakari,
> >>>
> >>> On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
> >>> <sakari.ailus@linux.intel.com> wrote:
> >>>>
> >>>> Hi Loic,
> >>>>
> >>>> On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
> >>>>> Ensure reset state is low in the power-on state and high in the
> >>>>> power-off state (assert reset). Note that the polarity is abstracted
> >>>>> by the GPIO subsystem, so the logic level reflects the intended reset
> >>>>> behavior.
> >>>>
> >>>> That's an interesting approach to fix DTS gone systematically wrong.
> >>>>
> >>>> I was thinking of the drivers that have this issue, too, but I would have
> >>>> introduced a new GPIO under a different name (many sensors use "enable",
> >>>> too). Any thoughts?
> >>>
> >>> Apologies for missing your point earlier. We can’t really name it
> >>> enable, as it performs the opposite function and that would be
> >>> confusing in the device tree description. A property like reset2 would
> >>> be more accurate, but I suspect such a binding wouldn’t be acceptable
> >>> from a device tree/bindings perspective.
> >>
> >> Many sensor datasheets document a pin called "xshutdown" or alike. That's
> >> not exactly "reset" or "enable" but it can be mapped to either and this can
> >> be seen in the existing bindings. The polarity is effectively the opposite,
> >> yes, but does that matter?
> >
> > I assume naming a pin 'xshutdown' or 'xreset' indicates that its
> > polarity is inverted at the driver level, the driver interprets the
> > shutdown or reset function as being active when the logical level is 0
> > (low), as they actually incorrectly do for the 'reset' gpio.
> >
> > From the driver’s perspective, this naming convention is acceptable;
> > however, it causes the devicetree description to slightly diverge from
> > the datasheet and leaves the reset property effectively inverted (and
> > therefore incorrect).
> >
> > Honestly, in this specific case, the simplest solution would be to fix
> > the driver, since there is currently no upstream devicetree using this
> > sensor. That would technically break backward compatibility for any
> > out-of-tree DTS (if they exist), but those would have been incorrect
> > in the first place.
> >
> > But yes, this seems like a good opportunity to discuss and define a
> > more general approach that can be applied to other drivers with
> > similar polarity or naming issues.
> >
> > Krzysztof, any thoughts?
>
> You need to first CC me. You sent it to the special bulk email
> address... Anyway, please be specific about the question.

Ultimately, I’d like to reach a consensus before moving forward with
V4, as several approaches have been discussed so far:

1. Keep the current (incorrect) driver logic: This was the approach I
used in V1 of this series, explicitly noting in the DTS that the
polarity was incorrect. However, this workaround was fairly rejected
as not being an acceptable solution.

2. Fix the driver logic: This was the approach in V2. It ensures
correct behavior going forward, especially since there is currently no
upstream DTB using this binding yet. The downside is that it would
consistently break any out-of-tree DTBs that *incorrectly* describe
the GPIO polarity.

3. Follow the wsa881x approach: this is V3, aiming for best-effort
backward compatibility. That said, it’s true that this approach does
not handle all cases.

There have also been discussions about introducing an additional
property for the same pin, with polarity described correctly... From a
DTS perspective, I believe this would likely be rejected.

Based on Laurent’s reply, he seems more inclined toward solutions 1
and 2. Would either of these approaches be acceptable from a DTS
standpoint?

> I responded to earlier message that your claims in your comment in this
> patch are clearly wrong, but what it is surprising me, it's second
> approach this month people completely ignore existing and new DTS. Other
> was MT7530 where author also claim all is fine, but actually both old
> and new DTS were broken. Same here.

Yes, the comment is oversimplified, which makes it incorrect in
certain cases. I’ll ensure the comment is accurate in the next version
if we decide to stick with this solution.

Regards,
Loic

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Laurent Pinchart]

On Tue, Dec 30, 2025 at 04:03:58PM +0100, Loic Poulain wrote:
> On Tue, Dec 30, 2025 at 2:54 PM Krzysztof Kozlowski wrote:
> > On 15/12/2025 11:19, Loic Poulain wrote:
> > > On Mon, Dec 15, 2025 at 10:40 AM Sakari Ailus wrote:
> > >> On Mon, Dec 15, 2025 at 10:35:15AM +0100, Loic Poulain wrote:
> > >>> On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus wrote:
> > >>>> On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
> > >>>>> Ensure reset state is low in the power-on state and high in the
> > >>>>> power-off state (assert reset). Note that the polarity is abstracted
> > >>>>> by the GPIO subsystem, so the logic level reflects the intended reset
> > >>>>> behavior.
> > >>>>
> > >>>> That's an interesting approach to fix DTS gone systematically wrong.
> > >>>>
> > >>>> I was thinking of the drivers that have this issue, too, but I would have
> > >>>> introduced a new GPIO under a different name (many sensors use "enable",
> > >>>> too). Any thoughts?
> > >>>
> > >>> Apologies for missing your point earlier. We can’t really name it
> > >>> enable, as it performs the opposite function and that would be
> > >>> confusing in the device tree description. A property like reset2 would
> > >>> be more accurate, but I suspect such a binding wouldn’t be acceptable
> > >>> from a device tree/bindings perspective.
> > >>
> > >> Many sensor datasheets document a pin called "xshutdown" or alike. That's
> > >> not exactly "reset" or "enable" but it can be mapped to either and this can
> > >> be seen in the existing bindings. The polarity is effectively the opposite,
> > >> yes, but does that matter?
> > >
> > > I assume naming a pin 'xshutdown' or 'xreset' indicates that its
> > > polarity is inverted at the driver level, the driver interprets the
> > > shutdown or reset function as being active when the logical level is 0
> > > (low), as they actually incorrectly do for the 'reset' gpio.
> > >
> > > From the driver’s perspective, this naming convention is acceptable;
> > > however, it causes the devicetree description to slightly diverge from
> > > the datasheet and leaves the reset property effectively inverted (and
> > > therefore incorrect).
> > >
> > > Honestly, in this specific case, the simplest solution would be to fix
> > > the driver, since there is currently no upstream devicetree using this
> > > sensor. That would technically break backward compatibility for any
> > > out-of-tree DTS (if they exist), but those would have been incorrect
> > > in the first place.
> > >
> > > But yes, this seems like a good opportunity to discuss and define a
> > > more general approach that can be applied to other drivers with
> > > similar polarity or naming issues.
> > >
> > > Krzysztof, any thoughts?
> >
> > You need to first CC me. You sent it to the special bulk email
> > address... Anyway, please be specific about the question.
> 
> Ultimately, I’d like to reach a consensus before moving forward with
> V4, as several approaches have been discussed so far:
> 
> 1. Keep the current (incorrect) driver logic: This was the approach I
> used in V1 of this series, explicitly noting in the DTS that the
> polarity was incorrect. However, this workaround was fairly rejected
> as not being an acceptable solution.
> 
> 2. Fix the driver logic: This was the approach in V2. It ensures
> correct behavior going forward, especially since there is currently no
> upstream DTB using this binding yet. The downside is that it would
> consistently break any out-of-tree DTBs that *incorrectly* describe
> the GPIO polarity.
> 
> 3. Follow the wsa881x approach: this is V3, aiming for best-effort
> backward compatibility. That said, it’s true that this approach does
> not handle all cases.
> 
> There have also been discussions about introducing an additional
> property for the same pin, with polarity described correctly... From a
> DTS perspective, I believe this would likely be rejected.
> 
> Based on Laurent’s reply, he seems more inclined toward solutions 1
> and 2. Would either of these approaches be acceptable from a DTS
> standpoint?

Do you know of DTs in the wild that use the ov9282 reset-gpios ? Based
on the git history, I see the driver was initially upstreamed by Intel,
and there's been lots of activity on the driver from Dave Stevenson from
Raspberry Pi.

Raspberry Pi modules don't wire the reset pin to a GPIO (the GPIO on the
connector controls the on-module regulators), so there should be no
regression if we changed the driver behaviour.

As the driver was upstreamed by Intel, I assume it may be used on
ACPI-based systems. Sakari, do you know what those machines are, and if
they expose the reset GPIO through ACPI ?

> > I responded to earlier message that your claims in your comment in this
> > patch are clearly wrong, but what it is surprising me, it's second
> > approach this month people completely ignore existing and new DTS. Other
> > was MT7530 where author also claim all is fine, but actually both old
> > and new DTS were broken. Same here.
> 
> Yes, the comment is oversimplified, which makes it incorrect in
> certain cases. I’ll ensure the comment is accurate in the next version
> if we decide to stick with this solution.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Krzysztof Kozlowski]

On 30/12/2025 16:03, Loic Poulain wrote:
> Hi Krzysztof,
> 
> On Tue, Dec 30, 2025 at 2:54 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 15/12/2025 11:19, Loic Poulain wrote:
>>> On Mon, Dec 15, 2025 at 10:40 AM Sakari Ailus
>>> <sakari.ailus@linux.intel.com> wrote:
>>>>
>>>> Hi Loic,
>>>>
>>>> On Mon, Dec 15, 2025 at 10:35:15AM +0100, Loic Poulain wrote:
>>>>> Hi Sakari,
>>>>>
>>>>> On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
>>>>> <sakari.ailus@linux.intel.com> wrote:
>>>>>>
>>>>>> Hi Loic,
>>>>>>
>>>>>> On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
>>>>>>> Ensure reset state is low in the power-on state and high in the
>>>>>>> power-off state (assert reset). Note that the polarity is abstracted
>>>>>>> by the GPIO subsystem, so the logic level reflects the intended reset
>>>>>>> behavior.
>>>>>>
>>>>>> That's an interesting approach to fix DTS gone systematically wrong.
>>>>>>
>>>>>> I was thinking of the drivers that have this issue, too, but I would have
>>>>>> introduced a new GPIO under a different name (many sensors use "enable",
>>>>>> too). Any thoughts?
>>>>>
>>>>> Apologies for missing your point earlier. We can’t really name it
>>>>> enable, as it performs the opposite function and that would be
>>>>> confusing in the device tree description. A property like reset2 would
>>>>> be more accurate, but I suspect such a binding wouldn’t be acceptable
>>>>> from a device tree/bindings perspective.
>>>>
>>>> Many sensor datasheets document a pin called "xshutdown" or alike. That's
>>>> not exactly "reset" or "enable" but it can be mapped to either and this can
>>>> be seen in the existing bindings. The polarity is effectively the opposite,
>>>> yes, but does that matter?
>>>
>>> I assume naming a pin 'xshutdown' or 'xreset' indicates that its
>>> polarity is inverted at the driver level, the driver interprets the
>>> shutdown or reset function as being active when the logical level is 0
>>> (low), as they actually incorrectly do for the 'reset' gpio.
>>>
>>> From the driver’s perspective, this naming convention is acceptable;
>>> however, it causes the devicetree description to slightly diverge from
>>> the datasheet and leaves the reset property effectively inverted (and
>>> therefore incorrect).
>>>
>>> Honestly, in this specific case, the simplest solution would be to fix
>>> the driver, since there is currently no upstream devicetree using this
>>> sensor. That would technically break backward compatibility for any
>>> out-of-tree DTS (if they exist), but those would have been incorrect
>>> in the first place.
>>>
>>> But yes, this seems like a good opportunity to discuss and define a
>>> more general approach that can be applied to other drivers with
>>> similar polarity or naming issues.
>>>
>>> Krzysztof, any thoughts?
>>
>> You need to first CC me. You sent it to the special bulk email
>> address... Anyway, please be specific about the question.
> 
> Ultimately, I’d like to reach a consensus before moving forward with
> V4, as several approaches have been discussed so far:
> 
> 1. Keep the current (incorrect) driver logic: This was the approach I
> used in V1 of this series, explicitly noting in the DTS that the
> polarity was incorrect. However, this workaround was fairly rejected
> as not being an acceptable solution.
> 
> 2. Fix the driver logic: This was the approach in V2. It ensures
> correct behavior going forward, especially since there is currently no
> upstream DTB using this binding yet. The downside is that it would
> consistently break any out-of-tree DTBs that *incorrectly* describe
> the GPIO polarity.

No... It will break EVERY out of tree DTBs, which was previously
working. It won't break only the ones which were NEVER working...

Breaking a working out of tree DTBs for a driver used for almost 5 years
is a no-go.

...unless you get acks from all platform maintainers (so ~40 SoC
maintainers?). Quite unlikely.

This should stay as is just like I did not fix none of other sensor drivers.

Best regards,
Krzysztof

Re: [PATCH v3 1/4] media: i2c: ov9282: Fix reset-gpio logical state

[Loic Poulain]

On Tue, Dec 30, 2025 at 4:41 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 30/12/2025 16:03, Loic Poulain wrote:
> > Hi Krzysztof,
> >
> > On Tue, Dec 30, 2025 at 2:54 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 15/12/2025 11:19, Loic Poulain wrote:
> >>> On Mon, Dec 15, 2025 at 10:40 AM Sakari Ailus
> >>> <sakari.ailus@linux.intel.com> wrote:
> >>>>
> >>>> Hi Loic,
> >>>>
> >>>> On Mon, Dec 15, 2025 at 10:35:15AM +0100, Loic Poulain wrote:
> >>>>> Hi Sakari,
> >>>>>
> >>>>> On Mon, Nov 17, 2025 at 6:30 PM Sakari Ailus
> >>>>> <sakari.ailus@linux.intel.com> wrote:
> >>>>>>
> >>>>>> Hi Loic,
> >>>>>>
> >>>>>> On Fri, Nov 14, 2025 at 02:38:19PM +0100, Loic Poulain wrote:
> >>>>>>> Ensure reset state is low in the power-on state and high in the
> >>>>>>> power-off state (assert reset). Note that the polarity is abstracted
> >>>>>>> by the GPIO subsystem, so the logic level reflects the intended reset
> >>>>>>> behavior.
> >>>>>>
> >>>>>> That's an interesting approach to fix DTS gone systematically wrong.
> >>>>>>
> >>>>>> I was thinking of the drivers that have this issue, too, but I would have
> >>>>>> introduced a new GPIO under a different name (many sensors use "enable",
> >>>>>> too). Any thoughts?
> >>>>>
> >>>>> Apologies for missing your point earlier. We can’t really name it
> >>>>> enable, as it performs the opposite function and that would be
> >>>>> confusing in the device tree description. A property like reset2 would
> >>>>> be more accurate, but I suspect such a binding wouldn’t be acceptable
> >>>>> from a device tree/bindings perspective.
> >>>>
> >>>> Many sensor datasheets document a pin called "xshutdown" or alike. That's
> >>>> not exactly "reset" or "enable" but it can be mapped to either and this can
> >>>> be seen in the existing bindings. The polarity is effectively the opposite,
> >>>> yes, but does that matter?
> >>>
> >>> I assume naming a pin 'xshutdown' or 'xreset' indicates that its
> >>> polarity is inverted at the driver level, the driver interprets the
> >>> shutdown or reset function as being active when the logical level is 0
> >>> (low), as they actually incorrectly do for the 'reset' gpio.
> >>>
> >>> From the driver’s perspective, this naming convention is acceptable;
> >>> however, it causes the devicetree description to slightly diverge from
> >>> the datasheet and leaves the reset property effectively inverted (and
> >>> therefore incorrect).
> >>>
> >>> Honestly, in this specific case, the simplest solution would be to fix
> >>> the driver, since there is currently no upstream devicetree using this
> >>> sensor. That would technically break backward compatibility for any
> >>> out-of-tree DTS (if they exist), but those would have been incorrect
> >>> in the first place.
> >>>
> >>> But yes, this seems like a good opportunity to discuss and define a
> >>> more general approach that can be applied to other drivers with
> >>> similar polarity or naming issues.
> >>>
> >>> Krzysztof, any thoughts?
> >>
> >> You need to first CC me. You sent it to the special bulk email
> >> address... Anyway, please be specific about the question.
> >
> > Ultimately, I’d like to reach a consensus before moving forward with
> > V4, as several approaches have been discussed so far:
> >
> > 1. Keep the current (incorrect) driver logic: This was the approach I
> > used in V1 of this series, explicitly noting in the DTS that the
> > polarity was incorrect. However, this workaround was fairly rejected
> > as not being an acceptable solution.
> >
> > 2. Fix the driver logic: This was the approach in V2. It ensures
> > correct behavior going forward, especially since there is currently no
> > upstream DTB using this binding yet. The downside is that it would
> > consistently break any out-of-tree DTBs that *incorrectly* describe
> > the GPIO polarity.
>
> No... It will break EVERY out of tree DTBs, which was previously
> working. It won't break only the ones which were NEVER working...
>
> Breaking a working out of tree DTBs for a driver used for almost 5 years
> is a no-go.
>
> ...unless you get acks from all platform maintainers (so ~40 SoC
> maintainers?). Quite unlikely.
>
> This should stay as is just like I did not fix none of other sensor drivers.

Understood, appreciate your input. I’ll keep the inverted logic as suggested.

Regards,
Loic

[PATCH v3 2/4] arm64: dts: qcom: qcm2290: Add pin configuration for mclks

[Loic Poulain]

Add pinctrl configuration for the four available camera master clocks (mclk).

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/agatti.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/agatti.dtsi b/arch/arm64/boot/dts/qcom/agatti.dtsi
index 8bf5c5583fc2..e14d1f444ab5 100644
--- a/arch/arm64/boot/dts/qcom/agatti.dtsi
+++ b/arch/arm64/boot/dts/qcom/agatti.dtsi
@@ -597,6 +597,34 @@ cci1_default: cci1-default-state {
 				bias-disable;
 			};
 
+			mclk0_default: mclk0-default-state {
+				pins = "gpio20";
+				function = "cam_mclk";
+				drive-strength = <16>;
+				bias-disable;
+			};
+
+			mclk1_default: mclk1-default-state {
+				pins = "gpio21";
+				function = "cam_mclk";
+				drive-strength = <16>;
+				bias-disable;
+			};
+
+			mclk2_default: mclk2-default-state {
+				pins = "gpio27";
+				function = "cam_mclk";
+				drive-strength = <16>;
+				bias-disable;
+			};
+
+			mclk3_default: mclk3-default-state {
+				pins = "gpio28";
+				function = "cam_mclk";
+				drive-strength = <16>;
+				bias-disable;
+			};
+
 			sdc1_state_on: sdc1-on-state {
 				clk-pins {
 					pins = "sdc1_clk";
-- 
2.34.1

Re: [PATCH v3 2/4] arm64: dts: qcom: qcm2290: Add pin configuration for mclks

[Konrad Dybcio]

On 11/14/25 2:38 PM, Loic Poulain wrote:
> Add pinctrl configuration for the four available camera master clocks (mclk).
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/agatti.dtsi | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/agatti.dtsi b/arch/arm64/boot/dts/qcom/agatti.dtsi
> index 8bf5c5583fc2..e14d1f444ab5 100644
> --- a/arch/arm64/boot/dts/qcom/agatti.dtsi
> +++ b/arch/arm64/boot/dts/qcom/agatti.dtsi
> @@ -597,6 +597,34 @@ cci1_default: cci1-default-state {
>  				bias-disable;
>  			};
>  
> +			mclk0_default: mclk0-default-state {
> +				pins = "gpio20";

Preferably these could be sorted by gpio idx

anyway

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

[PATCH v3 3/4] arm64: dts: qcom: qrb2210-rb1: Add PM8008 node

[Loic Poulain]

The PM8008 device is a dedicated camera PMIC integrating all the necessary
camera power management features.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 75 ++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
index 1b9ca957a94b..9814ac4896c5 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
@@ -267,6 +267,81 @@ &gpu_zap_shader {
 	firmware-name = "qcom/qcm2290/a702_zap.mbn";
 };
 
+&i2c1 {
+	clock-frequency = <400000>;
+
+	status = "okay";
+
+	pm8008: pmic@8 {
+		compatible = "qcom,pm8008";
+		reg = <0x8>;
+
+		interrupts-extended = <&tlmm 25 IRQ_TYPE_EDGE_RISING>;
+		reset-gpios = <&tlmm 26 GPIO_ACTIVE_LOW>;
+
+		vdd-l1-l2-supply = <&pm4125_s3>;
+		vdd-l3-l4-supply = <&vph_pwr>;
+		vdd-l5-supply = <&vph_pwr>;
+		vdd-l6-supply = <&vph_pwr>;
+		vdd-l7-supply = <&vph_pwr>;
+
+		gpio-controller;
+		#gpio-cells = <2>;
+		gpio-ranges = <&pm8008 0 0 2>;
+
+		interrupt-controller;
+		#interrupt-cells = <2>;
+
+		#thermal-sensor-cells = <0>;
+
+		status = "disabled";
+
+		regulators {
+			vreg_l1p: ldo1 {
+				regulator-name = "vreg_l1p";
+				regulator-min-microvolt = <528000>;
+				regulator-max-microvolt = <1200000>;
+			};
+
+			vreg_l2p: ldo2 {
+				regulator-name = "vreg_l2p";
+				regulator-min-microvolt = <528000>;
+				regulator-max-microvolt = <1200000>;
+			};
+
+			vreg_l3p: ldo3 {
+				regulator-name = "vreg_l3p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3400000>;
+			};
+
+			vreg_l4p: ldo4 {
+				regulator-name = "vreg_l4p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3404000>;
+			};
+
+			vreg_l5p: ldo5 {
+				regulator-name = "vreg_l5p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3400000>;
+			};
+
+			vreg_l6p: ldo6 {
+				regulator-name = "vreg_l6p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3400000>;
+			};
+
+			vreg_l7p: ldo7 {
+				regulator-name = "vreg_l7p";
+				regulator-min-microvolt = <1500000>;
+				regulator-max-microvolt = <3400000>;
+			};
+		};
+	};
+};
+
 &i2c2_gpio {
 	clock-frequency = <400000>;
 	status = "okay";
-- 
2.34.1

[PATCH v3 4/4] arm64: dts: qcom: qrb2210-rb1: Add overlay for vision mezzanine

[Loic Poulain]

This initial version includes support for OV9282 camera sensor.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
---
 arch/arm64/boot/dts/qcom/Makefile             |  5 ++
 .../qcom/qrb2210-rb1-vision-mezzanine.dtso    | 65 +++++++++++++++++++
 2 files changed, 70 insertions(+)
 create mode 100644 arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso

diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
index 6f34d5ed331c..af029d04758e 100644
--- a/arch/arm64/boot/dts/qcom/Makefile
+++ b/arch/arm64/boot/dts/qcom/Makefile
@@ -144,6 +144,11 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1.dtb
+
+qrb2210-rb1-vision-mezzanine-dtbs	:= qrb2210-rb1.dtb qrb2210-rb1-vision-mezzanine.dtbo
+
+dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1-vision-mezzanine.dtb
+
 dtb-$(CONFIG_ARCH_QCOM)	+= qrb4210-rb2.dtb
 dtb-$(CONFIG_ARCH_QCOM)	+= qrb5165-rb5.dtb
 
diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
new file mode 100644
index 000000000000..6f3d8de950ad
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/qcom,gcc-qcm2290.h>
+#include <dt-bindings/gpio/gpio.h>
+
+&pm8008 {
+	status = "okay";
+};
+
+&camss {
+	status = "okay";
+
+	vdd-csiphy-1p2-supply = <&pm4125_l5>;
+	vdd-csiphy-1p8-supply = <&pm4125_l13>;
+
+	ports {
+		port@0 {
+			csiphy0_ep: endpoint {
+				data-lanes = <0 1>;
+				remote-endpoint = <&ov9282_ep>;
+			};
+		};
+	};
+};
+
+&cci {
+	status = "okay";
+};
+
+&cci_i2c1 {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	/* Vision Mezzanine DIP3-1 must be ON (Selects camera CAM0A&B) */
+	camera@60 {
+		compatible = "ovti,ov9282";
+		reg = <0x60>;
+
+		reset-gpios = <&tlmm 18 GPIO_ACTIVE_LOW>;
+		pinctrl-0 = <&mclk3_default>;
+		pinctrl-names = "default";
+
+		clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
+		assigned-clocks = <&gcc GCC_CAMSS_MCLK3_CLK>;
+		assigned-clock-rates = <24000000>;
+
+		avdd-supply = <&vreg_l3p>;
+		dvdd-supply = <&vreg_l1p>;
+		dovdd-supply = <&vreg_l7p>;
+
+		port {
+			ov9282_ep: endpoint {
+				link-frequencies = /bits/ 64 <400000000>;
+				data-lanes = <1 2>;
+				remote-endpoint = <&csiphy0_ep>;
+                        };
+                };
+	};
+};
-- 
2.34.1

Re: [PATCH v3 4/4] arm64: dts: qcom: qrb2210-rb1: Add overlay for vision mezzanine

[Dmitry Baryshkov]

On Fri, Nov 14, 2025 at 02:38:22PM +0100, Loic Poulain wrote:
> This initial version includes support for OV9282 camera sensor.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/Makefile             |  5 ++
>  .../qcom/qrb2210-rb1-vision-mezzanine.dtso    | 65 +++++++++++++++++++
>  2 files changed, 70 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> 
> diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> index 6f34d5ed331c..af029d04758e 100644
> --- a/arch/arm64/boot/dts/qcom/Makefile
> +++ b/arch/arm64/boot/dts/qcom/Makefile
> @@ -144,6 +144,11 @@ dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qcs9100-ride-r3.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qdu1000-idp.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1.dtb
> +
> +qrb2210-rb1-vision-mezzanine-dtbs	:= qrb2210-rb1.dtb qrb2210-rb1-vision-mezzanine.dtbo
> +
> +dtb-$(CONFIG_ARCH_QCOM)	+= qrb2210-rb1-vision-mezzanine.dtb
> +
>  dtb-$(CONFIG_ARCH_QCOM)	+= qrb4210-rb2.dtb
>  dtb-$(CONFIG_ARCH_QCOM)	+= qrb5165-rb5.dtb
>  
> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> new file mode 100644
> index 000000000000..6f3d8de950ad
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1-vision-mezzanine.dtso
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +#include <dt-bindings/clock/qcom,gcc-qcm2290.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +&pm8008 {
> +	status = "okay";
> +};
> +
> +&camss {
> +	status = "okay";
> +
> +	vdd-csiphy-1p2-supply = <&pm4125_l5>;
> +	vdd-csiphy-1p8-supply = <&pm4125_l13>;

Status should be the last property

> +
> +	ports {
> +		port@0 {
> +			csiphy0_ep: endpoint {
> +				data-lanes = <0 1>;
> +				remote-endpoint = <&ov9282_ep>;
> +			};
> +		};
> +	};
> +};
> +

-- 
With best wishes
Dmitry