Re: [PATCH 1/3] PAL: Support of the fixed PHY

[Vitaly Bordug <vbordug@ru.mvista.com>]

На Wed, 21 Jun 2006 22:48:27 +0200
Michael Buesch <mb@bu3sch.de> записано:

> On Wednesday 21 June 2006 18:09, Vitaly Bordug wrote:
> 
> > +static int fixed_mdio_update_regs(struct fixed_info *fixed)
> > +{
> > +	u16 *regs = fixed->regs;
> > +	u16 bmsr = 0;
> > +	u16 bmcr = 0;
> > +
> > +	if(!regs) {
> > +		printk(KERN_ERR "%s: regs not set up",
> > __FUNCTION__);
> > +		return -1;
> 
> -EINVAL perhaps?
> 
OK
> > +static int fixed_mdio_register_device(int number, int speed, int
> > duplex) +{
> > +	struct mii_bus *new_bus;
> > +	struct fixed_info *fixed;
> > +	struct phy_device *phydev;
> > +	int err = 0;
> > +
> > +	struct device* dev = kzalloc(sizeof(struct device),
> > GFP_KERNEL); +
> > +	if (NULL == dev)
> > +		return -EINVAL;
> 
> -ENOMEM here.
OK

[snip]
> > +	/*
> > +	   the mdio bus has phy_id match... In order not to do it
> > +	   artificially, we are binding the driver here by hand;
> > +	   it will be the same
> > +	   for all the fixed phys anyway.
> > +	 */
> > +	down_write(&phydev->dev.bus->subsys.rwsem);
> > +
> > +	phydev->dev.driver = &fixed_mdio_driver.driver;
> > +
> > +	err = phydev->dev.driver->probe(&phydev->dev);
> > +	if(err < 0) {
> > +		printk(KERN_ERR "Phy %s: problems with fixed
> > driver\n",
> > +				phydev->dev.bus_id);
> > +		up_write(&phydev->dev.bus->subsys.rwsem);
> > +		goto bus_register_fail;
> 
> Probably need some additional error unwinding code.
> Of doesn't device_register() have to be reverted?
> What about phy_device_create()?
> 
Definitely. Moreover, as I noticed now, phy_driver_register also has to
be error-handled and such. Will fix/update and resubmit.

Thanks for the feedback.
--

Sincerely, Vitaly

_______________________________________________
Linuxppc-embedded mailing list
Linuxppc-embedded@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-embedded