From: Guenter Roeck <linux@roeck-us.net>
To: Bernhard Seibold <mail@bernhard-seibold.de>
Cc: linux-hwmon@vger.kernel.org
Subject: Re: [PATCH] hwmon: (nct6775) Support access via Asus WMI
Date: Thu, 6 May 2021 06:49:58 -0700 [thread overview]
Message-ID: <20210506134958.GA2267050@roeck-us.net> (raw)
In-Reply-To: <de9fba24-eb84-d967-268d-aa114c453f42@bernhard-seibold.de>
On Thu, May 06, 2021 at 09:02:01AM +0200, Bernhard Seibold wrote:
> On 06.05.2021 04:04, Guenter Roeck wrote:
> > On 5/5/21 1:12 PM, Bernhard Seibold wrote:
> >> Support accessing the chip via Asus WMI. This enables the driver to work
> >> on boards where the IO region is reserved by ACPI.
> >>
> >> Signed-off-by: Bernhard Seibold <mail@bernhard-seibold.de>
> >> Tested-by: Artem S. Tashkinov <aros@gmx.com>
> >
> > No, this makes the driver unmaintainable. There should be a separate
> > driver which only makes WMI/ACPI accesses for everything.
> >
> > Guenter
> >
>
> I'm not sure what exactly you are suggesting. I assume your suggestion
> isn't to duplicate 5000 lines of code in order to avoid having 100 lines
> of ifdef'ed code?
No idea, quite frankly. Sprinkling the driver with ifdefs and conditional
code for sure isn't the solution. You might consider dropping the ifdefs
and use dmi detection instead, possibly with a module parameter to bypass
the dmi detection. I'd rather have no ifdefs and miss some platforms where
this is untested.
You could try by splitting the single patch into several patches, with one
or more preparatory patches, and maybe write accessor functions to avoid
the runtime conditonals. Oh, and don't use C++ comments.
Guenter
next prev parent reply other threads:[~2021-05-06 13:50 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-05 20:12 [PATCH] hwmon: (nct6775) Support access via Asus WMI Bernhard Seibold
2021-05-06 2:04 ` Guenter Roeck
2021-05-06 7:02 ` Bernhard Seibold
2021-05-06 13:49 ` Guenter Roeck [this message]
2021-09-08 21:36 ` [PATCH 1/3] hwmon: (nct6775) Use sio_data in superio_* (v2) Denis Pauk
2021-09-08 21:36 ` [PATCH 2/3] hwmon: (nct6775) Use superio function pointers (v2) Denis Pauk
2021-09-09 16:53 ` Andy Shevchenko
2021-09-08 21:36 ` [PATCH 3/3] hwmon: (nct6775) Support access via Asus WMI (v2) Denis Pauk
2021-09-09 17:00 ` Guenter Roeck
2021-09-09 17:01 ` Andy Shevchenko
2021-09-09 16:50 ` [PATCH 1/3] hwmon: (nct6775) Use sio_data in superio_* (v2) Andy Shevchenko
2021-09-11 22:24 ` Denis Pauk
2021-09-12 14:43 ` Andy Shevchenko
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=20210506134958.GA2267050@roeck-us.net \
--to=linux@roeck-us.net \
--cc=linux-hwmon@vger.kernel.org \
--cc=mail@bernhard-seibold.de \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox