Re: [PATCH v2 -next] ASoc: tas2770: Fix build error without GPIOLIB

[Ladislav Michl]

Hi Michał,

On Sun, Oct 06, 2019 at 05:31:58PM +0200, mirq-linux@xxxxxxxxxxxx wrote:
> On Sun, Oct 06, 2019 at 06:46:31PM +0800, YueHaibing wrote:
> > If GPIOLIB is not set, building fails:
> > 
> > sound/soc/codecs/tas2770.c: In function tas2770_reset:
> > sound/soc/codecs/tas2770.c:38:3: error: implicit declaration of function gpiod_set_value_cansleep; did you mean gpio_set_value_cansleep? [-Werror=implicit-function-declaration]
> >    gpiod_set_value_cansleep(tas2770->reset_gpio, 0);
> >    ^~~~~~~~~~~~~~~~~~~~~~~~
> >    gpio_set_value_cansleep
> > sound/soc/codecs/tas2770.c: In function tas2770_i2c_probe:
> > sound/soc/codecs/tas2770.c:749:24: error: implicit declaration of function devm_gpiod_get_optional; did you mean devm_regulator_get_optional? [-Werror=implicit-function-declaration]
> >   tas2770->reset_gpio = devm_gpiod_get_optional(tas2770->dev,
> >                         ^~~~~~~~~~~~~~~~~~~~~~~
> >                         devm_regulator_get_optional
> > sound/soc/codecs/tas2770.c:751:13: error: GPIOD_OUT_HIGH undeclared (first use in this function); did you mean GPIOF_INIT_HIGH?
> >              GPIOD_OUT_HIGH);
> >              ^~~~~~~~~~~~~~
> >              GPIOF_INIT_HIGH
> > 
> > Reported-by: Hulk Robot <hulkci@xxxxxxxxxx>
> > Fixes: 1a476abc723e ("tas2770: add tas2770 smart PA kernel driver")
> > Suggested-by: Ladislav Michl <ladis@xxxxxxxxxxxxxx>
> > Signed-off-by: YueHaibing <yuehaibing@xxxxxxxxxx>
> > ---
> > v2: Add missing include file
> > ---
> >  sound/soc/codecs/tas2770.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/sound/soc/codecs/tas2770.c b/sound/soc/codecs/tas2770.c
> > index 9da88cc..a36d0d7 100644
> > --- a/sound/soc/codecs/tas2770.c
> > +++ b/sound/soc/codecs/tas2770.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/pm.h>
> >  #include <linux/i2c.h>
> >  #include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/pm_runtime.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/firmware.h>
> 
> The Kconfig part is missing - is this intended? If I guess correctly,
> the driver won't work without GPIOLIB, so it should either
> 'select GPIOLIB' or 'depends on GPIOLIB || COMPILE_TEST' or even
> 'select GPIOLIB if !COMPILE_TEST'.

while the first one is actually preferable I won't do this in this patch,
but rather generally. The same you can say about regulator, regmap and
other interfaces, so perhaps leaving that to patchset focusing on this
kind of problem seem to be better.

Btw, I guess linux/gpio/consumer.h is enough for this driver and including
linux/gpio.h should be dropped.

	ladis