linux-kernel - Re: [PATCH 1/7] dt-bindings: spi: zynqmp-qspi: Split the bus

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]

Message-ID: <f3160819-f6f4-4079-9562-802caa2fef20@linux.dev>
Date: Fri, 13 Jun 2025 11:57:34 -0400
From: Sean Anderson <sean.anderson@...ux.dev>
To: David Lechner <dlechner@...libre.com>, Mark Brown <broonie@...nel.org>,
 Michal Simek <michal.simek@....com>, linux-spi@...r.kernel.org
Cc: Jinjie Ruan <ruanjinjie@...wei.com>,
 linux-arm-kernel@...ts.infradead.org,
 Amit Kumar Mahapatra <amit.kumar-mahapatra@....com>,
 linux-kernel@...r.kernel.org, Miquel Raynal <miquel.raynal@...tlin.com>,
 Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Rob Herring <robh@...nel.org>,
 devicetree@...r.kernel.org,
 "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
 Jonathan Cameron <jic23@...nel.org>, Nuno Sá
 <nuno.sa@...log.com>
Subject: Re: [PATCH 1/7] dt-bindings: spi: zynqmp-qspi: Split the bus

On 6/13/25 10:20, David Lechner wrote:
> On 6/12/25 6:44 PM, Sean Anderson wrote:
>> Hi David,
>> 
>> I am (finally!) getting around to doing v2 of this series, and I ran
>> into a small problem with your proposed solution.
>> 
>> On 1/23/25 16:59, David Lechner wrote:
>>> ---
>>> From: David Lechner <dlechner@...libre.com>
>>> Date: Thu, 23 Jan 2025 15:35:19 -0600
>>> Subject: [PATCH 2/2] spi: add support for multi-bus controllers
>>>
>>> Add support for SPI controllers with multiple physical SPI buses.
>>>
>>> This is common in the type of controller that can be used with parallel
>>> flash memories, but can be used for general purpose SPI as well.
>>>
>>> To indicate support, a controller just needs to set ctlr->num_buses to
>>> something greater than 1. Peripherals indicate which bus they are
>>> connected to via device tree (ACPI support can be added if needed).
>>>
>>> In the future, this can be extended to support peripherals that also
>>> have multiple SPI buses to use those buses at the same time by adding
>>> a similar bus flags field to struct spi_transfer.
>>>
>>> Signed-off-by: David Lechner <dlechner@...libre.com>
>>> ---
>>>  drivers/spi/spi.c       | 26 +++++++++++++++++++++++++-
>>>  include/linux/spi/spi.h | 13 +++++++++++++
>>>  2 files changed, 38 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
>>> index 10c365e9100a..f7722e5e906d 100644
>>> --- a/drivers/spi/spi.c
>>> +++ b/drivers/spi/spi.c
>>> @@ -2364,7 +2364,7 @@ static void of_spi_parse_dt_cs_delay(struct device_node *nc,
>>>  static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>>>  			   struct device_node *nc)
>>>  {
>>> -	u32 value, cs[SPI_CS_CNT_MAX];
>>> +	u32 value, buses[8], cs[SPI_CS_CNT_MAX];
>>>  	int rc, idx;
>>>  
>>>  	/* Mode (clock phase/polarity/etc.) */
>>> @@ -2379,6 +2379,29 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>>>  	if (of_property_read_bool(nc, "spi-cs-high"))
>>>  		spi->mode |= SPI_CS_HIGH;
>>>  
>>> +	rc = of_property_read_variable_u32_array(nc, "spi-buses", buses, 1,
>>> +						 ARRAY_SIZE(buses));
>>> +	if (rc < 0 && rc != -EINVAL) {
>>> +		dev_err(&ctlr->dev, "%pOF has invalid 'spi-buses' property (%d)\n",
>>> +			nc, rc);
>>> +		return rc;
>>> +	}
>>> +
>>> +	if (rc == -EINVAL) {
>>> +		/* Default when property is omitted. */
>>> +		spi->buses = BIT(0);
>> 
>> For backwards compatibility, the default bus for CS 1 on gqspi must be 1
>> and not 0. Ideally there would be some hook for the master to fix things
>> up when the slaves are probed, but that doesn't seem to exist. I was
>> thinking about doing this with OF changesets. Do you have any better
>> ideas?
>> 
> 
> Does this work? 
> 
> 		spi->buses = BIT(cs[0]);
> 
> (would have to move all the new code after cs[0] is assigned of course)

Yeah, but do we really want to make this the default for all drivers?
This is really a quirk of the existing gqspi binding and I don't think
it makes sense in general.

--Sean