The PCI Controller of 2k1000 could not mask devices by
setting vender id & 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 "skip-scan" property in DT node of this
PCI device.
Signed-off-by: Liu Peibao <[email protected]>
---
drivers/pci/controller/pci-loongson.c | 62 +++++++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
index 05c50408f13b..4b97963977c7 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,20 @@ 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 list_head *list;
+ struct mask_entry *entry;
+
+ list_for_each(list, &priv->masklist) {
+ entry = list_entry(list, struct mask_entry, 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 +230,41 @@ static void __iomem *pci_loongson_map_bus(struct pci_bus *bus,
}
#ifdef CONFIG_OF
+/*
+ * The PCI Controller of 2k1000 could not mask devices by
+ * setting vender id & 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 "skip-scan" property
+ * in the device tree node of this PCI device.
+ */
+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 (of_property_read_bool(dn, "skip-scan")) {
+ 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 +364,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
Add the newly added "skip-scan" property for child node.
Signed-off-by: Liu Peibao <[email protected]>
---
Documentation/devicetree/bindings/pci/loongson.yaml | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/loongson.yaml b/Documentation/devicetree/bindings/pci/loongson.yaml
index a8324a9bd002..5c2fe9bf2c78 100644
--- a/Documentation/devicetree/bindings/pci/loongson.yaml
+++ b/Documentation/devicetree/bindings/pci/loongson.yaml
@@ -32,6 +32,13 @@ properties:
minItems: 1
maxItems: 3
+ child-node:
+ type: object
+ properties:
+ skip-scan:
+ description: avoid scanning this device.
+ type: boolean
+
required:
- compatible
--
2.20.1
在 2022/11/3 09:00, Liu Peibao 写道:
> The PCI Controller of 2k1000 could not mask devices by
> setting vender id & 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 "skip-scan" property in DT node of this
> PCI device.
If you are going to add a new property you need to document it somewhere.
From my point of view you can use existing property `status = "disabled"`
to for this semantic...
Thanks.
- Jiaxun
>
> Signed-off-by: Liu Peibao <[email protected]>
> ---
>
On 03/11/2022 05:00, Liu Peibao wrote:
> Add the newly added "skip-scan" property for child node.
This we can see from the patch log. You need to say answer here to "Why".
>
> Signed-off-by: Liu Peibao <[email protected]>
> ---
> Documentation/devicetree/bindings/pci/loongson.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/loongson.yaml b/Documentation/devicetree/bindings/pci/loongson.yaml
> index a8324a9bd002..5c2fe9bf2c78 100644
> --- a/Documentation/devicetree/bindings/pci/loongson.yaml
> +++ b/Documentation/devicetree/bindings/pci/loongson.yaml
> @@ -32,6 +32,13 @@ properties:
> minItems: 1
> maxItems: 3
>
> + child-node:
What is "child-node"?
> + type: object
> + properties:
> + skip-scan:
> + description: avoid scanning this device.
> + type: boolean
Why? Isn't status for that?
You also need additionalProperties: false/true, depending on what this
child-node is...
...and add the case illustrating it in the example.
Best regards,
Krzysztof
On 11/3/22 7:37 PM, Jiaxun Yang wrote:
>
>
> 在 2022/11/3 09:00, Liu Peibao 写道:
>> The PCI Controller of 2k1000 could not mask devices by
>> setting vender id & 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 "skip-scan" property in DT node of this
>> PCI device.
>
> If you are going to add a new property you need to document it somewhere.
>
> From my point of view you can use existing property `status = "disabled"`
> to for this semantic...
>
That is really constructive. I will let it work with `status = "disabled"`.
BR,
Peibao
On 11/4/22 5:53 AM, Krzysztof Kozlowski wrote:
> On 03/11/2022 05:00, Liu Peibao wrote:
>> Add the newly added "skip-scan" property for child node.
>
> This we can see from the patch log. You need to say answer here to "Why".
>
I will take care of this when I submit new patches.
>>
>> Signed-off-by: Liu Peibao <[email protected]>
>> ---
>> Documentation/devicetree/bindings/pci/loongson.yaml | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/loongson.yaml b/Documentation/devicetree/bindings/pci/loongson.yaml
>> index a8324a9bd002..5c2fe9bf2c78 100644
>> --- a/Documentation/devicetree/bindings/pci/loongson.yaml
>> +++ b/Documentation/devicetree/bindings/pci/loongson.yaml
>> @@ -32,6 +32,13 @@ properties:
>> minItems: 1
>> maxItems: 3
>>
>> + child-node:
>
> What is "child-node"?
>
>> + type: object
>> + properties:
>> + skip-scan:
>> + description: avoid scanning this device.
>> + type: boolean
>
> Why? Isn't status for that?
>
> You also need additionalProperties: false/true, depending on what this
> child-node is...
>
> ...and add the case illustrating it in the example.
>
>
I think I did add some vague description.
Since the opinion of Jiaxun in the [PATCH 1/2] is really more elegant
than what I did. I decide follow this opinion and as the `status = "disabled"`
property is existing, I will don't modify this yaml.
Thanks for your detailed review!
BR,
Peibao
Hi Liu,
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Liu-Peibao/PCI-loongson-add-skip-scan-property-for-child-DT-node/20221103-170125
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link: https://lore.kernel.org/r/20221103090040.836-1-liupeibao%40loongson.cn
patch subject: [PATCH 1/2] PCI: loongson: add skip-scan property for child DT node
config: loongarch-randconfig-m041-20221114
compiler: loongarch64-linux-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
smatch warnings:
drivers/pci/controller/pci-loongson.c:257 setup_masklist() error: not allocating enough for = 'entry' 24 vs 8
vim +/entry +257 drivers/pci/controller/pci-loongson.c
f93e71fb8e65ba Liu Peibao 2022-11-03 242 static int setup_masklist(struct loongson_pci *priv)
f93e71fb8e65ba Liu Peibao 2022-11-03 243 {
f93e71fb8e65ba Liu Peibao 2022-11-03 244 struct device *dev = &priv->pdev->dev;
f93e71fb8e65ba Liu Peibao 2022-11-03 245 struct device_node *dn, *parent = dev->of_node;
f93e71fb8e65ba Liu Peibao 2022-11-03 246 struct mask_entry *entry;
f93e71fb8e65ba Liu Peibao 2022-11-03 247 int devfn;
f93e71fb8e65ba Liu Peibao 2022-11-03 248
f93e71fb8e65ba Liu Peibao 2022-11-03 249 INIT_LIST_HEAD(&priv->masklist);
f93e71fb8e65ba Liu Peibao 2022-11-03 250
f93e71fb8e65ba Liu Peibao 2022-11-03 251 for_each_child_of_node(parent, dn) {
f93e71fb8e65ba Liu Peibao 2022-11-03 252 if (of_property_read_bool(dn, "skip-scan")) {
f93e71fb8e65ba Liu Peibao 2022-11-03 253 devfn = of_pci_get_devfn(dn);
f93e71fb8e65ba Liu Peibao 2022-11-03 254 if (devfn < 0)
f93e71fb8e65ba Liu Peibao 2022-11-03 255 continue;
f93e71fb8e65ba Liu Peibao 2022-11-03 256
f93e71fb8e65ba Liu Peibao 2022-11-03 @257 entry = devm_kzalloc(dev, sizeof(entry), GFP_KERNEL);
^^^^^^^^^^^^^
Should be sizeof(*entry)
f93e71fb8e65ba Liu Peibao 2022-11-03 258 if (!entry)
f93e71fb8e65ba Liu Peibao 2022-11-03 259 return -ENOMEM;
f93e71fb8e65ba Liu Peibao 2022-11-03 260
f93e71fb8e65ba Liu Peibao 2022-11-03 261 entry->devfn = devfn;
f93e71fb8e65ba Liu Peibao 2022-11-03 262 list_add_tail(&entry->entry, &priv->masklist);
f93e71fb8e65ba Liu Peibao 2022-11-03 263 }
f93e71fb8e65ba Liu Peibao 2022-11-03 264 }
f93e71fb8e65ba Liu Peibao 2022-11-03 265
f93e71fb8e65ba Liu Peibao 2022-11-03 266 return 0;
f93e71fb8e65ba Liu Peibao 2022-11-03 267 }
--
0-DAY CI Kernel Test Service
https://01.org/lkp