All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Woithe <jwoithe@just42.net>
To: Micha?? K??pie?? <kernel@kempniu.pl>
Cc: Darren Hart <dvhart@infradead.org>,
	Andy Shevchenko <andy@infradead.org>,
	platform-driver-x86@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 00/10] fujitsu-laptop: use device-specific data instead of module-wide globals
Date: Mon, 1 May 2017 22:35:36 +0930	[thread overview]
Message-ID: <20170501130536.GA25546@marvin.atrad.com.au> (raw)
In-Reply-To: <20170424133334.7064-1-kernel@kempniu.pl>

Hi Michael

On Mon, Apr 24, 2017 at 03:33:24PM +0200, Micha?? K??pie?? wrote:
> fujitsu-laptop registers two ACPI drivers.  Whenever an ACPI device with
> a matching identifier is found by the ACPI bus, a new instance of the
> relevant driver is bound to that ACPI device.  However, both ACPI
> drivers registered by fujitsu-laptop access module-wide global data
> structures, assuming neither ACPI driver will ever be instantiated more
> than once.  While there are currently no indications of such issues
> happening in the wild, it is theoretically possible for multiple
> FUJ02B1/FUJ02E3 ACPI devices to be present in the firmware, which would
> cause two instances of the relevant driver to simultaneously access
> module-wide globals without any locking in place.  Also, modern Fujitsu
> laptops ship without the FUJ02B1 ACPI device present in firmware,
> causing memory to be needlessly allocated inside fujitsu_init().
> 
> To future-proof the module and lay the groundwork for separating the two
> aforementioned ACPI drivers into separate modules, move away from
> module-wide global data structures by using device-specific data
> instead.

Apologies for the delay in getting this first set of feedback to you.  It's
a combination of the extent of the patch set and a very busy week.

This patch set represents another worthwhile clean up of the fujitsu-laptop
driver.  While I sincerely doubt any laptop vendor will place more than one
FUJ02B1 (or FUJ02E3) in a single machine, removing the dependency on global
variables makes the driver self contained and more consistent.  I have some
points of clarification which I will post as follow ups to the respective
patchs.

Regards
  jonathan

  parent reply	other threads:[~2017-05-01 13:06 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-24 13:33 [PATCH 00/10] fujitsu-laptop: use device-specific data instead of module-wide globals Michał Kępień
2017-04-24 13:33 ` [PATCH 01/10] platform/x86: fujitsu-laptop: introduce fext_*() helper functions Michał Kępień
2017-05-01 13:13   ` Jonathan Woithe
2017-05-02 13:24     ` Michał Kępień
2017-04-24 13:33 ` [PATCH 02/10] platform/x86: fujitsu-laptop: shorten names of acpi_handle fields Michał Kępień
2017-05-01 13:19   ` Jonathan Woithe
2017-05-01 16:09     ` Darren Hart
2017-04-24 13:33 ` [PATCH 03/10] platform/x86: fujitsu-laptop: explicitly pass ACPI handle to call_fext_func() Michał Kępień
2017-04-24 13:33 ` [PATCH 04/10] platform/x86: fujitsu-laptop: rework backlight power synchronization Michał Kępień
2017-05-01 13:32   ` Jonathan Woithe
2017-05-01 16:17     ` Darren Hart
2017-04-24 13:33 ` [PATCH 05/10] platform/x86: fujitsu-laptop: distinguish current uses of device-specific data Michał Kępień
2017-05-01 13:40   ` Jonathan Woithe
2017-04-24 13:33 ` [PATCH 06/10] platform/x86: fujitsu-laptop: allocate struct fujitsu_bl in acpi_fujitsu_bl_add() Michał Kępień
2017-04-24 13:33 ` [PATCH 07/10] platform/x86: fujitsu-laptop: use device-specific data in backlight code Michał Kępień
2017-04-24 13:33 ` [PATCH 08/10] platform/x86: fujitsu-laptop: allocate struct fujitsu_laptop in acpi_fujitsu_laptop_add() Michał Kępień
2017-04-24 13:33 ` [PATCH 09/10] platform/x86: fujitsu-laptop: use device-specific data in LED-related code Michał Kępień
2017-04-24 13:33 ` [PATCH 10/10] platform/x86: fujitsu-laptop: use device-specific data in remaining module code Michał Kępień
2017-05-01 13:05 ` Jonathan Woithe [this message]
2017-05-02 13:21   ` [PATCH 00/10] fujitsu-laptop: use device-specific data instead of module-wide globals Michał Kępień
2017-05-04 23:40     ` Jonathan Woithe
2017-05-05 16:15       ` Darren Hart
2017-05-06 12:31         ` Michał Kępień
2017-05-06 12:45           ` Michał Kępień
2017-05-06 14:21             ` Andy Shevchenko
2017-05-06 14:23               ` Andy Shevchenko
2017-05-08 16:01             ` Darren Hart
2017-05-09  9:35               ` Michał Kępień
2017-05-09 12:13                 ` Jonathan Woithe
2017-05-09 16:47                 ` Darren Hart
2017-05-09 21:24                   ` Rafael J. Wysocki
2017-05-11 13:52                     ` Michał Kępień
2017-05-11 14:37                       ` Rafael J. Wysocki
2017-05-11 15:38                         ` Darren Hart
2017-05-11 13:40                   ` Michał Kępień
2017-05-15 23:27                     ` Darren Hart
2017-05-16  0:06                       ` Jonathan Woithe
2017-05-16  6:40                         ` Michał Kępień
2017-05-15 23:56                     ` Jonathan Woithe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170501130536.GA25546@marvin.atrad.com.au \
    --to=jwoithe@just42.net \
    --cc=andy@infradead.org \
    --cc=dvhart@infradead.org \
    --cc=kernel@kempniu.pl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.