> Does this fix a user-visible problem? If so, what does it look like
> when the problem occurs?
No, I found the problem while debugging another issue and trying to
understand this piece of code and after looking into different versions of
PIIX4 datasheets. But I don't think it should prevent such a fix because
the code is straightforward and the spec is clear enough. If the existing
encoding was intentionally made like this by the code author due to the
inaccuracy of the spec, then it's very likely some code comments were
placed here. There are 2 possibilities:
- This fix breaks something. People should have to bisect the problem.
- This fix is valid. Some day people need PIIX4 mem quirks and they don't
have to run into a possibly well-hidden issue.
What do you think?
> I assume the other patches (2-4) are cleanups that don't fix any
> problems.
Definitely, as indicated by patch titles and commit messages.
Deng-Cheng
________________________________________
From: Bjorn Helgaas [bhelgaas@xxxxxxxxxx]
Sent: Friday, October 04, 2013 7:13 PM
To: DengCheng Zhu
Cc: linux-pci@xxxxxxxxxxxxxxx; James Hogan; Qais Yousef
Subject: Re: [PATCH v2 1/4] PCI/quirks: Fix PIIX4 memory base and size mask
On Fri, Oct 4, 2013 at 5:30 PM, Deng-Cheng Zhu <dengcheng.zhu@xxxxxxxxxx> wrote:
> From: Deng-Cheng Zhu <dengcheng.zhu@xxxxxxxxxx>
>
> According to Intel PIIX4 datasheet: The memory address consists of a 17-bit
> base address (AD[31:15]) and a 7-bit mask (AD[21:15]). This provides memory
> ranges from 32 Kbytes to 4 Mbytes in size.
>
> This is true for both devices 12 and 13. This patch fixes it.
Does this fix a user-visible problem? If so, what does it look like
when the problem occurs?
I assume the other patches (2-4) are cleanups that don't fix any problems.
> Ref 1: www.intel.com/assets/pdf/datasheet/290562.pdf
> April 1997 version 001 (p131 bottom, p226 top)
> Ref 2: www.intel.com/assets/pdf/specupdate/297738.pdf
> January 2002 version 017 (Nothing against the fix in the
> specification update was found.)
>
> Signed-off-by: Deng-Cheng Zhu <dengcheng.zhu@xxxxxxxxxx>
> Reviewed-by: James Hogan <james.hogan@xxxxxxxxxx>
> ---
> drivers/pci/quirks.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index f6c31fa..3e7e489 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -414,9 +414,9 @@ static void piix4_mem_quirk(struct pci_dev *dev, const char *name, unsigned int
> pci_read_config_dword(dev, port, &devres);
> if ((devres & enable) != enable)
> return;
> - base = devres & 0xffff0000;
> - mask = (devres & 0x3f) << 16;
> - size = 128 << 16;
> + base = devres & 0xffff8000;
> + mask = (devres & 0x7f) << 15;
> + size = 128 << 15;
> for (;;) {
> unsigned bit = size >> 1;
> if ((bit & mask) == bit)
> --
> 1.7.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
