| Message ID | 20230506073018.1411583-2-bigunclemax@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Allwinner R329/D1/R528/T113s SPI support | expand |
Hey Maksim, On Sat, May 06, 2023 at 10:30:09AM +0300, Maksim Kiselev wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > Allwinner R329 SPI has two controllers, and the second one has helper > functions for MIPI-DBI Type C. > > Add compatible strings for these controllers > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > --- > .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml > index de36c6a34a0f..2c1b8da35339 100644 > --- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml > +++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml > @@ -21,6 +21,8 @@ properties: > oneOf: > - const: allwinner,sun6i-a31-spi > - const: allwinner,sun8i-h3-spi > + - const: allwinner,sun50i-r329-spi > + - const: allwinner,sun50i-r329-spi-dbi From the driver patch: "Add basical support for these controllers. As we're not going to support the DBI functionality now, just implement the two kinds of controllers as the same." Should this not be set up as a fallback compatible, per Samuel's suggestion here: https://lore.kernel.org/lkml/9ae7d1ee-4e2d-f3c1-f55f-e96b0e449b63@sholland.org/ Thanks, Conor. > - items: > - enum: > - allwinner,sun8i-r40-spi > -- > 2.39.2 >
On 06/05/2023 09:30, Maksim Kiselev wrote: > From: Icenowy Zheng <icenowy@aosc.io> > > Allwinner R329 SPI has two controllers, and the second one has helper > functions for MIPI-DBI Type C. I wonder what is the difference of DBI compatible. You refer to "helper functions", which sounds like driver... do you mean some parts of SPI controller? > > Add compatible strings for these controllers > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > --- > .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml > index de36c6a34a0f..2c1b8da35339 100644 > --- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml > +++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml > @@ -21,6 +21,8 @@ properties: > oneOf: > - const: allwinner,sun6i-a31-spi > - const: allwinner,sun8i-h3-spi > + - const: allwinner,sun50i-r329-spi > + - const: allwinner,sun50i-r329-spi-dbi As Conor pointed out, nothing improved here. Best regards, Krzysztof
> Should this not be set up as a fallback compatible, per Samuel's > suggestion here: Ok, I'll do it in the next version. > I wonder what is the difference of DBI compatible. You refer to "helper > functions", which sounds like driver... do you mean some parts of SPI > controller? According to the D1 datasheet the SPI_DBI controller uses the same registers layout as the regular SPI0 controller. But also it has an additional DBI mode functionality. Support for this mode is not yet implemented. So there is no difference between 'sun50i-r329-spi' and 'sun50i-r329-spi-dbi' controllers types in the SPI driver. Maybe we should drop 'sun50i-r329-spi-dbi' compatible struct from here https://lore.kernel.org/lkml/20230506073018.1411583-5-bigunclemax@gmail.com/ for a while the DBI mode functionality will not be implemented?
On Sat, 6 May 2023 12:53:07 +0200 Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: Hi Maksim, > On 06/05/2023 09:30, Maksim Kiselev wrote: > > From: Icenowy Zheng <icenowy@aosc.io> > > > > Allwinner R329 SPI has two controllers, and the second one has helper > > functions for MIPI-DBI Type C. > > I wonder what is the difference of DBI compatible. You refer to "helper > functions", which sounds like driver... do you mean some parts of SPI > controller? So I checked the manuals, all of the D1, T113-s and R329 are the same in this respect: - There are two SPI controllers, the "first" one is what this series fully supports. - The second SPI controller has some additional registers and two extra bits in the control register to drive the DBI extension, but is otherwise compatible to the first one: So I'd suggest to introduce the following compatible string combinations to the binding *now*. We don't support the DBI bits (yet), but this would be the correct hardware description: For the R329: spi0: spi@4025000 { compatible = "allwinner,sun50i-r329-spi"; .... spi1: spi@4026000 { compatible = "allwinner,sun50i-r329-spi-dbi", "allwinner,sun50i-r329-spi"; For the D1/T113s: spi0: spi@4025000 { compatible = "allwinner,sun20i-d1-spi", "allwinner,sun50i-r329-spi"; .... spi1: spi@4026000 { compatible = "allwinner,sun20i-d1-spi-dbi", "allwinner,sun50i-r329-spi-dbi", "allwinner,sun50i-r329-spi"; I leave that as an exercise to the reader to convert that into the minimal set of DT schema lines ;-) I would suggest to add both the D1/T113s and the R329 bindings in this one patch, to reduce the churn. I guess if you do this, you could even drop Icenowy's authorship on this patch, since it has not much to do with the original version anymore. Cheers, Andre > > Add compatible strings for these controllers > > > > Signed-off-by: Icenowy Zheng <icenowy@aosc.io> > > > --- > > .../devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml > > index de36c6a34a0f..2c1b8da35339 100644 > > --- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml > > +++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml > > @@ -21,6 +21,8 @@ properties: > > oneOf: > > - const: allwinner,sun6i-a31-spi > > - const: allwinner,sun8i-h3-spi > > + - const: > > + - const: allwinner,sun50i-r329-spi-dbi > > As Conor pointed out, nothing improved here. > > Best regards, > Krzysztof >
On 06/05/2023 14:59, Maxim Kiselev wrote: >> Should this not be set up as a fallback compatible, per Samuel's >> suggestion here: > > Ok, I'll do it in the next version. > >> I wonder what is the difference of DBI compatible. You refer to "helper >> functions", which sounds like driver... do you mean some parts of SPI >> controller? > > According to the D1 datasheet the SPI_DBI controller uses the same > registers layout as the regular SPI0 controller. > But also it has an additional DBI mode functionality. Support for this > mode is not yet implemented. > So there is no difference between 'sun50i-r329-spi' and > 'sun50i-r329-spi-dbi' controllers types in the SPI driver. > > Maybe we should drop 'sun50i-r329-spi-dbi' compatible struct from here > https://lore.kernel.org/lkml/20230506073018.1411583-5-bigunclemax@gmail.com/ > for a while the DBI mode functionality will not be implemented? You need both compatibles, but keep DBI compatible with regular one. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml index de36c6a34a0f..2c1b8da35339 100644 --- a/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml +++ b/Documentation/devicetree/bindings/spi/allwinner,sun6i-a31-spi.yaml @@ -21,6 +21,8 @@ properties: oneOf: - const: allwinner,sun6i-a31-spi - const: allwinner,sun8i-h3-spi + - const: allwinner,sun50i-r329-spi + - const: allwinner,sun50i-r329-spi-dbi - items: - enum: - allwinner,sun8i-r40-spi