* Re: [PATCH] spi: atmel,quadspi: Define lan966x QSPI
2022-04-07 10:54 [PATCH] spi: atmel,quadspi: Define lan966x QSPI Kavyasree Kotagiri
@ 2022-04-07 11:02 ` Mark Brown
2022-04-07 11:23 ` Michael Walle
2022-04-07 12:05 ` Krzysztof Kozlowski
2022-04-07 12:54 ` Tudor.Ambarus
2 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2022-04-07 11:02 UTC (permalink / raw)
To: Kavyasree Kotagiri
Cc: robh+dt, krzk+dt, nicolas.ferre, alexandre.belloni,
claudiu.beznea, tudor.ambarus, linux-spi, devicetree,
linux-arm-kernel, linux-kernel, UNGLinuxDriver, Manohar.Puri
[-- Attachment #1: Type: text/plain, Size: 469 bytes --]
On Thu, Apr 07, 2022 at 04:24:20PM +0530, Kavyasree Kotagiri wrote:
> @@ -19,6 +19,7 @@ properties:
> - microchip,sam9x60-qspi
> - microchip,sama7g5-qspi
> - microchip,sama7g5-ospi
> + - microchip,lan966x-qspi
Generally DT compatibles should be for specific SoCs rather than having
wildcards in them, even if that means you have to list a lot of SoCs.
Having used wildcards in the past doesn't mean it's a good idea to
continue adding them!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: atmel,quadspi: Define lan966x QSPI
2022-04-07 11:02 ` Mark Brown
@ 2022-04-07 11:23 ` Michael Walle
2022-04-07 11:31 ` Mark Brown
0 siblings, 1 reply; 10+ messages in thread
From: Michael Walle @ 2022-04-07 11:23 UTC (permalink / raw)
To: broonie
Cc: Manohar.Puri, UNGLinuxDriver, alexandre.belloni, claudiu.beznea,
devicetree, kavyasree.kotagiri, krzk+dt, linux-arm-kernel,
linux-kernel, linux-spi, nicolas.ferre, robh+dt, tudor.ambarus,
Michael Walle
> > @@ -19,6 +19,7 @@ properties:
> > - microchip,sam9x60-qspi
> > - microchip,sama7g5-qspi
> > - microchip,sama7g5-ospi
> > + - microchip,lan966x-qspi
>
> Generally DT compatibles should be for specific SoCs rather than having
> wildcards in them, even if that means you have to list a lot of SoCs.
> Having used wildcards in the past doesn't mean it's a good idea to
> continue adding them!
The subject should also be prefixed with "dt-bindings: ".
Mark, I did a git log on
Documentation/devicetree/bindings/spi/atmel,quadspi.yaml and all the
subjects are without "dt-bindings:" although the original patch was with
that prefix [1]. Is that intended?
-michael
[1] https://lore.kernel.org/linux-devicetree/163962128492.2075495.3678727080606971257.b4-ty@kernel.org/
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: atmel,quadspi: Define lan966x QSPI
2022-04-07 11:23 ` Michael Walle
@ 2022-04-07 11:31 ` Mark Brown
2022-04-07 11:41 ` Michael Walle
0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2022-04-07 11:31 UTC (permalink / raw)
To: Michael Walle
Cc: Manohar.Puri, UNGLinuxDriver, alexandre.belloni, claudiu.beznea,
devicetree, kavyasree.kotagiri, krzk+dt, linux-arm-kernel,
linux-kernel, linux-spi, nicolas.ferre, robh+dt, tudor.ambarus
[-- Attachment #1: Type: text/plain, Size: 673 bytes --]
On Thu, Apr 07, 2022 at 01:23:45PM +0200, Michael Walle wrote:
> > > + - microchip,lan966x-qspi
> > Generally DT compatibles should be for specific SoCs rather than having
> > wildcards in them, even if that means you have to list a lot of SoCs.
> > Having used wildcards in the past doesn't mean it's a good idea to
> > continue adding them!
> The subject should also be prefixed with "dt-bindings: ".
I tend to complain about people doing that.
> Mark, I did a git log on
> Documentation/devicetree/bindings/spi/atmel,quadspi.yaml and all the
> subjects are without "dt-bindings:" although the original patch was with
> that prefix [1]. Is that intended?
Yes.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: atmel,quadspi: Define lan966x QSPI
2022-04-07 11:31 ` Mark Brown
@ 2022-04-07 11:41 ` Michael Walle
2022-04-07 12:04 ` Krzysztof Kozlowski
0 siblings, 1 reply; 10+ messages in thread
From: Michael Walle @ 2022-04-07 11:41 UTC (permalink / raw)
To: Mark Brown
Cc: Manohar.Puri, UNGLinuxDriver, alexandre.belloni, claudiu.beznea,
devicetree, kavyasree.kotagiri, krzk+dt, linux-arm-kernel,
linux-kernel, linux-spi, nicolas.ferre, robh+dt, tudor.ambarus
Am 2022-04-07 13:31, schrieb Mark Brown:
> On Thu, Apr 07, 2022 at 01:23:45PM +0200, Michael Walle wrote:
>> The subject should also be prefixed with "dt-bindings: ".
>
> I tend to complain about people doing that.
After all it is mentioned to use that prefix in
Documentation/devicetree/bindings/submitting-patches.rst. I try to
remember when submitting SPI related bindings.
-michael
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: atmel,quadspi: Define lan966x QSPI
2022-04-07 11:41 ` Michael Walle
@ 2022-04-07 12:04 ` Krzysztof Kozlowski
0 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-07 12:04 UTC (permalink / raw)
To: Michael Walle, Mark Brown
Cc: Manohar.Puri, UNGLinuxDriver, alexandre.belloni, claudiu.beznea,
devicetree, kavyasree.kotagiri, krzk+dt, linux-arm-kernel,
linux-kernel, linux-spi, nicolas.ferre, robh+dt, tudor.ambarus
On 07/04/2022 13:41, Michael Walle wrote:
> Am 2022-04-07 13:31, schrieb Mark Brown:
>> On Thu, Apr 07, 2022 at 01:23:45PM +0200, Michael Walle wrote:
>>> The subject should also be prefixed with "dt-bindings: ".
>>
>> I tend to complain about people doing that.
>
> After all it is mentioned to use that prefix in
> Documentation/devicetree/bindings/submitting-patches.rst. I try to
> remember when submitting SPI related bindings.
From my point of view, the dt-bindings prefix is still expected, just
after "spi:" (and other Marks' subsystems), because that's I am
filtering the bindings.
Your submissions had the prefix in wrong place, this one patch does not
have it all. :(
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: atmel,quadspi: Define lan966x QSPI
2022-04-07 10:54 [PATCH] spi: atmel,quadspi: Define lan966x QSPI Kavyasree Kotagiri
2022-04-07 11:02 ` Mark Brown
@ 2022-04-07 12:05 ` Krzysztof Kozlowski
2022-04-07 12:54 ` Tudor.Ambarus
2 siblings, 0 replies; 10+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-07 12:05 UTC (permalink / raw)
To: Kavyasree Kotagiri, broonie, robh+dt, krzk+dt
Cc: nicolas.ferre, alexandre.belloni, claudiu.beznea, tudor.ambarus,
linux-spi, devicetree, linux-arm-kernel, linux-kernel,
UNGLinuxDriver, Manohar.Puri
On 07/04/2022 12:54, Kavyasree Kotagiri wrote:
> LAN966x SoC supports 3 QSPI controllers. Each of them support
> data and clock frequency upto 100Mhz DDR and QUAD protocol.
>
> Signed-off-by: Kavyasree Kotagiri <kavyasree.kotagiri@microchip.com>
> ---
> Documentation/devicetree/bindings/spi/atmel,quadspi.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> index 1d493add4053..100d6e7f2748 100644
> --- a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> @@ -19,6 +19,7 @@ properties:
> - microchip,sam9x60-qspi
> - microchip,sama7g5-qspi
> - microchip,sama7g5-ospi
> + - microchip,lan966x-qspi
Expect the comment you got about wildcard, please also put it in
alphabetical order. As you can check, the other entries are ordered.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: atmel,quadspi: Define lan966x QSPI
2022-04-07 10:54 [PATCH] spi: atmel,quadspi: Define lan966x QSPI Kavyasree Kotagiri
2022-04-07 11:02 ` Mark Brown
2022-04-07 12:05 ` Krzysztof Kozlowski
@ 2022-04-07 12:54 ` Tudor.Ambarus
2022-04-08 11:52 ` Kavyasree.Kotagiri
2 siblings, 1 reply; 10+ messages in thread
From: Tudor.Ambarus @ 2022-04-07 12:54 UTC (permalink / raw)
To: Kavyasree.Kotagiri, broonie, robh+dt, krzk+dt
Cc: Nicolas.Ferre, alexandre.belloni, Claudiu.Beznea, linux-spi,
devicetree, linux-arm-kernel, linux-kernel, UNGLinuxDriver,
Manohar.Puri
On 4/7/22 13:54, Kavyasree Kotagiri wrote:
> LAN966x SoC supports 3 QSPI controllers. Each of them support
> data and clock frequency upto 100Mhz DDR and QUAD protocol.
How is this IP different than microchip,sama7g5-qspi? Does this speed
limitation come from the IP itself or from the board that you're using?
Neither of these instances support octal mode?
Cheers,
ta
>
> Signed-off-by: Kavyasree Kotagiri <kavyasree.kotagiri@microchip.com>
> ---
> Documentation/devicetree/bindings/spi/atmel,quadspi.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> index 1d493add4053..100d6e7f2748 100644
> --- a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> @@ -19,6 +19,7 @@ properties:
> - microchip,sam9x60-qspi
> - microchip,sama7g5-qspi
> - microchip,sama7g5-ospi
> + - microchip,lan966x-qspi
>
> reg:
> items:
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] spi: atmel,quadspi: Define lan966x QSPI
2022-04-07 12:54 ` Tudor.Ambarus
@ 2022-04-08 11:52 ` Kavyasree.Kotagiri
2022-04-11 14:46 ` Michael Walle
0 siblings, 1 reply; 10+ messages in thread
From: Kavyasree.Kotagiri @ 2022-04-08 11:52 UTC (permalink / raw)
To: Tudor.Ambarus, broonie, robh+dt, krzk+dt
Cc: Nicolas.Ferre, alexandre.belloni, Claudiu.Beznea, linux-spi,
devicetree, linux-arm-kernel, linux-kernel, UNGLinuxDriver,
Manohar.Puri
> > LAN966x SoC supports 3 QSPI controllers. Each of them support
> > data and clock frequency upto 100Mhz DDR and QUAD protocol.
>
> How is this IP different than microchip,sama7g5-qspi? Does this speed
> limitation come from the IP itself or from the board that you're using?
>
> Neither of these instances support octal mode?
>
Thanks for your comments. All the three instances support only QUAD protocol.
You are correct. There is no difference from sama7g5-qspi. Please ignore this patch. I will send next version of dt patches where I will use "microchip,sama7g5-qspi" for all my qspi nodes.
> Cheers,
> ta
>
> >
> > Signed-off-by: Kavyasree Kotagiri <kavyasree.kotagiri@microchip.com>
> > ---
> > Documentation/devicetree/bindings/spi/atmel,quadspi.yaml | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> > index 1d493add4053..100d6e7f2748 100644
> > --- a/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/atmel,quadspi.yaml
> > @@ -19,6 +19,7 @@ properties:
> > - microchip,sam9x60-qspi
> > - microchip,sama7g5-qspi
> > - microchip,sama7g5-ospi
> > + - microchip,lan966x-qspi
> >
> > reg:
> > items:
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] spi: atmel,quadspi: Define lan966x QSPI
2022-04-08 11:52 ` Kavyasree.Kotagiri
@ 2022-04-11 14:46 ` Michael Walle
0 siblings, 0 replies; 10+ messages in thread
From: Michael Walle @ 2022-04-11 14:46 UTC (permalink / raw)
To: kavyasree.kotagiri
Cc: Claudiu.Beznea, Manohar.Puri, Nicolas.Ferre, Tudor.Ambarus,
UNGLinuxDriver, alexandre.belloni, broonie, devicetree, krzk+dt,
linux-arm-kernel, linux-kernel, linux-spi, robh+dt, Michael Walle
> > > LAN966x SoC supports 3 QSPI controllers. Each of them support
> > > data and clock frequency upto 100Mhz DDR and QUAD protocol.
> >
> > How is this IP different than microchip,sama7g5-qspi? Does this speed
> > limitation come from the IP itself or from the board that you're using?
> >
> > Neither of these instances support octal mode?
> >
> Thanks for your comments. All the three instances support only QUAD
> protocol.
> You are correct. There is no difference from sama7g5-qspi. Please ignore
> this patch. I will send next version of dt patches where I will use
> "microchip,sama7g5-qspi" for all my qspi nodes.
Are you sure? There is a max frequency property in Tudor's sama7g5-qspi
driver (200/133MHz) which doesn't match neither the LAN9668 manual (which
states 150MHz on QSPI0 and 100MHZ on QSPI1, funny enough there is no
mention of QSPI2) nor does it match the max frequency set in the downstream
linux driver (24 MHz).
-michael
^ permalink raw reply [flat|nested] 10+ messages in thread