On Mon, 23 Jan 2023 12:30:32 +0000
Christophe Leroy <christophe.leroy@xxxxxxxxxx> wrote:
> Le 23/01/2023 à 13:17, Herve Codina a écrit :
> > Hi Christophe,
> >
> > On Mon, 23 Jan 2023 11:13:23 +0000
> > Christophe Leroy <christophe.leroy@xxxxxxxxxx> wrote:
> >
> >> Hi Hervé,
> >>
> >> Le 23/01/2023 à 09:56, Herve Codina a écrit :
> >>>
> >>> gpiochip_get_data() is defined only when CONFIG_GPIOLIB is set.
> >>> That's why the #if section is used.
> >>
> >> gpiochip_get_data() is still declared when CONFIG_GPIOLIB is not set, so
> >> it is not a problem, the call to it will be eliminated at buildtime.
> >>
> >> By the way, at the time being I get the following warnings:
> >>
> >> CC sound/soc/codecs/idt821034.o
> >> sound/soc/codecs/idt821034.c:310:12: warning: 'idt821034_read_slic_raw'
> >> defined but not used [-Wunused-function]
> >> 310 | static int idt821034_read_slic_raw(struct idt821034 *idt821034,
> >> u8 ch, u8 *slic_raw)
> >> | ^~~~~~~~~~~~~~~~~~~~~~~
> >> sound/soc/codecs/idt821034.c:305:11: warning:
> >> 'idt821034_get_written_slic_raw' defined but not used [-Wunused-function]
> >> 305 | static u8 idt821034_get_written_slic_raw(struct idt821034
> >> *idt821034, u8 ch)
> >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> sound/soc/codecs/idt821034.c:276:12: warning: 'idt821034_write_slic_raw'
> >> defined but not used [-Wunused-function]
> >> 276 | static int idt821034_write_slic_raw(struct idt821034
> >> *idt821034, u8 ch, u8 slic_raw)
> >> | ^~~~~~~~~~~~~~~~~~~~~~~~
> >> sound/soc/codecs/idt821034.c:271:11: warning: 'idt821034_get_slic_conf'
> >> defined but not used [-Wunused-function]
> >> 271 | static u8 idt821034_get_slic_conf(struct idt821034 *idt821034,
> >> u8 ch)
> >> | ^~~~~~~~~~~~~~~~~~~~~~~
> >> sound/soc/codecs/idt821034.c:250:12: warning: 'idt821034_set_slic_conf'
> >> defined but not used [-Wunused-function]
> >> 250 | static int idt821034_set_slic_conf(struct idt821034 *idt821034,
> >> u8 ch, u8 slic_dir)
> >> | ^~~~~~~~~~~~~~~~~~~~~~~
> >>
> >>
> >> With the following changes I have no warning and an objdump -x on
> >> idt821034.o shows no reference to gpiochip_get_data()
> >>
> >> diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
> >> index 5eb93fec6042..8b75388e22ce 100644
> >> --- a/sound/soc/codecs/idt821034.c
> >> +++ b/sound/soc/codecs/idt821034.c
> >> @@ -968,7 +968,6 @@ static const struct snd_soc_component_driver
> >> idt821034_component_driver = {
> >> .endianness = 1,
> >> };
> >>
> >> -#if IS_ENABLED(CONFIG_GPIOLIB)
> >> #define IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(_offset) (((_offset) /
> >> 5) % 4)
> >> #define IDT821034_GPIO_OFFSET_TO_SLIC_MASK(_offset) BIT((_offset) % 5)
> >>
> >> @@ -1133,12 +1132,6 @@ static int idt821034_gpio_init(struct idt821034
> >> *idt821034)
> >> return devm_gpiochip_add_data(&idt821034->spi->dev,
> >> &idt821034->gpio_chip,
> >> idt821034);
> >> }
> >> -#else /* IS_ENABLED(CONFIG_GPIOLIB) */
> >> -static int idt821034_gpio_init(struct idt821034 *idt821034)
> >> -{
> >> - return 0;
> >> -}
> >> -#endif
> >>
> >> static int idt821034_spi_probe(struct spi_device *spi)
> >> {
> >> @@ -1165,6 +1158,9 @@ static int idt821034_spi_probe(struct spi_device *spi)
> >> if (ret)
> >> return ret;
> >>
> >> + if (!IS_ENABLED(CONFIG_GPIOLIB))
> >> + return 0;
> >> +
> >> ret = idt821034_gpio_init(idt821034);
> >> if (ret)
> >> return ret;
> >>
> >>
> >> Christophe
> >
> > Right, I did the test too and indeed, I can remove the #if section.
> >
> > I will use (I think is clearer) at idt821034_spi_probe():
> > if (!IS_ENABLED(CONFIG_GPIOLIB)) {
> > ret = idt821034_gpio_init(idt821034);
> > if (ret)
> > return ret;
> > }
> >
>
>
> I guess you mean :
>
> if (IS_ENABLED(CONFIG_GPIOLIB))
Yes of course. Sorry for the typo.
>
>
> > Is that ok for you ?
>
>
>
> What about:
>
> if (IS_ENABLED(CONFIG_GPIOLIB))
> return idt821034_gpio_init(idt821034);
> else
> return 0;
>
> Christophe
Well, maybe this version ?
static int idt821034_spi_probe(struct spi_device *spi)
{
...
if (IS_ENABLED(CONFIG_GPIOLIB))
return idt821034_gpio_init(idt821034);
return 0;
}
Thanks,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
