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=-9.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 C2212C4338F for ; Mon, 26 Jul 2021 15:02:26 +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 8BC2B60F02 for ; Mon, 26 Jul 2021 15:02:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 8BC2B60F02 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=8XOiQ9MX/CGmHf1FKryXQvPVsIh7arIKPAKj9TGEvCU=; b=JaLM9/U/h5sD/xfD7Xae4qB5kD EganVx6mu4hLdhpwLDHkPzUeKIjyuOdXSGtxU5OtZ12XjK07DmZvyNrkAofywf/rTavi35r/BsYkI 6zldKhwkOMpucoPjn/h4shZ4xSOK2o05kLPtFpGXTcZpPV77Lk7Zs/QTI/+vFaRZ8sdul6QvbijCC FMOY7207LPUZf0lytuOk1h4OFxgswoOF0Lq1bNEeXO3nTvsye5hE/3StkQ70D1FcHyYvTTBt2ejKG oiLIO2lCN/8R2KN9KhOxdXLaOxPv3tNqmk1k0sqt7uXfKex8jOoKnTFhdZjAGcbAyURvxISp4mbAp 65Ps02rA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1m824y-00BfiV-0L; Mon, 26 Jul 2021 14:59:40 +0000 Received: from new4-smtp.messagingengine.com ([66.111.4.230]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1m824q-00BfUl-7v for linux-arm-kernel@lists.infradead.org; Mon, 26 Jul 2021 14:59:37 +0000 Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailnew.nyi.internal (Postfix) with ESMTP id 0F50E58041D; Mon, 26 Jul 2021 10:59:28 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Mon, 26 Jul 2021 10:59:28 -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=aafEayI1sVZWEQ+8Kldbq4Mehnl cqhooAS5lRNnCO7Q=; b=1xZtgOsNPwxRhKdemCXg5/P1UQ/TvHM4SDl8L/Wrepj uxm931dVSx7NZlCh5RaWw38eIDlyO+8DFnlyMMX7JNr0tM81i2kTMCUl8WpnodEI Crq/XQ8W8++H+nGxC/8Wy4ZsmoEXh7kNZVFKAOzZxFCDIrQa2ADfQzjag3RG/foi JLHNpezIBnAW9IYnySDRs2+i/5ItKePmc1l8of6gQRazAhWtKXjupKTAEyupanGJ B5ZGHbz5I4pbeFWBT4e9jv3JWk78QXmNAq/J83rC26g/6YC8QZjEABUqrun0zEnK 7M87gSb/vgKBJgRGMoQKkfrkwIH+TIVkp2Z9St0uJPQ== 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=aafEay I1sVZWEQ+8Kldbq4MehnlcqhooAS5lRNnCO7Q=; b=IDplYQoIDpe3BvYBGdCCGE ISvOzLFz4DkiFBdTRDWWDBFNzgh5CLpcQU4g6faWBjdPUWBApjP3wlIveSEqYACa oYsyrZXmbJjJGL864XGnfH8I7iBKk6AHkceKcTz/to6zJnCJ3W+fFwVznBm2cGJC T3fSBfryP7hSaY9eeIHYXOMSM40tD5PUAoznE0yPKXvT79NcOVFVOLZDYn7rSXZl l/eq5V9lVWe4kt6AQgjZD2E7w7UJCKfhUJ60AanLHqCVAKT9dn0JFA8ZNh6Dckm2 vi4jXw9L3lAfgZObV3Xu7JETGAp1uOZ/K/2CTf/1k01VQ3d7k8fQYnubzzJMQSBA == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrgeehgdekfecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomhepofgrgihimhgv ucftihhprghrugcuoehmrgigihhmvgestggvrhhnohdrthgvtghhqeenucggtffrrghtth gvrhhnpeelkeeghefhuddtleejgfeljeffheffgfeijefhgfeufefhtdevteegheeiheeg udenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehmrg igihhmvgestggvrhhnohdrthgvtghh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 26 Jul 2021 10:59:26 -0400 (EDT) Date: Mon, 26 Jul 2021 16:59:23 +0200 From: Maxime Ripard To: Andre Przywara Cc: Chen-Yu Tsai , Jernej Skrabec , Rob Herring , Icenowy Zheng , Samuel Holland , linux-arm-kernel@lists.infradead.org, linux-sunxi@googlegroups.com, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org, Ondrej Jirman , Alessandro Zummo , Alexandre Belloni , linux-rtc@vger.kernel.org Subject: Re: [PATCH v7 06/19] rtc: sun6i: Add support for RTCs without external LOSCs Message-ID: <20210726145923.mdk6llzqppczkv5v@gilmour> References: <20210615110636.23403-1-andre.przywara@arm.com> <20210615110636.23403-7-andre.przywara@arm.com> <20210616091431.6tm3zdf77p2x3upc@gilmour> <20210723001721.0bb02cf2@slackpad.fritz.box> MIME-Version: 1.0 In-Reply-To: <20210723001721.0bb02cf2@slackpad.fritz.box> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210726_075932_691059_F998B493 X-CRM114-Status: GOOD ( 42.57 ) 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="===============0835922217848071226==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============0835922217848071226== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="lfxsk3mfqv3tm5fy" Content-Disposition: inline --lfxsk3mfqv3tm5fy Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 23, 2021 at 12:17:21AM +0100, Andre Przywara wrote: > On Wed, 16 Jun 2021 11:14:31 +0200 > Maxime Ripard wrote: >=20 > Hi Maxime, >=20 > > On Tue, Jun 15, 2021 at 12:06:23PM +0100, Andre Przywara wrote: > > > Some newer Allwinner RTCs (for instance the one in the H616 SoC) lack > > > a pin for an external 32768 Hz oscillator. As a consequence, this LOSC > > > can't be selected as the RTC clock source, and we must rely on the > > > internal RC oscillator. > > > To allow additions of clocks to the RTC node, add a feature bit to ig= nore > > > any provided clocks for now (the current code would think this is the > > > external LOSC). Later DTs and code can then for instance add the PLL > > > based clock input, and older kernel won't get confused. > > >=20 > > > Signed-off-by: Andre Przywara =20 > >=20 > > Honestly, I don't really know if it's worth it at this point. > >=20 > > If we sums this up: > >=20 > > - The RTC has 2 features that we use, mostly centered around 2 > > registers set plus a global one > >=20 > > - Those 2 features are programmed in a completely different way > >=20 > > - Even the common part is different, given the discussion around the > > clocks that we have. > >=20 > > What is there to share in that driver aside from the probe, and maybe > > the interrupt handling? Instead of complicating this further with more > > special case that you were (rightfully) complaining about, shouldn't we > > just acknowledge the fact that it's a completely separate design and > > should be treated as such, with a completely separate driver? >=20 > So I had a look, and I don't think it justifies a separate driver: > - Indeed it looks like the core functionality is different, but there > are a lot of commonalities, with all the RTC and driver boilerplate, > register offsets, and also the special access pattern (rtc_wait and > rtc_setaie). > - The actual difference is really in the way the *date* is stored > (the time is still in 24h H/M/S format), and the missing LOSC input > clock - which is already optional for existing devices. The two > patches just make this obvious, by using if() statements at the parts > where they differ. My point was that the code that is shared, like the driver boilerplate, is much more complicated than it can be precisely because it's shared. I'd take two simple-but-with-some-redundancy drivers over one big, shared but complicated driver any day. But fine, I guess. > So we would end up with possibly some shared .c file, and two driver > front-end files, which I am not sure is really worth it. >=20 > Next I thought about providing separate rtc_class_ops, but even they > share a lot of code, so they would be possibly be calling a shared > function each. I don't think that is really better. >=20 > If you dislike the rather large if/else branches in the previous two > patches, I could move that out into separate functions, but I feel this > is more code, for no real benefit. >=20 > So for now I am tempted to keep it shared. I think Samuel had ideas for > bigger changes in the clock part, at which point we could revisit this > decision - for instance keep the RTC part (still quite similar) mostly > in a shared file, while modelling the clocks in separate files - in a > more "common clock" style for the new SoCs. What's the plan? Maxime --lfxsk3mfqv3tm5fy Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCYP7NywAKCRDj7w1vZxhR xRY+AP41nD/WBdA9hAMZ6UcQge286Jku1Q+P63JZGaIwFd1zPgD7BOEoO8cWt8Fg ep6EHpxzz7yzcEIouyu2MVJ5FNzHMQI= =nHk2 -----END PGP SIGNATURE----- --lfxsk3mfqv3tm5fy-- --===============0835922217848071226== 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 --===============0835922217848071226==--