On Mon, May 6, 2019 at 12:03 PM Christian Mauderer <oss@xxxxxxxxxxxxx> wrote: > > On 06/05/2019 18:21, Rob Herring wrote: > > On Sun, May 5, 2019 at 3:00 PM <oss@xxxxxxxxxxxxx> wrote: > >> > >> From: Christian Mauderer <oss@xxxxxxxxxxxxx> > >> > >> This patch adds the binding documentation for a simple SPI based LED > >> controller which use only one byte for setting the brightness. > >> > >> Signed-off-by: Christian Mauderer <oss@xxxxxxxxxxxxx> > >> --- > >> > >> Changes compared to v2: > >> - None > >> > >> Changes compared to v1: > >> - rename ubnt-spi to leds-spi-byte > >> - rename "ubnt-spi,off_bright" and "ubnt-spi,max_bright" to > >> "leds-spi-byte,off-value" and "leds-spi-byte,max-value" and mark them required > >> - rename led-controller node to "led-controller" > >> - extend description > >> - remove SPI controller > >> - use "white:status" for the example label > >> > >> > >> .../bindings/leds/leds-spi-byte.txt | 47 +++++++++++++++++++ > >> 1 file changed, 47 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/leds/leds-spi-byte.txt > >> > >> diff --git a/Documentation/devicetree/bindings/leds/leds-spi-byte.txt b/Documentation/devicetree/bindings/leds/leds-spi-byte.txt > >> new file mode 100644 > >> index 000000000000..1dd6ab03a56d > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/leds/leds-spi-byte.txt > >> @@ -0,0 +1,47 @@ > >> +* Single Byte SPI LED Device Driver. > >> + > >> +The driver can be used for controllers with a very simple SPI protocol: Only one > >> +byte will be sent. The value of the byte can be any value between the off-value > >> +and max-value defined in the properties. > >> + > >> +One example where the driver can be used is the controller in Ubiquiti airCube > >> +ISP devices. That LED controller is based on a 8 bit microcontroller (SONiX > >> +8F26E611LA) that has been programmed to control the single LED of the device. > > > > What about power control of the uC? > > You mean if the uC receives a reset or power cycle independent of the > main controller? I don't think that this can happen on that board. But I > don't have any schematics to prove that. I was really only pointing out potential issues around "generic" bindings. The protocol interface is not the only thing. > >> +The controller supports four modes depending on the highest two bits in a byte: > >> +One setting for brightness, the other three provide different blink patterns. > > > > This part seems in no way generic. > > > > How does one support the blink patterns? > > You are correct that this part is not generic. But a multi-purpose > driver like the one I proposed could deliver a basic support for the > device by controlling the brightness. > > It's only a basic support so the blink patterns are not supported. Then you would have to change/extend the binding when you want to support that. With a specific compatible, you only need a driver change. > I had a look at the functions in "struct led_classdev". There is a > blink_set(..) function that expects that delay_on and delay_off can be > set independent. That's not possible for hardware supported blinking on > this device. The other function pattern_set(..) would allow an even more > universal interface. All possible patterns of the LED could be covered > in that but I don't think that this is true the other way round. > > So in my opinion the only thing that can be implemented in a useful way > for that controller is the brightness. What the OS can support evolves and should be independent of the binding. > > > >> +With the leds-spi-byte driver a basic support for the brightness mode of that > >> +controller can be easily achieved by setting the minimum and maximum to the > >> +brightness modes minimum and maximum byte value. > >> + > >> +Required properties: > >> +- compatible: Should be "leds-spi-byte". > > > > Generally, we don't do "generic" bindings like this. The exceptions > > are either we have confidence they really can be generic or they where > > created before we knew better. A sample size of 1 doesn't convince me > > the former is true. > > I could construct another sample (some SPI-based digital potentiometer > where you set values between 17 and 213) but I doubt that it would be a > good idea to fight for the name. > > My original target device is a quite special one: I don't have a chip > number. The controller Ubiquiti built here is based on a microcontroller > that could be anything. The general device is named "Ubiquiti airCube > ISP" or (a short form that I found at some locations) ubnt-acb-isp. I > assume that they used the same controller in the non-ISP-version but I > haven't checked that. So how about one of these: > > - ubnt,spi-byte-led > - ubnt,spi-acb-led > - ubnt,acb-isp-led ubnt,acb-spi-led perhaps as the order is usually <product|soc>-<sub device>. Or the last one if you wanted to keep 'isp'. > > Most likely I'll get the off-value and max-value based on the binding > name then (0 and 63 for that device). So I'll just remove the two > parameters then. > > > > > This comment *only* applies to the binding, not the driver. Specific > > bindings can easily be bound to generic drivers. > > > > So the driver should still be called spi-byte (or something similar)? Yes, whatever the LED maintainers want there. Rob
