Skip to contentSkip to conversation

net: add 16 bit support for smc911x

PATCH New Jens Gehrlein 4 participants 8 messages Apr 29, 2008 – Apr 30, 2008 Network drivers

lore
Share & tools
lore

Jens Gehrlein

author
#0 lore raw

PATCH 1/1 net: add 16 bit support for smc911x

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(-)
drivers/net/smc911x.c +16 -1
index d22c889..841a64d 100644
@@ -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));
+
+	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);
+}
 #else
-#error "SMC911X: Only 32-bit bus is supported"
+#error "SMC911X: undefined buswidth"
 #endif
 
 #define mdelay(n)       udelay((n)*1000)
#1 lore raw

PATCH 1/1 net: add 16 bit support for smc911x

Hi Jens,

Quoted 29 lines · Jens Gehrlein

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(-)
drivers/net/smc911x.c +2 -0
index d22c889..841a64d 100644
@@ -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)
 {

drivers/net/smc911x.c static inline void reg_write(u32 addr, u32 val) · line 50

Quoted 10 lines
drivers/net/smc911x.c +6 -0
@@ -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.

drivers/net/smc911x.c static inline void reg_write(u32 addr, u32 val) · line 60

Quoted 7 lines
drivers/net/smc911x.c +6 -0
+	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);
Quoted 9 lines
drivers/net/smc911x.c +2 -1
+}
 #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

#2 lore raw

PATCH 1/1 net: add 16 bit support for smc911x

Quoted 2 lines

Thanks for doing this! Hopefully Magnus will get a chance to try on real hardware and we can put this issue to rest.

I'll try it later this week, definitely not today though.

/Magnus

#3 lore raw

PATCH 1/1 net: add 16 bit support for smc911x

Quoted 11 lines · Magnus Lilja
Quoted 3 lines

Thanks for doing this! Hopefully Magnus will get a chance to try on real hardware and we can put this issue to rest.

I'll try it later this week, definitely not today though.

/Magnus

Cool! Thanks Magnus.

regards,
Ben

#4 lore raw

PATCH 1/1 net: add 16 bit support for smc911x

In reply to Jens Gehrlein
Quoted 20 lines · Jens Gehrlein

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(-)
drivers/net/smc911x.c +0 -0
index d22c889..841a64d 100644

drivers/net/smc911x.c line 34

Quoted 5 lines
drivers/net/smc911x.c +2 -0
@@ -30,6 +30,8 @@
 #include <net.h>
 #include <miiphy.h>
 
+#define CONFIG_DRIVER_SMC911X_16_BIT 1
+

Looks like an obvious thing - you'll remove this define from the patch for final submission, right?

Thanks
Guennadi
---
Guennadi Liakhovetski

author
#5 lore raw

PATCH 1/1 net: add 16 bit support for smc911x

Quoted 25 lines · Guennadi Liakhovetski
Quoted 23 lines · Jens Gehrlein

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(-)
drivers/net/smc911x.c +0 -0
index d22c889..841a64d 100644
@@ -30,6 +30,8 @@
 #include <net.h>
 #include <miiphy.h>
Quoted 5 lines

drivers/net/smc911x.c line 34

Quoted 2 lines
drivers/net/smc911x.c +2 -0
+#define CONFIG_DRIVER_SMC911X_16_BIT 1
+

Looks like an obvious thing - you'll remove this define from the patch for final submission, right?

Oh, ah. You're right. I inserted this to do a compile test and to inspect the generated assembler code. This define should be later moved to the board specific header file.

Sorry.

Well spotted. Thank you.

Magnus, are you going to fix the code as proposed by Ben and Guennadi or
should I resubmit the patch?

Best Regards,
Jens

#6 lore raw

PATCH 1/1 net: add 16 bit support for smc911x

Hi Jens,

Quoted 39 lines · Jens Gehrlein
Quoted 26 lines · Guennadi Liakhovetski
Quoted 23 lines · Jens Gehrlein

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(-)
drivers/net/smc911x.c +0 -0
index d22c889..841a64d 100644
@@ -30,6 +30,8 @@
 #include <net.h>
 #include <miiphy.h>
Quoted 6 lines

drivers/net/smc911x.c

Quoted 3 lines
drivers/net/smc911x.c +2 -0
+#define CONFIG_DRIVER_SMC911X_16_BIT 1
+

Looks like an obvious thing - you'll remove this define from the patch for final submission, right?

Oh, ah. You're right. I inserted this to do a compile test and to inspect the generated assembler code. This define should be later moved to the board specific header file.

Quoted 7 lines

Sorry.

Well spotted. Thank you.

Magnus, are you going to fix the code as proposed by Ben and Guennadi or
should I resubmit the patch?

Ultimately, somebody needs to submit a patch. Since Magnus won't try it til later this week, it would be best if you post a patch incorporating changes. Then if it works, we're ready to go.

regards,
Ben

#7 lore raw

PATCH 1/1 net: add 16 bit support for smc911x

Hi all

Quoted 6 lines
Quoted 3 lines

Magnus, are you going to fix the code as proposed by Ben and Guennadi or
should I resubmit the patch?

Ultimately, somebody needs to submit a patch. Since Magnus won't try it til later this week, it would be best if you post a patch incorporating changes. Then if it works, we're ready to go.

May I suggest adding something like this to the smc911x.c-patch as well:

#if defined(CONFIG_DRIVER_SMC911X_32_BIT) && defined(CONFIG_DRIVER_SMC911X_16_BIT)
#error "SMC911X: Only one of CONFIG_DRIVER_SMC911X_32_BIT and CONFIG_DRIVER_SMC911X_16_BIT shall be set"
#endif

(but with the correct line wrapping, the above is written directly in gmail and I don't know how gmail will mangle the code)

Just in case someone accidentally defines both in the config file.

Also, I tested the patch with Ben's modifications and it seems to work on the i.MX31 Litekit board. Tried both ping and tftp file transfer with the 16 bit option (and 32 bit option).

Regards, Magnus