Re: [U-Boot] [PATCH] Marvell 88EXXXX Switch/PHY init support
> Well, actually I'm more interested in mv88e60xx.c support :) BTW: Which specific chip support you are looking for? I will try if I can help you > > From log I can see > " > Marvell MV88E61XX Switch Driver support > " > are MV88E61XX and MV88E60XX the same? Not really... Regards.. Prafulla . . ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Marvell 88EXXXX Switch/PHY init support
Well, actually I'm more interested in mv88e60xx.c support :) >From log I can see " Marvell MV88E61XX Switch Driver support " are MV88E61XX and MV88E60XX the same? Regards, Sergey 2009/5/20 Prafulla Wadaskar : >> -Original Message- >> From: Sergey Nikulov [mailto:sergey.niku...@gmail.com] >> Sent: Thursday, May 21, 2009 4:20 AM >> To: Prafulla Wadaskar >> Cc: u-boot@lists.denx.de; Ronen Shitrit >> Subject: Re: [U-Boot] [PATCH] Marvell 88E Switch/PHY init support >> >> Hi, >> >> Any updates with this switches support in uboot? > Hi Sergey > You can find patch here. This will be merged soon > http://git.denx.de/?p=u-boot/u-boot-net.git;a=commit;h=b67f915a20e7bcb0206021decfb6d7f2013892f2 > > There are few updates to it that will follow the same path > http://lists.denx.de/pipermail/u-boot/2009-May/052905.html > > Regards.. > Prafulla . . > > -- Best Regards, Sergey Nikulov ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Marvell 88EXXXX Switch/PHY init support
> -Original Message- > From: Sergey Nikulov [mailto:sergey.niku...@gmail.com] > Sent: Thursday, May 21, 2009 4:20 AM > To: Prafulla Wadaskar > Cc: u-boot@lists.denx.de; Ronen Shitrit > Subject: Re: [U-Boot] [PATCH] Marvell 88EXXXX Switch/PHY init support > > Hi, > > Any updates with this switches support in uboot? Hi Sergey You can find patch here. This will be merged soon http://git.denx.de/?p=u-boot/u-boot-net.git;a=commit;h=b67f915a20e7bcb0206021decfb6d7f2013892f2 There are few updates to it that will follow the same path http://lists.denx.de/pipermail/u-boot/2009-May/052905.html Regards.. Prafulla . . ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Marvell 88EXXXX Switch/PHY init support
Hi, Any updates with this switches support in uboot? 2009/4/3 Prafulla Wadaskar : > From: prafulla_wadaskar > > Chips supprted:- > 1. 88E61XX 6 port gbe swtich with 5 integrated PHYs > 2. 88E6061 6 port fe swtich with 5 integrated PHYs > 3. 88E1116 gbe transceiver > > Contributors: > Yotam Admon > Michael Blostein > Signed-off-by: prafulla_wadaskar > Reviewed by: Ronen Shitrit > --- > -- Best Regards, Sergey Nikulov ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Marvell 88EXXXX Switch/PHY init support
Dear Prafulla Wadaskar,
In message <1238798370-9245-4-git-send-email-prafu...@marvell.com> you wrote:
> From: prafulla_wadaskar
>
> Chips supprted:-
> 1. 88E61XX 6 port gbe swtich with 5 integrated PHYs
> 2. 88E6061 6 port fe swtich with 5 integrated PHYs
> 3. 88E1116 gbe transceiver
>
> Contributors:
> Yotam Admon
> Michael Blostein
> Signed-off-by: prafulla_wadaskar
> Reviewed by: Ronen Shitrit
ben has already commented on the incorrect location of this code in
the directory hierarchy. I will restrict my review to formal issues.
> diff --git a/board/Marvell/common/mv88e1116.c
> b/board/Marvell/common/mv88e1116.c
> new file mode 100644
> index 000..87ec550
> --- /dev/null
> +++ b/board/Marvell/common/mv88e1116.c
...
> +#ifndef MV88E1116_DEBUG
> +#define MV88E1116_DEBUG 0
> +#endif
> +#define DEBUG_PRINT MV88E1116_DEBUG
> +
> +#include
> +#include
> +#include "../common/ppc_error_no.h"
See before (i. e. get rid of this stuff, here and everywhere else; it
will make the code jkust simpler).
> diff --git a/board/Marvell/common/mv88e60xx.c
> b/board/Marvell/common/mv88e60xx.c
> new file mode 100644
> index 000..6034f7b
> --- /dev/null
> +++ b/board/Marvell/common/mv88e60xx.c
...
> +static void mv_switch_88e60xx_vlan_init(u32 eth_port_num,
> + u32 switch_cpu_port,
> + u32 switch_max_ports_num,
> + u32 switch_ports_ofs,
> + u32 switch_enabled_ports_mask)
> +{
> + u32 prt;
> + u16 reg;
> +
> + debug_print_ftrace();
> + /* be sure all ports are disabled */
> + for (prt = 0; prt < switch_max_ports_num; prt++) {
> + mv_sw_eth_phy_reg_read(eth_port_num, (switch_ports_ofs + prt),
> +MV88E60XX_PORT_CONTROL_REG, ®);
> + reg &= ~0x3;
> + mv_sw_eth_phy_reg_write(eth_port_num, (switch_ports_ofs + prt),
> + MV88E60XX_PORT_CONTROL_REG, reg);
Please read the CodingStyle document about resonable choice oif namess
for functions, variables, etc. Names like
mv_switch_88e60xx_vlan_init(), mv_sw_eth_phy_reg_read(),
switch_max_ports_num, etc. are a pain to write, and a bigger pain to
read. Please use SIMPLE, readable names.
...
> +U_BOOT_CMD(smi, CONFIG_SYS_MAXARGS, 1, do_smi,
> +"smi - isues read/write command on smi for switch registers\n",
> +"smi read [smiaddr] [regaddr] [page]\n"
> +"- read smi register command\n"
> +"smi write [smiaddr] [regaddr] [value] [page]\n"
> +"- write to register command\n"
> +"- run the commands in the environment variable(s) 'var'\n");
The definition of the U_BOOT_CMD macro has changed, please fix your
...
> +U_BOOT_CMD(dump60xxphy, CONFIG_SYS_MAXARGS, 1, do_dump60xxphy,
> +"dump60xxphy - dump 88360xx registers\n",
> +"var [...]\n"
> +"- run the commands in the environment variable(s) 'var'\n");
> +#endif /* CONFIG_CMD_DUMP60XXPHY */
Ditto.
Hm... if we the function of this is to "dump 88360xx registers", then
why is the command name "dump60xxphy" ?
60xx != 88360xx ??
Ditto for the rest of the file.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Eeeek!
'eval' on strings should have been named 'evil'.-- Tom Phoenix in
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Marvell 88EXXXX Switch/PHY init support
Hi Prafulla, Prafulla Wadaskar wrote: > From: prafulla_wadaskar > > Chips supprted:- > 1. 88E61XX 6 port gbe swtich with 5 integrated PHYs > 2. 88E6061 6 port fe swtich with 5 integrated PHYs > 3. 88E1116 gbe transceiver > > Contributors: > Yotam Admon > Michael Blostein > Signed-off-by: prafulla_wadaskar > Reviewed by: Ronen Shitrit > I'll review these changes, but as-is placing chip support in 'board/Marvell' is inappropriate. If they are pure ethernet devices they need to go in 'drivers/net'. We also need to move the Marvell system controller chips that are already in 'board/Marvell', but I'm not sure where they should go. As you see, there's a tremendous amount of redundancy there. regards, Ben ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
