From: sbabic@denx•de (Stefano Babic)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] ARM: mxs: Add initial support for DENX MX28
Date: Tue, 06 Sep 2011 11:13:02 +0200 [thread overview]
Message-ID: <4E65E41E.9000108@denx.de> (raw)
In-Reply-To: <20110906083926.GE28816@pengutronix.de>
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
=====================================================================
next prev parent reply other threads:[~2011-09-06 9:13 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-29 7:50 [PATCH] ARM: mxs: Add initial support for DENX MX28 Stefano Babic
2011-09-06 8:39 ` Uwe Kleine-König
2011-09-06 8:46 ` Wolfram Sang
2011-09-06 9:14 ` Stefano Babic
2011-09-06 9:10 ` Lothar Waßmann
2011-09-06 13:25 ` Shawn Guo
2011-09-07 6:28 ` Stefano Babic
2011-09-06 9:13 ` Stefano Babic [this message]
2011-09-07 6:45 ` [PATCH V2] " Stefano Babic
2011-09-07 9:30 ` Wolfram Sang
2011-09-07 9:37 ` Stefano Babic
2011-09-07 23:45 ` Marek Vasut
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=4E65E41E.9000108@denx.de \
--to=sbabic@denx$(echo .)de \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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