[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