On Wed, Dec 8, 2021 at 2:39 AM Srinivasa Rao Mandadapu <quic_srivasam@xxxxxxxxxxx> wrote: > > Extract the chip specific SM8250 data from the LPASS LPI pinctrl driver > to allow reusing the common code in the addition of subsequent > platforms. ... > @@ -661,8 +454,10 @@ static int lpi_pinctrl_probe(struct platform_device *pdev) > > return ret; > } > +EXPORT_SYMBOL(lpi_pinctrl_probe); > + Stray change. ... > +#ifndef __PINCTRL_LPASS_LPI_H__ > +#define __PINCTRL_LPASS_LPI_H__ Missed headers. At least bits.h. ... > +#define NO_SLEW -1 Naming sucks for the header. LPI_NO_SLEW ? ... > +struct lpi_pingroup { > + const char *name; > + const unsigned int *pins; > + unsigned int npins; > + unsigned int pin; > + /* Bit offset in slew register for SoundWire pins only */ > + int slew_offset; > + unsigned int *funcs; > + unsigned int nfuncs; > +}; Are you going to convert this to use struct group_desc? ... > + LPI_MUX__, Strange naming. Besides, if it is the terminator, drop the comma. -- With Best Regards, Andy Shevchenko