diff mbox series

[06/14] spi: dt-bindings: dw-apb-ssi: update spi-{r, t}x-bus-width for dwc-ssi

Message ID 20220618123035.563070-7-mail@conchuod.ie (mailing list archive)
State New, archived
Headers show
Series Canaan devicetree fixes | expand

Commit Message

Conor Dooley June 18, 2022, 12:30 p.m. UTC
From: Conor Dooley <conor.dooley@microchip.com>

snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
this.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/spi/snps,dw-apb-ssi.yaml         | 48 ++++++++++++++-----
 1 file changed, 35 insertions(+), 13 deletions(-)

Comments

Geert Uytterhoeven June 20, 2022, 8:02 a.m. UTC | #1
Hi Conor,

On Sat, Jun 18, 2022 at 2:32 PM Conor Dooley <mail@conchuod.ie> wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
> this.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -135,19 +135,41 @@ properties:
>        of the designware controller, and the upper limit is also subject to
>        controller configuration.
>
> -patternProperties:
> -  "^.*@[0-9a-f]+$":
> -    type: object
> -    properties:
> -      reg:
> -        minimum: 0
> -        maximum: 3
> -
> -      spi-rx-bus-width:
> -        const: 1
> -
> -      spi-tx-bus-width:
> -        const: 1
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: snps,dwc-ssi-1.01a
> +
> +then:
> +  patternProperties:
> +    "^.*@[0-9a-f]+$":
> +      type: object
> +      properties:
> +        reg:
> +          minimum: 0
> +          maximum: 3
> +
> +        spi-rx-bus-width:
> +          const: 4
> +
> +        spi-tx-bus-width:
> +          const: 4

These two also depend on the board (SPI device + wiring).
So all of [1, 2, 4] are valid values.

> +
> +else:
> +  patternProperties:
> +    "^.*@[0-9a-f]+$":
> +      type: object
> +      properties:
> +        reg:
> +          minimum: 0
> +          maximum: 3
> +
> +        spi-rx-bus-width:
> +          const: 1
> +
> +        spi-tx-bus-width:
> +          const: 1
>
>  unevaluatedProperties: false

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Conor Dooley June 20, 2022, 8:47 a.m. UTC | #2
On 20/06/2022 09:02, Geert Uytterhoeven wrote:
> Hi Conor,
> 
> On Sat, Jun 18, 2022 at 2:32 PM Conor Dooley <mail@conchuod.ie> wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
>> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
>> this.
>>
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Thanks for your patch!
> 
>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> @@ -135,19 +135,41 @@ properties:
>>         of the designware controller, and the upper limit is also subject to
>>         controller configuration.
>>
>> -patternProperties:
>> -  "^.*@[0-9a-f]+$":
>> -    type: object
>> -    properties:
>> -      reg:
>> -        minimum: 0
>> -        maximum: 3
>> -
>> -      spi-rx-bus-width:
>> -        const: 1
>> -
>> -      spi-tx-bus-width:
>> -        const: 1
>> +if:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        const: snps,dwc-ssi-1.01a
>> +
>> +then:
>> +  patternProperties:
>> +    "^.*@[0-9a-f]+$":
>> +      type: object
>> +      properties:
>> +        reg:
>> +          minimum: 0
>> +          maximum: 3
>> +
>> +        spi-rx-bus-width:
>> +          const: 4
>> +
>> +        spi-tx-bus-width:
>> +          const: 4
> 
> These two also depend on the board (SPI device + wiring).
> So all of [1, 2, 4] are valid values.

Yeah, I wasn't sure about this one as I was writing it.
enum: [1, 2, 4] I guess.
As always, thanks Geert!

Conor.

> 
>> +
>> +else:
>> +  patternProperties:
>> +    "^.*@[0-9a-f]+$":
>> +      type: object
>> +      properties:
>> +        reg:
>> +          minimum: 0
>> +          maximum: 3
>> +
>> +        spi-rx-bus-width:
>> +          const: 1
>> +
>> +        spi-tx-bus-width:
>> +          const: 1
>>
>>   unevaluatedProperties: false
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                  -- Linus Torvalds
Serge Semin June 20, 2022, 8:56 p.m. UTC | #3
On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
> this.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 48 ++++++++++++++-----
>  1 file changed, 35 insertions(+), 13 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..f2b9e3f062cd 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -135,19 +135,41 @@ properties:
>        of the designware controller, and the upper limit is also subject to
>        controller configuration.
>  
> -patternProperties:
> -  "^.*@[0-9a-f]+$":
> -    type: object
> -    properties:
> -      reg:
> -        minimum: 0
> -        maximum: 3
> -
> -      spi-rx-bus-width:
> -        const: 1
> -
> -      spi-tx-bus-width:
> -        const: 1
> +if:
> +  properties:
> +    compatible:
> +      contains:
> +        const: snps,dwc-ssi-1.01a
> +
> +then:
> +  patternProperties:
> +    "^.*@[0-9a-f]+$":
> +      type: object
> +      properties:
> +        reg:
> +          minimum: 0
> +          maximum: 3
> +
> +        spi-rx-bus-width:
> +          const: 4
> +
> +        spi-tx-bus-width:
> +          const: 4
> +
> +else:
> +  patternProperties:
> +    "^.*@[0-9a-f]+$":
> +      type: object
> +      properties:
> +        reg:
> +          minimum: 0
> +          maximum: 3
> +
> +        spi-rx-bus-width:
> +          const: 1
> +
> +        spi-tx-bus-width:
> +          const: 1

You can just use a more relaxed constraint "enum: [1 2 4 8]" here
irrespective from the compatible string. 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. So let's keep the
compatible-string-independent "patternProperties" here but just extend
the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
properties values.

Note the DW APB SSI/AHB SSI driver currently doesn't support the
enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
activated by means of the corresponding CSR. So most likely the DW AHB
SSI controllers need some specific setups too.

-Sergey

>  
>  unevaluatedProperties: false
>  
> -- 
> 2.36.1
>
Conor Dooley June 20, 2022, 9:06 p.m. UTC | #4
On 20/06/2022 21:56, Serge Semin wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>>
>> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
>> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
>> this.
>>
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 48 ++++++++++++++-----
>>  1 file changed, 35 insertions(+), 13 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..f2b9e3f062cd 100644
>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>> @@ -135,19 +135,41 @@ properties:
>>        of the designware controller, and the upper limit is also subject to
>>        controller configuration.
>>
>> -patternProperties:
>> -  "^.*@[0-9a-f]+$":
>> -    type: object
>> -    properties:
>> -      reg:
>> -        minimum: 0
>> -        maximum: 3
>> -
>> -      spi-rx-bus-width:
>> -        const: 1
>> -
>> -      spi-tx-bus-width:
>> -        const: 1
>> +if:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        const: snps,dwc-ssi-1.01a
>> +
>> +then:
>> +  patternProperties:
>> +    "^.*@[0-9a-f]+$":
>> +      type: object
>> +      properties:
>> +        reg:
>> +          minimum: 0
>> +          maximum: 3
>> +
>> +        spi-rx-bus-width:
>> +          const: 4
>> +
>> +        spi-tx-bus-width:
>> +          const: 4
>> +
>> +else:
>> +  patternProperties:
>> +    "^.*@[0-9a-f]+$":
>> +      type: object
>> +      properties:
>> +        reg:
>> +          minimum: 0
>> +          maximum: 3
>> +
>> +        spi-rx-bus-width:
>> +          const: 1
>> +
>> +        spi-tx-bus-width:
>> +          const: 1
> 
> You can just use a more relaxed constraint "enum: [1 2 4 8]" here

8 too? sure.

> irrespective from the compatible string. 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. So let's keep the
> compatible-string-independent "patternProperties" here but just extend
> the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
> properties values.

SGTM!

> 
> Note the DW APB SSI/AHB SSI driver currently doesn't support the
> enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
> bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
> DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
> activated by means of the corresponding CSR. So most likely the DW AHB
> SSI controllers need some specific setups too.

hmm, well I'll leave that up to people that have Canaan hardware!
Thanks,
Conor.

> 
> -Sergey
> 
>>
>>  unevaluatedProperties: false
>>
>> --
>> 2.36.1
>>
Damien Le Moal June 20, 2022, 10:46 p.m. UTC | #5
On 6/21/22 06:06, Conor.Dooley@microchip.com wrote:
> On 20/06/2022 21:56, Serge Semin wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>
>>> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
>>> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
>>> this.
>>>
>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>> ---
>>>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 48 ++++++++++++++-----
>>>  1 file changed, 35 insertions(+), 13 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..f2b9e3f062cd 100644
>>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>> @@ -135,19 +135,41 @@ properties:
>>>        of the designware controller, and the upper limit is also subject to
>>>        controller configuration.
>>>
>>> -patternProperties:
>>> -  "^.*@[0-9a-f]+$":
>>> -    type: object
>>> -    properties:
>>> -      reg:
>>> -        minimum: 0
>>> -        maximum: 3
>>> -
>>> -      spi-rx-bus-width:
>>> -        const: 1
>>> -
>>> -      spi-tx-bus-width:
>>> -        const: 1
>>> +if:
>>> +  properties:
>>> +    compatible:
>>> +      contains:
>>> +        const: snps,dwc-ssi-1.01a
>>> +
>>> +then:
>>> +  patternProperties:
>>> +    "^.*@[0-9a-f]+$":
>>> +      type: object
>>> +      properties:
>>> +        reg:
>>> +          minimum: 0
>>> +          maximum: 3
>>> +
>>> +        spi-rx-bus-width:
>>> +          const: 4
>>> +
>>> +        spi-tx-bus-width:
>>> +          const: 4
>>> +
>>> +else:
>>> +  patternProperties:
>>> +    "^.*@[0-9a-f]+$":
>>> +      type: object
>>> +      properties:
>>> +        reg:
>>> +          minimum: 0
>>> +          maximum: 3
>>> +
>>> +        spi-rx-bus-width:
>>> +          const: 1
>>> +
>>> +        spi-tx-bus-width:
>>> +          const: 1
>>
>> You can just use a more relaxed constraint "enum: [1 2 4 8]" here
> 
> 8 too? sure.
> 
>> irrespective from the compatible string. 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. So let's keep the
>> compatible-string-independent "patternProperties" here but just extend
>> the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
>> properties values.
> 
> SGTM!
> 
>>
>> Note the DW APB SSI/AHB SSI driver currently doesn't support the
>> enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
>> bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
>> DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
>> activated by means of the corresponding CSR. So most likely the DW AHB
>> SSI controllers need some specific setups too.
> 
> hmm, well I'll leave that up to people that have Canaan hardware!

I will test this series.

> Thanks,
> Conor.
> 
>>
>> -Sergey
>>
>>>
>>>  unevaluatedProperties: false
>>>
>>> --
>>> 2.36.1
>>>
>
Conor Dooley June 20, 2022, 10:49 p.m. UTC | #6
On 20/06/2022 23:46, Damien Le Moal wrote:
> On 6/21/22 06:06, Conor.Dooley@microchip.com wrote:
>> On 20/06/2022 21:56, Serge Semin wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
>>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>>
>>>> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
>>>> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
>>>> this.
>>>>
>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>>> ---
>>>>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 48 ++++++++++++++-----
>>>>  1 file changed, 35 insertions(+), 13 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..f2b9e3f062cd 100644
>>>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> @@ -135,19 +135,41 @@ properties:
>>>>        of the designware controller, and the upper limit is also subject to
>>>>        controller configuration.
>>>>
>>>> -patternProperties:
>>>> -  "^.*@[0-9a-f]+$":
>>>> -    type: object
>>>> -    properties:
>>>> -      reg:
>>>> -        minimum: 0
>>>> -        maximum: 3
>>>> -
>>>> -      spi-rx-bus-width:
>>>> -        const: 1
>>>> -
>>>> -      spi-tx-bus-width:
>>>> -        const: 1
>>>> +if:
>>>> +  properties:
>>>> +    compatible:
>>>> +      contains:
>>>> +        const: snps,dwc-ssi-1.01a
>>>> +
>>>> +then:
>>>> +  patternProperties:
>>>> +    "^.*@[0-9a-f]+$":
>>>> +      type: object
>>>> +      properties:
>>>> +        reg:
>>>> +          minimum: 0
>>>> +          maximum: 3
>>>> +
>>>> +        spi-rx-bus-width:
>>>> +          const: 4
>>>> +
>>>> +        spi-tx-bus-width:
>>>> +          const: 4
>>>> +
>>>> +else:
>>>> +  patternProperties:
>>>> +    "^.*@[0-9a-f]+$":
>>>> +      type: object
>>>> +      properties:
>>>> +        reg:
>>>> +          minimum: 0
>>>> +          maximum: 3
>>>> +
>>>> +        spi-rx-bus-width:
>>>> +          const: 1
>>>> +
>>>> +        spi-tx-bus-width:
>>>> +          const: 1
>>>
>>> You can just use a more relaxed constraint "enum: [1 2 4 8]" here
>>
>> 8 too? sure.
>>
>>> irrespective from the compatible string. 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. So let's keep the
>>> compatible-string-independent "patternProperties" here but just extend
>>> the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
>>> properties values.
>>
>> SGTM!
>>
>>>
>>> Note the DW APB SSI/AHB SSI driver currently doesn't support the
>>> enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
>>> bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
>>> DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
>>> activated by means of the corresponding CSR. So most likely the DW AHB
>>> SSI controllers need some specific setups too.
>>
>> hmm, well I'll leave that up to people that have Canaan hardware!
> 
> I will test this series.
> 

Cool, thanks.
I'll try to get a respin out tomorrow w/ the memory node "unfixed".
Conor.

>> Thanks,
>> Conor.
>>
>>>
>>> -Sergey
>>>
>>>>
>>>>  unevaluatedProperties: false
>>>>
>>>> --
>>>> 2.36.1
>>>>
>>
> 
>
Damien Le Moal June 20, 2022, 11:17 p.m. UTC | #7
On 6/21/22 07:49, Conor Dooley wrote:
> 
> 
> On 20/06/2022 23:46, Damien Le Moal wrote:
>> On 6/21/22 06:06, Conor.Dooley@microchip.com wrote:
>>> On 20/06/2022 21:56, Serge Semin wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
>>>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>>>
>>>>> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
>>>>> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
>>>>> this.
>>>>>
>>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>>>> ---
>>>>>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 48 ++++++++++++++-----
>>>>>  1 file changed, 35 insertions(+), 13 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..f2b9e3f062cd 100644
>>>>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>>> @@ -135,19 +135,41 @@ properties:
>>>>>        of the designware controller, and the upper limit is also subject to
>>>>>        controller configuration.
>>>>>
>>>>> -patternProperties:
>>>>> -  "^.*@[0-9a-f]+$":
>>>>> -    type: object
>>>>> -    properties:
>>>>> -      reg:
>>>>> -        minimum: 0
>>>>> -        maximum: 3
>>>>> -
>>>>> -      spi-rx-bus-width:
>>>>> -        const: 1
>>>>> -
>>>>> -      spi-tx-bus-width:
>>>>> -        const: 1
>>>>> +if:
>>>>> +  properties:
>>>>> +    compatible:
>>>>> +      contains:
>>>>> +        const: snps,dwc-ssi-1.01a
>>>>> +
>>>>> +then:
>>>>> +  patternProperties:
>>>>> +    "^.*@[0-9a-f]+$":
>>>>> +      type: object
>>>>> +      properties:
>>>>> +        reg:
>>>>> +          minimum: 0
>>>>> +          maximum: 3
>>>>> +
>>>>> +        spi-rx-bus-width:
>>>>> +          const: 4
>>>>> +
>>>>> +        spi-tx-bus-width:
>>>>> +          const: 4
>>>>> +
>>>>> +else:
>>>>> +  patternProperties:
>>>>> +    "^.*@[0-9a-f]+$":
>>>>> +      type: object
>>>>> +      properties:
>>>>> +        reg:
>>>>> +          minimum: 0
>>>>> +          maximum: 3
>>>>> +
>>>>> +        spi-rx-bus-width:
>>>>> +          const: 1
>>>>> +
>>>>> +        spi-tx-bus-width:
>>>>> +          const: 1
>>>>
>>>> You can just use a more relaxed constraint "enum: [1 2 4 8]" here
>>>
>>> 8 too? sure.
>>>
>>>> irrespective from the compatible string. 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. So let's keep the
>>>> compatible-string-independent "patternProperties" here but just extend
>>>> the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
>>>> properties values.
>>>
>>> SGTM!
>>>
>>>>
>>>> Note the DW APB SSI/AHB SSI driver currently doesn't support the
>>>> enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
>>>> bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
>>>> DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
>>>> activated by means of the corresponding CSR. So most likely the DW AHB
>>>> SSI controllers need some specific setups too.
>>>
>>> hmm, well I'll leave that up to people that have Canaan hardware!
>>
>> I will test this series.
>>
> 
> Cool, thanks.
> I'll try to get a respin out tomorrow w/ the memory node "unfixed".

OK. I will test that then :)

> Conor.
> 
>>> Thanks,
>>> Conor.
>>>
>>>>
>>>> -Sergey
>>>>
>>>>>
>>>>>  unevaluatedProperties: false
>>>>>
>>>>> --
>>>>> 2.36.1
>>>>>
>>>
>>
>>
Geert Uytterhoeven June 21, 2022, 7:03 a.m. UTC | #8
Hi Serge,

On Mon, Jun 20, 2022 at 10:56 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> >
> > snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
> > width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
> > this.
> >
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >  .../bindings/spi/snps,dw-apb-ssi.yaml         | 48 ++++++++++++++-----
> >  1 file changed, 35 insertions(+), 13 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..f2b9e3f062cd 100644
> > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > @@ -135,19 +135,41 @@ properties:
> >        of the designware controller, and the upper limit is also subject to
> >        controller configuration.
> >
> > -patternProperties:
> > -  "^.*@[0-9a-f]+$":
> > -    type: object
> > -    properties:
> > -      reg:
> > -        minimum: 0
> > -        maximum: 3
> > -
> > -      spi-rx-bus-width:
> > -        const: 1
> > -
> > -      spi-tx-bus-width:
> > -        const: 1
> > +if:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        const: snps,dwc-ssi-1.01a
> > +
> > +then:
> > +  patternProperties:
> > +    "^.*@[0-9a-f]+$":
> > +      type: object
> > +      properties:
> > +        reg:
> > +          minimum: 0
> > +          maximum: 3
> > +
> > +        spi-rx-bus-width:
> > +          const: 4
> > +
> > +        spi-tx-bus-width:
> > +          const: 4
> > +
> > +else:
> > +  patternProperties:
> > +    "^.*@[0-9a-f]+$":
> > +      type: object
> > +      properties:
> > +        reg:
> > +          minimum: 0
> > +          maximum: 3
> > +
> > +        spi-rx-bus-width:
> > +          const: 1
> > +
> > +        spi-tx-bus-width:
> > +          const: 1
>
> You can just use a more relaxed constraint "enum: [1 2 4 8]" here
> irrespective from the compatible string. 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. So let's keep the
> compatible-string-independent "patternProperties" here but just extend
> the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
> properties values.
>
> Note the DW APB SSI/AHB SSI driver currently doesn't support the
> enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
> bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
> DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
> activated by means of the corresponding CSR. So most likely the DW AHB
> SSI controllers need some specific setups too.

That doesn't matter here, as DT describes hardware, not software
limitations.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Serge Semin June 21, 2022, 9:32 a.m. UTC | #9
Hi Geert

On Tue, Jun 21, 2022 at 09:03:25AM +0200, Geert Uytterhoeven wrote:
> Hi Serge,
> 
> On Mon, Jun 20, 2022 at 10:56 PM Serge Semin <fancer.lancer@gmail.com> wrote:
> > On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
> > > From: Conor Dooley <conor.dooley@microchip.com>
> > >

[...]

> > > +
> > > +        spi-rx-bus-width:
> > > +          const: 1
> > > +
> > > +        spi-tx-bus-width:
> > > +          const: 1
> >
> > You can just use a more relaxed constraint "enum: [1 2 4 8]" here
> > irrespective from the compatible string. 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. So let's keep the
> > compatible-string-independent "patternProperties" here but just extend
> > the set of acceptable "spi-rx-bus-width" and "spi-tx-bus-width"
> > properties values.
> >

> > Note the DW APB SSI/AHB SSI driver currently doesn't support the
> > enhanced SPI modes. So I am not sure whether the multi-lines Rx/Tx SPI
> > bus indeed works for Canaan K210 AHB SSI controller. AFAICS from the
> > DW APB SSI v4.01a manual the Enhanced SPI mode needs to be properly
> > activated by means of the corresponding CSR. So most likely the DW AHB
> > SSI controllers need some specific setups too.
> 
> That doesn't matter here, as DT describes hardware, not software
> limitations.

Can't argue with that.) My note regarding the current DW APB SSI
driver was mainly addressed for the Canaan K210 users, since
should the SoC-based board really have the Quad Tx/Rx SPI bus
most likely the interface won't work with the full bandwidth.
So it is a good reason to perform the platform research and if
possible alter the driver accordingly.

-Sergey

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Conor Dooley June 21, 2022, 4:06 p.m. UTC | #10
On 21/06/2022 00:17, Damien Le Moal wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 6/21/22 07:49, Conor Dooley wrote:
>>
---8<---
>>>>
>>>> hmm, well I'll leave that up to people that have Canaan hardware!
>>>
>>> I will test this series.
>>>
>>
>> Cool, thanks.
>> I'll try to get a respin out tomorrow w/ the memory node "unfixed".
> 
> OK. I will test that then :)

Since the memory node hit that dt-schema snag, I have not sent a v2.
Going to be AFK for a few days, so I dropped the memory node change,
changed the spi binding & put the series on:
git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ canaan

If you get a chance to look at it great, if not I'll send a v2 once
the memory node is figured out.

Thanks,
Conor.
Serge Semin June 23, 2022, 10:25 a.m. UTC | #11
On Tue, Jun 21, 2022 at 04:06:21PM +0000, Conor.Dooley@microchip.com wrote:
> On 21/06/2022 00:17, Damien Le Moal wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On 6/21/22 07:49, Conor Dooley wrote:
> >>
> ---8<---
> >>>>
> >>>> hmm, well I'll leave that up to people that have Canaan hardware!
> >>>
> >>> I will test this series.
> >>>
> >>
> >> Cool, thanks.
> >> I'll try to get a respin out tomorrow w/ the memory node "unfixed".
> > 
> > OK. I will test that then :)
> 
> Since the memory node hit that dt-schema snag, I have not sent a v2.
> Going to be AFK for a few days, so I dropped the memory node change,
> changed the spi binding & put the series on:
> git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ canaan
> 

> If you get a chance to look at it great, if not I'll send a v2 once
> the memory node is figured out.

commit 84df6ca0f277 ("spi: dt-bindings: dw-apb-ssi: update
spi-{r,t}x-bus-width") looks good to me. Feel free to add my ack tag
to v2 of that patch.

-Sergey

> 
> Thanks,
> Conor.
>
Conor Dooley June 23, 2022, 12:41 p.m. UTC | #12
On 23/06/2022 11:25, Serge Semin wrote:
> On Tue, Jun 21, 2022 at 04:06:21PM +0000, Conor.Dooley@microchip.com wrote:
>> On 21/06/2022 00:17, Damien Le Moal wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On 6/21/22 07:49, Conor Dooley wrote:
>>>>
>> ---8<---
>>>>>>
>>>>>> hmm, well I'll leave that up to people that have Canaan hardware!
>>>>>
>>>>> I will test this series.
>>>>>
>>>>
>>>> Cool, thanks.
>>>> I'll try to get a respin out tomorrow w/ the memory node "unfixed".
>>>
>>> OK. I will test that then :)
>>
>> Since the memory node hit that dt-schema snag, I have not sent a v2.
>> Going to be AFK for a few days, so I dropped the memory node change,
>> changed the spi binding & put the series on:
>> git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ canaan
>>
> 
>> If you get a chance to look at it great, if not I'll send a v2 once
>> the memory node is figured out.
> 
> commit 84df6ca0f277 ("spi: dt-bindings: dw-apb-ssi: update
> spi-{r,t}x-bus-width") looks good to me. Feel free to add my ack tag
> to v2 of that patch.

Sure, thanks!
Rob Herring June 27, 2022, 5:15 p.m. UTC | #13
On Mon, Jun 20, 2022 at 09:06:34PM +0000, Conor.Dooley@microchip.com wrote:
> On 20/06/2022 21:56, Serge Semin wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
> >> From: Conor Dooley <conor.dooley@microchip.com>
> >>
> >> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
> >> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
> >> this.
> >>
> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> >> ---
> >>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 48 ++++++++++++++-----
> >>  1 file changed, 35 insertions(+), 13 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..f2b9e3f062cd 100644
> >> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> >> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> >> @@ -135,19 +135,41 @@ properties:
> >>        of the designware controller, and the upper limit is also subject to
> >>        controller configuration.
> >>
> >> -patternProperties:
> >> -  "^.*@[0-9a-f]+$":
> >> -    type: object
> >> -    properties:
> >> -      reg:
> >> -        minimum: 0
> >> -        maximum: 3
> >> -
> >> -      spi-rx-bus-width:
> >> -        const: 1
> >> -
> >> -      spi-tx-bus-width:
> >> -        const: 1
> >> +if:
> >> +  properties:
> >> +    compatible:
> >> +      contains:
> >> +        const: snps,dwc-ssi-1.01a
> >> +
> >> +then:
> >> +  patternProperties:
> >> +    "^.*@[0-9a-f]+$":
> >> +      type: object
> >> +      properties:
> >> +        reg:
> >> +          minimum: 0
> >> +          maximum: 3
> >> +
> >> +        spi-rx-bus-width:
> >> +          const: 4
> >> +
> >> +        spi-tx-bus-width:
> >> +          const: 4
> >> +
> >> +else:
> >> +  patternProperties:
> >> +    "^.*@[0-9a-f]+$":
> >> +      type: object
> >> +      properties:
> >> +        reg:
> >> +          minimum: 0
> >> +          maximum: 3
> >> +
> >> +        spi-rx-bus-width:
> >> +          const: 1
> >> +
> >> +        spi-tx-bus-width:
> >> +          const: 1
> > 
> > You can just use a more relaxed constraint "enum: [1 2 4 8]" here
> 
> 8 too? sure.

Then no constraints needed because the common definition already has 
this presumably.

Rob
Conor Dooley June 27, 2022, 6:05 p.m. UTC | #14
On 27/06/2022 18:15, Rob Herring wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Mon, Jun 20, 2022 at 09:06:34PM +0000, Conor.Dooley@microchip.com wrote:
>> On 20/06/2022 21:56, Serge Semin wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> On Sat, Jun 18, 2022 at 01:30:28PM +0100, Conor Dooley wrote:
>>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>>
>>>> snps,dwc-ssi-1.01a has a single user - the Canaan k210, which uses a
>>>> width of 4 for spi-{r,t}x-bus-width. Update the binding to reflect
>>>> this.
>>>>
>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>>> ---
>>>>  .../bindings/spi/snps,dw-apb-ssi.yaml         | 48 ++++++++++++++-----
>>>>  1 file changed, 35 insertions(+), 13 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..f2b9e3f062cd 100644
>>>> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
>>>> @@ -135,19 +135,41 @@ properties:
>>>>        of the designware controller, and the upper limit is also subject to
>>>>        controller configuration.
>>>>
>>>> -patternProperties:
>>>> -  "^.*@[0-9a-f]+$":
>>>> -    type: object
>>>> -    properties:
>>>> -      reg:
>>>> -        minimum: 0
>>>> -        maximum: 3
>>>> -
>>>> -      spi-rx-bus-width:
>>>> -        const: 1
>>>> -
>>>> -      spi-tx-bus-width:
>>>> -        const: 1
>>>> +if:
>>>> +  properties:
>>>> +    compatible:
>>>> +      contains:
>>>> +        const: snps,dwc-ssi-1.01a
>>>> +
>>>> +then:
>>>> +  patternProperties:
>>>> +    "^.*@[0-9a-f]+$":
>>>> +      type: object
>>>> +      properties:
>>>> +        reg:
>>>> +          minimum: 0
>>>> +          maximum: 3
>>>> +
>>>> +        spi-rx-bus-width:
>>>> +          const: 4
>>>> +
>>>> +        spi-tx-bus-width:
>>>> +          const: 4
>>>> +
>>>> +else:
>>>> +  patternProperties:
>>>> +    "^.*@[0-9a-f]+$":
>>>> +      type: object
>>>> +      properties:
>>>> +        reg:
>>>> +          minimum: 0
>>>> +          maximum: 3
>>>> +
>>>> +        spi-rx-bus-width:
>>>> +          const: 1
>>>> +
>>>> +        spi-tx-bus-width:
>>>> +          const: 1
>>>
>>> You can just use a more relaxed constraint "enum: [1 2 4 8]" here
>>
>> 8 too? sure.
> 
> Then no constraints needed because the common definition already has
> this presumably.

Almost, the default has 0 in the enum too.
I'll go with that - thanks!
Conor.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index e25d44c218f2..f2b9e3f062cd 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -135,19 +135,41 @@  properties:
       of the designware controller, and the upper limit is also subject to
       controller configuration.
 
-patternProperties:
-  "^.*@[0-9a-f]+$":
-    type: object
-    properties:
-      reg:
-        minimum: 0
-        maximum: 3
-
-      spi-rx-bus-width:
-        const: 1
-
-      spi-tx-bus-width:
-        const: 1
+if:
+  properties:
+    compatible:
+      contains:
+        const: snps,dwc-ssi-1.01a
+
+then:
+  patternProperties:
+    "^.*@[0-9a-f]+$":
+      type: object
+      properties:
+        reg:
+          minimum: 0
+          maximum: 3
+
+        spi-rx-bus-width:
+          const: 4
+
+        spi-tx-bus-width:
+          const: 4
+
+else:
+  patternProperties:
+    "^.*@[0-9a-f]+$":
+      type: object
+      properties:
+        reg:
+          minimum: 0
+          maximum: 3
+
+        spi-rx-bus-width:
+          const: 1
+
+        spi-tx-bus-width:
+          const: 1
 
 unevaluatedProperties: false