On 02/27/2013 11:25 PM, Tony Prisk wrote: > Thanks for the review Stephen, > > I have posted replies to most of your points/questions inline. > > The review was slightly more in-depth than I expected and as you > noticed, the patch is poor in quality. I was more 'testing the water' > that people were happy with the basic design of the code, rather than > the specifics as I have 4 SoC's worth of data to add, and didn't want to > have to redo it all if I had to make a data format change. > > It seems that on the whole the design is ok, so I will post a proper > patch series for this to be reviewed (after addressing the issues you > pointed out).
Oh, OK. It might have been a good idea to mention this in the patch description, or have posted it as [PATCH RFC]. >>> diff --git a/drivers/pinctrl/pinctrl-wm8850.c >>> b/drivers/pinctrl/pinctrl-wm8850.c >> >>> +/* Please keep sorted by bank/bit */ >>> +#define WMT_PIN_EXTGPIO0 WMT_PIN(0, 0) >>> +#define WMT_PIN_EXTGPIO1 WMT_PIN(0, 1) >> ... >>> +#define WMT_PIN_I2C2_SCL WMT_PIN(5, 12) >>> +#define WMT_PIN_I2C2_SDA WMT_PIN(5, 13) >> >> There are a lot of gaps in that list. Does the HW really not support pin >> muxing on the rest of the bits in the registers? > > Nobody who knows is willing to say :). Most of the code for Wondermedia > SoCs is based of the vendor source that has come out, and we therefore > only know what the vendor has included support for. > > No doubt there will be other pins which have valid functions, but we > don't know what they are and don't have support for them at the moment. > The reason I went with the bank/pin encoding is so that if/when we add > other pins, it won't affect any pin numbering. Doing the pin numbering that way was a good choice. So, there's an idea that when first defining a DT binding for HW, that binding should be complete and accurate, rather than being minimal, with a plan to incrementally expand it later. I would take that to mean that for a pinctrl binding, the list of pins/groups/function/config-options that the binding supports is complete. Obviously this isn't possible if there's no complete documentation for the HW:-( I hope this case will be allowed though, since any modifications to support new pins/groups/functions should be possible to add in a backwards-compatible way, especially with a forward-looking number scheme for the pins. We should consider this kind of issue if/when we actually implement stricter rules for modifications to DT bindings, so as not to disallow solving this problem. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
