[PATCH 09/15] net: hbl_en: add habanalabs Ethernet driver

[Omer Shpigelman]

On 6/19/24 11:01, Przemek Kitszel wrote:
> On 6/19/24 09:16, Omer Shpigelman wrote:
>> On 6/18/24 17:19, Andrew Lunn wrote:
> 
> [...]
> 
>>>>>> +module_param(poll_enable, bool, 0444);
>>>>>> +MODULE_PARM_DESC(poll_enable,
>>>>>> +              "Enable Rx polling rather than IRQ + NAPI (0 = no, 1 = yes, default: no)");
>>>>>
>>>>> Module parameters are not liked. This probably needs to go away.
>>>>>
>>>>
>>>> I see that various vendors under net/ethernet/* use module parameters.
>>>> Can't we add another one?
>>>
>>> Look at the history of those module parameters. Do you see many added
>>> in the last year? 5 years?
>>>
>>
>> I didn't check that prior to my submit. Regarding this "no new module
>> parameters allowed" rule, is that documented anywhere? if not, is that the
>> common practice? not to try to do something that was not done recently?
>> how "recently" is defined?
>> I just want to clarify this because it's hard to handle these submissions
>> when we write some code based on existing examples but then we are
>> rejected because "we don't do that here anymore".
>> I want to avoid future cases of this mismatch.
>>
> 
> best way is to read netdev ML, that way you will learn what interfaces
> are frowned upon and which are outright banned, sometimes you could
> judge yourself knowing which interfaces are most developed recently
> 
> in this module params example - they were introduced to allow init phase
> configuration of the device, that could not be postponed, what in the
> general case sounds like a workaround; hardest cases include huge swaths
> of (physical continuous) memory to be allocated, but for that there are
> now device tree binding solutions; more typical cases for networking are
> resolved via devlink reload
> 
> devlink parms are also the thing that should be used as a default for
> new parameters, the best if given parameter is not driver specific quirk
> 
> poll_enable sounds like something that should be a common param,
> but you have to better describe what you mean there
> (see napi_poll(), "Enable Rx polling" would mean to use that as default,
> do you mean busy polling or what?)

Yes, busy polling.
But never mind, I was informed that NAPI must be used so apparently I'll
need to anyway remove this polling mode and its module parameter.