Re: [U-Boot] [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support
Dirk Behme wrote:
> Ben Warren wrote:
>> [EMAIL PROTECTED] wrote:
>>
>>> Subject: [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support
>>>
>>> From: Dirk Behme <[EMAIL PROTECTED]>
>>>
>>> Add I2C and network support
>>>
>>>
>>
>> I2C and network bits are kinda unrelated, don't you think?
>>
>
> Ben: Yes, you are right ;) The OMAP3 patch is ~300k and I tried to put
> this in as less patches as possible while having no patch > 40k. This
> resulted in some more or less unrelated code in one patch. Alternative
> would be to have more than the ~11 patches at the list we already have.
>
>>> Index: u-boot_master/net/eth.c
>>> ===
>>> --- u-boot_master.orig/net/eth.c
>>> +++ u-boot_master/net/eth.c
>>> @@ -508,7 +508,7 @@ extern int emac4xx_miiphy_initialize(bd_
>>> extern int mcf52x2_miiphy_initialize(bd_t *bis);
>>> extern int ns7520_miiphy_initialize(bd_t *bis);
>>> extern int davinci_eth_miiphy_initialize(bd_t *bis);
>>> -
>>> +extern int eth_init(bd_t *bd);
>>>
>>> int eth_initialize(bd_t *bis)
>>> {
>>> @@ -532,6 +532,9 @@ int eth_initialize(bd_t *bis)
>>> #if defined(CONFIG_DRIVER_TI_EMAC)
>>> davinci_eth_miiphy_initialize(bis);
>>> #endif
>>> +#if defined(CONFIG_DRIVER_SMC911X)
>>> +eth_init(bis);
>>>
>>
>> This isn't the right place to call eth_init(). I know the namespaces
>> are pretty convoluted, but the eth_initialize() family of functions
>> are intended to do things like register devices, initialize data
>> structures etc. without actually enabling the device. eth_init()
>> enables a device. The SMC911X driver doesn' t have such a thing,
>> which is why none of the other boards that use this chip have
>> anything in this file.
>
> Mani, Steve: Any comments on this?
>
> Ben: Any hint where in existing code it is done right to take this as
> example?
>
eth_init() will get called in NetLoop() (net/net.c) whenever you send a
packet. You should never have to call it explicitly. There doesn't
appear to be a separate driver initialization for this controller.
> Thanks
>
> Dirk
regards,
Ben
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support
Ben Warren wrote:
> [EMAIL PROTECTED] wrote:
>
>> Subject: [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support
>>
>> From: Dirk Behme <[EMAIL PROTECTED]>
>>
>> Add I2C and network support
>>
>>
>
> I2C and network bits are kinda unrelated, don't you think?
>
Ben: Yes, you are right ;) The OMAP3 patch is ~300k and I tried to put
this in as less patches as possible while having no patch > 40k. This
resulted in some more or less unrelated code in one patch. Alternative
would be to have more than the ~11 patches at the list we already have.
>> Index: u-boot_master/net/eth.c
>> ===
>> --- u-boot_master.orig/net/eth.c
>> +++ u-boot_master/net/eth.c
>> @@ -508,7 +508,7 @@ extern int emac4xx_miiphy_initialize(bd_
>> extern int mcf52x2_miiphy_initialize(bd_t *bis);
>> extern int ns7520_miiphy_initialize(bd_t *bis);
>> extern int davinci_eth_miiphy_initialize(bd_t *bis);
>> -
>> +extern int eth_init(bd_t *bd);
>>
>> int eth_initialize(bd_t *bis)
>> {
>> @@ -532,6 +532,9 @@ int eth_initialize(bd_t *bis)
>> #if defined(CONFIG_DRIVER_TI_EMAC)
>> davinci_eth_miiphy_initialize(bis);
>> #endif
>> +#if defined(CONFIG_DRIVER_SMC911X)
>> +eth_init(bis);
>>
>
> This isn't the right place to call eth_init(). I know the namespaces are
> pretty convoluted, but the eth_initialize() family of functions are
> intended to do things like register devices, initialize data structures
> etc. without actually enabling the device. eth_init() enables a device.
> The SMC911X driver doesn' t have such a thing, which is why none of the
> other boards that use this chip have anything in this file.
Mani, Steve: Any comments on this?
Ben: Any hint where in existing code it is done right to take this as
example?
Thanks
Dirk
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support
[EMAIL PROTECTED] wrote:
> Subject: [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support
>
> From: Dirk Behme <[EMAIL PROTECTED]>
>
> Add I2C and network support
>
>
I2C and network bits are kinda unrelated, don't you think?
> Index: u-boot_master/net/eth.c
> ===
> --- u-boot_master.orig/net/eth.c
> +++ u-boot_master/net/eth.c
> @@ -508,7 +508,7 @@ extern int emac4xx_miiphy_initialize(bd_
> extern int mcf52x2_miiphy_initialize(bd_t *bis);
> extern int ns7520_miiphy_initialize(bd_t *bis);
> extern int davinci_eth_miiphy_initialize(bd_t *bis);
> -
> +extern int eth_init(bd_t *bd);
>
> int eth_initialize(bd_t *bis)
> {
> @@ -532,6 +532,9 @@ int eth_initialize(bd_t *bis)
> #if defined(CONFIG_DRIVER_TI_EMAC)
> davinci_eth_miiphy_initialize(bis);
> #endif
> +#if defined(CONFIG_DRIVER_SMC911X)
> + eth_init(bis);
>
This isn't the right place to call eth_init(). I know the namespaces are
pretty convoluted, but the eth_initialize() family of functions are
intended to do things like register devices, initialize data structures
etc. without actually enabling the device. eth_init() enables a device.
The SMC911X driver doesn' t have such a thing, which is why none of the
other boards that use this chip have anything in this file.
> +#endif
> return 0;
> }
> #endif
> ___
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support
Subject: [PATCH 08/11 v1] ARM: OMAP3: Add I2C and network support
From: Dirk Behme <[EMAIL PROTECTED]>
Add I2C and network support
Signed-off-by: Dirk Behme <[EMAIL PROTECTED]>
---
drivers/i2c/Makefile |1
drivers/i2c/omap24xx_i2c.c | 132 ++---
drivers/net/Makefile |1
net/eth.c |5 +
4 files changed, 84 insertions(+), 55 deletions(-)
Index: u-boot_master/drivers/i2c/Makefile
===
--- u-boot_master.orig/drivers/i2c/Makefile
+++ u-boot_master/drivers/i2c/Makefile
@@ -29,6 +29,7 @@ COBJS-$(CONFIG_FSL_I2C) += fsl_i2c.o
COBJS-$(CONFIG_I2C_MXC) += mxc_i2c.o
COBJS-$(CONFIG_DRIVER_OMAP1510_I2C) += omap1510_i2c.o
COBJS-$(CONFIG_DRIVER_OMAP24XX_I2C) += omap24xx_i2c.o
+COBJS-$(CONFIG_DRIVER_OMAP34XX_I2C) += omap24xx_i2c.o
COBJS-$(CONFIG_SOFT_I2C) += soft_i2c.o
COBJS-$(CONFIG_TSI108_I2C) += tsi108_i2c.o
Index: u-boot_master/drivers/i2c/omap24xx_i2c.c
===
--- u-boot_master.orig/drivers/i2c/omap24xx_i2c.c
+++ u-boot_master/drivers/i2c/omap24xx_i2c.c
@@ -25,8 +25,10 @@
#include
#include
+#define inb(a) __raw_readb(a)
+#define outb(a, v) __raw_writeb(a, v)
#define inw(a) __raw_readw(a)
-#define outw(a,v) __raw_writew(a,v)
+#define outw(a, v) __raw_writew(a, v)
static void wait_for_bb (void);
static u16 wait_for_pin (void);
@@ -40,28 +42,28 @@ void i2c_init (int speed, int slaveadd)
udelay(1000);
outw(0x0, I2C_SYSC); /* will probably self clear but */
- if (inw (I2C_CON) & I2C_CON_EN) {
- outw (0, I2C_CON);
+ if (inw(I2C_CON) & I2C_CON_EN) {
+ outw(0, I2C_CON);
udelay (5);
}
/* 12Mhz I2C module clock */
- outw (0, I2C_PSC);
+ outw(0, I2C_PSC);
speed = speed/1000; /* 100 or 400 */
scl = ((12000/(speed*2)) - 7); /* use 7 when PSC = 0 */
- outw (scl, I2C_SCLL);
- outw (scl, I2C_SCLH);
+ outw(scl, I2C_SCLL);
+ outw(scl, I2C_SCLH);
/* own address */
- outw (slaveadd, I2C_OA);
- outw (I2C_CON_EN, I2C_CON);
+ outw(slaveadd, I2C_OA);
+ outw(I2C_CON_EN, I2C_CON);
/* have to enable intrrupts or OMAP i2c module doesn't work */
- outw (I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE |
- I2C_IE_NACK_IE | I2C_IE_AL_IE, I2C_IE);
+ outw(I2C_IE_XRDY_IE | I2C_IE_RRDY_IE | I2C_IE_ARDY_IE |
+I2C_IE_NACK_IE | I2C_IE_AL_IE, I2C_IE);
udelay (1000);
flush_fifo();
- outw (0x, I2C_STAT);
- outw (0, I2C_CNT);
+ outw(0x, I2C_STAT);
+ outw(0, I2C_CNT);
}
static int i2c_read_byte (u8 devaddr, u8 regoffset, u8 * value)
@@ -73,11 +75,11 @@ static int i2c_read_byte (u8 devaddr, u8
wait_for_bb ();
/* one byte only */
- outw (1, I2C_CNT);
+ outw(1, I2C_CNT);
/* set slave address */
- outw (devaddr, I2C_SA);
+ outw(devaddr, I2C_SA);
/* no stop bit needed here */
- outw (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX, I2C_CON);
+ outw(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX, I2C_CON);
status = wait_for_pin ();
@@ -85,7 +87,7 @@ static int i2c_read_byte (u8 devaddr, u8
/* Important: have to use byte access */
*(volatile u8 *) (I2C_DATA) = regoffset;
udelay (2);
- if (inw (I2C_STAT) & I2C_STAT_NACK) {
+ if (inw(I2C_STAT) & I2C_STAT_NACK) {
i2c_error = 1;
}
} else {
@@ -94,42 +96,46 @@ static int i2c_read_byte (u8 devaddr, u8
if (!i2c_error) {
/* free bus, otherwise we can't use a combined transction */
- outw (0, I2C_CON);
- while (inw (I2C_STAT) || (inw (I2C_CON) & I2C_CON_MST)) {
+ outw(0, I2C_CON);
+ while (inw(I2C_STAT) || (inw(I2C_CON) & I2C_CON_MST)) {
udelay (1);
/* Have to clear pending interrupt to clear I2C_STAT */
- outw (0x, I2C_STAT);
+ outw(0x, I2C_STAT);
}
wait_for_bb ();
/* set slave address */
- outw (devaddr, I2C_SA);
+ outw(devaddr, I2C_SA);
/* read one byte from slave */
- outw (1, I2C_CNT);
+ outw(1, I2C_CNT);
/* need stop bit here */
- outw (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP,
- I2C_CON);
+ outw(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP,
+I2C_CON);
status = wait_for_pin ();
if (status & I2C_STAT_RRDY) {
- *value = inw (I2C_D
