On Mon, Sep 16, 2013 at 03:21:47PM +0100, Mark Rutland wrote:
> On Thu, Sep 12, 2013 at 05:53:04PM +0100, Guenter Roeck wrote:
> > On Thu, Sep 12, 2013 at 05:41:00PM +0100, Mark Rutland wrote:
> > > On Fri, Aug 30, 2013 at 05:29:37AM +0100, Guenter Roeck wrote:
> > > > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> > > > ---
> > > > .../devicetree/bindings/extcon/extcon-gpio | 23 ++++++++++++++++++++
> > > > 1 file changed, 23 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/extcon/extcon-gpio
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/extcon/extcon-gpio b/Documentation/devicetree/bindings/extcon/extcon-gpio
> > > > new file mode 100644
> > > > index 0000000..091ddc6
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/extcon/extcon-gpio
> > > > @@ -0,0 +1,23 @@
> > > > +Device-Tree bindings for extcon/extcon-gpio driver
> > >
> > > Bindings shouldn't refer to Linux-specifics like particular drivers.
> > > What class of hardware are you actually trying to describe?
> > >
> > Agreed. Question is where to put the bindings, as they are not really
> > specific to the extcon driver. The extcon driver merely implements
> > the bindings. This is why the "compatible" statement reads "gpio-connector"
> > and not "extcon-something".
> >
> > The bindings describe a connector managed through gpio pins.
>
> Ok, then that's what the binding document should state.
>
> >
> > > > +
> > > > +Required properties:
> > > > + - compatible = "gpio-connector";
> > > > + - presence-detect-gpios - presence detect gpio pin
> > > > +
> > > > +Optional properties:
> > > > + - debounce-interval - debounce interval in milli-seconds
> > > > + - state-on - on (connected) state
> > > > + - state-off - off (disconnected) state
> > > > + Depending on the type of connector or cable, states may
> > > > + for example be reported as "connected"/"disconnected"
> > > > + or "inserted"/"removed".
> > >
> > > I don't understand what the state-* properties describe. Do these
> > > provide semantic information to drivers? What is the full set of valid
> > > values?
> > >
> > That is merely text which is ultimately passed on to the user.
> > Guess 'semantic information' might be a way to phrase it.
>
> Where do these end up appearing to the user? Through names in the
Yes.
> filesystem? That's an ABI, which should be thoroughly described...
>
Tricky, as I can not really describe how the extcon driver implements it.
I can add something like above explanation, ie that the implementation is
expected to pass the property to the user. Would that be acceptable ?
> If it's arbitrary, why is it necessary at all? Surely sensible names for
> the state of the connector can be coded in the driver for the device
> attached to said connectors (which can be consistent and later changed
> if necessary)...
>
Good point. I took a "hint" from the implementation in the extcon driver,
which lets one do that. But doesn't your comment apply to pretty much all
"label" and similar properties in the various devicetree descriptions ?
> >
> > > > +
> > > > +Example node:
> > > > +
> > > > + some-connector {
> > > > + compatible = "gpio-connector";
> > > > + presence-detect-gpios = <&gpio1 7 1>;
> > > > + debounce-interval = <1>;
> > > > + state-on = "connected";
> > > > + state-on = "disconnected";
>
> I don't think that's a valid dts. I assume the second is meant to be
> "state-off"?
>
Yes, obviously.
> > > > + };
> > >
> > > I'm not sure how much value this adds to bindings over describing the
> > > gpios directly. This seems to add a layer of indirection because of
> > > Linux internals.
> > >
> > Not sure I understand what you mean with "describing the gpios directly".
> > Can you elaborate and/or provide an example ?
>
> Take a look at the MMC bindings [1], specifically the cd-gpios and
> wp-gpios properties. I don't see why the connection of the GPIO needs to
> be described by a wrapper device that doesn't really exist, when it can
> be described directly.
>
Question is - described to what or for what ?
I think what you are saying is that describing a generic connector via
devicetree is not acceptable, even though it _does_ describe hardware.
I would have to describe a specific connector for a specific hardware
instead, which in turn would need its own driver. Is that correct ?
Thanks,
Guenter
--
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
