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 48C98C4332F for ; Thu, 8 Dec 2022 21:01:00 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 79EFF850DB; Thu, 8 Dec 2022 22:00:58 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=tinet.cat Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 2F8E685062; Thu, 8 Dec 2022 22:00:57 +0100 (CET) Received: from mx1.tinet.cat (mx1.tinet.cat [195.77.216.146]) (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 AE0958512E for ; Thu, 8 Dec 2022 22:00:52 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=tinet.cat Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xdrudis@tinet.cat X-ASG-Debug-ID: 1670533250-163e7b330d516990001-4l7tJC Received: from smtp01.tinet.cat (smtp.tinet.org [195.77.216.131]) by mx1.tinet.cat with ESMTP id xeAEpi1OB03hPmgA; Thu, 08 Dec 2022 22:00:50 +0100 (CET) X-Barracuda-Envelope-From: xdrudis@tinet.cat X-Barracuda-Effective-Source-IP: smtp.tinet.org[195.77.216.131] X-Barracuda-Apparent-Source-IP: 195.77.216.131 Received: from localhost (190.red-79-152-181.dynamicip.rima-tde.net [79.152.181.190]) by smtp01.tinet.cat (Postfix) with ESMTPSA id A2722605DE41; Thu, 8 Dec 2022 22:00:50 +0100 (CET) Date: Thu, 8 Dec 2022 22:00:49 +0100 From: Xavier Drudis Ferran To: Marek Vasut Cc: Xavier Drudis Ferran , u-boot@lists.denx.de, Simon Glass , Philipp Tomsich , Kever Yang , Lukasz Majewski , Sean Anderson Subject: Re: [PATCH v3] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2. Message-ID: X-ASG-Orig-Subj: Re: [PATCH v3] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2. References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Barracuda-Connect: smtp.tinet.org[195.77.216.131] X-Barracuda-Start-Time: 1670533250 X-Barracuda-URL: https://webmail.tinet.cat:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 1888 X-Barracuda-BRTS-Status: 1 X-Barracuda-Bayes: SPAM GLOBAL 0.9180 1.0000 3.4197 X-Barracuda-Spam-Score: 3.42 X-Barracuda-Spam-Status: No, SCORE=3.42 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=6.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.102697 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 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.6 at phobos.denx.de X-Virus-Status: Clean El Thu, Dec 08, 2022 at 09:12:08PM +0100, Marek Vasut deia: > On 12/8/22 17:53, Xavier Drudis Ferran wrote: > > [...] > > > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > > @@ -7,7 +7,7 @@ > > */ > > #include > > -#include > > +#include > > #include > > #include > > #include > > @@ -168,6 +168,9 @@ static struct phy_ops rockchip_usb2phy_ops = { > > .of_xlate = rockchip_usb2phy_of_xlate, > > }; > > +static struct clk_ops rockchip_usb2phy_clk_ops = { > > +}; > > Is this empty structure needed ? Why ? > > Either it shouldn't be here, or it should implement some callbacks, like > clock enable/disable ? > I tried without it but it gave me a runtime error. I think I have the log somewhere if you want to see it. It looked like a null pointer dereference at first sight. I just added it and it got fixed. I didn't research what the failing functions were trying to do. Or is this a common case and there's some null_clk_ops() function or macro magic or something somewhere ? The thing is nobody is using this clk in u-boot. It's just its phandle in a clock phandle list that ehci-generic.c happens to use in bulk. So the default implementation seems to be enough to allocate, enable and release it in bulk with its clk set. Or at least to call the functions without error. As I have left it, it might not work if ever someone wants to use it. But should I try to implement it so that it is usable ? How should I test it ? Shouldn't we wait until someone has some real use for it ? Linux spent sometime without it in the phandle list until William wu discovered it was needed for suspend/resume, so u-boot may never need it. Or should I add a comment in the code ? > Otherwise looks much better, thanks ! Thanks.