Re: [PATCH 3/3] spi: dw: Drop default number of CS setting

[Serge Semin <fancer.lancer@xxxxxxxxx>]

On Thu, Feb 15, 2024 at 09:32:23PM +0200, Andy Shevchenko wrote:
> On Thu, Feb 15, 2024 at 09:00:48PM +0300, Serge Semin wrote:
> > DW APB/AHB SSI core now supports the procedure which automatically
> > determines the number of native CS. Thus there is no longer point in
> > defaulting to four CS if platform doesn't specify the real number.
> 
> ...
>
 
> > -	num_cs = 4;
> 
> Simply update the default here?
> 
> > -	device_property_read_u32(&pdev->dev, "num-cs", &num_cs);

Do you suggest to simply:

--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -364,8 +364,9 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 				     &dws->reg_io_width))
 		dws->reg_io_width = 4;
 
-	num_cs = 4;
+	num_cs = 0;
 
 	device_property_read_u32(&pdev->dev, "num-cs", &num_cs);
 
?

My idea was to make the statement looking closer to what is
implemented for "reg-io-width" property. An alternative to what you
suggest and to my patch can be converting the dw_spi::num_cs type to
u32 and pass it to the device_property_read_u32() method directly:

--- a/drivers/spi/spi-dw.h
+++ b/drivers/spi/spi-dw.h
 	u32			max_freq;	/* max bus freq supported */
 
 	u32			reg_io_width;	/* DR I/O width in bytes */
+	u32			num_cs;		/* supported slave numbers */
 	u16			bus_num;
-	u16			num_cs;		/* supported slave numbers */
 	void (*set_cs)(struct spi_device *spi, bool enable);
 
 	/* Current message transfer state info */
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -320,7 +320,6 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 	struct resource *mem;
 	struct dw_spi *dws;
 	int ret;
-	int num_cs;
 
 	dwsmmio = devm_kzalloc(&pdev->dev, sizeof(struct dw_spi_mmio),
 			       GFP_KERNEL);
@@ -364,8 +363,7 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
 				     &dws->reg_io_width))
 		dws->reg_io_width = 4;

-	num_cs = 4;
-
-       device_property_read_u32(&pdev->dev, "num-cs", &num_cs);
+       device_property_read_u32(&pdev->dev, "num-cs", &dws->num_cs);
-
-	dws->num_cs = num_cs;
 
 	init_func = device_get_match_data(&pdev->dev);
 	if (init_func) {

What do you think? Would that be better?

-Serge(y)

> > -
> > -	dws->num_cs = num_cs;
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
>