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

[Stefano Babic]

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.


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

>> +
> please remove this empty line.

Ok

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