From: Dmitry Rokosov <ddrokosov@sberdevices.ru>
To: Yu Tu <yu.tu@amlogic.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>, <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 <robh+dt@kernel.org>,
Neil Armstrong <neil.armstrong@linaro.org>,
Kevin Hilman <khilman@baylibre.com>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Conor Dooley <conor+dt@kernel.org>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
<kelvin.zhang@amlogic.com>, <qi.duan@amlogic.com>
Subject: Re: [PATCH V9 4/4] clk: meson: s4: add support for Amlogic S4 SoC peripheral clock controller
Date: Tue, 6 Jun 2023 18:38:41 +0300 [thread overview]
Message-ID: <20230606153841.plf5uh6eyzpzsm2e@CAB-WSD-L081021> (raw)
In-Reply-To: <1jwn0g39t2.fsf@starbuckisacylon.baylibre.com>
Hello Yu,
On Tue, Jun 06, 2023 at 04:38:15PM +0200, Jerome Brunet wrote:
>
> On Wed 17 May 2023 at 15:02, Yu Tu <yu.tu@amlogic.com> wrote:
>
> > Add the peripherals clock controller driver in the s4 SoC family.
> >
> > Signed-off-by: Yu Tu <yu.tu@amlogic.com>
> > ---
> > drivers/clk/meson/Kconfig | 12 +
> > drivers/clk/meson/Makefile | 1 +
> > drivers/clk/meson/s4-peripherals.c | 3830 ++++++++++++++++++++++++++++
> > drivers/clk/meson/s4-peripherals.h | 217 ++
> > 4 files changed, 4060 insertions(+)
> > create mode 100644 drivers/clk/meson/s4-peripherals.c
> > create mode 100644 drivers/clk/meson/s4-peripherals.h
> >
> > diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
> > index a663c90a3f3b..a6eb9fa15c74 100644
> > --- a/drivers/clk/meson/Kconfig
> > +++ b/drivers/clk/meson/Kconfig
> > @@ -128,4 +128,16 @@ config COMMON_CLK_S4_PLL
> > aka s4. Amlogic S805X2 and S905Y4 devices include AQ222 and AQ229.
> > Say Y if you want the board to work, because plls are the parent of most
> > peripherals.
> > +
> > +config COMMON_CLK_S4
> > + tristate "S4 SoC Peripherals clock controllers support"
> > + depends on ARM64
> > + default y
> > + select COMMON_CLK_MESON_REGMAP
> > + select COMMON_CLK_MESON_DUALDIV
> > + select COMMON_CLK_MESON_VID_PLL_DIV
> > + help
> > + Support for the Peripherals clock controller on Amlogic S805X2 and S905Y4
> > + devices, aka s4. Amlogic S805X2 and S905Y4 devices include AQ222 and AQ229.
> > + Say Y if you want peripherals to work.
> > endmenu
[...]
> > +static struct clk_regmap s4_rtc_32k_by_oscin = {
> > + .data = &(struct clk_regmap_gate_data){
> > + .offset = CLKCTRL_RTC_BY_OSCIN_CTRL0,
> > + .bit_idx = 30,
> > + },
> > + .hw.init = &(struct clk_init_data) {
> > + .name = "rtc_32k_by_oscin",
> > + .ops = &clk_regmap_gate_ops,
> > + .parent_hws = (const struct clk_hw *[]) {
> > + &s4_rtc_32k_by_oscin_sel.hw
> > + },
> > + .num_parents = 1,
> > + .flags = CLK_SET_RATE_PARENT,
> > + },
> > +};
> > +
> > +/*
> > + * This RTC clock can be supplied by an external 32KHz crystal oscillator.
> > + * If it is used, it should be documented in using fw_name and documented in the
> > + * Bindings. Not currently in use on this board.
> > + */
>
> This is confusing and not really helpful
> What you describe here is simply the purpose of fw_name ... so it does
> not warrant a specific comment
>
> > +static const struct clk_parent_data rtc_clk_sel_parent_data[] = {
> > + { .hw = &s4_rtc_32k_by_oscin.hw },
> > + { .hw = &s4_rtc_32k_by_oscin_div.hw },
> > + { .fw_name = "ext_32k", }
> > +};
> > +
> > +/*
> > + * All clocks that can be inherited from a more accurate RTC clock are marked
> > + * with the CLK_SET_RATE_NO_REPARENT flag. This is because in certain
> > + * situations, we may need to freeze their parent. The parent setup of these
> > + * clocks should be located on the device tree side.
> > + */
>
> It looks like the consensus is that CLK_SET_RATE_NO_REPARENT is not
> required. Please have at look at the discussion between Dmitry and
> Martin for the a1 controller
>
I hope below links will be helpful for you:
CLK_SET_RATE_NO_REPARENT IRC discussion:
https://libera.irclog.whitequark.org/linux-amlogic/2023-05-18
Clock driver LKML discussion about CLK_SET_RATE_NO_REPARENT:
https://lore.kernel.org/all/20230530120640.irugyrio3qa7czjy@CAB-WSD-L081021/
https://lore.kernel.org/all/20230524092750.ldm362chnpkwkcj4@CAB-WSD-L081021/
PWM discussion about special RTC case:
https://lore.kernel.org/all/20230522133739.7tc35zr2npsysopd@CAB-WSD-L081021/
And I apologize for any confusion I may have caused in our previous
discussion. I want to clarify that I have updated the implementation
of CLK_SET_RATE_NO_REPARENT after discussing it with Martin...
[...]
--
Thank you,
Dmitry
next prev parent reply other threads:[~2023-06-06 15:38 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-17 7:02 [PATCH V9 0/4] Add S4 SoC PLL and Peripheral clock Yu Tu
2023-05-17 7:02 ` [PATCH V9 1/4] dt-bindings: clock: document Amlogic S4 SoC PLL clock controller Yu Tu
2023-05-17 7:02 ` [PATCH V9 2/4] dt-bindings: clock: document Amlogic S4 SoC peripherals " Yu Tu
2023-05-17 7:02 ` [PATCH V9 3/4] clk: meson: S4: add support for Amlogic S4 SoC PLL clock driver Yu Tu
2023-06-06 14:02 ` Jerome Brunet
2023-06-08 2:54 ` Yu Tu
2023-06-08 8:53 ` Jerome Brunet
2023-06-08 9:30 ` Yu Tu
2023-06-08 11:32 ` Dmitry Rokosov
2023-06-08 12:46 ` Jerome Brunet
2023-06-08 21:02 ` Dmitry Rokosov
2023-06-06 14:34 ` Jerome Brunet
2023-06-08 2:29 ` Yu Tu
[not found] ` <20230517070215.28463-5-yu.tu@amlogic.com>
[not found] ` <1jwn0g39t2.fsf@starbuckisacylon.baylibre.com>
2023-06-06 15:38 ` Dmitry Rokosov [this message]
2023-06-08 3:26 ` [PATCH V9 4/4] clk: meson: s4: add support for Amlogic S4 SoC peripheral clock controller Yu Tu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230606153841.plf5uh6eyzpzsm2e@CAB-WSD-L081021 \
--to=ddrokosov@sberdevices.ru \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=jbrunet@baylibre.com \
--cc=kelvin.zhang@amlogic.com \
--cc=khilman@baylibre.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.blumenstingl@googlemail.com \
--cc=mturquette@baylibre.com \
--cc=neil.armstrong@linaro.org \
--cc=qi.duan@amlogic.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
--cc=yu.tu@amlogic.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).