[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-vankar at 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);
>