Re: [PATCH] spi: dw: Bail out early on unsupported target mode

[Damien Le Moal <dlemoal@xxxxxxxxxx>]

On 5/8/24 7:20 PM, Geert Uytterhoeven wrote:
> Currently, the DesignWare SPI controller driver supports only host mode.
> However, spi2 on the Kendryte K210 SoC supports only target mode,
> triggering an error message on e.g. SiPEED MAiXBiT since commit
> 98d75b9ef282f6b9 ("spi: dw: Drop default number of CS setting"):
> 
>     dw_spi_mmio 50240000.spi: error -22: problem registering spi host
>     dw_spi_mmio 50240000.spi: probe with driver dw_spi_mmio failed with error -22
> 
> As spi2 rightfully has no "num-cs" property, num_chipselect is now zero,
> causing spi_alloc_host() to fail to register the controller.  Before,
> the driver silently registered an SPI host controller with 4 chip
> selects.
> 
> Reject target mode early on and warn the user, getting rid of the
> error message.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

Looks good to me.

Reviewed-by: Damien Le Moal <dlemoal@xxxxxxxxxx>

> ---
> Commit 98d75b9ef282f6b9 is in spi/for-next (next-20240508 and later).
> 
> Overview of all SPI controllers on K210:
> 
>     spi0 (52000000.spi):
> 	num_cs = <4> in k210.dtsi
> 	num_cs = <1> override in sipeed_maix*.dts and canaan_kd233.dts
> 	DW_SPI_SER says 0xf (but value is unused)
> 
>     spi1 (53000000.spi):
> 	num_cs = <4> in k210.dtsi
> 	num_cs = <1> override in sipeed_maix*.dts and canaan_kd233.dts
> 	DW_SPI_SER says 0xf (but value is unused)
> 
>     spi2 (53000000.spi):
> 	spi-slave
> 	no num_cs property
> 	DW_SPI_SER says 0
> 	dw_spi_mmio 50240000.spi: error -22: problem registering spi host
> 	dw_spi_mmio 50240000.spi: probe with driver dw_spi_mmio failed with error -22
> 
>     spi3 (54000000.spi):
> 	num_cs = <4> in k210.dtsi
> 	DW_SPI_SER says 0x1 (but value is unused)
> 	Used in sipeed_maix*.dts, unused in canaan_kd233.dts
> 
> See also "[PATCH 2/2] riscv: dts: canaan: Disable I/O devices unless
> used"
> https://lore.kernel.org/r/f85d460efd7ad85ec59c9253c989b10a07f2ff24.1715163174.git.geert+renesas@xxxxxxxxx
> ---
>  drivers/spi/spi-dw-mmio.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index c56de35eca988ee4..819907e332c4b004 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -321,6 +321,11 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
>  	struct dw_spi *dws;
>  	int ret;
>  
> +	if (device_property_read_bool(&pdev->dev, "spi-slave")) {
> +		dev_warn(&pdev->dev, "spi-slave is not yet supported\n");
> +		return -ENODEV;
> +	}
> +
>  	dwsmmio = devm_kzalloc(&pdev->dev, sizeof(struct dw_spi_mmio),
>  			GFP_KERNEL);
>  	if (!dwsmmio)

-- 
Damien Le Moal
Western Digital Research