Received: by 2002:a05:6358:3188:b0:123:57c1:9b43 with SMTP id q8csp4252821rwd; Tue, 30 May 2023 02:43:06 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ53C2JuNDPOnFWTpta7J55/3LpzkyDQVcche0/79q3400ypqcxcvdzz5XnVdQ3ZFvPCw8Bg X-Received: by 2002:a17:902:e744:b0:1ac:86b5:70d9 with SMTP id p4-20020a170902e74400b001ac86b570d9mr10933749plf.32.1685439785947; Tue, 30 May 2023 02:43:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1685439785; cv=none; d=google.com; s=arc-20160816; b=ae7nrKvsoRzNu8qCnld971Mo2EZ/uANqsdNLO9F4ElO7/pR2PEZhzjCWNY6CeA9yLP XZ2C1aoQxMKLatkA5Nsu/Zu/n8pzMXCS6NpIS6T39dCdIe03o7eMPAGxostOKuBP5uM9 +1tY7EzLgRGzFvNiktn/XRKZd4tUo5a1251wu9/x3DqJyQ9MtxGWL3ACUpunZUChrmHm sWXVY/O2e4BufE7iCptznv3VpQ/Y/OOOEIuFvgdbECDgzosDQ/I/8QK94dDMjs1BWtwT RlCyzoJuGJ1ZT9gRa8prmn/OKmIFWHE40z4zZ73mXNRVvVWciOQjTowpwgv47oboZ0p6 bWEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=wp2pAf3CqrBt5skpkT7zvWMDCqetgcUZchDB3RC0Ee0=; b=zXeDhNF5NC1yJH/oD0dKvoCZ9r4gNcuS4UWj8iOQ4EACuA9XpXvUDtlpWdViGdDCVs ytSZXNfzVcP/3VhpyYzxgGkExBxNLL9P5pnxVIl9XJPu/NsIpLRmNaB3aifCxyPbkWUI tzUmKYaLYTEFYuhz9b3zuJQgsZfoXAMNYHDJOmwgVkPsd3PMRBCQVDWDQ+oHwtyJASgq tc+ldlJU7eFSEukkaaJHdoHamfK9xbhYvLkN6hCpureNJUdWMQlfE6E3j0QwiqDhLhy/ SoGED8MWqGBM1NXl5R/T+pvzrJx/jG2EwhRfR+bUxgsUN9Q4DqXBL6m/qoPoR1FZS36f Pf8A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@microchip.com header.s=mchp header.b=kXIirub0; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=microchip.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id p14-20020a170902b08e00b001b02a9bb4casi3761300plr.443.2023.05.30.02.42.53; Tue, 30 May 2023 02:43:05 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@microchip.com header.s=mchp header.b=kXIirub0; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=microchip.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229564AbjE3Jei (ORCPT + 99 others); Tue, 30 May 2023 05:34:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49124 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229475AbjE3Jef (ORCPT ); Tue, 30 May 2023 05:34:35 -0400 Received: from esa.microchip.iphmx.com (esa.microchip.iphmx.com [68.232.153.233]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A9C0B93; Tue, 30 May 2023 02:34:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1685439274; x=1716975274; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=wp2pAf3CqrBt5skpkT7zvWMDCqetgcUZchDB3RC0Ee0=; b=kXIirub0TyEkQRY6Ysquy7FBA3PGu3ATzksW1IY/Jz8Df2EbGHBsOXoC CBB5HVIm/pkx9lz+I6nvndUMyBrEHG//323opXgmWHrvDXGEsaJaLWIOR 55f9sADJdIn4tLB3glUx4umCXPIe36G5elhy9sG6czl7dwb3VZk2quFeS YbYvdvk4ae/df60WUu0PeuKfh/mX9D7h8Aef4XMUhlBwYEkXR4UB4YSwf a8fCiQN3YxfXzPoDSCn28eo+UEsPaA+/LufMW5L4AEyZ9xss8Zell3Obb tYtTmDg2Fz4wUlkeYgUpgkoXb1aLDvNCyrIaNkT3yTHJvg4AQ8hUoJh7u A==; X-IronPort-AV: E=Sophos;i="6.00,203,1681196400"; d="asc'?scan'208";a="216005891" X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN Received: from unknown (HELO email.microchip.com) ([170.129.1.10]) by esa5.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 30 May 2023 02:34:34 -0700 Received: from chn-vm-ex04.mchp-main.com (10.10.85.152) by chn-vm-ex01.mchp-main.com (10.10.85.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Tue, 30 May 2023 02:34:32 -0700 Received: from wendy (10.10.115.15) by chn-vm-ex04.mchp-main.com (10.10.85.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21 via Frontend Transport; Tue, 30 May 2023 02:34:29 -0700 Date: Tue, 30 May 2023 10:34:07 +0100 From: Conor Dooley To: Martin Blumenstingl CC: Dmitry Rokosov , , , , , , , , , , , , , , , Subject: Re: [PATCH v15 5/6] dt-bindings: clock: meson: add A1 Peripherals clock controller bindings Message-ID: <20230530-illusive-pushpin-1e35d0a50e0d@wendy> References: <20230517133309.9874-1-ddrokosov@sberdevices.ru> <20230517133309.9874-6-ddrokosov@sberdevices.ru> <20230522130033.a47vlybocme66rev@CAB-WSD-L081021> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="36vkeFCCRp4Cr9uu" Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, SPF_HELO_PASS,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --36vkeFCCRp4Cr9uu Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Yo, On Mon, May 29, 2023 at 10:38:33PM +0200, Martin Blumenstingl wrote: > On Mon, May 22, 2023 at 3:00=E2=80=AFPM Dmitry Rokosov wrote: > [...] > > > This IP block has at least one additional input called "sys_pll_div16= ". > > > My understanding is that the "sys_pll_div16" clock is generated by the > > > CPU clock controller. Support for the CPU clock controller > > > (dt-bindings and a driver) will be added at a later time by Dmitry. > > > How can we manage incrementally implementing the clock controllers? > > > From a hardware perspective the "sys_pll_div16" input is mandatory. > > > How to manage this in the .dts patches then (for example: does this > > > mean that Dmitry can only add the clock controller to the .dts when > > > all clock controller bindings have been implemented - or is there > > > another way)? > > > > You're absolutely right: currently, not all inputs are supported because > > the CPU clock controller isn't ready yet =E2=80=93 I'm working on it at= the > > moment. > > > > I understand your concerns about bindings and schema description, but > > there is an issue to be considered. I'm developing the entire clock > > controller A1 subsystem incrementally in three stages: peripherals and > > PLL, CPU, and Audio. This is because the CPU can operate at a static > > frequency and voltage, and the board boots normally without the CPU > > clock controller, thermal sensor, and OPP table. Audio is also > > important, but it's optional. On the other hand, without setting up the > > peripherals and PLL controllers, the board won't function because > > they're fundamental. > I understand your approach and I like it (without that incremental > approach you would probably be looking at a series with 15-20 > patches). >=20 > Maybe the dt-binding maintainers have a suggestion for us here? > Let me try to summarize the issue in a few bullet points: > - There's (at least) four clock controllers on the Amlogic A1 SoC > - Some of these clock controllers take the outputs of another clock > controller as inputs > - In this series patch the peripheral clock controller has an input > called "sys_pll_div16" > - The clock controller which provides the "sys_pll_div16" clock is not > implemented yet (my understanding is that implementing it and adding > it to this series is not easy: it would add even more patches that > need to be reviewed and in general it's a tricky clock controller to > implement as it manages the CPU clocks) If I am understanding correctly, this series implements the child controller and a parent, which is unimplemented, provides the child with sys_pll_div16. The thing I am missing is whether the child controller has some outputs that depend on this sys_pll_div16 input & whether those are documented in this series. Regardless, you should be able to add more output clocks without compatibility issues. > > Right now, we're in the first stage of the plan. Unfortunately, I can't > > disclose the exact names and number of clock bindings for the CPU and > > Audio, as they're still in development and only exist in my head or > > draft versions. > > > > If possible, I'd prefer to provide the new bindings and connections once > > all the appropriate drivers are finalized. > Question to Conor and Krzysztof (assuming you read my summary above): > Is it fine that Dmitry adds additional inputs to the peripheral clock > controller binding in later patches? Perhaps Krzysztof will disagree with me, but my take on it would be that the binding should describe the individual clock controller in its totality, but the driver can choose to only implement a subset of it. If you define the binding as only needing N inputs, but then later expand it to having N+M inputs, the driver will have to support N & N+M input clocks to preserve compatibility. If you define it as needing N+M inputs from the beginning, but only use N, there is no issue with backwards compatibility when you later use them all. > If not: how can we proceed in case we need to add them now (the > dt-binding example is the easy part for me as we can just make up a > phandle like &sys_pll_div16_clk and use that - but this can't work > when Dmitry tries to add the clock controller to meson-a1.dtsi) I would be inclined to do the same thing in the dts as the example, and make up a fixed-frequency clock and use it to plug the hole. When you have bindings etc written for the clock controller providing that clock, the fixed-frequency clock could be swapped out for the real one. > PS: Dmitry is trying to get this series into Linux 6.5. As far as I > remember the common clock maintainers don't take pull requests with > new features after -rc6 (which is in less than two weeks). > So time is getting a bit short and for me this is the very last > outstanding question. If you say that it's fine to add clocks later on > this will immediately get my Reviewed-by. I *think* that what I've just said should not get in the way of such a timeline, as it would only involve a "small" change to the dt-binding, but not require additional bindings or driver. Cheers, Conor. --36vkeFCCRp4Cr9uu Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZHXDDgAKCRB4tDGHoIJi 0tPiAQDxJgM01v9LEi2iF1dr7RKotniwIpWYNsLXwvRueTh4JAEAz4CqON+2GW8z dmSUkX/YHEFtQPiocrOvOz8I74RHVg0= =ipL3 -----END PGP SIGNATURE----- --36vkeFCCRp4Cr9uu--