Re: [U-Boot] [PATCH v3 6/6] arm: mvf600: Add basic support for Vybrid MVF600TWR board

Hi, Benoit,
On Tuesday, May 21, 2013 11:03:01 AM, Alison Wang wrote:
MVF600TWR is a board based on Vybrid MVF600 SoC.
This patch adds basic support for Vybrid MVF600TWR board.
Signed-off-by: Alison Wang b18965@freescale.com Signed-off-by: Jason Jin Jason.jin@freescale.com Signed-off-by: TsiChung Liew tsicliew@gmail.com
[...]
diff --git a/board/freescale/mvf600twr/mvf600twr.c b/board/freescale/mvf600twr/mvf600twr.c new file mode 100644 index 0000000..71ee12b --- /dev/null +++ b/board/freescale/mvf600twr/mvf600twr.c @@ -0,0 +1,413 @@ +/*
- Copyright 2013 Freescale Semiconductor, Inc.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#include <common.h> +#include <asm/io.h> +#include <asm/arch/imx-regs.h> +#include <asm/arch/mvf_pins.h> +#include <asm/arch/crm_regs.h> +#include <asm/arch/clock.h> +#include <mmc.h> +#include <fsl_esdhc.h> +#include <miiphy.h> +#include <netdev.h>
+DECLARE_GLOBAL_DATA_PTR;
+#define UART_PAD_CTRL (PAD_CTL_PUS_100K_UP | PAD_CTL_SPEED_MED
| \
PAD_CTL_DSE_25ohm | PAD_CTL_OBE_IBE_ENABLE)
+#define ESDHC_PAD_CTRL (PAD_CTL_PUE | PAD_CTL_PUS_100K_UP | \
PAD_CTL_SPEED_HIGH | PAD_CTL_DSE_20ohm | \
PAD_CTL_OBE_IBE_ENABLE)
With the changes that I have suggested in my review of your IOMUX patch, ESDHC_PAD_CTRL could be simplified by removing PAD_CTL_PUE.
And without those changes, UART_PAD_CTRL and ENET_PAD_CTRL in your current code set pull values that are actually unused (unless the corresponding PKE/PUE bits do not exist and default to pull in the pad control registers used with these definitions).
[Alison Wang] Agree.
+#define ENET_PAD_CTRL (PAD_CTL_PUS_47K_UP | PAD_CTL_SPEED_HIGH
| \
PAD_CTL_DSE_50ohm | PAD_CTL_OBE_IBE_ENABLE)
+#define DDR_PAD_CTRL PAD_CTL_DSE_25ohm
MUX_PAD_CTRL() could be added to the 4 pad control definitions above in order to avoid repeating it everywhere below. But using MUX_PAD_CTRL() relies on the fact that the pad control values in mvf_pins.h are all 0 (which is the case, but this is dangerous if this is changed later), so a better approach could be to use NEW_PAD_CTRL(), e.g.: NEW_PAD_CTRL(MVF600_PAD_DDR_A15__DDR_A_15, DDR_PAD_CTRL), instead of: MVF600_PAD_DDR_A15__DDR_A_15 | MUX_PAD_CTRL(DDR_PAD_CTRL),
[Alison Wang] I have a question about using NEW_PAD_CTRL(). If NEW_PAD_CTRL() is used, should the pad control values for MVF600_PAD_DDR_A15__DDR_A_15 in mvf_pins.h be the same as DDR_PAD_CTRL? I saw you didn't use the same value on other platforms, how do you define it?
Best Regards, Alison Wang

Hi Alison,
On Wednesday, May 22, 2013 10:23:12 AM, Wang Huan-B18965 wrote:
[...]
+#define ENET_PAD_CTRL (PAD_CTL_PUS_47K_UP | PAD_CTL_SPEED_HIGH
| \
PAD_CTL_DSE_50ohm | PAD_CTL_OBE_IBE_ENABLE)
+#define DDR_PAD_CTRL PAD_CTL_DSE_25ohm
MUX_PAD_CTRL() could be added to the 4 pad control definitions above in order to avoid repeating it everywhere below. But using MUX_PAD_CTRL() relies on the fact that the pad control values in mvf_pins.h are all 0 (which is the case, but this is dangerous if this is changed later), so a better approach could be to use NEW_PAD_CTRL(), e.g.: NEW_PAD_CTRL(MVF600_PAD_DDR_A15__DDR_A_15, DDR_PAD_CTRL), instead of: MVF600_PAD_DDR_A15__DDR_A_15 | MUX_PAD_CTRL(DDR_PAD_CTRL),
[Alison Wang] I have a question about using NEW_PAD_CTRL(). If NEW_PAD_CTRL() is used, should the pad control values for MVF600_PAD_DDR_A15__DDR_A_15 in mvf_pins.h be the same as DDR_PAD_CTRL? I saw you didn't use the same value on other platforms, how do you define it?
No, you don't have to change mvf_pins.h. That's what NEW_PAD_CTRL() is useful for: You can have any pad control value defined in mvf_pins.h, and a board can override the pad control values when using definitions from mvf_pins.h, without having to modify mvf_pins.h.
E.g.: --- mvf_pins.h: MVF600_PAD_PTA6__RMII0_CLKIN = IOMUX_PAD(0x0, 0x0, 2, 0x0, 0, PAD_CTRL1),
mvf600twr.c: NEW_PAD_CTRL(MVF600_PAD_PTA6__RMII0_CLKIN, PAD_CTRL2), --- would have the same effect as a theoretical: --- mvf_pins.h: MVF600_PAD_PTA6__RMII0_CLKIN = IOMUX_PAD(0x0, 0x0, 2, 0x0, 0, PAD_CTRL2),
mvf600twr.c: MVF600_PAD_PTA6__RMII0_CLKIN, ---
But if you think that the pad control values that you have defined in mvf600twr.c are not specific to this board and should be used as the default pad control values for all boards based on the MVF600, then you should move those definitions to mvf_pins.h, and use them there, which means that you will no longer need MUX_PAD_CTRL() or NEW_PAD_CTRL() in mvf600twr.c: --- mvf_pins.h: #define MVF600_DDR_PAD_CTRL PAD_CTL_DSE_25ohm ... MVF600_PAD_DDR_A15__DDR_A_15 = IOMUX_PAD(0x0220, 0x0220, 0, 0x0000, 0, MVF600_DDR_PAD_CTRL),
mvf600twr.c: MVF600_PAD_DDR_A15__DDR_A_15, ---
Best regards, Benoît

Hi, Benoit,
On Wednesday, May 22, 2013 10:23:12 AM, Wang Huan-B18965 wrote:
[...]
+#define ENET_PAD_CTRL (PAD_CTL_PUS_47K_UP | PAD_CTL_SPEED_HIGH
| \
PAD_CTL_DSE_50ohm | PAD_CTL_OBE_IBE_ENABLE)
+#define DDR_PAD_CTRL PAD_CTL_DSE_25ohm
MUX_PAD_CTRL() could be added to the 4 pad control definitions
above
in order to avoid repeating it everywhere below. But using MUX_PAD_CTRL() relies on the fact that the pad control values in mvf_pins.h are all 0 (which is the case, but this is dangerous if this is changed later), so a better approach could be to use
NEW_PAD_CTRL(), e.g.:
NEW_PAD_CTRL(MVF600_PAD_DDR_A15__DDR_A_15, DDR_PAD_CTRL),
instead of: MVF600_PAD_DDR_A15__DDR_A_15 | MUX_PAD_CTRL(DDR_PAD_CTRL),
[Alison Wang] I have a question about using NEW_PAD_CTRL(). If NEW_PAD_CTRL() is used, should the pad control values for MVF600_PAD_DDR_A15__DDR_A_15 in mvf_pins.h be the same as DDR_PAD_CTRL? I saw you didn't use the same value on other platforms, how do you define it?
No, you don't have to change mvf_pins.h. That's what NEW_PAD_CTRL() is useful for: You can have any pad control value defined in mvf_pins.h, and a board can override the pad control values when using definitions from mvf_pins.h, without having to modify mvf_pins.h.
E.g.:
mvf_pins.h: MVF600_PAD_PTA6__RMII0_CLKIN = IOMUX_PAD(0x0, 0x0, 2, 0x0, 0, PAD_CTRL1),
mvf600twr.c: NEW_PAD_CTRL(MVF600_PAD_PTA6__RMII0_CLKIN, PAD_CTRL2),
would have the same effect as a theoretical:
mvf_pins.h: MVF600_PAD_PTA6__RMII0_CLKIN = IOMUX_PAD(0x0, 0x0, 2, 0x0, 0, PAD_CTRL2),
mvf600twr.c: MVF600_PAD_PTA6__RMII0_CLKIN,
But if you think that the pad control values that you have defined in mvf600twr.c are not specific to this board and should be used as the default pad control values for all boards based on the MVF600, then you should move those definitions to mvf_pins.h, and use them there, which means that you will no longer need MUX_PAD_CTRL() or NEW_PAD_CTRL() in mvf600twr.c:
mvf_pins.h: #define MVF600_DDR_PAD_CTRL PAD_CTL_DSE_25ohm ... MVF600_PAD_DDR_A15__DDR_A_15 = IOMUX_PAD(0x0220, 0x0220, 0, 0x0000, 0, MVF600_DDR_PAD_CTRL),
mvf600twr.c: MVF600_PAD_DDR_A15__DDR_A_15,
[Alison Wang] I see. Thanks.
Best Regards, Alison Wang
participants (2)
-
Benoît Thébaudeau
-
Wang Huan-B18965