[v2,0/5] imx335: Support additional link-freq and TPG

This series adds support for additional link frequency and
test pattern generator on IMX335.

Patch 1/5 is a drive-by patch which drops setting of a reserved
register. This register has no effect in operation/streaming of the
sensor.

Patch 2/5 adds usage of v4l2_link_freq_to_bitmap V4L2 helper.

Patch 3/4 supports for additional link-frequency supported by IMX335

Patch 4/5 is also a prep-up patch for TPG introduction(in 5/5), as the test
pattern needs sensor to be powered up to apply the test pattern.

Patch 5/5 adds the TPG.

Changes in v2:
- add new patch 2/5 to use v4l2_link_freq_to_bitmap
- fixup a return; in 4/5

Matthias Fend (1):
  media: i2c: imx335: Add support for test pattern generator

Umang Jain (4):
  media: i2c: imx335: Drop setting of 0x3a00 register
  media: imx335: Use v4l2_link_freq_to_bitmap helper
  media: i2c: imx335: Support multiple link frequency
  media: i2c: imx335: Refactor power sequence to set controls

 drivers/media/i2c/imx335.c | 250 ++++++++++++++++++++++++++++++-------
 1 file changed, 205 insertions(+), 45 deletions(-)


base-commit: d7cb6098f1d4866ae864cccdbb3fdcea1176a7f6

Hi,

On 1/31/24 4:02 PM, Matthias Fend wrote:
> Hi Kieran,
>
> Am 31.01.2024 um 10:52 schrieb Kieran Bingham:
>> Hi Umang,
>>
>> + Cc: Matthias
>>
>> Quoting Umang Jain (2024-01-31 05:52:04)
>>> Register 0x3a00 is a reserved field as per the IMX335 datasheet,
>>> hence shouldn't be set by the driver.
>>
>> We still need to explain more about why we're dropping this register
>> write, and what effects it causes.
>>
>> Matthias - I believe this stemmed from the work you did, and I think I
>> recall that you stated this register write broke the CSI2 configuration?
>>
>> Can you clarify anything here for us please?
>
> yes, that's correct.
>
> Since this driver originally did not work in my setup, I came across 
> this register while searching for differences to my working reference 
> configuration.
> With the default value of this register (0x00), the driver works 
> perfectly. With the value previously written to it by the driver 
> (0x01), I cannot receive any frames.
> The problem may depend on the link frequency used.
> I can only use and test a frequency of 445.5MHz on my hardware. Since 
> only link frequencies of 594MHz were supported so far, this may not 
> have been a problem.
>
> Unfortunately I do not have a description of this register, so I can 
> only speculate about the exact cause.

Is it worth to frame the commit message around this speculation ?

My setup has no effect with this register being set or not.
>
> ~Matthias
>
>>
>> -- 
>> Kieran
>>
>>
>>>
>>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
>>> ---
>>>   drivers/media/i2c/imx335.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
>>> index 7a37eb327ff4..927b4806a5d7 100644
>>> --- a/drivers/media/i2c/imx335.c
>>> +++ b/drivers/media/i2c/imx335.c
>>> @@ -249,7 +249,6 @@ static const struct imx335_reg 
>>> mode_2592x1940_regs[] = {
>>>          {0x3794, 0x7a},
>>>          {0x3796, 0xa1},
>>>          {0x37b0, 0x36},
>>> -       {0x3a00, 0x01},
>>>   };
>>>     static const struct imx335_reg raw10_framefmt_regs[] = {
>>> -- 
>>> 2.41.0
>>>

Hi folks,

On Wed, Feb 07, 2024 at 09:51:32AM +0530, Umang Jain wrote:
> Hi,
> 
> On 1/31/24 4:02 PM, Matthias Fend wrote:
> > Hi Kieran,
> > 
> > Am 31.01.2024 um 10:52 schrieb Kieran Bingham:
> > > Hi Umang,
> > > 
> > > + Cc: Matthias
> > > 
> > > Quoting Umang Jain (2024-01-31 05:52:04)
> > > > Register 0x3a00 is a reserved field as per the IMX335 datasheet,
> > > > hence shouldn't be set by the driver.
> > > 
> > > We still need to explain more about why we're dropping this register
> > > write, and what effects it causes.
> > > 
> > > Matthias - I believe this stemmed from the work you did, and I think I
> > > recall that you stated this register write broke the CSI2 configuration?
> > > 
> > > Can you clarify anything here for us please?
> > 
> > yes, that's correct.
> > 
> > Since this driver originally did not work in my setup, I came across
> > this register while searching for differences to my working reference
> > configuration.
> > With the default value of this register (0x00), the driver works
> > perfectly. With the value previously written to it by the driver (0x01),
> > I cannot receive any frames.
> > The problem may depend on the link frequency used.
> > I can only use and test a frequency of 445.5MHz on my hardware. Since
> > only link frequencies of 594MHz were supported so far, this may not have
> > been a problem.
> > 
> > Unfortunately I do not have a description of this register, so I can
> > only speculate about the exact cause.
> 
> Is it worth to frame the commit message around this speculation ?
> 
> My setup has no effect with this register being set or not.

Very interesting case indeed.

Please elaborate this a little in the commit message, but the message
shouldn't be that because the register is marked rewerved, it shouldn't be
written (it's a commonplace when it comes to sensors :().