[PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz & rx_extra_headroom config from devicetree.

[PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz & rx_extra_headroom config from devicetree.

[Fred Lefranc]

Allow modification of two additional Frame Manager parameters :
- FM Max Frame Size : Can be changed to a value other than 1522
  (ie support Jumbo Frames)
- RX Extra Headroom

Signed-off-by: Fred Lefranc <hardware.evs@gmail.com>
---
 drivers/net/ethernet/freescale/fman/fman.c | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fman/fman.c b/drivers/net/ethernet/freescale/fman/fman.c
index 8f0db61cb1f6..bf4240eacf42 100644
--- a/drivers/net/ethernet/freescale/fman/fman.c
+++ b/drivers/net/ethernet/freescale/fman/fman.c
@@ -2862,6 +2862,32 @@ static struct fman *read_dts_node(struct platform_device *of_dev)
 		of_property_read_bool(fm_node, "fsl,erratum-a050385");
 #endif
 
+	/* Get Max Frame Size */
+	err = of_property_read_u32(fm_node, "fsl,max-frm-sz", &val);
+	if (!err) {
+		if (val > FSL_FM_MAX_POSSIBLE_FRAME_SIZE)
+			fsl_fm_max_frm = FSL_FM_MAX_POSSIBLE_FRAME_SIZE;
+		else if (val < FSL_FM_MIN_POSSIBLE_FRAME_SIZE)
+			fsl_fm_max_frm = FSL_FM_MIN_POSSIBLE_FRAME_SIZE;
+		else
+			fsl_fm_max_frm = (int)val;
+	}
+	dev_dbg(&of_dev->dev, "Configured Max Frame Size: %d\n",
+		fsl_fm_max_frm);
+
+	/* Get RX Extra Headroom Value */
+	err = of_property_read_u32(fm_node, "fsl,rx-extra-headroom", &val);
+	if (!err) {
+		if (val > FSL_FM_RX_EXTRA_HEADROOM_MAX)
+			fsl_fm_rx_extra_headroom = FSL_FM_RX_EXTRA_HEADROOM_MAX;
+		else if (val < FSL_FM_RX_EXTRA_HEADROOM_MIN)
+			fsl_fm_rx_extra_headroom = FSL_FM_RX_EXTRA_HEADROOM_MIN;
+		else
+			fsl_fm_rx_extra_headroom = (int)val;
+	}
+	dev_dbg(&of_dev->dev, "Configured RX Extra Headroom: %d\n",
+		fsl_fm_rx_extra_headroom);
+
 	return fman;
 
 fman_node_put:
-- 
2.25.1

[PATCH 2/2] net/fsl: fman dt binding: add fsl,max-frm-sz & fsl,rx-extra-headroom properties.

[Fred Lefranc]

Describes two additional parameters that could be optionally added
in devicetree.

Signed-off-by: Fred Lefranc <hardware.evs@gmail.com>
---
 .../devicetree/bindings/net/fsl-fman.txt      | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/fsl-fman.txt b/Documentation/devicetree/bindings/net/fsl-fman.txt
index 020337f3c05f..bcd0cf8ca9e9 100644
--- a/Documentation/devicetree/bindings/net/fsl-fman.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fman.txt
@@ -117,6 +117,26 @@ PROPERTIES
 		erratum A050385 which indicates that DMA transactions that are
 		split can result in a FMan lock.
 
+- fsl,max-frm-sz
+		Usage: optional
+		Value type: <u32>
+		Definition: Max frame size, across all interfaces.
+ 		Must be large enough to accommodate the network MTU, but small enough
+ 		to avoid wasting skb memory.
+		1522 by default.
+
+- fsl,rx-extra-headroom
+		Usage: optional
+		Value type: <u32>
+		Definition: Extra headroom for Rx buffers.
+ 		FMan is instructed to allocate, on the Rx path, this amount of
+		space at the beginning of a data buffer, beside the DPA private
+		data area and the IC fields.
+		Does not impact Tx buffer layout.
+		64 by default, it's needed on
+		particular forwarding scenarios that add extra headers to the
+		forwarded frame.
+
 =============================================================================
 FMan MURAM Node
 
-- 
2.25.1

Re: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz & rx_extra_headroom config from devicetree.

[Jakub Kicinski]

On Thu, 17 Feb 2022 17:05:26 +0100 Fred Lefranc wrote:
> Allow modification of two additional Frame Manager parameters :
> - FM Max Frame Size : Can be changed to a value other than 1522
>   (ie support Jumbo Frames)
> - RX Extra Headroom

This looks like pretty obvious config, not what device trees are for.

RE: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz & rx_extra_headroom config from devicetree.

[Madalin Bucur]

> -----Original Message-----
> From: Fred Lefranc <hardware.evs@gmail.com>
> Subject: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz &
> rx_extra_headroom config from devicetree.
> 
> Allow modification of two additional Frame Manager parameters :
> - FM Max Frame Size : Can be changed to a value other than 1522
>   (ie support Jumbo Frames)
> - RX Extra Headroom
> 
> Signed-off-by: Fred Lefranc <hardware.evs@gmail.com>

Hi, Fred,

there are module params already for both, look into

drivers/net/ethernet/freescale/fman/fman.c

for fsl_fm_rx_extra_headroom and fsl_fm_max_frm.

Regards,
Madalin

Re: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz & rx_extra_headroom config from devicetree.

[EVS Hardware Dpt]

Hi Madalin, Guys

I know, but it's somewhat difficult to use those parameters on kernel's
command line.
I don't think it's wrong to also add that in devicetree.
No removal, just an added feature.

For ethernet node in devicetree, there are a lot of configuration stuff like
max-frame-size to allow configuration of MTU
(and so potentially enable jumbo) and it's regarded as fine.

It's also the goal of this patch. Allow an easy configuration of
fsl_fm_max_frm from a dts. I added rx_extra_headroom for the sake of
completeness.

So I plead for this addition because I don't think it's wrong to do that and
I consider it's nicer to add an optional devicetree property rather than
adding a lot of obscure stuff on kernel's command line.

Hope you'll share my point of view.

Have a nice weekend Madalin, Guys,
Fred.

Le ven. 18 févr. 2022 à 08:23, Madalin Bucur <madalin.bucur@nxp.com> a écrit :
>
> > -----Original Message-----
> > From: Fred Lefranc <hardware.evs@gmail.com>
> > Subject: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz &
> > rx_extra_headroom config from devicetree.
> >
> > Allow modification of two additional Frame Manager parameters :
> > - FM Max Frame Size : Can be changed to a value other than 1522
> >   (ie support Jumbo Frames)
> > - RX Extra Headroom
> >
> > Signed-off-by: Fred Lefranc <hardware.evs@gmail.com>
>
> Hi, Fred,
>
> there are module params already for both, look into
>
> drivers/net/ethernet/freescale/fman/fman.c
>
> for fsl_fm_rx_extra_headroom and fsl_fm_max_frm.
>
> Regards,
> Madalin

RE: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz & rx_extra_headroom config from devicetree.

[Madalin Bucur]

> -----Original Message-----
> From: EVS Hardware Dpt <hardware.evs@gmail.com>
> Subject: Re: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz &
> rx_extra_headroom config from devicetree.
> 
> Hi Madalin, Guys
> 
> I know, but it's somewhat difficult to use those parameters on kernel's
> command line.
> I don't think it's wrong to also add that in devicetree.
> No removal, just an added feature.
> 
> For ethernet node in devicetree, there are a lot of configuration stuff
> like
> max-frame-size to allow configuration of MTU
> (and so potentially enable jumbo) and it's regarded as fine.
> 
> It's also the goal of this patch. Allow an easy configuration of
> fsl_fm_max_frm from a dts. I added rx_extra_headroom for the sake of
> completeness.
> 
> So I plead for this addition because I don't think it's wrong to do that
> and
> I consider it's nicer to add an optional devicetree property rather than
> adding a lot of obscure stuff on kernel's command line.
> 
> Hope you'll share my point of view.
> 
> Have a nice weekend Madalin, Guys,
> Fred.

Hi, Fred,

I understand your concerns in regards to usability but the device trees, as
explained earlier by Jakub, have a different role - they describe the HW,
rather than configure the SW on it. Removal of such config entries from the
device tree was one item on a long list to get the DPAA drivers upstreamed.

> Le ven. 18 févr. 2022 à 08:23, Madalin Bucur <madalin.bucur@nxp.com> a
> écrit :
> >
> > > -----Original Message-----
> > > From: Fred Lefranc <hardware.evs@gmail.com>
> > > Subject: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz &
> > > rx_extra_headroom config from devicetree.
> > >
> > > Allow modification of two additional Frame Manager parameters :
> > > - FM Max Frame Size : Can be changed to a value other than 1522
> > >   (ie support Jumbo Frames)
> > > - RX Extra Headroom
> > >
> > > Signed-off-by: Fred Lefranc <hardware.evs@gmail.com>
> >
> > Hi, Fred,
> >
> > there are module params already for both, look into
> >
> > drivers/net/ethernet/freescale/fman/fman.c
> >
> > for fsl_fm_rx_extra_headroom and fsl_fm_max_frm.
> >
> > Regards,
> > Madalin

Re: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz & rx_extra_headroom config from devicetree.

[EVS Hardware Dpt]

Hi Madalin, Guys,

I didn't have that historical part in mind. So, even if I still think there
are a lot of examples super close to what I'm proposing everywhere in
dts files, devicetree is out of equation.

Could I change the patchset to allow configuration of those two parameters
from config ? I won't remove configuration using module parameters,
just adding (what I think to be) an easier way of configuration.

What do you think?

Regards,
Fred.

Le ven. 18 févr. 2022 à 12:33, Madalin Bucur <madalin.bucur@nxp.com> a écrit :
>
> > -----Original Message-----
> > From: EVS Hardware Dpt <hardware.evs@gmail.com>
> > Subject: Re: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz &
> > rx_extra_headroom config from devicetree.
> >
> > Hi Madalin, Guys
> >
> > I know, but it's somewhat difficult to use those parameters on kernel's
> > command line.
> > I don't think it's wrong to also add that in devicetree.
> > No removal, just an added feature.
> >
> > For ethernet node in devicetree, there are a lot of configuration stuff
> > like
> > max-frame-size to allow configuration of MTU
> > (and so potentially enable jumbo) and it's regarded as fine.
> >
> > It's also the goal of this patch. Allow an easy configuration of
> > fsl_fm_max_frm from a dts. I added rx_extra_headroom for the sake of
> > completeness.
> >
> > So I plead for this addition because I don't think it's wrong to do that
> > and
> > I consider it's nicer to add an optional devicetree property rather than
> > adding a lot of obscure stuff on kernel's command line.
> >
> > Hope you'll share my point of view.
> >
> > Have a nice weekend Madalin, Guys,
> > Fred.
>
> Hi, Fred,
>
> I understand your concerns in regards to usability but the device trees, as
> explained earlier by Jakub, have a different role - they describe the HW,
> rather than configure the SW on it. Removal of such config entries from the
> device tree was one item on a long list to get the DPAA drivers upstreamed.
>
> > Le ven. 18 févr. 2022 à 08:23, Madalin Bucur <madalin.bucur@nxp.com> a
> > écrit :
> > >
> > > > -----Original Message-----
> > > > From: Fred Lefranc <hardware.evs@gmail.com>
> > > > Subject: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz &
> > > > rx_extra_headroom config from devicetree.
> > > >
> > > > Allow modification of two additional Frame Manager parameters :
> > > > - FM Max Frame Size : Can be changed to a value other than 1522
> > > >   (ie support Jumbo Frames)
> > > > - RX Extra Headroom
> > > >
> > > > Signed-off-by: Fred Lefranc <hardware.evs@gmail.com>
> > >
> > > Hi, Fred,
> > >
> > > there are module params already for both, look into
> > >
> > > drivers/net/ethernet/freescale/fman/fman.c
> > >
> > > for fsl_fm_rx_extra_headroom and fsl_fm_max_frm.
> > >
> > > Regards,
> > > Madalin

Re: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz & rx_extra_headroom config from devicetree.

[Rob Herring]

On Fri, Feb 18, 2022 at 01:28:00PM +0100, EVS Hardware Dpt wrote:
> Hi Madalin, Guys,

Please don't top post on maillists.

> 
> I didn't have that historical part in mind. So, even if I still think there
> are a lot of examples super close to what I'm proposing everywhere in
> dts files, devicetree is out of equation.
> 
> Could I change the patchset to allow configuration of those two parameters
> from config ? I won't remove configuration using module parameters,
> just adding (what I think to be) an easier way of configuration.
> 
> What do you think?

Config in DT is okay, but it depends on the type of config, who is 
doing the config, and when. Think of DT config like BIOS configuration, 
but without a UI to change it.

MTU configuration has been around forever and is common to lots of h/w. 
If we wanted to configure that in DT, it would already be a standard 
property.

Rob

> 
> Regards,
> Fred.
> 
> Le ven. 18 févr. 2022 à 12:33, Madalin Bucur <madalin.bucur@nxp.com> a écrit :
> >
> > > -----Original Message-----
> > > From: EVS Hardware Dpt <hardware.evs@gmail.com>
> > > Subject: Re: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz &
> > > rx_extra_headroom config from devicetree.
> > >
> > > Hi Madalin, Guys
> > >
> > > I know, but it's somewhat difficult to use those parameters on kernel's
> > > command line.
> > > I don't think it's wrong to also add that in devicetree.
> > > No removal, just an added feature.
> > >
> > > For ethernet node in devicetree, there are a lot of configuration stuff
> > > like
> > > max-frame-size to allow configuration of MTU
> > > (and so potentially enable jumbo) and it's regarded as fine.
> > >
> > > It's also the goal of this patch. Allow an easy configuration of
> > > fsl_fm_max_frm from a dts. I added rx_extra_headroom for the sake of
> > > completeness.
> > >
> > > So I plead for this addition because I don't think it's wrong to do that
> > > and
> > > I consider it's nicer to add an optional devicetree property rather than
> > > adding a lot of obscure stuff on kernel's command line.
> > >
> > > Hope you'll share my point of view.
> > >
> > > Have a nice weekend Madalin, Guys,
> > > Fred.
> >
> > Hi, Fred,
> >
> > I understand your concerns in regards to usability but the device trees, as
> > explained earlier by Jakub, have a different role - they describe the HW,
> > rather than configure the SW on it. Removal of such config entries from the
> > device tree was one item on a long list to get the DPAA drivers upstreamed.
> >
> > > Le ven. 18 févr. 2022 à 08:23, Madalin Bucur <madalin.bucur@nxp.com> a
> > > écrit :
> > > >
> > > > > -----Original Message-----
> > > > > From: Fred Lefranc <hardware.evs@gmail.com>
> > > > > Subject: [PATCH 1/2] net/fsl: fman: Allow fm_max_frame_sz &
> > > > > rx_extra_headroom config from devicetree.
> > > > >
> > > > > Allow modification of two additional Frame Manager parameters :
> > > > > - FM Max Frame Size : Can be changed to a value other than 1522
> > > > >   (ie support Jumbo Frames)
> > > > > - RX Extra Headroom
> > > > >
> > > > > Signed-off-by: Fred Lefranc <hardware.evs@gmail.com>
> > > >
> > > > Hi, Fred,
> > > >
> > > > there are module params already for both, look into
> > > >
> > > > drivers/net/ethernet/freescale/fman/fman.c
> > > >
> > > > for fsl_fm_rx_extra_headroom and fsl_fm_max_frm.
> > > >
> > > > Regards,
> > > > Madalin
>