Re: [PATCH] clk: imx: imx93: introduce clk_bypassed module parameter

[Greg KH]

On Thu, May 11, 2023 at 04:54:23PM +0800, Peng Fan wrote:
> 
> 
> On 5/10/2023 5:13 PM, Greg KH wrote:
> > Caution: This is an external email. Please take care when clicking links or opening attachments. When in doubt, report the message using the 'Report this email' button
> > 
> > 
> > On Wed, May 10, 2023 at 07:49:20AM +0000, Peng Fan wrote:
> > > 
> > > > Subject: Re: [PATCH] clk: imx: imx93: introduce clk_bypassed module
> > > > parameter
> > > > 
> > > > On Thu, May 04, 2023 at 04:55:06PM +0800, Peng Fan (OSS) wrote:
> > > > > From: Peng Fan <peng.fan@xxxxxxx>
> > > > > 
> > > > > With the clk names specified in clk_bypassed module parameter, give
> > > > > user an option to bypass the clk from managing them by Linux kernel.
> > > > 
> > > > As I said on another email, no, please do not add new module parameters
> > > > for drivers, this is not the 1990s
> > > 
> > > After a search of the list,
> > > https://lore.kernel.org/all/?q=module_param
> > > 
> > > I still see many drivers are adding module_param.
> > 
> > And they should not be doing so as it is almost always not a good idea
> > (note, some subsystems, like sound, do require it, as that's the api
> > they use, so this is not a blanket statement.)
> > 
> > > Is this is strict ban that new platform driver should not add
> > > module_param?
> > 
> > You need to really really really justify, and document in the changelog
> > text, why all of the other methods of configuring a platform driver will
> > not work in order to have it considered.
> 
> I just wanna use the module parateter to give user a choice to choose
> to bypass some clocks.

That is not what a module paramter is for, a "user" does not use them,
that would be required to be set at boot time.

> There are 100+ clocks in the driver. Different user
> may wanna different configuration. With device tree, it is
> not flexible.Such as user A may wanna bypass clock X, Y; user B may
> wanna bypass clock Z.

Device tree is supposed to be flexible to handle this.  If not, please
rework the way your driver handles the device tree information about the
clocks.

Again, module parameters is not the way to handle this issue, sorry.
Think about what would happen if all drivers were to attempt to have a
module parameter like this.  That would be unmaintainable.

> With module parameter, I could easily set it in bootargs.

Users do not set bootargs :)

> But anyway if this is not preferred, I need to find other way.

Use device tree please, that is what it is there for.

thanks,

greg k-h