mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Yegor Yefremov <yegorslists@googlemail.com>
Cc: barebox <barebox@lists.infradead.org>
Subject: Re: [PATCH] Add support for Baltos systems
Date: Tue, 31 May 2016 09:01:24 +0200	[thread overview]
Message-ID: <20160531070124.GC31666@pengutronix.de> (raw)
In-Reply-To: <CAGm1_kv9SNGG45Eb6GUvPAKZ0PAZtGiJPfXadd6ZLwY_VS94CQ@mail.gmail.com>

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.

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

  reply	other threads:[~2016-05-31  7:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-27 13:48 yegorslists
2016-05-30  5:33 ` Sascha Hauer
2016-05-30  9:46   ` Yegor Yefremov
2016-05-31  7:01     ` Sascha Hauer [this message]
2016-05-31  7:08       ` Yegor Yefremov
2016-05-31 18:46 ` Trent Piepho
2016-06-01  7:23   ` Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160531070124.GC31666@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=barebox@lists.infradead.org \
    --cc=yegorslists@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox