drm/bridge: fix -Wunused-const-variable= warning

[PATCH -next] drm/bridge: fix -Wunused-const-variable= warning

Posted by Zhu Wang 2 years, 6 months ago

When building with W=1, the following warning occurs.

drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c:48:17: warning: ‘anx781x_i2c_addresses’ defined but not used [-Wunused-const-variable=]
 static const u8 anx781x_i2c_addresses[] = {
                 ^~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c:40:17: warning: ‘anx7808_i2c_addresses’ defined but not used [-Wunused-const-variable=]
 static const u8 anx7808_i2c_addresses[] = {

The definition of above two data variables is included by the
macro CONFIG_OF, so we also include the data variable
definitions in the macro CONFIG_OF. And in addition the data
variable anx78xx_match_table is included in the macro CONFIG_OF,
so we add CONFIG_OF to the place where it is used.

Fixes: 5d97408e0d70 ("drm/bridge: move ANA78xx driver to analogix subdirectory")

Signed-off-by: Zhu Wang <wangzhu9@huawei.com>
---
 drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
index 06a3e3243e19..799b33cebdd5 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
@@ -37,6 +37,7 @@
 
 #define XTAL_CLK		270 /* 27M */
 
+#if IS_ENABLED(CONFIG_OF)
 static const u8 anx7808_i2c_addresses[] = {
 	[I2C_IDX_TX_P0] = 0x78,
 	[I2C_IDX_TX_P1] = 0x7a,
@@ -52,6 +53,7 @@ static const u8 anx781x_i2c_addresses[] = {
 	[I2C_IDX_RX_P0] = 0x7e,
 	[I2C_IDX_RX_P1] = 0x80,
 };
+#endif
 
 struct anx78xx_platform_data {
 	struct regulator *dvdd10;
@@ -1387,7 +1389,9 @@ MODULE_DEVICE_TABLE(of, anx78xx_match_table);
 static struct i2c_driver anx78xx_driver = {
 	.driver = {
 		   .name = "anx7814",
+#if IS_ENABLED(CONFIG_OF)
 		   .of_match_table = of_match_ptr(anx78xx_match_table),
+#endif
 		  },
 	.probe = anx78xx_i2c_probe,
 	.remove = anx78xx_i2c_remove,
-- 
2.17.1

Re: [PATCH -next] drm/bridge: fix -Wunused-const-variable= warning

Posted by Uwe Kleine-König 2 years, 6 months ago

[Cc += Arnd, Andy who did similar changes in the past]

Hello,

On Wed, Jul 26, 2023 at 06:06:26PM +0800, Zhu Wang wrote:
> When building with W=1, the following warning occurs.
> 
> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c:48:17: warning: ‘anx781x_i2c_addresses’ defined but not used [-Wunused-const-variable=]
>  static const u8 anx781x_i2c_addresses[] = {
>                  ^~~~~~~~~~~~~~~~~~~~~
> drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c:40:17: warning: ‘anx7808_i2c_addresses’ defined but not used [-Wunused-const-variable=]
>  static const u8 anx7808_i2c_addresses[] = {
> 
> The definition of above two data variables is included by the
> macro CONFIG_OF, so we also include the data variable
> definitions in the macro CONFIG_OF. And in addition the data
> variable anx78xx_match_table is included in the macro CONFIG_OF,
> so we add CONFIG_OF to the place where it is used.
> 
> Fixes: 5d97408e0d70 ("drm/bridge: move ANA78xx driver to analogix subdirectory")
> 
> Signed-off-by: Zhu Wang <wangzhu9@huawei.com>
> ---
>  drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> index 06a3e3243e19..799b33cebdd5 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c
> @@ -37,6 +37,7 @@
>  
>  #define XTAL_CLK		270 /* 27M */
>  
> +#if IS_ENABLED(CONFIG_OF)
>  static const u8 anx7808_i2c_addresses[] = {
>  	[I2C_IDX_TX_P0] = 0x78,
>  	[I2C_IDX_TX_P1] = 0x7a,
> @@ -52,6 +53,7 @@ static const u8 anx781x_i2c_addresses[] = {
>  	[I2C_IDX_RX_P0] = 0x7e,
>  	[I2C_IDX_RX_P1] = 0x80,
>  };
> +#endif

You can mark anx7808_i2c_addresses with __maybe_unused, then the #if
isn't needed.

>  struct anx78xx_platform_data {
>  	struct regulator *dvdd10;
> @@ -1387,7 +1389,9 @@ MODULE_DEVICE_TABLE(of, anx78xx_match_table);
>  static struct i2c_driver anx78xx_driver = {
>  	.driver = {
>  		   .name = "anx7814",
> +#if IS_ENABLED(CONFIG_OF)
>  		   .of_match_table = of_match_ptr(anx78xx_match_table),
> +#endif

If CONFIG_OF is disabled of_match_ptr(something) evaluates to NULL, so
you can drop the #if here.

Having said that the better fix is probably to just do

	.of_match_table = anx78xx_match_table,

as systems using ACPI can benefit from the of_match_table, too.
See b89a9e9890d4 ("gpio: aggregator: Remove CONFIG_OF and of_match_ptr()
protections") for an example.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Re: [PATCH -next] drm/bridge: fix -Wunused-const-variable= warning

Posted by Arnd Bergmann 2 years, 6 months ago

On Wed, Jul 26, 2023, at 12:54, Uwe Kleine-König wrote:
> On Wed, Jul 26, 2023 at 06:06:26PM +0800, Zhu Wang wrote:
>>
>> Fixes: 5d97408e0d70 ("drm/bridge: move ANA78xx driver to analogix subdirectory")

This is the wrong commit, the driver was just in a different place
before that, and the bug was already present in commit 0647e7dd3f7ab
("drm/bridge: Add Analogix anx78xx support").

>>  
>> +#if IS_ENABLED(CONFIG_OF)
>>  static const u8 anx7808_i2c_addresses[] = {
>>  	[I2C_IDX_TX_P0] = 0x78,
>>  	[I2C_IDX_TX_P1] = 0x7a,
>> @@ -52,6 +53,7 @@ static const u8 anx781x_i2c_addresses[] = {
>>  	[I2C_IDX_RX_P0] = 0x7e,
>>  	[I2C_IDX_RX_P1] = 0x80,
>>  };
>> +#endif
>
> You can mark anx7808_i2c_addresses with __maybe_unused, then the #if
> isn't needed.

Neither of these should be needed, as the driver only works with
CONFIG_OF anyway.

>>  struct anx78xx_platform_data {
>>  	struct regulator *dvdd10;
>> @@ -1387,7 +1389,9 @@ MODULE_DEVICE_TABLE(of, anx78xx_match_table);
>>  static struct i2c_driver anx78xx_driver = {
>>  	.driver = {
>>  		   .name = "anx7814",
>> +#if IS_ENABLED(CONFIG_OF)
>>  		   .of_match_table = of_match_ptr(anx78xx_match_table),
>> +#endif
>
> If CONFIG_OF is disabled of_match_ptr(something) evaluates to NULL, so
> you can drop the #if here.
>
> Having said that the better fix is probably to just do
>
> 	.of_match_table = anx78xx_match_table,
>
> as systems using ACPI can benefit from the of_match_table, too.
> See b89a9e9890d4 ("gpio: aggregator: Remove CONFIG_OF and of_match_ptr()
> protections") for an example.

Agreed, removing the #ifdef checks and the of_match_ptr()
wrapper is the right solution here. I see similar things
in other bridge drivers that could be changed at the same
time:

$ git grep of_match_ptr drivers/gpu/drm/bridge/
drivers/gpu/drm/bridge/analogix/analogix-anx6345.c:                .of_match_table = of_match_ptr(anx6345_match_table),
drivers/gpu/drm/bridge/analogix/analogix-anx78xx.c:                .of_match_table = of_match_ptr(anx78xx_match_table),
drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c:            .of_match_table = of_match_ptr(mhdp_ids),
drivers/gpu/drm/bridge/chrontel-ch7033.c:               .of_match_table = of_match_ptr(ch7033_dt_ids),
drivers/gpu/drm/bridge/sil-sii8620.c:           .of_match_table = of_match_ptr(sii8620_dt_match),
drivers/gpu/drm/bridge/ti-tfp410.c:             .of_match_table = of_match_ptr(tfp410_match),

The other ones are even worse because they use of_match_ptr()
without the corresponding #ifdef around the match table, so
the of_match_ptr() does not even have the effect of saving a few
bytes of .data segment.

     Arnd