[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250220091904.GA1230612@maili.marvell.com>
Date: Thu, 20 Feb 2025 14:49:04 +0530
From: Ratheesh Kannoth <rkannoth@...vell.com>
To: parvathi <parvathi@...thit.com>
CC: <danishanwar@...com>, <rogerq@...nel.org>, <andrew+netdev@...n.ch>,
<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
<pabeni@...hat.com>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>, <nm@...com>, <ssantosh@...nel.org>,
<richardcochran@...il.com>, <basharath@...thit.com>,
<schnelle@...ux.ibm.com>, <diogo.ivo@...mens.com>,
<m-karicheri2@...com>, <horms@...nel.org>, <jacob.e.keller@...el.com>,
<m-malladi@...com>, <javier.carrasco.cruz@...il.com>, <afd@...com>,
<s-anna@...com>, <linux-arm-kernel@...ts.infradead.org>,
<netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <pratheesh@...com>, <prajith@...com>,
<vigneshr@...com>, <praneeth@...com>, <srk@...com>, <rogerq@...com>,
<krishna@...thit.com>, <pmohan@...thit.com>, <mohan@...thit.com>
Subject: Re: [PATCH net-next v3 02/10] net: ti: prueth: Adds ICSSM Ethernet
driver
On 2025-02-14 at 11:16:54, parvathi (parvathi@...thit.com) wrote:
> From: Roger Quadros <rogerq@...com>
>
> +static int icssm_prueth_probe(struct platform_device *pdev)
> +{
> + struct device_node *eth0_node = NULL, *eth1_node = NULL;
> + struct device_node *eth_node, *eth_ports_node;
> + enum pruss_pru_id pruss_id0, pruss_id1;
> + struct device *dev = &pdev->dev;
> + struct device_node *np;
> + struct prueth *prueth;
> + int i, ret;
> +
> + np = dev->of_node;
> + if (!np)
> + return -ENODEV; /* we don't support non DT */
> +
> + prueth = devm_kzalloc(dev, sizeof(*prueth), GFP_KERNEL);
> + if (!prueth)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, prueth);
> + prueth->dev = dev;
> + prueth->fw_data = device_get_match_data(dev);
> +
> + eth_ports_node = of_get_child_by_name(np, "ethernet-ports");
> + if (!eth_ports_node)
> + return -ENOENT;
> +
> + for_each_child_of_node(eth_ports_node, eth_node) {
> + u32 reg;
> +
> + if (strcmp(eth_node->name, "ethernet-port"))
> + continue;
> + ret = of_property_read_u32(eth_node, "reg", ®);
> + if (ret < 0) {
> + dev_err(dev, "%pOF error reading port_id %d\n",
> + eth_node, ret);
> + }
> +
> + of_node_get(eth_node);
> +
> + if (reg == 0) {
> + eth0_node = eth_node;
> + if (!of_device_is_available(eth0_node)) {
> + of_node_put(eth0_node);
> + eth0_node = NULL;
> + }
> + } else if (reg == 1) {
> + eth1_node = eth_node;
> + if (!of_device_is_available(eth1_node)) {
> + of_node_put(eth1_node);
> + eth1_node = NULL;
> + }
It depends on your DT, but is there any chance that more than once, we reach here in else case.
{ if (of_device_is_availabke(...) } ?
> + } else {
> + dev_err(dev, "port reg should be 0 or 1\n");
> + }
> + }
> +
> + of_node_put(eth_ports_node);
> +
> + /* At least one node must be present and available else we fail */
> + if (!eth0_node && !eth1_node) {
> + dev_err(dev, "neither port0 nor port1 node available\n");
> + return -ENODEV;
> + }
> +
> + if (eth0_node == eth1_node) {
> + dev_err(dev, "port0 and port1 can't have same reg\n");
> + of_node_put(eth0_node);
> + return -ENODEV;
> + }
> +
> + prueth->eth_node[PRUETH_MAC0] = eth0_node;
> + prueth->eth_node[PRUETH_MAC1] = eth1_node;
...
+
> + if (eth0_node) {
> + prueth->pru0 = pru_rproc_get(np, 0, &pruss_id0);
> + if (IS_ERR(prueth->pru0)) {
> + ret = PTR_ERR(prueth->pru0);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "unable to get PRU0: %d\n", ret);
> + goto put_pru;
> + }
> + }
> +
> + if (eth1_node) {
> + prueth->pru1 = pru_rproc_get(np, 1, &pruss_id1);
> + if (IS_ERR(prueth->pru1)) {
> + ret = PTR_ERR(prueth->pru1);
> + if (ret != -EPROBE_DEFER)
> + dev_err(dev, "unable to get PRU1: %d\n", ret);
> + goto put_pru;
> + }
> + }
> +
> + /* setup netdev interfaces */
> + if (eth0_node) {
> + ret = icssm_prueth_netdev_init(prueth, eth0_node);
> + if (ret) {
> + if (ret != -EPROBE_DEFER) {
> + dev_err(dev, "netdev init %s failed: %d\n",
> + eth0_node->name, ret);
> + }
> + goto put_pru;
> + }
> + }
> +
> + if (eth1_node) {
> + ret = icssm_prueth_netdev_init(prueth, eth1_node);
> + if (ret) {
> + if (ret != -EPROBE_DEFER) {
> + dev_err(dev, "netdev init %s failed: %d\n",
> + eth1_node->name, ret);
> + }
> + goto netdev_exit;
> + }
> + }
> +
> + /* register the network devices */
> + if (eth0_node) {
> + ret = register_netdev(prueth->emac[PRUETH_MAC0]->ndev);
> + if (ret) {
> + dev_err(dev, "can't register netdev for port MII0");
> + goto netdev_exit;
> + }
> +
> + prueth->registered_netdevs[PRUETH_MAC0] =
> + prueth->emac[PRUETH_MAC0]->ndev;
> + }
> +
> + if (eth1_node) {
> + ret = register_netdev(prueth->emac[PRUETH_MAC1]->ndev);
> + if (ret) {
> + dev_err(dev, "can't register netdev for port MII1");
> + goto netdev_unregister;
> + }
> +
> + prueth->registered_netdevs[PRUETH_MAC1] =
> + prueth->emac[PRUETH_MAC1]->ndev;
> + }
> +
> + if (eth1_node)
> + of_node_put(eth1_node);
> + if (eth0_node)
> + of_node_put(eth0_node);
nit: A function would be better than code duplication for eth0_node and eth1_node ?
> + return 0;
> +
> +netdev_unregister:
> + for (i = 0; i < PRUETH_NUM_MACS; i++) {
> + if (!prueth->registered_netdevs[i])
> + continue;
> + unregister_netdev(prueth->registered_netdevs[i]);
> + }
> +
> +netdev_exit:
> + for (i = 0; i < PRUETH_NUM_MACS; i++) {
> + eth_node = prueth->eth_node[i];
> + if (!eth_node)
> + continue;
> +
> + icssm_prueth_netdev_exit(prueth, eth_node);
> + }
> +
> +put_pru:
> + if (eth1_node) {
> + if (prueth->pru1)
> + pru_rproc_put(prueth->pru1);
> + of_node_put(eth1_node);
> + }
> +
> + if (eth0_node) {
> + if (prueth->pru0)
> + pru_rproc_put(prueth->pru0);
> + of_node_put(eth0_node);
> + }
> +
> + return ret;
> +}
> +
Powered by blists - more mailing lists