Re: [PATCH v10 11/14] platform/x86: Split the alienware-wmi driver

[Andy Shevchenko]

On Fri, Feb 14, 2025 at 05:21:00PM -0500, Kurt Borja wrote:
> On Tue Feb 11, 2025 at 2:04 PM -05, Andy Shevchenko wrote:
> > On Tue, Feb 11, 2025 at 12:59:53PM -0500, Kurt Borja wrote:
> >> On Tue Feb 11, 2025 at 11:56 AM -05, Andy Shevchenko wrote:
> >> > On Fri, Feb 07, 2025 at 10:46:07AM -0500, Kurt Borja wrote:

...

> >> >> +		set_bit(profile, choices);
> >> >
> >> > Do you need it to be atomic?
> >> 
> >> I don't think so. `choices` belongs to this thread only.
> >
> > So, __set_bit() will suffice then.
> 
> For some reason I thought `set_bit` was the non-atomic one. This is good
> to know.

JFYI it is an old agreement that foo() is the locked (atomic) version of
__foo(), but this agreement is broken in many cases nowadays (in many drivers
:( unfortunately).

...

> >> >> +void __exit alienware_wmax_wmi_exit(void)
> >> >> +{
> >> >> +	wmi_driver_unregister(&alienware_wmax_wmi_driver);
> >> >> +}
> >> >
> >> > Why not moving these boilerplate to ->probe() and use module_wmi_driver()?
> >> 
> >> This 3 files are a single module and it has two WMI drivers so this
> >> can't be used.
> >
> > Can it be split to two separate modules then?
> 
> These two WMI drivers share a lot of features on old alienware models.
> Hence why I decided to link them together. IMO this bit of boilerplate
> is a fair tradeoff.

It can be split to three files, common parts, mod1, mod2.

-- 
With Best Regards,
Andy Shevchenko