Re: [PATCH v2 02/13] net: ti: am65-cpsw-nuss: Define and switch to bind method for CPSW driver

[Chintan Vankar]

Hello Alexander and Roger,

On 20/02/25 01:18, Sverdlin, Alexander wrote:

Hi Chintan!

On Wed, 2025-02-19 at 16:18 +0530, Chintan Vankar wrote:

CPSW driver is defined as UCLASS_MISC driver which needs to be probed
explicitly. Define bind method for CPSW driver to scan and bind
ethernet-ports with UCLASS_ETH driver which will eventually probe CPSW
driver and avoids probing CPSW driver explicitly.

Signed-off-by: Chintan Vankar <c-van...@ti.com>
---

This patch is new in this series.

  drivers/net/ti/am65-cpsw-nuss.c | 129 +++++++++++++++++++-------------
  1 file changed, 75 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
index c70b42f6bcc..12c66095cce 100644
--- a/drivers/net/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ti/am65-cpsw-nuss.c
@@ -132,6 +132,7 @@ struct am65_cpsw_priv {
        struct am65_cpsw_common *cpsw_common;
        u32                     port_id;
        struct phy_device       *phydev;
+       bool                    probe_done;
  };

#ifdef PKTSIZE_ALIGN

[...]

@@ -675,6 +729,13 @@ static int am65_cpsw_port_probe(struct udevice *dev)
        char portname[32];
        int ret;

+ if (!priv->probe_done) {

+               ret = am65_cpsw_probe_nuss(dev->parent);
+               if (ret < 0)
+                       return ret;
+               priv->probe_done = true;
+       }
+

So this particular port will probe the parent device only once... But what 
about the
next port? Will it probe the same parent once again?

Is it actually necessary if device_probe() actually ensures, that all parents 
are
being probed first?

The probe_done you invent is probably much better served with
"dev_get_flags(dev) & DM_FLAG_ACTIVATED"?

Thank you Alexander for reviewing the patch and pointing out this since
I was not aware of dev->flags.

We can observe in device_probe() method that while probing a driver for
the dev we first check whether it's parents are probed or not. That is
also true for "am65_cpsw_port_probe" and that's why I don't think we
need to probe "am65_cpsw_probe_nuss" from it's child
am65_cpsw_port_probe since every time we probe child it's parent will
eventually get probed from device_probe method.

Also I have defined bind method by replacing a probe for
"am65_cpsw_probe_nuss" but also removed probe from the U_BOOT_DRIVER
definition by mistake, I will correct it in next version.

Let me know what you think.

Regards,
Chintan.

        priv->dev = dev;

cpsw_common = dev_get_priv(dev->parent);