2019-12-05 11:46:06

by Xiongfeng Wang

Subject: [PATCH] PCI: Add quirk to disable unused BAR for hisilicon NP devices 5896

Add pci quirk for hisilicon PCI Network Processor devices 5896.
The size of the unused BAR3 is set as 265T wrongly. This patch disalbes
this bar.

Signed-off-by: Xiongfeng Wang <[email protected]>
---
drivers/pci/quirks.c | 29 +++++++++++++++++++++++++++++
include/linux/pci_ids.h | 1 +
2 files changed, 30 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4937a08..7dfb272 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5431,3 +5431,32 @@ static void quirk_reset_lenovo_thinkpad_p50_nvgpu(struct pci_dev *pdev)
DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1,
PCI_CLASS_DISPLAY_VGA, 8,
quirk_reset_lenovo_thinkpad_p50_nvgpu);
+
+static void quirk_hisi_fixup_np_class(struct pci_dev *pdev)
+{
+ u32 class = pdev->class;
+
+ pdev->class = PCI_BASE_CLASS_NETWORK << 8;
+ pci_info(pdev, "PCI class overriden (%#08x -> %#08x)\n",
+ class, pdev->class);
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HISI_5896,
+ quirk_hisi_fixup_np_class);
+
+/*
+ * Hisilicon NP devices 5896 BAR3 size is misreported as 256T. Actually, this
+ * BAR is unused, so let's disable it.
+ */
+#define HISI_5896_WRONG_BAR 3
+static void quirk_hisi_fixup_np_bar(struct pci_dev *pdev)
+{
+ struct resource *r = &pdev->resource[HISI_5896_WRONG_BAR];
+
+ r->start = 0;
+ r->end = 0;
+ r->flags = 0;
+
+ pci_info(pdev, "disable BAR %d\n", HISI_5896_WRONG_BAR);
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HISI_5896,
+ quirk_hisi_fixup_np_bar);
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 2302d133..56e2b91 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2558,6 +2558,7 @@
#define PCI_DEVICE_ID_KORENIX_JETCARDF3 0x17ff

#define PCI_VENDOR_ID_HUAWEI 0x19e5
+#define PCI_DEVICE_ID_HISI_5896 0x5896 /* Hisilicon NP devices 5896 */

#define PCI_VENDOR_ID_NETRONOME 0x19ee
#define PCI_DEVICE_ID_NETRONOME_NFP4000 0x4000
--
1.7.12.4

2019-12-05 12:16:16

by Andrew Murray

Subject: Re: [PATCH] PCI: Add quirk to disable unused BAR for hisilicon NP devices 5896

On Thu, Dec 05, 2019 at 07:40:41PM +0800, Xiongfeng Wang wrote:
> Add pci quirk for hisilicon PCI Network Processor devices 5896.

Can you capatalise HiSilicon correctly (capital H and S), both here and
in the subject line?

s/devices 5896/5896 devices/ ?

> The size of the unused BAR3 is set as 265T wrongly. This patch disalbes

s/disalbes/disables/

> this bar.
>
> Signed-off-by: Xiongfeng Wang <[email protected]>
> ---
> drivers/pci/quirks.c | 29 +++++++++++++++++++++++++++++
> include/linux/pci_ids.h | 1 +
> 2 files changed, 30 insertions(+)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 4937a08..7dfb272 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5431,3 +5431,32 @@ static void quirk_reset_lenovo_thinkpad_p50_nvgpu(struct pci_dev *pdev)
> DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1,
> PCI_CLASS_DISPLAY_VGA, 8,
> quirk_reset_lenovo_thinkpad_p50_nvgpu);
> +
> +static void quirk_hisi_fixup_np_class(struct pci_dev *pdev)
> +{
> + u32 class = pdev->class;
> +
> + pdev->class = PCI_BASE_CLASS_NETWORK << 8;
> + pci_info(pdev, "PCI class overriden (%#08x -> %#08x)\n",
> + class, pdev->class);

Why is this in here? This is completely unrelated to the commit message.

> +}
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HISI_5896,
> + quirk_hisi_fixup_np_class);
> +
> +/*
> + * Hisilicon NP devices 5896 BAR3 size is misreported as 256T. Actually, this
> + * BAR is unused, so let's disable it.

Does this mean that the existing driver doesn't use this BAR? Is a better fix
up the BAR to report the correct size?


> + */
> +#define HISI_5896_WRONG_BAR 3

I'd suggest this define is possibly not required.

> +static void quirk_hisi_fixup_np_bar(struct pci_dev *pdev)
> +{
> + struct resource *r = &pdev->resource[HISI_5896_WRONG_BAR];
> +
> + r->start = 0;
> + r->end = 0;
> + r->flags = 0;
> +
> + pci_info(pdev, "disable BAR %d\n", HISI_5896_WRONG_BAR);

It might be more helpful to describe why, e.g. "Disabling invalid BAR 3"
or similar.


> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HISI_5896,
> + quirk_hisi_fixup_np_bar);
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 2302d133..56e2b91 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2558,6 +2558,7 @@
> #define PCI_DEVICE_ID_KORENIX_JETCARDF3 0x17ff
>
> #define PCI_VENDOR_ID_HUAWEI 0x19e5
> +#define PCI_DEVICE_ID_HISI_5896 0x5896 /* Hisilicon NP devices 5896 */
>
> #define PCI_VENDOR_ID_NETRONOME 0x19ee
> #define PCI_DEVICE_ID_NETRONOME_NFP4000 0x4000

Thanks,

Andrew Murray

> --
> 1.7.12.4
>

2019-12-05 12:25:49

by Xiongfeng Wang

Subject: Re: [PATCH] PCI: Add quirk to disable unused BAR for hisilicon NP devices 5896

Hi Andrew，
Thanks for your advice. I will change them and send another version.

Thanks,
Xiongfeng

On 2019/12/5 20:15, Andrew Murray wrote:
> On Thu, Dec 05, 2019 at 07:40:41PM +0800, Xiongfeng Wang wrote:
>> Add pci quirk for hisilicon PCI Network Processor devices 5896.
>
> Can you capatalise HiSilicon correctly (capital H and S), both here and
> in the subject line?
>
> s/devices 5896/5896 devices/ ?
>
>> The size of the unused BAR3 is set as 265T wrongly. This patch disalbes
>
> s/disalbes/disables/
>
>> this bar.
>>
>> Signed-off-by: Xiongfeng Wang <[email protected]>
>> ---
>> drivers/pci/quirks.c | 29 +++++++++++++++++++++++++++++
>> include/linux/pci_ids.h | 1 +
>> 2 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 4937a08..7dfb272 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -5431,3 +5431,32 @@ static void quirk_reset_lenovo_thinkpad_p50_nvgpu(struct pci_dev *pdev)
>> DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_NVIDIA, 0x13b1,
>> PCI_CLASS_DISPLAY_VGA, 8,
>> quirk_reset_lenovo_thinkpad_p50_nvgpu);
>> +
>> +static void quirk_hisi_fixup_np_class(struct pci_dev *pdev)
>> +{
>> + u32 class = pdev->class;
>> +
>> + pdev->class = PCI_BASE_CLASS_NETWORK << 8;
>> + pci_info(pdev, "PCI class overriden (%#08x -> %#08x)\n",
>> + class, pdev->class);
>
> Why is this in here? This is completely unrelated to the commit message.
>
>> +}
>> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HISI_5896,
>> + quirk_hisi_fixup_np_class);
>> +
>> +/*
>> + * Hisilicon NP devices 5896 BAR3 size is misreported as 256T. Actually, this
>> + * BAR is unused, so let's disable it.
>
> Does this mean that the existing driver doesn't use this BAR? Is a better fix
> up the BAR to report the correct size?
>
>
>> + */
>> +#define HISI_5896_WRONG_BAR 3
>
> I'd suggest this define is possibly not required.
>
>> +static void quirk_hisi_fixup_np_bar(struct pci_dev *pdev)
>> +{
>> + struct resource *r = &pdev->resource[HISI_5896_WRONG_BAR];
>> +
>> + r->start = 0;
>> + r->end = 0;
>> + r->flags = 0;
>> +
>> + pci_info(pdev, "disable BAR %d\n", HISI_5896_WRONG_BAR);
>
> It might be more helpful to describe why, e.g. "Disabling invalid BAR 3"
> or similar.
>
>
>> +}
>> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, PCI_DEVICE_ID_HISI_5896,
>> + quirk_hisi_fixup_np_bar);
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 2302d133..56e2b91 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -2558,6 +2558,7 @@
>> #define PCI_DEVICE_ID_KORENIX_JETCARDF3 0x17ff
>>
>> #define PCI_VENDOR_ID_HUAWEI 0x19e5
>> +#define PCI_DEVICE_ID_HISI_5896 0x5896 /* Hisilicon NP devices 5896 */
>>
>> #define PCI_VENDOR_ID_NETRONOME 0x19ee
>> #define PCI_DEVICE_ID_NETRONOME_NFP4000 0x4000
>
> Thanks,
>
> Andrew Murray
>
>> --
>> 1.7.12.4
>>
>
> .
>