Hi All,
These patches are trivial fixes for rpc-if SPI driver.
Cheers,
Prabhakar
Lad Prabhakar (2):
spi: rpc-if: Avoid use of C++ style comments
spi: rpc-if: Remove CONFIG_PM_SLEEP ifdefery
drivers/spi/spi-rpc-if.c | 25 ++++++++++---------------
1 file changed, 10 insertions(+), 15 deletions(-)
--
2.17.1
Replace C++ style comment with C style.
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/spi/spi-rpc-if.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/spi/spi-rpc-if.c b/drivers/spi/spi-rpc-if.c
index 3579675485a5..bf64da322e67 100644
--- a/drivers/spi/spi-rpc-if.c
+++ b/drivers/spi/spi-rpc-if.c
@@ -1,11 +1,11 @@
// SPDX-License-Identifier: GPL-2.0
-//
-// RPC-IF SPI/QSPI/Octa driver
-//
-// Copyright (C) 2018 ~ 2019 Renesas Solutions Corp.
-// Copyright (C) 2019 Macronix International Co., Ltd.
-// Copyright (C) 2019 - 2020 Cogent Embedded, Inc.
-//
+/*
+ * RPC-IF SPI/QSPI/Octa driver
+ *
+ * Copyright (C) 2018 ~ 2019 Renesas Solutions Corp.
+ * Copyright (C) 2019 Macronix International Co., Ltd.
+ * Copyright (C) 2019 - 2020 Cogent Embedded, Inc.
+ */
#include <linux/module.h>
#include <linux/platform_device.h>
--
2.17.1
Use __maybe_unused for the suspend()/resume() hooks and get rid of
the CONFIG_PM_SLEEP ifdefery to improve the code.
Suggested-by: Pavel Machek <[email protected]>
Signed-off-by: Lad Prabhakar <[email protected]>
---
drivers/spi/spi-rpc-if.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-rpc-if.c b/drivers/spi/spi-rpc-if.c
index bf64da322e67..b600becd4691 100644
--- a/drivers/spi/spi-rpc-if.c
+++ b/drivers/spi/spi-rpc-if.c
@@ -176,15 +176,14 @@ static int rpcif_spi_remove(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM_SLEEP
-static int rpcif_spi_suspend(struct device *dev)
+static int __maybe_unused rpcif_spi_suspend(struct device *dev)
{
struct spi_controller *ctlr = dev_get_drvdata(dev);
return spi_controller_suspend(ctlr);
}
-static int rpcif_spi_resume(struct device *dev)
+static int __maybe_unused rpcif_spi_resume(struct device *dev)
{
struct spi_controller *ctlr = dev_get_drvdata(dev);
@@ -192,17 +191,13 @@ static int rpcif_spi_resume(struct device *dev)
}
static SIMPLE_DEV_PM_OPS(rpcif_spi_pm_ops, rpcif_spi_suspend, rpcif_spi_resume);
-#define DEV_PM_OPS (&rpcif_spi_pm_ops)
-#else
-#define DEV_PM_OPS NULL
-#endif
static struct platform_driver rpcif_spi_driver = {
.probe = rpcif_spi_probe,
.remove = rpcif_spi_remove,
.driver = {
.name = "rpc-if-spi",
- .pm = DEV_PM_OPS,
+ .pm = &rpcif_spi_pm_ops,
},
};
module_platform_driver(rpcif_spi_driver);
--
2.17.1
On 12/30/20 5:57 PM, Lad Prabhakar wrote:
> Replace C++ style comment with C style.
Note that the switch to // was made following the SPI maintainer's request...
> Signed-off-by: Lad Prabhakar <[email protected]>
[...]
MBR, Sergei
On Wed 2020-12-30 14:57:08, Lad Prabhakar wrote:
> Use __maybe_unused for the suspend()/resume() hooks and get rid of
> the CONFIG_PM_SLEEP ifdefery to improve the code.
>
> Suggested-by: Pavel Machek <[email protected]>
Acked-by: Pavel Machek <[email protected]>
> Signed-off-by: Lad Prabhakar <[email protected]>
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Hi Sergei,
On Wed, Dec 30, 2020 at 4:27 PM Sergei Shtylyov
<[email protected]> wrote:
>
> On 12/30/20 5:57 PM, Lad Prabhakar wrote:
>
> > Replace C++ style comment with C style.
>
> Note that the switch to // was made following the SPI maintainer's request...
>
Thanks for letting me know, let's drop this patch.
Cheers,
Prabhakar
> > Signed-off-by: Lad Prabhakar <[email protected]>
> [...]
>
> MBR, Sergei
On Wed, 30 Dec 2020 14:57:06 +0000, Lad Prabhakar wrote:
> These patches are trivial fixes for rpc-if SPI driver.
>
> Cheers,
> Prabhakar
>
> Lad Prabhakar (2):
> spi: rpc-if: Avoid use of C++ style comments
> spi: rpc-if: Remove CONFIG_PM_SLEEP ifdefery
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[2/2] spi: rpc-if: Remove CONFIG_PM_SLEEP ifdefery
commit: 9584fc95cadc0b86e5e01cefcff0ab2b31ee3a5b
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
Hi Prabhakar,
On Wed, Dec 30, 2020 at 4:00 PM Lad Prabhakar
<[email protected]> wrote:
> Use __maybe_unused for the suspend()/resume() hooks and get rid of
> the CONFIG_PM_SLEEP ifdefery to improve the code.
>
> Suggested-by: Pavel Machek <[email protected]>
> Signed-off-by: Lad Prabhakar <[email protected]>
Thanks for your patch!
> --- a/drivers/spi/spi-rpc-if.c
> +++ b/drivers/spi/spi-rpc-if.c
> @@ -176,15 +176,14 @@ static int rpcif_spi_remove(struct platform_device *pdev)
> return 0;
> }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int rpcif_spi_suspend(struct device *dev)
> +static int __maybe_unused rpcif_spi_suspend(struct device *dev)
> {
> struct spi_controller *ctlr = dev_get_drvdata(dev);
>
> return spi_controller_suspend(ctlr);
> }
>
> -static int rpcif_spi_resume(struct device *dev)
> +static int __maybe_unused rpcif_spi_resume(struct device *dev)
> {
> struct spi_controller *ctlr = dev_get_drvdata(dev);
>
> @@ -192,17 +191,13 @@ static int rpcif_spi_resume(struct device *dev)
> }
>
> static SIMPLE_DEV_PM_OPS(rpcif_spi_pm_ops, rpcif_spi_suspend, rpcif_spi_resume);
> -#define DEV_PM_OPS (&rpcif_spi_pm_ops)
> -#else
> -#define DEV_PM_OPS NULL
> -#endif
>
> static struct platform_driver rpcif_spi_driver = {
> .probe = rpcif_spi_probe,
> .remove = rpcif_spi_remove,
> .driver = {
> .name = "rpc-if-spi",
> - .pm = DEV_PM_OPS,
> + .pm = &rpcif_spi_pm_ops,
You're aware rpcif_spi_pm_ops is now always referenced and thus emitted,
increasing kernel size by 92 bytes if CONFIG_PM_SLEEP=n?
This may matter for RZ/A SoCs running from internal SRAM.
> },
> };
> module_platform_driver(rpcif_spi_driver);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Mon, Jan 04, 2021 at 09:25:17PM +0000, Lad, Prabhakar wrote:
> > > .name = "rpc-if-spi",
> > > - .pm = DEV_PM_OPS,
> > > + .pm = &rpcif_spi_pm_ops,
> > You're aware rpcif_spi_pm_ops is now always referenced and thus emitted,
> > increasing kernel size by 92 bytes if CONFIG_PM_SLEEP=n?
> > This may matter for RZ/A SoCs running from internal SRAM.
> Hmm didn't realise this would be an issue on RZ/A.
> Mark, could you please drop this patch from your branch.
Please send an incremental patch with an appropriate changelog.
Hi Geert,
Thank you for the review.
On Mon, Jan 4, 2021 at 12:34 PM Geert Uytterhoeven <[email protected]> wrote:
>
> Hi Prabhakar,
>
> On Wed, Dec 30, 2020 at 4:00 PM Lad Prabhakar
> <[email protected]> wrote:
> > Use __maybe_unused for the suspend()/resume() hooks and get rid of
> > the CONFIG_PM_SLEEP ifdefery to improve the code.
> >
> > Suggested-by: Pavel Machek <[email protected]>
> > Signed-off-by: Lad Prabhakar <[email protected]>
>
> Thanks for your patch!
>
> > --- a/drivers/spi/spi-rpc-if.c
> > +++ b/drivers/spi/spi-rpc-if.c
> > @@ -176,15 +176,14 @@ static int rpcif_spi_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > -#ifdef CONFIG_PM_SLEEP
> > -static int rpcif_spi_suspend(struct device *dev)
> > +static int __maybe_unused rpcif_spi_suspend(struct device *dev)
> > {
> > struct spi_controller *ctlr = dev_get_drvdata(dev);
> >
> > return spi_controller_suspend(ctlr);
> > }
> >
> > -static int rpcif_spi_resume(struct device *dev)
> > +static int __maybe_unused rpcif_spi_resume(struct device *dev)
> > {
> > struct spi_controller *ctlr = dev_get_drvdata(dev);
> >
> > @@ -192,17 +191,13 @@ static int rpcif_spi_resume(struct device *dev)
> > }
> >
> > static SIMPLE_DEV_PM_OPS(rpcif_spi_pm_ops, rpcif_spi_suspend, rpcif_spi_resume);
> > -#define DEV_PM_OPS (&rpcif_spi_pm_ops)
> > -#else
> > -#define DEV_PM_OPS NULL
> > -#endif
> >
> > static struct platform_driver rpcif_spi_driver = {
> > .probe = rpcif_spi_probe,
> > .remove = rpcif_spi_remove,
> > .driver = {
> > .name = "rpc-if-spi",
> > - .pm = DEV_PM_OPS,
> > + .pm = &rpcif_spi_pm_ops,
>
> You're aware rpcif_spi_pm_ops is now always referenced and thus emitted,
> increasing kernel size by 92 bytes if CONFIG_PM_SLEEP=n?
> This may matter for RZ/A SoCs running from internal SRAM.
>
Hmm didn't realise this would be an issue on RZ/A.
Mark, could you please drop this patch from your branch.
Cheers,
Prabhakar
> > },
> > };
> > module_platform_driver(rpcif_spi_driver);
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> 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
Hi!
> > > > .name = "rpc-if-spi",
> > > > - .pm = DEV_PM_OPS,
> > > > + .pm = &rpcif_spi_pm_ops,
>
> > > You're aware rpcif_spi_pm_ops is now always referenced and thus emitted,
> > > increasing kernel size by 92 bytes if CONFIG_PM_SLEEP=n?
> > > This may matter for RZ/A SoCs running from internal SRAM.
>
> > Hmm didn't realise this would be an issue on RZ/A.
>
> > Mark, could you please drop this patch from your branch.
>
> Please send an incremental patch with an appropriate changelog.
Let's fix this properly. I'm pretty sure we have some macros that can
solve this without re-introducing the ifdefs...
(Besides... 92 bytes. How big is kernel these days? 4MB? More? How
much SRAM do you have?)
Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Hi Pavel,
On Tue, Jan 5, 2021 at 12:40 AM Pavel Machek <[email protected]> wrote:
> > > > > .name = "rpc-if-spi",
> > > > > - .pm = DEV_PM_OPS,
> > > > > + .pm = &rpcif_spi_pm_ops,
> >
> > > > You're aware rpcif_spi_pm_ops is now always referenced and thus emitted,
> > > > increasing kernel size by 92 bytes if CONFIG_PM_SLEEP=n?
> > > > This may matter for RZ/A SoCs running from internal SRAM.
> >
> > > Hmm didn't realise this would be an issue on RZ/A.
> >
> > > Mark, could you please drop this patch from your branch.
> >
> > Please send an incremental patch with an appropriate changelog.
>
> Let's fix this properly. I'm pretty sure we have some macros that can
> solve this without re-introducing the ifdefs...
There's pm_ptr(), but it uses CONFIG_PM as a selector, not CONFIG_PM_SLEEP.
> (Besides... 92 bytes. How big is kernel these days? 4MB? More? How
> much SRAM do you have?)
92 bytes is indeed not much (on 64-bit it would be doubled).
Still, it's good to make people think about innocent looking changes,
once in a while.
RZ/A1H and RZ/A1M have 10 resp. 5 MiB of SRAM.
RZ/A2 has 4 MiB SRAM, which is sufficient to run Linux when used with
XIP (requires a one-line Kconfig change rmk has been vetoing for years).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi Pavel,
On Tue, Jan 5, 2021 at 11:42 AM Pavel Machek <[email protected]> wrote:
> > On Tue, Jan 5, 2021 at 12:40 AM Pavel Machek <[email protected]> wrote:
> > > > > > > .name = "rpc-if-spi",
> > > > > > > - .pm = DEV_PM_OPS,
> > > > > > > + .pm = &rpcif_spi_pm_ops,
> > > >
> > > > > > You're aware rpcif_spi_pm_ops is now always referenced and thus emitted,
> > > > > > increasing kernel size by 92 bytes if CONFIG_PM_SLEEP=n?
> > > > > > This may matter for RZ/A SoCs running from internal SRAM.
> > > >
> > > > > Hmm didn't realise this would be an issue on RZ/A.
> > > >
> > > > > Mark, could you please drop this patch from your branch.
> > > >
> > > > Please send an incremental patch with an appropriate changelog.
> > >
> > > Let's fix this properly. I'm pretty sure we have some macros that can
> > > solve this without re-introducing the ifdefs...
> >
> > There's pm_ptr(), but it uses CONFIG_PM as a selector, not CONFIG_PM_SLEEP.
>
> Okay; so we could introduce pm_sleep_ptr().
>
> Or we could just put single #ifdef CONFIG_PM_SLEEP around the .pm
> assignment? That would be improvement on the original, and still
> result in the same binary, right?
Indeed.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
Hi!
> On Tue, Jan 5, 2021 at 12:40 AM Pavel Machek <[email protected]> wrote:
> > > > > > .name = "rpc-if-spi",
> > > > > > - .pm = DEV_PM_OPS,
> > > > > > + .pm = &rpcif_spi_pm_ops,
> > >
> > > > > You're aware rpcif_spi_pm_ops is now always referenced and thus emitted,
> > > > > increasing kernel size by 92 bytes if CONFIG_PM_SLEEP=n?
> > > > > This may matter for RZ/A SoCs running from internal SRAM.
> > >
> > > > Hmm didn't realise this would be an issue on RZ/A.
> > >
> > > > Mark, could you please drop this patch from your branch.
> > >
> > > Please send an incremental patch with an appropriate changelog.
> >
> > Let's fix this properly. I'm pretty sure we have some macros that can
> > solve this without re-introducing the ifdefs...
>
> There's pm_ptr(), but it uses CONFIG_PM as a selector, not CONFIG_PM_SLEEP.
Okay; so we could introduce pm_sleep_ptr().
Or we could just put single #ifdef CONFIG_PM_SLEEP around the .pm
assignment? That would be improvement on the original, and still
result in the same binary, right?
> > (Besides... 92 bytes. How big is kernel these days? 4MB? More? How
> > much SRAM do you have?)
>
> 92 bytes is indeed not much (on 64-bit it would be doubled).
> Still, it's good to make people think about innocent looking changes,
> once in a while.
>
> RZ/A1H and RZ/A1M have 10 resp. 5 MiB of SRAM.
> RZ/A2 has 4 MiB SRAM, which is sufficient to run Linux when used with
> XIP (requires a one-line Kconfig change rmk has been vetoing for
> years).
Aha, that is a bit smaller than I expected.
Best regards,
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany