Hello Russell,
Thanks for the review.
On Sat, 14 May 2022 18:18:17 +0100
"Russell King (Oracle)" <linux@xxxxxxxxxxxxxxx> wrote:
> On Sat, May 14, 2022 at 05:06:52PM +0200, Maxime Chevallier wrote:
> > +static int ipqess_do_ioctl(struct net_device *netdev, struct ifreq
> > *ifr, int cmd) +{
> > + struct ipqess *ess = netdev_priv(netdev);
> > +
> > + switch (cmd) {
> > + case SIOCGMIIPHY:
> > + case SIOCGMIIREG:
> > + case SIOCSMIIREG:
> > + return phylink_mii_ioctl(ess->phylink, ifr, cmd);
> > + default:
> > + break;
> > + }
> > +
> > + return -EOPNOTSUPP;
> > +}
>
> Is there a reason this isn't just:
>
> return phylink_mii_ioctl(ess->phylink, ifr, cmd);
Not really, an oversight on my part, I'll address that in v3
> ?
>
> > +static int ipqess_axi_probe(struct platform_device *pdev)
> > +{
> > + struct device_node *np = pdev->dev.of_node;
> > + struct net_device *netdev;
> > + phy_interface_t phy_mode;
> > + struct resource *res;
> > + struct ipqess *ess;
> > + int i, err = 0;
> > +
> > + netdev = devm_alloc_etherdev_mqs(&pdev->dev, sizeof(struct
> > ipqess),
> > + IPQESS_NETDEV_QUEUES,
> > + IPQESS_NETDEV_QUEUES);
> > + if (!netdev)
> > + return -ENOMEM;
> > +
> > + ess = netdev_priv(netdev);
> > + ess->netdev = netdev;
> > + ess->pdev = pdev;
> > + spin_lock_init(&ess->stats_lock);
> > + SET_NETDEV_DEV(netdev, &pdev->dev);
> > + platform_set_drvdata(pdev, netdev);
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + ess->hw_addr = devm_ioremap_resource(&pdev->dev, res);
> > + if (IS_ERR(ess->hw_addr))
> > + return PTR_ERR(ess->hw_addr);
> > +
> > + err = of_get_phy_mode(np, &phy_mode);
> > + if (err) {
> > + dev_err(&pdev->dev, "incorrect phy-mode\n");
> > + return err;
> > + }
> > +
> > + ess->ess_clk = devm_clk_get(&pdev->dev, "ess");
> > + if (!IS_ERR(ess->ess_clk))
> > + clk_prepare_enable(ess->ess_clk);
> > +
> > + ess->ess_rst = devm_reset_control_get(&pdev->dev, "ess");
> > + if (IS_ERR(ess->ess_rst))
> > + goto err_clk;
> > +
> > + ipqess_reset(ess);
> > +
> > + ess->phylink_config.dev = &netdev->dev;
> > + ess->phylink_config.type = PHYLINK_NETDEV;
> > +
> > + __set_bit(PHY_INTERFACE_MODE_INTERNAL,
> > + ess->phylink_config.supported_interfaces);
>
> No mac capabilities?
My bad too, I also missed that. I'll also address that in v3.
> > +
> > + ess->phylink = phylink_create(&ess->phylink_config,
> > + of_fwnode_handle(np),
> > phy_mode,
> > + &ipqess_phylink_mac_ops);
>
Thanks,
Maxime
