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). > 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
