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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 16 Feb 2024 18:36:39 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Mark Brown <broonie@...nel.org>, 
	Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>, linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] spi: dw: Drop default number of CS setting

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
> 
> 
>