| Message ID | 20220627194003.2395484-5-mail@conchuod.ie (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Canaan devicetree fixes | expand |
On Mon, Jun 27, 2022 at 08:39:52PM +0100, Conor Dooley wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > Most users of dw-apb-ssi use spi-{r,t}x-bus-width of 1, however the > Canaan k210 is wired up for a width of 4. > Quoting Serge: > The modern DW APB SSI controllers of v.4.* and newer also support the > enhanced SPI Modes too (Dual, Quad and Octal). Since the IP-core > version is auto-detected at run-time there is no way to create a > DT-schema correctly constraining the Rx/Tx SPI bus widths. > /endquote > > As such, drop the restriction on only supporting a bus width of 1. > > Link: https://lore.kernel.org/all/20220620205654.g7fyipwytbww5757@mobilestation/ > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > Serge, I dropped your R-b when I swapped to the default > property since it changed the enum. > --- > Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 6 ------ > 1 file changed, 6 deletions(-) > > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml > index e25d44c218f2..0a43d6e0ef91 100644 > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml > @@ -143,12 +143,6 @@ patternProperties: > minimum: 0 > maximum: 3 > > - spi-rx-bus-width: > - const: 1 > - > - spi-tx-bus-width: > - const: 1 > - My comment was: > > > You can just use a more relaxed constraint "enum: [1 2 4 8]" here > > > > 8 too? sure. Then Rob said: > Then no constraints needed because the common definition already has > this presumably. IMO preserving the device-specific constraints even if they match the generic ones has some maintainability benefits. What if you get to discover a new HW which supports Hexal mode? Then you would have needed to update the common schema constraints. But that would have caused permitting the unsupported bus-mode for all the schemas, which isn't correct. So as I see it the explicit bus-width enumeration would be ok to have here. But I'll leave it for Rob to make a final decision. Rob > unevaluatedProperties: false > > required: > -- > 2.36.1 >
On Mon, Jun 27, 2022 at 11:21:49PM +0300, Serge Semin wrote: > On Mon, Jun 27, 2022 at 08:39:52PM +0100, Conor Dooley wrote: > > From: Conor Dooley <conor.dooley@microchip.com> > > > > Most users of dw-apb-ssi use spi-{r,t}x-bus-width of 1, however the > > Canaan k210 is wired up for a width of 4. > > Quoting Serge: > > The modern DW APB SSI controllers of v.4.* and newer also support the > > enhanced SPI Modes too (Dual, Quad and Octal). Since the IP-core > > version is auto-detected at run-time there is no way to create a > > DT-schema correctly constraining the Rx/Tx SPI bus widths. > > /endquote > > > > As such, drop the restriction on only supporting a bus width of 1. > > > > Link: https://lore.kernel.org/all/20220620205654.g7fyipwytbww5757@mobilestation/ > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > > --- > > Serge, I dropped your R-b when I swapped to the default > > property since it changed the enum. > > --- > > Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 6 ------ > > 1 file changed, 6 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml > > index e25d44c218f2..0a43d6e0ef91 100644 > > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml > > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml > > @@ -143,12 +143,6 @@ patternProperties: > > minimum: 0 > > maximum: 3 > > > > > - spi-rx-bus-width: > > - const: 1 > > - > > - spi-tx-bus-width: > > - const: 1 > > - > > My comment was: > > > > You can just use a more relaxed constraint "enum: [1 2 4 8]" here > > > > > > 8 too? sure. > Then Rob said: > > Then no constraints needed because the common definition already has > > this presumably. > > IMO preserving the device-specific constraints even if they match the > generic ones has some maintainability benefits. What if you get to > discover a new HW which supports Hexal mode? x16? Wouldn't we be back to parallel NOR and the problems with parallel buses? > Then you would have > needed to update the common schema constraints. But that would have > caused permitting the unsupported bus-mode for all the schemas, which > isn't correct. So as I see it the explicit bus-width enumeration would > be ok to have here. But I'll leave it for Rob to make a final > decision. Assuming a new width does appear, it's just a matter of time before the DW block has a new rev supporting it too, so there's 2 places to update. Also, a given platform may pinout less than the block supports, so you can't ever be 100% sure an out of range value is in a DT. But either way is okay with me. If you do keep constraints, you only need 'maximum: 8'. Acked-by: Rob Herring <robh@kernel.org> Rob
diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml index e25d44c218f2..0a43d6e0ef91 100644 --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml @@ -143,12 +143,6 @@ patternProperties: minimum: 0 maximum: 3 - spi-rx-bus-width: - const: 1 - - spi-tx-bus-width: - const: 1 - unevaluatedProperties: false required: