| Message ID | 20250116232118.2694169-2-sean.anderson@linux.dev |
|---|---|
| State | New |
| Headers | show |
| Series | spi: zynqmp-gqspi: Split the bus and add GPIO support | expand |
On 1/16/25 5:21 PM, Sean Anderson wrote: > This device supports two separate SPI busses: ... > @@ -84,5 +94,32 @@ examples: > resets = <&zynqmp_reset ZYNQMP_RESET_QSPI>; > reg = <0x0 0xff0f0000 0x0 0x1000>, > <0x0 0xc0000000 0x0 0x8000000>; > + > + spi-lower { > + #address-cells = <1>; > + #size-cells = <0>; > + num-cs = <2>; > + cs-gpios = <0>, <&gpio 5>; > + > + flash@0 { > + reg = <0>; > + compatible = "jedec,spi-nor"; > + }; > + > + flash@1 { > + reg = <1>; > + compatible = "jedec,spi-nor"; > + }; > + }; > + > + spi-upper { > + #address-cells = <1>; > + #size-cells = <0>; > + > + flash@0 { > + reg = <0>; > + compatible = "jedec,spi-nor"; > + }; > + }; > }; > }; In the IIO subsystem, we've been recently working on several "advanced" ADCs that could use a SPI controller like this. These ADCs have multiple input channels that perform conversions in parallel and the data for each channel needs to be read back on a separate serial line (MISO) at the same time. Another similar case is to have two separate chips, but they share a conversion trigger and essentially operate as a single composite device rather than two distinct devices [1]. This would be similar to some ADCs that are daisy-chained where we consider all of the chips in the chain as a single composite device, but they would be in parallel rather than chained. [1]: https://lore.kernel.org/linux-iio/e5e8eba7-2455-488b-a36f-e246844e11fd@baylibre.com/ For those use cases though, as mentioned above, we only have a single device that would be connected to both buses. So for such a SPI controller with multiple buses, I was envisioning that instead of adding child nodes for each of the child buses, that we would do something like add a spi-buses property to the spi peripheral bindings where you could specify one or more buses that a device was connected to. e.g. a device connected to the lower bus would be spi-buses = <0>; one connected to the upper bus would be spi-buses = <1>; and a device connected to both would be spi-buses = <0>, <1>;. This would also work for SPI controllers that have 4 or 8 busses. SPI controllers like these have a striping feature that allows to control both buses at the same to either mirror the same data on both buses at the same time when writing, e.g. for configuration or to read and write two different bytes at the same time. A peripheral driver for device that was connected to both buses could make use of this feature to craft a single SPI message with transfers containing (new) parameters that specify which bus to use (one or both) and, in the case of using both buses, to mirror or stripe the data. Could we make a single device connected to both buses like this work using the proposed spi-lower and spi-upper or should we consider a different binding like the one I suggested?
On 1/21/25 19:16, David Lechner wrote: > On 1/16/25 5:21 PM, Sean Anderson wrote: >> This device supports two separate SPI busses: > > ... > >> @@ -84,5 +94,32 @@ examples: >> resets = <&zynqmp_reset ZYNQMP_RESET_QSPI>; >> reg = <0x0 0xff0f0000 0x0 0x1000>, >> <0x0 0xc0000000 0x0 0x8000000>; >> + >> + spi-lower { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + num-cs = <2>; >> + cs-gpios = <0>, <&gpio 5>; >> + >> + flash@0 { >> + reg = <0>; >> + compatible = "jedec,spi-nor"; >> + }; >> + >> + flash@1 { >> + reg = <1>; >> + compatible = "jedec,spi-nor"; >> + }; >> + }; >> + >> + spi-upper { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + flash@0 { >> + reg = <0>; >> + compatible = "jedec,spi-nor"; >> + }; >> + }; >> }; >> }; > > In the IIO subsystem, we've been recently working on several "advanced" ADCs > that could use a SPI controller like this. These ADCs have multiple input > channels that perform conversions in parallel and the data for each channel > needs to be read back on a separate serial line (MISO) at the same time. Another > similar case is to have two separate chips, but they share a conversion trigger > and essentially operate as a single composite device rather than two distinct > devices [1]. This would be similar to some ADCs that are daisy-chained where we > consider all of the chips in the chain as a single composite device, but they > would be in parallel rather than chained. > > [1]: https://lore.kernel.org/linux-iio/e5e8eba7-2455-488b-a36f-e246844e11fd@baylibre.com/ > > For those use cases though, as mentioned above, we only have a single device > that would be connected to both buses. So for such a SPI controller with > multiple buses, I was envisioning that instead of adding child nodes for each > of the child buses, that we would do something like add a spi-buses property > to the spi peripheral bindings where you could specify one or more buses that > a device was connected to. > > e.g. a device connected to the lower bus would be spi-buses = <0>; one connected > to the upper bus would be spi-buses = <1>; and a device connected to both would > be spi-buses = <0>, <1>;. This would also work for SPI controllers that have > 4 or 8 busses. > > SPI controllers like these have a striping feature that allows to control both > buses at the same to either mirror the same data on both buses at the same > time when writing, e.g. for configuration or to read and write two different > bytes at the same time. A peripheral driver for device that was connected to > both buses could make use of this feature to craft a single SPI message with > transfers containing (new) parameters that specify which bus to use (one or > both) and, in the case of using both buses, to mirror or stripe the data. > > Could we make a single device connected to both buses like this work using > the proposed spi-lower and spi-upper or should we consider a different binding > like the one I suggested? If you are willing to do the work to rewrite the SPI subsystem to handle this, then I don't object to it in principle. Using a property would also help with forwards compatibility. On the other hand, separate busses are easier to implement since they integrate better with the SPI subsystem (e.g. you can just call spi_register_controller to create all the slaves). There have been some previous patches from Xilinx to handle this use case [1], but IMO they were pretty hacky. They got this feature merged into U-Boot and it broke many other boards and took a lot of cleanup to fix. So I have intentionally only tackled the unsynchronized use case since that requires no modification to areas outside of this driver. I don't need the "parallel" use case and I am not interested in doing the work required to implement it. --Sean [1] https://lore.kernel.org/linux-spi/20221017121249.19061-1-amit.kumar-mahapatra@amd.com/
On 1/23/25 16:59, David Lechner wrote: > On 1/23/25 10:24 AM, Sean Anderson wrote: >> On 1/21/25 19:16, David Lechner wrote: >>> On 1/16/25 5:21 PM, Sean Anderson wrote: > > ... > >>> Could we make a single device connected to both buses like this work using >>> the proposed spi-lower and spi-upper or should we consider a different binding >>> like the one I suggested? >> >> If you are willing to do the work to rewrite the SPI subsystem to handle >> this, then I don't object to it in principle. Using a property would >> also help with forwards compatibility. On the other hand, separate >> busses are easier to implement since they integrate better with the SPI >> subsystem (e.g. you can just call spi_register_controller to create all >> the slaves). >> >> There have been some previous patches from Xilinx to handle this >> use case [1], but IMO they were pretty hacky. They got this feature >> merged into U-Boot and it broke many other boards and took a lot of >> cleanup to fix. So I have intentionally only tackled the unsynchronized >> use case since that requires no modification to areas outside of this >> driver. I don't need the "parallel" use case and I am not interested in >> doing the work required to implement it. >> >> --Sean >> >> [1] https://lore.kernel.org/linux-spi/20221017121249.19061-1-amit.kumar-mahapatra@amd.com/ > > Fair enough, and I think it can be done without breaking things like the multi > CS support did. > > Here are a couple of patches. Feel free to resubmit them with your series if > they work for you. To make it work with your series, you should just need to > modify the .dts to look like this: > > + flash@0 { > + compatible = "jedec,spi-nor"; > + reg = <0>; > + spi-buses = <0>; /* lower */ > + }; > + > + flash@1 { > + reg = <1>; > + compatible = "jedec,spi-nor"; > + /* also OK to omit property in case of spi-buses = <0>; */ > + }; > + > + flash@2 { > + reg = <2>; > + compatible = "jedec,spi-nor"; > + spi-buses = <1>; /* upper */ > + }; > > > Then drop patch "spi: zynqmp-gqspi: Split the bus" of course. > > In zynqmp_qspi_probe(), add a line: > > ctlr->num_buses = 2; > > And in the zynqmp_qspi_transfer_one() function, use spi->buses to select the > correct bus: > > xqspi->genfifobus = FIELD_PREP(GQSPI_GENFIFO_BUS_MASK, spi->buses); > > I don't have a SPI controller on hand with multiple buses, so I don't have > any patch adding support to a specific controller. But I did build and run this > and hacked in some stuff to the drivers I am working on to make sure it is > working as advertised as best as I could with a single bus. Your patches LGTM. I will use them for v2. Mark do you have any comments on this approach? --Sean
On Thu, Jan 23, 2025 at 05:37:16PM -0500, Sean Anderson wrote: > Your patches LGTM. I will use them for v2. Mark do you have any comments on this > approach? It looks fine.
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@baylibre.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@baylibre.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? --Sean > + } else { > + for (idx = 0; idx < rc; idx++) { > + if (buses[idx] >= ctlr->num_buses) { > + dev_err(&ctlr->dev, > + "%pOF has out of range 'spi-buses' property (%d)\n", > + nc, buses[idx]); > + return -EINVAL; > + } > + spi->buses |= BIT(buses[idx]); > + } > + } > + > /* Device DUAL/QUAD mode */ > if (!of_property_read_u32(nc, "spi-tx-bus-width", &value)) { > switch (value) { > @@ -3072,6 +3095,7 @@ struct spi_controller *__spi_alloc_controller(struct device *dev, > mutex_init(&ctlr->add_lock); > ctlr->bus_num = -1; > ctlr->num_chipselect = 1; > + ctlr->num_buses = 1; > ctlr->slave = slave; > if (IS_ENABLED(CONFIG_SPI_SLAVE) && slave) > ctlr->dev.class = &spi_slave_class; > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h > index 4c087009cf97..bc45d70e8c45 100644 > --- a/include/linux/spi/spi.h > +++ b/include/linux/spi/spi.h > @@ -187,6 +187,11 @@ struct spi_device { > struct device dev; > struct spi_controller *controller; > u32 max_speed_hz; > + /* > + * Bit flags indicating which buses this device is connected to. Only > + * applicable to multi-bus controllers. > + */ > + u8 buses; > u8 chip_select[SPI_CS_CNT_MAX]; > u8 bits_per_word; > bool rt; > @@ -570,6 +575,14 @@ struct spi_controller { > */ > u16 num_chipselect; > > + /* > + * Some specialized SPI controllers can have more than one physical > + * bus interface per controller. This specifies the number of buses > + * in that case. Other controllers do not need to set this (defaults > + * to 1). > + */ > + u16 num_buses; > + > /* Some SPI controllers pose alignment requirements on DMAable > * buffers; let protocol drivers know about these requirements. > */ > -- > 2.43.0 > > >
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@baylibre.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@baylibre.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)
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@baylibre.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@baylibre.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
On 6/13/25 10:57 AM, Sean Anderson wrote: > 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@baylibre.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@baylibre.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. > Can we just leave spi->buses unset then and leave it up to the controller driver to interpret that as "default" and handle it appropriately? OF changessets seems overkill to me.
diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml index 901e15fcce2d..12c547c4f1ba 100644 --- a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml @@ -39,6 +39,18 @@ properties: resets: maxItems: 1 + spi-lower: + type: object + $ref: spi-controller.yaml# + unevaluatedProperties: false + description: The "lower" bus (SPI0). On the ZynqMP this uses MIO pins 0-5. + + spi-upper: + type: object + $ref: spi-controller.yaml# + unevaluatedProperties: false + description: The "upper" bus (SPI1). On the ZynqMP this uses MIO pins 7-12. + required: - compatible - reg @@ -50,8 +62,6 @@ required: unevaluatedProperties: false allOf: - - $ref: spi-controller.yaml# - - if: properties: compatible: @@ -75,7 +85,7 @@ examples: #address-cells = <2>; #size-cells = <2>; - qspi: spi@ff0f0000 { + qspi: spi-controller@ff0f0000 { compatible = "xlnx,zynqmp-qspi-1.0"; clocks = <&zynqmp_clk QSPI_REF>, <&zynqmp_clk LPD_LSBUS>; clock-names = "ref_clk", "pclk"; @@ -84,5 +94,32 @@ examples: resets = <&zynqmp_reset ZYNQMP_RESET_QSPI>; reg = <0x0 0xff0f0000 0x0 0x1000>, <0x0 0xc0000000 0x0 0x8000000>; + + spi-lower { + #address-cells = <1>; + #size-cells = <0>; + num-cs = <2>; + cs-gpios = <0>, <&gpio 5>; + + flash@0 { + reg = <0>; + compatible = "jedec,spi-nor"; + }; + + flash@1 { + reg = <1>; + compatible = "jedec,spi-nor"; + }; + }; + + spi-upper { + #address-cells = <1>; + #size-cells = <0>; + + flash@0 { + reg = <0>; + compatible = "jedec,spi-nor"; + }; + }; }; };
This device supports two separate SPI busses: "lower" (SPI0) and "upper" (SPI1). Each SPI bus has separate clock and data lines, as well as a hardware-controlled chip select. The current binding does not model this situation. It exposes one bus, where CS 0 uses the lower bus and the lower chip select, and CS 1 uses the upper bus and the upper chip select. It is not possible to use the upper chip select with the lower bus (or vice versa). GPIO chip selects are unsupported, and there would be no way to specify which bus to use if they were. Split the "merged" bus into an upper and lower bus, each with their own subnodes. Signed-off-by: Sean Anderson <sean.anderson@linux.dev> --- .../bindings/spi/spi-zynqmp-qspi.yaml | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-)