On Thu, Mar 28 2019, Sergio Paracuellos wrote:
> Device tree has been simplified to don't use child nodes and use
> the #phy-cells property instead. Change the driver accordly implementing
> custom 'xlate' function to return the correct phy for each port.
>
> Signed-off-by: Sergio Paracuellos <sergio.paracuel...@gmail.com>
> ---
> .../staging/mt7621-pci-phy/pci-mt7621-phy.c | 26 ++++++++++++++++---
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
> b/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
> index 98c06308143c..557e6a53fc1e 100644
> --- a/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
> +++ b/drivers/staging/mt7621-pci-phy/pci-mt7621-phy.c
> @@ -78,6 +78,8 @@
>
> #define RG_PE1_FRC_MSTCKDIV BIT(5)
>
> +#define MAX_PHYS 2
> +
> /**
> * struct mt7621_pci_phy_instance - Mt7621 Pcie PHY device
> * @phy: pointer to the kernel PHY device
> @@ -289,6 +291,20 @@ static const struct phy_ops mt7621_pci_phy_ops = {
> .owner = THIS_MODULE,
> };
>
> +static struct phy *mt7621_pcie_phy_of_xlate(struct device *dev,
> + struct of_phandle_args *args)
> +{
> + struct mt7621_pci_phy *mt7621_phy = dev_get_drvdata(dev);
> +
> + if (args->args_count == 0)
> + return mt7621_phy->phys[0]->phy;
> +
> + if (WARN_ON(args->args[0] >= MAX_PHYS))
> + return ERR_PTR(-ENODEV);
> +
> + return mt7621_phy->phys[args->args[0]]->phy;
> +}
> +
> static int mt7621_pci_phy_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -299,6 +315,7 @@ static int mt7621_pci_phy_probe(struct platform_device
> *pdev)
> struct resource res;
> int port, ret;
> void __iomem *port_base;
> + u32 phy_num;
>
> phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> if (!phy)
> @@ -325,8 +342,9 @@ static int mt7621_pci_phy_probe(struct platform_device
> *pdev)
> return PTR_ERR(port_base);
> }
>
> - port = 0;
> - for_each_child_of_node(np, child_np) {
This isn't the old place that you depend on the children nodes.
A little earlier, you have
phy->nphys = of_get_child_count(np);
which now sets nphys to zero. I changed this to
phy->nphys = MAX_PHYS;
> + of_property_read_u32(dev->of_node, "#phy-cells", &phy_num);
> +
> + for (port = 0; port < phy_num + 1; port++) {
I don't think it is correct to use #phy-cells as the number of phys.
#phy-cells is the number of arguments - the args_count seen in
mt7621_pci_phy_probe.
I think you should either assume phy_num is MAX_PHYS, or have built-in
knowledge of when it is 1 and when it is too.
There is minimal cost to allocating an extra phy, so I would go with
MAX_PHYS.
> struct mt7621_pci_phy_instance *instance;
> struct phy *pphy;
>
> @@ -338,7 +356,7 @@ static int mt7621_pci_phy_probe(struct platform_device
> *pdev)
>
> phy->phys[port] = instance;
>
> - pphy = devm_phy_create(dev, child_np, &mt7621_pci_phy_ops);
> + pphy = devm_phy_create(dev, dev->of_node, &mt7621_pci_phy_ops);
> if (IS_ERR(phy)) {
> dev_err(dev, "failed to create phy\n");
> ret = PTR_ERR(phy);
> @@ -352,7 +370,7 @@ static int mt7621_pci_phy_probe(struct platform_device
> *pdev)
> port++;
This "port++" duplicates the "port++" that you introduced in the "for"
loop above.
Thanks,
NeilBrown
> }
>
> - provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> + provider = devm_of_phy_provider_register(dev, mt7621_pcie_phy_of_xlate);
>
> return PTR_ERR_OR_ZERO(provider);
>
> --
> 2.19.1
signature.asc
Description: PGP signature
_______________________________________________ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
