[PATCH 00/19] media: i2c: ds90ub9xx: Error handling, UB9702 improvements

[PATCH 00/19] media: i2c: ds90ub9xx: Error handling, UB9702 improvements

[Tomi Valkeinen]

Hi,

This series has two main parts: 1) add error handling all around, and 2)
update the drivers according to latest (mostly non-public) information
from TI.

The "Update UB9702 init sequences" patch basically rewrites the init
sequence from scratch, and to make that patch easier to read, the
previous patch first removes the current init sequence. In the final
version these two patches need to be squashed together.

 Tomi

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Jai Luthra (5):
      media: i2c: ds90ub953: Speed-up I2C watchdog timer
      media: i2c: ds90ub960: Enable SSCG for UB9702
      media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property
      media: i2c: ds90ub960: Configure serializer using back-channel
      media: i2c: ds90ub9xx: Set serializer temperature ramp

Tomi Valkeinen (14):
      media: i2c: ds90ub953: Fix error prints
      media: i2c: ds90ub913: Align ub913_read() with other similar functions
      media: i2c: ds90ub9xx: Add err parameter to read/write funcs
      media: i2c: ds90ub960: Add error handling to multiple places
      media: i2c: ds90ub953: Add error handling to ub953_log_status()
      media: i2c: ds90ub913: Add error handling to ub913_log_status()
      media: i2c: ds90ub960: Move UB9702 registers to a separate section
      media: i2c: ds90ub960: Add UB9702 specific registers
      media: i2c: ds90ub960: Split ub960_init_tx_ports()
      media: i2c: ds90ub960: Refresh ub960_init_tx_ports_ub9702()
      media: i2c: ds90ub960: Add RX port iteration support
      media: i2c: ds90ub960: Move all RX port init code into ub960_init_rx_ports()
      media: i2c: ds90ub960: Remove old ub9702 RX port init code (SQUASH)
      media: i2c: ds90ub960: Update UB9702 init sequences

 .../bindings/media/i2c/ti,ds90ub960.yaml           |    7 +
 drivers/media/i2c/ds90ub913.c                      |   78 +-
 drivers/media/i2c/ds90ub953.c                      |  170 +-
 drivers/media/i2c/ds90ub960.c                      | 2266 +++++++++++++++-----
 include/media/i2c/ds90ub9xx.h                      |   15 +
 5 files changed, 1927 insertions(+), 609 deletions(-)
---
base-commit: 40ed9e9b2808beeb835bd0ed971fb364c285d39c
change-id: 20250110-ub9xx-improvements-9172b44eb0bc

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

[PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Tomi Valkeinen]

From: Jai Luthra <jai.luthra@ideasonboard.com>

The serializer's I2C address on the FPD-Link bus is usually communicated
to the deserializer once the forward-channel is established. But in some
cases it might be necessary to program the serializer (over the
back-channel) before the forward-channel is established.

This can be used e.g. to correct serializer configuration which
otherwise would prevent the FC to be enabled.

Add a new optional property to specify the I2C address of the
serializer.

Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: devicetree@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
---
 Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
index 0b71e6f911a8..e17b508b6409 100644
--- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
@@ -75,6 +75,13 @@ properties:
               address on the I2C bus where the deserializer resides are
               forwarded to the serializer.
 
+          i2c-addr:
+            $ref: /schemas/types.yaml#/definitions/uint32
+            description:
+              The strap I2C address of the serializer. This is used by the
+              deserializer to internally communicate to the serializer over the
+              FPD-Link back-channel.
+
           ti,rx-mode:
             $ref: /schemas/types.yaml#/definitions/uint32
             enum:

-- 
2.43.0

Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Krzysztof Kozlowski]

On Fri, Jan 10, 2025 at 11:14:17AM +0200, Tomi Valkeinen wrote:
> From: Jai Luthra <jai.luthra@ideasonboard.com>
> 
> The serializer's I2C address on the FPD-Link bus is usually communicated
> to the deserializer once the forward-channel is established. But in some
> cases it might be necessary to program the serializer (over the
> back-channel) before the forward-channel is established.
> 
> This can be used e.g. to correct serializer configuration which
> otherwise would prevent the FC to be enabled.
> 
> Add a new optional property to specify the I2C address of the
> serializer.
> 
> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>

Why only these folks? Why not all of the maintainers?

Anyway, Please drop the autogenerated scripts/get_maintainer.pl CC-entries from
commit msg. There is no single need to store automated output of
get_maintainers.pl in the git log. It can be easily re-created at any
given time, thus its presence in the git history is redundant and
obfuscates the log.

If you need it for your own patch management purposes, keep it under the
--- separator.

> ---
>  Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> index 0b71e6f911a8..e17b508b6409 100644
> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
> @@ -75,6 +75,13 @@ properties:
>                address on the I2C bus where the deserializer resides are
>                forwarded to the serializer.
>  
> +          i2c-addr:
> +            $ref: /schemas/types.yaml#/definitions/uint32

Why isn't this part of reg, if that's the same device? If that is not
the same device, you are not expected to encode addresses of other
devices in this device. Address of 'foo' is not a property of device
'bar'. Phandles or graphs express relationships between devices.

Best regards,
Krzysztof

Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Tomi Valkeinen]

Hi,

On 11/01/2025 12:31, Krzysztof Kozlowski wrote:
> On Fri, Jan 10, 2025 at 11:14:17AM +0200, Tomi Valkeinen wrote:
>> From: Jai Luthra <jai.luthra@ideasonboard.com>
>>
>> The serializer's I2C address on the FPD-Link bus is usually communicated
>> to the deserializer once the forward-channel is established. But in some
>> cases it might be necessary to program the serializer (over the
>> back-channel) before the forward-channel is established.
>>
>> This can be used e.g. to correct serializer configuration which
>> otherwise would prevent the FC to be enabled.
>>
>> Add a new optional property to specify the I2C address of the
>> serializer.
>>
>> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> Cc: devicetree@vger.kernel.org
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> 
> Why only these folks? Why not all of the maintainers?

The whole series is sent to the media list and maintainers. I thought 
this single patch doesn't warrant sending the whole series to DT list 
and maintainers, so I cc'd them here.

> Anyway, Please drop the autogenerated scripts/get_maintainer.pl CC-entries from
> commit msg. There is no single need to store automated output of
> get_maintainers.pl in the git log. It can be easily re-created at any
> given time, thus its presence in the git history is redundant and
> obfuscates the log.

I think that's a valid point.

> If you need it for your own patch management purposes, keep it under the
> --- separator.

I'm using b4. I don't know how to do that with b4, but I'll look into it.

>> ---
>>   Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> index 0b71e6f911a8..e17b508b6409 100644
>> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>> @@ -75,6 +75,13 @@ properties:
>>                 address on the I2C bus where the deserializer resides are
>>                 forwarded to the serializer.
>>   
>> +          i2c-addr:
>> +            $ref: /schemas/types.yaml#/definitions/uint32
> 
> Why isn't this part of reg, if that's the same device? If that is not
> the same device, you are not expected to encode addresses of other
> devices in this device. Address of 'foo' is not a property of device
> 'bar'. Phandles or graphs express relationships between devices.

With the understanding of the HW I have right now, I would have added 
the i2c address as the address of the serializer node, with reg 
property. I would probably also do a few other changes to the bindings...

But as we already have the current bindings, adding the i2c-addr felt 
like an easy way to keep backwards compatibility and add the address of 
the serializer.

However, thinking about this more, maybe we could just go and add the 
address of the serializer with reg, in the ds90ub953 bindings. It's the 
ub960 driver that needs the address, but it shouldn't be much trouble to 
get that from the ub953's data.

But we need to keep the address optional to keep the backwards 
compatibility. If it's not defined, the ub960 will automatically receive 
the serializer's address when the link goes up (as it is handled now).

  Tomi

Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Krzysztof Kozlowski]

On 14/01/2025 12:50, Tomi Valkeinen wrote:
> Hi,
> 
> On 11/01/2025 12:31, Krzysztof Kozlowski wrote:
>> On Fri, Jan 10, 2025 at 11:14:17AM +0200, Tomi Valkeinen wrote:
>>> From: Jai Luthra <jai.luthra@ideasonboard.com>
>>>
>>> The serializer's I2C address on the FPD-Link bus is usually communicated
>>> to the deserializer once the forward-channel is established. But in some
>>> cases it might be necessary to program the serializer (over the
>>> back-channel) before the forward-channel is established.
>>>
>>> This can be used e.g. to correct serializer configuration which
>>> otherwise would prevent the FC to be enabled.
>>>
>>> Add a new optional property to specify the I2C address of the
>>> serializer.
>>>
>>> Signed-off-by: Jai Luthra <jai.luthra@ideasonboard.com>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> Cc: devicetree@vger.kernel.org
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
>>
>> Why only these folks? Why not all of the maintainers?
> 
> The whole series is sent to the media list and maintainers. I thought 
> this single patch doesn't warrant sending the whole series to DT list 
> and maintainers, so I cc'd them here.


I was wondering why only some of the DT maintainers, not all? My usual
assumption is: you are not using get_maintainers.pl or you are working
on an old kernel.


> 
>> Anyway, Please drop the autogenerated scripts/get_maintainer.pl CC-entries from
>> commit msg. There is no single need to store automated output of
>> get_maintainers.pl in the git log. It can be easily re-created at any
>> given time, thus its presence in the git history is redundant and
>> obfuscates the log.
> 
> I think that's a valid point.
> 
>> If you need it for your own patch management purposes, keep it under the
>> --- separator.
> 
> I'm using b4. I don't know how to do that with b4, but I'll look into it.
> 
>>> ---
>>>   Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>>> index 0b71e6f911a8..e17b508b6409 100644
>>> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>>> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>>> @@ -75,6 +75,13 @@ properties:
>>>                 address on the I2C bus where the deserializer resides are
>>>                 forwarded to the serializer.
>>>   
>>> +          i2c-addr:
>>> +            $ref: /schemas/types.yaml#/definitions/uint32
>>
>> Why isn't this part of reg, if that's the same device? If that is not
>> the same device, you are not expected to encode addresses of other
>> devices in this device. Address of 'foo' is not a property of device
>> 'bar'. Phandles or graphs express relationships between devices.
> 
> With the understanding of the HW I have right now, I would have added 
> the i2c address as the address of the serializer node, with reg 
> property. I would probably also do a few other changes to the bindings...
> 
> But as we already have the current bindings, adding the i2c-addr felt 
> like an easy way to keep backwards compatibility and add the address of 
> the serializer.
> 
> However, thinking about this more, maybe we could just go and add the 
> address of the serializer with reg, in the ds90ub953 bindings. It's the 
> ub960 driver that needs the address, but it shouldn't be much trouble to 
> get that from the ub953's data.
> 
> But we need to keep the address optional to keep the backwards 
> compatibility. If it's not defined, the ub960 will automatically receive 
> the serializer's address when the link goes up (as it is handled now).


The 'reg' can grow and should not cause ABI issues, because
implementations should just ignore additional entry.

Best regards,
Krzysztof

Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Konstantin Ryabitsev]

On Tue, Jan 14, 2025 at 01:50:08PM +0200, Tomi Valkeinen wrote:
> > If you need it for your own patch management purposes, keep it under the
> > --- separator.
> 
> I'm using b4. I don't know how to do that with b4, but I'll look into it.

You just put them under --- in your git commit message and b4 will preserve
it as-is when it renders the email.

-K

Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Tomi Valkeinen]

Hi,

On 15/01/2025 17:53, Konstantin Ryabitsev wrote:
> On Tue, Jan 14, 2025 at 01:50:08PM +0200, Tomi Valkeinen wrote:
>>> If you need it for your own patch management purposes, keep it under the
>>> --- separator.
>>
>> I'm using b4. I don't know how to do that with b4, but I'll look into it.
> 
> You just put them under --- in your git commit message and b4 will preserve
> it as-is when it renders the email.

Alright, thanks!

  Tomi

Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Krzysztof Kozlowski]

On 15/01/2025 17:09, Tomi Valkeinen wrote:
> Hi,
> 
> On 15/01/2025 17:53, Konstantin Ryabitsev wrote:
>> On Tue, Jan 14, 2025 at 01:50:08PM +0200, Tomi Valkeinen wrote:
>>>> If you need it for your own patch management purposes, keep it under the
>>>> --- separator.
>>>
>>> I'm using b4. I don't know how to do that with b4, but I'll look into it.
>>
>> You just put them under --- in your git commit message and b4 will preserve
>> it as-is when it renders the email.
> 
> Alright, thanks!
> 
That's exactly what I wrote:

"If you need it for your own patch management purposes, keep it under
the --- separator."

It's nothing related to b4 - it comes from standard git commit and git
send-email.

Best regards,
Krzysztof

Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Tomi Valkeinen]

Hi,

On 15/01/2025 18:46, Krzysztof Kozlowski wrote:
> On 15/01/2025 17:09, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 15/01/2025 17:53, Konstantin Ryabitsev wrote:
>>> On Tue, Jan 14, 2025 at 01:50:08PM +0200, Tomi Valkeinen wrote:
>>>>> If you need it for your own patch management purposes, keep it under the
>>>>> --- separator.
>>>>
>>>> I'm using b4. I don't know how to do that with b4, but I'll look into it.
>>>
>>> You just put them under --- in your git commit message and b4 will preserve
>>> it as-is when it renders the email.
>>
>> Alright, thanks!
>>
> That's exactly what I wrote:
> 
> "If you need it for your own patch management purposes, keep it under
> the --- separator."
> 
> It's nothing related to b4 - it comes from standard git commit and git
> send-email.

Yep. For some odd reason I was under the impression that I can't have 
that in a commit message in my local branch, but would have to add the 
---, and the lines after that, to the mail file. And so, as b4 handles 
creating and sending the mails, I wasn't sure how to get in between.

This does make my life a bit easier, so I'm glad this topic came up =).

  Tomi

Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Tomi Valkeinen]

Hi,

On 15/01/2025 10:40, Krzysztof Kozlowski wrote:

>>> Why only these folks? Why not all of the maintainers?
>>
>> The whole series is sent to the media list and maintainers. I thought
>> this single patch doesn't warrant sending the whole series to DT list
>> and maintainers, so I cc'd them here.
> 
> 
> I was wondering why only some of the DT maintainers, not all? My usual
> assumption is: you are not using get_maintainers.pl or you are working
> on an old kernel.

It's simpler than that: a copy-paste mistake. I'm not sure how I managed 
to miss Conor there.

Any preference on cc'ing the DT maintainers and the dt-list only for 
this patch (I'll use --- this time, I promise!), or just sending the 
whole series also to DT people?

  Tomi

Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr" link property

[Krzysztof Kozlowski]

On 15/01/2025 18:19, Tomi Valkeinen wrote:
> Hi,
> 
> On 15/01/2025 10:40, Krzysztof Kozlowski wrote:
> 
>>>> Why only these folks? Why not all of the maintainers?
>>>
>>> The whole series is sent to the media list and maintainers. I thought
>>> this single patch doesn't warrant sending the whole series to DT list
>>> and maintainers, so I cc'd them here.
>>
>>
>> I was wondering why only some of the DT maintainers, not all? My usual
>> assumption is: you are not using get_maintainers.pl or you are working
>> on an old kernel.
> 
> It's simpler than that: a copy-paste mistake. I'm not sure how I managed 
> to miss Conor there.
> 
> Any preference on cc'ing the DT maintainers and the dt-list only for 
> this patch (I'll use --- this time, I promise!), or just sending the 
> whole series also to DT people?
I think only Rob uses DT list entirely. I use both - DT list and being
directly CC-ed on emails, and I think Conor relies on being Cc-ed
directly. IOW, just cc all maintainers + DT list. Unless you touch
multiple subsystems, there is rarely a point to strip get_maintainers.pl
output from maintainers/reviewers/submitters. Do not confuse in skipping
non-maintainers pointed by Git-fallback (not applicable to b4, I think),
because these should be used carefully.

Best regards,
Krzysztof