From: Guenter Roeck <guenter.roeck@ericsson.com>
To: Natarajan Gurumoorthy <natg@google.com>
Cc: Jean Delvare <khali@linux-fr.org>,
Wim Van Sebroeck <wim@iguana.be>,
Mike Waychison <mikew@google.com>,
"lm-sensors@lm-sensors.org" <lm-sensors@lm-sensors.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>
Subject: Re: [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers
Date: Fri, 15 Apr 2011 04:05:29 -0700 [thread overview]
Message-ID: <20110415110529.GA23594@ericsson.com> (raw)
In-Reply-To: <BANLkTi=464Q8z7GzDZw9Mb-j-L4O0bPRjg@mail.gmail.com>
On Fri, Apr 15, 2011 at 04:46:04AM -0400, 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;
>
At least for my part, I would not agree to that. If another driver misbehaves
and does not release the region, one of your CPU cores will be stuck
in an endless loop.
> outb(0x87, REG);
> outb(0x01, REG);
> outb(0x55, REG);
> outb(0x55, REG);
> }
>
> What I am suggesting is not returning an error and instead keep
> calling "request_muxed_region" till it succeeds. If superio_enter
> returns an error then we will have to rewrite a large chunk of
> it8712f_wdt to deal with it. There are 8-9 calls to superio_enter. We
> will have supreio_enter returning errors at awkward places in the
> driver where the current logic has no code to deal with errors.
>
Can't help it. I browsed through it earlier and didn't think it was that bad.
Just pass the error on to the next level until you can return it.
Guenter
> In case of properly written drivers the while loop will eventually
> exit. I agree this is ugly as sin but it limits the perturbation of
> the driver. Feedback please.
>
> Regards
> Nat
>
>
> > Thanks,
> > Guenter
> >
> >
>
>
>
> --
> Regards
> Nat Gurumoorthy AB6SJ
WARNING: multiple messages have this Message-ID (diff)
From: Guenter Roeck <guenter.roeck@ericsson.com>
To: Natarajan Gurumoorthy <natg@google.com>
Cc: Jean Delvare <khali@linux-fr.org>,
Wim Van Sebroeck <wim@iguana.be>,
Mike Waychison <mikew@google.com>,
"lm-sensors@lm-sensors.org" <lm-sensors@lm-sensors.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>
Subject: Re: [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87
Date: Fri, 15 Apr 2011 11:05:29 +0000 [thread overview]
Message-ID: <20110415110529.GA23594@ericsson.com> (raw)
In-Reply-To: <BANLkTi=464Q8z7GzDZw9Mb-j-L4O0bPRjg@mail.gmail.com>
On Fri, Apr 15, 2011 at 04:46:04AM -0400, 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;
>
At least for my part, I would not agree to that. If another driver misbehaves
and does not release the region, one of your CPU cores will be stuck
in an endless loop.
> outb(0x87, REG);
> outb(0x01, REG);
> outb(0x55, REG);
> outb(0x55, REG);
> }
>
> What I am suggesting is not returning an error and instead keep
> calling "request_muxed_region" till it succeeds. If superio_enter
> returns an error then we will have to rewrite a large chunk of
> it8712f_wdt to deal with it. There are 8-9 calls to superio_enter. We
> will have supreio_enter returning errors at awkward places in the
> driver where the current logic has no code to deal with errors.
>
Can't help it. I browsed through it earlier and didn't think it was that bad.
Just pass the error on to the next level until you can return it.
Guenter
> In case of properly written drivers the while loop will eventually
> exit. I agree this is ugly as sin but it limits the perturbation of
> the driver. Feedback please.
>
> Regards
> Nat
>
>
> > Thanks,
> > Guenter
> >
> >
>
>
>
> --
> Regards
> Nat Gurumoorthy AB6SJ
_______________________________________________
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-15 11:06 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 ` Guenter Roeck [this message]
2011-04-15 11:05 ` Guenter Roeck
2011-04-18 7:33 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 watchdog drivers Hans de Goede
2011-04-18 7:33 ` [lm-sensors] [PATCH v6 1/2] Use "request_muxed_region" in it87 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=20110415110529.GA23594@ericsson.com \
--to=guenter.roeck@ericsson.com \
--cc=khali@linux-fr.org \
--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.