[PATCH 0/5] Add support for MSCC Ocelot SPI

[PATCH 0/5] Add support for MSCC Ocelot SPI

[Alexandre Belloni]

Hello,

The MSCC MIPS SoC line uses a designware IP for the SPI controller but
still requires some special handling to give control of the SPI
interface to the IP and also has a specific handling for the chip
select.

Patches 1 to 3 should go through the SPI tree while 4 and 5 should
probably got throught the MIPS tree once patch 3 has been reviewed by
the DT maintainers.

Alexandre Belloni (5):
  spi: dw: fix possible race condition
  spi: dw: allow providing own set_cs callback
  spi: dw-mmio: add MSCC Ocelot support
  mips: dts: mscc: Add spi on Ocelot
  mips: dts: mscc: enable spi and NOR flash support on ocelot PCB123

 .../bindings/spi/snps,dw-apb-ssi.txt          |  5 +-
 arch/mips/boot/dts/mscc/ocelot.dtsi           | 11 +++
 arch/mips/boot/dts/mscc/ocelot_pcb123.dts     | 10 +++
 drivers/spi/spi-dw-mmio.c                     | 86 +++++++++++++++++++
 drivers/spi/spi-dw.c                          |  6 +-
 drivers/spi/spi-dw.h                          |  1 +
 6 files changed, 116 insertions(+), 3 deletions(-)

-- 
2.18.0

[PATCH 1/5] spi: dw: fix possible race condition

[Alexandre Belloni]

It is possible to get an interrupt as soon as it is requested.  dw_spi_irq
does spi_controller_get_devdata(master) and expects it to be different than
NULL. However, spi_controller_set_devdata() is called after request_irq(),
resulting in the following crash:

CPU 0 Unable to handle kernel paging request at virtual address 00000030, epc == 8058e09c, ra == 8018ff90
[...]
Call Trace:
[<8058e09c>] dw_spi_irq+0x8/0x64
[<8018ff90>] __handle_irq_event_percpu+0x70/0x1d4
[<80190128>] handle_irq_event_percpu+0x34/0x8c
[<801901c4>] handle_irq_event+0x44/0x80
[<801951a8>] handle_level_irq+0xdc/0x194
[<8018f580>] generic_handle_irq+0x38/0x50
[<804c6924>] ocelot_irq_handler+0x104/0x1c0
[<8018f580>] generic_handle_irq+0x38/0x50
[<8075c1d8>] do_IRQ+0x18/0x24
[<804c4714>] plat_irq_dispatch+0xa4/0x150
[<80106ba8>] except_vec_vi_end+0xb8/0xc4
[<8075ba5c>] _raw_spin_unlock_irqrestore+0x14/0x20
[<801926c8>] __setup_irq+0x53c/0x8e0
[<80192e28>] request_threaded_irq+0xf4/0x1e8
[<8058ed18>] dw_spi_add_host+0x264/0x2c4
[<8058f2ec>] dw_spi_mmio_probe+0x258/0x27c
[<8051f4a4>] platform_drv_probe+0x58/0xbc
[<8051daa8>] driver_probe_device+0x308/0x40c
[<8051dc9c>] __driver_attach+0xf0/0xf8
[<8051b698>] bus_for_each_dev+0x78/0xcc
[<8051c2c0>] bus_add_driver+0x1b4/0x228
[<8051e840>] driver_register+0x84/0x154
[<801001d8>] do_one_initcall+0x54/0x1ac
[<80880e90>] kernel_init_freeable+0x1ec/0x2ac
[<80755310>] kernel_init+0x14/0x110
[<80106698>] ret_from_kernel_thread+0x14/0x1c
Code: 00000000  8ca40050  8c820008 <8c420030> 0000000f  3042003f  10400007  00000000  8ca20230

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/spi/spi-dw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index f693bfe95ab9..a087464efdd7 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -485,6 +485,8 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	dws->dma_inited = 0;
 	dws->dma_addr = (dma_addr_t)(dws->paddr + DW_SPI_DR);
 
+	spi_controller_set_devdata(master, dws);
+
 	ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dev_name(dev),
 			  master);
 	if (ret < 0) {
@@ -518,7 +520,6 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 		}
 	}
 
-	spi_controller_set_devdata(master, dws);
 	ret = devm_spi_register_controller(dev, master);
 	if (ret) {
 		dev_err(&master->dev, "problem registering spi master\n");
-- 
2.18.0

[PATCH 2/5] spi: dw: allow providing own set_cs callback

[Alexandre Belloni]

Allow platform specific drivers to provide their own set_cs callback when
the IP integration requires it.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/spi/spi-dw.c | 3 +++
 drivers/spi/spi-dw.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index a087464efdd7..f76e31faf694 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -507,6 +507,9 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	master->dev.of_node = dev->of_node;
 	master->flags = SPI_MASTER_GPIO_SS;
 
+	if (dws->set_cs)
+		master->set_cs = dws->set_cs;
+
 	/* Basic HW init */
 	spi_hw_init(dev, dws);
 
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 2cde2473b3e9..446013022849 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -112,6 +112,7 @@ struct dw_spi {
 	u32			reg_io_width;	/* DR I/O width in bytes */
 	u16			bus_num;
 	u16			num_cs;		/* supported slave numbers */
+	void (*set_cs)(struct spi_device *spi, bool enable);
 
 	/* Current message transfer state info */
 	size_t			len;
-- 
2.18.0

[PATCH 3/5] spi: dw-mmio: add MSCC Ocelot support

[Alexandre Belloni]

Because the SPI controller deasserts the chip select when the TX fifo is
empty (which may happen in the middle of a transfer), the CS should be
handled by linux. Unfortunately, some or all of the first four chip
selects are not muxable as GPIOs, depending on the SoC.

There is a way to bitbang those pins by using the SPI boot controller so
use it to set the chip selects.

At init time, it is also necessary to give control of the SPI interface to
the Designware IP.

Cc: Rob Herring <robh+dt@kernel.org>
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 .../bindings/spi/snps,dw-apb-ssi.txt          |  5 +-
 drivers/spi/spi-dw-mmio.c                     | 86 +++++++++++++++++++
 2 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
index 204b311e0400..d97b9fc4c1cb 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.txt
@@ -1,8 +1,9 @@
 Synopsys DesignWare AMBA 2.0 Synchronous Serial Interface.
 
 Required properties:
-- compatible : "snps,dw-apb-ssi"
-- reg : The register base for the controller.
+- compatible : "snps,dw-apb-ssi" or "mscc,<soc>-spi"
+- reg : The register base for the controller. For "mscc,<soc>-spi", a second
+  register set is required (named ICPU_CFG:SPI_MST)
 - interrupts : One interrupt, used by the controller.
 - #address-cells : <1>, as required by generic SPI binding.
 - #size-cells : <0>, also as required by generic SPI binding.
diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index d25cc4037e23..324b8679b03b 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -15,11 +15,13 @@
 #include <linux/slab.h>
 #include <linux/spi/spi.h>
 #include <linux/scatterlist.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_gpio.h>
 #include <linux/of_platform.h>
 #include <linux/property.h>
+#include <linux/regmap.h>
 
 #include "spi-dw.h"
 
@@ -28,8 +30,87 @@
 struct dw_spi_mmio {
 	struct dw_spi  dws;
 	struct clk     *clk;
+	void           *priv;
 };
 
+#define MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL	0x24
+#define OCELOT_IF_SI_OWNER_MASK			GENMASK(5, 4)
+#define OCELOT_IF_SI_OWNER_OFFSET		4
+#define MSCC_IF_SI_OWNER_SISL			0
+#define MSCC_IF_SI_OWNER_SIBM			1
+#define MSCC_IF_SI_OWNER_SIMC			2
+
+#define MSCC_SPI_MST_SW_MODE			0x14
+#define MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE	BIT(13)
+#define MSCC_SPI_MST_SW_MODE_SW_SPI_CS(x)	(x << 5)
+struct dw_spi_mscc {
+	struct regmap       *syscon;
+	void __iomem        *spi_mst;
+};
+
+/*
+ * The SPI master controller automatically deasserts
+ * chip select when the tx fifo is empty. The chip selects then needs to be
+ * either driven as GPIOs or, for the first 4 using the the SPI boot controller
+ * registers. the final chip select is an OR gate between the SPI master
+ * controller and the SPI boot controller.
+ */
+static void dw_spi_mscc_set_cs(struct spi_device *spi, bool enable)
+{
+	struct dw_spi *dws = spi_master_get_devdata(spi->master);
+	struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio, dws);
+	struct dw_spi_mscc *dwsmscc = dwsmmio->priv;
+	u32 cs = spi->chip_select;
+
+	if (cs < 4) {
+		u32 sw_mode = MSCC_SPI_MST_SW_MODE_SW_PIN_CTRL_MODE;
+
+		if (!enable)
+			sw_mode |= MSCC_SPI_MST_SW_MODE_SW_SPI_CS(BIT(cs));
+
+		writel(sw_mode, dwsmscc->spi_mst + MSCC_SPI_MST_SW_MODE);
+	}
+
+	if (!enable)
+		dw_writel(dws, DW_SPI_SER, BIT(cs));
+}
+
+static int dw_spi_mscc_init(struct platform_device *pdev,
+			    struct dw_spi_mmio *dwsmmio)
+{
+	struct dw_spi_mscc *dwsmscc;
+	struct resource *res;
+
+	dwsmscc = devm_kzalloc(&pdev->dev, sizeof(struct dw_spi_mscc),
+			       GFP_KERNEL);
+	if (!dwsmscc)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	dwsmscc->spi_mst = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(dwsmscc->spi_mst)) {
+		dev_err(&pdev->dev, "SPI_MST region map failed\n");
+		return PTR_ERR(dwsmscc->spi_mst);
+	}
+
+	dwsmscc->syscon = syscon_regmap_lookup_by_compatible("mscc,ocelot-cpu-syscon");
+	if (IS_ERR(dwsmscc->syscon))
+		return PTR_ERR(dwsmscc->syscon);
+
+	/* Deassert all CS */
+	writel(0, dwsmscc->spi_mst + MSCC_SPI_MST_SW_MODE);
+
+	/* Select the owner of the SI interface */
+	regmap_update_bits(dwsmscc->syscon, MSCC_CPU_SYSTEM_CTRL_GENERAL_CTRL,
+			   OCELOT_IF_SI_OWNER_MASK,
+			   MSCC_IF_SI_OWNER_SIMC << OCELOT_IF_SI_OWNER_OFFSET);
+
+	dwsmmio->dws.set_cs = dw_spi_mscc_set_cs;
+	dwsmmio->priv = dwsmscc;
+
+	return 0;
+}
+
 static int dw_spi_mmio_probe(struct platform_device *pdev)
 {
 	struct dw_spi_mmio *dwsmmio;
@@ -99,6 +180,10 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 		}
 	}
 
+	ret = dw_spi_mscc_init(pdev, dwsmmio);
+	if (ret)
+		goto out;
+
 	ret = dw_spi_add_host(&pdev->dev, dws);
 	if (ret)
 		goto out;
@@ -123,6 +208,7 @@ static int dw_spi_mmio_remove(struct platform_device *pdev)
 
 static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "snps,dw-apb-ssi", },
+	{ .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_init},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
-- 
2.18.0

[PATCH 4/5] mips: dts: mscc: Add spi on Ocelot

[Alexandre Belloni]

Add support for the SPI controller

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/mips/boot/dts/mscc/ocelot.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/mips/boot/dts/mscc/ocelot.dtsi b/arch/mips/boot/dts/mscc/ocelot.dtsi
index 4f33dbc67348..f7616a476247 100644
--- a/arch/mips/boot/dts/mscc/ocelot.dtsi
+++ b/arch/mips/boot/dts/mscc/ocelot.dtsi
@@ -91,6 +91,17 @@
 			status = "disabled";
 		};
 
+		spi: spi@101000 {
+			compatible = "mscc,ocelot-spi", "snps,dw-apb-ssi";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x101000 0x100>, <0x3c 0x18>;
+			interrupts = <9>;
+			clocks = <&ahb_clk>;
+
+			status = "disabled";
+		};
+
 		switch@1010000 {
 			compatible = "mscc,vsc7514-switch";
 			reg = <0x1010000 0x10000>,
-- 
2.18.0

[PATCH 5/5] mips: dts: mscc: enable spi and NOR flash support on ocelot PCB123

[Alexandre Belloni]

Ocelot PCB123 has a SPI NOR connected on its SPI bus.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/mips/boot/dts/mscc/ocelot_pcb123.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
index 4ccd65379059..2266027759f9 100644
--- a/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
+++ b/arch/mips/boot/dts/mscc/ocelot_pcb123.dts
@@ -26,6 +26,16 @@
 	status = "okay";
 };
 
+&spi {
+	status = "okay";
+
+	flash@0 {
+		compatible = "macronix,mx25l25635f", "jedec,spi-nor";
+		spi-max-frequency = <20000000>;
+		reg = <0>;
+	};
+};
+
 &mdio0 {
 	status = "okay";
 };
-- 
2.18.0

Re: [PATCH 1/5] spi: dw: fix possible race condition

[Andy Shevchenko]

On Tue, Jul 17, 2018 at 5:23 PM, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> It is possible to get an interrupt as soon as it is requested.  dw_spi_irq
> does spi_controller_get_devdata(master) and expects it to be different than
> NULL. However, spi_controller_set_devdata() is called after request_irq(),
> resulting in the following crash:
>
> CPU 0 Unable to handle kernel paging request at virtual address 00000030, epc == 8058e09c, ra == 8018ff90
> [...]
> Call Trace:
> [<8058e09c>] dw_spi_irq+0x8/0x64
> [<8018ff90>] __handle_irq_event_percpu+0x70/0x1d4
> [<80190128>] handle_irq_event_percpu+0x34/0x8c
> [<801901c4>] handle_irq_event+0x44/0x80
> [<801951a8>] handle_level_irq+0xdc/0x194
> [<8018f580>] generic_handle_irq+0x38/0x50
> [<804c6924>] ocelot_irq_handler+0x104/0x1c0
> [<8018f580>] generic_handle_irq+0x38/0x50
> [<8075c1d8>] do_IRQ+0x18/0x24
> [<804c4714>] plat_irq_dispatch+0xa4/0x150
> [<80106ba8>] except_vec_vi_end+0xb8/0xc4
> [<8075ba5c>] _raw_spin_unlock_irqrestore+0x14/0x20
> [<801926c8>] __setup_irq+0x53c/0x8e0
> [<80192e28>] request_threaded_irq+0xf4/0x1e8
> [<8058ed18>] dw_spi_add_host+0x264/0x2c4
> [<8058f2ec>] dw_spi_mmio_probe+0x258/0x27c
> [<8051f4a4>] platform_drv_probe+0x58/0xbc
> [<8051daa8>] driver_probe_device+0x308/0x40c
> [<8051dc9c>] __driver_attach+0xf0/0xf8
> [<8051b698>] bus_for_each_dev+0x78/0xcc
> [<8051c2c0>] bus_add_driver+0x1b4/0x228
> [<8051e840>] driver_register+0x84/0x154
> [<801001d8>] do_one_initcall+0x54/0x1ac
> [<80880e90>] kernel_init_freeable+0x1ec/0x2ac
> [<80755310>] kernel_init+0x14/0x110
> [<80106698>] ret_from_kernel_thread+0x14/0x1c
> Code: 00000000  8ca40050  8c820008 <8c420030> 0000000f  3042003f  10400007  00000000  8ca20230
>

Good catch!

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  drivers/spi/spi-dw.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
> index f693bfe95ab9..a087464efdd7 100644
> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -485,6 +485,8 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>         dws->dma_inited = 0;
>         dws->dma_addr = (dma_addr_t)(dws->paddr + DW_SPI_DR);
>
> +       spi_controller_set_devdata(master, dws);
> +
>         ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dev_name(dev),
>                           master);
>         if (ret < 0) {
> @@ -518,7 +520,6 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
>                 }
>         }
>
> -       spi_controller_set_devdata(master, dws);
>         ret = devm_spi_register_controller(dev, master);
>         if (ret) {
>                 dev_err(&master->dev, "problem registering spi master\n");
> --
> 2.18.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-spi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] spi: dw: fix possible race condition

[Andy Shevchenko]

On Wed, Jul 18, 2018 at 12:30 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Jul 17, 2018 at 5:23 PM, Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:

> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
>> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Shouldn't it have a Fixes tag?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/5] spi: dw-mmio: add MSCC Ocelot support

[Andy Shevchenko]

On Tue, Jul 17, 2018 at 5:23 PM, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> Because the SPI controller deasserts the chip select when the TX fifo is
> empty (which may happen in the middle of a transfer), the CS should be
> handled by linux. Unfortunately, some or all of the first four chip
> selects are not muxable as GPIOs, depending on the SoC.
>
> There is a way to bitbang those pins by using the SPI boot controller so
> use it to set the chip selects.
>
> At init time, it is also necessary to give control of the SPI interface to
> the Designware IP.

> +       ret = dw_spi_mscc_init(pdev, dwsmmio);
> +       if (ret)
> +               goto out;

> +       { .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_init},

Looks like you were thinking about something like

init_func = device_get_match_data(...);
if (init_func) {
 ret = init_func();
 if (ret)
   return ret;
}

?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/5] spi: dw-mmio: add MSCC Ocelot support

[Alexandre Belloni]

On 18/07/2018 00:34:37+0300, Andy Shevchenko wrote:
> On Tue, Jul 17, 2018 at 5:23 PM, Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> > Because the SPI controller deasserts the chip select when the TX fifo is
> > empty (which may happen in the middle of a transfer), the CS should be
> > handled by linux. Unfortunately, some or all of the first four chip
> > selects are not muxable as GPIOs, depending on the SoC.
> >
> > There is a way to bitbang those pins by using the SPI boot controller so
> > use it to set the chip selects.
> >
> > At init time, it is also necessary to give control of the SPI interface to
> > the Designware IP.
> 
> > +       ret = dw_spi_mscc_init(pdev, dwsmmio);
> > +       if (ret)
> > +               goto out;
> 
> > +       { .compatible = "mscc,ocelot-spi", .data = dw_spi_mscc_init},
> 
> Looks like you were thinking about something like
> 
> init_func = device_get_match_data(...);
> if (init_func) {
>  ret = init_func();
>  if (ret)
>    return ret;
> }
> 
> ?
> 

Ah sure, I forgot to do that after testing.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/5] spi: dw: fix possible race condition

[Alexandre Belloni]

On 18/07/2018 00:30:49+0300, Andy Shevchenko wrote:
> On Wed, Jul 18, 2018 at 12:30 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Tue, Jul 17, 2018 at 5:23 PM, Alexandre Belloni
> > <alexandre.belloni@bootlin.com> wrote:
> 
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> >
> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> 
> Shouldn't it have a Fixes tag?
> 

Well, I'm not sure how far this can be backported. It also seems nobody
ever hit that while our hardware will hit it at every boot.

I'll try to find out.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/5] spi: dw: fix possible race condition

[Andy Shevchenko]

On Wed, Jul 18, 2018 at 12:42 AM, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 18/07/2018 00:30:49+0300, Andy Shevchenko wrote:
>> On Wed, Jul 18, 2018 at 12:30 AM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>> > On Tue, Jul 17, 2018 at 5:23 PM, Alexandre Belloni
>> > <alexandre.belloni@bootlin.com> wrote:
>>
>> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> >
>> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>>
>> Shouldn't it have a Fixes tag?
>>
>
> Well, I'm not sure how far this can be backported. It also seems nobody
> ever hit that while our hardware will hit it at every boot.
>
> I'll try to find out.

No-one enabled CONFIG_DEBUG_SHIRQ?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] spi: dw: fix possible race condition

[Alexandre Belloni]

On 18/07/2018 00:54:21+0300, Andy Shevchenko wrote:
> On Wed, Jul 18, 2018 at 12:42 AM, Alexandre Belloni
> <alexandre.belloni@bootlin.com> wrote:
> > On 18/07/2018 00:30:49+0300, Andy Shevchenko wrote:
> >> On Wed, Jul 18, 2018 at 12:30 AM, Andy Shevchenko
> >> <andy.shevchenko@gmail.com> wrote:
> >> > On Tue, Jul 17, 2018 at 5:23 PM, Alexandre Belloni
> >> > <alexandre.belloni@bootlin.com> wrote:
> >>
> >> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> >> >
> >> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >>
> >> Shouldn't it have a Fixes tag?
> >>
> >
> > Well, I'm not sure how far this can be backported. It also seems nobody
> > ever hit that while our hardware will hit it at every boot.
> >
> > I'll try to find out.
> 
> No-one enabled CONFIG_DEBUG_SHIRQ?
> 

Nope, this is a real HW IRQ. I meant find out up to when it can be
sanely backported.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 1/5] spi: dw: fix possible race condition

[Andy Shevchenko]

On Wed, Jul 18, 2018 at 1:13 AM, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
> On 18/07/2018 00:54:21+0300, Andy Shevchenko wrote:
>> On Wed, Jul 18, 2018 at 12:42 AM, Alexandre Belloni
>> <alexandre.belloni@bootlin.com> wrote:
>> > On 18/07/2018 00:30:49+0300, Andy Shevchenko wrote:

>> > Well, I'm not sure how far this can be backported. It also seems nobody
>> > ever hit that while our hardware will hit it at every boot.

>> No-one enabled CONFIG_DEBUG_SHIRQ?

> Nope, this is a real HW IRQ. I meant find out up to when it can be
> sanely backported.

I meant that before your case no-one tested with that option enabled
which will behave like you describe (IRQ gets fired as soon as being
registered).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/5] spi: dw: fix possible race condition

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]

On Tue, Jul 17, 2018 at 04:23:10PM +0200, Alexandre Belloni wrote:
> It is possible to get an interrupt as soon as it is requested.  dw_spi_irq
> does spi_controller_get_devdata(master) and expects it to be different than
> NULL. However, spi_controller_set_devdata() is called after request_irq(),
> resulting in the following crash:
> 
> CPU 0 Unable to handle kernel paging request at virtual address 00000030, epc == 8058e09c, ra == 8018ff90
> [...]
> Call Trace:
> [<8058e09c>] dw_spi_irq+0x8/0x64
> [<8018ff90>] __handle_irq_event_percpu+0x70/0x1d4
> [<80190128>] handle_irq_event_percpu+0x34/0x8c
> [<801901c4>] handle_irq_event+0x44/0x80
> [<801951a8>] handle_level_irq+0xdc/0x194
> [<8018f580>] generic_handle_irq+0x38/0x50
> [<804c6924>] ocelot_irq_handler+0x104/0x1c0
> [<8018f580>] generic_handle_irq+0x38/0x50

Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative then it's
usually better to pull out the relevant sections.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/5] spi: dw: allow providing own set_cs callback

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

On Tue, Jul 17, 2018 at 04:23:11PM +0200, Alexandre Belloni wrote:
> Allow platform specific drivers to provide their own set_cs callback when
> the IP integration requires it.

The following changes since commit ce397d215ccd07b8ae3f71db689aedb85d56ab40:

  Linux 4.18-rc1 (2018-06-17 08:04:49 +0900)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git tags/spi-dw-set-cs

for you to fetch changes up to 62dbbae483b6452e688b8e26c4d024d484117697:

  spi: dw: allow providing own set_cs callback (2018-07-18 13:22:37 +0100)

----------------------------------------------------------------
spi: dw: Allow custom set_cs_callback

Allow platform specific drivers to provide their own set_cs callback when
the IP integration requires it.

----------------------------------------------------------------
Alexandre Belloni (1):
      spi: dw: allow providing own set_cs callback

 drivers/spi/spi-dw.c | 3 +++
 drivers/spi/spi-dw.h | 1 +
 2 files changed, 4 insertions(+)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Applied "spi: dw: allow providing own set_cs callback" to the spi tree

[Mark Brown]

The patch

   spi: dw: allow providing own set_cs callback

has been applied to the spi tree at

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

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

From 62dbbae483b6452e688b8e26c4d024d484117697 Mon Sep 17 00:00:00 2001
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
Date: Tue, 17 Jul 2018 16:23:11 +0200
Subject: [PATCH] spi: dw: allow providing own set_cs callback

Allow platform specific drivers to provide their own set_cs callback when
the IP integration requires it.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-dw.c | 3 +++
 drivers/spi/spi-dw.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index f693bfe95ab9..0d8ccb8be5ec 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -505,6 +505,9 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	master->dev.of_node = dev->of_node;
 	master->flags = SPI_MASTER_GPIO_SS;
 
+	if (dws->set_cs)
+		master->set_cs = dws->set_cs;
+
 	/* Basic HW init */
 	spi_hw_init(dev, dws);
 
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
index 2cde2473b3e9..446013022849 100644
--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
@@ -112,6 +112,7 @@ struct dw_spi {
 	u32			reg_io_width;	/* DR I/O width in bytes */
 	u16			bus_num;
 	u16			num_cs;		/* supported slave numbers */
+	void (*set_cs)(struct spi_device *spi, bool enable);
 
 	/* Current message transfer state info */
 	size_t			len;
-- 
2.18.0

Applied "spi: dw: fix possible race condition" to the spi tree

[Mark Brown]

The patch

   spi: dw: fix possible race condition

has been applied to the spi tree at

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

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

From 66b19d762378785d1568b5650935205edfeb0503 Mon Sep 17 00:00:00 2001
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
Date: Tue, 17 Jul 2018 16:23:10 +0200
Subject: [PATCH] spi: dw: fix possible race condition

It is possible to get an interrupt as soon as it is requested.  dw_spi_irq
does spi_controller_get_devdata(master) and expects it to be different than
NULL. However, spi_controller_set_devdata() is called after request_irq(),
resulting in the following crash:

CPU 0 Unable to handle kernel paging request at virtual address 00000030, epc == 8058e09c, ra == 8018ff90
[...]
Call Trace:
[<8058e09c>] dw_spi_irq+0x8/0x64
[<8018ff90>] __handle_irq_event_percpu+0x70/0x1d4
[<80190128>] handle_irq_event_percpu+0x34/0x8c
[<801901c4>] handle_irq_event+0x44/0x80
[<801951a8>] handle_level_irq+0xdc/0x194
[<8018f580>] generic_handle_irq+0x38/0x50
[<804c6924>] ocelot_irq_handler+0x104/0x1c0

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-dw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index f693bfe95ab9..a087464efdd7 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -485,6 +485,8 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 	dws->dma_inited = 0;
 	dws->dma_addr = (dma_addr_t)(dws->paddr + DW_SPI_DR);
 
+	spi_controller_set_devdata(master, dws);
+
 	ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dev_name(dev),
 			  master);
 	if (ret < 0) {
@@ -518,7 +520,6 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 		}
 	}
 
-	spi_controller_set_devdata(master, dws);
 	ret = devm_spi_register_controller(dev, master);
 	if (ret) {
 		dev_err(&master->dev, "problem registering spi master\n");
-- 
2.18.0