Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

[Herve Codina]

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;
	}

Is that ok for you ?

Thanks,
Hervé
-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com