| Message ID | 1250096221-11000-4-git-send-email-lrodriguez@atheros.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, Aug 12, 2009 at 12:57 PM, Luis R. Rodriguez<lrodriguez@atheros.com> wrote: > This matches ath9k, providing consistency when reading both drivers. > > Signed-off-by: Luis R. Rodriguez <lrodriguez@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?
On Wed, Aug 12, 2009 at 10:13 AM, Bob Copeland<me@bobcopeland.com> wrote: > On Wed, Aug 12, 2009 at 12:57 PM, Luis R. > Rodriguez<lrodriguez@atheros.com> wrote: >> This matches ath9k, providing consistency when reading both drivers. >> >> Signed-off-by: Luis R. Rodriguez <lrodriguez@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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 12, 2009 at 1:32 PM, Luis R. Rodriguez<lrodriguez@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.
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); } /* @@ -544,7 +544,7 @@ ath5k_pci_probe(struct pci_dev *pdev, __set_bit(ATH_STAT_INVALID, sc->status); sc->iobase = mem; /* So we can unmap it on detach */ - sc->common.cachelsz = csz * sizeof(u32); /* convert to bytes */ + sc->common.cachelsz = csz << 2; /* convert to bytes */ sc->opmode = NL80211_IFTYPE_STATION; sc->bintval = 1000; mutex_init(&sc->lock);
This matches ath9k, providing consistency when reading both drivers. Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com> --- drivers/net/wireless/ath/ath5k/base.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)