* [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
* 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).