On Wed, Nov 4, 2009 at 2:29 PM, Matthew Wilcox <matthew@xxxxxx> wrote:
> On Wed, Nov 04, 2009 at 05:14:26PM -0500, Luis R. Rodriguez wrote:
>> On Wed, Nov 04, 2009 at 02:04:11PM -0800, Luis R. Rodriguez wrote:
>> > On Wed, Nov 4, 2009 at 2:00 PM, Matthew Wilcox <matthew@xxxxxx> wrote:
>> > > On Wed, Nov 04, 2009 at 01:52:30PM -0800, Luis R. Rodriguez wrote:
>> > >> > Even better: I just confirmation from our systems team that our legacy
>> > >> > devices and 11n PCI devices don't support MWR so I'll remove all that
>> > >> > cruft crap.
>> > >>
>> > >> I meant MWI of course.
>> > >
>> > > Yes, but they don't necessarily just use cacheline size for MWI ... some
>> > > devices use cacheline size for setting up data structures. ?Might be
>> > > worth just checking explicitly that they don't use the cacheline size
>> > > register for anything.
>> >
>> > Oh right -- so the typical Atheros hack for this is to check the cache
>> > line size, and if its 0 set it to L1_CACHE_BYTES. Then eventually read
>> > from PCI_CACHE_LINE_SIZE pci config to align the skb data. So what I
>> > was doing now is removing all this cruft and replacing it with a
>> > generic allocator for atheros drivers that aligns simply to the
>> > L1_CACHE_BYTES. Sound kosher?
>>
>> Something like this:
>
> Doesn't look kosher to me. You're not programming the CLS register
> now at all, which means you're relying on something else having set
> it up for you. If you could EXPORT_SYMBOL(pci_set_cacheline_size)
> and include a call to it somewhere, that would be good. You can rely
> on pci_cache_line_size not changing after the system has booted.
I see what you mean but the concern would be *if the hw does use it
for some internal stuff* right?
>> /*
>> - * Cache line size is used to size and align various
>> - * structures used to communicate with the hardware.
>> - */
>> - pci_read_config_byte(pdev, PCI_CACHE_LINE_SIZE, &csz);
>> - if (csz == 0) {
>> - /*
>> - * Linux 2.4.18 (at least) writes the cache line size
>> - * register as a 16-bit wide register which is wrong.
>> - * We must have this setup properly for rx buffer
>> - * DMA to work so force a reasonable value here if it
>> - * comes up zero.
>> - */
>> - csz = L1_CACHE_BYTES >> 2;
>> - pci_write_config_byte(pdev, PCI_CACHE_LINE_SIZE, csz);
>> - }
>
> These comments are what give me pause.
I see -- OK well I believe the above is in reference to driver code
which uses the written to value on the PCI_CACHE_LINE_SIZE to later
align the skb->data for our RX buffers.
At least our systems team does not believe we rely on the
PCI_CACHE_LINE_SIZE internally on the hardware for anything.
But if there is concern over the doubt of old hardware probably
relying it for some obscure things no one remembers then yeah its best
to leave it huh.
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
