On Mon, May 11, 2020 at 04:59:11PM +0200, Ricardo Cañuelo wrote: > Hi Rob, > > What's your opinion on this? > > Some context: It's about bindings that define signed integer properties > with range checks that go below and above zero. The schema checker fails > because, apparently, it interprets every cell value as an uint32, which > makes the range check fail for negative numbers. The real fix is dtc needs to carry the sign thru to the yaml output format. In the mean time, perhaps the schema should have an unsigned range for signed types. Rob > > On mié 06-05-2020 18:53:20, Laurent Pinchart wrote: > > 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 > > I haven't found any examples of yaml bindings defining signed integer > properties such as this, what's the norm in this case? Do you agree with > any of the proposed solutions? Do you have a better suggestion? > > Thanks, > Ricardo _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
