On 11/22/2016 03:48 PM, Christoph Fritz wrote:
>
Commit message is missing
[...]
> diff --git a/board/samtec/vining2000/imximage.cfg
> b/board/samtec/vining2000/imximage.cfg
> new file mode 100644
> index 0000000..4133dda
> --- /dev/null
> +++ b/board/samtec/vining2000/imximage.cfg
> @@ -0,0 +1,132 @@
> +/*
> + * Copyright (C) 2016 samtec automotive software & electronics gmbh
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#define __ASSEMBLY__
> +#include <config.h>
Is this needed at all ?
> +/* image version */
> +
> +IMAGE_VERSION 2
[...]
> diff --git a/board/samtec/vining2000/vining2000.c
> b/board/samtec/vining2000/vining2000.c
> new file mode 100644
> index 0000000..e73e57b
> --- /dev/null
> +++ b/board/samtec/vining2000/vining2000.c
[...]
> +static iomux_v3_cfg_t const uart1_pads[] = {
> + MX6_PAD_GPIO1_IO04__UART1_TX | MUX_PAD_CTRL(UART_PAD_CTRL),
> + MX6_PAD_GPIO1_IO05__UART1_RX | MUX_PAD_CTRL(UART_PAD_CTRL),
> +};
> +
> +static iomux_v3_cfg_t const usdhc2_pads[] = {
> + MX6_PAD_SD2_CLK__USDHC2_CLK | MUX_PAD_CTRL(0x10059),
Use macros for this MUX_PAD_CTRL() argument, fix globally.
> + MX6_PAD_SD2_CMD__USDHC2_CMD | MUX_PAD_CTRL(0x17059),
> + MX6_PAD_SD2_DATA0__USDHC2_DATA0 | MUX_PAD_CTRL(0x17059),
> + MX6_PAD_SD2_DATA1__USDHC2_DATA1 | MUX_PAD_CTRL(0x17059),
> + MX6_PAD_SD2_DATA2__USDHC2_DATA2 | MUX_PAD_CTRL(0x17059),
> + MX6_PAD_SD2_DATA3__USDHC2_DATA3 | MUX_PAD_CTRL(0x17059),
> + MX6_PAD_LCD1_VSYNC__GPIO3_IO_28 | MUX_PAD_CTRL(0x80000000),
> +};
[...]
> +int board_eth_init(bd_t *bis)
> +{
> + struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
> + int reg, ret;
> + unsigned char eth1addr[6];
> +
> + imx_iomux_v3_setup_multiple_pads(fec1_pads, ARRAY_SIZE(fec1_pads));
> + gpio_direction_output(IMX_GPIO_NR(5, 9) , 0);
> +
> + reg = readl(&iomuxc_regs->gpr[1]);
> + /* Generate phy reference clock via pin IOMUX ENET_REF_CLK1/2 by erasing
> + * ENET1/2_TX_CLK_DIR gpr1[14:13], so that reference clock is driven by
> + * ref_enetpll0/1.
> + */
Multiline comment has this format:
/*
* foo
* bar
*/
> + reg &= ~(IOMUX_GPR1_FEC1_CLOCK_MUX2_SEL_MASK |
> + IOMUX_GPR1_FEC2_CLOCK_MUX2_SEL_MASK);
> +
> + /* Enable ENET1/2_TX_CLK output driver. */
> + reg |= IOMUX_GPR1_FEC1_CLOCK_MUX1_SEL_MASK |
> + IOMUX_GPR1_FEC2_CLOCK_MUX1_SEL_MASK;
> + writel(reg, &iomuxc_regs->gpr[1]);
clrsetbits_le32() here.
> + ret = enable_fec_anatop_clock(0, ENET_50MHZ);
> +
> + if (ret)
> + printf("FEC anatop MXC: %s:failed (%0x)\n", __func__, ret);
> +
> + gpio_set_value(IMX_GPIO_NR(5, 9), 1);
> + mdelay(5);
> + gpio_direction_output(IMX_GPIO_NR(5, 9) , 0);
> + mdelay(5);
> + gpio_set_value(IMX_GPIO_NR(5, 9), 1);
> + mdelay(5);
What's this random GPIO poking for ?
> + ret = fecmxc_initialize_multi(bis, 0, CONFIG_FEC_MXC_PHYADDR,
> + IMX_FEC_BASE);
> + if (ret)
> + printf("FEC MXC: %s:failed\n", __func__);
> +
> + /* just to get secound mac address */
> + imx_get_mac_from_fuse(1, eth1addr);
> + if (is_valid_ethaddr(eth1addr))
> + if (!getenv("eth1addr"))
Wrap this into single conditional -- if (foo() && !bar)
> + eth_setenv_enetaddr("eth1addr", eth1addr);
> +
> + return ret;
> +}
> +
> +#define PC MUX_PAD_CTRL(I2C_PAD_CTRL)
> +/* I2C1 for PMIC */
> +static struct i2c_pads_info i2c_pad_info1 = {
> + .scl = {
> + .i2c_mode = MX6_PAD_GPIO1_IO00__I2C1_SCL | PC,
> + .gpio_mode = MX6_PAD_GPIO1_IO00__GPIO1_IO_0 | PC,
> + .gp = IMX_GPIO_NR(1, 0),
> + },
> + .sda = {
> + .i2c_mode = MX6_PAD_GPIO1_IO01__I2C1_SDA | PC,
> + .gpio_mode = MX6_PAD_GPIO1_IO01__GPIO1_IO_1 | PC,
> + .gp = IMX_GPIO_NR(1, 1),
> + },
> +};
> +
> +struct pmic *pfuze_init(unsigned char i2cbus)
static struct ?
> +{
> + struct pmic *p;
> + int ret;
> + unsigned int reg;
> +
> + ret = power_pfuze100_init(i2cbus);
> + if (ret)
> + return NULL;
> +
> + p = pmic_get("PFUZE100");
> + ret = pmic_probe(p);
> + if (ret)
> + return NULL;
> +
> + pmic_reg_read(p, PFUZE100_DEVICEID, ®);
> + printf("PMIC: PFUZE100 ID=0x%02x\n", reg);
> +
> + /* Set SW1AB stanby volage to 0.975V */
> + pmic_reg_read(p, PFUZE100_SW1ABSTBY, ®);
> + reg &= ~SW1x_STBY_MASK;
> + reg |= SW1x_0_975V;
> + pmic_reg_write(p, PFUZE100_SW1ABSTBY, reg);
> +
> + /* Set SW1AB/VDDARM step ramp up time from 16us to 4us/25mV */
> + pmic_reg_read(p, PFUZE100_SW1ABCONF, ®);
> + reg &= ~SW1xCONF_DVSSPEED_MASK;
> + reg |= SW1xCONF_DVSSPEED_4US;
> + pmic_reg_write(p, PFUZE100_SW1ABCONF, reg);
> +
> + /* Set SW1C standby voltage to 0.975V */
> + pmic_reg_read(p, PFUZE100_SW1CSTBY, ®);
> + reg &= ~SW1x_STBY_MASK;
> + reg |= SW1x_0_975V;
> + pmic_reg_write(p, PFUZE100_SW1CSTBY, reg);
> +
> + /* Set SW1C/VDDSOC step ramp up time from 16us to 4us/25mV */
> + pmic_reg_read(p, PFUZE100_SW1CCONF, ®);
> + reg &= ~SW1xCONF_DVSSPEED_MASK;
> + reg |= SW1xCONF_DVSSPEED_4US;
> + pmic_reg_write(p, PFUZE100_SW1CCONF, reg);
> +
> + return p;
> +}
> +
> +int pfuze_mode_init(struct pmic *p, u32 mode)
static int ?
> +{
> + unsigned char offset, i, switch_num;
> + u32 id;
> + int ret;
> +
> + pmic_reg_read(p, PFUZE100_DEVICEID, &id);
> + id = id & 0xf;
> +
> + if (id == 0) {
> + switch_num = 6;
> + offset = PFUZE100_SW1CMODE;
> + } else if (id == 1) {
> + switch_num = 4;
> + offset = PFUZE100_SW2MODE;
> + } else {
> + printf("Not supported, id=%d\n", id);
> + return -EINVAL;
> + }
> +
> + ret = pmic_reg_write(p, PFUZE100_SW1ABMODE, mode);
Why do you need to do this write ?
> + if (ret < 0) {
> + printf("Set SW1AB mode error!\n");
> + return ret;
> + }
> +
> + for (i = 0; i < switch_num - 1; i++) {
> + ret = pmic_reg_write(p, offset + i * SWITCH_SIZE, mode);
> + if (ret < 0) {
> + printf("Set switch 0x%x mode error!\n",
> + offset + i * SWITCH_SIZE);
> + return ret;
> + }
> + }
> +
> + return ret;
> +}
> +
> +int power_init_board(void)
> +{
> + struct pmic *p;
> + int ret;
> +
> + p = pfuze_init(I2C_PMIC);
> + if (!p)
> + return -ENODEV;
> +
> + ret = pfuze_mode_init(p, APS_PFM);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_USB_EHCI_MX6
> +#define USB_OTHERREGS_OFFSET 0x800
> +#define UCTRL_PWR_POL (1 << 9)
> +
> +static iomux_v3_cfg_t const usb_otg_pads[] = {
> + /* OGT1 */
> + MX6_PAD_GPIO1_IO09__USB_OTG1_PWR | MUX_PAD_CTRL(NO_PAD_CTRL),
> + MX6_PAD_GPIO1_IO10__ANATOP_OTG1_ID | MUX_PAD_CTRL(NO_PAD_CTRL),
> + /* OTG2 */
> + MX6_PAD_GPIO1_IO12__USB_OTG2_PWR | MUX_PAD_CTRL(NO_PAD_CTRL)
> +};
> +
> +static void setup_usb(void)
> +{
> + imx_iomux_v3_setup_multiple_pads(usb_otg_pads, ARRAY_SIZE(
> + usb_otg_pads));
> +}
> +
> +int board_usb_phy_mode(int port)
> +{
> + if (port == 1)
> + return USB_INIT_HOST;
> + else
> + return usb_phy_mode(port);
> +}
> +
> +int board_ehci_hcd_init(int port)
> +{
> + u32 *usbnc_usb_ctrl;
> +
> + if (port > 1)
> + return -EINVAL;
> +
> + usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET +
> + port * 4);
drivers/usb/host/ehci-mx6.c usb_power_config() does that for you ,
please fix.
> + /* Set Power polarity */
> + setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> +
> + return 0;
> +}
> +#endif
> +
> +int board_phy_config(struct phy_device *phydev)
> +{
> + if (phydev->drv->config)
> + phydev->drv->config(phydev);
Is this really needed?
> + return 0;
> +}
> +
> +int set_pwm_leds(void)
> +{
> +#ifdef CONFIG_PWM_IMX
> + imx_iomux_v3_setup_multiple_pads(pwm_led_pads, ARRAY_SIZE(
> + pwm_led_pads));
> + /* enable backlight PWM 2, green LED */
> + if (pwm_init(1, 0, 0))
> + goto error;
> + /* duty cycle 200ns, period: 8000ns */
> + if (pwm_config(1, 200, 8000))
> + goto error;
> + if (pwm_enable(1))
> + goto error;
> +
> + /* enable backlight PWM 1, blue LED */
> + if (pwm_init(0, 0, 0))
> + goto error;
> + /* duty cycle 200ns, period: 8000ns */
> + if (pwm_config(0, 200, 8000))
> + goto error;
> + if (pwm_enable(0))
> + goto error;
> +
> + /* enable backlight PWM 6, red LED */
> + if (pwm_init(5, 0, 0))
> + goto error;
> + /* duty cycle 200ns, period: 8000ns */
> + if (pwm_config(5, 200, 8000))
> + goto error;
> + if (pwm_enable(5))
> + goto error;
> +#else
> + imx_iomux_v3_setup_multiple_pads(gpio_led_pads, ARRAY_SIZE(
> + gpio_led_pads));
please don't split this in the middle of ARRAY_SIZE(, put thoe whole
ARRAY_SIZE on new line and indent below the gpio_led_pads. Fix globally.
> + gpio_direction_output(IMX_GPIO_NR(5, 14) , 1); /* green */
> + gpio_direction_output(IMX_GPIO_NR(5, 20) , 1); /* red */
> + gpio_direction_output(IMX_GPIO_NR(5, 15) , 1); /* blue */
> +#endif
> +
> + return 0;
> +error:
> + printf("error LED\n");
This error message is useless, either reword it or remove it.
> + return -1;
> +}
> +
> +#define ADC1_HC0 (ADC1_BASE_ADDR + 0x00)
> +#define ADC1_HS (ADC1_BASE_ADDR + 0x08)
> +#define ADC1_R0 (ADC1_BASE_ADDR + 0x0c)
> +#define ADC1_CFG (ADC1_BASE_ADDR + 0x14)
> +#define ADC1_GC (ADC1_BASE_ADDR + 0x18)
> +#define MAX_ADC_CNT 4096
> +
> +int read_adc(void)
static !
> +{
> + u32 reg;
> + unsigned int cnt;
> +
> + writel(0x308, ADC1_CFG);
Use macros instead of magic values.
> + /* start auto calibration */
> + reg = readl(ADC1_GC);
> + reg |= (1 << 7);
> + writel(reg, ADC1_GC);
> + for (cnt = 0; readl(ADC1_GC) & (1 << 7) || cnt > MAX_ADC_CNT; cnt++)
Use BIT(n) instead of (1 << n), in a macro.
> + udelay(1);
> + if (cnt > MAX_ADC_CNT)
> + goto adc_fail;
> +
> + /* start conversation */
> + writel(0, ADC1_HC0);
> +
> + /* wait for conversation */
> + for (cnt = 0; !readl(ADC1_HS) || cnt > MAX_ADC_CNT; cnt++)
> + udelay(1);
> + if (cnt > MAX_ADC_CNT)
> + goto adc_fail;
> +
> + /* read result */
> + reg = readl(ADC1_R0);
> +
> + return reg;
> +
> +adc_fail:
> + printf("ADC failure\n");
> + return -1;
> +}
> +
> +#define VAL_UPPER 2498
> +#define VAL_LOWER 1550
> +
> +int set_pin_state(void)
static ...
> +{
> + int val = read_adc();
> +
> + if (val >= VAL_UPPER)
> + setenv("pin_state", "connected");
> + else if (val < VAL_UPPER && val >= VAL_LOWER)
> + setenv("pin_state", "open");
> + else if (val < VAL_LOWER)
> + setenv("pin_state", "button");
> +
> + return val;
> +}
> +
> +int board_late_init(void)
> +{
> + int ret;
> +
> + ret = set_pwm_leds();
> + if (ret)
> + return ret;
> +
> + set_pin_state();
> +
> + return ret;
> +}
> +
> +int board_early_init_f(void)
> +{
> + setup_iomux_uart();
> +
> +#ifdef CONFIG_USB_EHCI_MX6
> + setup_usb();
Implement empty setup_usb() for case where CONFIG_USB_EHCI_MX6 is not
set and remove this check here since setup_usb() will always be defined.
> +#endif
> +
> + return 0;
> +}
> +
> +static struct fsl_esdhc_cfg usdhc_cfg[2] = {
> + {USDHC4_BASE_ADDR},
> + {USDHC2_BASE_ADDR, 0, 4},
> +};
> +
> +#define USDHC2_CD_GPIO IMX_GPIO_NR(3, 28)
> +
> +int board_mmc_getcd(struct mmc *mmc)
> +{
> + struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
> + int ret = 0;
> +
> + switch (cfg->esdhc_base) {
> + case USDHC4_BASE_ADDR:
> + ret = 1; /* Assume uSDHC4 is always present */
> + break;
> + case USDHC2_BASE_ADDR:
> + ret = !gpio_get_value(USDHC2_CD_GPIO);
> + break;
default: case is missing
> + }
> +
> + return ret;
> +}
> +
> +int board_mmc_init(bd_t *bis)
> +{
> + int ret;
> +
> + /*
> + * According to the board_mmc_init() the following map is done:
> + * (U-Boot device node) (Physical Port)
> + * mmc0 USDHC4
> + * mmc1 USDHC2
> + */
> + imx_iomux_v3_setup_multiple_pads(
> + usdhc4_pads, ARRAY_SIZE(usdhc4_pads));
> + usdhc_cfg[0].sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK);
> +
> + imx_iomux_v3_setup_multiple_pads(
> + usdhc2_pads, ARRAY_SIZE(usdhc2_pads));
> + gpio_direction_input(USDHC2_CD_GPIO);
> + usdhc_cfg[1].sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK);
> +
> + ret = fsl_esdhc_initialize(bis, &usdhc_cfg[0]);
> + ret |= fsl_esdhc_initialize(bis, &usdhc_cfg[1]);
Do not apply bitwise operations on signed integer values , just check
one after the other and fail with a meaningful error message.
> + if (ret) {
> + printf("Warning: failed to initialize mmc dev\n");
> + return ret;
> + }
> +
> + return 0;
> +}
[...]
> diff --git a/include/configs/vining2000.h b/include/configs/vining2000.h
> new file mode 100644
> index 0000000..a379ee2
> --- /dev/null
> +++ b/include/configs/vining2000.h
> @@ -0,0 +1,171 @@
> +/*
> + * Copyright (C) 2016 samtec automotive software & electronics gmbh
> + *
> + * Configuration settings for the Samtec VIN|ING 2000 board.
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#ifndef __CONFIG_H
> +#define __CONFIG_H
> +
> +#include "mx6_common.h"
> +
> +#ifdef CONFIG_SPL
> +#include "imx6_spl.h"
> +#endif
> +
> +/* Size of malloc() pool */
> +#define CONFIG_SYS_MALLOC_LEN (3 * SZ_1M)
> +
> +#define CONFIG_BOARD_EARLY_INIT_F
> +
> +#define CONFIG_MXC_UART
> +#define CONFIG_MXC_UART_BASE UART1_BASE
Look at the distro_bootcmd.h
> +#define CONFIG_EXTRA_ENV_SETTINGS \
> + "script=boot.scr\0" \
> + "image=zImage-vining2000\0" \
> + "console=ttymxc0\0" \
> + "fdt_high=0xffffffff\0" \
> + "initrd_high=0xffffffff\0" \
> + "initrd_addr=0x90000000\0" \
> + "fdt_file=imx6sx-samtec-vining2000.dtb\0" \
> + "fdt_addr=0x88000000\0" \
> + "boot_fdt=try\0" \
> + "loadaddr=0x80800000\0" \
> + "ip_dyn=yes\0" \
> + "mmcdev=0\0" \
> + "mmcpart=1\0" \
> + "mmcroot=/dev/mmcblk1p1 rootwait rw\0" \
> + "emmcargs=setenv bootargs console=${console},${baudrate} " \
> + "root=${mmcroot} fsckfix panic=3\0" \
> + "loadbootscript=" \
> + "fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${script};\0" \
Drop this fatload stuff, just use 'load' command and autodetect FS.
Avoid fat altogether if possible.
> + "bootscript=echo Running bootscript from mmc ...; " \
> + "source\0" \
> + "loadimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr} ${image}\0" \
> + "loadfdt=fatload mmc ${mmcdev}:${mmcpart} ${fdt_addr} ${fdt_file}\0" \
> + "emmcboot=echo Booting from emmc ...; " \
> + "run emmcargs; mmc dev 0 0; mmc partconf 0 1 0x1 0x1; " \
> + "fatload mmc 0:1 ${fdt_addr} oftree; " \
> + "fatload mmc 0:1 ${loadaddr} zImage; " \
> + "bootz ${loadaddr} - ${fdt_addr};\0" \
> + "netargs=setenv bootargs console=${console},${baudrate} " \
> + "root=/dev/nfs " \
> + "ip=dhcp rw \0" \
> + "netboot=echo Booting from net ...; " \
> + "run netargs; " \
> + "if test ${ip_dyn} = yes; then " \
> + "setenv get_cmd dhcp; " \
> + "else " \
> + "setenv get_cmd tftp; " \
> + "fi; " \
> + "${get_cmd} ${image}; " \
> + "if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \
> + "if ${get_cmd} ${fdt_addr} ${fdt_file}; then " \
> + "bootz ${loadaddr} - ${fdt_addr}; " \
> + "else " \
> + "if test ${boot_fdt} = try; then " \
> + "bootz; " \
> + "else " \
> + "echo WARN: Cannot load the DT; " \
> + "fi; " \
> + "fi; " \
> + "else " \
> + "bootz; " \
> + "fi;\0"
> +
> +#define CONFIG_BOOTCOMMAND \
> + "run emmcboot; run netboot;"
> +
> +/* Miscellaneous configurable options */
> +#define CONFIG_SYS_MEMTEST_START 0x80000000
[...]
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
