From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from new1-smtp.messagingengine.com (new1-smtp.messagingengine.com [66.111.4.221]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 88AE23FCD for ; Thu, 9 Sep 2021 08:45:43 +0000 (UTC) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailnew.nyi.internal (Postfix) with ESMTP id 56097580AF7; Thu, 9 Sep 2021 04:45:42 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Thu, 09 Sep 2021 04:45:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm3; bh=fXIbsZdMOwnGpikZ6f1eRUW6Zp6 DapCjyxIfszyQnyE=; b=2OV64XDRwKpf6BMIUyOBlsL07FXgOmvHe0OG/Z6bjzz ajaLwvxnDBz3oV97VMioFXPeyksXWqn2Jg0hTp01SIH3CnPcUf07R1HwdXa89oc5 ke3l8sVQ+B4b9tiJnn7R1VCNTG3j+rD12uBmyhDwHRIqhKoiX6NFFD5S/UfuoEuG mIkvVGV7B/IzY4N3pBK87J//9B4kOy3RjLxgUyvYXRfksbpTCxtQnd0Q3AV8DkP6 ByZsfRKBFnPz80/ym5TTVik/cCX2BxMA0HWoLFvhdBsIqSrXQvp27txRk108uAaD nj2eXK4vt3GEwMpj+YN4bfV4DUclspx3eWm9DCOPe2w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=fXIbsZ dMOwnGpikZ6f1eRUW6Zp6DapCjyxIfszyQnyE=; b=KiupMEmaeBiTfsAPFuMTny +0eK63fG8iVe8wC0DAoXdnJuPE6HcZ+wlCcM3bIrgguLICEpP5AwZquoUlYbLuWs siLD6XiJ+lWPaxcK3LwnOCOCVnvuwo68wMD/fL9hiw9PpGel8fEacFW+eyqzqOf1 YNDyuZ+s8PfkbXOej/ylZ8Dj58j2F0NgXgIC50kz0sI+mn9wNsp+YeuAo1FWt6TR jW61JLo52b/soHUMCNG/hX+JKGlN07Bf2e14Q2/bwmQTpb4jg/6YXrO9yDtgzoLW ZY0OIv7Ze6pvLTpyB5TNZrpngihLluNbqhmSU1AY4J6MwtZkflncMom/MWtJC34A == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudefledgtdejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnhepleekgeehhfdutdeljefgleejffehfffgieejhffgueefhfdtveetgeehieeh gedunecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepmh grgihimhgvsegtvghrnhhordhtvggthh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 9 Sep 2021 04:45:40 -0400 (EDT) Date: Thu, 9 Sep 2021 10:45:38 +0200 From: Maxime Ripard To: Samuel Holland Cc: Chen-Yu Tsai , Jernej Skrabec , Rob Herring , Michael Turquette , Stephen Boyd , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU driver Message-ID: <20210909084538.jeqltc7b3rtqvu4h@gilmour> References: <20210901053951.60952-1-samuel@sholland.org> <20210903145013.hn6dv7lfyvfys374@gilmour> <4a187add-462b-dfe4-868a-fdab85258b8d@sholland.org> Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="auq6fgnyvsqqa77k" Content-Disposition: inline In-Reply-To: <4a187add-462b-dfe4-868a-fdab85258b8d@sholland.org> --auq6fgnyvsqqa77k Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 03, 2021 at 10:21:13AM -0500, Samuel Holland wrote: > On 9/3/21 9:50 AM, Maxime Ripard wrote: > > Hi, > >=20 > > On Wed, Sep 01, 2021 at 12:39:44AM -0500, Samuel Holland wrote: > >> This patch series adds a CCU driver for the RTC in the H616 and R329. > >> The extra patches at the end of this series show how it would be > >> explanded to additional hardware variants. > >> > >> The driver is intended to support the existing binding used for the H6, > >> but also an updated binding which includes all RTC input clocks. I do > >> not know how to best represent that binding -- that is a major reason > >> why this series is an RFC. > >> > >> A future patch series could add functionality to the driver to manage > >> IOSC calibration at boot and during suspend/resume. > >> > >> It may be possible to support all of these hardware variants in the > >> existing RTC clock driver and avoid some duplicate code, but I'm > >> concerned about the complexity there, without any of the CCU > >> abstraction. > >> > >> This series is currently based on top of the other series I just sent > >> (clk: sunxi-ng: Lifetime fixes and module support), but I can rebase it > >> elsewhere. > >=20 > > I'm generally ok with this, it makes sense to move it to sunxi-ng, > > especially with that other series of yours. > >=20 > > My main concern about this is the split driver approach. We used to have > > that before in the RTC too, but it was mostly due to the early clock > > requirements. With your previous work, that requirement is not there > > anymore and we can just register it as a device just like the other > > clock providers. >=20 > That's a good point. Originally, I had this RTC CCU providing osc24M, so > it did need to be an early provider. But with the current version, we > could have the RTC platform driver call devm_sunxi_ccu_probe. That does > seem cleaner. >=20 > (Since it wasn't immediately obvious to me why this works: the only > early provider remaining is the sun5i CCU, and it doesn't use the sun6i > RTC driver.) >=20 > > And since we can register all those clocks at device probe time, we > > don't really need to split the driver in two (and especially in two > > different places). The only obstacle to this after your previous series > > is that we don't have of_sunxi_ccu_probe / devm_sunxi_ccu_probe > > functions public, but that can easily be fixed by moving their > > definition to include/linux/clk/sunxi-ng.h >=20 > Where are you thinking the clock definitions would go? We don't export > any of those structures (ccu_mux, ccu_common) or macros > (SUNXI_CCU_GATE_DATA) in a public header either. Ah, right... > Would you want to export those? That seems like a lot of churn. Or would > we put the CCU descriptions in drivers/clk/sunxi-ng and export a > function that the RTC driver can call? (Or some other idea?) I guess we could export it. There's some fairly big headers in include/linux/clk already (tegra and ti), it's not uAPI and we do have reasons to do so, so I guess it's fine. I'd like to avoid having two drivers for the same device if possible, especially in two separate places. This creates some confusion since the general expectation is that there's only one driver per device. There's also the fact that this could lead to subtle bugs since the probe order is the link order (or module loading). And synchronizing access to registers between those two drivers will be hard, while we could just share the same spin lock between the RTC and clock drivers if they are instanciated in the same place. Maxime --auq6fgnyvsqqa77k Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYTnJsgAKCRDj7w1vZxhR xWKfAP9WolSmwsGNKd7TEH30FKvAYAxNAYNgceOQQI81F6ZUaAD/X64s6BKisuDB r08jQKq1pjleoCJbhAkLWl4nN9Wn7gU= =qRHE -----END PGP SIGNATURE----- --auq6fgnyvsqqa77k-- 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 X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 65A7CC433EF for ; Thu, 9 Sep 2021 08:47:52 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 2D2DF6113E for ; Thu, 9 Sep 2021 08:47:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 2D2DF6113E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=cerno.tech Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To: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=vLSHE/tOeJoSj1lSYIqRDyQFfbLyc7a28VwCfiyKc6g=; b=0uMA2li6NvI5j9/BUBfZPMQ/s8 /67SqKLI29MZIMKqqnZKu+dlcAFXhyBWBTW3/RPirx3rHjo21xmQY3mFQ+dQiGAK154yUNtckfhLy R7AShXN6qAV4A3GDy1aQgjjVzsNHR+CySOcn3B5CjdaVetdEjETzixrZSen3KLrsClYmOTAfqzlLt BsubM6vsCzZpYNmpVShIiCgWGqt4JE9bWVTeLUnLO/I+tQ5Y9ei2k/fT/8lcWdr1/jVOpUcwxvtzS dVXSm2OV796s4iwlmThPR+dtLAjk1563q8Otz9XfIp49xC34x3p4/pFAU+9IktUdVMygP5jYlr0lV J6gMqvGQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mOFgv-008bcT-Bi; Thu, 09 Sep 2021 08:45:53 +0000 Received: from new1-smtp.messagingengine.com ([66.111.4.221]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mOFgr-008bao-Jb for linux-arm-kernel@lists.infradead.org; Thu, 09 Sep 2021 08:45:51 +0000 Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailnew.nyi.internal (Postfix) with ESMTP id 56097580AF7; Thu, 9 Sep 2021 04:45:42 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Thu, 09 Sep 2021 04:45:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cerno.tech; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm3; bh=fXIbsZdMOwnGpikZ6f1eRUW6Zp6 DapCjyxIfszyQnyE=; b=2OV64XDRwKpf6BMIUyOBlsL07FXgOmvHe0OG/Z6bjzz ajaLwvxnDBz3oV97VMioFXPeyksXWqn2Jg0hTp01SIH3CnPcUf07R1HwdXa89oc5 ke3l8sVQ+B4b9tiJnn7R1VCNTG3j+rD12uBmyhDwHRIqhKoiX6NFFD5S/UfuoEuG mIkvVGV7B/IzY4N3pBK87J//9B4kOy3RjLxgUyvYXRfksbpTCxtQnd0Q3AV8DkP6 ByZsfRKBFnPz80/ym5TTVik/cCX2BxMA0HWoLFvhdBsIqSrXQvp27txRk108uAaD nj2eXK4vt3GEwMpj+YN4bfV4DUclspx3eWm9DCOPe2w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=fXIbsZ dMOwnGpikZ6f1eRUW6Zp6DapCjyxIfszyQnyE=; b=KiupMEmaeBiTfsAPFuMTny +0eK63fG8iVe8wC0DAoXdnJuPE6HcZ+wlCcM3bIrgguLICEpP5AwZquoUlYbLuWs siLD6XiJ+lWPaxcK3LwnOCOCVnvuwo68wMD/fL9hiw9PpGel8fEacFW+eyqzqOf1 YNDyuZ+s8PfkbXOej/ylZ8Dj58j2F0NgXgIC50kz0sI+mn9wNsp+YeuAo1FWt6TR jW61JLo52b/soHUMCNG/hX+JKGlN07Bf2e14Q2/bwmQTpb4jg/6YXrO9yDtgzoLW ZY0OIv7Ze6pvLTpyB5TNZrpngihLluNbqhmSU1AY4J6MwtZkflncMom/MWtJC34A == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudefledgtdejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvffukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpeforgigihhm vgcutfhiphgrrhguuceomhgrgihimhgvsegtvghrnhhordhtvggthheqnecuggftrfgrth htvghrnhepleekgeehhfdutdeljefgleejffehfffgieejhffgueefhfdtveetgeehieeh gedunecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepmh grgihimhgvsegtvghrnhhordhtvggthh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 9 Sep 2021 04:45:40 -0400 (EDT) Date: Thu, 9 Sep 2021 10:45:38 +0200 From: Maxime Ripard To: Samuel Holland Cc: Chen-Yu Tsai , Jernej Skrabec , Rob Herring , Michael Turquette , Stephen Boyd , devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU driver Message-ID: <20210909084538.jeqltc7b3rtqvu4h@gilmour> References: <20210901053951.60952-1-samuel@sholland.org> <20210903145013.hn6dv7lfyvfys374@gilmour> <4a187add-462b-dfe4-868a-fdab85258b8d@sholland.org> MIME-Version: 1.0 In-Reply-To: <4a187add-462b-dfe4-868a-fdab85258b8d@sholland.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210909_014549_770704_E9F06E76 X-CRM114-Status: GOOD ( 46.55 ) 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: , Content-Type: multipart/mixed; boundary="===============5486836577935964727==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============5486836577935964727== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="auq6fgnyvsqqa77k" Content-Disposition: inline --auq6fgnyvsqqa77k Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 03, 2021 at 10:21:13AM -0500, Samuel Holland wrote: > On 9/3/21 9:50 AM, Maxime Ripard wrote: > > Hi, > >=20 > > On Wed, Sep 01, 2021 at 12:39:44AM -0500, Samuel Holland wrote: > >> This patch series adds a CCU driver for the RTC in the H616 and R329. > >> The extra patches at the end of this series show how it would be > >> explanded to additional hardware variants. > >> > >> The driver is intended to support the existing binding used for the H6, > >> but also an updated binding which includes all RTC input clocks. I do > >> not know how to best represent that binding -- that is a major reason > >> why this series is an RFC. > >> > >> A future patch series could add functionality to the driver to manage > >> IOSC calibration at boot and during suspend/resume. > >> > >> It may be possible to support all of these hardware variants in the > >> existing RTC clock driver and avoid some duplicate code, but I'm > >> concerned about the complexity there, without any of the CCU > >> abstraction. > >> > >> This series is currently based on top of the other series I just sent > >> (clk: sunxi-ng: Lifetime fixes and module support), but I can rebase it > >> elsewhere. > >=20 > > I'm generally ok with this, it makes sense to move it to sunxi-ng, > > especially with that other series of yours. > >=20 > > My main concern about this is the split driver approach. We used to have > > that before in the RTC too, but it was mostly due to the early clock > > requirements. With your previous work, that requirement is not there > > anymore and we can just register it as a device just like the other > > clock providers. >=20 > That's a good point. Originally, I had this RTC CCU providing osc24M, so > it did need to be an early provider. But with the current version, we > could have the RTC platform driver call devm_sunxi_ccu_probe. That does > seem cleaner. >=20 > (Since it wasn't immediately obvious to me why this works: the only > early provider remaining is the sun5i CCU, and it doesn't use the sun6i > RTC driver.) >=20 > > And since we can register all those clocks at device probe time, we > > don't really need to split the driver in two (and especially in two > > different places). The only obstacle to this after your previous series > > is that we don't have of_sunxi_ccu_probe / devm_sunxi_ccu_probe > > functions public, but that can easily be fixed by moving their > > definition to include/linux/clk/sunxi-ng.h >=20 > Where are you thinking the clock definitions would go? We don't export > any of those structures (ccu_mux, ccu_common) or macros > (SUNXI_CCU_GATE_DATA) in a public header either. Ah, right... > Would you want to export those? That seems like a lot of churn. Or would > we put the CCU descriptions in drivers/clk/sunxi-ng and export a > function that the RTC driver can call? (Or some other idea?) I guess we could export it. There's some fairly big headers in include/linux/clk already (tegra and ti), it's not uAPI and we do have reasons to do so, so I guess it's fine. I'd like to avoid having two drivers for the same device if possible, especially in two separate places. This creates some confusion since the general expectation is that there's only one driver per device. There's also the fact that this could lead to subtle bugs since the probe order is the link order (or module loading). And synchronizing access to registers between those two drivers will be hard, while we could just share the same spin lock between the RTC and clock drivers if they are instanciated in the same place. Maxime --auq6fgnyvsqqa77k Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYTnJsgAKCRDj7w1vZxhR xWKfAP9WolSmwsGNKd7TEH30FKvAYAxNAYNgceOQQI81F6ZUaAD/X64s6BKisuDB r08jQKq1pjleoCJbhAkLWl4nN9Wn7gU= =qRHE -----END PGP SIGNATURE----- --auq6fgnyvsqqa77k-- --===============5486836577935964727== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============5486836577935964727==--