Re: [RFC PATCH] dt-bindings: display: ti,tfp410.txt: convert to yaml

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

On Tue, Apr 28, 2020 at 12:49:28PM +0300, Tomi Valkeinen wrote:
> On 28/04/2020 12:20, Ricardo Cañuelo wrote:
> 
> > 2) The definition of ti,deskew in the original binding seems to be
> > tailored to the current driver and the way it's defined may not be very
> > DT-friendly.
> > 
> >    This parameter maps to a 3-bit field in a hardware register that takes
> >    a value from 0 to 7, so the [-4, 3] range described for this would map
> >    to [000, 111]: -4 -> 000, -3 -> 001, -2 -> 010, ... 3 -> 111.
> > 
> >    Then, the driver parses the parameter (unsigned) and casts it to a
> >    signed integer to get a number in the [-4, 3] range.
> 
> Interestingly the current example has ti,deskew = <4>...
> 
> >    A vendor-specific property must have a type definition in json-schema,
> >    so if I translate the original bindings semantics directly, I should
> >    define ti,deskew as an int32, but this makes dt_binding_check fail if
> >    the property has a negative value in the example because of the
> >    internal representation of cells as unsigned integers:
> > 
> >       ti,deskew:0:0: 4294967293 is greater than the maximum of 2147483647
> 
> I don't quite understand this. We cannot have negative numbers in dts files? Or we can, but 
> dt_binding_check doesn't handle them correctly? Or that int32 is not supported in yaml bindings?
> 
> >    So I can think of two solutions to this:
> > 
> >    a) Keep the ti,deskew property as an uint32 and document the valid
> >    range ([-4, 3]) in the property description (this is what this patch
> >    does currently).
> > 
> >    b) Redefine this property to be closer to the datasheet description
> >    (ie. unsigned integers from 0 to 7) and adapt the driver accordingly.
> >    This would also let us define its range properly using minimum and
> >    maximum properties for it.
> > 
> >    I think (b) is the right thing to do but I want to know your
> >    opinion. Besides, I don't have this hardware at hand and if I updated
> >    the driver I wouldn't be able to test it.
> 
> I don't think anyone has used deskew property, so I guess changing it is not out of the question.
> 
> Laurent, did you have a board that needs deskew when you added it to tfp410?

I didn't if I remember correctly, I just mapped it to the hardware
features. The hardware register indeed takes a value between 0 and 7,
and that is mapped to [-4,3] x t(STEP). I don't mind either option.

-- 
Regards,

Laurent Pinchart