2015-11-19 00:51:28

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: [PATCH] PCI: Fix logic OF logic in pci_dma_configure()

This patch fixes a bug introduced by previous commit,
which incorrectly checkes the of_node of the end-point device.
Instead, it should check the of_node of the host bridge.

Fixes: 50230713b639 ("PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()")
Reported-by: Robin Murphy <[email protected]>
Cc: Rafael J. Wysocki <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Signed-off-by: Suravee Suthikulpanit <[email protected]>
---
drivers/pci/probe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e735c72..edb1984 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1685,8 +1685,8 @@ static void pci_dma_configure(struct pci_dev *dev)
{
struct device *bridge = pci_get_host_bridge_device(dev);

- if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
- if (bridge->parent)
+ if (IS_ENABLED(CONFIG_OF) &&
+ bridge->parent && bridge->parent->of_node) {
of_dma_configure(&dev->dev, bridge->parent->of_node);
} else if (has_acpi_companion(bridge)) {
struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
--
2.1.0


2015-11-19 00:54:28

by Suthikulpanit, Suravee

[permalink] [raw]
Subject: Re: [PATCH] PCI: Fix logic OF logic in pci_dma_configure()

Arg... sorry for the typo in the subject. It should say:

[PATCH] PCI: Fix OF logic in pci_dma_configure()

Suravee

On 11/18/2015 6:49 PM, Suravee Suthikulpanit wrote:
> This patch fixes a bug introduced by previous commit,
> which incorrectly checkes the of_node of the end-point device.
> Instead, it should check the of_node of the host bridge.
>
> Fixes: 50230713b639 ("PCI: OF: Move of_pci_dma_configure() to pci_dma_configure()")
> Reported-by: Robin Murphy <[email protected]>
> Cc: Rafael J. Wysocki <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
> drivers/pci/probe.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index e735c72..edb1984 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1685,8 +1685,8 @@ static void pci_dma_configure(struct pci_dev *dev)
> {
> struct device *bridge = pci_get_host_bridge_device(dev);
>
> - if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
> - if (bridge->parent)
> + if (IS_ENABLED(CONFIG_OF) &&
> + bridge->parent && bridge->parent->of_node) {
> of_dma_configure(&dev->dev, bridge->parent->of_node);
> } else if (has_acpi_companion(bridge)) {
> struct acpi_device *adev = to_acpi_device_node(bridge->fwnode);
>

2015-11-19 01:27:31

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] PCI: Fix logic OF logic in pci_dma_configure()

On Wednesday, November 18, 2015 06:54:16 PM Suravee Suthikulanit wrote:
> Arg... sorry for the typo in the subject. It should say:
>
> [PATCH] PCI: Fix OF logic in pci_dma_configure()

OK, applied.

Thanks,
Rafael

2015-11-19 08:24:28

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH] PCI: Fix logic OF logic in pci_dma_configure()

On Wednesday 18 November 2015 16:49:52 Suravee Suthikulpanit wrote:
> - if (IS_ENABLED(CONFIG_OF) && dev->dev.of_node) {
> - if (bridge->parent)
> + if (IS_ENABLED(CONFIG_OF) &&
> + bridge->parent && bridge->parent->of_node) {
> of_dma_configure(&dev->dev, bridge->parent->of_node);
>

Acked-by: Arnd Bergmann <[email protected]>

I was wondering for a bit whether we should warn about the case where there
is no of_node pointer, as that would mean we cannot do DMA on that PCI
device, but on second thought we have to do it exactly like you do, because
we would otherwise break machines that do not use DT.

Arnd