Thanks, David and Josh.<br>
<br>
Except the coding style problem that David mentioned, do you have other comment about my patch set?<br>
<br>
Regards,<br>
Duc Dang.<br><br><div class="gmail_quote">On Thu, Mar 1, 2012 at 1:25 AM, David Miller <span dir="ltr"><<a href="mailto:davem@davemloft.net">davem@davemloft.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
From: Josh Boyer <<a href="mailto:jwboyer@gmail.com">jwboyer@gmail.com</a>><br>
Date: Wed, 29 Feb 2012 08:43:46 -0500<br>
<br>
> On Fri, Feb 17, 2012 at 3:07 AM, Duc Dang <<a href="mailto:dhdang@apm.com">dhdang@apm.com</a>> wrote:<br>
>> This patch includes:<br>
>><br>
>> Configure EMAC PHY clock source (clock from PHY or internal clock).<br>
>><br>
>> Do not advertise PHY half duplex capability as APM821XX EMAC does not<br>
>> support half duplex mode.<br>
>><br>
>> Add changes to support configuring jumbo frame for APM821XX EMAC.<br>
>><br>
>> Signed-off-by: Duc Dang <<a href="mailto:dhdang@apm.com">dhdang@apm.com</a>><br>
><br>
> This should have been sent to netdev. CC'ing them now.<br>
><br>
> Ben and David, I can take this change through the 4xx tree if it looks OK to<br>
> both of you. The pre-requisite DTS patch will go through my tree, so it might<br>
> make sense to keep them together.<br>
<br>
Well the patch has coding style problems, for one:<br>
<br>
>> + dev->features |= (EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE<br>
>> + | EMAC_FTR_APM821XX_NO_HALF_DUPLEX<br>
>> + | EMAC_FTR_460EX_PHY_CLK_FIX);<br>
<br>
Should be:<br>
<br>
>> + dev->features |= (EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE |<br>
>> + EMAC_FTR_APM821XX_NO_HALF_DUPLEX |<br>
>> + EMAC_FTR_460EX_PHY_CLK_FIX);<br>
<br>
And this:<br>
<br>
>> + dev->phy_feat_exc = (SUPPORTED_1000baseT_Half<br>
>> + | SUPPORTED_100baseT_Half<br>
>> + | SUPPORTED_10baseT_Half);<br>
<br>
Should be:<br>
<br>
>> + dev->phy_feat_exc = (SUPPORTED_1000baseT_Half |<br>
>> + SUPPORTED_100baseT_Half |<br>
>> + SUPPORTED_10baseT_Half);<br>
</blockquote></div><br>