From: Hans de Goede <hdegoede@redhat.com>
To: Natarajan Gurumoorthy <natg@google.com>
Cc: Guenter Roeck <guenter.roeck@ericsson.com>,
"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"lm-sensors@lm-sensors.org" <lm-sensors@lm-sensors.org>,
Wim Van Sebroeck <wim@iguana.be>,
Mike Waychison <mikew@google.com>
Subject: Re: [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers
Date: Mon, 18 Apr 2011 09:33:12 +0200 [thread overview]
Message-ID: <4DABE938.6020400@redhat.com> (raw)
In-Reply-To: <BANLkTi=464Q8z7GzDZw9Mb-j-L4O0bPRjg@mail.gmail.com>
Hi,
On 04/15/2011 10:46 AM, Natarajan Gurumoorthy wrote:
> On Thu, Apr 14, 2011 at 10:58 PM, Guenter Roeck
> <guenter.roeck@ericsson.com> wrote:
>> I have seen the list. I don't think you should fix everything in one go.
>> First step might be to get the w83697hf and it87 to work together, then
>> go from there.
>>
>> Is there a reason for loading (or trying to load) both the it87 and
>> the w83697hf driver at the same time ? Those drivers are usually only
>> loaded if the respective chip is known to exist. If there is no reason
>> to try loading both drivers, a simple workaround would be to not do it.
>>
> Guenter,
> I agree the above should never happen. The only way the 2
> drivers will be loaded at the same time is a misconfigured kernel
> where these 2 drivers get built and the rc scripts end up loading them
> too. If we are agreed that I suggest we make the superio_enter routine
> be the following:
>
> static inline void
> superio_enter(void)
> {
> /*
> * Reserve REG and REG + 1 for exclusive access.
> */
> while (!request_muxed_region(REG, 2, WATCHDOG_NAME))
> continue;
>
Natajaran,
Please stop being so stubborn. XXX people have told you not to put a
while loop here, but simply to check for an error and propagate
it (in all places where superio_enter gets called).
Your bogus argument of the driver then failing to load when another
non muxed superio driver loads is really far fetched. No system ever
has 2 super io chips. So as long as all ite drivers use the muxed
version all will be fine.
And if some unforeseen scenario happens where the request_muxed_region
fails, we will get a bug report about a driver failing to load (*). Which
is a lot better for the end user then being stuck in an endless loop.
Regards,
Hans
*) And deal with that then.
WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede@redhat.com>
To: Natarajan Gurumoorthy <natg@google.com>
Cc: Guenter Roeck <guenter.roeck@ericsson.com>,
"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"lm-sensors@lm-sensors.org" <lm-sensors@lm-sensors.org>,
Wim Van Sebroeck <wim@iguana.be>,
Mike Waychison <mikew@google.com>
Subject: Re: [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87
Date: Mon, 18 Apr 2011 07:33:12 +0000 [thread overview]
Message-ID: <4DABE938.6020400@redhat.com> (raw)
In-Reply-To: <BANLkTi=464Q8z7GzDZw9Mb-j-L4O0bPRjg@mail.gmail.com>
Hi,
On 04/15/2011 10:46 AM, Natarajan Gurumoorthy wrote:
> On Thu, Apr 14, 2011 at 10:58 PM, Guenter Roeck
> <guenter.roeck@ericsson.com> wrote:
>> I have seen the list. I don't think you should fix everything in one go.
>> First step might be to get the w83697hf and it87 to work together, then
>> go from there.
>>
>> Is there a reason for loading (or trying to load) both the it87 and
>> the w83697hf driver at the same time ? Those drivers are usually only
>> loaded if the respective chip is known to exist. If there is no reason
>> to try loading both drivers, a simple workaround would be to not do it.
>>
> Guenter,
> I agree the above should never happen. The only way the 2
> drivers will be loaded at the same time is a misconfigured kernel
> where these 2 drivers get built and the rc scripts end up loading them
> too. If we are agreed that I suggest we make the superio_enter routine
> be the following:
>
> static inline void
> superio_enter(void)
> {
> /*
> * Reserve REG and REG + 1 for exclusive access.
> */
> while (!request_muxed_region(REG, 2, WATCHDOG_NAME))
> continue;
>
Natajaran,
Please stop being so stubborn. XXX people have told you not to put a
while loop here, but simply to check for an error and propagate
it (in all places where superio_enter gets called).
Your bogus argument of the driver then failing to load when another
non muxed superio driver loads is really far fetched. No system ever
has 2 super io chips. So as long as all ite drivers use the muxed
version all will be fine.
And if some unforeseen scenario happens where the request_muxed_region
fails, we will get a bug report about a driver failing to load (*). Which
is a lot better for the end user then being stuck in an endless loop.
Regards,
Hans
*) And deal with that then.
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
next prev parent reply other threads:[~2011-04-18 7:33 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-14 21:36 [PATCH v6 0/2] Make all it87 drivers SMP safe Nat Gurumoorthy
2011-04-14 21:36 ` [lm-sensors] " Nat Gurumoorthy
2011-04-14 21:37 ` [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers Nat Gurumoorthy
2011-04-14 21:37 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 Nat Gurumoorthy
2011-04-14 22:49 ` [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers Guenter Roeck
2011-04-14 22:49 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 Guenter Roeck
2011-04-15 1:18 ` [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers Natarajan Gurumoorthy
2011-04-15 1:18 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 Natarajan Gurumoorthy
2011-04-15 2:40 ` [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers Guenter Roeck
2011-04-15 2:40 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 Guenter Roeck
2011-04-15 3:14 ` [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers Natarajan Gurumoorthy
2011-04-15 3:14 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 Natarajan Gurumoorthy
2011-04-15 5:58 ` [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers Guenter Roeck
2011-04-15 5:58 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 Guenter Roeck
2011-04-15 8:46 ` [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers Natarajan Gurumoorthy
2011-04-15 8:46 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 Natarajan Gurumoorthy
2011-04-15 11:05 ` [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers Guenter Roeck
2011-04-15 11:05 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 Guenter Roeck
2011-04-18 7:33 ` Hans de Goede [this message]
2011-04-18 7:33 ` Hans de Goede
2011-04-14 21:38 ` [PATCH v6 2/2] Use "request_muxed_region" in it87 hwmon drivers Nat Gurumoorthy
2011-04-14 21:38 ` [lm-sensors] [PATCH v6 2/2] Use "request_muxed_region" in it87 Nat Gurumoorthy
2011-04-15 21:07 ` [PATCH v6 0/2] Make all it87 drivers SMP safe Jarod Wilson
2011-04-15 21:07 ` [lm-sensors] " Jarod Wilson
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=4DABE938.6020400@redhat.com \
--to=hdegoede@redhat.com \
--cc=guenter.roeck@ericsson.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=lm-sensors@lm-sensors.org \
--cc=mikew@google.com \
--cc=natg@google.com \
--cc=wim@iguana.be \
/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.