[PATCH v3 01/15] net: ti: am65-cpsw-nuss: Define bind method for CPSW driver

Vankar, Chintan c-vankar at ti.com
Tue Mar 4 05:45:03 CET 2025


Hello Roger,

On 2/27/2025 3:56 PM, Roger Quadros wrote:
> Hi,
> 
> On 25/02/2025 13:48, 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.
> 
> Thank you for doing this. Overall it looks good. Just some cosmetic changes
> suggested below.
> 

Thank you for reviewing the series, I will make the changes mentioned by
you and post next version.

Regards,
Chintan.

>>
>> Signed-off-by: Chintan Vankar <c-vankar at ti.com>
>> ---
>>
>> Link to v2:
>> https://lore.kernel.org/r/20250219104831.2315464-3-c-vankar@ti.com/
>>
>> Changes from v2 to v3:
>> - Removed if condition not required in CPSW driver as suggested by
>>    Alexander Sverdlin.
>>
>>   drivers/net/ti/am65-cpsw-nuss.c | 120 ++++++++++++++++++--------------
>>   1 file changed, 67 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
>> index c70b42f6bcc..10513cf92f2 100644
>> --- a/drivers/net/ti/am65-cpsw-nuss.c
>> +++ b/drivers/net/ti/am65-cpsw-nuss.c
>> @@ -667,6 +667,59 @@ static int am65_cpsw_ofdata_parse_phy(struct udevice *dev)
>>   	return 0;
>>   }
>>   
>> +static int am65_cpsw_probe_nuss(struct udevice *dev)
>> +{
>> +	struct am65_cpsw_common *cpsw_common = dev_get_priv(dev);
>> +	int ret, i;
>> +
>> +	cpsw_common->dev = dev;
>> +	cpsw_common->ss_base = dev_read_addr(dev);
>> +	if (cpsw_common->ss_base == FDT_ADDR_T_NONE)
>> +		return -EINVAL;
>> +
>> +	ret = power_domain_get_by_index(dev, &cpsw_common->pwrdmn, 0);
>> +	if (ret) {
>> +		dev_err(dev, "failed to get pwrdmn: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_get_by_name(dev, "fck", &cpsw_common->fclk);
>> +	if (ret) {
>> +		power_domain_free(&cpsw_common->pwrdmn);
>> +		dev_err(dev, "failed to get clock %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	cpsw_common->cpsw_base = cpsw_common->ss_base + AM65_CPSW_CPSW_NU_BASE;
>> +	cpsw_common->ale_base = cpsw_common->cpsw_base +
>> +				AM65_CPSW_CPSW_NU_ALE_BASE;
>> +
>> +	for (i = 0; i < AM65_CPSW_CPSWNU_MAX_PORTS; i++) {
>> +		struct am65_cpsw_port *port = &cpsw_common->ports[i];
>> +
>> +		port->port_base = cpsw_common->cpsw_base +
>> +				  AM65_CPSW_CPSW_NU_PORTS_OFFSET +
>> +				  (i * AM65_CPSW_CPSW_NU_PORTS_OFFSET);
>> +		port->port_sgmii_base = cpsw_common->ss_base +
>> +					(i * AM65_CPSW_SGMII_BASE);
>> +		port->macsl_base = port->port_base +
>> +				   AM65_CPSW_CPSW_NU_PORT_MACSL_OFFSET;
>> +	}
>> +
>> +	cpsw_common->bus_freq =
>> +			dev_read_u32_default(dev, "bus_freq",
>> +					     AM65_CPSW_MDIO_BUS_FREQ_DEF);
>> +
>> +	dev_info(dev, "K3 CPSW: nuss_ver: 0x%08X cpsw_ver: 0x%08X ale_ver: 0x%08X Ports:%u\n",
>> +		 readl(cpsw_common->ss_base),
>> +		 readl(cpsw_common->cpsw_base),
>> +		 readl(cpsw_common->ale_base),
>> +		 cpsw_common->port_num);
>> +
>> +	power_domain_free(&cpsw_common->pwrdmn);
>> +	return ret;
>> +}
>> +
> 
> Instead of moving the am65_cpsw_probe_nuss() please leave it where it was
> and add the new am65_cpsw_nuss_bind() after it.
> This way it will show only what really changed in diff and is easier to review.
> 
>>   static int am65_cpsw_port_probe(struct udevice *dev)
>>   {
>>   	struct am65_cpsw_priv *priv = dev_get_priv(dev);
>> @@ -697,45 +750,30 @@ out:
>>   	return ret;
>>   }
>>   
>> -static int am65_cpsw_probe_nuss(struct udevice *dev)
>> +static int am65_cpsw_nuss_bind(struct udevice *dev)
>>   {
>>   	struct am65_cpsw_common *cpsw_common = dev_get_priv(dev);
>> -	ofnode ports_np, node;
>> -	int ret, i;
>> +	struct uclass_driver *drv;
>>   	struct udevice *port_dev;
>> +	ofnode ports_np, node;
>> +	int ret;
>>   
>> -	cpsw_common->dev = dev;
>> -	cpsw_common->ss_base = dev_read_addr(dev);
>> -	if (cpsw_common->ss_base == FDT_ADDR_T_NONE)
>> -		return -EINVAL;
>> -
>> -	ret = power_domain_get_by_index(dev, &cpsw_common->pwrdmn, 0);
>> -	if (ret) {
>> -		dev_err(dev, "failed to get pwrdmn: %d\n", ret);
>> -		return ret;
>> -	}
>> -
>> -	ret = clk_get_by_name(dev, "fck", &cpsw_common->fclk);
>> -	if (ret) {
>> -		power_domain_free(&cpsw_common->pwrdmn);
>> -		dev_err(dev, "failed to get clock %d\n", ret);
>> -		return ret;
>> +	drv = lists_uclass_lookup(UCLASS_ETH);
>> +	if (!drv) {
>> +		puts("Cannot find eth driver");
>> +		return -ENOENT;
> 
> I'm not sure why this check is required.
> drv is not used anywhere.
>  >>   	}
>>   
>> -	cpsw_common->cpsw_base = cpsw_common->ss_base + AM65_CPSW_CPSW_NU_BASE;
>> -	cpsw_common->ale_base = cpsw_common->cpsw_base +
>> -				AM65_CPSW_CPSW_NU_ALE_BASE;
>> -
>> +	cpsw_common->dev = dev;
>>   	ports_np = dev_read_subnode(dev, "ethernet-ports");
>>   	if (!ofnode_valid(ports_np)) {
>> -		ret = -ENOENT;
>> -		goto out;
>> +		return -ENOENT;
>>   	}
>>   
>>   	ofnode_for_each_subnode(node, ports_np) {
>>   		const char *node_name;
>> -		u32 port_id;
>>   		bool disabled;
>> +		u32 port_id;
>>   
>>   		node_name = ofnode_get_name(node);
>>   
>> @@ -745,14 +783,13 @@ static int am65_cpsw_probe_nuss(struct udevice *dev)
>>   		if (ret) {
>>   			dev_err(dev, "%s: failed to get port_id (%d)\n",
>>   				node_name, ret);
>> -			goto out;
>> +			return ret;
>>   		}
>>   
>>   		if (port_id >= AM65_CPSW_CPSWNU_MAX_PORTS) {
>>   			dev_err(dev, "%s: invalid port_id (%d)\n",
>>   				node_name, port_id);
>> -			ret = -EINVAL;
>> -			goto out;
>> +			return -EINVAL;
>>   		}
>>   		cpsw_common->port_num++;
>>   
>> @@ -768,30 +805,6 @@ static int am65_cpsw_probe_nuss(struct udevice *dev)
>>   			dev_err(dev, "Failed to bind to %s node\n", ofnode_get_name(node));
>>   	}
>>   
>> -	for (i = 0; i < AM65_CPSW_CPSWNU_MAX_PORTS; i++) {
>> -		struct am65_cpsw_port *port = &cpsw_common->ports[i];
>> -
>> -		port->port_base = cpsw_common->cpsw_base +
>> -				  AM65_CPSW_CPSW_NU_PORTS_OFFSET +
>> -				  (i * AM65_CPSW_CPSW_NU_PORTS_OFFSET);
>> -		port->port_sgmii_base = cpsw_common->ss_base +
>> -					(i * AM65_CPSW_SGMII_BASE);
>> -		port->macsl_base = port->port_base +
>> -				   AM65_CPSW_CPSW_NU_PORT_MACSL_OFFSET;
>> -	}
>> -
>> -	cpsw_common->bus_freq =
>> -			dev_read_u32_default(dev, "bus_freq",
>> -					     AM65_CPSW_MDIO_BUS_FREQ_DEF);
>> -
>> -	dev_info(dev, "K3 CPSW: nuss_ver: 0x%08X cpsw_ver: 0x%08X ale_ver: 0x%08X Ports:%u\n",
>> -		 readl(cpsw_common->ss_base),
>> -		 readl(cpsw_common->cpsw_base),
>> -		 readl(cpsw_common->ale_base),
>> -		 cpsw_common->port_num);
>> -
>> -out:
>> -	power_domain_free(&cpsw_common->pwrdmn);
>>   	return ret;
>>   }
>>   
>> @@ -806,6 +819,7 @@ U_BOOT_DRIVER(am65_cpsw_nuss) = {
>>   	.name	= "am65_cpsw_nuss",
>>   	.id	= UCLASS_MISC,
>>   	.of_match = am65_cpsw_nuss_ids,
>> +	.bind	= am65_cpsw_nuss_bind,
>>   	.probe	= am65_cpsw_probe_nuss,
>>   	.priv_auto = sizeof(struct am65_cpsw_common),
>>   };
> 


More information about the U-Boot mailing list