diff mbox series

[v2,1/6] dt-bindings: spi: sun6i: add DT bindings for Allwinner R329 SPI

Message ID 20230506073018.1411583-2-bigunclemax@gmail.com
State New
Headers
Series Allwinner R329/D1/R528/T113s SPI support |

Commit Message

Maxim Kiselev May 6, 2023, 7:30 a.m. UTC 
  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(+)

Comments

Conor Dooley May 6, 2023, 10:45 a.m. UTC | #1 
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
>
Krzysztof Kozlowski May 6, 2023, 10:53 a.m. UTC | #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
Maxim Kiselev May 6, 2023, 12:59 p.m. UTC | #3 
> 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?
Andre Przywara May 6, 2023, 9:58 p.m. UTC | #4 
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
>
Krzysztof Kozlowski May 7, 2023, 7:42 a.m. UTC | #5 
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 mbox series

Patch

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