> On Tue, 2009-02-10 at 18:09 +0530, [email protected] wrote:
> > From: Chaithrika U S <[email protected]>
> >
> > Remove the harcoded PHY mask value, instead obtain PHY address from
> > platform data
> >
> > Signed-off-by: Chaithrika U S <[email protected]>
> > ---
> > arch/arm/mach-davinci/devices.c | 2 +
> > arch/arm/mach-davinci/include/mach/emac.h | 1 +
> > drivers/net/davinci_emac.c | 78 ++++++++++++++++++-
> ---------
> > 3 files changed, 53 insertions(+), 28 deletions(-)
> >
> > diff --git a/arch/arm/mach-davinci/devices.c b/arch/arm/mach-
> davinci/devices.c
> > index 29863d0..380337a 100644
> > --- a/arch/arm/mach-davinci/devices.c
> > +++ b/arch/arm/mach-davinci/devices.c
> > @@ -34,6 +34,7 @@
> >
> >
> > #define DAVINCI_I2C_BASE 0x01C21000
> > +#define DAVINCI_EVM_PHY_MASK 0x2
>
> Just wondering why DAVINCI_EVM_PHY_MASK is 0x2, it seems that the code
> only checks for non-zero phy_mask.
The phy address of the Ethernet device is 0x01 on DM644x and DM646x EVM.
This translates to PHY mask value of 0x02 (1 << phy_addr).
>
>
> > #define DAVINCI_EMAC_BASE 0x01C80000
> > #define DAVINCI_EMAC_CNTRL_OFFSET 0x0000
> > #define DAVINCI_EMAC_CNTRL_MOD_OFFSET 0x1000
> > @@ -270,6 +271,7 @@ static struct emac_platform_data emac_pdata = {
> > .ctrl_ram_offset = DAVINCI_EMAC_CNTRL_RAM_OFFSET,
> > .mdio_reg_offset = DAVINCI_EMAC_MDIO_OFFSET,
> > .ctrl_ram_size = DAVINCI_EMAC_CNTRL_RAM_SIZE,
> > + .phy_mask = DAVINCI_EVM_PHY_MASK,
> > };
> >
> > static struct platform_device davinci_emac_device = {
> > diff --git a/arch/arm/mach-davinci/include/mach/emac.h
> b/arch/arm/mach-davinci/include/mach/emac.h
> > index 7b8c52e..2ceb671 100644
> > --- a/arch/arm/mach-davinci/include/mach/emac.h
> > +++ b/arch/arm/mach-davinci/include/mach/emac.h
> > @@ -18,6 +18,7 @@ struct emac_platform_data {
> > u32 ctrl_ram_offset;
> > u32 mdio_reg_offset;
> > u32 ctrl_ram_size;
> > + u32 phy_mask;
> > };
> >
> > void davinci_init_emac(char *mac_addr);
> > diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
> > index 88263f3..9189a2b 100644
> > --- a/drivers/net/davinci_emac.c
> > +++ b/drivers/net/davinci_emac.c
> > @@ -149,7 +149,6 @@ static const char emac_version_string[] = "TI
> DaVinci EMAC Linux v6.0";
> > #define EMAC_MIN_FREQUENCY_FOR_1000MBPS (125000000)
> >
> > /* TODO: This should come from platform data */
> > -#define EMAC_EVM_PHY_MASK (0x2)
> > #define EMAC_EVM_MLINK_MASK (0)
> > #define EMAC_EVM_BUS_FREQUENCY (76500000) /* PLL/6 i.e
> 76.5 MHz */
> > #define EMAC_EVM_MDIO_FREQUENCY (2200000) /* PHY bus
> frequency */
> > @@ -505,6 +504,7 @@ struct emac_priv {
> > struct timer_list periodic_timer;
> > u32 periodic_ticks;
> > u32 timer_active;
> > + u32 phy_mask;
> > /* mii_bus,phy members */
> > struct mii_bus *mii_bus;
> > struct phy_device *phydev;
> > @@ -699,7 +699,11 @@ static int emac_get_settings(struct net_device
> *ndev,
> > struct ethtool_cmd *ecmd)
> > {
> > struct emac_priv *priv = netdev_priv(ndev);
> > - return phy_ethtool_gset(priv->phydev, ecmd);
> > + if (priv->phy_mask)
> > + return phy_ethtool_gset(priv->phydev, ecmd);
> > + else
> > + return -EOPNOTSUPP;
> > +
> > }
> >
> > /**
> > @@ -713,7 +717,11 @@ static int emac_get_settings(struct net_device
> *ndev,
> > static int emac_set_settings(struct net_device *ndev, struct
> ethtool_cmd *ecmd)
> > {
> > struct emac_priv *priv = netdev_priv(ndev);
> > - return phy_ethtool_sset(priv->phydev, ecmd);
> > + if (priv->phy_mask)
> > + return phy_ethtool_sset(priv->phydev, ecmd);
> > + else
> > + return -EOPNOTSUPP;
>
> Do we need to set phydev->addr or is that set automatically somewhere?
>
phydev->addr is set by the mdio module.
> > +
> > }
> >
> >
> > @@ -746,7 +754,10 @@ static void emac_update_phystatus(struct
> emac_priv *priv)
> >
> > mac_control = emac_read(EMAC_MACCONTROL);
> >
> > - new_duplex = priv->phydev->duplex;
> > + if (priv->phy_mask)
> > + new_duplex = priv->phydev->duplex;
> > + else
> > + new_duplex = DUPLEX_FULL;
> >
> > /* We get called only if link has changed
> (speed/duplex/status) */
> > if ((priv->link) && (new_duplex != priv->duplex)) {
> > @@ -2443,34 +2454,43 @@ static int emac_dev_open(struct net_device
> *ndev)
> >
> > /* find the first phy */
> > priv->phydev = NULL;
> > - for (phy_addr = 0; phy_addr < PHY_MAX_ADDR; phy_addr++) {
> > - if (priv->mii_bus->phy_map[phy_addr]) {
> > - priv->phydev = priv->mii_bus-
> >phy_map[phy_addr];
> > - break;
> > + if (priv->phy_mask) {
> > + for (phy_addr = 0; phy_addr < PHY_MAX_ADDR; phy_addr++)
> {
> > + if (priv->mii_bus->phy_map[phy_addr]) {
> > + priv->phydev = priv->mii_bus-
> >phy_map[phy_addr];
> > + break;
> > + }
>
> Address 0, I believe is broadcast PHY. Would this cause problem for
> DM644x EVM with LX971?
>
> What happens if we have multiple phys? For example wired and wireless
> or even optical interface?
>
> Also, if we can scan for PHYs, do we still need to hard code the
> phy_mask in platform data? Can the phy_mask be determined dynamically
> based on the scan?
If there are multiple PHYs the first phy found will be connected.
If all the phy address have to be scanned, then the mask should be set to
0xffffffff.
The mask is used to make sure you only look at PHYs you care about.
Setting the mask to 0 will not access registers of any PHY.
>
> > }
> > - }
> >
> > - if (!priv->phydev) {
> > - printk(KERN_ERR "%s: no PHY found\n", ndev->name);
> > - return -1;
> > - }
> > + if (!priv->phydev) {
> > + printk(KERN_ERR "%s: no PHY found\n", ndev-
> >name);
> > + return -1;
> > + }
> >
> > - priv->phydev = phy_connect(ndev, priv->phydev->dev.bus_id,
> > + priv->phydev = phy_connect(ndev, priv->phydev-
> >dev.bus_id,
> > &emac_adjust_link, 0,
> PHY_INTERFACE_MODE_MII);
> >
> > - if (IS_ERR(priv->phydev)) {
> > - printk(KERN_ERR "%s: Could not attach to PHY\n", ndev-
> >name);
> > - return PTR_ERR(priv->phydev);
> > - }
> > + if (IS_ERR(priv->phydev)) {
> > + printk(KERN_ERR "%s: Could not attach to
> PHY\n",
> > + ndev-
> >name);
> > + return PTR_ERR(priv->phydev);
> > + }
> >
> > - priv->link = 0;
> > - priv->speed = 0;
> > - priv->duplex = -1;
> > + priv->link = 0;
> > + priv->speed = 0;
> > + priv->duplex = -1;
> >
> > - printk(KERN_INFO "%s: attached PHY driver [%s] "
> > - "(mii_bus:phy_addr=%s, id=%x)\n", ndev->name,
> > - priv->phydev->drv->name, priv->phydev->dev.bus_id,
> > - priv->phydev->phy_id);
> > + printk(KERN_INFO "%s: attached PHY driver [%s] "
> > + "(mii_bus:phy_addr=%s, id=%x)\n", ndev->name,
> > + priv->phydev->drv->name, priv->phydev-
> >dev.bus_id,
> > + priv->phydev->phy_id);
> > + } else{
> > + /* No PHY , fix the link, speed and duplex settings */
> > + priv->link = 1;
> > + priv->speed = SPEED_100;
> > + priv->duplex = DUPLEX_FULL;
>
> Any particular reason for this default? Seems SPEED_10 DUPLEX_HALF is
> the lowest (perhaps safest) setting.
This is to take care of the case where a mask of 0 will be used most likely,
i.e. a
switch connected to the MAC instead of a PHY. This replicates the
EMAC_SPEED_NO_PHY case in the original driver
Thanks,
Chaithrika
>
> > + emac_update_phystatus(priv);
> > + }
> >
> > if (!netif_running(ndev)) /* debug only - to avoid compiler
> warning */
> > emac_dump_regs(priv);
> > @@ -2478,7 +2498,8 @@ static int emac_dev_open(struct net_device
> *ndev)
> > if (netif_msg_drv(priv))
> > dev_notice(EMAC_DEV, "DaVinci EMAC: Opened %s\n",
> ndev->name);
> >
> > - phy_start(priv->phydev);
> > + if (priv->phy_mask)
> > + phy_start(priv->phydev);
> >
> > return 0;
> >
> > @@ -2637,8 +2658,9 @@ static int __devinit davinci_emac_probe(struct
> platform_device *pdev)
> > return -ENODEV;
> > }
> >
> > - /* MAC addr: from platform_data */
> > + /* MAC addr and PHY mask from platform_data */
> > memcpy(priv->mac_addr, pdata->mac_addr, 6);
> > + priv->phy_mask = pdata->phy_mask;
> >
> > /* Get EMAC platform data */
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > @@ -2720,7 +2742,7 @@ static int __devinit davinci_emac_probe(struct
> platform_device *pdev)
> > emac_mii->write = emac_mii_write,
> > emac_mii->reset = emac_mii_reset,
> > emac_mii->irq = mii_irqs,
> > - emac_mii->phy_mask = ~(EMAC_EVM_PHY_MASK);
> > + emac_mii->phy_mask = ~(priv->phy_mask);
> > emac_mii->parent = &pdev->dev;
> > emac_mii->priv = priv->remap_addr + pdata->mdio_reg_offset;
> > snprintf(priv->mii_bus->id, MII_BUS_ID_SIZE, "%x", priv->pdev-
> >id);
> _______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
