All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andres Salomon <dilinger@queued.net>
To: Jordan Crouse <jordan.crouse@amd.com>
Cc: Jens Rottmann <JRottmann@LiPPERTEmbedded.de>,
	linux-geode@bombadil.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: MFGPT driver inhibits boot on some boards
Date: Thu, 31 Jul 2008 16:53:46 -0400	[thread overview]
Message-ID: <20080731165346.73901e9e@fred> (raw)
In-Reply-To: <20080731143635.GA12585@cosmic.amd.com>

On Thu, 31 Jul 2008 08:36:35 -0600
Jordan Crouse <jordan.crouse@amd.com> wrote:

> On 31/07/08 09:23 +0200, Jens Rottmann wrote:
> > The driver defaults to IRQ7. Our boards use this for the parallel
> > port. This alone would be ok, but the parallel port is on a LPC
> > SIO, so IRQ7 is routed to the LPC bus in MSR 51400025, so MFGPT
> > IRQs are not received.
> > 
> > Possible solution 1: make geode_mfgpt_set_irq() clear the needed
> > bit in MSR 51400025. This would break lp0, but at least Linux would
> > boot and users could cat /proc/interrupts and see what's going on.
> > 
> > Possible solution 2: make geode_mfgpt_set_irq() check the bit and
> > fail if the IRQ is routed to LPC. (I think I'd prefer this over 1.)
> 
> Either of these solutions is the "right" solution. I prefer
> automatically detecting the SIO interrupts and finding a free
> interrupt. 
> 

Except the broken lp0 would be a problem, no?


> > But that's not clean, even an IRQ not routed to LPC might be the
> > wrong one. The BIOS we're using (Insyde BIOS, uses VSA and 5
> > MFGPTs, but leaves 3 timers available) exports a PCI header for
> > MFGPT, which of course gets an IRQ assigned (LNKB-->IRQ11), and
> > this is the right IRQ to use. Some autodetection instead of
> > hardcoding IRQ7 would be perfect. But looking for the PCI header
> > won't work, because AMD removed them from the spec, most BIOSes
> > don't support them (any more) and some day our BIOS will hide them,
> > too.
> 
> I know the platform you are talking about - that was a special test
> case the MFGPT header.  It would have become standard, but then
> circumstances intervened, and we lost the resources to push that into
> the VSA and to all of our BIOS vendors.  It is easiest for us to
> assume that we won't have the PCI header for the MFGPT.
> 
> > I guess MSR 51400022 aka MSR_PIC_ZSEL_LOW is the key here.
> > geode_mfgpt_set_irq() overwrites this unconditionally, which I
> > think is bad. If the BIOS has already set an IRQ here, the driver
> > should keep it and assume it to be ok.
> 
> Agreed.
> 
> > Possible solution 3: keep the IRQ already chosen in MSR_PIC_ZSEL_LOW
> > unless it's 0, only in that case use IRQ7
> 
> This goes against our basic policy of not trusting the BIOS.
> 

At some level, we have to trust the BIOS (especially when its is
filling in MSRs for us).   I think I prefer #3.  At the same time,
Jordan has a better idea of how BIOSes out there might manage to
screw up the entries in PIC_ZSEL_LOW... 


[...]
> > IMHO hardcoding IRQ7 is bad. Yes, you can override it, but this
> > isn't some non-critical device driver like audio where only this
> > one device won't work. And MFGPT can't be compiled as module, so
> > initramfs can't do anything about it.
> > I think, if there is any chance to do it, Linux should be able to
> > boot without any parameters given - even if with reduced
> > functionality.
> 
> I agree 100%.

Ditto.

  reply	other threads:[~2008-07-31 20:54 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-31  7:23 MFGPT driver inhibits boot on some boards Jens Rottmann
2008-07-31 14:36 ` Jordan Crouse
2008-07-31 20:53   ` Andres Salomon [this message]
2008-07-31 20:57   ` pci resource conflicts Andres Salomon
2008-08-01 15:25   ` MFGPT driver inhibits boot on some boards Jens Rottmann
2008-08-01 16:00     ` Jordan Crouse
2008-08-04 12:40       ` [PATCH] geode-mfgpt: check IRQ before using MFGPT as clocksource Jens Rottmann
2008-08-15 15:11         ` Ingo Molnar
2008-08-15 15:29           ` Jordan Crouse
2008-08-15 15:53             ` Ingo Molnar

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=20080731165346.73901e9e@fred \
    --to=dilinger@queued.net \
    --cc=JRottmann@LiPPERTEmbedded.de \
    --cc=jordan.crouse@amd.com \
    --cc=linux-geode@bombadil.infradead.org \
    --cc=linux-kernel@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.