Re: [PATCH v5 2/5] MFD: RK808: Add new mfd driver for RK808

[Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]

On Mon, 25 Aug 2014, Doug Anderson wrote:
> On Mon, Aug 25, 2014 at 6:31 AM, Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> > The RK808 chip is a power management IC for multimedia and handheld
> > devices. It contains the following components:
> >
> > - Regulators
> > - RTC
> >
> > The RK808 core driver is registered as a platform driver and provides
> > communication through I2C with the host device for the different
> > components.
> >
> > Signed-off-by: Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> You need a Signed-off-by: Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> > ---

[...]

> > +static struct rk808 *g_rk808;
> 
> I think Lee's "Grim" comment here was that prefixing globals with "g_"
> is not consistent with the Linux coding style.  Just remove the "g_".

That and the seemingly unavoidable use of a global pointer.

[...]

> > +static const struct i2c_device_id rk808_ids[] = {
> > +        { "rk808", 0 },
> 
> I think Lee wanted the above to be:
> 
>        { "rk808", },

Right, but the ',' is now superfluous.

[...]

> I didn't do a thorough review, just compared to Lee's old feedback.
> Maybe a good idea to get in the habit to responding to others comments
> with "Done" so others know you have addressed each comment?

Please only do this locally or in your head.  Reading replies to
reviews containing only a break-down of what has been fixed is a waste
of everyone's time.  If/when replying to comments/observations that
you do _not_ agree with, please snip out all of the ones that you _do_
agree with.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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