Re: [PATCH v2 2/4] platform: x86: dell-smbios: Add a generic dell-smbios notifier chain

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday 27 October 2016 14:45:20 Hans de Goede wrote:
> HI,
> 
> On 27-10-16 12:32, Pali Rohár wrote:
> >On Monday 24 October 2016 15:45:02 Hans de Goede wrote:
> >>Hi,
> >>
> >>On 24-10-16 15:43, Pali Rohár wrote:
> >>>On Monday 24 October 2016 15:37:31 Hans de Goede wrote:
> >>>>Well WMI events get enabled via a SMBIOS call,
> >>>
> >>>This is truth only for few laptops and only for one WMI event.
> >>>Everything else is automatically enabled, no call is needed to issue.
> >>>
> >>>>and dell-laptop uses SMBIOS exclusively
> >>>
> >>>IIRC dell-led.ko uses also dell-smbio.ko, so it is not exclusive for
> >>>dell-laptop.
> >>>
> >>>>so it seems to fit. Basically this is a case of
> >>>>we have to put this somewhere and dell-smbios is the best fit IMHO.
> >>>
> >>>Agree, we need to put it somewhere...
> >>>
> >>>Basically we need to solve problem how (currently) 3 kernel modules can
> >>>communicate. Does not kernel support such "bus/event" mechanism for
> >>>this?
> >>
> >>Yes it does, that is exactly what notifiers are for, but we need to
> >>declare the bus somewhere. I still believe dell-smbios is the best
> >>place.
> >
> >But dell_smbios_register_notifier() name is totally confusing. It does
> >not register any notifier for SMBIOS. Nor it have nothing common with
> >SMBIOS API.
> >
> >Also there is absolutely no need that dell-rbtn.ko needs to depends on
> >dell-smbios.ko. dell-rbtn.ko is ACPI driver which does not use any of
> >SMBIOS API.
> >
> >Right now I'm not saying what is the best place for that notifier (as I
> >still do not have ideal candidate). I'm just saying that notifier is not
> >part of SMBIOS API and therefore dell-smbios.ko is not right place for
> >it.
> >
> >So currently we have these different APIs for dell notebook drivers:
> >* ACPI (used in dell-rbtn.ko and dell-smo8800.ko)
> >* WMI (used in dell-wmi.ko, dell-wmi-aio.ko, dell-led.ko)
> >* SMBIOS (used in dell-laptop.ko, dell-wmi.ko and dell-led.ko)
> >* some other platform code (used in dell-laptop.ko)
> >
> >And now notifier is needed for drivers:
> >* dell-laptop.ko
> >* dell-wmi.ko
> >* dell-rbtn.ko
> >
> >And if I look at above two sets, none of above drivers is good candidate
> >for central notifier functions... Maybe we should really introduce new
> >separate file where will central dell notifier live?
> 
> We could put this in a dell-common or some such. My main reason for
> going with dell-smbios is that dell-smbios is a "library" module,
> loading it does not do anything other then make symbols available
> for use by other modules. So using it to store the notifier is safe
> (no side effects even if e.g. only dell-rbtn gets loaded).

I understand your motivation.

> Given that we already have dell-smbios as dell library functions
> module, I think that adding a dell-common is a bit overkill.

New module is probably really overkill... But cannot we link add those
notifier functions statically? So it would not be new module.

> I can rename the notifier, maybe use dell_laptop*notifier as names,
> since dell-laptop is the main consumer of the notifications ?

Yes, this is name is better!

-- 
Pali Rohár
pali.rohar@xxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux