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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 46B12C7EE2F for ; Sat, 27 May 2023 22:00:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229459AbjE0WAO (ORCPT ); Sat, 27 May 2023 18:00:14 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35644 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229437AbjE0WAL (ORCPT ); Sat, 27 May 2023 18:00:11 -0400 Received: from wnew3-smtp.messagingengine.com (wnew3-smtp.messagingengine.com [64.147.123.17]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CC668D9; Sat, 27 May 2023 15:00:09 -0700 (PDT) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailnew.west.internal (Postfix) with ESMTP id CE0682B069B3; Sat, 27 May 2023 18:00:04 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Sat, 27 May 2023 18:00:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=flygoat.com; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to; s=fm2; t= 1685224804; x=1685232004; bh=xOIzaS7TUXsjeL9bN87xBLkkvq7cSpZ1H0T BffNzGWA=; b=SelzHpDvDO2/Uc8sItLHkL+JRlOvhcW97Mg+l+n6jEx9v/4mEwo PPmre7YNjuzhzCDTSVbbTp1Fr5KrfvZCZY7gGk8lII5WCo0jeue7owokkpmI2hVh V7O05e1Dp1qdECUKkezUBFr24dSoJvqagmsm6jLNCyVRC6FlNBZei6rQtQVoHAC4 Xc1AgMC0OzI3/6AzZHr2DvyLAXOKDOdisz1RxdhLum2JEpsEnEf+Hyk8/VgVc8G4 yUlFMLUXrNQ79MQXiJj8EVOWCf8qDvQmcwVqx10qtllEQqsSAyBK3ij5L6eDZiII d2b0uTohwjcoVnJBDs7lB8y4jkljUNbwKxQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:sender:subject:subject:to:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t= 1685224804; x=1685232004; bh=xOIzaS7TUXsjeL9bN87xBLkkvq7cSpZ1H0T BffNzGWA=; b=LPAOotBQXGDhjkLWauny8SGUXXp8g/81IsNlqnMZV+7aKHnHpqg wp+FpodNaV2G66QTv8e4nTA64x06BhM23snxWMiVBFjwkGyLukjBlik6mcl1FrzW YQHUgM1TYw7mThwjL2lwEq3CY/CKWPDyhkZ+mVwbymwAuln5f5G7fSVlE3T8p1uN Qa04eNvZRvhXcKDz95xMjeuGNBEEBrCLUHeTNU2z/KFAJPK5GHtv3d0WRYkk4w/Z 6/H7cH9DY6eSDuHFbRQu+swjpKCAOO56qkaq9rXPdP8RUexffxdoQ22X9pvHncNf N1+8SGFv0ZnwRFyPbPFtSmRHK05PhMYYpKA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrfeekvddgtdehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurheptggguffhjgffvefgkfhfvffosehtqhhmtdhhtdejnecuhfhrohhmpeflihgr gihunhcujggrnhhguceojhhirgiguhhnrdihrghnghesfhhlhihgohgrthdrtghomheqne cuggftrfgrthhtvghrnhepuddtjeffteetfeekjeeiheefueeigeeutdevieejveeihfff ledvgfduiefhvddtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh hfrhhomhepjhhirgiguhhnrdihrghnghesfhhlhihgohgrthdrtghomh X-ME-Proxy: Feedback-ID: ifd894703:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sat, 27 May 2023 17:59:59 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\)) Subject: Re: [PATCH V4 1/5] dt-bindings: rtc: Remove the LS2X from the trivial RTCs From: Jiaxun Yang In-Reply-To: <20230527-passing-unfixed-39e01b787808@spud> Date: Sat, 27 May 2023 22:59:48 +0100 Cc: Binbin Zhou , Conor Dooley , Binbin Zhou , Alessandro Zummo , Alexandre Belloni , linux-rtc@vger.kernel.org, Rob Herring , Krzysztof Kozlowski , Conor Dooley , devicetree@vger.kernel.org, Huacai Chen , Huacai Chen , Xuerui Wang , loongarch@lists.linux.dev, Thomas Bogendoerfer , "linux-mips@vger.kernel.org" , Kelvin Cheung , zhao zhang , Yang Ling , loongson-kernel@lists.loongnix.cn Content-Transfer-Encoding: quoted-printable Message-Id: <14EF9F21-8150-40D9-8870-E9151C4882CF@flygoat.com> References: <9a2fbd6860f37760ca6089c150fd6f67628405f6.1684983279.git.zhoubinbin@loongson.cn> <20230525-custody-oversleep-f778eddf981c@spud> <20230526-dolly-reheat-06c4d5658415@wendy> <20230527-passing-unfixed-39e01b787808@spud> To: Conor Dooley X-Mailer: Apple Mail (2.3731.500.231) Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org > 2023=E5=B9=B45=E6=9C=8827=E6=97=A5 17:23=EF=BC=8CConor Dooley = =E5=86=99=E9=81=93=EF=BC=9A >=20 > On Sat, May 27, 2023 at 05:13:39PM +0100, Jiaxun Yang wrote: >>> 2023=E5=B9=B45=E6=9C=8827=E6=97=A5 10:22=EF=BC=8CBinbin Zhou = =E5=86=99=E9=81=93=EF=BC=9A >>> On Fri, May 26, 2023 at 8:07=E2=80=AFPM Conor Dooley = wrote: >>>> On Fri, May 26, 2023 at 09:37:02AM +0800, Binbin Zhou wrote: >>>>> On Fri, May 26, 2023 at 1:05=E2=80=AFAM Conor Dooley = wrote: >>>>>> On Thu, May 25, 2023 at 08:55:23PM +0800, Binbin Zhou wrote: >>>>=20 >>>>>>>> +properties: >>>>>>> + compatible: >>>>>>> + enum: >>>>>>> + - loongson,ls1b-rtc >>>>>>> + - loongson,ls1c-rtc >>>>>>> + - loongson,ls7a-rtc >>>>>>> + - loongson,ls2k0500-rtc >>>>>>> + - loongson,ls2k1000-rtc >>>>>>> + - loongson,ls2k2000-rtc >>>>>>=20 >>>>>> |+static const struct of_device_id loongson_rtc_of_match[] =3D { >>>>>> |+ { .compatible =3D "loongson,ls1b-rtc", .data =3D = &ls1x_rtc_config }, >>>>>> |+ { .compatible =3D "loongson,ls1c-rtc", .data =3D = &ls1x_rtc_config }, >>>>>> |+ { .compatible =3D "loongson,ls7a-rtc", .data =3D = &generic_rtc_config }, >>>>>> |+ { .compatible =3D "loongson,ls2k0500-rtc", .data =3D = &generic_rtc_config }, >>>>>> |+ { .compatible =3D "loongson,ls2k1000-rtc", .data =3D = &ls2k1000_rtc_config }, >>>>>> |+ { .compatible =3D "loongson,ls2k2000-rtc", .data =3D = &generic_rtc_config }, >>>>>> |+ { /* sentinel */ } >>>>>> |+}; >>>>>>=20 >>>>>> This is a sign to me that your compatibles here are could do with = some >>>>>> fallbacks. Both of the ls1 ones are compatible with each other & = there >>>>>> are three that are generic. >>>>>>=20 >>>>>> I would allow the following: >>>>>> "loongson,ls1b-rtc" >>>>>> "loongson,ls1c-rtc", "loongson,ls1b-rtc" >>>>>> "loongson,ls7a-rtc" >>>>>> "loongson,ls2k0500-rtc", "loongson,ls7a-rtc" >>>>>> "loongson,ls2k2000-rtc", "loongson,ls7a-rtc" >>>>>> "loongson,ls2k1000-rtc" >>>>>>=20 >>>>>> And then the driver only needs: >>>>>> |+static const struct of_device_id loongson_rtc_of_match[] =3D { >>>>>> |+ { .compatible =3D "loongson,ls1b-rtc", .data =3D = &ls1x_rtc_config }, >>>>>> |+ { .compatible =3D "loongson,ls7a-rtc", .data =3D = &generic_rtc_config }, >>>>>> |+ { .compatible =3D "loongson,ls2k1000-rtc", .data =3D = &ls2k1000_rtc_config }, >>>>>> |+ { /* sentinel */ } >>>>>> |+}; >>>>>>=20 >>>>>> And ~if~when you add support for more devices in the future that = are >>>>>> compatible with the existing ones no code changes are required. >>>>>=20 >>>>> Hi Conor: >>>>>=20 >>>>> Thanks for your reply. >>>>>=20 >>>>> Yes, this is looking much cleaner. But it can't show every chip = that >>>>> supports that driver. >>>>>=20 >>>>> As we know, Loongson is a family of chips: >>>>> ls1b/ls1c represent the Loongson-1 family of CPU chips; >>>>> ls7a represents the Loongson LS7A bridge chip; >>>>> ls2k0500/ls2k1000/ls2k2000 represent the Loongson-2 family of CPU = chips. >>>>>=20 >>>>> Based on my previous conversations with Krzysztof, it seems that >>>>> soc-based to order compatible is more popular, so I have listed = all >>>>> the chips that support that RTC driver. >>>>=20 >>>> Right. You don't actually have to list them all *in the driver* = though, >>>> just in the binding and in the devicetree. I think what you have = missed >>>> is: >>>>>> I would allow the following: >>>>>> "loongson,ls1b-rtc" >>>>>> "loongson,ls1c-rtc", "loongson,ls1b-rtc" >>>>>> "loongson,ls7a-rtc" >>>>>> "loongson,ls2k0500-rtc", "loongson,ls7a-rtc" >>>>>> "loongson,ls2k2000-rtc", "loongson,ls7a-rtc" >>>>>> "loongson,ls2k1000-rtc" >>>>=20 >>>> This is what you would put in the compatible section of a = devicetree >>>> node, using "fallback compatibles". So for a ls1c you put in >>>> compatible =3D "loongson,ls1c-rtc", "loongson,ls1b-rtc"; >>>> and the kernel first tries to find a driver that supports >>>> "loongson,ls1c-rtc" but if that fails it tries to find one that = supports >>>> "loongson,ls1b-rtc". This gives you the best of both worlds - you = can >>>> add support easily for new systems (when an ls1d comes out, you = don't >>>> even need to change the driver for it to just work!) and you have a >>>> soc-specific compatible in case you need to add some workaround for >>>> hardware errata etc in the future. >>>=20 >>> I seem to understand what you are talking about. >>> I hadn't delved into "fallback compatibles" before, so thanks for = the >>> detailed explanation. >>>=20 >>> In fact, I have thought before if there is a good way to do it other >>> than adding comptable to the driver frequently, and "fallback >>> compatibles" should be the most suitable. >>>=20 >>> So in the dt-bindings file, should we just write this: >=20 > Not quite, because you still need to allow for ls1b-rtc and ls7a-rtc > appearing on their own. That's just two more entries like the > ls2k1000-rtc one. >=20 >>>=20 >>> compatible: >>> oneOf: >>> - items: >>> - enum: >>> - loongson,ls1c-rtc >>> - const: loongson,ls1b-rtc >>> - items: >>> - enum: >>> - loongson,ls2k0500-rtc >>> - loongson,ls2k2000-rtc >>> - const: loongson,ls7a-rtc >=20 >>> - items: >>> - const: loongson,ls2k1000-rtc >=20 > This one is just "const:", you don't need "items: const:". > I didn't test this, but I figure it would be: > compatible: > oneOf: > - items: > - enum: > - loongson,ls1c-rtc > - const: loongson,ls1b-rtc > - items: > - enum: > - loongson,ls2k0500-rtc > - loongson,ls2k2000-rtc > - const: loongson,ls7a-rtc > - const: loongson,ls1b-rtc > - const: loongson,ls2k1000-rtc > - const: loongson,ls7a-rtc >=20 >> My recommendation is leaving compatible string as is. >=20 > "as is" meaning "as it is right now in Linus' tree", or "as it is in > this patch"? Ah sorry I meant in this patch. Since there won=E2=80=99t be any new ls1x chip that will boot Linux any = time soon (due to Loongson move away from MIPS but LoongArch32 is undefined for now), and rest compatible strings are wide enough to cover their family, I think = the present compatible strings in this patch describes hardware best. Thanks - Jiaxun >=20 > Cheers, > Conor.