Hello Jacek, On 10/05/2019 22:42, Jacek Anaszewski wrote: > Hi Christian, > > On 5/10/19 9:50 PM, Christian Mauderer wrote: >> On 07/05/2019 11:52, Christian Mauderer wrote: >>> On 06/05/2019 22:25, Pavel Machek wrote: >>>> Hi! >>>> >>>>>>> Ok, I'm afraid I caused this. What should the compatible be, then? >>>>>> >>>>>> Knowing nothing about the h/w other than the above description: >>>>>> ubiquiti,aircube-leds >>>>>> >>>>>> Not sure if that's a registered or correct vendor prefix though. >>>>>> >>>>>> Rob >>>>>> >>>>> >>>>> Where would such a vendor prefix be registered? Does that mean that >>>>> only >>>>> the vendor is allowed to use it? In that case: How would a reverse >>>>> engineered prefix look like? >>>> >>>> You can use it, too. It is in >>>> Documentation/devicetree/bindings/vendor-prefixes.txt : >>>> >>>> ubnt Ubiquiti Networks >>>> >>>> So you can probably use ubnt, prefix. >>>> >>>>> (still with some missing parts like U-Boot) about two weeks later. >>>>> I had >>>>> a look at it and they are not using a device tree. So there is no >>>>> "official" string that I could deduce from that archive. >>>> >>>> Mainline is the master. You are more "official" than them ;-). >>>> Pavel >>>> >>> >>> Hello >>> >>> let me summarize the direction before I create a v4: >>> >>> Rob Herring suggested "ubnt,acb-spi-led" for the binding name in his >>> Mail from 06.05.2019 17:59 UTC. If no one objects, I'll use that. >>> >>> With the more specific name I'll remove the off-value and max-value from >>> the device tree. Instead I'll create some look up table in the driver. >>> based on the name or go back to the defines like in the v1 patch. What >>> kind of solution would be preferable depends on the next question: >>> >>> How should I name the driver? Should I use a device specific name like >>> in v1 again (most likely now acb-spi-led)? That would allow to >>> potentially add a hardware supported blinking in that driver. The >>> alternative would be the more generic name that it has now >>> (leds-spi-byte) without any plans to add the blinking but it could be >>> potentially used for example for a digital potentiometer based >>> brightness setting. >>> >>> Note that I didn't really had planned to implement the blinking support >>> because I don't have a use case for it. So it would be either a feature >>> that I would add because someone insists. Or it could be added in the >>> future by a user who wants that feature (maybe Ubiquiti when they >>> upgrade their kernel?). >>> >>> If it is a required feature for that driver: Please note that although >>> of course I would do some basic tests during development it would be a >>> mostly unused and therefore untested feature. >>> >>> Best regards >>> >>> Christian >>> >> >> Hello, >> >> sorry for repeating my question. I assume I wrote to much text hiding >> it: How should I name the driver? >> >> The name for the binding is clear (ubnt,acb-spi-led). Only the driver is >> left (keep leds-spi-byte or rename to leds-ubnt-acb-spi or something >> else). > > Why leds-spi-byte name would prevent addition of blink support? It can > be always added basing on OF compatible. If it is to be generic SPI > byte driver, then I'd use leds-spi-byte. Actually also the things > like allowed brightness levels could be determined basing on that, > and not in device tree, but in the driver. > > Please compare how e.g. drivers/leds/leds-is31fl32xx.c accomplishes > that. > I would have expected that adding a lot of device specific code (in that case blinking) to a multi-purpose driver would be bad style. But I'll go for the generic name if that is the accepted way. I already mentioned multiple times that my target is currently only the brightness. So the device specific code maybe is added quite a bit in the future anyway in which case it would still be possible to rename a part (if it isn't used otherwise) or at least split it into it's own c-file. I'll prepare a v4 in the near future and send it to the list. I only learned that it would be a good idea to wait for at least a day for some other opinions before doing that ;-) Best regards Christian
