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

На Wed, 21 Jun 2006 22:48:27 +0200
Michael Buesch [off-list ref] записано:

On Wednesday 21 June 2006 18:09, Vitaly Bordug wrote:

quoted

+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

quoted

+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]

quoted

+	/*
+	   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