On Wed, Feb 24, 2016 at 12:57 PM, David Rivshin (Allworx) <drivshin.allworx@xxxxxxxxx> wrote: > On Tue, 23 Feb 2016 19:15:08 -0600 > Rob Herring <robh@xxxxxxxxxx> wrote: > >> On Tue, Feb 23, 2016 at 01:17:24PM -0500, David Rivshin (Allworx) wrote: >> > From: David Rivshin <drivshin@xxxxxxxxxxx> >> > >> > This adds a binding description for the is31fl3236/35/18/16 I2C LED >> > drivers. >> > >> > Signed-off-by: David Rivshin <drivshin@xxxxxxxxxxx> >> > --- >> > .../devicetree/bindings/leds/leds-is31fl32xx.txt | 51 ++++++++++++++++++++++ >> > 1 file changed, 51 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/leds/leds-is31fl32xx.txt >> >> Acked-by: Rob Herring <robh@xxxxxxxxxx> > > Thanks, Rob. I just want to double check whether you noticed some > binding-related questions I had in the coverletter [1]. In hindsight > I should probably have included them in the patch comments as well so > they'd show up in patchwork. For convenience, I'll repeat them here: I had not. > I choose to have 'reg' be 1-based in order to follow the hardware > documentation. It seems 0-based is the normal choice, although HW > docs also usually use the 0-based numbering. Perhaps being consistent > with other bindings is more important than being consistent with the > datasheet's numbering? Usually reg is some type of address, so you should follow whatever is more natural for accessing the h/w. Ideally, it is not just an index for s/w convenience. > 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 (and > then removing the @n from the subnode names). That would solve the > 'reg' question above as a side-effect, but would be inconstant with > other LED bindings (tca6507, pca963x, tlc59108, etc). I would expect the reg value is either the register (base) address for each channel or a bit offset in a register if they are all in the same register. Or it could be how the pins are labeled on the package if neither of those work. > Note that the recently-added (and only in for-next) SN3218 driver > uses a 0-based 'reg' property, and the SN3218 has the same HW doc > numbering as the IS31FL32xx family (indeed the IS31FL3218 appears > to be a rebranded SN3218). So perhaps that sets the precedent if > there was not one before? If they are the same, then you should use the SN3218 binding. If it has problems, then it is not too late to fix it. You may want to add another compatible string if they are not truly the exact same die. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
