From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org, yangbo.lu@nxp.com,
xiaoliang.yang_1@nxp.com, UNGLinuxDriver@microchip.com,
claudiu.manoil@nxp.com, andrew@lunn.ch, vivien.didelot@gmail.com,
f.fainelli@gmail.com, kuba@kernel.org
Subject: Re: [PATCH v2 net 6/8] net: mscc: ocelot: refactor ports parsing code into a dedicated function
Date: Fri, 18 Sep 2020 17:28:40 +0200 [thread overview]
Message-ID: <20200918152840.GC9675@piout.net> (raw)
In-Reply-To: <20200918010730.2911234-7-olteanv@gmail.com>
On 18/09/2020 04:07:28+0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>
> mscc_ocelot_probe() is already pretty large and hard to follow. So move
> the code for parsing ports in a separate function.
>
> This makes it easier for the next patch to just call
> mscc_ocelot_release_ports from the error path of mscc_ocelot_init_ports.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Reviewed-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Tested-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
> Changes in v2:
> Keep a reference to the 'ports' OF node at caller side, in
> mscc_ocelot_probe, because we need to populate ocelot->num_phys_ports
> early. The ocelot_init() function depends on it being set correctly.
>
> drivers/net/ethernet/mscc/ocelot_vsc7514.c | 209 +++++++++++----------
> 1 file changed, 110 insertions(+), 99 deletions(-)
>
> diff --git a/drivers/net/ethernet/mscc/ocelot_vsc7514.c b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
> index a1cbb20a7757..ff4a01424953 100644
> --- a/drivers/net/ethernet/mscc/ocelot_vsc7514.c
> +++ b/drivers/net/ethernet/mscc/ocelot_vsc7514.c
> @@ -896,11 +896,115 @@ static struct ptp_clock_info ocelot_ptp_clock_info = {
> .enable = ocelot_ptp_enable,
> };
>
> +static int mscc_ocelot_init_ports(struct platform_device *pdev,
> + struct device_node *ports)
> +{
> + struct ocelot *ocelot = platform_get_drvdata(pdev);
> + struct device_node *portnp;
> + int err;
> +
> + ocelot->ports = devm_kcalloc(ocelot->dev, ocelot->num_phys_ports,
> + sizeof(struct ocelot_port *), GFP_KERNEL);
> + if (!ocelot->ports)
> + return -ENOMEM;
> +
> + /* No NPI port */
> + ocelot_configure_cpu(ocelot, -1, OCELOT_TAG_PREFIX_NONE,
> + OCELOT_TAG_PREFIX_NONE);
> +
> + for_each_available_child_of_node(ports, portnp) {
> + struct ocelot_port_private *priv;
> + struct ocelot_port *ocelot_port;
> + struct device_node *phy_node;
> + phy_interface_t phy_mode;
> + struct phy_device *phy;
> + struct regmap *target;
> + struct resource *res;
> + struct phy *serdes;
> + char res_name[8];
> + u32 port;
> +
> + if (of_property_read_u32(portnp, "reg", &port))
> + continue;
> +
> + snprintf(res_name, sizeof(res_name), "port%d", port);
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> + res_name);
> + target = ocelot_regmap_init(ocelot, res);
> + if (IS_ERR(target))
> + continue;
> +
> + phy_node = of_parse_phandle(portnp, "phy-handle", 0);
> + if (!phy_node)
> + continue;
> +
> + phy = of_phy_find_device(phy_node);
> + of_node_put(phy_node);
> + if (!phy)
> + continue;
> +
> + err = ocelot_probe_port(ocelot, port, target, phy);
> + if (err) {
> + of_node_put(portnp);
> + return err;
> + }
> +
> + ocelot_port = ocelot->ports[port];
> + priv = container_of(ocelot_port, struct ocelot_port_private,
> + port);
> +
> + of_get_phy_mode(portnp, &phy_mode);
> +
> + ocelot_port->phy_mode = phy_mode;
> +
> + switch (ocelot_port->phy_mode) {
> + case PHY_INTERFACE_MODE_NA:
> + continue;
> + case PHY_INTERFACE_MODE_SGMII:
> + break;
> + case PHY_INTERFACE_MODE_QSGMII:
> + /* Ensure clock signals and speed is set on all
> + * QSGMII links
> + */
> + ocelot_port_writel(ocelot_port,
> + DEV_CLOCK_CFG_LINK_SPEED
> + (OCELOT_SPEED_1000),
> + DEV_CLOCK_CFG);
> + break;
> + default:
> + dev_err(ocelot->dev,
> + "invalid phy mode for port%d, (Q)SGMII only\n",
> + port);
> + of_node_put(portnp);
> + return -EINVAL;
> + }
> +
> + serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> + if (IS_ERR(serdes)) {
> + err = PTR_ERR(serdes);
> + if (err == -EPROBE_DEFER)
> + dev_dbg(ocelot->dev, "deferring probe\n");
> + else
> + dev_err(ocelot->dev,
> + "missing SerDes phys for port%d\n",
> + port);
> +
> + of_node_put(portnp);
> + return err;
> + }
> +
> + priv->serdes = serdes;
> + }
> +
> + return 0;
> +}
> +
> static int mscc_ocelot_probe(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> - struct device_node *ports, *portnp;
> int err, irq_xtr, irq_ptp_rdy;
> + struct device_node *ports;
> struct ocelot *ocelot;
> struct regmap *hsio;
> unsigned int i;
> @@ -985,19 +1089,12 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
>
> ports = of_get_child_by_name(np, "ethernet-ports");
> if (!ports) {
> - dev_err(&pdev->dev, "no ethernet-ports child node found\n");
> + dev_err(ocelot->dev, "no ethernet-ports child node found\n");
> return -ENODEV;
> }
>
> ocelot->num_phys_ports = of_get_child_count(ports);
>
> - ocelot->ports = devm_kcalloc(&pdev->dev, ocelot->num_phys_ports,
> - sizeof(struct ocelot_port *), GFP_KERNEL);
> - if (!ocelot->ports) {
> - err = -ENOMEM;
> - goto out_put_ports;
> - }
> -
> ocelot->vcap_is2_keys = vsc7514_vcap_is2_keys;
> ocelot->vcap_is2_actions = vsc7514_vcap_is2_actions;
> ocelot->vcap = vsc7514_vcap_props;
> @@ -1006,6 +1103,10 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> if (err)
> goto out_put_ports;
>
> + err = mscc_ocelot_init_ports(pdev, ports);
> + if (err)
> + goto out_put_ports;
> +
> if (ocelot->ptp) {
> err = ocelot_init_timestamp(ocelot, &ocelot_ptp_clock_info);
> if (err) {
> @@ -1015,96 +1116,6 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
> }
> }
>
> - /* No NPI port */
> - ocelot_configure_cpu(ocelot, -1, OCELOT_TAG_PREFIX_NONE,
> - OCELOT_TAG_PREFIX_NONE);
> -
> - for_each_available_child_of_node(ports, portnp) {
> - struct ocelot_port_private *priv;
> - struct ocelot_port *ocelot_port;
> - struct device_node *phy_node;
> - phy_interface_t phy_mode;
> - struct phy_device *phy;
> - struct regmap *target;
> - struct resource *res;
> - struct phy *serdes;
> - char res_name[8];
> - u32 port;
> -
> - if (of_property_read_u32(portnp, "reg", &port))
> - continue;
> -
> - snprintf(res_name, sizeof(res_name), "port%d", port);
> -
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> - res_name);
> - target = ocelot_regmap_init(ocelot, res);
> - if (IS_ERR(target))
> - continue;
> -
> - phy_node = of_parse_phandle(portnp, "phy-handle", 0);
> - if (!phy_node)
> - continue;
> -
> - phy = of_phy_find_device(phy_node);
> - of_node_put(phy_node);
> - if (!phy)
> - continue;
> -
> - err = ocelot_probe_port(ocelot, port, target, phy);
> - if (err) {
> - of_node_put(portnp);
> - goto out_put_ports;
> - }
> -
> - ocelot_port = ocelot->ports[port];
> - priv = container_of(ocelot_port, struct ocelot_port_private,
> - port);
> -
> - of_get_phy_mode(portnp, &phy_mode);
> -
> - ocelot_port->phy_mode = phy_mode;
> -
> - switch (ocelot_port->phy_mode) {
> - case PHY_INTERFACE_MODE_NA:
> - continue;
> - case PHY_INTERFACE_MODE_SGMII:
> - break;
> - case PHY_INTERFACE_MODE_QSGMII:
> - /* Ensure clock signals and speed is set on all
> - * QSGMII links
> - */
> - ocelot_port_writel(ocelot_port,
> - DEV_CLOCK_CFG_LINK_SPEED
> - (OCELOT_SPEED_1000),
> - DEV_CLOCK_CFG);
> - break;
> - default:
> - dev_err(ocelot->dev,
> - "invalid phy mode for port%d, (Q)SGMII only\n",
> - port);
> - of_node_put(portnp);
> - err = -EINVAL;
> - goto out_put_ports;
> - }
> -
> - serdes = devm_of_phy_get(ocelot->dev, portnp, NULL);
> - if (IS_ERR(serdes)) {
> - err = PTR_ERR(serdes);
> - if (err == -EPROBE_DEFER)
> - dev_dbg(ocelot->dev, "deferring probe\n");
> - else
> - dev_err(ocelot->dev,
> - "missing SerDes phys for port%d\n",
> - port);
> -
> - of_node_put(portnp);
> - goto out_put_ports;
> - }
> -
> - priv->serdes = serdes;
> - }
> -
> register_netdevice_notifier(&ocelot_netdevice_nb);
> register_switchdev_notifier(&ocelot_switchdev_nb);
> register_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
> --
> 2.25.1
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
next prev parent reply other threads:[~2020-09-18 15:28 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-18 1:07 [PATCH v2 net 0/8] Bugfixes in Microsemi Ocelot switch driver Vladimir Oltean
2020-09-18 1:07 ` [PATCH v2 net 1/8] net: mscc: ocelot: fix race condition with TX timestamping Vladimir Oltean
2020-09-18 15:23 ` Alexandre Belloni
2020-09-18 15:24 ` Alexandre Belloni
2020-09-18 15:24 ` Alexandre Belloni
2020-09-18 15:24 ` Alexandre Belloni
2020-09-18 15:24 ` Alexandre Belloni
2020-09-18 15:25 ` Alexandre Belloni
2020-09-18 15:25 ` Alexandre Belloni
2020-09-18 15:26 ` Alexandre Belloni
2020-09-18 15:27 ` Alexandre Belloni
2020-09-18 1:07 ` [PATCH v2 net 2/8] net: mscc: ocelot: add locking for the port TX timestamp ID Vladimir Oltean
2020-09-18 2:37 ` Florian Fainelli
2020-09-18 15:27 ` Alexandre Belloni
2020-09-18 1:07 ` [PATCH v2 net 3/8] net: dsa: seville: fix buffer size of the queue system Vladimir Oltean
2020-09-18 2:37 ` Florian Fainelli
2020-09-18 15:27 ` Alexandre Belloni
2020-09-18 1:07 ` [PATCH v2 net 4/8] net: mscc: ocelot: check for errors on memory allocation of ports Vladimir Oltean
2020-09-18 2:35 ` Florian Fainelli
2020-09-18 15:27 ` Alexandre Belloni
2020-09-18 1:07 ` [PATCH v2 net 5/8] net: mscc: ocelot: error checking when calling ocelot_init() Vladimir Oltean
2020-09-18 2:35 ` Florian Fainelli
2020-09-18 15:28 ` Alexandre Belloni
2020-09-18 1:07 ` [PATCH v2 net 6/8] net: mscc: ocelot: refactor ports parsing code into a dedicated function Vladimir Oltean
2020-09-18 2:34 ` Florian Fainelli
2020-09-18 15:28 ` Alexandre Belloni [this message]
2020-09-18 1:07 ` [PATCH v2 net 7/8] net: mscc: ocelot: unregister net devices on unbind Vladimir Oltean
2020-09-18 2:34 ` Florian Fainelli
2020-09-18 15:28 ` Alexandre Belloni
2020-09-18 1:07 ` [PATCH v2 net 8/8] net: mscc: ocelot: deinitialize only initialized ports Vladimir Oltean
2020-09-18 2:36 ` Florian Fainelli
2020-09-18 15:29 ` Alexandre Belloni
2020-09-18 20:52 ` [PATCH v2 net 0/8] Bugfixes in Microsemi Ocelot switch driver David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200918152840.GC9675@piout.net \
--to=alexandre.belloni@bootlin.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=andrew@lunn.ch \
--cc=claudiu.manoil@nxp.com \
--cc=davem@davemloft.net \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=vivien.didelot@gmail.com \
--cc=xiaoliang.yang_1@nxp.com \
--cc=yangbo.lu@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.