On Thu, Nov 15, 2018 at 11:54:48AM +0100, Linus Walleij wrote:
> On Thu, Nov 8, 2018 at 11:14 AM Charles Keepax
> <[email protected]> wrote:
> 
> > Lochnagar is an evaluation and development board for Cirrus
> > Logic Smart CODEC and Amp devices. It allows the connection of
> > most Cirrus Logic devices on mini-cards, as well as allowing
> > connection of various application processor systems to provide a
> > full evaluation platform. This driver supports the board
> > controller chip on the Lochnagar board.
> >
> > Lochnagar provides many pins which can generally be used for an
> > audio function such as an AIF or a PDM interface, but also as
> > GPIOs.
> >
> > Signed-off-by: Charles Keepax <[email protected]>
> 
> This v4 version looks very good to me!
> Reviewed-by: Linus Walleij <[email protected]>
> 
> Nitpicks (fix or not, my review tag stands):
> 
> >  drivers/pinctrl/cirrus/pinctrl-lochnagar.c | 1145 
> > ++++++++++++++++++++++++++++
> >  drivers/pinctrl/cirrus/pinctrl-lochnagar.h |  112 +++
> 
> Why don't you just include that header file in the C file.
> 112 lines +/- on a 1145 line file doesn't matter.
> 
> > +       priv->gpio_chip = template_chip;
> 
> I would just skip the template and just assign struct members
> directly.
> 
> Bonus if you set ->label to something instance-unique as well.
> 

No objection to any of those need to respin for Mark's comments
on the regulators anyway, so will wrap these in and add your
reviewed by.

Thanks,
Charles

Reply via email to