linux-kernel - Re: [PATCH v7 00/14] platform/x86: alienware-wmi driver rework

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c6e10cdd-6fba-6e8b-0913-66766cb9248e@linux.intel.com>
Date: Mon, 3 Feb 2025 11:20:46 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Kurt Borja <kuurtb@...il.com>
cc: platform-driver-x86@...r.kernel.org, Armin Wolf <W_Armin@....de>, 
    Mario Limonciello <mario.limonciello@....com>, 
    Hans de Goede <hdegoede@...hat.com>, Dell.Client.Kernel@...l.com, 
    LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 00/14] platform/x86: alienware-wmi driver rework

On Mon, 3 Feb 2025, Kurt Borja wrote:

> Hi!
>
> I bring some last minute modifications.
> 
> I found commit
> 
> 	8d8fc146dd7a ("nvmem: core: switch to use device_add_groups()")
> 
> which states that it's unnecesary to call device_remove_groups() when
> the device is removed, so I dropped it to simplify things.

Hi Kurt,

> I also found commit
> 
> 	957961b6dcc8 ("hwmon: (oxp-sensors) Move tt_toggle attribute to dev_groups")
> 
> which states that no driver should add sysfs groups while probing the
> device as it races with userspace, so I re-added PROBE_FORCE_SYNCHRONOUS
> to the platform driver, so groups are added only after the device has
> finished probing.
>
> I'm not 100% sure that the second commit message applies here, but it is
> revd-by Greg K-H so I added it just in case.

Which is why .dev_groups should be used as it is able to avoid those 
races on driver core level.

Why you call device_add_groups() at all? Can't you just insert it into 
.dev_groups member in alienware_wmax_wmi_driver?

-- 
 i.

> Aside from that, I added .pprof to awcc_quirks because I'm going to add
> support for new features after this series, and it makes sense that
> force_platform_profile only forces the pprof and not other upcoming
> features.
> 
> ~ Kurt
> ---
> [02/14]
>   - In alienware_alienfx_setup() add a devm action to remove the created
>     platform device
>   - Drop device_remove_groups() in WMAX .remove callback
>   - Add PROBE_FORCE_SYNCHRONOUS to the platform driver
>   - Drop .remove callbacks on both WMI drivers
> 
> [03/14]
>   - Add awcc_platform_profile_init() to create the platform_profile
>     device on quirks->thermal == true condition
> 
> [07/14]
>   - Add .pprof to awcc_quirks
> 
> [10/14]
>   - Drop unused member `quirks` on `alienfx_priv` (remnant of another
>     version)
> 
> v6: https://lore.kernel.org/platform-driver-x86/20250127040406.17112-1-kuurtb@gmail.com/
> 
> Kurt Borja (14):
>   platform/x86: alienware-wmi: Add a state container for LED control
>     feature
>   platform/x86: alienware-wmi: Add WMI Drivers
>   platform/x86: alienware-wmi: Add a state container for thermal control
>     methods
>   platform/x86: alienware-wmi: Refactor LED control methods
>   platform/x86: alienware-wmi: Refactor hdmi, amplifier, deepslp methods
>   platform/x86: alienware-wmi: Refactor thermal control methods
>   platform/x86: alienware-wmi: Split DMI table
>   MAINTAINERS: Update ALIENWARE WMI DRIVER entry
>   platform/x86: Rename alienware-wmi.c
>   platform/x86: Add alienware-wmi.h
>   platform/x86: Split the alienware-wmi driver
>   platform/x86: dell: Modify Makefile alignment
>   platform/x86: Update alienware-wmi config entries
>   platform/x86: alienware-wmi: Update header and module information
> 
>  MAINTAINERS                                   |    4 +-
>  drivers/platform/x86/dell/Kconfig             |   30 +-
>  drivers/platform/x86/dell/Makefile            |   45 +-
>  .../platform/x86/dell/alienware-wmi-base.c    |  496 +++++++
>  .../platform/x86/dell/alienware-wmi-legacy.c  |   95 ++
>  .../platform/x86/dell/alienware-wmi-wmax.c    |  790 +++++++++++
>  drivers/platform/x86/dell/alienware-wmi.c     | 1249 -----------------
>  drivers/platform/x86/dell/alienware-wmi.h     |  101 ++
>  8 files changed, 1534 insertions(+), 1276 deletions(-)
>  create mode 100644 drivers/platform/x86/dell/alienware-wmi-base.c
>  create mode 100644 drivers/platform/x86/dell/alienware-wmi-legacy.c
>  create mode 100644 drivers/platform/x86/dell/alienware-wmi-wmax.c
>  delete mode 100644 drivers/platform/x86/dell/alienware-wmi.c
>  create mode 100644 drivers/platform/x86/dell/alienware-wmi.h
> 
> 
> base-commit: 05dbaf8dd8bf537d4b4eb3115ab42a5fb40ff1f5
>