* [PATCH] mbox: qcom: avoid unused-variable warning
@ 2019-09-20 14:55 Arnd Bergmann
2019-09-20 16:45 ` Stephen Boyd
0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2019-09-20 14:55 UTC (permalink / raw)
To: Andy Gross, Jassi Brar
Cc: Arnd Bergmann, Niklas Cassel, Jorge Ramirez-Ortiz,
Bjorn Andersson, Stephen Boyd, Jassi Brar, linux-arm-msm,
linux-kernel
Without CONFIG_OF, there is no reference to the apcs_clk_match_table[]
array, causing a harmless warning:
drivers/mailbox/qcom-apcs-ipc-mailbox.c:57:28: error: unused variable 'apcs_clk_match_table' [-Werror,-Wunused-variable]
const struct of_device_id apcs_clk_match_table[] = {
Move the variable out of the variable scope and mark it 'static'
to avoid the warning (static const variables get silently dropped
by the compiler), and avoid the on-stack copy at the same time.
Fixes: 78c86458a440 ("mbox: qcom: add APCS child device for QCS404")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/mailbox/qcom-apcs-ipc-mailbox.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
index eeebafd546e5..10557a950c2d 100644
--- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c
+++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c
@@ -45,6 +45,12 @@ static const struct mbox_chan_ops qcom_apcs_ipc_ops = {
.send_data = qcom_apcs_ipc_send_data,
};
+static const struct of_device_id apcs_clk_match_table[] = {
+ { .compatible = "qcom,msm8916-apcs-kpss-global", },
+ { .compatible = "qcom,qcs404-apcs-apps-global", },
+ {}
+};
+
static int qcom_apcs_ipc_probe(struct platform_device *pdev)
{
struct qcom_apcs_ipc *apcs;
@@ -54,11 +60,6 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev)
void __iomem *base;
unsigned long i;
int ret;
- const struct of_device_id apcs_clk_match_table[] = {
- { .compatible = "qcom,msm8916-apcs-kpss-global", },
- { .compatible = "qcom,qcs404-apcs-apps-global", },
- {}
- };
apcs = devm_kzalloc(&pdev->dev, sizeof(*apcs), GFP_KERNEL);
if (!apcs)
--
2.20.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] mbox: qcom: avoid unused-variable warning 2019-09-20 14:55 [PATCH] mbox: qcom: avoid unused-variable warning Arnd Bergmann @ 2019-09-20 16:45 ` Stephen Boyd 2019-09-20 19:27 ` Arnd Bergmann 0 siblings, 1 reply; 7+ messages in thread From: Stephen Boyd @ 2019-09-20 16:45 UTC (permalink / raw) To: Andy Gross, Arnd Bergmann, Jassi Brar Cc: Arnd Bergmann, Niklas Cassel, Jorge Ramirez-Ortiz, Bjorn Andersson, Jassi Brar, linux-arm-msm, linux-kernel Quoting Arnd Bergmann (2019-09-20 07:55:29) > Without CONFIG_OF, there is no reference to the apcs_clk_match_table[] > array, causing a harmless warning: > > drivers/mailbox/qcom-apcs-ipc-mailbox.c:57:28: error: unused variable 'apcs_clk_match_table' [-Werror,-Wunused-variable] > const struct of_device_id apcs_clk_match_table[] = { > > Move the variable out of the variable scope and mark it 'static' > to avoid the warning (static const variables get silently dropped > by the compiler), and avoid the on-stack copy at the same time. > > Fixes: 78c86458a440 ("mbox: qcom: add APCS child device for QCS404") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/mailbox/qcom-apcs-ipc-mailbox.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/mailbox/qcom-apcs-ipc-mailbox.c b/drivers/mailbox/qcom-apcs-ipc-mailbox.c > index eeebafd546e5..10557a950c2d 100644 > --- a/drivers/mailbox/qcom-apcs-ipc-mailbox.c > +++ b/drivers/mailbox/qcom-apcs-ipc-mailbox.c > @@ -45,6 +45,12 @@ static const struct mbox_chan_ops qcom_apcs_ipc_ops = { > .send_data = qcom_apcs_ipc_send_data, > }; > > +static const struct of_device_id apcs_clk_match_table[] = { > + { .compatible = "qcom,msm8916-apcs-kpss-global", }, > + { .compatible = "qcom,qcs404-apcs-apps-global", }, > + {} > +}; > + > static int qcom_apcs_ipc_probe(struct platform_device *pdev) > { > struct qcom_apcs_ipc *apcs; > @@ -54,11 +60,6 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev) > void __iomem *base; > unsigned long i; > int ret; > - const struct of_device_id apcs_clk_match_table[] = { Does marking it static here work too? It would be nice to limit the scope of this variable to this function instead of making it a global. Also, it might be slightly smaller code size if that works. > - { .compatible = "qcom,msm8916-apcs-kpss-global", }, > - { .compatible = "qcom,qcs404-apcs-apps-global", }, > - {} > - }; > > apcs = devm_kzalloc(&pdev->dev, sizeof(*apcs), GFP_KERNEL); > if (!apcs) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mbox: qcom: avoid unused-variable warning 2019-09-20 16:45 ` Stephen Boyd @ 2019-09-20 19:27 ` Arnd Bergmann [not found] ` <20190920210622.51382205F4@mail.kernel.org> 0 siblings, 1 reply; 7+ messages in thread From: Arnd Bergmann @ 2019-09-20 19:27 UTC (permalink / raw) To: Stephen Boyd Cc: Andy Gross, Jassi Brar, Niklas Cassel, Jorge Ramirez-Ortiz, Bjorn Andersson, Jassi Brar, linux-arm-msm, linux-kernel On Fri, Sep 20, 2019 at 6:45 PM Stephen Boyd <sboyd@kernel.org> wrote: > > @@ -54,11 +60,6 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev) > > void __iomem *base; > > unsigned long i; > > int ret; > > - const struct of_device_id apcs_clk_match_table[] = { > > Does marking it static here work too? It would be nice to limit the > scope of this variable to this function instead of making it a global. > Also, it might be slightly smaller code size if that works. No, I just tried and the warning returned. Arnd ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20190920210622.51382205F4@mail.kernel.org>]
* Re: [PATCH] mbox: qcom: avoid unused-variable warning [not found] ` <20190920210622.51382205F4@mail.kernel.org> @ 2019-09-26 13:07 ` Geert Uytterhoeven 2019-09-27 18:26 ` Stephen Boyd 0 siblings, 1 reply; 7+ messages in thread From: Geert Uytterhoeven @ 2019-09-26 13:07 UTC (permalink / raw) To: Stephen Boyd Cc: Arnd Bergmann, Geert Uytterhoeven, Andy Gross, Jassi Brar, Niklas Cassel, Jorge Ramirez-Ortiz, Bjorn Andersson, Jassi Brar, linux-arm-msm, linux-kernel Hi Stephen, On Fri, Sep 20, 2019 at 11:06 PM Stephen Boyd <sboyd@kernel.org> wrote: > Quoting Arnd Bergmann (2019-09-20 12:27:50) > > On Fri, Sep 20, 2019 at 6:45 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > @@ -54,11 +60,6 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev) > > > > void __iomem *base; > > > > unsigned long i; > > > > int ret; > > > > - const struct of_device_id apcs_clk_match_table[] = { > > > > > > Does marking it static here work too? It would be nice to limit the > > > scope of this variable to this function instead of making it a global. > > > Also, it might be slightly smaller code size if that works. > > > > No, I just tried and the warning returned. It's there for the convenience for the user, so he doesn't have to add it himself explicitly. > ("of/device: Nullify match table in of_match_device() for CONFIG_OF=n"), > but it's been 5 years! Surely we can revert this change now that commit > 219a7bc577e6 ("spi: rspi: Use of_device_get_match_data() helper") is > merged. Right, the particular error case in the RSPI driver can no longer happen. Calling of_device_get_match_data() is the better solution anyway, as that uses the match table stored in dev->driver->of_match_table, so there's no need to pass the table explicitly, and conditionally. Hence... > --- a/drivers/leds/leds-pca9532.c > +++ b/drivers/leds/leds-pca9532.c > @@ -472,7 +472,7 @@ pca9532_of_populate_pdata(struct device *dev, struct device_node *np) > int i = 0; > const char *state; > > - match = of_match_device(of_pca9532_leds_match, dev); > + match = of_match_device(of_match_ptr(of_pca9532_leds_match), dev); > if (!match) > return ERR_PTR(-ENODEV); Please convert to of_device_get_match_data() instead of adding of_match_ptr() invocations... > @@ -525,7 +525,7 @@ static int pca9532_probe(struct i2c_client *client, > dev_err(&client->dev, "no platform data\n"); > return -EINVAL; > } > - of_id = of_match_device(of_pca9532_leds_match, > + of_id = of_match_device(of_match_ptr(of_pca9532_leds_match), Likewise. > --- a/sound/soc/jz4740/jz4740-i2s.c > +++ b/sound/soc/jz4740/jz4740-i2s.c > @@ -503,7 +503,7 @@ static int jz4740_i2s_dev_probe(struct platform_device *pdev) > if (!i2s) > return -ENOMEM; > > - match = of_match_device(jz4740_of_matches, &pdev->dev); > + match = of_match_device(of_match_ptr(jz4740_of_matches), &pdev->dev); Given of_device_get_match_data() returns NULL, not an error code, even this one could use of_device_get_match_data(). > if (match) > i2s->version = (enum jz47xx_i2s_version)match->data; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mbox: qcom: avoid unused-variable warning 2019-09-26 13:07 ` Geert Uytterhoeven @ 2019-09-27 18:26 ` Stephen Boyd 2019-09-27 19:10 ` Geert Uytterhoeven 0 siblings, 1 reply; 7+ messages in thread From: Stephen Boyd @ 2019-09-27 18:26 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Arnd Bergmann, Geert Uytterhoeven, Andy Gross, Jassi Brar, Niklas Cassel, Jorge Ramirez-Ortiz, Bjorn Andersson, Jassi Brar, linux-arm-msm, linux-kernel Quoting Geert Uytterhoeven (2019-09-26 06:07:13) > Hi Stephen, > > On Fri, Sep 20, 2019 at 11:06 PM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Arnd Bergmann (2019-09-20 12:27:50) > > > On Fri, Sep 20, 2019 at 6:45 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > > @@ -54,11 +60,6 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev) > > > > > void __iomem *base; > > > > > unsigned long i; > > > > > int ret; > > > > > - const struct of_device_id apcs_clk_match_table[] = { > > > > > > > > Does marking it static here work too? It would be nice to limit the > > > > scope of this variable to this function instead of making it a global. > > > > Also, it might be slightly smaller code size if that works. > > > > > > No, I just tried and the warning returned. > > It's there for the convenience for the user, so he doesn't have to add it > himself explicitly. > > > ("of/device: Nullify match table in of_match_device() for CONFIG_OF=n"), > > but it's been 5 years! Surely we can revert this change now that commit > > 219a7bc577e6 ("spi: rspi: Use of_device_get_match_data() helper") is > > merged. > > Right, the particular error case in the RSPI driver can no longer happen. > > Calling of_device_get_match_data() is the better solution anyway, as > that uses the match table stored in dev->driver->of_match_table, so > there's no need to pass the table explicitly, and conditionally. > > Hence... > > > --- a/drivers/leds/leds-pca9532.c > > +++ b/drivers/leds/leds-pca9532.c > > @@ -472,7 +472,7 @@ pca9532_of_populate_pdata(struct device *dev, struct device_node *np) > > int i = 0; > > const char *state; > > > > - match = of_match_device(of_pca9532_leds_match, dev); > > + match = of_match_device(of_match_ptr(of_pca9532_leds_match), dev); > > if (!match) > > return ERR_PTR(-ENODEV); > > Please convert to of_device_get_match_data() instead of adding > of_match_ptr() invocations... How is this workable? I left it as of_match_device() because the value returned may be 0 for the enum and that looks the same as NULL. > > > @@ -525,7 +525,7 @@ static int pca9532_probe(struct i2c_client *client, > > dev_err(&client->dev, "no platform data\n"); > > return -EINVAL; > > } > > - of_id = of_match_device(of_pca9532_leds_match, > > + of_id = of_match_device(of_match_ptr(of_pca9532_leds_match), > > Likewise. > > > --- a/sound/soc/jz4740/jz4740-i2s.c > > +++ b/sound/soc/jz4740/jz4740-i2s.c > > @@ -503,7 +503,7 @@ static int jz4740_i2s_dev_probe(struct platform_device *pdev) > > if (!i2s) > > return -ENOMEM; > > > > - match = of_match_device(jz4740_of_matches, &pdev->dev); > > + match = of_match_device(of_match_ptr(jz4740_of_matches), &pdev->dev); > > Given of_device_get_match_data() returns NULL, not an error code, even > this one could use of_device_get_match_data(). > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mbox: qcom: avoid unused-variable warning 2019-09-27 18:26 ` Stephen Boyd @ 2019-09-27 19:10 ` Geert Uytterhoeven 2019-10-03 21:16 ` Stephen Boyd 0 siblings, 1 reply; 7+ messages in thread From: Geert Uytterhoeven @ 2019-09-27 19:10 UTC (permalink / raw) To: Stephen Boyd Cc: Arnd Bergmann, Geert Uytterhoeven, Andy Gross, Jassi Brar, Niklas Cassel, Jorge Ramirez-Ortiz, Bjorn Andersson, Jassi Brar, linux-arm-msm, linux-kernel Hi Stephen, On Fri, Sep 27, 2019 at 8:26 PM Stephen Boyd <sboyd@kernel.org> wrote: > Quoting Geert Uytterhoeven (2019-09-26 06:07:13) > > On Fri, Sep 20, 2019 at 11:06 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > Quoting Arnd Bergmann (2019-09-20 12:27:50) > > > > On Fri, Sep 20, 2019 at 6:45 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > @@ -54,11 +60,6 @@ static int qcom_apcs_ipc_probe(struct platform_device *pdev) > > > > > > void __iomem *base; > > > > > > unsigned long i; > > > > > > int ret; > > > > > > - const struct of_device_id apcs_clk_match_table[] = { > > > > > > > > > > Does marking it static here work too? It would be nice to limit the > > > > > scope of this variable to this function instead of making it a global. > > > > > Also, it might be slightly smaller code size if that works. > > > > > > > > No, I just tried and the warning returned. > > > > It's there for the convenience for the user, so he doesn't have to add it > > himself explicitly. > > > > > ("of/device: Nullify match table in of_match_device() for CONFIG_OF=n"), > > > but it's been 5 years! Surely we can revert this change now that commit > > > 219a7bc577e6 ("spi: rspi: Use of_device_get_match_data() helper") is > > > merged. > > > > Right, the particular error case in the RSPI driver can no longer happen. > > > > Calling of_device_get_match_data() is the better solution anyway, as > > that uses the match table stored in dev->driver->of_match_table, so > > there's no need to pass the table explicitly, and conditionally. > > > > Hence... > > > > > --- a/drivers/leds/leds-pca9532.c > > > +++ b/drivers/leds/leds-pca9532.c > > > @@ -472,7 +472,7 @@ pca9532_of_populate_pdata(struct device *dev, struct device_node *np) > > > int i = 0; > > > const char *state; > > > > > > - match = of_match_device(of_pca9532_leds_match, dev); > > > + match = of_match_device(of_match_ptr(of_pca9532_leds_match), dev); > > > if (!match) > > > return ERR_PTR(-ENODEV); > > > > Please convert to of_device_get_match_data() instead of adding > > of_match_ptr() invocations... > > How is this workable? I left it as of_match_device() because the value > returned may be 0 for the enum and that looks the same as NULL. This function is used for the DT case only, so there will always be a match. Hence you can do devid = (int)(uintptr_t)of_device_get_match_data(dev). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mbox: qcom: avoid unused-variable warning 2019-09-27 19:10 ` Geert Uytterhoeven @ 2019-10-03 21:16 ` Stephen Boyd 0 siblings, 0 replies; 7+ messages in thread From: Stephen Boyd @ 2019-10-03 21:16 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Arnd Bergmann, Geert Uytterhoeven, Andy Gross, Jassi Brar, Niklas Cassel, Jorge Ramirez-Ortiz, Bjorn Andersson, Jassi Brar, linux-arm-msm, linux-kernel Quoting Geert Uytterhoeven (2019-09-27 12:10:13) > On Fri, Sep 27, 2019 at 8:26 PM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Geert Uytterhoeven (2019-09-26 06:07:13) > > > On Fri, Sep 20, 2019 at 11:06 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Arnd Bergmann (2019-09-20 12:27:50) > > > > > On Fri, Sep 20, 2019 at 6:45 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > > --- a/drivers/leds/leds-pca9532.c > > > > +++ b/drivers/leds/leds-pca9532.c > > > > @@ -472,7 +472,7 @@ pca9532_of_populate_pdata(struct device *dev, struct device_node *np) > > > > int i = 0; > > > > const char *state; > > > > > > > > - match = of_match_device(of_pca9532_leds_match, dev); > > > > + match = of_match_device(of_match_ptr(of_pca9532_leds_match), dev); > > > > if (!match) > > > > return ERR_PTR(-ENODEV); > > > > > > Please convert to of_device_get_match_data() instead of adding > > > of_match_ptr() invocations... > > > > How is this workable? I left it as of_match_device() because the value > > returned may be 0 for the enum and that looks the same as NULL. > > This function is used for the DT case only, so there will always be a match. > Hence you can do devid = (int)(uintptr_t)of_device_get_match_data(dev). > Ok. Let me send out a pile of patches. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-03 21:16 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-20 14:55 [PATCH] mbox: qcom: avoid unused-variable warning Arnd Bergmann
2019-09-20 16:45 ` Stephen Boyd
2019-09-20 19:27 ` Arnd Bergmann
[not found] ` <20190920210622.51382205F4@mail.kernel.org>
2019-09-26 13:07 ` Geert Uytterhoeven
2019-09-27 18:26 ` Stephen Boyd
2019-09-27 19:10 ` Geert Uytterhoeven
2019-10-03 21:16 ` Stephen Boyd
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).