[PATCH net-next v2 0/3] Initial support for PIC64-HPSC/HX Ethernet endpoint

[PATCH net-next v2 0/3] Initial support for PIC64-HPSC/HX Ethernet endpoint

[Charles Perry]

Hello,

This series add basic support for Microchip "PIC64-HPSC" and "PIC64HX"
Ethernet endpoint. Both SoCs contain 4 GEM IP with support for
MII/RGMII/SGMII/USXGMII at rates of 10M to 10G. Only RGMII and SGMII at a
rate of 1G is tested for now. Each GEM IP has 8 priority queues and the
revision register reads 0x220c010e.

One particularity of this instantiation of GEM is that the MDIO controller
within the GEM IP is disconnected from any physical pin and the SoC rely on
another standalone MDIO controller.

The maximum jumbo frame size also seems to be different on PIC64-HPSC/HX
(16383) than what most other platforms use (10240). I've found that I need
to tweak a bit the MTU calculation for this, otherwise the RXBS field of
the DMACFG register overflows. See patch 2 for more details.

PIC64-HPSC/HX also supports other features guarded behind CAPS bit like
MACB_CAPS_QBV but I've omitted those intentionally because I didn't test
these.

Thanks,
Charles

Changes in v2:
  - Use separate compatibles for PIC64-HPSC and PIC64HX
  - "p64h" -> "pic64hpsc"
  - Merge patch 2 into patch 1

Charles Perry (3):
  dt-bindings: net: cdns,macb: add a compatible for Microchip pic64hpsc
  net: macb: add safeguards for jumbo frame larger than 10240
  net: macb: add support for Microchip pic64hpsc ethernet endpoint

 .../devicetree/bindings/net/cdns,macb.yaml    | 19 +++++++++++++++++++
 drivers/net/ethernet/cadence/macb_main.c      | 16 ++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

-- 
2.47.3

[PATCH net-next v2 1/3] dt-bindings: net: cdns,macb: add a compatible for Microchip pic64hpsc

[Charles Perry]

Add "microchip,pic64hpsc-gem" for "PIC64-HPSC" and
"microchip,pic64hx-gem" for "PIC64HX", compatible with the former.

The generic compatible "cdns,gem" works but offers limited features.
Keep it as a fallback.

The GEM IPs within pic64hpsc have their MDIO controllers
unconnected from any physical pin. Add a check to prevent adding PHYs
under the GEM node.

Signed-off-by: Charles Perry <charles.perry@microchip.com>
---
 .../devicetree/bindings/net/cdns,macb.yaml    | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index cb14c35ba996..b934abd98e20 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -70,6 +70,14 @@ properties:
               - microchip,sama7d65-gem   # Microchip SAMA7D65 gigabit ethernet interface
           - const: microchip,sama7g5-gem # Microchip SAMA7G5 gigabit ethernet interface
 
+      - items:
+          - const: microchip,pic64hpsc-gem # Microchip PIC64-HPSC
+          - const: cdns,gem
+      - items:
+          - const: microchip,pic64hx-gem   # Microchip PIC64HX
+          - const: microchip,pic64hpsc-gem # Microchip PIC64-HPSC
+          - const: cdns,gem
+
   reg:
     minItems: 1
     items:
@@ -196,6 +204,17 @@ allOf:
       required:
         - phys
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: microchip,p64h-gem
+    then:
+      patternProperties:
+        "^ethernet-phy@[0-9a-f]$": false
+      properties:
+        mdio: false
+
 unevaluatedProperties: false
 
 examples:
-- 
2.47.3

[PATCH net-next v2 2/3] net: macb: add safeguards for jumbo frame larger than 10240

[Charles Perry]

The RX buffers for GEM can have a maximum size of 16320 bytes
(0xff in the RXBS field of the DMACFG register means 255*64 =
16320 bytes).

The GEM IP has configurable maximum jumbo frame length that can go up to
16383. The actual value for this limit can be found in the
       "jumbo_max_length" field (bits 0..13) of the DCFG2 register.
Currently, the macb driver doesn't use the DCFG2 register when
determining the max MTU, instead an hardcoded value (jumbo_max_len in
struct macb_config) is used for each platform. Right now the maximum
value for jumbo_max_len is 10240 (0x2800).

GEM uses one buffer per packet which means that one buffer must allow
room for the max MTU plus L2 encapsulation and alignment. This is a
limitation of the driver.

This commit adds a limit to max_mtu and rx_buffer_size so that the RXBS
field can never overflow when a large MTU is used.

With this commit, it is now possible to add new platforms with a
jumbo_max_len of 16383 so that the hardware properties of each IP can be
properly captured in struct macb_config.

Signed-off-by: Charles Perry <charles.perry@microchip.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/ethernet/cadence/macb_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index dc661e5a0769..96e15f58e173 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -50,6 +50,7 @@ struct sifive_fu540_macb_mgmt {
 
 #define MACB_RX_BUFFER_SIZE	128
 #define RX_BUFFER_MULTIPLE	64  /* bytes */
+#define RX_BUFFER_MAX		(0xFF * RX_BUFFER_MULTIPLE) /* 16320 bytes */
 
 #define DEFAULT_RX_RING_SIZE	512 /* must be power of 2 */
 #define MIN_RX_RING_SIZE	64
@@ -2393,7 +2394,7 @@ static void macb_init_rx_buffer_size(struct macb *bp, size_t size)
 	if (!macb_is_gem(bp)) {
 		bp->rx_buffer_size = MACB_RX_BUFFER_SIZE;
 	} else {
-		bp->rx_buffer_size = size;
+		bp->rx_buffer_size = MIN(size, RX_BUFFER_MAX);
 
 		if (bp->rx_buffer_size % RX_BUFFER_MULTIPLE) {
 			netdev_dbg(bp->dev,
@@ -5588,7 +5589,8 @@ static int macb_probe(struct platform_device *pdev)
 	/* MTU range: 68 - 1518 or 10240 */
 	dev->min_mtu = GEM_MTU_MIN_SIZE;
 	if ((bp->caps & MACB_CAPS_JUMBO) && bp->jumbo_max_len)
-		dev->max_mtu = bp->jumbo_max_len - ETH_HLEN - ETH_FCS_LEN;
+		dev->max_mtu = MIN(bp->jumbo_max_len, RX_BUFFER_MAX) -
+				ETH_HLEN - ETH_FCS_LEN;
 	else
 		dev->max_mtu = 1536 - ETH_HLEN - ETH_FCS_LEN;
 
-- 
2.47.3

[PATCH net-next v2 3/3] net: macb: add support for Microchip pic64hpsc ethernet endpoint

[Charles Perry]

pic64hpsc doesn't have the USRIO register so MACB_CAPS_USRIO_DISABLED is
used.

pic64hpsc does support PTP and has the timestamping unit so
MACB_CAPS_GEM_HAS_PTP is used.

jumbo_max_len is set to 16383 (0x3FFF) as reported by the DCFG2 register
bits 0..13. The JML register also has a default value of 0x3FFF.

dma_burst_length is set to 16 because that's what most other platforms
use and it worked for me so far. There is one other mode where bursts of
up to 256 are allowed but this might impact negatively other masters on
the NOC.  The register default value is 4 (bursts up to 4).

Signed-off-by: Charles Perry <charles.perry@microchip.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
 drivers/net/ethernet/cadence/macb_main.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 96e15f58e173..d1fc6a9b9fe5 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5414,6 +5414,15 @@ static const struct macb_config raspberrypi_rp1_config = {
 	.jumbo_max_len = 10240,
 };
 
+static const struct macb_config pic64hpsc_config = {
+	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
+		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_USRIO_DISABLED,
+	.dma_burst_length = 16,
+	.clk_init = macb_clk_init,
+	.init = init_reset_optional,
+	.jumbo_max_len = 16383,
+};
+
 static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,at91sam9260-macb", .data = &at91sam9260_config },
 	{ .compatible = "cdns,macb" },
@@ -5432,6 +5441,7 @@ static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "cdns,zynq-gem", .data = &zynq_config }, /* deprecated */
 	{ .compatible = "sifive,fu540-c000-gem", .data = &fu540_c000_config },
 	{ .compatible = "microchip,mpfs-macb", .data = &mpfs_config },
+	{ .compatible = "microchip,pic64hpsc-gem", .data = &pic64hpsc_config},
 	{ .compatible = "microchip,sama7g5-gem", .data = &sama7g5_gem_config },
 	{ .compatible = "microchip,sama7g5-emac", .data = &sama7g5_emac_config },
 	{ .compatible = "mobileye,eyeq5-gem", .data = &eyeq5_config },
-- 
2.47.3

Re: [PATCH net-next v2 1/3] dt-bindings: net: cdns,macb: add a compatible for Microchip pic64hpsc

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1982 bytes --]

On Tue, Mar 10, 2026 at 02:22:00PM -0700, Charles Perry wrote:
> Add "microchip,pic64hpsc-gem" for "PIC64-HPSC" and
> "microchip,pic64hx-gem" for "PIC64HX", compatible with the former.
> 
> The generic compatible "cdns,gem" works but offers limited features.
> Keep it as a fallback.
> 
> The GEM IPs within pic64hpsc have their MDIO controllers
> unconnected from any physical pin. Add a check to prevent adding PHYs
> under the GEM node.
> 
> Signed-off-by: Charles Perry <charles.perry@microchip.com>

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

> ---
>  .../devicetree/bindings/net/cdns,macb.yaml    | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> index cb14c35ba996..b934abd98e20 100644
> --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> @@ -70,6 +70,14 @@ properties:
>                - microchip,sama7d65-gem   # Microchip SAMA7D65 gigabit ethernet interface
>            - const: microchip,sama7g5-gem # Microchip SAMA7G5 gigabit ethernet interface
>  
> +      - items:
> +          - const: microchip,pic64hpsc-gem # Microchip PIC64-HPSC
> +          - const: cdns,gem
> +      - items:
> +          - const: microchip,pic64hx-gem   # Microchip PIC64HX
> +          - const: microchip,pic64hpsc-gem # Microchip PIC64-HPSC
> +          - const: cdns,gem
> +
>    reg:
>      minItems: 1
>      items:
> @@ -196,6 +204,17 @@ allOf:
>        required:
>          - phys
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: microchip,p64h-gem
> +    then:
> +      patternProperties:
> +        "^ethernet-phy@[0-9a-f]$": false
> +      properties:
> +        mdio: false
> +
>  unevaluatedProperties: false
>  
>  examples:
> -- 
> 2.47.3
> 

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

Re: [net-next,v2,1/3] dt-bindings: net: cdns,macb: add a compatible for Microchip pic64hpsc

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email says "Not sure about the first one but the second one looks
legit"

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
dt-bindings: net: cdns,macb: add a compatible for Microchip pic64hpsc

This commit adds "microchip,pic64hpsc-gem" for "PIC64-HPSC" and
"microchip,pic64hx-gem" for "PIC64HX" compatible strings. It also adds a
conditional schema check to prevent adding PHYs under the GEM node for
hardware where MDIO controllers are unconnected from physical pins.

> diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> @@ -70,6 +70,14 @@
>                - microchip,sama7d65-gem   # Microchip SAMA7D65 gigabit ethernet interface
>            - const: microchip,sama7g5-gem # Microchip SAMA7G5 gigabit ethernet interface
>
> +      - items:
> +          - const: microchip,pic64hpsc-gem # Microchip PIC64-HPSC
> +          - const: cdns,gem
> +      - items:
> +          - const: microchip,pic64hx-gem   # Microchip PIC64HX
> +          - const: microchip,pic64hpsc-gem # Microchip PIC64-HPSC
> +          - const: cdns,gem

The fallback structure appears reversed from what was recommended in v1
review. Conor Dooley suggested that pic64hpsc should fallback to pic64hx:

  - items:
      - const: microchip,pic64hx-gem
      - const: cdns,gem
  - items:
      - const: microchip,pic64hpsc-gem
      - const: microchip,pic64hx-gem
      - const: cdns,gem

However, the current implementation has pic64hx falling back to pic64hpsc
instead. Was this reversal intentional, or should the order match the
reviewer's recommendation?

https://lore.kernel.org/netdev/20260303-emperor-childlike-6c43d8a91753@spud/

>
>    reg:
>      minItems: 1

[ ... ]

> @@ -196,6 +204,17 @@
>        required:
>          - phys
>
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: microchip,p64h-gem
> +    then:
> +      patternProperties:
> +        "^ethernet-phy@[0-9a-f]$": false
> +      properties:
> +        mdio: false

The conditional check references 'microchip,p64h-gem', but the compatible
strings actually added above are 'microchip,pic64hpsc-gem' and
'microchip,pic64hx-gem'. This causes the validation rule to never trigger
for the new hardware variants.

According to the commit message, the GEM IPs within pic64hpsc have their
MDIO controllers unconnected from physical pins, and this check should
prevent adding PHYs under the GEM node. However, device trees using
'microchip,pic64hpsc-gem' or 'microchip,pic64hx-gem' with invalid
MDIO/ethernet-phy configurations will silently pass schema validation.

Should this check use 'microchip,pic64hpsc-gem' instead?

This appears to be a remnant from v1 of the patch, where 'p64h' was used
as an abbreviation. Conor Dooley requested in v1 that the full 'pic'
spelling be used instead of the shortened form, and the compatible names
were updated accordingly, but this schema check was missed.

https://lore.kernel.org/netdev/20260303-emperor-childlike-6c43d8a91753@spud/

>
>  unevaluatedProperties: false
>
>  examples:
-- 
pw-bot: cr

Re: [net-next,v2,1/3] dt-bindings: net: cdns,macb: add a compatible for Microchip pic64hpsc

[Charles Perry]

On Thu, Mar 12, 2026 at 06:49:43PM -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email says "Not sure about the first one but the second one looks
> legit"
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> dt-bindings: net: cdns,macb: add a compatible for Microchip pic64hpsc
> 
> This commit adds "microchip,pic64hpsc-gem" for "PIC64-HPSC" and
> "microchip,pic64hx-gem" for "PIC64HX" compatible strings. It also adds a
> conditional schema check to prevent adding PHYs under the GEM node for
> hardware where MDIO controllers are unconnected from physical pins.
> 
> > diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> > --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> > +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> > @@ -70,6 +70,14 @@
> >                - microchip,sama7d65-gem   # Microchip SAMA7D65 gigabit ethernet interface
> >            - const: microchip,sama7g5-gem # Microchip SAMA7G5 gigabit ethernet interface
> >
> > +      - items:
> > +          - const: microchip,pic64hpsc-gem # Microchip PIC64-HPSC
> > +          - const: cdns,gem
> > +      - items:
> > +          - const: microchip,pic64hx-gem   # Microchip PIC64HX
> > +          - const: microchip,pic64hpsc-gem # Microchip PIC64-HPSC
> > +          - const: cdns,gem
> 
> The fallback structure appears reversed from what was recommended in v1
> review. Conor Dooley suggested that pic64hpsc should fallback to pic64hx:
> 
>   - items:
>       - const: microchip,pic64hx-gem
>       - const: cdns,gem
>   - items:
>       - const: microchip,pic64hpsc-gem
>       - const: microchip,pic64hx-gem
>       - const: cdns,gem
> 
> However, the current implementation has pic64hx falling back to pic64hpsc
> instead. Was this reversal intentional, or should the order match the
> reviewer's recommendation?
> 

Yes, this is intentional.

> https://lore.kernel.org/netdev/20260303-emperor-childlike-6c43d8a91753@spud/
> 
> >
> >    reg:
> >      minItems: 1
> 
> [ ... ]
> 
> > @@ -196,6 +204,17 @@
> >        required:
> >          - phys
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: microchip,p64h-gem
> > +    then:
> > +      patternProperties:
> > +        "^ethernet-phy@[0-9a-f]$": false
> > +      properties:
> > +        mdio: false
> 
> The conditional check references 'microchip,p64h-gem', but the compatible
> strings actually added above are 'microchip,pic64hpsc-gem' and
> 'microchip,pic64hx-gem'. This causes the validation rule to never trigger
> for the new hardware variants.
> 
> According to the commit message, the GEM IPs within pic64hpsc have their
> MDIO controllers unconnected from physical pins, and this check should
> prevent adding PHYs under the GEM node. However, device trees using
> 'microchip,pic64hpsc-gem' or 'microchip,pic64hx-gem' with invalid
> MDIO/ethernet-phy configurations will silently pass schema validation.
> 
> Should this check use 'microchip,pic64hpsc-gem' instead?
> 

Oops! Good catch! I'll send a v3.

Thanks,
Charles