* [PATCH v3] PCI: loongson: skip scanning unavailable child device
@ 2022-11-04 10:53 Liu Peibao
2022-11-07 21:15 ` Bjorn Helgaas
0 siblings, 1 reply; 3+ messages in thread
From: Liu Peibao @ 2022-11-04 10:53 UTC (permalink / raw)
To: Bjorn Helgaas, Rob Herring, Krzysztof Kozlowski,
Lorenzo Pieralisi, Krzysztof Wilczyński, Jiaxun Yang
Cc: Huacai Chen, Jianmin Lv, Yinbo Zhu, Liu Peibao, linux-pci, linux-kernel
The PCI Controller of 2k1000 could not mask devices by
setting vender id or device id in configuration space header
as invalid values. When there are pins shareble between
the platform device and PCI device, if the platform device
is preferred, we should not scan this PCI device. In the
above scene, add `status = "disabled"` property in DT node
of this PCI device.
Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
---
V2 -> V3: 1. use list_for_each_entry() for more clearly.
2. fix wrong use of sizeof().
V1 -> V2: use existing property "status" instead of adding new property.
drivers/pci/controller/pci-loongson.c | 55 +++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
index 05c50408f13b..c7dd88eac885 100644
--- a/drivers/pci/controller/pci-loongson.c
+++ b/drivers/pci/controller/pci-loongson.c
@@ -40,11 +40,21 @@ struct loongson_pci_data {
struct pci_ops *ops;
};
+#ifdef CONFIG_OF
+struct mask_entry {
+ struct list_head entry;
+ unsigned int devfn;
+};
+#endif
+
struct loongson_pci {
void __iomem *cfg0_base;
void __iomem *cfg1_base;
struct platform_device *pdev;
const struct loongson_pci_data *data;
+#ifdef CONFIG_OF
+ struct list_head masklist;
+#endif
};
/* Fixup wrong class code in PCIe bridges */
@@ -194,6 +204,18 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus,
return NULL;
}
+#ifdef CONFIG_OF
+ /* Don't access devices in masklist */
+ if (pci_is_root_bus(bus)) {
+ struct mask_entry *entry;
+
+ list_for_each_entry(entry, &priv->masklist, entry) {
+ if (devfn == entry->devfn)
+ return NULL;
+ }
+ }
+#endif
+
/* CFG0 can only access standard space */
if (where < PCI_CFG_SPACE_SIZE && priv->cfg0_base)
return cfg0_map(priv, bus, devfn, where);
@@ -206,6 +228,36 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus,
}
#ifdef CONFIG_OF
+static int setup_masklist(struct loongson_pci *priv)
+{
+ struct device *dev = &priv->pdev->dev;
+ struct device_node *dn, *parent = dev->of_node;
+ struct mask_entry *entry;
+ int devfn;
+
+ INIT_LIST_HEAD(&priv->masklist);
+
+ for_each_child_of_node(parent, dn) {
+ /*
+ * if device is not available, add this to masklist
+ * to avoid scanning it.
+ */
+ if (!of_device_is_available(dn)) {
+ devfn = of_pci_get_devfn(dn);
+ if (devfn < 0)
+ continue;
+
+ entry = devm_kzalloc(dev, sizeof(*entry), GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ entry->devfn = devfn;
+ list_add_tail(&entry->entry, &priv->masklist);
+ }
+ }
+
+ return 0;
+}
static int loongson_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
{
@@ -305,6 +357,9 @@ static int loongson_pci_probe(struct platform_device *pdev)
}
}
+ if (setup_masklist(priv))
+ return -ENOMEM;
+
bridge->sysdata = priv;
bridge->ops = priv->data->ops;
bridge->map_irq = loongson_map_irq;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v3] PCI: loongson: skip scanning unavailable child device
2022-11-04 10:53 [PATCH v3] PCI: loongson: skip scanning unavailable child device Liu Peibao
@ 2022-11-07 21:15 ` Bjorn Helgaas
2022-11-08 3:57 ` Liu Peibao
0 siblings, 1 reply; 3+ messages in thread
From: Bjorn Helgaas @ 2022-11-07 21:15 UTC (permalink / raw)
To: Liu Peibao
Cc: Bjorn Helgaas, Rob Herring, Krzysztof Kozlowski,
Lorenzo Pieralisi, Krzysztof Wilczyński, Jiaxun Yang,
Huacai Chen, Jianmin Lv, Yinbo Zhu, linux-pci, linux-kernel
Capitalize subject line to match other commits:
930c6074d7dd ("PCI: loongson: Work around LS7A incorrect Interrupt Pin registers")
2410e3301fcc ("PCI: loongson: Don't access non-existent devices")
cd89edda4002 ("PCI: loongson: Add ACPI init support")
dee449aafd48 ("PCI: loongson: Use generic 8/16/32-bit config ops on LS2K/LS7A")
On Fri, Nov 04, 2022 at 06:53:40PM +0800, Liu Peibao wrote:
> The PCI Controller of 2k1000 could not mask devices by
> setting vender id or device id in configuration space header
> as invalid values. When there are pins shareble between
> the platform device and PCI device, if the platform device
> is preferred, we should not scan this PCI device. In the
> above scene, add `status = "disabled"` property in DT node
> of this PCI device.
Rewrap this to fill 75 columns.
s/id/ID/
s/shareble/shareable/
> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
> ---
> V2 -> V3: 1. use list_for_each_entry() for more clearly.
> 2. fix wrong use of sizeof().
> V1 -> V2: use existing property "status" instead of adding new property.
>
>
> drivers/pci/controller/pci-loongson.c | 55 +++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> index 05c50408f13b..c7dd88eac885 100644
> --- a/drivers/pci/controller/pci-loongson.c
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -40,11 +40,21 @@ struct loongson_pci_data {
> struct pci_ops *ops;
> };
>
> +#ifdef CONFIG_OF
> +struct mask_entry {
> + struct list_head entry;
> + unsigned int devfn;
> +};
> +#endif
> +
> struct loongson_pci {
> void __iomem *cfg0_base;
> void __iomem *cfg1_base;
> struct platform_device *pdev;
> const struct loongson_pci_data *data;
> +#ifdef CONFIG_OF
> + struct list_head masklist;
> +#endif
> };
>
> /* Fixup wrong class code in PCIe bridges */
> @@ -194,6 +204,18 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus,
> return NULL;
> }
>
> +#ifdef CONFIG_OF
> + /* Don't access devices in masklist */
> + if (pci_is_root_bus(bus)) {
> + struct mask_entry *entry;
> +
> + list_for_each_entry(entry, &priv->masklist, entry) {
> + if (devfn == entry->devfn)
> + return NULL;
> + }
> + }
> +#endif
I would probably get rid of the masklist and just search for a disabled
property when reading config offset 0 (vendor ID). That's not a
performance path anyway. And this seems similar to the
FLAG_DEV_HIDDEN path where you probably don't need to do it for all
controllers.
> /* CFG0 can only access standard space */
> if (where < PCI_CFG_SPACE_SIZE && priv->cfg0_base)
> return cfg0_map(priv, bus, devfn, where);
> @@ -206,6 +228,36 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus,
> }
>
> #ifdef CONFIG_OF
> +static int setup_masklist(struct loongson_pci *priv)
> +{
> + struct device *dev = &priv->pdev->dev;
> + struct device_node *dn, *parent = dev->of_node;
> + struct mask_entry *entry;
> + int devfn;
> +
> + INIT_LIST_HEAD(&priv->masklist);
> +
> + for_each_child_of_node(parent, dn) {
> + /*
> + * if device is not available, add this to masklist
> + * to avoid scanning it.
> + */
> + if (!of_device_is_available(dn)) {
> + devfn = of_pci_get_devfn(dn);
> + if (devfn < 0)
> + continue;
> +
> + entry = devm_kzalloc(dev, sizeof(*entry), GFP_KERNEL);
> + if (!entry)
> + return -ENOMEM;
> +
> + entry->devfn = devfn;
> + list_add_tail(&entry->entry, &priv->masklist);
> + }
> + }
> +
> + return 0;
> +}
>
> static int loongson_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> {
> @@ -305,6 +357,9 @@ static int loongson_pci_probe(struct platform_device *pdev)
> }
> }
>
> + if (setup_masklist(priv))
> + return -ENOMEM;
> +
> bridge->sysdata = priv;
> bridge->ops = priv->data->ops;
> bridge->map_irq = loongson_map_irq;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v3] PCI: loongson: skip scanning unavailable child device
2022-11-07 21:15 ` Bjorn Helgaas
@ 2022-11-08 3:57 ` Liu Peibao
0 siblings, 0 replies; 3+ messages in thread
From: Liu Peibao @ 2022-11-08 3:57 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Rob Herring, Krzysztof Kozlowski,
Lorenzo Pieralisi, Krzysztof Wilczyński, Jiaxun Yang,
Huacai Chen, Jianmin Lv, Yinbo Zhu, linux-pci, linux-kernel
On 11/8/22 5:15 AM, Bjorn Helgaas wrote:
> Capitalize subject line to match other commits:
>
> 930c6074d7dd ("PCI: loongson: Work around LS7A incorrect Interrupt Pin registers")
> 2410e3301fcc ("PCI: loongson: Don't access non-existent devices")
> cd89edda4002 ("PCI: loongson: Add ACPI init support")
> dee449aafd48 ("PCI: loongson: Use generic 8/16/32-bit config ops on LS2K/LS7A")
>
OK, I will do this in the next version patch.
> On Fri, Nov 04, 2022 at 06:53:40PM +0800, Liu Peibao wrote:
>> The PCI Controller of 2k1000 could not mask devices by
>> setting vender id or device id in configuration space header
>> as invalid values. When there are pins shareble between
>> the platform device and PCI device, if the platform device
>> is preferred, we should not scan this PCI device. In the
>> above scene, add `status = "disabled"` property in DT node
>> of this PCI device.
>
> Rewrap this to fill 75 columns.
> > s/id/ID/
> s/shareble/shareable/
>
OK, I will take care in the next version patch.
>> Signed-off-by: Liu Peibao <liupeibao@loongson.cn>
>> ---
>> V2 -> V3: 1. use list_for_each_entry() for more clearly.
>> 2. fix wrong use of sizeof().
>> V1 -> V2: use existing property "status" instead of adding new property.
>>
>>
>> drivers/pci/controller/pci-loongson.c | 55 +++++++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
>> index 05c50408f13b..c7dd88eac885 100644
>> --- a/drivers/pci/controller/pci-loongson.c
>> +++ b/drivers/pci/controller/pci-loongson.c
>> @@ -40,11 +40,21 @@ struct loongson_pci_data {
>> struct pci_ops *ops;
>> };
>>
>> +#ifdef CONFIG_OF
>> +struct mask_entry {
>> + struct list_head entry;
>> + unsigned int devfn;
>> +};
>> +#endif
>> +
>> struct loongson_pci {
>> void __iomem *cfg0_base;
>> void __iomem *cfg1_base;
>> struct platform_device *pdev;
>> const struct loongson_pci_data *data;
>> +#ifdef CONFIG_OF
>> + struct list_head masklist;
>> +#endif
>> };
>>
>> /* Fixup wrong class code in PCIe bridges */
>> @@ -194,6 +204,18 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus,
>> return NULL;
>> }
>>
>> +#ifdef CONFIG_OF
>> + /* Don't access devices in masklist */
>> + if (pci_is_root_bus(bus)) {
>> + struct mask_entry *entry;
>> +
>> + list_for_each_entry(entry, &priv->masklist, entry) {
>> + if (devfn == entry->devfn)
>> + return NULL;
>> + }
>> + }
>> +#endif
>
> I would probably get rid of the masklist and just search for a disabled
> property when reading config offset 0 (vendor ID). That's not a
> performance path anyway. And this seems similar to the
> FLAG_DEV_HIDDEN path where you probably don't need to do it for all
> controllers.
>
Thanks for your idea! This really helps a lot!
I will follow your idea and rework it.
Yes, this is similar to the FLAG_DEV_HIDDEN path and currently, this path
is only needed by 2K1000 PCI controller.
BR,
Peibao
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-08 3:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-04 10:53 [PATCH v3] PCI: loongson: skip scanning unavailable child device Liu Peibao
2022-11-07 21:15 ` Bjorn Helgaas
2022-11-08 3:57 ` Liu Peibao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).