Hello, Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote on Thu, 22 Nov 2018 22:04:16 +0100: > Add a driver to support COMPHY, a hardware block providing shared > serdes PHYs on Marvell Armada 3700. This driver uses SMC calls and > rely on having an up-to-date firmware. > > SATA, PCie and USB3 host mode have been tested successfully with an > ESPRESSObin. SGMII mode cannot be tested with this platform. > > Co-Developed-by: Evan Wang <xswang@xxxxxxxxxxx> > Signed-off-by: Evan Wang <xswang@xxxxxxxxxxx> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx> > --- [...] > +static struct phy *mvebu_a3700_comphy_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct mvebu_a3700_comphy_lane *lane; > + struct phy *phy; > + > + if (WARN_ON(args->args[0] >= MVEBU_A3700_COMPHY_PORTS)) > + return ERR_PTR(-EINVAL); > + > + phy = of_phy_simple_xlate(dev, args); > + if (IS_ERR(phy)) > + return phy; > + > + lane = phy_get_drvdata(phy); > + if (lane->port >= 0) > + return ERR_PTR(-EBUSY); This is not valid. It works only the first time the consumer gets a PHY for this lane. If the consumer put the PHY (module is unloaded) and then gets the PHY again (module is re-loaded a second time), the port is already set to != -1 and this will exit with an error and prevent the module to probe. > + > + lane->port = args->args[0]; I will remove the above check and transform it into a valid "port" value right here. Note: the same applies to the cp110 comphy driver on which the driver structure is based. > + > + return phy; > +} > + > +static int mvebu_a3700_comphy_probe(struct platform_device *pdev) > +{ > + struct phy_provider *provider; > + struct device_node *child; > + > + for_each_available_child_of_node(pdev->dev.of_node, child) { > + struct mvebu_a3700_comphy_lane *lane; > + struct phy *phy; > + int ret; > + u32 lane_id; > + > + ret = of_property_read_u32(child, "reg", &lane_id); > + if (ret < 0) { > + dev_err(&pdev->dev, "missing 'reg' property (%d)\n", > + ret); > + continue; > + } > + > + if (lane_id >= MVEBU_A3700_COMPHY_LANES) { > + dev_err(&pdev->dev, "invalid 'reg' property\n"); > + continue; > + } > + > + lane = devm_kzalloc(&pdev->dev, sizeof(*lane), GFP_KERNEL); > + if (!lane) > + return -ENOMEM; > + > + phy = devm_phy_create(&pdev->dev, child, > + &mvebu_a3700_comphy_ops); > + if (IS_ERR(phy)) > + return PTR_ERR(phy); > + > + lane->dev = &pdev->dev; > + lane->mode = PHY_MODE_INVALID; > + lane->id = lane_id; > + lane->port = -1; > + phy_set_drvdata(phy, lane); > + } > + > + provider = devm_of_phy_provider_register(&pdev->dev, > + mvebu_a3700_comphy_xlate); > + return PTR_ERR_OR_ZERO(provider); > +} > + > +static const struct of_device_id mvebu_a3700_comphy_of_match_table[] = { > + { .compatible = "marvell,comphy-a3700" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, mvebu_a3700_comphy_of_match_table); > + > +static struct platform_driver mvebu_a3700_comphy_driver = { > + .probe = mvebu_a3700_comphy_probe, > + .driver = { > + .name = "mvebu-a3700-comphy", > + .of_match_table = mvebu_a3700_comphy_of_match_table, > + }, > +}; > +module_platform_driver(mvebu_a3700_comphy_driver); > + > +MODULE_AUTHOR("Miquèl Raynal <miquel.raynal@xxxxxxxxxxx>"); > +MODULE_DESCRIPTION("Common PHY driver for A3700"); > +MODULE_LICENSE("GPL v2"); Thanks, Miquèl