[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAKxU2N-zuZfCwU83UA1SQ4c8JrJc+oGSg0Vu28P316uZsVMgcQ@mail.gmail.com>
Date: Thu, 28 Aug 2025 13:17:05 -0700
From: Rosen Penev <rosenp@...il.com>
To: Daniel Machon <daniel.machon@...rochip.com>
Cc: netdev@...r.kernel.org, horatiu.vultur@...rochip.com
Subject: Re: [PATCH net-next 2/2] net: lan966x: convert fwnode to of
On Thu, Aug 28, 2025 at 4:12 AM Daniel Machon
<daniel.machon@...rochip.com> wrote:
>
> > This is a purely OF driver. There's no need for fwnode to handle any of
> > this, with the exception being phylik_create. Use of_fwnode_handle for
> > that.
> >
> > Signed-off-by: Rosen Penev <rosenp@...il.com>
> > ---
> > .../ethernet/microchip/lan966x/lan966x_main.c | 32 ++++++++++---------
> > .../ethernet/microchip/lan966x/lan966x_main.h | 2 +-
> > 2 files changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > index 8bf28915c030..d778806dcfc6 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > @@ -183,7 +183,7 @@ static int lan966x_port_open(struct net_device *dev)
> > ANA_PORT_CFG_PORTID_VAL,
> > lan966x, ANA_PORT_CFG(port->chip_port));
> >
> > - err = phylink_fwnode_phy_connect(port->phylink, port->fwnode, 0);
> > + err = phylink_of_phy_connect(port->phylink, port->dnode, 0);
> > if (err) {
> > netdev_err(dev, "Could not attach to PHY\n");
> > return err;
> > @@ -767,8 +767,8 @@ static void lan966x_cleanup_ports(struct lan966x *lan966x)
> > port->phylink = NULL;
> > }
> >
> > - if (port->fwnode)
> > - fwnode_handle_put(port->fwnode);
> > + if (port->dnode)
> > + of_node_put(port->dnode);
> > }
> >
> > disable_irq(lan966x->xtr_irq);
> > @@ -1081,7 +1081,7 @@ static int lan966x_reset_switch(struct lan966x *lan966x)
> >
> > static int lan966x_probe(struct platform_device *pdev)
> > {
> > - struct fwnode_handle *ports, *portnp;
> > + struct device_node *ports, *portnp;
> > struct lan966x *lan966x;
> > int err;
> >
> > @@ -1179,7 +1179,7 @@ static int lan966x_probe(struct platform_device *pdev)
> > }
> > }
> >
> > - ports = device_get_named_child_node(&pdev->dev, "ethernet-ports");
> > + ports = of_get_child_by_name(pdev->dev.of_node, "ethernet-ports");
> > if (!ports)
> > return dev_err_probe(&pdev->dev, -ENODEV,
> > "no ethernet-ports child found\n");
> > @@ -1191,25 +1191,27 @@ static int lan966x_probe(struct platform_device *pdev)
> > lan966x_stats_init(lan966x);
> >
> > /* go over the child nodes */
> > - fwnode_for_each_available_child_node(ports, portnp) {
> > + for_each_available_child_of_node(ports, portnp) {
> > phy_interface_t phy_mode;
> > struct phy *serdes;
> > u32 p;
> >
> > - if (fwnode_property_read_u32(portnp, "reg", &p))
> > + if (of_property_read_u32(portnp, "reg", &p))
> > continue;
> >
> > - phy_mode = fwnode_get_phy_mode(portnp);
> > - err = lan966x_probe_port(lan966x, p, phy_mode, portnp);
> > + err = of_get_phy_mode(portnp, &phy_mode);
> > + if (err)
> > + goto cleanup_ports;
> > +
> > + err = lan966x_probe_port(lan966x, p, phy_mode, of_fwnode_handle(portnp));
>
> As I see it, you could change the signature of lan966x_probe_port() to accept a
> struct device_node, and instead pass that. Then you can convert it to fwnode
> for phylink_create, and ditch to_of_node().
Will fix.
>
> Same goes for lan966x_port_parse_delays(), here you can change
> fwnode_for_each_available_child_node() to for_each_available_child_of_node()
> and fwnode_property_read_u32() to of_property_read_u32().
I don't see this lan966x_port_parse_delays function.
>
> That will get rid of all the struct fwnode_handle uses and be more consistent.
>
> > if (err)
> > goto cleanup_ports;
> >
> > /* Read needed configuration */
> > lan966x->ports[p]->config.portmode = phy_mode;
> > - lan966x->ports[p]->fwnode = fwnode_handle_get(portnp);
> > + lan966x->ports[p]->dnode = of_node_get(portnp);
> >
> > - serdes = devm_of_phy_optional_get(lan966x->dev,
> > - to_of_node(portnp), NULL);
> > + serdes = devm_of_phy_optional_get(lan966x->dev, portnp, NULL);
> > if (IS_ERR(serdes)) {
> > err = PTR_ERR(serdes);
> > goto cleanup_ports;
> > @@ -1222,7 +1224,7 @@ static int lan966x_probe(struct platform_device *pdev)
> > goto cleanup_ports;
> > }
> >
> > - fwnode_handle_put(ports);
> > + of_node_put(ports);
> >
> > lan966x_mdb_init(lan966x);
> > err = lan966x_fdb_init(lan966x);
> > @@ -1255,8 +1257,8 @@ static int lan966x_probe(struct platform_device *pdev)
> > lan966x_fdb_deinit(lan966x);
> >
> > cleanup_ports:
> > - fwnode_handle_put(ports);
> > - fwnode_handle_put(portnp);
> > + of_node_put(ports);
> > + of_node_put(portnp);
> >
> > lan966x_cleanup_ports(lan966x);
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > index 4f75f0688369..bafb8f5ee64d 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > @@ -407,7 +407,7 @@ struct lan966x_port {
> > struct lan966x_port_config config;
> > struct phylink *phylink;
> > struct phy *serdes;
> > - struct fwnode_handle *fwnode;
> > + struct device_node *dnode;
> >
> > u8 ptp_tx_cmd;
> > bool ptp_rx_cmd;
> > --
> > 2.51.0
> >
>
> /Daniel
Powered by blists - more mailing lists