Re: [ath5k-devel] [PATCH 3/3] ath5k: use bit shift operators for cache line size

2009-08-12 Thread Bob Copeland
On Wed, Aug 12, 2009 at 12:57 PM, Luis R.
Rodriguezlrodrig...@atheros.com wrote:
 This matches ath9k, providing consistency when reading both drivers.

 Signed-off-by: Luis R. Rodriguez lrodrig...@atheros.com
 ---
  drivers/net/wireless/ath/ath5k/base.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/net/wireless/ath/ath5k/base.c 
 b/drivers/net/wireless/ath/ath5k/base.c
 index 63c2b57..2b3cf39 100644
 --- a/drivers/net/wireless/ath/ath5k/base.c
 +++ b/drivers/net/wireless/ath/ath5k/base.c
 @@ -471,7 +471,7 @@ ath5k_pci_probe(struct pci_dev *pdev,
                 * DMA to work so force a reasonable value here if it
                 * comes up zero.
                 */
 -               csz = L1_CACHE_BYTES / sizeof(u32);
 +               csz = L1_CACHE_BYTES  2;
                pci_write_config_byte(pdev, PCI_CACHE_LINE_SIZE, csz);

I'm not sure it's better, although the whole thing seems bogus to
me.  Is there really a modern machine where PCI cache line size should
only be four bytes?

-- 
Bob Copeland %% www.bobcopeland.com
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel


Re: [ath5k-devel] [PATCH 3/3] ath5k: use bit shift operators for cache line size

2009-08-12 Thread Luis R. Rodriguez
On Wed, Aug 12, 2009 at 10:13 AM, Bob Copelandm...@bobcopeland.com wrote:
 On Wed, Aug 12, 2009 at 12:57 PM, Luis R.
 Rodriguezlrodrig...@atheros.com wrote:
 This matches ath9k, providing consistency when reading both drivers.

 Signed-off-by: Luis R. Rodriguez lrodrig...@atheros.com
 ---
  drivers/net/wireless/ath/ath5k/base.c |    4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/drivers/net/wireless/ath/ath5k/base.c 
 b/drivers/net/wireless/ath/ath5k/base.c
 index 63c2b57..2b3cf39 100644
 --- a/drivers/net/wireless/ath/ath5k/base.c
 +++ b/drivers/net/wireless/ath/ath5k/base.c
 @@ -471,7 +471,7 @@ ath5k_pci_probe(struct pci_dev *pdev,
                 * DMA to work so force a reasonable value here if it
                 * comes up zero.
                 */
 -               csz = L1_CACHE_BYTES / sizeof(u32);
 +               csz = L1_CACHE_BYTES  2;
                pci_write_config_byte(pdev, PCI_CACHE_LINE_SIZE, csz);

 I'm not sure it's better,

I did this for consistency between drivers but yes the advantage with
a shift is it should be cheaper than a multiplication. Although I am
not sure if simple multiplications get optimized by either the
compiler or an architecture to shifts.

 although the whole thing seems bogus to
 me.  Is there really a modern machine where PCI cache line size should
 only be four bytes?

Beats me, I was just matching the code for ath9k. The whole cache
alignment practice seems to be debatable to me and and hoping Sam
Leffer might recall the exact reasonings behind it.

Whether we remove this though would be a change which should go
through a separate patch I think.

  Luis
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel


Re: [ath5k-devel] [PATCH 3/3] ath5k: use bit shift operators for cache line size

2009-08-12 Thread Bob Copeland
On Wed, Aug 12, 2009 at 1:32 PM, Luis R.
Rodriguezlrodrig...@atheros.com wrote:
 diff --git a/drivers/net/wireless/ath/ath5k/base.c 
 b/drivers/net/wireless/ath/ath5k/base.c
 index 63c2b57..2b3cf39 100644
 --- a/drivers/net/wireless/ath/ath5k/base.c
 +++ b/drivers/net/wireless/ath/ath5k/base.c
 @@ -471,7 +471,7 @@ ath5k_pci_probe(struct pci_dev *pdev,
                 * DMA to work so force a reasonable value here if it
                 * comes up zero.
                 */
 -               csz = L1_CACHE_BYTES / sizeof(u32);
 +               csz = L1_CACHE_BYTES  2;
                pci_write_config_byte(pdev, PCI_CACHE_LINE_SIZE, csz);

 I'm not sure it's better,

 I did this for consistency between drivers but yes the advantage with
 a shift is it should be cheaper than a multiplication. Although I am
 not sure if simple multiplications get optimized by either the
 compiler or an architecture to shifts.

It shouldn't matter in the above case -- division by two constants.
In the multiplication by power of 2 constant case, it should also
get optimized by the compiler.  ' 2' looks like magic though, maybe
a comment to say why?

 although the whole thing seems bogus to
 me.  Is there really a modern machine where PCI cache line size should
 only be four bytes?

To correct above, I misread what it was doing.. it's getting cpu cache
size and dividing by 4 to get the number of words, if cache line size
was zeroed initially.  Ok, I'll go back to sleep now.

Whether needed or not, there's a lot of confusing comments and
voodoo around the stuff (something about 2.4 kernels...) that would
be nice to clear up.

 Whether we remove this though would be a change which should go
 through a separate patch I think.

Yeah, that's reasonable.

-- 
Bob Copeland %% www.bobcopeland.com
___
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel