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: <58b309d9-de03-4818-8d38-a27cc68466db@ideasonboard.com>
Date: Mon, 2 Jun 2025 10:00:15 +0300
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Yemike Abhilash Chandra <y-abhilashchandra@...com>
Cc: hverkuil@...all.nl, sakari.ailus@...ux.intel.com,
 laurent.pinchart@...asonboard.com, vaishnav.a@...com, u-kumar1@...com,
 jai.luthra@...ux.dev, linux-media@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 mchehab@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org
Subject: Re: [PATCH 1/2] media: dt-bindings: ti,ds90ub960: Add bindings for
 DS90UB954-Q1

Hi,

On 28/05/2025 08:35, Yemike Abhilash Chandra wrote:
> Hi Tomi,
> 
> Thanks for the review.
> 
> On 27/05/25 10:30, Tomi Valkeinen wrote:
>> Hi,
>>
>> On 23/05/2025 11:36, Yemike Abhilash Chandra wrote:
>>> DS90UB954-Q1 is an FPDLink-III deserializer that is mostly register
>>> compatible with DS90UB960-Q1. The main difference is that it supports
>>> half of the RX and TX ports, i.e. 2x FPDLink RX ports and 1x CSI TX
>>> port. Therefore, add support for DS90UB954 within the existing bindings.
>>>
>>> Link: https://www.ti.com/lit/gpn/ds90ub954-q1
>>> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@...com>
>>> ---
>>>   Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/i2c/
>>> ti,ds90ub960.yaml b/Documentation/devicetree/bindings/media/i2c/
>>> ti,ds90ub960.yaml
>>> index 4dcbd2b039a5..b2d4300d7846 100644
>>> --- a/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>>> +++ b/Documentation/devicetree/bindings/media/i2c/ti,ds90ub960.yaml
>>> @@ -19,6 +19,7 @@ allOf:
>>>   properties:
>>>     compatible:
>>>       enum:
>>> +      - ti,ds90ub954-q1
>>>         - ti,ds90ub960-q1
>>>         - ti,ds90ub9702-q1
>>>   
>>
>> Does this pass the dt checks? The binding lists ports 0-5 as required.
> 
> Thanks for pointing this out. It is passing DT checks since we have marked
> port 4 and port 5 as disabled in DT, but I now understand that approach is
> not acceptable.
> 
> Ports 0–3 are documented as FPD-Link inputs, but on UB954, only ports 0
> and 1 are inputs,
> while port 2 is CSI TX. Should I conditionally modify required ports for
> UB954(0-2) and
> UB960/UB9702 (0-5), even though port 2's description would mismatch?
> (In bindings it would be described as FPD-Link input but it will be
> modeled as CSI TX in DT).
> 
> Alternatively, we can describe the ports block separately for each
> compatible to
> ensure correctness. Please let me know which approach you prefer.

I think you have two options here: either use ports 0, 1, 2 for UB954,
or use ports 0, 1, 4 for UB954. I think the first option makes more sense.

The bindings have to be correct, you can't do "even though port 2's
description would mismatch". Probably the clearest way is just to have a
conditional and define all the ports separately for ub954 and for the rest.

You can check e.g. renesas,du.yaml, which does rather complex conditionals.

I do wonder, though, if the port definitions could be more brief by
somehow defining the RX and TX port details only once, instead of
replicating the same for every port.

Note that the bindings also allow link0-3, which for UB954 should be
link0-1.

 Tomi


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ