2020-05-28 19:11:40

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH] spi: bcm2835: Implement shutdown callback

Make sure we clear the FIFOs, stop the block, disable the clock and
release the DMA channel.

Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/spi/spi-bcm2835.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 20d8581fdf88..237bd306c268 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -1391,6 +1391,15 @@ static int bcm2835_spi_remove(struct platform_device *pdev)
return 0;
}

+static void bcm2835_spi_shutdown(struct platform_device *pdev)
+{
+ int ret;
+
+ ret = bcm2835_spi_remove(pdev);
+ if (ret)
+ dev_err(&pdev->dev, "failed to shutdown\n");
+}
+
static const struct of_device_id bcm2835_spi_match[] = {
{ .compatible = "brcm,bcm2835-spi", },
{}
@@ -1404,6 +1413,7 @@ static struct platform_driver bcm2835_spi_driver = {
},
.probe = bcm2835_spi_probe,
.remove = bcm2835_spi_remove,
+ .shutdown = bcm2835_spi_shutdown,
};
module_platform_driver(bcm2835_spi_driver);

--
2.17.1


2020-05-29 17:21:10

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] spi: bcm2835: Implement shutdown callback

On Thu, 28 May 2020 12:06:05 -0700, Florian Fainelli wrote:
> Make sure we clear the FIFOs, stop the block, disable the clock and
> release the DMA channel.

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: bcm2835: Implement shutdown callback
commit: 118eb0e52eb74b899053a0f46dfe7e178788d23b

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

2020-05-29 17:49:51

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH] spi: bcm2835: Implement shutdown callback

On Thu, May 28, 2020 at 12:06:05PM -0700, Florian Fainelli wrote:
> Make sure we clear the FIFOs, stop the block, disable the clock and
> release the DMA channel.

To what end? Why is this change necessary? Sorry but this seems like
an awfully terse commit message.

Thanks,

Lukas

>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/spi/spi-bcm2835.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
> index 20d8581fdf88..237bd306c268 100644
> --- a/drivers/spi/spi-bcm2835.c
> +++ b/drivers/spi/spi-bcm2835.c
> @@ -1391,6 +1391,15 @@ static int bcm2835_spi_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static void bcm2835_spi_shutdown(struct platform_device *pdev)
> +{
> + int ret;
> +
> + ret = bcm2835_spi_remove(pdev);
> + if (ret)
> + dev_err(&pdev->dev, "failed to shutdown\n");
> +}
> +
> static const struct of_device_id bcm2835_spi_match[] = {
> { .compatible = "brcm,bcm2835-spi", },
> {}
> @@ -1404,6 +1413,7 @@ static struct platform_driver bcm2835_spi_driver = {
> },
> .probe = bcm2835_spi_probe,
> .remove = bcm2835_spi_remove,
> + .shutdown = bcm2835_spi_shutdown,
> };
> module_platform_driver(bcm2835_spi_driver);
>
> --
> 2.17.1
>

2020-05-29 17:50:36

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH] spi: bcm2835: Implement shutdown callback

On 5/29/20 10:47 AM, Lukas Wunner wrote:
> On Thu, May 28, 2020 at 12:06:05PM -0700, Florian Fainelli wrote:
>> Make sure we clear the FIFOs, stop the block, disable the clock and
>> release the DMA channel.
>
> To what end? Why is this change necessary? Sorry but this seems like
> an awfully terse commit message.

To ensure clocks are disabled and to save power in low power modes used
on 7211 for instance.
--
Florian

2020-05-29 18:00:39

by Lukas Wunner

[permalink] [raw]
Subject: Re: [PATCH] spi: bcm2835: Implement shutdown callback

On Fri, May 29, 2020 at 10:48:11AM -0700, Florian Fainelli wrote:
> On 5/29/20 10:47 AM, Lukas Wunner wrote:
> > On Thu, May 28, 2020 at 12:06:05PM -0700, Florian Fainelli wrote:
> >> Make sure we clear the FIFOs, stop the block, disable the clock and
> >> release the DMA channel.
> >
> > To what end? Why is this change necessary? Sorry but this seems like
> > an awfully terse commit message.
>
> To ensure clocks are disabled and to save power in low power modes used
> on 7211 for instance.

Thanks for the explanation, that's an important tidbit. I wasn't even
aware that this SPI controller is used on SoCs beyond the Raspberry Pi
ones. Does the BCM7211 use shared interrupts for this controller?
Does it have DMA DREQ attached?

2020-05-29 18:07:33

by Florian Fainelli

[permalink] [raw]
Subject: Re: [PATCH] spi: bcm2835: Implement shutdown callback

On 5/29/20 10:58 AM, Lukas Wunner wrote:
> On Fri, May 29, 2020 at 10:48:11AM -0700, Florian Fainelli wrote:
>> On 5/29/20 10:47 AM, Lukas Wunner wrote:
>>> On Thu, May 28, 2020 at 12:06:05PM -0700, Florian Fainelli wrote:
>>>> Make sure we clear the FIFOs, stop the block, disable the clock and
>>>> release the DMA channel.
>>>
>>> To what end? Why is this change necessary? Sorry but this seems like
>>> an awfully terse commit message.
>>
>> To ensure clocks are disabled and to save power in low power modes used
>> on 7211 for instance.
>
> Thanks for the explanation, that's an important tidbit. I wasn't even
> aware that this SPI controller is used on SoCs beyond the Raspberry Pi
> ones. Does the BCM7211 use shared interrupts for this controller?
> Does it have DMA DREQ attached?

For all practical purposes you can consider that 7211 is identical to a
2711, it does use shared interrupts for this controller and there is a
DRAM DREQ attached as well. Where they differ are on the display, video,
and memory controller MAC.
--
Florian