[PATCH v2 2/2] powerpc/44x: Add more changes for APM821XX EMAC driver
Duc Dang
dhdang at apm.com
Mon Mar 5 19:45:20 EST 2012
Thanks, Stefan.
Please see my answer bellow.
Regards,
Duc Dang.
On Mon, Mar 5, 2012 at 2:41 PM, Stefan Roese <sr at denx.de> 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 <dhdang at apm.com>
> > ---
> > 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20120305/bc7075b6/attachment.html>
More information about the Linuxppc-dev
mailing list