* [PATCH v2] ath10k: Replace ioread with mb to drain write buffer
@ 2015-01-31 0:14 ` Peter Oh
0 siblings, 0 replies; 7+ messages in thread
From: Peter Oh @ 2015-01-31 0:14 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
Using ioread() to perform draining write buffer is excessive.
Use compact API, mb(), that intended to be used for the case.
It reduces total 14 CPU clocks per interrupt.
Signed-off-by: Peter Oh <poh@qca.qualcomm.com>
---
drivers/net/wireless/ath/ath10k/pci.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index e6972b0..f1e6980 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -353,10 +353,8 @@ static void ath10k_pci_disable_and_clear_legacy_irq(struct ath10k *ar)
ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + PCIE_INTR_CLR_ADDRESS,
PCIE_INTR_FIRMWARE_MASK | PCIE_INTR_CE_MASK_ALL);
- /* IMPORTANT: this extra read transaction is required to
- * flush the posted write buffer. */
- (void)ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
- PCIE_INTR_ENABLE_ADDRESS);
+ /* drain write buffer */
+ mb();
}
static void ath10k_pci_enable_legacy_irq(struct ath10k *ar)
@@ -365,10 +363,8 @@ static void ath10k_pci_enable_legacy_irq(struct ath10k *ar)
PCIE_INTR_ENABLE_ADDRESS,
PCIE_INTR_FIRMWARE_MASK | PCIE_INTR_CE_MASK_ALL);
- /* IMPORTANT: this extra read transaction is required to
- * flush the posted write buffer. */
- (void)ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
- PCIE_INTR_ENABLE_ADDRESS);
+ /* drain write buffer */
+ mb();
}
static inline const char *ath10k_pci_get_irq_method(struct ath10k *ar)
--
1.9.1
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2] ath10k: Replace ioread with mb to drain write buffer
@ 2015-01-31 0:14 ` Peter Oh
0 siblings, 0 replies; 7+ messages in thread
From: Peter Oh @ 2015-01-31 0:14 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
Using ioread() to perform draining write buffer is excessive.
Use compact API, mb(), that intended to be used for the case.
It reduces total 14 CPU clocks per interrupt.
Signed-off-by: Peter Oh <poh@qca.qualcomm.com>
---
drivers/net/wireless/ath/ath10k/pci.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index e6972b0..f1e6980 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -353,10 +353,8 @@ static void ath10k_pci_disable_and_clear_legacy_irq(struct ath10k *ar)
ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + PCIE_INTR_CLR_ADDRESS,
PCIE_INTR_FIRMWARE_MASK | PCIE_INTR_CE_MASK_ALL);
- /* IMPORTANT: this extra read transaction is required to
- * flush the posted write buffer. */
- (void)ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
- PCIE_INTR_ENABLE_ADDRESS);
+ /* drain write buffer */
+ mb();
}
static void ath10k_pci_enable_legacy_irq(struct ath10k *ar)
@@ -365,10 +363,8 @@ static void ath10k_pci_enable_legacy_irq(struct ath10k *ar)
PCIE_INTR_ENABLE_ADDRESS,
PCIE_INTR_FIRMWARE_MASK | PCIE_INTR_CE_MASK_ALL);
- /* IMPORTANT: this extra read transaction is required to
- * flush the posted write buffer. */
- (void)ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
- PCIE_INTR_ENABLE_ADDRESS);
+ /* drain write buffer */
+ mb();
}
static inline const char *ath10k_pci_get_irq_method(struct ath10k *ar)
--
1.9.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ath10k: Replace ioread with mb to drain write buffer
2015-01-31 0:14 ` Peter Oh
(?)
@ 2015-01-31 0:22 ` Peter Oh
-1 siblings, 0 replies; 7+ messages in thread
From: Peter Oh @ 2015-01-31 0:22 UTC (permalink / raw)
To: ath10k
On 01/30/2015 04:14 PM, Peter Oh wrote:
> Using ioread() to perform draining write buffer is excessive.
> Use compact API, mb(), that intended to be used for the case.
> It reduces total 14 CPU clocks per interrupt.
>
> Signed-off-by: Peter Oh <poh@qca.qualcomm.com>
> ---
> drivers/net/wireless/ath/ath10k/pci.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> index e6972b0..f1e6980 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -353,10 +353,8 @@ static void ath10k_pci_disable_and_clear_legacy_irq(struct ath10k *ar)
> ath10k_pci_write32(ar, SOC_CORE_BASE_ADDRESS + PCIE_INTR_CLR_ADDRESS,
> PCIE_INTR_FIRMWARE_MASK | PCIE_INTR_CE_MASK_ALL);
>
> - /* IMPORTANT: this extra read transaction is required to
> - * flush the posted write buffer. */
> - (void)ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
> - PCIE_INTR_ENABLE_ADDRESS);
> + /* drain write buffer */
> + mb();
It figured out that ath10k_pci_read32() use dsb with 'st' while wmb() is
use with 'sy'.
So updated it from wmb() to mb() for the same effect as ath10k_pci_read32().
> }
>
> static void ath10k_pci_enable_legacy_irq(struct ath10k *ar)
> @@ -365,10 +363,8 @@ static void ath10k_pci_enable_legacy_irq(struct ath10k *ar)
> PCIE_INTR_ENABLE_ADDRESS,
> PCIE_INTR_FIRMWARE_MASK | PCIE_INTR_CE_MASK_ALL);
>
> - /* IMPORTANT: this extra read transaction is required to
> - * flush the posted write buffer. */
> - (void)ath10k_pci_read32(ar, SOC_CORE_BASE_ADDRESS +
> - PCIE_INTR_ENABLE_ADDRESS);
> + /* drain write buffer */
> + mb();
> }
>
> static inline const char *ath10k_pci_get_irq_method(struct ath10k *ar)
Thanks,
Peter
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ath10k: Replace ioread with mb to drain write buffer
2015-01-31 0:14 ` Peter Oh
@ 2015-02-06 8:58 ` Jakub Kiciński
-1 siblings, 0 replies; 7+ messages in thread
From: Jakub Kiciński @ 2015-02-06 8:58 UTC (permalink / raw)
To: Peter Oh; +Cc: linux-wireless, ath10k
On Fri, 30 Jan 2015 16:14:30 -0800, Peter Oh wrote:
> Using ioread() to perform draining write buffer is excessive.
> Use compact API, mb(), that intended to be used for the case.
> It reduces total 14 CPU clocks per interrupt.
>
> Signed-off-by: Peter Oh <poh@qca.qualcomm.com>
I have no idea what the code does but this change looks suspicious.
Usually the point of ioread() is to flush the interconnect buffers
while mb() ensures ordering only from the CPU perspective.
Could you provide the reason *why* flushing buffers is unnecessary
in the commit message?
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ath10k: Replace ioread with mb to drain write buffer
@ 2015-02-06 8:58 ` Jakub Kiciński
0 siblings, 0 replies; 7+ messages in thread
From: Jakub Kiciński @ 2015-02-06 8:58 UTC (permalink / raw)
To: Peter Oh; +Cc: ath10k, linux-wireless
On Fri, 30 Jan 2015 16:14:30 -0800, Peter Oh wrote:
> Using ioread() to perform draining write buffer is excessive.
> Use compact API, mb(), that intended to be used for the case.
> It reduces total 14 CPU clocks per interrupt.
>
> Signed-off-by: Peter Oh <poh@qca.qualcomm.com>
I have no idea what the code does but this change looks suspicious.
Usually the point of ioread() is to flush the interconnect buffers
while mb() ensures ordering only from the CPU perspective.
Could you provide the reason *why* flushing buffers is unnecessary
in the commit message?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ath10k: Replace ioread with mb to drain write buffer
2015-02-06 8:58 ` Jakub Kiciński
@ 2015-02-06 14:53 ` Jakub Kiciński
-1 siblings, 0 replies; 7+ messages in thread
From: Jakub Kiciński @ 2015-02-06 14:53 UTC (permalink / raw)
To: Peter Oh; +Cc: linux-wireless, ath10k
On Fri, 6 Feb 2015 09:58:46 +0100, Jakub Kiciński wrote:
> On Fri, 30 Jan 2015 16:14:30 -0800, Peter Oh wrote:
> > Using ioread() to perform draining write buffer is excessive.
> > Use compact API, mb(), that intended to be used for the case.
> > It reduces total 14 CPU clocks per interrupt.
> >
> > Signed-off-by: Peter Oh <poh@qca.qualcomm.com>
>
> I have no idea what the code does but this change looks suspicious.
> Usually the point of ioread() is to flush the interconnect buffers
> while mb() ensures ordering only from the CPU perspective.
>
> Could you provide the reason *why* flushing buffers is unnecessary
> in the commit message?
I just noticed the discussion on the first version of the patch.
Sorry about the noise.
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ath10k: Replace ioread with mb to drain write buffer
@ 2015-02-06 14:53 ` Jakub Kiciński
0 siblings, 0 replies; 7+ messages in thread
From: Jakub Kiciński @ 2015-02-06 14:53 UTC (permalink / raw)
To: Peter Oh; +Cc: ath10k, linux-wireless
On Fri, 6 Feb 2015 09:58:46 +0100, Jakub Kiciński wrote:
> On Fri, 30 Jan 2015 16:14:30 -0800, Peter Oh wrote:
> > Using ioread() to perform draining write buffer is excessive.
> > Use compact API, mb(), that intended to be used for the case.
> > It reduces total 14 CPU clocks per interrupt.
> >
> > Signed-off-by: Peter Oh <poh@qca.qualcomm.com>
>
> I have no idea what the code does but this change looks suspicious.
> Usually the point of ioread() is to flush the interconnect buffers
> while mb() ensures ordering only from the CPU perspective.
>
> Could you provide the reason *why* flushing buffers is unnecessary
> in the commit message?
I just noticed the discussion on the first version of the patch.
Sorry about the noise.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-02-06 14:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-31 0:14 [PATCH v2] ath10k: Replace ioread with mb to drain write buffer Peter Oh
2015-01-31 0:14 ` Peter Oh
2015-01-31 0:22 ` Peter Oh
2015-02-06 8:58 ` Jakub Kiciński
2015-02-06 8:58 ` Jakub Kiciński
2015-02-06 14:53 ` Jakub Kiciński
2015-02-06 14:53 ` Jakub Kiciński
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.