Re: [PATCH] Add support for Baltos systems

[Yegor Yefremov <yegorslists@googlemail.com>]

On Tue, May 31, 2016 at 9:01 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, May 30, 2016 at 11:46:12AM +0200, Yegor Yefremov wrote:
>> 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.
>
> Ok, so it's really 1000Hz. Note that if you start Linux with the barebox
> device tree you will end up with 1000Hz under Linux aswell, so it might
> be worth looking for another solution to this problem.

We don't start Linux with barebox device tree. This is really only a
minimal configuration, that is needed to configure barebox itself. We
use a FIT image, that holds about 6 different full-featured DTBs.

Yegor

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