Thanks, Stefan. Please see my answer bellow. Regards, Duc Dang. On Mon, Mar 5, 2012 at 2:41 PM, Stefan Roese wrote: > On Monday 05 March 2012 05:03:44 Duc Dang wrote: > > This patch includes: > > > > Configure EMAC PHY clock source (clock from PHY or internal clock). > > > > Do not advertise PHY half duplex capability as APM821XX EMAC does not > > support half duplex mode. > > > > Add changes to support configuring jumbo frame for APM821XX EMAC. > > > > Signed-off-by: Duc Dang > > --- > > v2: > > Fix coding style problems. > > > > drivers/net/ethernet/ibm/emac/core.c | 26 +++++++++++++++++++++++++- > > drivers/net/ethernet/ibm/emac/core.h | 13 +++++++++++-- > > drivers/net/ethernet/ibm/emac/emac.h | 5 ++++- > > 3 files changed, 40 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/ethernet/ibm/emac/core.c > > b/drivers/net/ethernet/ibm/emac/core.c index 2abce96..ad671c5 100644 > > --- a/drivers/net/ethernet/ibm/emac/core.c > > +++ b/drivers/net/ethernet/ibm/emac/core.c > > @@ -434,6 +434,11 @@ static inline u32 emac_iff2rmr(struct net_device > > *ndev) else if (!netdev_mc_empty(ndev)) > > r |= EMAC_RMR_MAE; > > > > + if (emac_has_feature(dev, EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE)) { > > + r &= ~EMAC4_RMR_MJS_MASK; > > + r |= EMAC4_RMR_MJS(ndev->mtu); > > + } > > + > > return r; > > } > > > > @@ -965,6 +970,7 @@ static int emac_resize_rx_ring(struct emac_instance > > *dev, int new_mtu) int rx_sync_size = emac_rx_sync_size(new_mtu); > > int rx_skb_size = emac_rx_skb_size(new_mtu); > > int i, ret = 0; > > + int mr1_jumbo_bit_change = 0; > > > > mutex_lock(&dev->link_lock); > > emac_netif_stop(dev); > > @@ -1013,7 +1019,14 @@ static int emac_resize_rx_ring(struct > emac_instance > > *dev, int new_mtu) } > > skip: > > /* Check if we need to change "Jumbo" bit in MR1 */ > > - if ((new_mtu > ETH_DATA_LEN) ^ (dev->ndev->mtu > ETH_DATA_LEN)) { > > + if (emac_has_feature(dev, EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE)) > > + mr1_jumbo_bit_change = (new_mtu > ETH_DATA_LEN) || > > + (dev->ndev->mtu > ETH_DATA_LEN); > > + else > > + mr1_jumbo_bit_change = (new_mtu > ETH_DATA_LEN) ^ > > + (dev->ndev->mtu > ETH_DATA_LEN); > > + > > + if (mr1_jumbo_bit_change) { > > /* This is to prevent starting RX channel in > emac_rx_enable() > */ > > set_bit(MAL_COMMAC_RX_STOPPED, &dev->commac.flags); > > > > @@ -2471,6 +2484,7 @@ static int __devinit emac_init_phy(struct > > emac_instance *dev) > > > > /* Disable any PHY features not supported by the platform */ > > dev->phy.def->features &= ~dev->phy_feat_exc; > > + dev->phy.features &= ~dev->phy_feat_exc; > > > > /* Setup initial link parameters */ > > if (dev->phy.features & SUPPORTED_Autoneg) { > > @@ -2568,6 +2582,10 @@ static int __devinit emac_init_config(struct > > emac_instance *dev) if (of_device_is_compatible(np, "ibm,emac-405ex") || > > of_device_is_compatible(np, "ibm,emac-405exr")) > > dev->features |= EMAC_FTR_440EP_PHY_CLK_FIX; > > + if (of_device_is_compatible(np, "ibm,emac-apm821xx")) > > + dev->features |= > (EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE > | > > + EMAC_FTR_APM821XX_NO_HALF_DUPLEX | > > + EMAC_FTR_460EX_PHY_CLK_FIX); > > Nitpick: Parentheses for multi line statement please. > [Duc Dang] I wil add parentheses as you suggested. > } else if (of_device_is_compatible(np, "ibm,emac4")) { > > dev->features |= EMAC_FTR_EMAC4; > > if (of_device_is_compatible(np, "ibm,emac-440gx")) > > @@ -2818,6 +2836,12 @@ static int __devinit emac_probe(struct > > platform_device *ofdev) dev->stop_timeout = STOP_TIMEOUT_100; > > INIT_DELAYED_WORK(&dev->link_work, emac_link_timer); > > > > + /* Some SoCs like APM821xx does not support Half Duplex mode. */ > > + if (emac_has_feature(dev, EMAC_FTR_APM821XX_NO_HALF_DUPLEX)) > > + dev->phy_feat_exc = (SUPPORTED_1000baseT_Half | > > + SUPPORTED_100baseT_Half | > > + SUPPORTED_10baseT_Half); > > + > > Nitpick: Parentheses for multi line statement please. > > > /* Find PHY if any */ > > err = emac_init_phy(dev); > > if (err != 0) > > diff --git a/drivers/net/ethernet/ibm/emac/core.h > > b/drivers/net/ethernet/ibm/emac/core.h index fa3ec57..9dea85a 100644 > > --- a/drivers/net/ethernet/ibm/emac/core.h > > +++ b/drivers/net/ethernet/ibm/emac/core.h > > @@ -325,7 +325,14 @@ struct emac_instance { > > * Set if we need phy clock workaround for 460ex or 460gt > > */ > > #define EMAC_FTR_460EX_PHY_CLK_FIX 0x00000400 > > - > > +/* > > + * APM821xx requires Jumbo frame size set explicitly > > + */ > > +#define EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE 0x00000800 > > +/* > > + * APM821xx does not support Half Duplex mode > > + */ > > +#define EMAC_FTR_APM821XX_NO_HALF_DUPLEX 0x00001000 > > > > /* Right now, we don't quite handle the always/possible masks on the > > * most optimal way as we don't have a way to say something like > > @@ -353,7 +360,9 @@ enum { > > EMAC_FTR_NO_FLOW_CONTROL_40x | > > #endif > > EMAC_FTR_460EX_PHY_CLK_FIX | > > - EMAC_FTR_440EP_PHY_CLK_FIX, > > + EMAC_FTR_440EP_PHY_CLK_FIX | > > + EMAC_APM821XX_REQ_JUMBO_FRAME_SIZE | > > + EMAC_FTR_APM821XX_NO_HALF_DUPLEX, > > }; > > > > static inline int emac_has_feature(struct emac_instance *dev, > > diff --git a/drivers/net/ethernet/ibm/emac/emac.h > > b/drivers/net/ethernet/ibm/emac/emac.h index 1568278..36bcd69 100644 > > --- a/drivers/net/ethernet/ibm/emac/emac.h > > +++ b/drivers/net/ethernet/ibm/emac/emac.h > > @@ -212,7 +212,10 @@ struct emac_regs { > > #define EMAC4_RMR_RFAF_64_1024 0x00000006 > > #define EMAC4_RMR_RFAF_128_2048 0x00000007 > > #define EMAC4_RMR_BASE EMAC4_RMR_RFAF_128_2048 > > - > > +#if defined(CONFIG_APM821xx) > > +#define EMAC4_RMR_MJS_MASK 0x0001fff8 > > +#define EMAC4_RMR_MJS(s) (((s) << 3) & > EMAC4_RMR_MJS_MASK) > > +#endif > > Is this "#if defined" really needed? If not, I suggest you drop it. > > [Duc Dang] Only APM821XX and later SoCs have this register field, but it > is no harm to drop the #if defined as 460EX/460GT/405EX have this register > field as reserved. So I will drop it. > > Thanks, > Stefan >