From: Lee Jones <lee.jones@linaro.org>
To: Prarit Bhargava <prarit@redhat.com>
Cc: linux-kernel@vger.kernel.org, Peter Tyser <ptyser@xes-inc.com>,
Samuel Ortiz <sameo@linux.intel.com>
Subject: Re: [PATCH] x86, lpc, Allow only one load of lpc_ich
Date: Wed, 3 Sep 2014 13:19:41 +0100 [thread overview]
Message-ID: <20140903121941.GF28985@lee--X1> (raw)
In-Reply-To: <540701B5.3020409@redhat.com>
On Wed, 03 Sep 2014, Prarit Bhargava wrote:
> On 09/03/2014 07:35 AM, Lee Jones wrote:
> >>>> This occurs because there are two LPC devices on the system:
> >>>>
> >>>> 00:1f.0 ISA bridge: Intel Corporation 82801JIB (ICH10) LPC Interface Controller
> >>>> 80:1f.0 ISA bridge: Intel Corporation 82801JIB (ICH10) LPC Interface Controller
> >>>>
> >>>> which AFAICT is a hardware configuration error that can be resolved in
> >>>> firmware by hiding the second LPC device. Having two of these results in
> >>>> two GPIO mappings and two Watchdog Timers which doesn't make much sense.
> >>>>
> >>>> An end user has no idea what the splats mean. We should inform the user that
> >>>> the issue lies with the hardware and that they should contact their vendor
> >>>> for resolution.
> >>>
> >>> Why is it a problem for 2 of these devices to exist on a single system?
> >>>
> >>> Shouldn't the driver just be able to handle 2 devices?
> >>>
>
> >>
> >> You end up with two watchdogs on the same system (and more confusingly they use
> >> the same global interface). Additionally you end up with two sets of GPIOs
> >> which also use the same global interface ... not good.
> >
> > I understand the problem with the _driver_, but why is it a problem
> > that two of these _devices_ exist on one system? Bailing out of the
> > second .probe() sounds hacky to me. The driver should know that this
> > is possible and act accordingly, or the second devices shouldn't be
> > registered.
> >
> > Can these devices be controlled seperately?
>
> Let's just try and address this for now ... They can be controlled separately
> but that's not the issue here.
>
> Consider just the watchdog timer (because it is easier to explain). The way the
> watchdog timer works is that we write to it every 30 seconds (or so ... it
> depends on your setup obviously). If we don't write to it within 30 seconds the
> system will panic and reboot.
>
> Now ... suppose you have two on a system. To what end? It doesn't make sense to
> have two. Either you can write from userspace or you can't. Now you have two
> running with different timeouts -- why? That doesn't make sense either. It
You only have 2 running if you start them both.
> isn't like the one with the longer timeout is ever going to cause a reboot.
>
> Does that explain things better? It's a not a real-world scenario.
On the h/w I'm currently working on, we have 3 Watchdogs.
> You also asked ...
>
> > Then why do they [have two devices specified]?
>
> Because the vendor didn't/forgot to hide one from the kernel in BIOS -- hence
> FW_BUG.
If only one is useful, why have the second one in the first place?
If the devices are present and we can see them, why not have 2? Some
users might find a use for them.
In the WARNING you submitted only sysfs was having a hard time.
Perhaps the real fix would be to allow the Watchdog and GPIO driver to
change their name when registering, so they can each have their own
sysfs entries.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
next prev parent reply other threads:[~2014-09-03 12:19 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-02 21:58 [PATCH] x86, lpc, Allow only one load of lpc_ich Prarit Bhargava
2014-09-03 7:43 ` Lee Jones
2014-09-03 10:13 ` Prarit Bhargava
2014-09-03 11:35 ` Lee Jones
2014-09-03 11:55 ` Prarit Bhargava
2014-09-03 12:19 ` Lee Jones [this message]
2014-09-03 12:23 ` Prarit Bhargava
2014-09-03 12:36 ` Lee Jones
2014-09-03 15:57 ` Guenter Roeck
2014-09-03 17:29 ` Peter Tyser
2014-09-03 17:56 ` Prarit Bhargava
2014-09-03 19:13 ` Peter Tyser
2014-09-03 20:08 ` Guenter Roeck
2014-09-03 19:22 ` Guenter Roeck
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=20140903121941.GF28985@lee--X1 \
--to=lee.jones@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=prarit@redhat.com \
--cc=ptyser@xes-inc.com \
--cc=sameo@linux.intel.com \
/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