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

[Rob Herring <robh@kernel.org>]

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
>