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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <19c22201-e3ec-4d07-97ae-c149b172e480@ideasonboard.com>
Date: Tue, 14 Jan 2025 13:50:08 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Mauro Carvalho Chehab <mchehab@...nel.org>, linux-media@...r.kernel.org,
 linux-kernel@...r.kernel.org, Devarsh Thakkar <devarsht@...com>,
 Jai Luthra <jai.luthra@...asonboard.com>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>, devicetree@...r.kernel.org,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>
Subject: Re: [PATCH 17/19] media: dt-bindings: ti,ds90ub960: Add "i2c-addr"
 link property

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@...asonboard.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@...asonboard.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
>> Cc: devicetree@...r.kernel.org
>> Cc: Rob Herring <robh@...nel.org>
>> Cc: Krzysztof Kozlowski <krzk+dt@...nel.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