| Message ID | 20230311173303.262618-1-krzysztof.kozlowski@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6ea1e67788f30710d1991de42802cbdeb67afec4 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [01/12] net: dsa: lantiq_gswip: mark OF related data as maybe unused | expand |
On Sat, Mar 11, 2023 at 06:32:52PM +0100, Krzysztof Kozlowski wrote: > The driver can be compile tested with !CONFIG_OF making certain data > unused: > > drivers/net/dsa/lantiq_gswip.c:1888:34: error: ‘xway_gphy_match’ defined but not used [-Werror=unused-const-variable=] > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > --- Do you happen to have any context as to why of_match_node() without CONFIG_OF is implemented as: #define of_match_node(_matches, _node) NULL and not as: static inline const struct of_device_id * of_match_node(const struct of_device_id *matches, const struct device_node *node) { return NULL; } ? Generally, the static inline shim function model is nicer, because it allows us to not scatter __maybe_unused all around.
On 11/03/2023 19:14, Vladimir Oltean wrote: > On Sat, Mar 11, 2023 at 06:32:52PM +0100, Krzysztof Kozlowski wrote: >> The driver can be compile tested with !CONFIG_OF making certain data >> unused: >> >> drivers/net/dsa/lantiq_gswip.c:1888:34: error: ‘xway_gphy_match’ defined but not used [-Werror=unused-const-variable=] >> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- > > Do you happen to have any context as to why of_match_node() without > CONFIG_OF is implemented as: > > #define of_match_node(_matches, _node) NULL > > and not as: > > static inline const struct of_device_id * > of_match_node(const struct of_device_id *matches, > const struct device_node *node) > { > return NULL; > } > > ? > > Generally, the static inline shim function model is nicer, because it > allows us to not scatter __maybe_unused all around. Sorry, I don't follow. I don't touch that wrappers, just fix errors related to OF device ID tables, although in few cases it is indeed related to of_match_node. Best regards, Krzysztof
On Sun, Mar 12, 2023 at 11:20:38AM +0100, Krzysztof Kozlowski wrote: > On 11/03/2023 19:14, Vladimir Oltean wrote: > > On Sat, Mar 11, 2023 at 06:32:52PM +0100, Krzysztof Kozlowski wrote: > >> The driver can be compile tested with !CONFIG_OF making certain data > >> unused: > >> > >> drivers/net/dsa/lantiq_gswip.c:1888:34: error: ‘xway_gphy_match’ defined but not used [-Werror=unused-const-variable=] > >> > >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> --- > > > > Do you happen to have any context as to why of_match_node() without > > CONFIG_OF is implemented as: > > > > #define of_match_node(_matches, _node) NULL > > > > and not as: > > > > static inline const struct of_device_id * > > of_match_node(const struct of_device_id *matches, > > const struct device_node *node) > > { > > return NULL; > > } > > > > ? > > > > Generally, the static inline shim function model is nicer, because it > > allows us to not scatter __maybe_unused all around. > > Sorry, I don't follow. I don't touch that wrappers, just fix errors > related to OF device ID tables, although in few cases it is indeed > related to of_match_node. I'm saying this because in lantiq_gswip.c, xway_gphy_match is accessed through of_match_node(). If the shim definition for of_match_node() was different, the variable wouldn't have been unused with CONFIG_OF=n. I guess it's worth considering changing that wrapper instead of adding the __maybe_unused.
On Sun, 12 Mar 2023 12:57:29 +0200 Vladimir Oltean wrote: > > Sorry, I don't follow. I don't touch that wrappers, just fix errors > > related to OF device ID tables, although in few cases it is indeed > > related to of_match_node. > > I'm saying this because in lantiq_gswip.c, xway_gphy_match is accessed > through of_match_node(). If the shim definition for of_match_node() was > different, the variable wouldn't have been unused with CONFIG_OF=n. > I guess it's worth considering changing that wrapper instead of adding > the __maybe_unused. Hi Krzysztof, have you had a chance to check if using an empty static inline is enough to silence the compiler? Seems like it could save us quite some churn? Or do we want the of_match_node() decorations to go away in general?
Hello: This series was applied to netdev/net-next.git (main) by David S. Miller <davem@davemloft.net>: On Sat, 11 Mar 2023 18:32:52 +0100 you wrote: > The driver can be compile tested with !CONFIG_OF making certain data > unused: > > drivers/net/dsa/lantiq_gswip.c:1888:34: error: ‘xway_gphy_match’ defined but not used [-Werror=unused-const-variable=] > > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > [...] Here is the summary with links: - [01/12] net: dsa: lantiq_gswip: mark OF related data as maybe unused https://git.kernel.org/netdev/net-next/c/6ea1e67788f3 - [02/12] net: dsa: lan9303: drop of_match_ptr for ID table https://git.kernel.org/netdev/net-next/c/ced5c5a0a2ea - [03/12] net: dsa: seville_vsc9953: drop of_match_ptr for ID table https://git.kernel.org/netdev/net-next/c/1eb8566dd08d - [04/12] net: dsa: ksz9477: drop of_match_ptr for ID table https://git.kernel.org/netdev/net-next/c/00923ff2e1ba - [05/12] net: dsa: ocelot: drop of_match_ptr for ID table https://git.kernel.org/netdev/net-next/c/0f17b42827ae - [06/12] net: phy: ks8995: drop of_match_ptr for ID table https://git.kernel.org/netdev/net-next/c/b0b7d1b6260b - [07/12] net: ieee802154: adf7242: drop of_match_ptr for ID table https://git.kernel.org/netdev/net-next/c/3df09beef650 - [08/12] net: ieee802154: mcr20a: drop of_match_ptr for ID table https://git.kernel.org/netdev/net-next/c/3896c40b7824 - [09/12] net: ieee802154: at86rf230: drop of_match_ptr for ID table https://git.kernel.org/netdev/net-next/c/32b7030681a4 - [10/12] net: ieee802154: ca8210: drop of_match_ptr for ID table https://git.kernel.org/netdev/net-next/c/cdfe4fc4d946 - [11/12] net: ieee802154: adf7242: drop owner from driver https://git.kernel.org/netdev/net-next/c/059fa9972340 - [12/12] net: ieee802154: ca8210: drop owner from driver https://git.kernel.org/netdev/net-next/c/613a3c44a373 You are awesome, thank you!
On Wed, 15 Mar 2023 08:20:20 +0000 patchwork-bot+netdevbpf@kernel.org wrote: > This series was applied to netdev/net-next.git (main) > by David S. Miller <davem@davemloft.net>: :) :) :)
On 15/03/2023 06:22, Jakub Kicinski wrote: > On Sun, 12 Mar 2023 12:57:29 +0200 Vladimir Oltean wrote: >>> Sorry, I don't follow. I don't touch that wrappers, just fix errors >>> related to OF device ID tables, although in few cases it is indeed >>> related to of_match_node. >> >> I'm saying this because in lantiq_gswip.c, xway_gphy_match is accessed >> through of_match_node(). If the shim definition for of_match_node() was >> different, the variable wouldn't have been unused with CONFIG_OF=n. >> I guess it's worth considering changing that wrapper instead of adding >> the __maybe_unused. > > Hi Krzysztof, have you had a chance to check if using an empty static > inline is enough to silence the compiler? Seems like it could save > us quite some churn? Or do we want the of_match_node() decorations > to go away in general? I am pretty sure fixing of_match_node() and of_match_ptr() (independent case) would supersed this patchset, but it is a bit bigger change than I have available time now. I didn't try it yet. Best regards, Krzysztof
diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c index 05ecaa007ab1..3c76a1a14aee 100644 --- a/drivers/net/dsa/lantiq_gswip.c +++ b/drivers/net/dsa/lantiq_gswip.c @@ -1885,7 +1885,7 @@ static const struct xway_gphy_match_data xrx300_gphy_data = { .ge_firmware_name = "lantiq/xrx300_phy11g_a21.bin", }; -static const struct of_device_id xway_gphy_match[] = { +static const struct of_device_id xway_gphy_match[] __maybe_unused = { { .compatible = "lantiq,xrx200-gphy-fw", .data = NULL }, { .compatible = "lantiq,xrx200a1x-gphy-fw", .data = &xrx200a1x_gphy_data }, { .compatible = "lantiq,xrx200a2x-gphy-fw", .data = &xrx200a2x_gphy_data },
The driver can be compile tested with !CONFIG_OF making certain data unused: drivers/net/dsa/lantiq_gswip.c:1888:34: error: ‘xway_gphy_match’ defined but not used [-Werror=unused-const-variable=] Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- drivers/net/dsa/lantiq_gswip.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)