Re: [PATCH v3] Add generic GPIO-tilt driver

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Tue, Nov 29, 2011 at 04:12:40PM +0100, Heiko Stübner wrote:
> Hi Shubhrajyoti,
> 
> Am Dienstag, 29. November 2011, 15:59:29 schrieb Shubhrajyoti Datta:
> > Hi Heiko,
> > 
> > On Tue, Nov 29, 2011 at 5:54 PM, Heiko Stübner <heiko@sntech.de> wrote:
> > > +static void gpio_tilt_polled_poll(struct input_polled_dev *dev)
> > > +{
> > > +       struct gpio_tilt_polled_dev *tdev = dev->private;
> > > +       struct gpio_tilt_platform_data *pdata = tdev->pdata;
> > > +       struct input_dev *input = dev->input;
> > > +       struct gpio_tilt_state *tilt_state = NULL;
> > > +       int state, i;
> > > +
> > > +       if (tdev->count < tdev->threshold) {
> > 
> > Nitpick : the braces are not necessary.
> hmm ... for me it's easier to read if the style of the braces does not change
> between if and else, i.e. both with or without braces.
> And the kernel CodingStyle-file seems also to be in my favor ;-)
> [starting from line 169].

Yep, when one branch requires braces it is preferred to have then on
all branches.

> > > +static int __devinit gpio_tilt_polled_probe(struct platform_device
> > > *pdev) +{
> > > +       struct gpio_tilt_platform_data *pdata = pdev->dev.platform_data;
> > > +       struct device *dev = &pdev->dev;
> > > +       struct gpio_tilt_polled_dev *tdev;
> > > +       struct input_polled_dev *poll_dev;
> > > +       struct input_dev *input;
> > > +       int error, i;
> > > +
> > > +       if (!pdata || !pdata->poll_interval)
> > > +               return -EINVAL;
> > 
> > Could we have a default poll interval if passed 0 ?
> > Feel free to ignore if you feel otherwise.
> For me either way is fine but I'm not sure what would be a generic
> enough default value for most cases (500, 1000 ? ).
> 

I'd rather error out then give default which we can't sanely select.
Then platform integrator will be alerted early and select appropriate
value.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html