Hi Punnaiah,
Thanks for the review...
> > +
> > +#define XILINX_GMII2RGMII_REG 0x10
> > +#define BMCR_SPEED10 0x00
>
> Move this macro to mii.h
Sure will fix in the next version...
>
> > +
> > +struct gmii2rgmii {
> > + struct phy_device *phy_dev;
> > + struct phy_driver *phy_drv;
> > + struct phy_driver conv_phy_drv;
> > + int addr;
> > +};
> > +
> > +static int xgmiitorgmii_read_status(struct phy_device *phydev) {
> > + struct gmii2rgmii *priv = (struct gmii2rgmii *)phydev->priv;
> > + u16 val = 0;
> > +
> > + priv->phy_drv->read_status(phydev);
> > +
> > + val = mdiobus_read(phydev->mdio.bus, priv->addr,
> > XILINX_GMII2RGMII_REG);
> > +
>
> Since its read and then modify, you should mask the speed field With zero
> and
> then write new value.
>
Sure will fix in the next version...
>
> > + switch (phydev->speed) {
> > + case SPEED_1000:
> > + val |= BMCR_SPEED1000;
> > + case SPEED_100:
> > + val |= BMCR_SPEED100;
> > + case SPEED_10:
> > + val |= BMCR_SPEED10;
> > + }
> > +
> > + mdiobus_write(phydev->mdio.bus, priv->addr,
> > XILINX_GMII2RGMII_REG, val);
> > +
> > + return 0;
> > +}
> > +
> > +int xgmiitorgmii_probe(struct mdio_device *mdiodev) {
> > + struct device *dev = &mdiodev->dev;
> > + struct device_node *np = dev->of_node, *phy_node;
> > + struct gmii2rgmii *priv;
> > + int ret;
> > +
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + phy_node = of_parse_phandle(np, "phy-handle", 0);
> > + if (IS_ERR(phy_node)) {
> > + dev_err(dev, "Couldn't parse phy-handle\n");
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + priv->phy_dev = of_phy_find_device(phy_node);
> > + if (!priv->phy_dev) {
> > + ret = -EPROBE_DEFER;
> > + dev_info(dev, "Couldn't find phydev\n");
> > + goto out;
> > + }
> > +
> > + priv->addr = mdiodev->addr;
> > + priv->phy_drv = priv->phy_dev->drv;
> > + memcpy(&priv->conv_phy_drv, priv->phy_dev->drv,
> > + sizeof(struct phy_driver));
> > + priv->conv_phy_drv.read_status = xgmiitorgmii_read_status;
> > + priv->phy_dev->priv = priv;
> > + priv->phy_dev->drv = &priv->conv_phy_drv;
> > +
> > + return 0;
> > +out:
> > + return ret;
>
> Since there is no resource cleaning here, you could consider Return from this
> function in above conditions and avoid goto here.
>
Sure will fix in the next version...
Regards,
Kedar.
> Regards,
> Punnaiah
