Re: [PATCH v2 2/2] media: imx335: Fix reset-gpio handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi all,

On 30/07/24 2:40 pm, Laurent Pinchart wrote:
On Tue, Jul 30, 2024 at 10:42:01AM +0200, Krzysztof Kozlowski wrote:
On 30/07/2024 10:24, Sakari Ailus wrote:
Hi Krzysztof,

On Mon, Jul 29, 2024 at 04:09:39PM +0200, Krzysztof Kozlowski wrote:
On 29/07/2024 13:04, Umang Jain wrote:
Rectify the logical value of reset-gpio so that it is set to
0 (disabled) during power-on and to 1 (enabled) during power-off.

Meanwhile at it, set the reset-gpio to GPIO_OUT_HIGH at initialization
time to make sure it starts off in reset.

Fixes: 45d19b5fb9ae ("media: i2c: Add imx335 camera sensor driver")
Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx>
---
  drivers/media/i2c/imx335.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

This will break all the users, so no. At least not without mentioning
ABI break and some sort of investigating how customers or users are
affected.
I know the original authors aren't using the driver anymore and it took
quite a bit of time until others started to contribute to it so I suspect
the driver hasn't been in use for that long. There are no instances of the
device in the in-kernel DTS either.

Any DTS author should have also noticed the issue but of course there's a
risk someone could have just changed the polarity and not bothered to chech
what it was supposed to be.

I agree the commit message should be more vocal about the effects on
existing DTS.
I can imagine that all users (out of tree, in this case) inverted
polarity in DTS based on what's implemented. You could go with some
trivial hack, like I did for one of codecs - see 738455858a2d ("ASoC:
codecs: wsa881x: Use proper shutdown GPIO polarity"), but I remember
Mark Brown rejected similar commit for newer drivers.
I don't think there's any out-of-tree user, because when we started
using the recently driver, it required lots of fixes to even work at
all. I'll let Kieran and Umang comment on that, I haven't follow the
development in details.

indeed, initially we had to put up fixes like :

14a60786d72e ("media: imx335: Set reserved register to default value")
81495a59baeba ("media: imx335: Fix active area height discrepency")

to make the sensor work properly on our platforms. Only after that we had a base to support more capabilities on the sensor (multiple lanes support, flips, TPG etc.)







[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux