[U-Boot] [PATCH] arm, imx6: add support for aristainetos2 board

[Heiko Schocher <hs@denx.de>]

Hello Stefano,

added Anatolij to cc for the rotate logo question ...

Am 08.05.2015 13:19, schrieb Stefano Babic:
> Hi Heiko,
>
> On 12/04/2015 10:24, Heiko Schocher wrote:
>> add support for imx6dl based aristainetos2 board
>>
>> U-Boot 2015.04-rc5-00066-g60f6ed4 (Apr 10 2015 - 08:46:27)
>>
>> CPU:   Freescale i.MX6DL rev1.1 at 792 MHz
>> Reset cause: WDOG
>> Board: aristaitenos2
>>         Watchdog enabled
>> I2C:   ready
>> DRAM:  1 GiB
>> NAND:  1024 MiB
>> MMC:   FSL_SDHC: 0
>> SF: Detected N25Q128A with page size 256 Bytes, erase size 64 KiB, total 16 MiB
>> Display: lg4573 (480x800)
>> In:    serial
>> Out:   serial
>> Err:   serial
>> Net:   FEC [PRIME]
>> Hit any key to stop autoboot:  0
>> =>
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> ---
>>
>>   arch/arm/Kconfig                      |   5 +
>>   board/aristainetos2/Kconfig           |  12 +
>>   board/aristainetos2/MAINTAINERS       |   6 +
>>   board/aristainetos2/Makefile          |  12 +
>>   board/aristainetos2/aristainetos2.c   | 917 ++++++++++++++++++++++++++++++++++
>>   board/aristainetos2/aristainetos2.cfg |  34 ++
>>   board/aristainetos2/axi.cfg           |  22 +
>>   board/aristainetos2/clocks.cfg        |  24 +
>>   board/aristainetos2/ddr-setup.cfg     |  59 +++
>>   board/aristainetos2/nt5cc256m16cp.cfg |  60 +++
>>   configs/aristainetos2_defconfig       |   3 +
>>   include/configs/aristainetos2.h       | 351 +++++++++++++
>>   12 files changed, 1505 insertions(+)
>>   create mode 100644 board/aristainetos2/Kconfig
>>   create mode 100644 board/aristainetos2/MAINTAINERS
>>   create mode 100644 board/aristainetos2/Makefile
>>   create mode 100644 board/aristainetos2/aristainetos2.c
>>   create mode 100644 board/aristainetos2/aristainetos2.cfg
>>   create mode 100644 board/aristainetos2/axi.cfg
>>   create mode 100644 board/aristainetos2/clocks.cfg
>>   create mode 100644 board/aristainetos2/ddr-setup.cfg
>>   create mode 100644 board/aristainetos2/nt5cc256m16cp.cfg
>>   create mode 100644 configs/aristainetos2_defconfig
>>   create mode 100644 include/configs/aristainetos2.h
>>
[...]
>> diff --git a/board/aristainetos2/aristainetos2.c b/board/aristainetos2/aristainetos2.c
>> new file mode 100644
>> index 0000000..98e1bc7
>> --- /dev/null
>> +++ b/board/aristainetos2/aristainetos2.c
>> @@ -0,0 +1,917 @@
[...]
>> +struct i2c_pads_info i2c_pad_info4 = {
>> +	.scl = {
>> +		.i2c_mode = MX6_PAD_GPIO_7__I2C4_SCL | PC,
>> +		.gpio_mode = MX6_PAD_GPIO_7__GPIO1_IO07 | PC,
>> +		.gp = IMX_GPIO_NR(1, 7)
>> +	},
>> +	.sda = {
>> +		.i2c_mode = MX6_PAD_GPIO_8__I2C4_SDA | PC,
>> +		.gpio_mode = MX6_PAD_GPIO_8__GPIO1_IO08 | PC,
>> +		.gp = IMX_GPIO_NR(1, 8)
>> +	}
>> +};
>> +
>> +static int lgdisplay;
>
> It looks like strange to pass a parameter via a global variable. No
> other way ?

Hmm.. yes, I can check the "panel" environmentvariable ... changed.

>> +
>> +int dram_init(void)
>> +{
>> +	gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE);
>> +
>> +	return 0;
>> +}
[...]
>> +int board_phy_config(struct phy_device *phydev)
>> +{
>> +#if defined(CONFIG_PHY_MICREL_KSZ9031)
>
> Is there another possibly phy chip ? If not, can we get rid of the #ifdef ?

No other phy ... removed.

>> +	/* control data pad skew - devaddr = 0x02, register = 0x04 */
>> +	ksz9031_phy_extended_write(phydev, 0x02,
>> +				   MII_KSZ9031_EXT_RGMII_CTRL_SIG_SKEW,
>> +				   MII_KSZ9031_MOD_DATA_NO_POST_INC, 0x0000);
>> +	/* rx data pad skew - devaddr = 0x02, register = 0x05 */
>> +	ksz9031_phy_extended_write(phydev, 0x02,
>> +				   MII_KSZ9031_EXT_RGMII_RX_DATA_SKEW,
>> +				   MII_KSZ9031_MOD_DATA_NO_POST_INC, 0x0000);
>> +	/* tx data pad skew - devaddr = 0x02, register = 0x06 */
>> +	ksz9031_phy_extended_write(phydev, 0x02,
>> +				   MII_KSZ9031_EXT_RGMII_TX_DATA_SKEW,
>> +				   MII_KSZ9031_MOD_DATA_NO_POST_INC, 0x0000);
>> +	/* gtx and rx clock pad skew - devaddr = 0x02, register = 0x08 */
>> +	ksz9031_phy_extended_write(phydev, 0x02,
>> +				   MII_KSZ9031_EXT_RGMII_CLOCK_SKEW,
>> +				   MII_KSZ9031_MOD_DATA_NO_POST_INC, 0x03FF);
>> +#endif /* CONFIG_PHY_MICREL_KSZ9031 */
[...]
>> +/*
>> + * Rotate the BMP_LOGO (only)
>> + * Will only work, if the logo is square, as
>> + * BMP_LOGO_HEIGHT and BMP_LOGO_WIDTH are defines, not variables
>> + */
>> +void rotate_logo(int rotations)
>> +{
>> +	unsigned char out_logo[BMP_LOGO_WIDTH * BMP_LOGO_HEIGHT];
>> +	unsigned char *in_logo;
>> +	int   i, j;
>> +
>> +	if (BMP_LOGO_WIDTH != BMP_LOGO_HEIGHT)
>> +		return;
>> +
>> +	in_logo = bmp_logo_bitmap;
>> +
>> +	/* one 90 degree rotation */
>> +	if (rotations == 1  ||  rotations == 2  ||  rotations == 3)
>> +		rotate_logo_one(out_logo, in_logo);
>> +
>> +	/* second 90 degree rotation */
>> +	if (rotations == 2  ||  rotations == 3)
>> +		rotate_logo_one(in_logo, out_logo);
>> +
>> +	/* third 90 degree rotation */
>> +	if (rotations == 3)
>> +		rotate_logo_one(out_logo, in_logo);
>> +
>> +	/* copy result back to original array */
>> +	if (rotations == 1  ||  rotations == 3)
>> +		for (i = 0; i < BMP_LOGO_WIDTH; i++)
>> +			for (j = 0; j < BMP_LOGO_HEIGHT; j++)
>> +				in_logo[i * BMP_LOGO_WIDTH + j] =
>> +				out_logo[i * BMP_LOGO_WIDTH + j];
>> +}
>> +#endif
>
> Should we put this code in a more common location ? As candidate I see
> common/lcd_console_rotation.c or drivers/video/cfb_console.c

I am unsure, as the logo must be a square ... added Anatolij to
Cc ... @Anatolij: What do you think, has such a function a chance
(and if yes, where is the best place for it?)

>> +
>> +static void enable_display_power(void)
>> +{
>> +	imx_iomux_v3_setup_multiple_pads(backlight_pads,
>> +					 ARRAY_SIZE(backlight_pads));
>> +
>> +	/* backlight enable */
>> +	gpio_direction_output(IMX_GPIO_NR(6, 31), 1);
>> +	/* LCD power enable */
>> +	gpio_direction_output(IMX_GPIO_NR(6, 15), 1);
>> +
>> +	/* enable backlight PWM 1 */
>> +	if (pwm_init(0, 0, 0))
>> +		goto error;
>> +	/* duty cycle 500ns, period: 3000ns */
>> +	if (pwm_config(0, 50000, 300000))
>> +		goto error;
>> +	if (pwm_enable(0))
>> +		goto error;
>> +	return;
>> +
>> +error:
>> +	puts("error init pwm for backlight\n");
>> +	return;
>> +}
[...]
>> +static void enable_spi(struct display_info_t const *dev)
>> +{
>
> Names are just a little confusing. There is a setup_spi and enable_spi,
> and it takes a bit to understand that enable_spi is only for display.
> You could rename enable_spi with a name related to video/display.

enable_spi_display ?

>> +	struct iomuxc *iomux = (struct iomuxc *)IOMUXC_BASE_ADDR;
>> +	struct mxc_ccm_reg *ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
>> +	int reg;
>> +	s32 timeout = 100000;
>> +
>> +#if defined(CONFIG_VIDEO_BMP_LOGO)
>> +	rotate_logo(3);  /* portrait display in landscape mode */
>> +#endif
>> +
>> +	lgdisplay = 1;
>> +	ipu_set_ldb_clock(28341000);
>
> Where is coming from this value ? Can you add a comment for it ?

This is coming from the displays parameters, IIRC I got them from
the customer ... I try to get this info and add some more info into
a comment.

>> +
>> +	reg = readl(&ccm->cs2cdr);
>> +
>> +	/* select pll 5 clock */
>> +	reg &= ~(MXC_CCM_CS2CDR_LDB_DI0_CLK_SEL_MASK
>> +		| MXC_CCM_CS2CDR_LDB_DI1_CLK_SEL_MASK);
>> +	writel(reg, &ccm->cs2cdr);
>> +
[...]
>> +#endif                         /* __ARISTAINETOS2_CONFIG_H */
>>
>
> Best regards,
> Stefano Babic

Thanks for your review!

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany