[PATCH] ARM: mxs: Add initial support for DENX MX28

On 09/06/2011 10:39 AM, Uwe Kleine-K?nig wrote:

Hello Stefano,

Hi Uwe,

Can you please Cc: kernel at pengutronix for your next mxs patches
(additional to or instead of s.hauer)? You increase your chances to get
a reply when Sascha is on vacation.

Yes, thanks ! I am not aware of it, I will do it for V2.

quoted

+	/*
+	 * OCOTP only stores the last 4 octets for each mac address,
+	 * so hard-code DENX OUI (C0:E5:4E) here.
+	 */
+	for (i = 0; i < 2; i++) {
+		val = ocotp[i * 4];
+		mx28_fec_pdata[i].mac[0] = 0xC0;
+		mx28_fec_pdata[i].mac[1] = 0xE5;
+		mx28_fec_pdata[i].mac[2] = 0x4E;
+		mx28_fec_pdata[i].mac[3] = (val >> 16) & 0xff;
+		mx28_fec_pdata[i].mac[4] = (val >> 8) & 0xff;
+		mx28_fec_pdata[i].mac[5] = (val >> 0) & 0xff;
+	}
+
+	return 0;
+
+error:

As there is only a single place that can produce an error, I'd handle it
directly there. That is, move the printk and return into the body of the
if (!ocotp) above.

You are right, I fix in V2.

quoted

+

please remove this empty line.

Ok

quoted

+
+	if (m28evk_fec_get_mac())
+		pr_warn("%s: failed on fec mac setup\n", __func__);

I'd suggest to only register the fecs if get_mac() succeeded?

I do not know - if m28evk_fec_get_mac fails (ok, it *should* not fail..)
it should be still possible to add the MAC address via ifconfig (with hw
ethar <mac>, I mean) if the device is present, at least this is what I
supposed in error case.

Also you might want to skip that pr_warn as m28evk_fec_get_mac already
prints an error on failure.

Right, I'll do it

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================