On Tuesday 05 July 2022 14:55:10 Krzysztof Kozlowski wrote: > On 05/07/2022 14:15, Pali Rohár wrote: > >>>> You need to describe the items, if it is really two items. However your > >>>> example has only one item, so this was not tested and won't work. > >>> > >>> Ehm? Example has two items in the reg. > >> > >> No, you have exactly one item. > >> <0x13 0x1d> > >> > >> Two items are for example: > >> <0x13 0x1d>, <0x23 0x1d> > > > > Ok. So I should change maxItems to 1 in this case? > > Yes > > > > > And how you want to describe those items? > > In that case, no need to describe. > > > > >>> > >>>> You'll get warning from Rob's robot soon... but you should test the > >>>> bindings instead. > >>> > >>> I have tested bindings on the real hardware and it is working fine > >>> together with the driver from patch 2/2. > >> > >> Bindings cannot be tested on real hardware. Bindings are tested with > >> dt_binding_check, as explained in writing-schema.rst > > > > Ou... this is something which I was not able to run, it just does not > > work, throws lot of python dependency hell errors and it spend more than > > hour with it. So sorry, I really cannot run it. Maybe it would be a wise > > to provide web service for these checks for those who cannot run them > > locally? > > It's one pip command to install and one make command to run... I would > say easy to start using, unless of course you use some unusual distro > without Python 3 (cannot believe nowadays...) or without pip. > > Rob's bot will test it for you. Ok, so lets wait for the robot. After that I will try to fix found issues and send a new patch version. > Anyway, in such case please mark your bindings always as RFT, so we will > not waste time on reviewing obvious stuff which is found by automated > tools. I think we both agree that reviewers time should not be used for > trivial stuff already pointed out by compiler/linter/automation. Yes! > > > >>> > >>>>> + > >>>>> + "#address-cells": > >>>>> + const: 1 > >>>>> + > >>>>> + "#size-cells": > >>>>> + const: 0 > >>>>> + > >>>>> +patternProperties: > >>>>> + "^multi-led@[0-7]$": > >>>>> + type: object > >>>>> + $ref: leds-class-multicolor.yaml# > >>>> > >>>> This looks incorrect, unless you rebased on my patchset? > >>> > >>> So what is the correct? (I used inspiration from > >>> cznic,turris-omnia-leds.yaml file) > >> > >> Which according to current multicolor bindings is not correct. Correct > >> is pwm-multicolor. However if you rebase on [1], it looks fine, except > >> missing unevaluatedProperties. > > > > Ok. So does it mean that I should just add > > "unevaluatedProperties: false"? > > Yes, on that level of indentation, so: > $ref: leds-class-multicolor.yaml# > unevaluatedProperties: false Ok. > > > >> [1] > >> https://lore.kernel.org/all/20220624112106.111351-1-krzysztof.kozlowski@xxxxxxxxxx/ > >> > >>> > >>>>> + > >>>>> + properties: > >>>>> + reg: > >>>>> + minimum: 0 > >>>>> + maximum: 7 > >>>>> + > >>>>> + required: > >>>>> + - reg > >>>>> + > >>>>> +additionalProperties: false > >>>>> + > >>>>> +examples: > >>>>> + - | > >>>>> + > >>>> > >>>> No blank line. > >>> > >>> Ok. > >>> > >>>>> + #include <dt-bindings/leds/common.h> > >>>>> + > >>>>> + cpld@3,0 { > >>>> > >>>> Generic node name. > >>> > >>> Is not cpld name generic enough? > >> > >> No, it means nothing to me. Just like "a", "ashjd" or "wrls". > > > > If you never heard about it, I would suggest to read something about > > Programmable logic devices. It is interesting category of hardware with > > which you can play. CPLD and FPGA are very often used in lot of products > > and FPGA is very easy for playing and programming custom logic. > > The are many different acronyms in the language so without context might > be tricky to connect the dots. Anyway, playing with FPGA is really a fun! > > > > For example on wikipedia is list of different technologies of > > programmable logic devices: > > https://en.wikipedia.org/wiki/Programmable_logic_device > > > > So if you want more generic name, just name it "pld"? > > That one would be fine. > > > But as it is CPLD > > device I would suggest to name it really as "cpld". It does not matter > > from which manufactor you have CPLD, just like it does not matter from > > which manufactor you have NAND. > > Then cpld is fine as well. Ok, so stick with cpld. > > > > From bus point of view, cpld is like nand or nor nodes in DTS. All of > > them refers to specific memory map of chip selects on the local bus. > > > >> "The name of a node should be somewhat generic, reflecting the function > >> of the device and not its precise programming > >> model. If appropriate, the name should be one of the following choices:" > > > > Hm... You forgot to send what are those "choices:"? > > I didn't, I just assumed you will Google it (or use other web-search > engine of your choice) to get the spec. As this is a quote, Google > results should be very accurate. No need to duplicate entire pages of > publicly available specification. This was the first thing which I did when I read email. No usable result. So the next thing was that I started git grep on the linux tree. Again no result. So at the end I come to the conclusion that you forgot to copy+paste whole quote or something like that. Now I started searching a bit more and found it in following documentation: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation Original link to the quote would be useful (but now I have it). > Best regards, > Krzysztof
