Re: [PATCH v2] Input: rotary_encoder - support binary encoding of states

["Uwe Kleine-König" <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]

Hello Dmitry,

On Thu, Apr 07, 2016 at 11:15:13AM -0700, Dmitry Torokhov wrote:
> On Thu, Mar 24, 2016 at 08:57:12AM +0100, Uwe Kleine-König wrote:
> > diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > index 6c9f0c8a846c..e85ce3dea480 100644
> > --- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > +++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > @@ -20,6 +20,8 @@ Optional properties:
> >    2: Half-period mode
> >    4: Quarter-period mode
> >  - wakeup-source: Boolean, rotary encoder can wake up the system.
> > +- rotary-encoder,encoding: String, the method used to encode steps.
> > +  Supported are "gray" (the default and more common) and "binary".
> >  
> >  Deprecated properties:
> >  - rotary-encoder,half-period: Makes the driver work on half-period mode.
> > [...]
> > +	err = device_property_read_string(dev, "rotary-encoder,encoding",
> > +					  &encoding);
> > +	if (!err && encoding[0] == 'b')
> 
> Why do we only match on first letter? I'd prefer we did better parsing
> (i.e. only accepted valid encodings or no encoding property.

IMHO it's not a problem that

	rotary-encoder,encoding = "blablubfasel";

isn't rejected by interpretet as "binary" instead. That's a bit like
compilers that are free to do whatever they want if the source code
contains something non-standard or undefined.

But I admit this might become a problem when the set of allowed
encodings is expanded in the future. So I will resend a more stricter
version.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html