[PATCH AUTOSEL 5.12 03/43] spi: Fix spi device unregister flow

[PATCH AUTOSEL 5.12 03/43] spi: Fix spi device unregister flow

[Sasha Levin]

From: Saravana Kannan <saravanak@google.com>

[ Upstream commit c7299fea67696db5bd09d924d1f1080d894f92ef ]

When an SPI device is unregistered, the spi->controller->cleanup() is
called in the device's release callback. That's wrong for a couple of
reasons:

1. spi_dev_put() can be called before spi_add_device() is called. And
   it's spi_add_device() that calls spi_setup(). This will cause clean()
   to get called without the spi device ever being setup.

2. There's no guarantee that the controller's driver would be present by
   the time the spi device's release function gets called.

3. It also causes "sleeping in atomic context" stack dump[1] when device
   link deletion code does a put_device() on the spi device.

Fix these issues by simply moving the cleanup from the device release
callback to the actual spi_unregister_device() function.

[1] - https://lore.kernel.org/lkml/CAHp75Vc=FCGcUyS0v6fnxme2YJ+qD+Y-hQDQLa2JhWNON9VmsQ@mail.gmail.com/

Signed-off-by: Saravana Kannan <saravanak@google.com>
Link: https://lore.kernel.org/r/20210426235638.1285530-1-saravanak@google.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/spi/spi.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 8da4fe475b84..4278d9a63636 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -47,10 +47,6 @@ static void spidev_release(struct device *dev)
 {
 	struct spi_device	*spi = to_spi_device(dev);
 
-	/* spi controllers may cleanup for released devices */
-	if (spi->controller->cleanup)
-		spi->controller->cleanup(spi);
-
 	spi_controller_put(spi->controller);
 	kfree(spi->driver_override);
 	kfree(spi);
@@ -558,6 +554,12 @@ static int spi_dev_check(struct device *dev, void *data)
 	return 0;
 }
 
+static void spi_cleanup(struct spi_device *spi)
+{
+	if (spi->controller->cleanup)
+		spi->controller->cleanup(spi);
+}
+
 /**
  * spi_add_device - Add spi_device allocated with spi_alloc_device
  * @spi: spi_device to register
@@ -622,11 +624,13 @@ int spi_add_device(struct spi_device *spi)
 
 	/* Device may be bound to an active driver when this returns */
 	status = device_add(&spi->dev);
-	if (status < 0)
+	if (status < 0) {
 		dev_err(dev, "can't add %s, status %d\n",
 				dev_name(&spi->dev), status);
-	else
+		spi_cleanup(spi);
+	} else {
 		dev_dbg(dev, "registered child %s\n", dev_name(&spi->dev));
+	}
 
 done:
 	mutex_unlock(&spi_add_lock);
@@ -713,6 +717,8 @@ void spi_unregister_device(struct spi_device *spi)
 	if (!spi)
 		return;
 
+	spi_cleanup(spi);
+
 	if (spi->dev.of_node) {
 		of_node_clear_flag(spi->dev.of_node, OF_POPULATED);
 		of_node_put(spi->dev.of_node);
-- 
2.30.2

Re: [PATCH AUTOSEL 5.12 03/43] spi: Fix spi device unregister flow

[Lukas Wunner]

On Thu, Jun 03, 2021 at 01:06:53PM -0400, Sasha Levin wrote:
> From: Saravana Kannan <saravanak@google.com>
> 
> [ Upstream commit c7299fea67696db5bd09d924d1f1080d894f92ef ]

This commit shouldn't be backported to stable by itself, it requires
that the following fixups are applied on top of it:

* Upstream commit 27e7db56cf3d ("spi: Don't have controller clean up spi
  device before driver unbind")

* spi.git commit 2ec6f20b33eb ("spi: Cleanup on failure of initial setup")
  https://git.kernel.org/broonie/spi/c/2ec6f20b33eb

Note that the latter is queued for v5.13, but hasn't landed there yet.
So you probably need to back out c7299fea6769 from the stable queue and
wait for 2ec6f20b33eb to land in upstream.

Since you've applied c7299fea6769 to v5.12, v5.10, v5.4, v4.14 and v4.19
stable trees, the two fixups listed above need to be backported to all
of them.

Thanks,

Lukas

Re: [PATCH AUTOSEL 5.12 03/43] spi: Fix spi device unregister flow

[Sasha Levin]

On Sun, Jun 06, 2021 at 01:10:28PM +0200, Lukas Wunner wrote:
>On Thu, Jun 03, 2021 at 01:06:53PM -0400, Sasha Levin wrote:
>> From: Saravana Kannan <saravanak@google.com>
>>
>> [ Upstream commit c7299fea67696db5bd09d924d1f1080d894f92ef ]
>
>This commit shouldn't be backported to stable by itself, it requires
>that the following fixups are applied on top of it:
>
>* Upstream commit 27e7db56cf3d ("spi: Don't have controller clean up spi
>  device before driver unbind")
>
>* spi.git commit 2ec6f20b33eb ("spi: Cleanup on failure of initial setup")
>  https://git.kernel.org/broonie/spi/c/2ec6f20b33eb
>
>Note that the latter is queued for v5.13, but hasn't landed there yet.
>So you probably need to back out c7299fea6769 from the stable queue and
>wait for 2ec6f20b33eb to land in upstream.
>
>Since you've applied c7299fea6769 to v5.12, v5.10, v5.4, v4.14 and v4.19
>stable trees, the two fixups listed above need to be backported to all
>of them.

I took those two patches into 5.12-5.4, but as they needed a more
complex backport for 4.14 and 4.19, I've dropped c7299fea67 from those
trees.

-- 
Thanks,
Sasha

Re: [PATCH AUTOSEL 5.12 03/43] spi: Fix spi device unregister flow

[Saravana Kannan]

On Thu, Jun 10, 2021 at 10:55 AM Sasha Levin <sashal@kernel.org> wrote:
>
> On Sun, Jun 06, 2021 at 01:10:28PM +0200, Lukas Wunner wrote:
> >On Thu, Jun 03, 2021 at 01:06:53PM -0400, Sasha Levin wrote:
> >> From: Saravana Kannan <saravanak@google.com>
> >>
> >> [ Upstream commit c7299fea67696db5bd09d924d1f1080d894f92ef ]
> >
> >This commit shouldn't be backported to stable by itself, it requires
> >that the following fixups are applied on top of it:
> >
> >* Upstream commit 27e7db56cf3d ("spi: Don't have controller clean up spi
> >  device before driver unbind")
> >
> >* spi.git commit 2ec6f20b33eb ("spi: Cleanup on failure of initial setup")
> >  https://git.kernel.org/broonie/spi/c/2ec6f20b33eb
> >
> >Note that the latter is queued for v5.13, but hasn't landed there yet.
> >So you probably need to back out c7299fea6769 from the stable queue and
> >wait for 2ec6f20b33eb to land in upstream.
> >
> >Since you've applied c7299fea6769 to v5.12, v5.10, v5.4, v4.14 and v4.19
> >stable trees, the two fixups listed above need to be backported to all
> >of them.
>
> I took those two patches into 5.12-5.4, but as they needed a more
> complex backport for 4.14 and 4.19, I've dropped c7299fea67 from those
> trees.

Sounds good. Also, there was a subsequent "Fixes" for this patch and I
think another "Fixes" for the "Fixes". So, before picking this up,
maybe make sure those Fixes patches are pickable too?

-Saravana

Re: [PATCH AUTOSEL 5.12 03/43] spi: Fix spi device unregister flow

[Lukas Wunner]

On Thu, Jun 10, 2021 at 12:22:40PM -0700, Saravana Kannan wrote:
> On Thu, Jun 10, 2021 at 10:55 AM Sasha Levin <sashal@kernel.org> wrote:
> > On Sun, Jun 06, 2021 at 01:10:28PM +0200, Lukas Wunner wrote:
> > >On Thu, Jun 03, 2021 at 01:06:53PM -0400, Sasha Levin wrote:
> > >> From: Saravana Kannan <saravanak@google.com>
> > >>
> > >> [ Upstream commit c7299fea67696db5bd09d924d1f1080d894f92ef ]
> > >
> > >This commit shouldn't be backported to stable by itself, it requires
> > >that the following fixups are applied on top of it:
> > >
> > >* Upstream commit 27e7db56cf3d ("spi: Don't have controller clean up spi
> > >  device before driver unbind")
> > >
> > >* spi.git commit 2ec6f20b33eb ("spi: Cleanup on failure of initial setup")
> > >  https://git.kernel.org/broonie/spi/c/2ec6f20b33eb
> > >
> > >Note that the latter is queued for v5.13, but hasn't landed there yet.
> > >So you probably need to back out c7299fea6769 from the stable queue and
> > >wait for 2ec6f20b33eb to land in upstream.
> > >
> > >Since you've applied c7299fea6769 to v5.12, v5.10, v5.4, v4.14 and v4.19
> > >stable trees, the two fixups listed above need to be backported to all
> > >of them.
> >
> > I took those two patches into 5.12-5.4, but as they needed a more
> > complex backport for 4.14 and 4.19, I've dropped c7299fea67 from those
> > trees.
> 
> Sounds good. Also, there was a subsequent "Fixes" for this patch and I
> think another "Fixes" for the "Fixes". So, before picking this up,
> maybe make sure those Fixes patches are pickable too?

Aren't those the commits I've listed above?  Or did I miss any fixes?
I'm not aware of any others besides these two.

Thanks,

Lukas

Re: [PATCH AUTOSEL 5.12 03/43] spi: Fix spi device unregister flow

[Saravana Kannan]

On Thu, Jun 10, 2021 at 12:26 PM Lukas Wunner <lukas@wunner.de> wrote:
>
> On Thu, Jun 10, 2021 at 12:22:40PM -0700, Saravana Kannan wrote:
> > On Thu, Jun 10, 2021 at 10:55 AM Sasha Levin <sashal@kernel.org> wrote:
> > > On Sun, Jun 06, 2021 at 01:10:28PM +0200, Lukas Wunner wrote:
> > > >On Thu, Jun 03, 2021 at 01:06:53PM -0400, Sasha Levin wrote:
> > > >> From: Saravana Kannan <saravanak@google.com>
> > > >>
> > > >> [ Upstream commit c7299fea67696db5bd09d924d1f1080d894f92ef ]
> > > >
> > > >This commit shouldn't be backported to stable by itself, it requires
> > > >that the following fixups are applied on top of it:
> > > >
> > > >* Upstream commit 27e7db56cf3d ("spi: Don't have controller clean up spi
> > > >  device before driver unbind")
> > > >
> > > >* spi.git commit 2ec6f20b33eb ("spi: Cleanup on failure of initial setup")
> > > >  https://git.kernel.org/broonie/spi/c/2ec6f20b33eb
> > > >
> > > >Note that the latter is queued for v5.13, but hasn't landed there yet.
> > > >So you probably need to back out c7299fea6769 from the stable queue and
> > > >wait for 2ec6f20b33eb to land in upstream.
> > > >
> > > >Since you've applied c7299fea6769 to v5.12, v5.10, v5.4, v4.14 and v4.19
> > > >stable trees, the two fixups listed above need to be backported to all
> > > >of them.
> > >
> > > I took those two patches into 5.12-5.4, but as they needed a more
> > > complex backport for 4.14 and 4.19, I've dropped c7299fea67 from those
> > > trees.
> >
> > Sounds good. Also, there was a subsequent "Fixes" for this patch and I
> > think another "Fixes" for the "Fixes". So, before picking this up,
> > maybe make sure those Fixes patches are pickable too?
>
> Aren't those the commits I've listed above?  Or did I miss any fixes?
> I'm not aware of any others besides these two.
>

Ah, those are the ones. I didn't see them. My bad.

-Saravana

Re: [PATCH AUTOSEL 5.12 03/43] spi: Fix spi device unregister flow

[Lukas Wunner]

On Thu, Jun 10, 2021 at 12:30:18PM -0700, Saravana Kannan wrote:
> On Thu, Jun 10, 2021 at 12:26 PM Lukas Wunner <lukas@wunner.de> wrote:
> >
> > On Thu, Jun 10, 2021 at 12:22:40PM -0700, Saravana Kannan wrote:
> > > On Thu, Jun 10, 2021 at 10:55 AM Sasha Levin <sashal@kernel.org> wrote:
> > > > On Sun, Jun 06, 2021 at 01:10:28PM +0200, Lukas Wunner wrote:
> > > > >On Thu, Jun 03, 2021 at 01:06:53PM -0400, Sasha Levin wrote:
> > > > >> From: Saravana Kannan <saravanak@google.com>
> > > > >>
> > > > >> [ Upstream commit c7299fea67696db5bd09d924d1f1080d894f92ef ]
> > > > >
> > > > >This commit shouldn't be backported to stable by itself, it requires
> > > > >that the following fixups are applied on top of it:
> > > > >
> > > > >* Upstream commit 27e7db56cf3d ("spi: Don't have controller clean up spi
> > > > >  device before driver unbind")
> > > > >
> > > > >* spi.git commit 2ec6f20b33eb ("spi: Cleanup on failure of initial setup")
> > > > >  https://git.kernel.org/broonie/spi/c/2ec6f20b33eb
> > > > >
> > > > >Note that the latter is queued for v5.13, but hasn't landed there yet.
> > > > >So you probably need to back out c7299fea6769 from the stable queue and
> > > > >wait for 2ec6f20b33eb to land in upstream.
> > > > >
> > > > >Since you've applied c7299fea6769 to v5.12, v5.10, v5.4, v4.14 and v4.19
> > > > >stable trees, the two fixups listed above need to be backported to all
> > > > >of them.
> > > >
> > > > I took those two patches into 5.12-5.4, but as they needed a more
> > > > complex backport for 4.14 and 4.19, I've dropped c7299fea67 from those
> > > > trees.
> > >
> > > Sounds good. Also, there was a subsequent "Fixes" for this patch and I
> > > think another "Fixes" for the "Fixes". So, before picking this up,
> > > maybe make sure those Fixes patches are pickable too?
> >
> > Aren't those the commits I've listed above?  Or did I miss any fixes?
> > I'm not aware of any others besides these two.
> 
> Ah, those are the ones. I didn't see them. My bad.

All good.  Sasha says that backporting the fixes is a little more
involved in the case of 4.14 and 4.19.  Do you consider the issue
critical enough that it should be addressed in those stable kernels
as well?  (I assume the issue concerns Android devices, not sure
in how far those are using 4.14 and 4.19?)

Thanks,

Lukas

Re: [PATCH AUTOSEL 5.12 03/43] spi: Fix spi device unregister flow

[Saravana Kannan]

On Thu, Jun 10, 2021 at 3:29 PM Lukas Wunner <lukas@wunner.de> wrote:
>
> On Thu, Jun 10, 2021 at 12:30:18PM -0700, Saravana Kannan wrote:
> > On Thu, Jun 10, 2021 at 12:26 PM Lukas Wunner <lukas@wunner.de> wrote:
> > >
> > > On Thu, Jun 10, 2021 at 12:22:40PM -0700, Saravana Kannan wrote:
> > > > On Thu, Jun 10, 2021 at 10:55 AM Sasha Levin <sashal@kernel.org> wrote:
> > > > > On Sun, Jun 06, 2021 at 01:10:28PM +0200, Lukas Wunner wrote:
> > > > > >On Thu, Jun 03, 2021 at 01:06:53PM -0400, Sasha Levin wrote:
> > > > > >> From: Saravana Kannan <saravanak@google.com>
> > > > > >>
> > > > > >> [ Upstream commit c7299fea67696db5bd09d924d1f1080d894f92ef ]
> > > > > >
> > > > > >This commit shouldn't be backported to stable by itself, it requires
> > > > > >that the following fixups are applied on top of it:
> > > > > >
> > > > > >* Upstream commit 27e7db56cf3d ("spi: Don't have controller clean up spi
> > > > > >  device before driver unbind")
> > > > > >
> > > > > >* spi.git commit 2ec6f20b33eb ("spi: Cleanup on failure of initial setup")
> > > > > >  https://git.kernel.org/broonie/spi/c/2ec6f20b33eb
> > > > > >
> > > > > >Note that the latter is queued for v5.13, but hasn't landed there yet.
> > > > > >So you probably need to back out c7299fea6769 from the stable queue and
> > > > > >wait for 2ec6f20b33eb to land in upstream.
> > > > > >
> > > > > >Since you've applied c7299fea6769 to v5.12, v5.10, v5.4, v4.14 and v4.19
> > > > > >stable trees, the two fixups listed above need to be backported to all
> > > > > >of them.
> > > > >
> > > > > I took those two patches into 5.12-5.4, but as they needed a more
> > > > > complex backport for 4.14 and 4.19, I've dropped c7299fea67 from those
> > > > > trees.
> > > >
> > > > Sounds good. Also, there was a subsequent "Fixes" for this patch and I
> > > > think another "Fixes" for the "Fixes". So, before picking this up,
> > > > maybe make sure those Fixes patches are pickable too?
> > >
> > > Aren't those the commits I've listed above?  Or did I miss any fixes?
> > > I'm not aware of any others besides these two.
> >
> > Ah, those are the ones. I didn't see them. My bad.
>
> All good.  Sasha says that backporting the fixes is a little more
> involved in the case of 4.14 and 4.19.  Do you consider the issue
> critical enough that it should be addressed in those stable kernels
> as well?  (I assume the issue concerns Android devices, not sure
> in how far those are using 4.14 and 4.19?)

It isn't android specific, but I don't think it's critical for those kernels.

-Saravana

[PATCH AUTOSEL 5.12 04/43] spi: spi-zynq-qspi: Fix stack violation bug

[Sasha Levin]

From: Karen Dombroski <karen.dombroski@marsbioimaging.com>

[ Upstream commit 6d5ff8e632a4f2389c331e5554cd1c2a9a28c7aa ]

When the number of bytes for the op is greater than one, the read could
run off the end of the function stack and cause a crash.

This patch restores the behaviour of safely reading out of the original
opcode location.

Signed-off-by: Karen Dombroski <karen.dombroski@marsbioimaging.com>
Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
Link: https://lore.kernel.org/r/20210429053802.17650-3-amit.kumar-mahapatra@xilinx.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/spi/spi-zynq-qspi.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-zynq-qspi.c b/drivers/spi/spi-zynq-qspi.c
index 5d8a5ee62fa2..2765289028fa 100644
--- a/drivers/spi/spi-zynq-qspi.c
+++ b/drivers/spi/spi-zynq-qspi.c
@@ -528,18 +528,17 @@ static int zynq_qspi_exec_mem_op(struct spi_mem *mem,
 	struct zynq_qspi *xqspi = spi_controller_get_devdata(mem->spi->master);
 	int err = 0, i;
 	u8 *tmpbuf;
-	u8 opcode = op->cmd.opcode;
 
 	dev_dbg(xqspi->dev, "cmd:%#x mode:%d.%d.%d.%d\n",
-		opcode, op->cmd.buswidth, op->addr.buswidth,
+		op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth,
 		op->dummy.buswidth, op->data.buswidth);
 
 	zynq_qspi_chipselect(mem->spi, true);
 	zynq_qspi_config_op(xqspi, mem->spi);
 
-	if (op->cmd.nbytes) {
+	if (op->cmd.opcode) {
 		reinit_completion(&xqspi->data_completion);
-		xqspi->txbuf = &opcode;
+		xqspi->txbuf = (u8 *)&op->cmd.opcode;
 		xqspi->rxbuf = NULL;
 		xqspi->tx_bytes = op->cmd.nbytes;
 		xqspi->rx_bytes = op->cmd.nbytes;
-- 
2.30.2

[PATCH AUTOSEL 5.12 16/43] spi: sprd: Add missing MODULE_DEVICE_TABLE

[Sasha Levin]

From: Chunyan Zhang <chunyan.zhang@unisoc.com>

[ Upstream commit 7907cad7d07e0055789ec0c534452f19dfe1fc80 ]

MODULE_DEVICE_TABLE is used to extract the device information out of the
driver and builds a table when being compiled. If using this macro,
kernel can find the driver if available when the device is plugged in,
and then loads that driver and initializes the device.

Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
Link: https://lore.kernel.org/r/20210512093534.243040-1-zhang.lyra@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/spi/spi-sprd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-sprd.c b/drivers/spi/spi-sprd.c
index b41a75749b49..28e70db9bbba 100644
--- a/drivers/spi/spi-sprd.c
+++ b/drivers/spi/spi-sprd.c
@@ -1068,6 +1068,7 @@ static const struct of_device_id sprd_spi_of_match[] = {
 	{ .compatible = "sprd,sc9860-spi", },
 	{ /* sentinel */ }
 };
+MODULE_DEVICE_TABLE(of, sprd_spi_of_match);
 
 static struct platform_driver sprd_spi_driver = {
 	.driver = {
-- 
2.30.2