From: David Rivshin <redacted>

This series adds support for the ISSI IS31FL32xx family of I2C LED
drivers. I'm posting this first as an RFC as there are a couple of
items I'd especially like feedback on:

In the binding, I choose to have 'reg' be 1-based in order to follow
the hardware documentation. It seems 0-based is the normal choice.

In the binding, I used the 'reg' property for the LED channel, as
it seemed ePAPR required that name. I also considered naming the
property 'chan', and not pretending that it represented a bus
address at all.

In the binding, max-brightness is an optional property. In other
bindings it seems to be either unsupported, or required.

If there is a problem with the devicetree for an LED subnode, I
ignore that one node and continue on with the rest. I could see
an argument for just bailing on the whole probe if any subnode is
bad.

I left the LED enable bit always enabled, and just let the PWM be
set to 0 for "off" naturally.

I did not see a need to use regmap, as there was never a need for
read/modify/write. In the case of the 3218/3216 This was facilitated
by the above choice.

I did not see for a mutex to protect any of the data, as it was
read-only after probing, and I2C core already has a mutex to protect
the device.

I implemented support for all 4 devices in a single driver mainly
because I really need is31fl3236 support, but I had access to an
is31fl3216 eval board early, and could get a lot of milage out of it.
The other two parts were similar enough to one of those that it was
trivial include them as well (although untested).
    - The 3236 and 3235 are nearly identical (differing just in
      number of channels and some register addresses).
    - The 3218 is like the 3236/3235, except it packs the enable
      bits into 3 registers, doesn't support per-channel
      max-current divisor, and lacks a global control register.
    - The 3216 has the most differences. It has some additional
      register differences, and goes on to supports a number
      of (relatively) complex extra features:
        - using some channels as GPIOs
        - HW animation of LEDs
        - HW modulation of LEDs according to an audio input
I could certainly see an argument for having the 3236/3235 driver be
separate from the other devices. I'm not sure where the desired
balance between duplicate code (mostly in probing) vs simpler drivers.
For reference, I think about 70 lines (15%) are unique to the 3216,
and another 20 (5%) unique to the 3218.

I should also mention that I just noticed the is31fl3218 and
is31fl3216 appear to be the same devices as the SI-EN SN3218 and
SN3216. ISSI acquired SI-EN in 2011, and seems to have just rebranded
those devices. Either this driver or the recently-added SN3218 driver
should work for both the ISSI and SN "3218" parts.
Obviously we don't need both implementations, though I have no
preference for which one to use. For instance, I'd have no argument
with just adding a compatible entry for is31fl3218 to the sn3218
driver (since that's trivial), remove the 3216/3218 support from
this driver, and call it a day. I suspect that anyone actually using
the 3216 in product would need some of the advanced functions that
I did not implement anyways.

David Rivshin (3):
  DT: Add vendor prefix for Integrated Silicon Solutions Inc.
  DT: leds: Add binding for the ISSI IS31FL32xx family of LED drivers
  leds: Add driver for the ISSI IS31FL32xx family of LED drivers

 .../devicetree/bindings/leds/leds-is31fl32xx.txt   |  51 +++
 .../devicetree/bindings/vendor-prefixes.txt        |   1 +
 drivers/leds/Kconfig                               |   9 +
 drivers/leds/Makefile                              |   1 +
 drivers/leds/leds-is31fl32xx.c                     | 442 +++++++++++++++++++++
 5 files changed, 504 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt
 create mode 100644 drivers/leds/leds-is31fl32xx.c

-- 
2.5.0