Hi Roy,
Re: the subject: please next time put the version in the subjetc line too. You may benefit from using patman, which helps managing patch series and handles such things as adding version in suject lines and Cc:ing people according to tags, etc. see tools/patman/README.
Also:
On Fri, 20 Dec 2013 13:32:34 +0100, Roy Spliet rspliet@eclipso.eu wrote:
Required for (but potentially not limited to) the snowball board. Implementation is inspired by the linux smsc911x implementation, but by using a (pre-compiler) constant, things should be optimised by the compiler for a shift of 0.
Signed-off-by: Roy Spliet rspliet@eclipso.eu
drivers/net/smc911x.h | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/net/smc911x.h b/drivers/net/smc911x.h index acae0cf..7144722 100644 --- a/drivers/net/smc911x.h +++ b/drivers/net/smc911x.h @@ -19,6 +19,12 @@ CONFIG_SMC911X_16_BIT shall be set" #endif
+#ifndef CONFIG_SMC911X_SHIFT +#define CONFIG_SMC911X_SHIFT 0 +#endif
+#define smc911x_shift(i) ((i) << CONFIG_SMC911X_SHIFT)
I find that defining a macro just to shift a field by a constant value is over-engineering. It is not really worse to do the (i << NNN) at the point(s) of use, and the reader will immediately understand what's being done.
#if defined (CONFIG_SMC911X_32_BIT) static inline u32 __smc911x_reg_read(struct eth_device *dev, u32 offset) { @@ -37,14 +43,14 @@ void smc911x_reg_write(struct eth_device *dev, u32 offset, u32 val) #elif defined (CONFIG_SMC911X_16_BIT) static inline u32 smc911x_reg_read(struct eth_device *dev, u32 offset) {
- volatile u16 *addr_16 = (u16 *)(dev->iobase + offset);
- return ((*addr_16 & 0x0000ffff) | (*(addr_16 + 1) << 16));
- volatile u16 *addr_16 = (u16 *)(dev->iobase + smc911x_shift(offset));
- return (*addr_16 & 0x0000FFFF) | (*(addr_16 + smc911x_shift(1)) << 16);
} static inline void smc911x_reg_write(struct eth_device *dev, u32 offset, u32 val) {
- *(volatile u16 *)(dev->iobase + offset) = (u16)val;
- *(volatile u16 *)(dev->iobase + offset + 2) = (u16)(val >> 16);
- *(volatile u16 *)(dev->iobase + smc911x_shift(offset)) = (u16)val;
- *(volatile u16 *)(dev->iobase + smc911x_shift(offset + 2)) = (u16)val;
} #else #error "SMC911X: undefined bus width"
Amicalement,