On 31/07/2024 11:02, Kieran Bingham wrote:
Quoting Umang Jain (2024-07-31 06:41:35)
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@ideasonboard.com > --- > 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.)
I would also add that we had to provide control for the regulators to be able to power the device as well in fea91ee73b7c ("media: i2c: imx335: Enable regulator supplies").
Hm? That's not a proof of anything. Supplies are often turned on by default.
Given the driver was posted from Intel, I would have anticipated perhaps the driver was in fact only actually tested by Intel on ACPI platforms - yet with no ACPI table registered in the driver - even that could likely be considered broken.
Nope, that does not work like that. Their platforms and such sensors are often used on DT based boards. Not mentioning even PRP0001.
Based on that I have a high confidence that there are no current users of this driver (except us).
Nope, wrong conclusions, not that many arguments.
Best regards, Krzysztof