[PATCH net-next 0/4] Initial support for p64h GEM

[PATCH net-next 0/4] Initial support for p64h GEM

[Charles Perry]

Hello,

This series add basic support for Microchip "PIC64-HPSC" and "PIC64HX"
(abbreviated "p64h") Ethernet endpoint. Both MPUs 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 p64h rely on
another standalone MDIO controller. For that reason, I've added a dt
binding rule to forbid phys from being added under the gem DT node. See
patch 2.

The maximum jumbo frame size also seems to be different on p64h (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 3 for more details.

p64h 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

Charles Perry (4):
  dt-bindings: net: cdns,macb: add a compatible for Microchip p64h
  dt-bindings: net: cdns,macb: forbid phy nodes for Microchip p64h
  net: macb: add safeguards for jumbo frame larger than 10240
  net: macb: add support for Microchip p64h ethernet endpoint

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

-- 
2.47.3

[PATCH net-next 1/4] dt-bindings: net: cdns,macb: add a compatible for Microchip p64h

[Charles Perry]

"p64h" is shorthand for "PIC64-HPSC" and "PIC64HX"

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

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

diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index cb14c35ba996..dff350302098 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -27,6 +27,7 @@ properties:
 
       - items:
           - enum:
+              - microchip,p64h-gem    # Microchip P64H SoC
               - xlnx,versal-gem       # Xilinx Versal
               - xlnx,zynq-gem         # Xilinx Zynq-7xxx SoC
               - xlnx,zynqmp-gem       # Xilinx Zynq Ultrascale+ MPSoC
-- 
2.47.3

[PATCH net-next 2/4] dt-bindings: net: cdns,macb: forbid phy nodes for Microchip p64h

[Charles Perry]

The GEM IPs within Microchip p64h have their MDIO controllers
unconnected from any physical pin.

When compiling a p64h device tree with a phy on a GEM node with
CHECK_DTBS=1, this generates an error like:

```
linux/arch/riscv/boot/dts/microchip/p64h-hb130x.dtb:
ethernet@40004180000 (microchip,p64h-gem): ethernet-phy@0: False
schema does not allow {'reg': [[0]]}
	from schema $id:
http://devicetree.org/schemas/net/cdns,macb.yaml#
```

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

diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
index dff350302098..be66cc9a42fd 100644
--- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
+++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
@@ -197,6 +197,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 3/4] 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 "jumbo_max_length" field (bits 0..13) of the DCFG2 register
can take a value of up to 16383 (0x3FFF). This field is not used
when determining the max MTU, instead an hardcoded value
(jumbo_max_len) 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 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 that
have their gem_jumbo_max_length set to 16383.

Signed-off-by: Charles Perry <charles.perry@microchip.com>
---
 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 c39a5a1e5732..cc48fd494458 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
@@ -2387,7 +2388,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,
@@ -5582,7 +5583,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 4/4] net: macb: add support for Microchip p64h ethernet endpoint

[Charles Perry]

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

p64h 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>
---
 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 cc48fd494458..d32a16e54214 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -5408,6 +5408,15 @@ static const struct macb_config raspberrypi_rp1_config = {
 	.jumbo_max_len = 10240,
 };
 
+static const struct macb_config p64h_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" },
@@ -5426,6 +5435,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,p64h-gem", .data = &p64h_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 2/4] dt-bindings: net: cdns,macb: forbid phy nodes for Microchip p64h

[Conor Dooley]

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

On Tue, Mar 03, 2026 at 10:03:16AM -0800, Charles Perry wrote:
> The GEM IPs within Microchip p64h have their MDIO controllers
> unconnected from any physical pin.
> 
> When compiling a p64h device tree with a phy on a GEM node with
> CHECK_DTBS=1, this generates an error like:
> 
> ```
> linux/arch/riscv/boot/dts/microchip/p64h-hb130x.dtb:
> ethernet@40004180000 (microchip,p64h-gem): ethernet-phy@0: False
> schema does not allow {'reg': [[0]]}
> 	from schema $id:
> http://devicetree.org/schemas/net/cdns,macb.yaml#
> ```

This should just be part of the patch adding the compatible. Adding it
incorrectly only to fix it up one patch later doesn't make sense.
Additionally, remove this information about the error adding this
produces, all you need here is the justification for it.

pw-bot: changes-requested

> 
> Signed-off-by: Charles Perry <charles.perry@microchip.com>
> ---
>  Documentation/devicetree/bindings/net/cdns,macb.yaml | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> index dff350302098..be66cc9a42fd 100644
> --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> @@ -197,6 +197,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: [PATCH net-next 1/4] dt-bindings: net: cdns,macb: add a compatible for Microchip p64h

[Conor Dooley]

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

On Tue, Mar 03, 2026 at 10:03:15AM -0800, Charles Perry wrote:
> "p64h" is shorthand for "PIC64-HPSC" and "PIC64HX"

No, sorry. If these are different SoCs they need to have SoC-specific
compatibles, particularly since PIC64HY could be something that is not
compatible with these devices. It'd be fine to add
"microchip,pic64hpsc-gem" with "microchip,pic64hx-gem" as a fallback
though, since they do appear to be very very very similar devices and
can clearly share the same match data in the driver. That's what pic64gx
and mpfs do.

pw-bot: changes-requested

Cheers,
Conor.

> 
> The generic compatible "cdns,gem" works but offers limited features.
> Keep it as a fallback.
> 
> Signed-off-by: Charles Perry <charles.perry@microchip.com>
> ---
>  Documentation/devicetree/bindings/net/cdns,macb.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> index cb14c35ba996..dff350302098 100644
> --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> @@ -27,6 +27,7 @@ properties:
>  
>        - items:
>            - enum:
> +              - microchip,p64h-gem    # Microchip P64H SoC
>                - xlnx,versal-gem       # Xilinx Versal
>                - xlnx,zynq-gem         # Xilinx Zynq-7xxx SoC
>                - xlnx,zynqmp-gem       # Xilinx Zynq Ultrascale+ MPSoC
> -- 
> 2.47.3
> 

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

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

[Charles Perry]

On Tue, Mar 03, 2026 at 06:18:48PM +0000, Conor Dooley wrote:
> On Tue, Mar 03, 2026 at 10:03:15AM -0800, Charles Perry wrote:
> > "p64h" is shorthand for "PIC64-HPSC" and "PIC64HX"
> 
> No, sorry. If these are different SoCs they need to have SoC-specific
> compatibles, particularly since PIC64HY could be something that is not
> compatible with these devices. It'd be fine to add
> "microchip,pic64hpsc-gem" with "microchip,pic64hx-gem" as a fallback
> though, since they do appear to be very very very similar devices and

Yes, "very very very similar" is the right term.

> can clearly share the same match data in the driver. That's what pic64gx
> and mpfs do.

Ok, no problem. Like this? :

```
      - items:
          - enum:
              - microchip,pic64hpsc-gem
              - microchip,pic64hx-gem
          - const: microchip,pic64h-gem
          - const: cdns,gem
```

Also, how important is it to use "pic64h" vs "p64h"?

Much of the downstream development uses "p64h" in compatibles, file names,
function names, etc. and it would create some overhead to rename
everything.

Although, as I think of it, it might not be a bad thing since it would
allow to quickly identify the mainstream from the downstream code.

Thank you,
Charles

> 
> pw-bot: changes-requested
> 
> Cheers,
> Conor.
> 
> > 
> > The generic compatible "cdns,gem" works but offers limited features.
> > Keep it as a fallback.
> > 
> > Signed-off-by: Charles Perry <charles.perry@microchip.com>
> > ---
> >  Documentation/devicetree/bindings/net/cdns,macb.yaml | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> > index cb14c35ba996..dff350302098 100644
> > --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> > +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> > @@ -27,6 +27,7 @@ properties:
> >  
> >        - items:
> >            - enum:
> > +              - microchip,p64h-gem    # Microchip P64H SoC
> >                - xlnx,versal-gem       # Xilinx Versal
> >                - xlnx,zynq-gem         # Xilinx Zynq-7xxx SoC
> >                - xlnx,zynqmp-gem       # Xilinx Zynq Ultrascale+ MPSoC
> > -- 
> > 2.47.3
> > 

Re: [PATCH net-next 2/4] dt-bindings: net: cdns,macb: forbid phy nodes for Microchip p64h

[Charles Perry]

On Tue, Mar 03, 2026 at 06:11:42PM +0000, Conor Dooley wrote:
> On Tue, Mar 03, 2026 at 10:03:16AM -0800, Charles Perry wrote:
> > The GEM IPs within Microchip p64h have their MDIO controllers
> > unconnected from any physical pin.
> > 
> > When compiling a p64h device tree with a phy on a GEM node with
> > CHECK_DTBS=1, this generates an error like:
> > 
> > ```
> > linux/arch/riscv/boot/dts/microchip/p64h-hb130x.dtb:
> > ethernet@40004180000 (microchip,p64h-gem): ethernet-phy@0: False
> > schema does not allow {'reg': [[0]]}
> > 	from schema $id:
> > http://devicetree.org/schemas/net/cdns,macb.yaml#
> > ```
> 
> This should just be part of the patch adding the compatible. Adding it
> incorrectly only to fix it up one patch later doesn't make sense.
> Additionally, remove this information about the error adding this
> produces, all you need here is the justification for it.
> 

Ok

Thanks,
Charles

> pw-bot: changes-requested
> 
> > 
> > Signed-off-by: Charles Perry <charles.perry@microchip.com>
> > ---
> >  Documentation/devicetree/bindings/net/cdns,macb.yaml | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/cdns,macb.yaml b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> > index dff350302098..be66cc9a42fd 100644
> > --- a/Documentation/devicetree/bindings/net/cdns,macb.yaml
> > +++ b/Documentation/devicetree/bindings/net/cdns,macb.yaml
> > @@ -197,6 +197,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
> > 

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

[Conor Dooley]

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

On Tue, Mar 03, 2026 at 10:54:01AM -0800, Charles Perry wrote:
> On Tue, Mar 03, 2026 at 06:18:48PM +0000, Conor Dooley wrote:
> > On Tue, Mar 03, 2026 at 10:03:15AM -0800, Charles Perry wrote:
> > > "p64h" is shorthand for "PIC64-HPSC" and "PIC64HX"
> > 
> > No, sorry. If these are different SoCs they need to have SoC-specific
> > compatibles, particularly since PIC64HY could be something that is not
> > compatible with these devices. It'd be fine to add
> > "microchip,pic64hpsc-gem" with "microchip,pic64hx-gem" as a fallback
> > though, since they do appear to be very very very similar devices and
> 
> Yes, "very very very similar" is the right term.

FWIW, if the only difference was binning or fusing, having the same
compatible for more than one device could be okay. But these are
actually like polarfire-soc and rt-polarfire-soc, where they look very
similar to software but the rt process means that they're physically
different, right?

> > can clearly share the same match data in the driver. That's what pic64gx
> > and mpfs do.
> 
> Ok, no problem. Like this? :
> 
> ```
>       - items:
>           - enum:
>               - microchip,pic64hpsc-gem
>               - microchip,pic64hx-gem
>           - const: microchip,pic64h-gem
>           - const: cdns,gem

I was thinking
- items:
    - const: microchip,pic64hx-gem
    - const: cdns,gem
- items:
    - const: microchip,pic64hpsc-gem
    - const: microchip,pic64hx-gem
    - const: cdns,gem

And getting rid of the "pic64h" that may end up not being sufficiently
common in the future.

> Also, how important is it to use "pic64h" vs "p64h"?

Oh I didn't even notice that tbh, I just have "pic64" muscle memory from
the pic64gx, and the pic32mzda also spells it out. My OCD says says "pic",
and the fact that all the marketing stuff uses "pic" kinda votes in that
favour too.

> Much of the downstream development uses "p64h" in compatibles, file names,
> function names, etc. and it would create some overhead to rename
> everything.

This is probably one of the easiest things to absorb from a
up/down stream perspective, you could (and probably will on other
submissions) get far more disruptive feedback than something that's
effectively cosmetic and can be solved by adding a single line to a
driver. To be honest, I don't care if you rename functions or filenames,
it's only devicetree related things that I personally care about.

> Although, as I think of it, it might not be a bad thing since it would
> allow to quickly identify the mainstream from the downstream code.

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

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

[Charles Perry]

On Tue, Mar 03, 2026 at 07:32:25PM +0000, Conor Dooley wrote:
> On Tue, Mar 03, 2026 at 10:54:01AM -0800, Charles Perry wrote:
> > On Tue, Mar 03, 2026 at 06:18:48PM +0000, Conor Dooley wrote:
> > > On Tue, Mar 03, 2026 at 10:03:15AM -0800, Charles Perry wrote:
> > > > "p64h" is shorthand for "PIC64-HPSC" and "PIC64HX"
> > > 
> > > No, sorry. If these are different SoCs they need to have SoC-specific
> > > compatibles, particularly since PIC64HY could be something that is not
> > > compatible with these devices. It'd be fine to add
> > > "microchip,pic64hpsc-gem" with "microchip,pic64hx-gem" as a fallback
> > > though, since they do appear to be very very very similar devices and
> > 
> > Yes, "very very very similar" is the right term.
> 
> FWIW, if the only difference was binning or fusing, having the same
> compatible for more than one device could be okay. But these are
> actually like polarfire-soc and rt-polarfire-soc, where they look very
> similar to software but the rt process means that they're physically
> different, right?
> 

I know they are functionally equivalent but some blocks are fused. I don't
know about manufacturing processes, sorry.

> > > can clearly share the same match data in the driver. That's what pic64gx
> > > and mpfs do.
> > 
> > Ok, no problem. Like this? :
> > 
> > ```
> >       - items:
> >           - enum:
> >               - microchip,pic64hpsc-gem
> >               - microchip,pic64hx-gem
> >           - const: microchip,pic64h-gem
> >           - const: cdns,gem
> 
> I was thinking
> - items:
>     - const: microchip,pic64hx-gem
>     - const: cdns,gem
> - items:
>     - const: microchip,pic64hpsc-gem
>     - const: microchip,pic64hx-gem
>     - const: cdns,gem
> 
> And getting rid of the "pic64h" that may end up not being sufficiently
> common in the future.
> 

Ok sounds good but I'll make "pic64hpsc" the favorite child and let
"pic64hx" fallback to "pic64hpsc".

> > Also, how important is it to use "pic64h" vs "p64h"?
> 
> Oh I didn't even notice that tbh, I just have "pic64" muscle memory from
> the pic64gx, and the pic32mzda also spells it out. My OCD says says "pic",
> and the fact that all the marketing stuff uses "pic" kinda votes in that
> favour too.
> 

Ok, lets use "pic".

> > Much of the downstream development uses "p64h" in compatibles, file names,
> > function names, etc. and it would create some overhead to rename
> > everything.
> 
> This is probably one of the easiest things to absorb from a
> up/down stream perspective, you could (and probably will on other
> submissions) get far more disruptive feedback than something that's
> effectively cosmetic and can be solved by adding a single line to a
> driver. To be honest, I don't care if you rename functions or filenames,
> it's only devicetree related things that I personally care about.
> 

Understood.

Thank you,
Charles

> > Although, as I think of it, it might not be a bad thing since it would
> > allow to quickly identify the mainstream from the downstream code.

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

[Simon Horman]

On Tue, Mar 03, 2026 at 10:03:17AM -0800, Charles Perry wrote:
> 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 "jumbo_max_length" field (bits 0..13) of the DCFG2 register
> can take a value of up to 16383 (0x3FFF). This field is not used
> when determining the max MTU, instead an hardcoded value
> (jumbo_max_len) 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 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 that
> have their gem_jumbo_max_length set to 16383.
> 
> Signed-off-by: Charles Perry <charles.perry@microchip.com>

Hi Charles,

I am sorry if this question is a bit naïve.

I understand the need to clamp the max_mtu to avoid overflowing RXBS.
And that this hasn't been an issue up until now due to the maximum
value of jumbo_max_len used in the driver.

But I'm unclear on the relationship between DCFG2 and the max_mtu.
Why does it need to be set to a value larger than that corresponding to
the maximum mtu and RX buf size?

...

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

[Charles Perry]

On Thu, Mar 05, 2026 at 11:40:10AM +0000, Simon Horman wrote:
> On Tue, Mar 03, 2026 at 10:03:17AM -0800, Charles Perry wrote:
> > 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 "jumbo_max_length" field (bits 0..13) of the DCFG2 register
> > can take a value of up to 16383 (0x3FFF). This field is not used
> > when determining the max MTU, instead an hardcoded value
> > (jumbo_max_len) 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 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 that
> > have their gem_jumbo_max_length set to 16383.
> > 
> > Signed-off-by: Charles Perry <charles.perry@microchip.com>
> 
> Hi Charles,
> 
> I am sorry if this question is a bit naïve.
> 
> I understand the need to clamp the max_mtu to avoid overflowing RXBS.
> And that this hasn't been an issue up until now due to the maximum
> value of jumbo_max_len used in the driver.
> 
> But I'm unclear on the relationship between DCFG2 and the max_mtu.
> Why does it need to be set to a value larger than that corresponding to
> the maximum mtu and RX buf size?
> 

Hello Simon,

The DCFG2 register is the max_mtu value, there's some public documentation
for this for AMD versal [1]. "gem_jumbo_max_length" is a define in the RTL
code, the hardware designer probably makes a tradeoff between gate count
and the max_mtu. The maximum value for this is 0x3FFF (16383).

The maximum buffer size is 255 * 64 = 16320

The GEM driver, in its current state, uses one buffer per frame, so the MTU
needs to be clamped at the maximum buffer size.

We could just set 16320 instead of 16383 into the "jumbo_max_len" of
"struct macb_config" but it would mix information about what the hardware
supports vs what the software support. My approach is to put what the
hardware support in "struct macb_config" and clamp it later when
calculating max_mtu because we know we have a software limitation.

Thanks,
Charles

[1]: https://docs.amd.com/r/en-US/am012-versal-register-reference/IP_Config2-GEM-Register

> ...

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

[Simon Horman]

On Thu, Mar 05, 2026 at 06:24:24AM -0800, Charles Perry wrote:
> On Thu, Mar 05, 2026 at 11:40:10AM +0000, Simon Horman wrote:
> > On Tue, Mar 03, 2026 at 10:03:17AM -0800, Charles Perry wrote:
> > > 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 "jumbo_max_length" field (bits 0..13) of the DCFG2 register
> > > can take a value of up to 16383 (0x3FFF). This field is not used
> > > when determining the max MTU, instead an hardcoded value
> > > (jumbo_max_len) 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 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 that
> > > have their gem_jumbo_max_length set to 16383.
> > > 
> > > Signed-off-by: Charles Perry <charles.perry@microchip.com>
> > 
> > Hi Charles,
> > 
> > I am sorry if this question is a bit naïve.
> > 
> > I understand the need to clamp the max_mtu to avoid overflowing RXBS.
> > And that this hasn't been an issue up until now due to the maximum
> > value of jumbo_max_len used in the driver.
> > 
> > But I'm unclear on the relationship between DCFG2 and the max_mtu.
> > Why does it need to be set to a value larger than that corresponding to
> > the maximum mtu and RX buf size?
> > 
> 
> Hello Simon,
> 
> The DCFG2 register is the max_mtu value, there's some public documentation
> for this for AMD versal [1]. "gem_jumbo_max_length" is a define in the RTL
> code, the hardware designer probably makes a tradeoff between gate count
> and the max_mtu. The maximum value for this is 0x3FFF (16383).
> 
> The maximum buffer size is 255 * 64 = 16320
> 
> The GEM driver, in its current state, uses one buffer per frame, so the MTU
> needs to be clamped at the maximum buffer size.
> 
> We could just set 16320 instead of 16383 into the "jumbo_max_len" of
> "struct macb_config" but it would mix information about what the hardware
> supports vs what the software support. My approach is to put what the
> hardware support in "struct macb_config" and clamp it later when
> calculating max_mtu because we know we have a software limitation.

Hi Charles,

Thanks for the explanation. I agree that it is best not to conflate
software and hardware support. And that the approach you have taken
here makes sense.

I do think it would be nice to add a bit more detail to the commit message,
along the lines of the text above. But I'll leave that call up to you.

Overall, this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next 4/4] net: macb: add support for Microchip p64h ethernet endpoint

[Simon Horman]

On Tue, Mar 03, 2026 at 10:03:18AM -0800, Charles Perry wrote:
> p64h doesn't have the USRIO register so MACB_CAPS_USRIO_DISABLED is
> used.
> 
> p64h 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>

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

[Charles Perry]

On Fri, Mar 06, 2026 at 01:04:26PM +0000, Simon Horman wrote:
> On Thu, Mar 05, 2026 at 06:24:24AM -0800, Charles Perry wrote:
> > On Thu, Mar 05, 2026 at 11:40:10AM +0000, Simon Horman wrote:
> > > On Tue, Mar 03, 2026 at 10:03:17AM -0800, Charles Perry wrote:
> > > > 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 "jumbo_max_length" field (bits 0..13) of the DCFG2 register
> > > > can take a value of up to 16383 (0x3FFF). This field is not used
> > > > when determining the max MTU, instead an hardcoded value
> > > > (jumbo_max_len) 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 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 that
> > > > have their gem_jumbo_max_length set to 16383.
> > > > 
> > > > Signed-off-by: Charles Perry <charles.perry@microchip.com>
> > > 
> > > Hi Charles,
> > > 
> > > I am sorry if this question is a bit naïve.
> > > 
> > > I understand the need to clamp the max_mtu to avoid overflowing RXBS.
> > > And that this hasn't been an issue up until now due to the maximum
> > > value of jumbo_max_len used in the driver.
> > > 
> > > But I'm unclear on the relationship between DCFG2 and the max_mtu.
> > > Why does it need to be set to a value larger than that corresponding to
> > > the maximum mtu and RX buf size?
> > > 
> > 
> > Hello Simon,
> > 
> > The DCFG2 register is the max_mtu value, there's some public documentation
> > for this for AMD versal [1]. "gem_jumbo_max_length" is a define in the RTL
> > code, the hardware designer probably makes a tradeoff between gate count
> > and the max_mtu. The maximum value for this is 0x3FFF (16383).
> > 
> > The maximum buffer size is 255 * 64 = 16320
> > 
> > The GEM driver, in its current state, uses one buffer per frame, so the MTU
> > needs to be clamped at the maximum buffer size.
> > 
> > We could just set 16320 instead of 16383 into the "jumbo_max_len" of
> > "struct macb_config" but it would mix information about what the hardware
> > supports vs what the software support. My approach is to put what the
> > hardware support in "struct macb_config" and clamp it later when
> > calculating max_mtu because we know we have a software limitation.
> 
> Hi Charles,
> 
> Thanks for the explanation. I agree that it is best not to conflate
> software and hardware support. And that the approach you have taken
> here makes sense.
> 
> I do think it would be nice to add a bit more detail to the commit message,
> along the lines of the text above. But I'll leave that call up to you.
> 

Ok, I'll clarify what is hardware specific vs software specific.

Thank you for the review,
Charles

> Overall, this looks good to me.
> 
> Reviewed-by: Simon Horman <horms@kernel.org>