Re: [U-Boot] [PATCH v3] mx6sx: Add initial support for Samtec VIN|ING 2000 board

On 11/23/2016 07:39 PM, Christoph Fritz wrote:

This patch adds initial support for Samtec VIN|ING 2000 board.

Signed-off-by: Christoph Fritz chf.fritz@googlemail.com

Changes since v1:

• add more comments (enet phy init)
• fix layout style e.g. multi-line-comments
• use pinmux macros
• use helper funcs clrsetbits_le32() and wait_for_bit()
• make some functions static
• drop board_ehci_hcd_init() and board_phy_config()
• simplify environment

Changes since v2:

• adapt error handling in board_eth_init() and read_adc()
• purge unused macros
• use config_distro_bootcmd as environment
• fix CONFIG_PWM_IMX undef handling

[...]

+int board_eth_init(bd_t *bis) +{

• struct iomuxc *iomuxc_regs = (struct iomuxc *)IOMUXC_BASE_ADDR;
• int ret;
• unsigned char eth1addr[6];
• /* just to get secound mac address */
• imx_get_mac_from_fuse(1, eth1addr);
• if (!getenv("eth1addr") && is_valid_ethaddr(eth1addr))

• eth_setenv_enetaddr("eth1addr", eth1addr);
  

• imx_iomux_v3_setup_multiple_pads(fec1_pads, ARRAY_SIZE(fec1_pads));
• /*

• * 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 and enable ENET1/2_TX_CLK output driver.
  

• */
  

• clrsetbits_le32(&iomuxc_regs->gpr[1],

• 	IOMUX_GPR1_FEC1_CLOCK_MUX2_SEL_MASK |
  

• 	IOMUX_GPR1_FEC2_CLOCK_MUX2_SEL_MASK,
  

• 	IOMUX_GPR1_FEC1_CLOCK_MUX1_SEL_MASK |
  

• 	IOMUX_GPR1_FEC2_CLOCK_MUX1_SEL_MASK);
  

• ret = enable_fec_anatop_clock(0, ENET_50MHZ);
• if (ret) {

You can drop the newline above.

• printf("FEC anatop MXC: %s:failed (%0x)\n",  __func__, ret);
  

• goto eth_fail;
  

• }

[...]

+static int read_adc(unsigned int *val)

s/unsigned int/u32/

+{

• int ret;
• void __iomem *b = map_physmem(ADC1_BASE_ADDR, 0x100, MAP_NOCACHE);
• /* use software mode */
• writel(ADCx_CFG_SWMODE, b + ADCx_CFG);
• /* start auto calibration */
• setbits_le32(b + ADCx_GC, ADCx_GC_CAL);
• ret = wait_for_bit("ADC", b + ADCx_GC, ADCx_GC_CAL, ADCx_GC_CAL, 10, 0);
• if (ret)

• goto adc_exit;
  

• /* start conversation */

"conversion"

• writel(0, b + ADCx_HC0);
• /* wait for conversation */

DTTO

• ret = wait_for_bit("ADC", b + ADCx_HS, ADCx_HS_C0, ADCx_HS_C0, 10, 0);
• if (ret)

• goto adc_exit;
  

• /* read result */
• *val = readl(b + ADCx_R0);

+adc_exit:

• if (ret)

• printf("ADC failure (ret=%i)\n", ret);
  

• unmap_physmem(b, MAP_NOCACHE);
• return ret;

+}

+#define VAL_UPPER 2498 +#define VAL_LOWER 1550

+static int set_pin_state(void) +{

• unsigned int val;
• int ret;
• ret = read_adc(&val);
• if (ret)

• return ret;
  

• 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)

Isn't this just catch-all statement ? You can drop the condition.

• setenv("pin_state", "button");
  

• return ret;

+}

[...]