Re: [PATCH] Add support for Baltos systems

[Yegor Yefremov <yegorslists@googlemail.com>]

On Mon, May 30, 2016 at 7:33 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 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.

Will remove.

>> +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.

We use this scheme since years, so this cannot be changed now.

>> +             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.

Will do.

>> +     }
>> +
>> +     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?

First we used 100KHz, but under some circumstances it turned out, that
SoC <-> PMIC communication got stuck and as a result CPU was running
with lower frequency. So after more testing it turned out, that 1KHz
is the most "stable" frequency It is only important that these first
communication steps occur at 1KHz, after this any supported speed is
stable.

Yegor

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