[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <e2e5d376-a307-4ddc-bebb-5c00141dcc01@app.fastmail.com>
Date: Wed, 26 Jul 2023 13:30:49 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
"Zhu Wang" <wangzhu9@...wei.com>
Cc: "Andrzej Hajda" <andrzej.hajda@...el.com>,
"Neil Armstrong" <neil.armstrong@...aro.org>, rfoss@...nel.org,
"laurent.pinchart" <Laurent.pinchart@...asonboard.com>,
jonas@...boo.se, "Jernej Skrabec" <jernej.skrabec@...il.com>,
"Dave Airlie" <airlied@...il.com>,
"Daniel Vetter" <daniel@...ll.ch>,
"Javier Martinez Canillas" <javierm@...hat.com>,
"Doug Anderson" <dianders@...omium.org>,
benjamin.mugnier@...s.st.com,
Marek Behún <kabel@...nel.org>,
anarsoul@...il.com, duwe@...e.de,
"Maxime Ripard" <mripard@...nel.org>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
"Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH -next] drm/bridge: fix -Wunused-const-variable= warning
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
Powered by blists - more mailing lists