Hi Jens,
Jens Gehrlein wrote:
Hi Ben, Magnus,
here is my patch to enable 2x16 bit accesses to the LAN9x1x. I still not have a HW, so may I ask you to test it and, if applicable, fix the code?
Best Regards, Jens
drivers/net/smc911x.c | 17 ++++++++++++++++- 1 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index d22c889..841a64d 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -30,6 +30,8 @@ #include <net.h> #include <miiphy.h>
+#define CONFIG_DRIVER_SMC911X_16_BIT 1
#ifdef CONFIG_DRIVER_SMC911X_32_BIT static inline u32 reg_read(u32 addr) { @@ -39,8 +41,21 @@ static inline void reg_write(u32 addr, u32 val) { *(volatile u32*)addr = val; } +#elif CONFIG_DRIVER_SMC911X_16_BIT +static inline u32 reg_read(u32 addr) +{
- u32 val_lo = (u32)(*(volatile u16*)addr);
- u32 val_hi = (u32)(*(volatile u16*)(addr | 2));
This won't work. Presumably addr will already be a multiple of 2, so the bit-wise OR will make val_lo and val_hi identical. Here's how I'd do it:
volatile u16 *addr_16 = (u16 *)addr; return ((*addr_16 & 0x0000ffff) | (*(addr_16 + 1) << 16));
Of course, this could be done as a 1-liner with castings, but it becomes even more unreadable that way. The mask here might be unnecessary too.
- return (val_hi << 16) | val_lo;
+} +static inline void reg_write(u32 addr, u32 val) +{
- *(volatile u16*)addr = (u16)val;
- *(volatile u16*)(addr | 2) = (u16)(val >> 16);
Same problem here with the bitwise OR. Second line should be: *(volatile u16*)(addr + 2) = (u16)(val >> 16);
+} #else -#error "SMC911X: Only 32-bit bus is supported" +#error "SMC911X: undefined buswidth" #endif
#define mdelay(n) udelay((n)*1000)
Thanks for doing this! Hopefully Magnus will get a chance to try on real hardware and we can put this issue to rest.
regards, Ben