[RFC] smsc911x device tree binding questions

[Grant Likely]

On Fri, Sep 3, 2010 at 6:02 AM, Lorenzo Pieralisi
<Lorenzo.Pieralisi at arm.com> wrote:
>> -----Original Message-----
>> From: Steve.Glendinning at smsc.com [mailto:Steve.Glendinning at smsc.com]
>> Sent: 29 August 2010 13:00
>> To: Grant Likely; Lorenzo Pieralisi
>> Cc: Catalin Marinas; David Miller; devicetree-discuss at lists.ozlabs.org;
>> glikely at secretlab.ca; Ian.Saturley at smsc.com
>> Subject: Re: [RFC] smsc911x device tree binding questions
>>
>> Hi Lorenzo,
>>
>> > > - the SMSC911x host bus size can be retrieved from the chip idrev (for
>> > > chip idrev having just 16-bit bus IF) or for controllers like 9118
>> that
>> > > can have both (32/16), there is a D32/D16 HW strap readable from
>> HW_CFG.
>> > > The problem with these flags is that they define bus size from a CPU
>> > > perspective if I got it right, in particular if the CPU can access
>> > > the SMSC with 32-bit load/store to access 32 bit registers or
>> > > have to resort to two "locked" 16-bit accesses to ensure
>> > > atomicity. I think it is possible on some architectures to use a
>> 32-bit
>> > > load/store even if the smsc911x IF is 16-bit (e.g. bus downsize
>> converters,
>> > > which ensure 16-bit load/store locking).
>> > > It depends on the bus interconnect architecture. I think we
>> > > need a property to define this, thoughts much appreciated.
>>
>> Absolutely right - this flag defines whether the DRIVER has to use 16
>> or 32 bit accesses in order to succesfully access the chip.  If the bus
>> hardware is capable of performing atomic pairs of 16-bit operations
>> when we make a 32-bit read then this is very much preferred.  The 16-bit
>> path is MUCH slower as it has to take a spinlock on every access to
>> ensure the two reads/writes are atomic.  It's there as a last resort,
>> and its use is discouraged if avoidable.
>>
>> Autodetecting the bus width from register values is interesting, you'd
>> have to test that you can reliably read the registers in all of the above
>> modes.  It's my understanding that if the device is strapped for 16-bit
>> access it expects to see atomic pairs of reads, so I guess you could make
>> a pair of 32-bit reads (of something known like BYTE_TEST) and analyse
>> what you get back to determine how you're connected?
>
> Yes, we could try something like the code in smsc911x_init
> (BYTE_TEST pair of reads) but more generic to "probe" for the interconnection
> and set the flags accordingly.
>
> A question though(16-bit/32-bit controllers, e.g. 9118):
>
> If the device is strapped to 16-bit host bus mode, a single read 32-bit would
> reveal us if the bus converts the load 32-bit to two load 16-bit or
> not because the D[31:16] data bus lines are left in a high impedance state.
> Right ?
>
> Maybe a pair of reads - same address -  is safer, the controller invalidate
> the read (what does it mean in terms of bytes read ?) if it samples the same
> address (load 32 not split in 16-bit loads) on consecutive reads.
> If the conversion is correct the smsc returns the BYTE_TEST value otherwise
> we would notice the problem.
>
> I can write the code in the DT port context, but I do not have any
> 16-bit strapped smsc lan controller at hand, so I cannot test it, but I can post
> the patch for review and testing on the list.

It probably is not worth the effort.  Anyone bringing up this driver
on a new board will know what the bus access width is, and that will
never change.  Just encode it into a device tree property and be done
with it.

g.