From: Ondrej Jirman <[email protected]>
Add powerdown-gpio binding as it is required for some boards.
Signed-off-by: Ondrej Jirman <[email protected]>
Signed-off-by: Luis Garcia <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Pavel Machek <[email protected]>
---
Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
index c978abc0cdb3..33338139e6e8 100644
--- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
@@ -36,6 +36,10 @@ properties:
reg:
maxItems: 1
+ powerdown-gpios:
+ description:
+ Reference to the GPIO connected to the PWDN pin, if any.
+
reset-gpios:
description: |-
Reference to the GPIO connected to the XCLR pin, if any.
--
2.44.0
Hi Luis,
On Wed, May 01, 2024 at 09:24:38AM -0600, [email protected] wrote:
> From: Ondrej Jirman <[email protected]>
>
> Add powerdown-gpio binding as it is required for some boards.
I thought the conclusion was that this wasn't a property of the sensor? If
it needs to be controlled, then this should take place somewhere else than
in the sensor driver.
>
> Signed-off-by: Ondrej Jirman <[email protected]>
> Signed-off-by: Luis Garcia <[email protected]>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
> Reviewed-by: Pavel Machek <[email protected]>
> ---
> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> index c978abc0cdb3..33338139e6e8 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> @@ -36,6 +36,10 @@ properties:
> reg:
> maxItems: 1
>
> + powerdown-gpios:
> + description:
> + Reference to the GPIO connected to the PWDN pin, if any.
> +
> reset-gpios:
> description: |-
> Reference to the GPIO connected to the XCLR pin, if any.
--
Regards,
Sakari Ailus
On Fri, May 17, 2024 at 08:31:35AM +0000, Sakari Ailus wrote:
> Hi Luis,
>
> On Wed, May 01, 2024 at 09:24:38AM -0600, [email protected] wrote:
> > From: Ondrej Jirman <[email protected]>
> >
> > Add powerdown-gpio binding as it is required for some boards.
>
> I thought the conclusion was that this wasn't a property of the sensor? If
> it needs to be controlled, then this should take place somewhere else than
> in the sensor driver.
I'll take patches up to the previous and then 24 and 25 from this version.
The rest will need some rework.
--
Sakari Ailus
On Fri, May 17, 2024 at 08:31:35AM GMT, Sakari Ailus wrote:
> Hi Luis,
>
> On Wed, May 01, 2024 at 09:24:38AM -0600, [email protected] wrote:
> > From: Ondrej Jirman <[email protected]>
> >
> > Add powerdown-gpio binding as it is required for some boards.
>
> I thought the conclusion was that this wasn't a property of the sensor? If
> it needs to be controlled, then this should take place somewhere else than
> in the sensor driver.
It's a property of the sensor modules. It's just optional on
some, eg. (pin 8):
https://assets-global.website-files.com/63b65bd4974577341e1fe194/654290d4d0fb173e87f754ed_IMX_258_FF_drawing.png
Where else should it be so that the module is described properly in the
DT and the powerdown signal can be used as part of powerup/down sequence
of the sensor?
regards,
o.
> >
> > Signed-off-by: Ondrej Jirman <[email protected]>
> > Signed-off-by: Luis Garcia <[email protected]>
> > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> > Reviewed-by: Pavel Machek <[email protected]>
> > ---
> > Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > index c978abc0cdb3..33338139e6e8 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > @@ -36,6 +36,10 @@ properties:
> > reg:
> > maxItems: 1
> >
> > + powerdown-gpios:
> > + description:
> > + Reference to the GPIO connected to the PWDN pin, if any.
> > +
> > reset-gpios:
> > description: |-
> > Reference to the GPIO connected to the XCLR pin, if any.
>
> --
> Regards,
>
> Sakari Ailus
Hi Ondřej
On Mon, 20 May 2024 at 13:55, Ondřej Jirman <[email protected]> wrote:
>
> On Fri, May 17, 2024 at 08:31:35AM GMT, Sakari Ailus wrote:
> > Hi Luis,
> >
> > On Wed, May 01, 2024 at 09:24:38AM -0600, [email protected] wrote:
> > > From: Ondrej Jirman <[email protected]>
> > >
> > > Add powerdown-gpio binding as it is required for some boards.
> >
> > I thought the conclusion was that this wasn't a property of the sensor? If
> > it needs to be controlled, then this should take place somewhere else than
> > in the sensor driver.
>
> It's a property of the sensor modules. It's just optional on
> some, eg. (pin 8):
>
> https://assets-global.website-files.com/63b65bd4974577341e1fe194/654290d4d0fb173e87f754ed_IMX_258_FF_drawing.png
>
> Where else should it be so that the module is described properly in the
> DT and the powerdown signal can be used as part of powerup/down sequence
> of the sensor?
From v3 [1] Luis reported testing dropping the powerdown-gpio on a PPP
and it working fine.
I linked to the IMX258 datasheet in the same thread[2], and that
datasheet does not include such a signal on the imx258 sensor itself.
If your module has a powerdown gpio, then you'll have to ask the
module vendor what it is actually connected to. Potentially it relates
to the VCM driver rather than the sensor.
Dave
[1] https://www.spinics.net/lists/linux-media/msg252519.html
[2] https://www.spinics.net/lists/linux-media/msg252496.html
> regards,
> o.
>
> > >
> > > Signed-off-by: Ondrej Jirman <[email protected]>
> > > Signed-off-by: Luis Garcia <[email protected]>
> > > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> > > Reviewed-by: Pavel Machek <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > index c978abc0cdb3..33338139e6e8 100644
> > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> > > @@ -36,6 +36,10 @@ properties:
> > > reg:
> > > maxItems: 1
> > >
> > > + powerdown-gpios:
> > > + description:
> > > + Reference to the GPIO connected to the PWDN pin, if any.
> > > +
> > > reset-gpios:
> > > description: |-
> > > Reference to the GPIO connected to the XCLR pin, if any.
> >
> > --
> > Regards,
> >
> > Sakari Ailus
Hi Ondřej,
On Mon, May 20, 2024 at 02:55:26PM +0200, Ondřej Jirman wrote:
> On Fri, May 17, 2024 at 08:31:35AM GMT, Sakari Ailus wrote:
> > Hi Luis,
> >
> > On Wed, May 01, 2024 at 09:24:38AM -0600, [email protected] wrote:
> > > From: Ondrej Jirman <[email protected]>
> > >
> > > Add powerdown-gpio binding as it is required for some boards.
> >
> > I thought the conclusion was that this wasn't a property of the sensor? If
> > it needs to be controlled, then this should take place somewhere else than
> > in the sensor driver.
>
> It's a property of the sensor modules. It's just optional on
> some, eg. (pin 8):
>
> https://assets-global.website-files.com/63b65bd4974577341e1fe194/654290d4d0fb173e87f754ed_IMX_258_FF_drawing.png
>
> Where else should it be so that the module is described properly in the
> DT and the powerdown signal can be used as part of powerup/down sequence
> of the sensor?
That's an interesting case. The document does suggest the PWDN pin isn't
connected though.
Is this pin used for something? If so, what is it? The sensor doesn't have
it and generally the module in this respect just contains wiring.
Someone earlier suggested this could have been related to the VCM.
--
Regards,
Sakari Ailus
Hi Dave,
On Mon, May 20, 2024 at 02:20:28PM GMT, Dave Stevenson wrote:
> Hi Ondřej
>
> [...]
>
> From v3 [1] Luis reported testing dropping the powerdown-gpio on a PPP
> and it working fine.
>
> I linked to the IMX258 datasheet in the same thread[2], and that
> datasheet does not include such a signal on the imx258 sensor itself.
>
> If your module has a powerdown gpio, then you'll have to ask the
> module vendor what it is actually connected to. Potentially it relates
> to the VCM driver rather than the sensor.
I've tested it in various ways (inverting the signal, etc.) and it doesn't
seem to do anything. So these patches related to powerdown-gpio can
be dropped.
Kind regards,
o.
> Dave
>
> [1] https://www.spinics.net/lists/linux-media/msg252519.html
> [2] https://www.spinics.net/lists/linux-media/msg252496.html
On 5/20/24 06:50, Sakari Ailus wrote:
> On Fri, May 17, 2024 at 08:31:35AM +0000, Sakari Ailus wrote:
>> Hi Luis,
>>
>> On Wed, May 01, 2024 at 09:24:38AM -0600, [email protected] wrote:
>>> From: Ondrej Jirman <[email protected]>
>>>
>>> Add powerdown-gpio binding as it is required for some boards.
>>
>> I thought the conclusion was that this wasn't a property of the sensor? If
>> it needs to be controlled, then this should take place somewhere else than
>> in the sensor driver.
>
> I'll take patches up to the previous and then 24 and 25 from this version.
> The rest will need some rework.
>
Last time i looked at the comments for these two patch discussion was still
going on so i was submitting fixes for the other patches while this continued.
Looks like the final consensuses is for these two to be dropped so I just
submitted v6 with them both dropped. I also tested it on my PPP and it still
continues to work as expected just like it did before. Take a look at v6 and
let me know what else requires rework on there.
