* [PATCH] PCI: Add quirk for setting valid class for FSL PCI host bridge
@ 2013-07-26 7:23 Chunhe Lan
2013-07-26 13:07 ` Bjorn Helgaas
0 siblings, 1 reply; 4+ messages in thread
From: Chunhe Lan @ 2013-07-26 7:23 UTC (permalink / raw)
To: linux-pci; +Cc: bhelgaas, Chunhe Lan
Freescale platform has class code = 0x0b2000, when it boots. This makes
kernel PCI bus code to setup these devices resulting into the following
notice information when trying to enable them:
pci 0000:00:00.0: ignoring class 0x0b2000 (doesn't match header type 01)
This patch adds a ID specific(PCI_VENDOR_ID_FREESCALE & PCI_ANY_ID based)
'early' fixup quirk to replace class code with PCI_CLASS_BRIDGE_PCI as
class. Then the above information disappears.
Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/quirks.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index e85d230..906a04a3 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2772,6 +2772,16 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x342e, vtd_mask_spec_errors);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x3c28, vtd_mask_spec_errors);
#endif
+#ifdef CONFIG_FSL_PCI
+static void quirk_freescale_class(struct pci_dev *dev)
+{
+ dev_info(&dev->dev, "Setting PCI class for FSL PCI host bridge\n");
+ dev->class = (PCI_CLASS_BRIDGE_PCI << 8) | (dev->class & 0xff);
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
+ quirk_freescale_class);
+#endif
+
static void fixup_ti816x_class(struct pci_dev *dev)
{
/* TI 816x devices do not have class code set when in PCIe boot mode */
--
1.7.6.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: Add quirk for setting valid class for FSL PCI host bridge
2013-07-26 7:23 [PATCH] PCI: Add quirk for setting valid class for FSL PCI host bridge Chunhe Lan
@ 2013-07-26 13:07 ` Bjorn Helgaas
2013-07-26 13:09 ` Bjorn Helgaas
2013-07-26 14:01 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2013-07-26 13:07 UTC (permalink / raw)
To: Chunhe Lan
Cc: linux-pci, Benjamin Herrenschmidt, Paul Mackerras, Minghuan Lian,
Kumar Gala, Anton Vorontsov
[+cc Ben, Paul, Minghuan, Kumar, Anton, linuxppc-dev
On Fri, Jul 26, 2013 at 1:23 AM, Chunhe Lan <Chunhe.Lan@freescale.com> wrote:
> Freescale platform has class code = 0x0b2000, when it boots. This makes
> kernel PCI bus code to setup these devices resulting into the following
> notice information when trying to enable them:
>
> pci 0000:00:00.0: ignoring class 0x0b2000 (doesn't match header type 01)
>
> This patch adds a ID specific(PCI_VENDOR_ID_FREESCALE & PCI_ANY_ID based)
> 'early' fixup quirk to replace class code with PCI_CLASS_BRIDGE_PCI as
> class. Then the above information disappears.
>
> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/pci/quirks.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index e85d230..906a04a3 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2772,6 +2772,16 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x342e, vtd_mask_spec_errors);
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x3c28, vtd_mask_spec_errors);
> #endif
>
> +#ifdef CONFIG_FSL_PCI
> +static void quirk_freescale_class(struct pci_dev *dev)
> +{
> + dev_info(&dev->dev, "Setting PCI class for FSL PCI host bridge\n");
> + dev->class = (PCI_CLASS_BRIDGE_PCI << 8) | (dev->class & 0xff);
> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
> + quirk_freescale_class);
1. This looks an awful lot like quirk_fsl_pcie_head().
quirk_fsl_pcie_head() is currently a HEADER quirk, but that runs
*after* pci_setup_device(), where dev->class is used. I suspect this
should be an EARLY quirk instead. But this quirk has a long history
as a HEADER quirk, and I'm sure many people have tested it, so I don't
know what's going on.
2. I assume the Freescale vendor ID might be used for devices other
than bridges, so I doubt it would be correct to overwrite the class
for *every* Freescale device as you're doing here.
> +#endif
> +
> static void fixup_ti816x_class(struct pci_dev *dev)
> {
> /* TI 816x devices do not have class code set when in PCIe boot mode */
> --
> 1.7.6.5
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: Add quirk for setting valid class for FSL PCI host bridge
2013-07-26 13:07 ` Bjorn Helgaas
@ 2013-07-26 13:09 ` Bjorn Helgaas
2013-07-26 14:01 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2013-07-26 13:09 UTC (permalink / raw)
To: Chunhe Lan
Cc: linux-pci, Benjamin Herrenschmidt, Paul Mackerras, Minghuan Lian,
Kumar Gala
[-cc Anton (bounced)]
On Fri, Jul 26, 2013 at 7:07 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [+cc Ben, Paul, Minghuan, Kumar, Anton, linuxppc-dev
>
> On Fri, Jul 26, 2013 at 1:23 AM, Chunhe Lan <Chunhe.Lan@freescale.com> wrote:
>> Freescale platform has class code = 0x0b2000, when it boots. This makes
>> kernel PCI bus code to setup these devices resulting into the following
>> notice information when trying to enable them:
>>
>> pci 0000:00:00.0: ignoring class 0x0b2000 (doesn't match header type 01)
>>
>> This patch adds a ID specific(PCI_VENDOR_ID_FREESCALE & PCI_ANY_ID based)
>> 'early' fixup quirk to replace class code with PCI_CLASS_BRIDGE_PCI as
>> class. Then the above information disappears.
>>
>> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> ---
>> drivers/pci/quirks.c | 10 ++++++++++
>> 1 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index e85d230..906a04a3 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -2772,6 +2772,16 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x342e, vtd_mask_spec_errors);
>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x3c28, vtd_mask_spec_errors);
>> #endif
>>
>> +#ifdef CONFIG_FSL_PCI
>> +static void quirk_freescale_class(struct pci_dev *dev)
>> +{
>> + dev_info(&dev->dev, "Setting PCI class for FSL PCI host bridge\n");
>> + dev->class = (PCI_CLASS_BRIDGE_PCI << 8) | (dev->class & 0xff);
>> +}
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
>> + quirk_freescale_class);
>
> 1. This looks an awful lot like quirk_fsl_pcie_head().
> quirk_fsl_pcie_head() is currently a HEADER quirk, but that runs
> *after* pci_setup_device(), where dev->class is used. I suspect this
> should be an EARLY quirk instead. But this quirk has a long history
> as a HEADER quirk, and I'm sure many people have tested it, so I don't
> know what's going on.
>
> 2. I assume the Freescale vendor ID might be used for devices other
> than bridges, so I doubt it would be correct to overwrite the class
> for *every* Freescale device as you're doing here.
>
>> +#endif
>> +
>> static void fixup_ti816x_class(struct pci_dev *dev)
>> {
>> /* TI 816x devices do not have class code set when in PCIe boot mode */
>> --
>> 1.7.6.5
>>
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: Add quirk for setting valid class for FSL PCI host bridge
2013-07-26 13:07 ` Bjorn Helgaas
2013-07-26 13:09 ` Bjorn Helgaas
@ 2013-07-26 14:01 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2013-07-26 14:01 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Chunhe Lan, linux-pci, Paul Mackerras, Minghuan Lian, Kumar Gala,
Anton Vorontsov
On Fri, 2013-07-26 at 07:07 -0600, Bjorn Helgaas wrote:
>
> 1. This looks an awful lot like quirk_fsl_pcie_head().
> quirk_fsl_pcie_head() is currently a HEADER quirk, but that runs
> *after* pci_setup_device(), where dev->class is used. I suspect this
> should be an EARLY quirk instead. But this quirk has a long history
> as a HEADER quirk, and I'm sure many people have tested it, so I don't
> know what's going on.
>
> 2. I assume the Freescale vendor ID might be used for devices other
> than bridges, so I doubt it would be correct to overwrite the class
> for *every* Freescale device as you're doing here.
That wouldn't be the first time we need a quirk like that, it looks like
HW folks love playing with the class codes in stupid ways.
I remember once trying to change the FSP header quirk to an early quirk
and breaking something else doing so ... At some point I ran out of time
debugging that stuff and put it on my TODO list ... you know the file
you open in O_APPEND mode :-) That was a year ago or so...
So yes, something is fishy here and in need of a better fix, and at this
stage I'd say FSL folks should sort it out. These things need to be
early quirks and if that breaks something we need to know what and why.
Also we tend to have them in arch/powerpc next to the relevant driver
code... that quirks.c file is just a mess and certainly not the right
place for something like that.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-26 14:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-26 7:23 [PATCH] PCI: Add quirk for setting valid class for FSL PCI host bridge Chunhe Lan
2013-07-26 13:07 ` Bjorn Helgaas
2013-07-26 13:09 ` Bjorn Helgaas
2013-07-26 14:01 ` Benjamin Herrenschmidt
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.