From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: rjw@rjwysocki.net, lenb@kernel.org, catalin.marinas@arm.com,
will.deacon@arm.com, hanjun.guo@linaro.org,
linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH V3 2/4] ACPI/scan: Clean up acpi_check_dma
Date: Tue, 13 Oct 2015 08:52:21 -0700 [thread overview]
Message-ID: <561D28B5.7030303@amd.com> (raw)
In-Reply-To: <20150914163411.GL829@google.com>
Hi Bjorn,
Thanks for your feedback. And sorry for late response. Some how I didn't
see this earlier. Please see my comments below.
On 09/14/2015 09:34 AM, Bjorn Helgaas wrote:
>> [..]
>> So, in order to simplify the function, this patch renames acpi_check_dma()
>> to acpi_check_dma_coherency() to clearly indicate the purpose of this
>> function, and only returns an integer where -1 means DMA not supported,
>> 1 means coherent DMA, and 0 means non-coherent DMA.
>
> I think acpi_check_dma_coherency() is better, but only slightly. It
> still doesn't give a hint about the *sense* of the return value. I
> think it'd be easier to read if there were two functions, e.g.,
I have been going back-and-forth between the current version, and the
two-function-approach in the past. I can definitely go with this route
if you would prefer. Although, if acpi_dma_is_coherent() == 0, it would
be ambiguous whether DMA is not supported or non-coherent DMA is
supported. Then, we would need to call acpi_dma_is_supported() to find
out. So, that's okay with you?
>> [...]
>> +
>> + /**
>> + * Currently, we only support _CCA=1 (i.e. coherent_dma=1)
>> + * This should be equivalent to specifying dma-coherent for
>> + * a device in OF.
>> + *
>> + * For the case when _CCA=0 (i.e. coherent_dma=0 && cca_seen=1),
>> + * we have two choices:
>> + * 1. Do not support and disable DMA.
>
> I know you didn't write this comment, but do we actually *disable* DMA in
> the sense of turning off PCI bus mastering or calling an ACPI method that
> disables DMA by this device? I suspect we just don't set up DMA ops and
> masks for this device.
Actually, I wrote this comment. When we disable DMA, we basically set
dma-mask=0 and do not setup DMA ops as you mentioned. We don't actually
mess with the hardware.
Thanks,
Suravee
WARNING: multiple messages have this Message-ID (diff)
From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: <rjw@rjwysocki.net>, <lenb@kernel.org>, <catalin.marinas@arm.com>,
<will.deacon@arm.com>, <hanjun.guo@linaro.org>,
<linux-acpi@vger.kernel.org>, <linux-pci@vger.kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>,
Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH V3 2/4] ACPI/scan: Clean up acpi_check_dma
Date: Tue, 13 Oct 2015 08:52:21 -0700 [thread overview]
Message-ID: <561D28B5.7030303@amd.com> (raw)
In-Reply-To: <20150914163411.GL829@google.com>
Hi Bjorn,
Thanks for your feedback. And sorry for late response. Some how I didn't
see this earlier. Please see my comments below.
On 09/14/2015 09:34 AM, Bjorn Helgaas wrote:
>> [..]
>> So, in order to simplify the function, this patch renames acpi_check_dma()
>> to acpi_check_dma_coherency() to clearly indicate the purpose of this
>> function, and only returns an integer where -1 means DMA not supported,
>> 1 means coherent DMA, and 0 means non-coherent DMA.
>
> I think acpi_check_dma_coherency() is better, but only slightly. It
> still doesn't give a hint about the *sense* of the return value. I
> think it'd be easier to read if there were two functions, e.g.,
I have been going back-and-forth between the current version, and the
two-function-approach in the past. I can definitely go with this route
if you would prefer. Although, if acpi_dma_is_coherent() == 0, it would
be ambiguous whether DMA is not supported or non-coherent DMA is
supported. Then, we would need to call acpi_dma_is_supported() to find
out. So, that's okay with you?
>> [...]
>> +
>> + /**
>> + * Currently, we only support _CCA=1 (i.e. coherent_dma=1)
>> + * This should be equivalent to specifying dma-coherent for
>> + * a device in OF.
>> + *
>> + * For the case when _CCA=0 (i.e. coherent_dma=0 && cca_seen=1),
>> + * we have two choices:
>> + * 1. Do not support and disable DMA.
>
> I know you didn't write this comment, but do we actually *disable* DMA in
> the sense of turning off PCI bus mastering or calling an ACPI method that
> disables DMA by this device? I suspect we just don't set up DMA ops and
> masks for this device.
Actually, I wrote this comment. When we disable DMA, we basically set
dma-mask=0 and do not setup DMA ops as you mentioned. We don't actually
mess with the hardware.
Thanks,
Suravee
WARNING: multiple messages have this Message-ID (diff)
From: suravee.suthikulpanit@amd.com (Suravee Suthikulpanit)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V3 2/4] ACPI/scan: Clean up acpi_check_dma
Date: Tue, 13 Oct 2015 08:52:21 -0700 [thread overview]
Message-ID: <561D28B5.7030303@amd.com> (raw)
In-Reply-To: <20150914163411.GL829@google.com>
Hi Bjorn,
Thanks for your feedback. And sorry for late response. Some how I didn't
see this earlier. Please see my comments below.
On 09/14/2015 09:34 AM, Bjorn Helgaas wrote:
>> [..]
>> So, in order to simplify the function, this patch renames acpi_check_dma()
>> to acpi_check_dma_coherency() to clearly indicate the purpose of this
>> function, and only returns an integer where -1 means DMA not supported,
>> 1 means coherent DMA, and 0 means non-coherent DMA.
>
> I think acpi_check_dma_coherency() is better, but only slightly. It
> still doesn't give a hint about the *sense* of the return value. I
> think it'd be easier to read if there were two functions, e.g.,
I have been going back-and-forth between the current version, and the
two-function-approach in the past. I can definitely go with this route
if you would prefer. Although, if acpi_dma_is_coherent() == 0, it would
be ambiguous whether DMA is not supported or non-coherent DMA is
supported. Then, we would need to call acpi_dma_is_supported() to find
out. So, that's okay with you?
>> [...]
>> +
>> + /**
>> + * Currently, we only support _CCA=1 (i.e. coherent_dma=1)
>> + * This should be equivalent to specifying dma-coherent for
>> + * a device in OF.
>> + *
>> + * For the case when _CCA=0 (i.e. coherent_dma=0 && cca_seen=1),
>> + * we have two choices:
>> + * 1. Do not support and disable DMA.
>
> I know you didn't write this comment, but do we actually *disable* DMA in
> the sense of turning off PCI bus mastering or calling an ACPI method that
> disables DMA by this device? I suspect we just don't set up DMA ops and
> masks for this device.
Actually, I wrote this comment. When we disable DMA, we basically set
dma-mask=0 and do not setup DMA ops as you mentioned. We don't actually
mess with the hardware.
Thanks,
Suravee
next prev parent reply other threads:[~2015-10-13 15:52 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-26 13:54 [PATCH V3 0/4] PCI: ACPI: Setting up DMA coherency for PCI device from _CCA attribute Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-08-26 13:54 ` [PATCH V3 1/4] Honor ACPI _CCA attribute setting Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-08-26 13:54 ` [PATCH V3 2/4] ACPI/scan: Clean up acpi_check_dma Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-09-14 16:34 ` Bjorn Helgaas
2015-09-14 16:34 ` Bjorn Helgaas
2015-10-13 15:52 ` Suravee Suthikulpanit [this message]
2015-10-13 15:52 ` Suravee Suthikulpanit
2015-10-13 15:52 ` Suravee Suthikulpanit
2015-10-13 23:53 ` Suravee Suthikulanit
2015-10-13 23:53 ` Suravee Suthikulanit
2015-10-13 23:53 ` Suravee Suthikulanit
2015-10-20 2:17 ` Bjorn Helgaas
2015-10-20 2:17 ` Bjorn Helgaas
2015-10-20 12:53 ` Suravee Suthikulpanit
2015-10-20 12:53 ` Suravee Suthikulpanit
2015-10-20 12:53 ` Suravee Suthikulpanit
2015-08-26 13:54 ` [PATCH V3 3/4] PCI: OF: Move of_pci_dma_configure() to pci_dma_configure() Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-08-26 13:54 ` [PATCH V3 4/4] PCI: ACPI: Add support for PCI device DMA coherency Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-08-26 13:54 ` Suravee Suthikulpanit
2015-09-09 12:16 ` [PATCH V3 0/4] PCI: ACPI: Setting up DMA coherency for PCI device from _CCA attribute Suthikulpanit, Suravee
2015-09-09 12:16 ` Suthikulpanit, Suravee
2015-09-09 12:16 ` Suthikulpanit, Suravee
2015-09-09 20:38 ` Rafael J. Wysocki
2015-09-09 20:38 ` Rafael J. Wysocki
2015-09-10 2:48 ` Suthikulpanit, Suravee
2015-09-10 2:48 ` Suthikulpanit, Suravee
2015-09-10 2:48 ` Suthikulpanit, Suravee
2015-10-12 19:51 ` Suravee Suthikulpanit
2015-10-12 19:51 ` Suravee Suthikulpanit
2015-10-12 19:51 ` Suravee Suthikulpanit
2015-10-12 20:27 ` Rafael J. Wysocki
2015-10-12 20:27 ` Rafael J. Wysocki
2015-10-12 20:02 ` Suravee Suthikulpanit
2015-10-12 20:02 ` Suravee Suthikulpanit
2015-10-12 20:02 ` Suravee Suthikulpanit
2015-09-10 9:15 ` Hanjun Guo
2015-09-10 9:15 ` Hanjun Guo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=561D28B5.7030303@amd.com \
--to=suravee.suthikulpanit@amd.com \
--cc=bhelgaas@google.com \
--cc=catalin.marinas@arm.com \
--cc=hanjun.guo@linaro.org \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.