On Wed, 2009-02-11 at 11:34 +0530, Subrahmanya, Chaithrika wrote:
> > 
> > On Tuesday 10 February 2009, chaithr...@ti.com wrote:
> > >  #define DAVINCI_I2C_BASE            0x01C21000
> > > +#define DAVINCI_EVM_PHY_MASK           0x2
> > >  #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,
> > 
> > Surely the PHY address mask should be board-specific,
> > if it's got to be hard-wired.
> > 
> > But ... is it not practical to dynamically probe it?
> > That's what most Ethernet drivers do.  "The MDIO module
> > continuously polls all 32 MDIO addresses in order to
> > enumerate the PHY devices in the system" ... so when
> > that polling returns a single PHY there should be no
> > question about which one to use.
> > 
> 
> The PHY mask was put in place originally to take care of the case where PHYs 
> connected to multiple instances of EMAC are serviced by single MDIO module. 
> It helped differentiate which PHY belongs to which MAC. However, we don't 
> have 
> that case on DaVinci (yet). I don't think the driver is fully there to take 
> care of that 
> case, but we wanted to retain this support just in case.

Will there be a case where multiple phys hang off of a single MAC?

> 
> Also, it helps tell the driver to not care about any connected PHY (by 
> setting the 
> mask to 0). The OMAP-L137 EVM has a switch connected to the MAC instead 
> of a PHY. In that case, most likely, mask value of zero will be used.

We should be able to read the device id and make decisions based on
device id.  Will that be preferred over hard coded values?

> 
> > 
> > 
> > >  /* TODO: This should come from platform data */

I guess this comment can now be removed :)

> > > -#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 */
> > 
> > EVM_MLINK_MASK is not used in the driver.
> > Ditto EVM_BUS_FREQUENCY.  Might as well
> > just delete them in this patch.
> > 
> > And I suspect that EVM_MDIO_FREQUENCY should
> > be derived from the EMAC clock ...
> 
> OK, will do the changes and submit a patch.
> 
> Thanks,
> Chaithrika
> 
> > 
> > - Dave
> > _______________________________________________
> Davinci-linux-open-source mailing list
> Davinci-linux-open-source@linux.davincidsp.com
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source


_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to