Re: [PATCH] Add support for Baltos systems

[Sascha Hauer <s.hauer@pengutronix.de>]

On Fri, May 27, 2016 at 03:48:55PM +0200, yegorslists@googlemail.com wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>
> 
> OnRISC Baltos devices are based on a am335x SoC and can be booted
> either from MMC or NAND.
> 
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---

> +
> +typedef struct _BSP_VS_HWPARAM
> +{
> +        uint32_t Magic;
> +        uint32_t HwRev;
> +        uint32_t SerialNumber;
> +        char PrdDate[11];
> +        uint16_t SystemId;
> +        uint8_t MAC1[6];
> +        uint8_t MAC2[6];
> +        uint8_t MAC3[6];
> +} __attribute__ ((packed)) BSP_VS_HWPARAM;
> +
> +BSP_VS_HWPARAM hw_param;

No typedef for struct types please.

> +static void baltos_validate_eeprom(void)
> +{
> +	printf("Baltos: incorrect magic number (0x%x) in EEPROM\n",
> +			hw_param.Magic);
> +
> +	/* fill default values */
> +	hw_param.SystemId = 210;
> +
> +	hw_param.MAC1[0] = 0x00;
> +	hw_param.MAC1[1] = 0x00;
> +	hw_param.MAC1[2] = 0x00;
> +	hw_param.MAC1[3] = 0x00;
> +	hw_param.MAC1[4] = 0x00;
> +	hw_param.MAC1[5] = 0x01;
> +
> +	hw_param.MAC2[0] = 0x00;
> +	hw_param.MAC2[1] = 0x00;
> +	hw_param.MAC2[2] = 0x00;
> +	hw_param.MAC2[3] = 0x00;
> +	hw_param.MAC2[4] = 0x00;
> +	hw_param.MAC2[5] = 0x02;
> +
> +	hw_param.MAC3[0] = 0x00;
> +	hw_param.MAC3[1] = 0x00;
> +	hw_param.MAC3[2] = 0x00;
> +	hw_param.MAC3[3] = 0x00;
> +	hw_param.MAC3[4] = 0x00;
> +	hw_param.MAC3[5] = 0x03;
> +}
> +
> +static int baltos_read_eeprom(void)
> +{
> +	size_t size;
> +	char *buf, var_buf[32];
> +	int rc;
> +	unsigned char mac_addr[6];
> +
> +	rc = read_file_2("/dev/eeprom0", &size, (void *)&buf, sizeof(hw_param));
> +	if (rc && rc != -EFBIG) {
> +		return rc;
> +	}
> +
> +	memcpy(&hw_param, buf, sizeof(hw_param));
> +
> +	free(buf);
> +
> +	if (hw_param.Magic != 0xDEADBEEF) {

The magic for a valid EEPROM is 0xdeadbeef? What a poor choice, this
value is commonly used for *invalid* values.

> +		baltos_validate_eeprom();

When you can't find a valid MAC Address in the EEPROM then you should
just return here. barebox will then use a random MAC Address.

> +	}
> +
> +	sprintf(var_buf, "%d", hw_param.SystemId);
> +	globalvar_add_simple("board.id", var_buf);
> +
> +	/* setup MAC1 */
> +	mac_addr[0] = hw_param.MAC1[0];
> +	mac_addr[1] = hw_param.MAC1[1];
> +	mac_addr[2] = hw_param.MAC1[2];
> +	mac_addr[3] = hw_param.MAC1[3];
> +	mac_addr[4] = hw_param.MAC1[4];
> +	mac_addr[5] = hw_param.MAC1[5];
> +
> +	eth_register_ethaddr(0, mac_addr);
> +
> +	/* setup MAC2 */
> +	mac_addr[0] = hw_param.MAC2[0];
> +	mac_addr[1] = hw_param.MAC2[1];
> +	mac_addr[2] = hw_param.MAC2[2];
> +	mac_addr[3] = hw_param.MAC2[3];
> +	mac_addr[4] = hw_param.MAC2[4];
> +	mac_addr[5] = hw_param.MAC2[5];
> +
> +	eth_register_ethaddr(1, mac_addr);
> +
> +	return 0;
> +}

> +
> +&i2c1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c1_pins>;
> +
> +	status = "okay";
> +	clock-frequency = <1000>;

1kHz? This looks suspicous. Shouldn't this be 100000 or 400000?


Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox