Hi, czw., 11 lut 2021 o 15:19 Andrew Lunn <andrew@xxxxxxx> napisał(a): > > On Thu, Feb 11, 2021 at 08:22:19AM +0000, Stefan Chulski wrote: > > > > > > > > ---------------------------------------------------------------------- > > > From: <stefanc@xxxxxxxxxxx> > > > Date: Wed, 10 Feb 2021 11:48:17 +0200 > > > > > > > > > > > +static int bm_underrun_protect = 1; > > > > + > > > > +module_param(bm_underrun_protect, int, 0444); > > > > +MODULE_PARM_DESC(bm_underrun_protect, "Set BM underrun protect > > > > +feature (0-1), def=1"); > > > > > > No new module parameters, please. > > > > Ok, I would remove new module parameters. > > By the way why new module parameters forbitten? > > Historically, module parameters are a bad interface for > configuration. Vendors have stuffed all sorts of random junk into > module parameters. There is little documentation. Different drivers > can have similar looking module parameters which do different > things. Or different module parameters, which actually do the same > thing. But maybe with slightly different parameters. > > We get a much better overall result if you stop and think for a > while. How can this be made a generic configuration knob which > multiple vendors could use? And then add it to ethtool. Extend the > ethtool -h text and the man page. Maybe even hack some other vendors > driver to make use of it. > > Or we have also found out, that pushing back on parameters like this, > the developers goes back and looks at the code, and sometimes figures > out a way to automatically do the right thing, removing the > configuration knob, and just making it all simpler for the user to > use. I think of 2 alternatives: * `ethtool --set-priv-flags` - in such case there is a question if switching this particular feature in runtime is a good idea. * New DT/ACPI property - it is a hardware feature after all, so maybe let the user decide whether to enable it on the platform description level. What do you think? Best regards, Marcin
