> On Wed, 2009-02-11 at 11:34 +0530, Subrahmanya, Chaithrika wrote:
> > >
> > > On Tuesday 10 February 2009, [email protected] 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?
> 

No, in my experience I have not yet come across a single MAC having 
multiple PHYs.

> >
> > 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?

Are you referring to the cpu_is_xxx( ) macro, to get the device id info?
The PHY type being used is board specific and not SoC specific.
Adding a device id verification code for each EVM/board may make 
the driver code more difficult to handle as the number of boards increase.

> 
> >
> > >
> > >
> > > >  /* TODO: This should come from platform data */
> 
> I guess this comment can now be removed :)

OK

> 
> > > > -#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
> > [email protected]
> > http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-
> source
> _______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to