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

[Marek Vasut]

On 11/23/2016 11:26 PM, Christoph Fritz wrote:
> This patch adds initial support for Samtec VIN|ING 2000 board.
> 
> Signed-off-by: Christoph Fritz <chf.fritz at 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
> Changes since v3:
>  - fix comment spelling
>  - use u32
>  - rearrange if condition

[...]

> diff --git a/board/samtec/vining_2000/Kconfig b/board/samtec/vining_2000/Kconfig
> new file mode 100644
> index 0000000..f40b3b4
> --- /dev/null
> +++ b/board/samtec/vining_2000/Kconfig
> @@ -0,0 +1,12 @@
> +if TARGET_VINING_2000

Darn, minor nit, should be TARGET_SAMTEC_VINING_2000 (for consistency
with VINING_FPGA).

> +config SYS_BOARD
> +	default "vining_2000"
> +
> +config SYS_VENDOR
> +	default "samtec"
> +
> +config SYS_CONFIG_NAME
> +	default "vining_2000"
> +
> +endif

[...]

> +#ifdef CONFIG_PWM_IMX
> +static int set_pwm_leds(void)
> +{
> +	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;

Looking at this, doesn't pwm_init() / pwm_config() return a valid error
code ? If so, propagate it out of this function instead of always
returning -EINVAL .

> +	/* 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;
> +
> +	return 0;
> +error:
> +	return -EINVAL;
> +}
> +#else
> +static int set_pwm_leds(void)
> +{
> +	return 0;
> +}
> +#endif

[...]

Looks great otherwise, thanks.

-- 
Best regards,
Marek Vasut