Received-SPF: pass (google.com: domain of linux-kernel+bounces-68890-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 16 Feb 2024 18:36:39 +0300
From: Serge Semin <fancer.lancer@gmail.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Mark Brown <broonie@kernel.org>, 
	Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>, linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] spi: dw: Drop default number of CS setting
Message-ID: <4sbbhf4ltdwrmj7rrr6f7lnjbdxrwfjoutmcgsh2c44jy5fxzj@xgqdscqdnkkv>
References: <20240215180102.13887-1-fancer.lancer@gmail.com>
 <20240215180102.13887-4-fancer.lancer@gmail.com>
 <Zc5mxyTjq6X_QRsQ@smile.fi.intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Zc5mxyTjq6X_QRsQ@smile.fi.intel.com>

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