Hi Niklas, On 02/11/2018 10:34, Niklas Söderlund wrote: > Hi Laurent, Jacopo > > Thanks for your comments. > > On 2018-10-05 13:02:53 +0300, Laurent Pinchart wrote: >> Hi Jacopo, >> >> On Friday, 5 October 2018 11:49:45 EEST jacopo mondi wrote: >>> On Fri, Oct 05, 2018 at 01:00:47AM +0300, Laurent Pinchart wrote: >>>> On Friday, 5 October 2018 00:42:17 EEST Laurent Pinchart wrote: >>>>> On Thursday, 4 October 2018 23:41:34 EEST Niklas Söderlund wrote: >>>>>> From: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx> >>>>>> >>>>>> The CSI-2 transmitters can use a different number of lanes to transmit >>>>>> data. Make the data-lanes mandatory for the endpoints describe the >>>>> >>>>> s/describe/that describe/ ? >>>>> >>>>>> transmitters as no good default can be set to fallback on. >>>>>> >>>>>> Signed-off-by: Niklas Söderlund >>>>>> <niklas.soderlund+renesas@xxxxxxxxxxxx> >>>>>> --- >>>>>> >>>>>> Documentation/devicetree/bindings/media/i2c/adv748x.txt | 3 +++ >>>>>> 1 file changed, 3 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/adv748x.txt >>>>>> b/Documentation/devicetree/bindings/media/i2c/adv748x.txt index >>>>>> 5dddc95f9cc46084..f9dac01ab795fc28 100644 >>>>>> --- a/Documentation/devicetree/bindings/media/i2c/adv748x.txt >>>>>> +++ b/Documentation/devicetree/bindings/media/i2c/adv748x.txt >>>>>> @@ -50,6 +50,9 @@ are numbered as follows. >>>>>> >>>>>> The digital output port nodes must contain at least one endpoint. >>>>>> >>>>>> +The endpoints described in TXA and TXB ports must if present contain >>>>>> +the data-lanes property as described in video-interfaces.txt. >>>>>> + >>>>> >>>>> Would it make sense to merge those two paragraphs, as they refer to the >>>>> same endpoint ? >>>>> >>>>> "The digital output port nodes, when present, shall contain at least one >>>>> endpoint. Each of those endpoints shall contain the data-lanes property >>>>> as described in video-interfaces.txt." >>>>> >>>>> (DT bindings normally use "shall" instead of "must", but that hasn't >>>>> really been enforced.) >>>>> >>>>> If you want to keep the paragraphs separate, I would recommend using >>>>> "digital output ports" instead of "TXA and TXB" in the second paragraph >>>>> for consistency (or the other way around). >>>>> >>>>> I'm fine with any of the above option, so please pick your favourite, >>>>> and add >>>>> >>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> >>>> >>>> I just realized that TXB only supports a single data lane, so we may want >>>> not to have a data-lanes property for TXB. >>> >>> Isn't it better to restrict its value to <1> but make it mandatory >>> anyhow? I understand conceptually that property should not be there, >>> as it has a single acceptable value, but otherwise we need to traeat >>> differently the two output ports, in documentation and code. >> >> The two ports are different, so I wouldn't be shocked if we handled them >> differently :-) I believe it would actually reduce the code size (and save CPU >> cycles at runtime). > > I'm leaning towards Jacopo on this that I think it's more clear to treat > the two the same. I also think it adheres to the notion that DT shall > describe hardware which I think this adds value. Also I only have > datasheets for adv7482 so I can't be sure other adv748x don't support > more then one lane on TXB. > > I do not feel strongly about this so I'm open to treating them > differently. I might keep it as is for v3 if no one screams to loud :-) I personally think that TXA and TXB are 'the same' entities, just in different configurations, and so I would say they are 'the same' too. For reference, from the adv748x.h header file: * The ADV748x range of receivers have the following configurations: * * Analog HDMI MHL 4-Lane 1-Lane * In In CSI CSI * ADV7480 X X X * ADV7481 X X X X X * ADV7482 X X X X So both the ADV7481 and ADV7482 have both a TXA and a TXB, where the ADV7480 has only the TXA. TXB is always only 'one-lane' -- Kieran > >> >>> Why not inserting a paragraph with the required endpoint properties >>> description? >>> >>> Eg: >>> >>> Required endpoint properties: >>> - data-lanes: See "video-interfaces.txt" for description. The property >>> is mandatory for CSI-2 output endpoints and the accepted values >>> depends on which endpoint the property is applied to: >>> - TXA: accepted values are <1>, <2>, <4> >>> - TXB: accepted value is <1> >>> >>>>>> Ports are optional if they are not connected to anything at the >>>>>> hardware level. >>>>>> >>>>>> Example: >> >> -- >> Regards, >> >> Laurent Pinchart >>
