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 40FEBC4332F for ; Wed, 23 Nov 2022 13:36:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238758AbiKWNgZ (ORCPT ); Wed, 23 Nov 2022 08:36:25 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50038 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237917AbiKWNgG (ORCPT ); Wed, 23 Nov 2022 08:36:06 -0500 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C4E20BF72 for ; Wed, 23 Nov 2022 05:23:49 -0800 (PST) Received: by mail-wr1-x42e.google.com with SMTP id e11so16457730wru.8 for ; Wed, 23 Nov 2022 05:23:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:in-reply-to:organization:references:cc:to :content-language:subject:reply-to:from:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=TkiWRuYD2knqqdxwJxwHHjuebCRQBiEK9jcJD+g0pcM=; b=T9svSE0oUJMN5OvhyI0f/M31vDqpQ+NUh5/oWXHGxorTb8dzZzZ0x4eXE498Tkkw/W abjsnsCS07Wczaavcy/hMSQEtEyjSiz93fk/+7CXBp+hlXGEBgMrbpKmkoZC8/lvWsPK 8t1tud5Oth6y/8qZuLNul5+oKiYS74/z58c4SYvU0XkjGNVLrro4uSsqooH9J3O8B17v 3pfC254xIKae2OMEee8E5P1L/cZpwi3h1XsHMgwshJw0V1jWPai0gzx3t50QnepgeLxu ZRYiZO3nhdjtbV1vpIMEiyHHD1l/C/laC+gUKz7xYzCf4UsJZQfoTV7DYMvU6FY/kx4J h1XQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:organization:references:cc:to :content-language:subject:reply-to:from:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=TkiWRuYD2knqqdxwJxwHHjuebCRQBiEK9jcJD+g0pcM=; b=eS/amhq22EfZseavJkojM4czk4z2YBgoBMZ8Y5H7+gENS892k3PJTziQb/I27iQRm7 fRUg4fq9dAzP3F1qQqfqtwEtnq+JwmWpGo/1XWmhLd8rXvw1YM/pjpNm8cn9XEnzMNhH nw3zaRI7cDzg6LYMtZSYdFfM9PiBqambGpNd55f0hXtls433ZNrFStoK4sRzNZ0OAYjC SZjl3Pj6lI0g+eHgyX0uaXvVk0fn6FPW82t8P6j/zQuLA02kF4obHogqGMGUDYEHLPAF wga2aWSMd2mZDJ/5IgM1eF7AGAkf20HYGYIaGlYTf8AfyAuIfqCxx66KrBvN6HxTBQ2q p4zA== X-Gm-Message-State: ANoB5pmaLf3dutDFTIex3sGmeu/Kh9/uweVxOfBq7/4mKNjlY1AAZF7O zQJ14dyhbN0VdnhGUn2NbGH0Vw== X-Google-Smtp-Source: AA0mqf5RrnqX/NxyNHmTNkJKTpMhPMIOxJ1w2SapdUDQw0M+Nb66CgbKIgCfLWeRWjW8+FJ4WRPgnQ== X-Received: by 2002:a5d:69d2:0:b0:241:ef59:e41f with SMTP id s18-20020a5d69d2000000b00241ef59e41fmr1455431wrw.486.1669209828269; Wed, 23 Nov 2022 05:23:48 -0800 (PST) Received: from ?IPV6:2a01:e0a:982:cbb0:e551:24c3:152c:7c05? ([2a01:e0a:982:cbb0:e551:24c3:152c:7c05]) by smtp.gmail.com with ESMTPSA id c1-20020adffb01000000b0023657e1b980sm16452329wrr.53.2022.11.23.05.23.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 23 Nov 2022 05:23:47 -0800 (PST) Message-ID: Date: Wed, 23 Nov 2022 14:23:46 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.2 From: Neil Armstrong Reply-To: neil.armstrong@linaro.org Subject: Re: [PATCH V5 1/4] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver and bindings Content-Language: en-US To: Yu Tu , Krzysztof Kozlowski , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Rob Herring , Jerome Brunet , Kevin Hilman , Michael Turquette , Stephen Boyd , Krzysztof Kozlowski , Martin Blumenstingl Cc: kelvin.zhang@amlogic.com References: <20221123021346.18136-1-yu.tu@amlogic.com> <20221123021346.18136-2-yu.tu@amlogic.com> <92b570ea-3ddc-8e91-5a7a-ed601bb7c02c@amlogic.com> Organization: Linaro Developer Services In-Reply-To: <92b570ea-3ddc-8e91-5a7a-ed601bb7c02c@amlogic.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-clk@vger.kernel.org Hi, On 23/11/2022 12:16, Yu Tu wrote: > Hi Krzysztof, >     Thank you for your reply. > > On 2022/11/23 18:08, Krzysztof Kozlowski wrote: >> [ EXTERNAL EMAIL ] >> >> On 23/11/2022 03:13, Yu Tu wrote: >>> Add the S4 PLL clock controller found and bindings in the s4 SoC family. >>> >>> Signed-off-by: Yu Tu >>> --- >>>   .../bindings/clock/amlogic,s4-pll-clkc.yaml   |  51 + >> >> This is v5 and still bindings are here? Bindings are always separate >> patches. Use subject prefixes matching the subsystem (git log --oneline >> -- ...). >> >> And this was split, wasn't it? What happened here?!? > > Put bindings and clock driver patch together from Jerome. Maybe you can read this chat history. > https://lore.kernel.or/all/1jy1v6z14n.fsf@starbuckisacylon.baylibre.com/ Jerome was asking you to send 2 patchsets, one with : - bindings in separate patches - drivers in separate patches and a second with DT changes. Then when the bindings + clocks patches are merged, a pull request of the bindings can be done to me so I can merge it with DT. > >> >> >>>   MAINTAINERS                                   |   1 + >>>   drivers/clk/meson/Kconfig                     |  13 + >>>   drivers/clk/meson/Makefile                    |   1 + >>>   drivers/clk/meson/s4-pll.c                    | 875 ++++++++++++++++++ >>>   drivers/clk/meson/s4-pll.h                    |  88 ++ >>>   .../dt-bindings/clock/amlogic,s4-pll-clkc.h   |  30 + >>>   7 files changed, 1059 insertions(+) >>>   create mode 100644 Documentation/devicetree/bindings/clock/amlogic,s4-pll-clkc.yaml >>>   create mode 100644 drivers/clk/meson/s4-pll.c >>>   create mode 100644 drivers/clk/meson/s4-pll.h >>>   create mode 100644 include/dt-bindings/clock/amlogic,s4-pll-clkc.h >>> >>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,s4-pll-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,s4-pll-clkc.yaml >>> new file mode 100644 >>> index 000000000000..fd517e8ef14f >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/clock/amlogic,s4-pll-clkc.yaml >>> @@ -0,0 +1,51 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/clock/amlogic,s4-pll-clkc.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Amlogic Meson S serials PLL Clock Controller >>> + >>> +maintainers: >>> +  - Neil Armstrong >>> +  - Jerome Brunet >>> +  - Yu Tu >>> + >> One blank line. > >  I will delete this, on next version patch. > >> >>> + >>> +properties: >>> +  compatible: >>> +    const: amlogic,s4-pll-clkc >>> + >>> +  reg: >>> +    maxItems: 1 >>> + >>> +  clocks: >>> +    maxItems: 1 >>> + >>> +  clock-names: >>> +    items: >>> +      - const: xtal >>> + >>> +  "#clock-cells": >>> +    const: 1 >>> + >>> +required: >>> +  - compatible >>> +  - reg >>> +  - clocks >>> +  - clock-names >>> +  - "#clock-cells" >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> +  - | >>> +    clkc_pll: clock-controller@fe008000 { >>> +      compatible = "amlogic,s4-pll-clkc"; >>> +      reg = <0xfe008000 0x1e8>; >>> +      clocks = <&xtal>; >>> +      clock-names = "xtal"; >>> +      #clock-cells = <1>; >>> +    }; >> >> >>> +#endif /* __MESON_S4_PLL_H__ */ >>> diff --git a/include/dt-bindings/clock/amlogic,s4-pll-clkc.h b/include/dt-bindings/clock/amlogic,s4-pll-clkc.h >>> new file mode 100644 >>> index 000000000000..345f87023886 >>> --- /dev/null >>> +++ b/include/dt-bindings/clock/amlogic,s4-pll-clkc.h >> >> This belongs to bindings patch, not driver. >> >>> @@ -0,0 +1,30 @@ >>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ >>> +/* >>> + * Copyright (c) 2021 Amlogic, Inc. All rights reserved. >>> + * Author: Yu Tu >>> + */ >>> + >>> +#ifndef _DT_BINDINGS_CLOCK_AMLOGIC_S4_PLL_CLKC_H >>> +#define _DT_BINDINGS_CLOCK_AMLOGIC_S4_PLL_CLKC_H >>> + >>> +/* >>> + * CLKID index values >>> + */ >>> + >>> +#define CLKID_FIXED_PLL            1 >>> +#define CLKID_FCLK_DIV2            3 >> >> Indexes start from 0 and are incremented by 1. Not by 2. >> >> NAK. > > I remember Jerome discussing this with you.You can look at this submission history. > https://lore.kernel.org/all/c088e01c-0714-82be-8347-6140daf56640@linaro.org/ Historically we did that by only exposing part of the numbers, controlling which clocks were part of the bindings. But it seems this doesn't make sens anymore, maybe it would be time to put all the clock ids in the bindings for this new SoC and break with the previous strategy. Neil > >> >> Best regards, >> Krzysztof >> >> .