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 <[email protected]>
---
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
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.
> -----Original Message-----
> From: Fred Lefranc <[email protected]>
> 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 <[email protected]>
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
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 <[email protected]> a écrit :
>
> > -----Original Message-----
> > From: Fred Lefranc <[email protected]>
> > 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 <[email protected]>
>
> 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
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 <[email protected]> a écrit :
>
> > -----Original Message-----
> > From: EVS Hardware Dpt <[email protected]>
> > 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 <[email protected]> a
> > écrit :
> > >
> > > > -----Original Message-----
> > > > From: Fred Lefranc <[email protected]>
> > > > 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 <[email protected]>
> > >
> > > 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
> -----Original Message-----
> From: EVS Hardware Dpt <[email protected]>
> 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 <[email protected]> a
> écrit :
> >
> > > -----Original Message-----
> > > From: Fred Lefranc <[email protected]>
> > > 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 <[email protected]>
> >
> > 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
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 <[email protected]> a ?crit :
> >
> > > -----Original Message-----
> > > From: EVS Hardware Dpt <[email protected]>
> > > 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 <[email protected]> a
> > > ?crit :
> > > >
> > > > > -----Original Message-----
> > > > > From: Fred Lefranc <[email protected]>
> > > > > 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 <[email protected]>
> > > >
> > > > 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
>