[U-Boot] several problems with ethernet on MCF5445x

Dear list,
I am trying to get ethernet to work on my custom MCF54455 board and having some trouble.
I have a DP83848J PHY connected in MII mode to each of the FEC0 and FEC1 ports (seperate MDIO connection). Both PHYs are set to address 0x0. *)
In my config (basically copied from M54455EVB), I set:
#define CONFIG_NET_MULTI 1 #define CONFIG_MII 1 #define CONFIG_MII_INIT 1 #define CONFIG_SYS_DISCOVER_PHY #define CONFIG_SYS_FEC0_PINMUX 0 #define CONFIG_SYS_FEC1_PINMUX 0 #define CONFIG_SYS_FEC0_MIIBASE CONFIG_SYS_FEC0_IOBASE #define CONFIG_SYS_FEC1_MIIBASE CONFIG_SYS_FEC1_IOBASE [...] #define CONFIG_HAS_ETH1 /* * this is defined on a per-target basis in immap.h - not a very clean * solution, so I define it here: */ #define CONFIG_SYS_FEC1_IOBASE (MMAP_FEC1)
This gives me 2 FEC devices on startup.
My problems: - with CONFIG_SYS_FEC1_MIIBASE set as above, U-Boot crashes upon "Retry count exceeded; starting again" during unsuccessful dhcp; this does not happen when I set it to CONFIG_SYS_FEC0_IOBASE. (But this setting seems wrong to me, as I have seperate MDIO connections) - ethernet does not work at all unless I comment out if (info->iobase == CONFIG_SYS_FEC0_IOBASE) gpio->par_fec |= GPIO_PAR_FEC_FEC0_RMII_GPIO; else gpio->par_fec |= GPIO_PAR_FEC_FEC1_RMII_ATA; in mcf5445x/cpu_init.c: fecpin_setclear() (and initialize PAR_FEC in my own board init code). I do not understand what this code is for at all? - what sense do CONFIG_SYS_FEC[01]_PINMUX have? They are filled into the "pinmux" field of struct fec_info_s in mcffec.c, but I did not find a place using them? From their naming I would have expected them to be responsible for correct pin mux setting (PAR_FEC), but then they would have to be used by fecpin_setclear(), which again would introduce weird dependencies on the mcffec.c code.
*) Is it needed/useful to set both PHYs to different addresses although they are connected to seperate MDIO connections (and seemingly correctly found when booting the linux kernel)?
Can anybody comment on this? Did I overlook something about where the pin assignment is normally configured, or is it really just some strange coincidence that it works on the EVB but not on my board?
Regards, Wolfgang

On Tue, 2010-03-23 at 15:50 +0100, w.wegner@astro-kom.de wrote:
Dear list,
I am trying to get ethernet to work on my custom MCF54455 board and having some trouble.
I have a DP83848J PHY connected in MII mode to each of the FEC0 and FEC1 ports (seperate MDIO connection). Both PHYs are set to address 0x0. *)
Are you sure this is what you want? From the data sheet:
Strapping PHY Address 0 puts the part into Isolate Mode.
When in the MII isolate mode, the DP83848J does not respond to packet data present at TXD[3:0], TX_EN inputs and presents a high impedance on the TX_CLK, RX_CLK, RX_DV, RX_ER, RXD[3:0], COL, and CRS outputs. When in Isolate mode, the DP83848J will continue to respond to all management transactions.
John

Dear John,
On 23 Mar 2010 at 10:11, John Schmoller wrote:
On Tue, 2010-03-23 at 15:50 +0100, w.wegner@astro-kom.de wrote:
I have a DP83848J PHY connected in MII mode to each of the FEC0 and FEC1 ports (seperate MDIO connection). Both PHYs are set to address 0x0. *)
Are you sure this is what you want? From the data sheet:
Strapping PHY Address 0 puts the part into Isolate Mode.
[...]
sorry, my fault... I forgot about the internal pullup and did not read carefully that U-Boot correctly identified the PHY at address 1.
So, both PHYs are strapped to address 0x01.
My concern was, if there is any problem arising from two PHYs sharing the same address while being accessed through different physical interfaces. E.g., it seems I can only access one of the PHYs via the "mii" commands - I am not sure if this is a limitation originating from the identical addresses, or some other problem with my setup.
Thanks for pointing out that mistake!
Regards, Wolfgang

Wolfgang,
As long as you are not using only one FEC's mdio/mdc to communicate the both PHY at address 1.
Best Regards, TsiChung
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of w.wegner@astro-kom.de Sent: Tuesday, March 23, 2010 10:40 AM To: John Schmoller; u-boot@lists.denx.de Subject: Re: [U-Boot] several problems with ethernet on MCF5445x
Dear John,
On 23 Mar 2010 at 10:11, John Schmoller wrote:
On Tue, 2010-03-23 at 15:50 +0100, w.wegner@astro-kom.de wrote:
I have a DP83848J PHY connected in MII mode to each of the FEC0 and FEC1 ports (seperate MDIO connection). Both PHYs are set to address 0x0. *)
Are you sure this is what you want? From the data sheet:
Strapping PHY Address 0 puts the part into Isolate Mode.
[...]
sorry, my fault... I forgot about the internal pullup and did not read carefully that U-Boot correctly identified the PHY at address 1.
So, both PHYs are strapped to address 0x01.
My concern was, if there is any problem arising from two PHYs sharing the same address while being accessed through different physical interfaces. E.g., it seems I can only access one of the PHYs via the "mii" commands - I am not sure if this is a limitation originating from the identical addresses, or some other problem with my setup.
Thanks for pointing out that mistake!
Regards, Wolfgang
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Hi all,
I still have problems with the ethernet on my custom MCF54452 board.
Here is my current configuration:
/* this is defined for M54455EVB in m68k/immap.h *sigh* */ #define CONFIG_SYS_FEC1_IOBASE (MMAP_FEC1)
#define CONFIG_NET_MULTI 1 #define CONFIG_MII 1 #define CONFIG_MII_INIT 1 #define CONFIG_SYS_DISCOVER_PHY #define CONFIG_SYS_RX_ETH_BUFFER 8 #define CONFIG_SYS_FAULT_ECHO_LINK_DOWN #define CONFIG_SYS_FEC0_PINMUX 0 #define CONFIG_SYS_FEC1_PINMUX 0 #define CONFIG_SYS_FEC0_MIIBASE CONFIG_SYS_FEC0_IOBASE #define CONFIG_SYS_FEC1_MIIBASE CONFIG_SYS_FEC1_IOBASE #define MCFFEC_TOUT_LOOP 50000 #define CONFIG_HAS_ETH1
I modified fecpin_setclear() in cpu_init.c to set par_fec to GPIO_PAR_FEC_FEC[01]_MII, still have to make a proper configuration for this.
Now U-Boot recognizes both FEC0 and FEC1, but I can only access FEC0 with the mii commands. Switching to FEC1 is accepted, but all commands work on FEC0 (I can easily check this by manually switching the status LEDs via the PHY registers). FEC0 is working - though with frequent "TX timeout" messages, I did not yet figure out how to explicitly switch to FEC1.
When a command like dhcp times out and eth_init tries to switch to the other interface (FEC1) with eth_current->init(), U-Boot locks up. I still have to look further into this what exactly fails.
Furthermore, it seems strange to me that I have to set CONFIG_FEC_SHARED_PHY=y in the linux configuration to get both PHYs recognized, but I definitely have both PHYs wired to separate pins in the hardware.
What I am unsure about: - how am I supposed to set CONFIG_SYS_FEC1_MIIBASE? I think I have to set it to CONFIG_SYS_FEC1_IOBASE because I have seperate PHYs connected to seperate physical interfaces, is this correct? - is there maybe any relation to the (from my view inverted) setting of CONFIG_SHARED_PHY in the linux kernel?
Best regards, Wolfgang

Hi,
On 26 Mar 2010 at 14:13, w.wegner@astro-kom.de wrote: [...]
When a command like dhcp times out and eth_init tries to switch to the other interface (FEC1) with eth_current->init(), U-Boot locks up. I still have to look further into this what exactly fails.
the lockup itself is caused by mii_discover_phy in mcfmii.c.
Apparently, phytype is detected as 0x0, and then in this loop
for (i = 0; i < (sizeof(phyinfo) / sizeof(phy_info_t)); i++) { if (phyinfo[i].phyid == phytype) { #ifdef ET_DEBUG printf("phyid %x - %s\n", phyinfo[i].phyid, phyinfo[i].strid); #endif strcpy(info->phy_name, phyinfo[i].strid); info->phyname_init = 1; found = 1; break; } }
phytype is matched against the last entry of phyinfo erroneously:
phy_info_t phyinfo[] = { {0x0022561B, "AMD79C784VC"}, /* AMD 79C784VC */ [...] {0, 0} };
However, I did not yet find out why phytype is detected as 0x0...
In any case, what should be the correct termination condition for the above loop? Of course for (i = 0; i < (sizeof(phyinfo) / sizeof(phy_info_t) - 1); i++) would do the trick, but it seems overly complicated to me. Any better ideas?
Wolfgang

On 26 Mar 2010 at 15:55, w.wegner@astro-kom.de wrote:
phytype is matched against the last entry of phyinfo erroneously:
phy_info_t phyinfo[] = { {0x0022561B, "AMD79C784VC"}, /* AMD 79C784VC */ [...] {0, 0} };
ouch, things are even worse... I did not quote enough of the source code of mcfmii.c:
#ifndef CONFIG_SYS_UNSPEC_PHYID # define CONFIG_SYS_UNSPEC_PHYID 0 #endif #ifndef CONFIG_SYS_UNSPEC_STRID # define CONFIG_SYS_UNSPEC_STRID 0 #endif [...]
phy_info_t phyinfo[] = { {0x0022561B, "AMD79C784VC"}, /* AMD 79C784VC */ [...] #if defined(CONFIG_SYS_UNSPEC_PHYID) && defined(CONFIG_SYS_UNSPEC_STRID) {CONFIG_SYS_UNSPEC_PHYID, CONFIG_SYS_UNSPEC_STRID}, #endif {0, 0} };
So the list phyinfo is terminated by a double {0, 0}, {0, 0}.
Can phyid == 0x0 be used as a simple termination condition, or could this be a valid phy id?
Wolfgang

Hi all,
the problems boiled down to two things: - full MII and non-shared MII-management interface is not currently supported for MCF5445x - when a PHY returns id 0x0, matching for this ID results in a lock-up in mcfmii.c
considering PHY ID 0x0 as broken anyways, I could get my board to run with both ethernet interfaces with the patch below. As this is not yet thoroughly tested and my tree is too much out of sync at the moment, I do not provide a proper patch for now, but maybe this already helps somebody else in case he runs into the same problems.
Regards, Wolfgang
diff --git a/cpu/mcf5445x/cpu_init.c b/cpu/mcf5445x/cpu_init.c index 48b37df..8b86d6d 100644 --- a/cpu/mcf5445x/cpu_init.c +++ b/cpu/mcf5445x/cpu_init.c @@ -152,13 +152,32 @@ int fecpin_setclear(struct eth_device *dev, int setclear) struct fec_info_s *info = (struct fec_info_s *)dev->priv;
if (setclear) { +#ifdef CONFIG_SYS_FEC_NO_SHARED_MII + if (info->iobase == CONFIG_SYS_FEC0_IOBASE) + gpio->par_feci2c |= + (GPIO_PAR_FECI2C_MDC0_MDC0 | + GPIO_PAR_FECI2C_MDIO0_MDIO0); + else + gpio->par_feci2c |= + (GPIO_PAR_FECI2C_MDC1_MDC1 | + GPIO_PAR_FECI2C_MDIO1_MDIO1); +#else gpio->par_feci2c |= (GPIO_PAR_FECI2C_MDC0_MDC0 | GPIO_PAR_FECI2C_MDIO0_MDIO0); +#endif
if (info->iobase == CONFIG_SYS_FEC0_IOBASE) +#ifdef CONFIG_SYS_FEC_FULL_MII + gpio->par_fec |= GPIO_PAR_FEC_FEC0_MII; +#else gpio->par_fec |= GPIO_PAR_FEC_FEC0_RMII_GPIO; +#endif else +#ifdef CONFIG_SYS_FEC_FULL_MII + gpio->par_fec |= GPIO_PAR_FEC_FEC1_MII; +#else gpio->par_fec |= GPIO_PAR_FEC_FEC1_RMII_ATA; +#endif } else { gpio->par_feci2c &= ~(GPIO_PAR_FECI2C_MDC0_MDC0 | GPIO_PAR_FECI2C_MDIO0_MDIO0); diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c index 4acc29e..e83bb07 100644 --- a/drivers/net/mcfmii.c +++ b/drivers/net/mcfmii.c @@ -185,7 +185,10 @@ int mii_discover_phy(struct eth_device *dev) printf("PHY @ 0x%x pass %d\n", phyno, pass); #endif
- for (i = 0; i < (sizeof(phyinfo) / sizeof(phy_info_t)); i++) { + for (i = 0; + (i < (sizeof(phyinfo) / sizeof(phy_info_t))) + && (phyinfo[i].phyid != 0); + i++) { if (phyinfo[i].phyid == phytype) { #ifdef ET_DEBUG printf("phyid %x - %s\n",

Signed-off-by: Wolfgang Wegner w.wegner@astro-kom.de --- drivers/net/mcfmii.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c index 4acc29e..83c0873 100644 --- a/drivers/net/mcfmii.c +++ b/drivers/net/mcfmii.c @@ -185,7 +185,11 @@ int mii_discover_phy(struct eth_device *dev) printf("PHY @ 0x%x pass %d\n", phyno, pass); #endif
- for (i = 0; i < (sizeof(phyinfo) / sizeof(phy_info_t)); i++) { + for (i = 0; + (i < (sizeof(phyinfo) + / sizeof(phy_info_t))) + && (phyinfo[i].phyid != 0); + i++) { if (phyinfo[i].phyid == phytype) { #ifdef ET_DEBUG printf("phyid %x - %s\n",

This patch adds the possibility to handle seperate PHYs to MCF5445x. Naming is chosen to resemble the contrary CONFIG_FEC_SHARED_PHY in the linux kernel.
Signed-off-by: Wolfgang Wegner w.wegner@astro-kom.de --- cpu/mcf5445x/cpu_init.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/cpu/mcf5445x/cpu_init.c b/cpu/mcf5445x/cpu_init.c index 8d51d35..db03296 100644 --- a/cpu/mcf5445x/cpu_init.c +++ b/cpu/mcf5445x/cpu_init.c @@ -185,8 +185,19 @@ int fecpin_setclear(struct eth_device *dev, int setclear) struct fec_info_s *info = (struct fec_info_s *)dev->priv;
if (setclear) { +#ifdef CONFIG_SYS_FEC_NO_SHARED_PHY + if (info->iobase == CONFIG_SYS_FEC0_IOBASE) + gpio->par_feci2c |= + (GPIO_PAR_FECI2C_MDC0_MDC0 | + GPIO_PAR_FECI2C_MDIO0_MDIO0); + else + gpio->par_feci2c |= + (GPIO_PAR_FECI2C_MDC1_MDC1 | + GPIO_PAR_FECI2C_MDIO1_MDIO1); +#else gpio->par_feci2c |= (GPIO_PAR_FECI2C_MDC0_MDC0 | GPIO_PAR_FECI2C_MDIO0_MDIO0); +#endif
if (info->iobase == CONFIG_SYS_FEC0_IOBASE) gpio->par_fec |= GPIO_PAR_FEC_FEC0_RMII_GPIO;

This patch adds support for full MII interface on MCF5445x (in contrast to RMII as used on the evaluation boards).
Signed-off-by: Wolfgang Wegner w.wegner@astro-kom.de --- cpu/mcf5445x/cpu_init.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/cpu/mcf5445x/cpu_init.c b/cpu/mcf5445x/cpu_init.c index db03296..2389019 100644 --- a/cpu/mcf5445x/cpu_init.c +++ b/cpu/mcf5445x/cpu_init.c @@ -207,10 +207,19 @@ int fecpin_setclear(struct eth_device *dev, int setclear) gpio->par_feci2c &= ~(GPIO_PAR_FECI2C_MDC0_MDC0 | GPIO_PAR_FECI2C_MDIO0_MDIO0);
- if (info->iobase == CONFIG_SYS_FEC0_IOBASE) + if (info->iobase == CONFIG_SYS_FEC0_IOBASE) { +#ifdef CONFIG_SYS_FEC_FULL_MII + gpio->par_fec |= GPIO_PAR_FEC_FEC0_MII; +#else gpio->par_fec &= GPIO_PAR_FEC_FEC0_UNMASK; - else +#endif + } else { +#ifdef CONFIG_SYS_FEC_FULL_MII + gpio->par_fec |= GPIO_PAR_FEC_FEC1_MII; +#else gpio->par_fec &= GPIO_PAR_FEC_FEC1_UNMASK; +#endif + } } return 0; }

Acked.
Best Regards, TsiChung
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Wolfgang Wegner Sent: Tuesday, March 30, 2010 12:20 PM To: u-boot@lists.denx.de Cc: Wolfgang Wegner Subject: [U-Boot] [PATCH] add CONFIG_SYS_FEC_NO_SHARED_PHY for MCF5445x
This patch adds the possibility to handle seperate PHYs to MCF5445x. Naming is chosen to resemble the contrary CONFIG_FEC_SHARED_PHY in the linux kernel.
Signed-off-by: Wolfgang Wegner w.wegner@astro-kom.de --- cpu/mcf5445x/cpu_init.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/cpu/mcf5445x/cpu_init.c b/cpu/mcf5445x/cpu_init.c index 8d51d35..db03296 100644 --- a/cpu/mcf5445x/cpu_init.c +++ b/cpu/mcf5445x/cpu_init.c @@ -185,8 +185,19 @@ int fecpin_setclear(struct eth_device *dev, int setclear) struct fec_info_s *info = (struct fec_info_s *)dev->priv;
if (setclear) { +#ifdef CONFIG_SYS_FEC_NO_SHARED_PHY + if (info->iobase == CONFIG_SYS_FEC0_IOBASE) + gpio->par_feci2c |= + (GPIO_PAR_FECI2C_MDC0_MDC0 | + GPIO_PAR_FECI2C_MDIO0_MDIO0); + else + gpio->par_feci2c |= + (GPIO_PAR_FECI2C_MDC1_MDC1 | + GPIO_PAR_FECI2C_MDIO1_MDIO1); +#else gpio->par_feci2c |= (GPIO_PAR_FECI2C_MDC0_MDC0 | GPIO_PAR_FECI2C_MDIO0_MDIO0); +#endif
if (info->iobase == CONFIG_SYS_FEC0_IOBASE) gpio->par_fec |= GPIO_PAR_FEC_FEC0_RMII_GPIO; -- 1.5.6.5
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Hi Wolfgang,
On 3/30/2010 10:19 AM, Wolfgang Wegner wrote:
Signed-off-by: Wolfgang Wegnerw.wegner@astro-kom.de
drivers/net/mcfmii.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c index 4acc29e..83c0873 100644 --- a/drivers/net/mcfmii.c +++ b/drivers/net/mcfmii.c @@ -185,7 +185,11 @@ int mii_discover_phy(struct eth_device *dev) printf("PHY @ 0x%x pass %d\n", phyno, pass); #endif
for (i = 0; i< (sizeof(phyinfo) / sizeof(phy_info_t)); i++) {
for (i = 0;
(i< (sizeof(phyinfo)
/ sizeof(phy_info_t)))
&& (phyinfo[i].phyid != 0);
#ifdef ET_DEBUG printf("phyid %x - %s\n",i++) { if (phyinfo[i].phyid == phytype) {
This is brutal. Using 8-space tabs really does a good job of highlighting deep nesting of conditionals. If you're unable to make the driver easier to read (and I understand if that's the case), my preference would be to keep the sizeof()s on one line and combine the second && term and the i++ on another. This makes lines > 80 chars, but it was already that way.
regards, Ben

Hi Ben,
On 4 Apr 2010 at 22:44, Ben Warren wrote:
Hi Wolfgang,
On 3/30/2010 10:19 AM, Wolfgang Wegner wrote:
Signed-off-by: Wolfgang Wegnerw.wegner@astro-kom.de
drivers/net/mcfmii.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c index 4acc29e..83c0873 100644 --- a/drivers/net/mcfmii.c +++ b/drivers/net/mcfmii.c @@ -185,7 +185,11 @@ int mii_discover_phy(struct eth_device *dev) printf("PHY @ 0x%x pass %d\n", phyno, pass); #endif
for (i = 0; i< (sizeof(phyinfo) / sizeof(phy_info_t)); i++) {
for (i = 0;
(i< (sizeof(phyinfo)
/ sizeof(phy_info_t)))
&& (phyinfo[i].phyid != 0);
#ifdef ET_DEBUG printf("phyid %x - %s\n",i++) { if (phyinfo[i].phyid == phytype) {
This is brutal. Using 8-space tabs really does a good job of highlighting deep nesting of conditionals. If you're unable to make the driver easier to read (and I understand if that's the case), my preference would be to keep the sizeof()s on one line and combine the second && term and the i++ on another. This makes lines > 80 chars, but it was already that way.
I agree, but was worried if the > 80 char line would be a reason to reject the patch and wanted to have it as non-intrusive as possible.
If there are no objections, I am happy to supply a second version with formatting as you suggested.
Regards, Wolfgang

Signed-off-by: Wolfgang Wegner w.wegner@astro-kom.de --- modified formatting of inner for() loop and changed if(cond){...} to if(!cond) continue; to avoid deep indentation.
drivers/net/mcfmii.c | 45 +++++++++++++++++++++++---------------------- 1 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/net/mcfmii.c b/drivers/net/mcfmii.c index 4acc29e..060bdd7 100644 --- a/drivers/net/mcfmii.c +++ b/drivers/net/mcfmii.c @@ -175,38 +175,39 @@ int mii_discover_phy(struct eth_device *dev) #ifdef ET_DEBUG printf("PHY type 0x%x pass %d type\n", phytype, pass); #endif - if (phytype != 0xffff) { - phyaddr = phyno; - phytype <<= 16; - phytype |= - mii_send(mk_mii_read(phyno, PHY_PHYIDR2)); + if (phytype == 0xffff) + continue; + phyaddr = phyno; + phytype <<= 16; + phytype |= + mii_send(mk_mii_read(phyno, PHY_PHYIDR2));
#ifdef ET_DEBUG - printf("PHY @ 0x%x pass %d\n", phyno, pass); + printf("PHY @ 0x%x pass %d\n", phyno, pass); #endif
- for (i = 0; i < (sizeof(phyinfo) / sizeof(phy_info_t)); i++) { - if (phyinfo[i].phyid == phytype) { + for (i = 0; (i < (sizeof(phyinfo) / sizeof(phy_info_t))) + && (phyinfo[i].phyid != 0); i++) { + if (phyinfo[i].phyid == phytype) { #ifdef ET_DEBUG - printf("phyid %x - %s\n", - phyinfo[i].phyid, - phyinfo[i].strid); + printf("phyid %x - %s\n", + phyinfo[i].phyid, + phyinfo[i].strid); #endif - strcpy(info->phy_name, phyinfo[i].strid); - info->phyname_init = 1; - found = 1; - break; - } + strcpy(info->phy_name, phyinfo[i].strid); + info->phyname_init = 1; + found = 1; + break; } + }
- if (!found) { + if (!found) { #ifdef ET_DEBUG - printf("0x%08x\n", phytype); + printf("0x%08x\n", phytype); #endif - strcpy(info->phy_name, "unknown"); - info->phyname_init = 1; - break; - } + strcpy(info->phy_name, "unknown"); + info->phyname_init = 1; + break; } } }

Hi Wolfgang,
On 4/6/2010 2:13 AM, Wolfgang Wegner wrote:
Signed-off-by: Wolfgang Wegnerw.wegner@astro-kom.de
modified formatting of inner for() loop and changed if(cond){...} to if(!cond) continue; to avoid deep indentation.
drivers/net/mcfmii.c | 45 +++++++++++++++++++++++---------------------- 1 files changed, 23 insertions(+), 22 deletions(-)
Much better. Applied to net repo.
thanks, Ben

Hi,
I just sent another proposal having the for(...) in only two lines by replacing the outer indentation level if (phytype != 0xffff) {... } to if (phytype == 0xffff) continue;
Any comments on this?
Regards, Wolfgang
participants (6)
-
Ben Warren
-
John Schmoller
-
Liew Tsi Chung-R5AAHP
-
w.wegner@astro-kom.de
-
Wolfgang Wegner
-
wolfgang@leila.ping.de