Re: [PATCH v3 1/3] arm64: dts: cn913x: add device trees for COM Express boards

Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed

* Re: [PATCH v3 1/3] arm64: dts: cn913x: add device trees for COM Express boards
       [not found] ` <20231026084735.3595944-2-enachman@marvell.com>
@ 2023-10-26 17:32   ` Andrew Lunn
  2023-10-27 11:12     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Lunn @ 2023-10-26 17:32 UTC (permalink / raw)
  To: Elad Nachman
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, gregory.clement,
	sebastian.hesselbarth, pali, mrkiko.rs, devicetree, linux-kernel,
	linux-arm-kernel, cyuval

> +#include "cn9131-db-comexpress.dtsi"
> +
> +/ {
> +	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier board";
> +	compatible = "marvell,ac5x_rd_carrier", "marvell,cn9131", "marvell,cn9130",
> +		     "marvell,armada-ap807-quad", "marvell,armada-ap807";

This is really a question to the DT Maintainers. This is a carrier
board for a standardised Com express type 7 board. In theory, you
should be able to plug any Com Express module into it, not just
Marvells. So should the compatible list just have a compatible for the
carrier itself? Not the module which would normally be mounted in it?

Should the carrier have a .dtsi file describing it? And then we have a
.dts file which combines the module .dtsi and the carrier .dtsi?

Sorry i did not ask this earlier, i was thinking more about SolidRuns
systems, which tend to have custom SOMs and customs carriers, so you
can only really mount one particular SOM into one particular
carrier. But that is not true here.

	 Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/3] arm64: dts: cn913x: add device trees for COM Express boards
  2023-10-26 17:32   ` [PATCH v3 1/3] arm64: dts: cn913x: add device trees for COM Express boards Andrew Lunn
@ 2023-10-27 11:12     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-27 11:12 UTC (permalink / raw)
  To: Andrew Lunn, Elad Nachman
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, gregory.clement,
	sebastian.hesselbarth, pali, mrkiko.rs, devicetree, linux-kernel,
	linux-arm-kernel, cyuval

On 26/10/2023 19:32, Andrew Lunn wrote:
>> +#include "cn9131-db-comexpress.dtsi"
>> +
>> +/ {
>> +	model = "Marvell Armada AC5X RD COM EXPRESS type 7 carrier board";
>> +	compatible = "marvell,ac5x_rd_carrier", "marvell,cn9131", "marvell,cn9130",

Except wrong naming for compatible, I really do not understand what you
want to add here. If AC5X is the carrier, what is the model name of
entire product? If AC5X is not the carrier, where it the carrier?

>> +		     "marvell,armada-ap807-quad", "marvell,armada-ap807";
> 
> This is really a question to the DT Maintainers. This is a carrier
> board for a standardised Com express type 7 board. In theory, you
> should be able to plug any Com Express module into it, not just
> Marvells. So should the compatible list just have a compatible for the
> carrier itself? Not the module which would normally be mounted in it?

Yes, because there are some common parts of the carrier board.

> 
> Should the carrier have a .dtsi file describing it? And then we have a
> .dts file which combines the module .dtsi and the carrier .dtsi?

Depends, how this is organized depends on possible re-usage etc. Usually
answer is: yes, carrier board should have DTSI.

> 
> Sorry i did not ask this earlier, i was thinking more about SolidRuns
> systems, which tend to have custom SOMs and customs carriers, so you
> can only really mount one particular SOM into one particular
> carrier. But that is not true here.
Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

[parent not found: <20231026084735.3595944-3-enachman@marvell.com>]

* Re: [PATCH v3 2/3] dt-bindings: arm64: dts: add dt-bindings for ac5x rd carrier
       [not found] ` <20231026084735.3595944-3-enachman@marvell.com>
@ 2023-10-26 14:08   ` Conor Dooley
  2023-10-26 17:44   ` Andrew Lunn
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Conor Dooley @ 2023-10-26 14:08 UTC (permalink / raw)
  To: Elad Nachman
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, andrew,
	gregory.clement, sebastian.hesselbarth, pali, mrkiko.rs,
	devicetree, linux-kernel, linux-arm-kernel, cyuval


[-- Attachment #1.1: Type: text/plain, Size: 1354 bytes --]

On Thu, Oct 26, 2023 at 11:47:34AM +0300, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add dt bindings for AC5X RD COM Express Type 7 carrier board.
> This board will Accept a CN9131 COM Express Type 7 CPU module.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> ---
>  .../devicetree/bindings/arm/marvell/armada-7k-8k.yaml     | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> index 52d78521e412..71bc94047d1b 100644
> --- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> @@ -60,4 +60,12 @@ properties:
>            - const: marvell,armada-ap807-quad
>            - const: marvell,armada-ap807
>  
> +      - description: AC5X RD COM Express Carrier for Armada CN9131 SoC with one external CP
> +        items:
> +          - const: marvell,ac5x_rd_carrier
> +          - const: marvell,cn9131
> +          - const: marvell,cn9130
> +          - const: marvell,armada-ap807-quad
> +          - const: marvell,armada-ap807
> +
>  additionalProperties: true
> -- 
> 2.25.1
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: arm64: dts: add dt-bindings for ac5x rd carrier
       [not found] ` <20231026084735.3595944-3-enachman@marvell.com>
  2023-10-26 14:08   ` [PATCH v3 2/3] dt-bindings: arm64: dts: add dt-bindings for ac5x rd carrier Conor Dooley
@ 2023-10-26 17:44   ` Andrew Lunn
  2023-10-27 11:09   ` Krzysztof Kozlowski
  2023-10-27 11:15   ` Krzysztof Kozlowski
  3 siblings, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2023-10-26 17:44 UTC (permalink / raw)
  To: Elad Nachman
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, gregory.clement,
	sebastian.hesselbarth, pali, mrkiko.rs, devicetree, linux-kernel,
	linux-arm-kernel, cyuval

On Thu, Oct 26, 2023 at 11:47:34AM +0300, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add dt bindings for AC5X RD COM Express Type 7 carrier board.
> This board will Accept a CN9131 COM Express Type 7 CPU module.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  .../devicetree/bindings/arm/marvell/armada-7k-8k.yaml     | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> index 52d78521e412..71bc94047d1b 100644
> --- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> @@ -60,4 +60,12 @@ properties:
>            - const: marvell,armada-ap807-quad
>            - const: marvell,armada-ap807
>  
> +      - description: AC5X RD COM Express Carrier for Armada CN9131 SoC with one external CP

This description is i think technically wrong. You have the
combination of the carrier and the module.

> +        items:
> +          - const: marvell,ac5x_rd_carrier
> +          - const: marvell,cn9131
> +          - const: marvell,cn9130
> +          - const: marvell,armada-ap807-quad
> +          - const: marvell,armada-ap807

  Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: arm64: dts: add dt-bindings for ac5x rd carrier
       [not found] ` <20231026084735.3595944-3-enachman@marvell.com>
  2023-10-26 14:08   ` [PATCH v3 2/3] dt-bindings: arm64: dts: add dt-bindings for ac5x rd carrier Conor Dooley
  2023-10-26 17:44   ` Andrew Lunn
@ 2023-10-27 11:09   ` Krzysztof Kozlowski
  2023-10-27 11:15   ` Krzysztof Kozlowski
  3 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-27 11:09 UTC (permalink / raw)
  To: Elad Nachman, robh+dt, krzysztof.kozlowski+dt, conor+dt, andrew,
	gregory.clement, sebastian.hesselbarth, pali, mrkiko.rs,
	devicetree, linux-kernel, linux-arm-kernel
  Cc: cyuval

On 26/10/2023 10:47, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add dt bindings for AC5X RD COM Express Type 7 carrier board.
> This board will Accept a CN9131 COM Express Type 7 CPU module.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>  .../devicetree/bindings/arm/marvell/armada-7k-8k.yaml     | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> index 52d78521e412..71bc94047d1b 100644
> --- a/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-7k-8k.yaml
> @@ -60,4 +60,12 @@ properties:
>            - const: marvell,armada-ap807-quad
>            - const: marvell,armada-ap807
>  
> +      - description: AC5X RD COM Express Carrier for Armada CN9131 SoC with one external CP
> +        items:
> +          - const: marvell,ac5x_rd_carrier

No underscores in compatibles. Do you see them anywhere? Please use
existing, recent code as start of your contributions, not something
buggy 5 years old.

Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 2/3] dt-bindings: arm64: dts: add dt-bindings for ac5x rd carrier
       [not found] ` <20231026084735.3595944-3-enachman@marvell.com>
                     ` (2 preceding siblings ...)
  2023-10-27 11:09   ` Krzysztof Kozlowski
@ 2023-10-27 11:15   ` Krzysztof Kozlowski
  3 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Kozlowski @ 2023-10-27 11:15 UTC (permalink / raw)
  To: Elad Nachman, robh+dt, krzysztof.kozlowski+dt, conor+dt, andrew,
	gregory.clement, sebastian.hesselbarth, pali, mrkiko.rs,
	devicetree, linux-kernel, linux-arm-kernel
  Cc: cyuval

On 26/10/2023 10:47, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Add dt bindings for AC5X RD COM Express Type 7 carrier board.
> This board will Accept a CN9131 COM Express Type 7 CPU module.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>

Please order your patches correctly, so bindings come before their
users. You cannot have other way.

> ---
>  .../devicetree/bindings/arm/marvell/armada-7k-8k.yaml     | 8 ++++++++
>  1 file changed, 8 insertions(+)
Best regards,
Krzysztof


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-10-27 11:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20231026084735.3595944-1-enachman@marvell.com>
     [not found] ` <20231026084735.3595944-2-enachman@marvell.com>
2023-10-26 17:32   ` [PATCH v3 1/3] arm64: dts: cn913x: add device trees for COM Express boards Andrew Lunn
2023-10-27 11:12     ` Krzysztof Kozlowski
     [not found] ` <20231026084735.3595944-3-enachman@marvell.com>
2023-10-26 14:08   ` [PATCH v3 2/3] dt-bindings: arm64: dts: add dt-bindings for ac5x rd carrier Conor Dooley
2023-10-26 17:44   ` Andrew Lunn
2023-10-27 11:09   ` Krzysztof Kozlowski
2023-10-27 11:15   ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox