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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BE73ECDE032 for ; Thu, 26 Sep 2024 18:54:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=+KusUymBZTpYYnq4WmcbbE/FZsVAjrJbZNZbABf3CAo=; b=DHJofvJ0xgFf5I5yzRmH97/+Yd eH1+xpNB85wXY5gcM4dC/Fu1BH91K+HdICzGZZZ2ahlhnY19SpvrVSxc04ge9xpizi7g6ZWHLKow/ JL55+LjnmKiPya69HMO25AyPNLpA6W5uw9XbxRblowjO+sC9ek+DehjU8w9shbQgo/ZJaz3QAb7Vv 36R/KlqoqFCj2HdpyP/1vT8w0L6B/2tYeho0fO0/qENHTCLqq0xKaqUqboztE1xsoyR97LXjQBnMA PxDLdNoKekqGJ4kY6fToYoZOd5kk9NynJXuJSIOEVKbKYln25/QSO8Ql7slCk31FaWFJ9WVpvjQbD wc+npPYA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1sttd2-00000009Bbf-3hbp; Thu, 26 Sep 2024 18:54:16 +0000 Received: from mail-pl1-x634.google.com ([2607:f8b0:4864:20::634]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1sttWo-00000009Aak-3Mq2 for linux-arm-kernel@lists.infradead.org; Thu, 26 Sep 2024 18:47:52 +0000 Received: by mail-pl1-x634.google.com with SMTP id d9443c01a7336-206f9b872b2so12370045ad.3 for ; Thu, 26 Sep 2024 11:47:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tenstorrent.com; s=google; t=1727376470; x=1727981270; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=+KusUymBZTpYYnq4WmcbbE/FZsVAjrJbZNZbABf3CAo=; b=ANQnlyiK3mqF4HYlZfumLHsUyftJU0XwuEIWlPBFPYums4pPLVTYbB1qV92OSUIZBQ iniVJjPyt78lwgW1qzz1hQjzklk9KXIrBqyW3jN2/AVsc4AhXnxl5UCbHJTBwM+yJCDj 5kPmrYPqkK12gTaHMGQqDtt2JHgiqdoosTmMLX3nwJuCpQDhmD4312U3zEHjpLROQeun HHAvHFdfnG/arYSiehI2LBvJIce5r3AUPOUD0ZW+cVtM5XULniWumoPsHwOoLWmUohQG 7LyFRwCqTwLnuk4YR+2mGA5RolS8KbNjyNsTon/mR+ELqy/fjuEkVdCAs0j78hK2zynv TLyA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727376470; x=1727981270; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=+KusUymBZTpYYnq4WmcbbE/FZsVAjrJbZNZbABf3CAo=; b=eIuT4pjnuTvyHx0bIJqEdg+ak408JaKjVhudPiM2uUNDbHSP7XGTKLdvmVV+oDciMF Glls6XhUirbVPYqfliHJt0aXWa4i1cT/rrec708c7e8st0TlHeW3xRA02aNoe/8Q2+67 vIwT9cPxyM3I3E8oxI9mL8rDDm6quu8e1Q9HqQiyL4Up4Hpek9jnkq5U/+eND9pygsdz +4J8xh9TN8SP9Efn1ddLs32H/CyjDfWARm74/YtJk5NcA2hefarLi1MAKcrbv8ekn7Sp oyxADmLYWw3eOeBzoTubJTIXNJRaoVAFN6dq1npibtaKBaQnsbZB9oxTWVRjUwCmyQI4 DRqw== X-Forwarded-Encrypted: i=1; AJvYcCUyYMw88pMArivMVfMT790vQH/8p4Cq4m1CcVvhhTadEdeqk+hETEOhlUVkPh/xEwW9jCA2iE9MbaWm+Tv5zpd0@lists.infradead.org X-Gm-Message-State: AOJu0YzuHsnJaAVio2csHyayDdUuWfCy2bk6MDhyUOpgjDqLAiHJLgo4 S7y4zNxS7LvJBcLu37aSWpHgddPYtCuOPd+8BjfYeDiWufTZS51/zBvzLRKSPVA= X-Google-Smtp-Source: AGHT+IGNxuTgkN1leT2RdvlA5PUTH+t79/qJCqmKY/KS0CmrWCtetVvj/vYEI9+Bz4MIXVKD0WkcsQ== X-Received: by 2002:a17:903:2285:b0:207:7952:e6d4 with SMTP id d9443c01a7336-20b367ca162mr9258295ad.4.1727376469796; Thu, 26 Sep 2024 11:47:49 -0700 (PDT) Received: from x1 (71-34-69-82.ptld.qwest.net. [71.34.69.82]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-20b37e4ee7bsm1571565ad.234.2024.09.26.11.47.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 11:47:49 -0700 (PDT) Date: Thu, 26 Sep 2024 11:47:47 -0700 From: Drew Fustini To: Andrew Lunn Cc: "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Alexandre Torgue , Giuseppe Cavallaro , Jose Abreu , Jisheng Zhang , Maxime Coquelin , Emil Renner Berthing , Drew Fustini , Guo Ren , Fu Wei , Conor Dooley , Paul Walmsley , Palmer Dabbelt , Albert Ou , netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-riscv@lists.infradead.org Subject: Re: [PATCH v2 2/3] net: stmmac: Add glue layer for T-HEAD TH1520 SoC Message-ID: References: <20240926-th1520-dwmac-v2-0-f34f28ad1dc9@tenstorrent.com> <20240926-th1520-dwmac-v2-2-f34f28ad1dc9@tenstorrent.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240926_114750_867033_E60301B8 X-CRM114-Status: GOOD ( 25.60 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Sep 26, 2024 at 08:32:00PM +0200, Andrew Lunn wrote: > > +static int thead_dwmac_init(struct platform_device *pdev, void *priv) > > +{ > > + struct thead_dwmac *dwmac = priv; > > + int ret; > > + > > + ret = thead_dwmac_set_phy_if(dwmac->plat); > > + if (ret) > > + return ret; > > + > > + ret = thead_dwmac_set_txclk_dir(dwmac->plat); > > + if (ret) > > + return ret; > > + > > + ret = regmap_write(dwmac->apb_regmap, GMAC_RXCLK_DELAY_CTRL, > > + GMAC_RXCLK_DELAY_VAL(dwmac->rx_delay)); > > + if (ret) > > + return dev_err_probe(dwmac->dev, ret, > > + "failed to set GMAC RX clock delay\n"); > > + > > + ret = regmap_write(dwmac->apb_regmap, GMAC_TXCLK_DELAY_CTRL, > > + GMAC_TXCLK_DELAY_VAL(dwmac->tx_delay)); > > + if (ret) > > + return dev_err_probe(dwmac->dev, ret, > > + "failed to set GMAC TX clock delay\n"); > > + > > + thead_dwmac_fix_speed(dwmac, SPEED_1000, 0); > > Is this needed? I would expect this to be called when the PHY has link > and you know the link speed. So why set it here? Good point. I've removed this line and the probe still completes okay and the Ethernet connection is working okay. > > + > > + return thead_dwmac_enable_clk(dwmac->plat); > > +} > > + > > +static int thead_dwmac_probe(struct platform_device *pdev) > > +{ > > + struct device_node *np = pdev->dev.of_node; > > + struct stmmac_resources stmmac_res; > > + struct plat_stmmacenet_data *plat; > > + struct thead_dwmac *dwmac; > > + void __iomem *apb; > > + u32 delay; > > + int ret; > > + > > + ret = stmmac_get_platform_resources(pdev, &stmmac_res); > > + if (ret) > > + return dev_err_probe(&pdev->dev, ret, > > + "failed to get resources\n"); > > + > > + plat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac); > > + if (IS_ERR(plat)) > > + return dev_err_probe(&pdev->dev, PTR_ERR(plat), > > + "dt configuration failed\n"); > > + > > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL); > > + if (!dwmac) > > + return -ENOMEM; > > + > > + /* hardware default is 0 for the rx and tx internal clock delay */ > > + dwmac->rx_delay = 0; > > + dwmac->tx_delay = 0; > > + > > + /* rx and tx internal delay properties are optional */ > > + if (!of_property_read_u32(np, "thead,rx-internal-delay", &delay)) { > > + if (delay > GMAC_RXCLK_DELAY_MASK) > > + dev_warn(&pdev->dev, > > + "thead,rx-internal-delay (%u) exceeds max (%lu)\n", > > + delay, GMAC_RXCLK_DELAY_MASK); > > + else > > + dwmac->rx_delay = delay; > > + } > > + > > So you keep going, with an invalid value? It is better to use > dev_err() and return -EINVAL. The DT write will then correct their > error when the device fails to probe. My intention was to keep the default of 0 if the dt property exists and exceeds the max value. I had considered failing the probe but I wasn't sure that was too severe of a reaction to a bad value for the delay. > > If you decide to keep this... I'm not sure these properties are > needed. Given your reply to the cover letter, I think it does make sense for me to remove handling of these delay properties since the units of the delay bit field are unknown and the hardware I have is okay with the default delay. > > > +MODULE_AUTHOR("Jisheng Zhang "); > > Please add a second author, if you have taken over this driver. Yes, Jisheng is no longer working on it, so I will add myself. Thanks, Drew