From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E7BD3C19F2E for ; Thu, 27 Feb 2025 10:26:57 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 6A3D980FE1; Thu, 27 Feb 2025 11:26:56 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="I0gTxw5m"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 5BAEB80FF0; Thu, 27 Feb 2025 11:26:55 +0100 (CET) Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 519C580F12 for ; Thu, 27 Feb 2025 11:26:52 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=kernel.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=rogerq@kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 087105C0631; Thu, 27 Feb 2025 10:26:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4EE2AC4CEDD; Thu, 27 Feb 2025 10:26:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1740652010; bh=FOeFECw+yfToLVlL7mgHiQ9yOrZB9AS9Ifj3KsV8DBU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=I0gTxw5miQnYNu2Caxt5BP/o1jzuDiGyUm9jCJQkW4ql0ryRbEpwmgZ2qLXVMTSzb Pc+IyWMuuVFFHEFjT5XVBMeccjGjZPo3Bb7ezwZqbhFmkkeeppnuL+cbZ2qRi4Jo9x s6CDI0Em3xraOn8s0hgJGtsWszAFY+2hfmvj3msZv9ryQ1MioLctO3u/Tvr9BvqNtv NxTUvftzslu3Hdg5saA0sie8UqU5K7e8+u/9/A0k5KfCML3pImXOBEwwvjEjwhZSlE mG4cMDlFcljQXz9Suw4knJrMxugVUmJ9zmAnqc/1nFafYgW6CGITdUH853mmMoiD2i b6I649OtRm4nQ== Message-ID: Date: Thu, 27 Feb 2025 12:26:41 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 01/15] net: ti: am65-cpsw-nuss: Define bind method for CPSW driver To: Chintan Vankar , Michael Walle , Matthias Schiffer , MD Danish Anwar , Hari Nagalla , Manorit Chawdhry , Jonathan Humphreys , Santhosh Kumar K , Ilias Apalodimas , Neha Malcom Francis , Andreas Dannenberg , Andrew Davis , Alexander Sverdlin , Joao Paulo Goncalves , Kishon Vijay Abraham I , Wadim Egorov , Ramon Fried , Joe Hershberger , Jayesh Choudhary , Vaishnav Achath , Bryan Brattlof , Vignesh Raghavendra , Tom Rini Cc: s-vadapalli@ti.com, u-boot@lists.denx.de References: <20250225114903.2080616-1-c-vankar@ti.com> <20250225114903.2080616-2-c-vankar@ti.com> Content-Language: en-US From: Roger Quadros In-Reply-To: <20250225114903.2080616-2-c-vankar@ti.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean 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. > > Signed-off-by: Chintan Vankar > --- > > 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), > }; -- cheers, -roger