[PATCH V9 4/4] clk: meson: s4: add support for Amlogic S4 SoC peripheral clock controller
Yu Tu
yu.tu at amlogic.com
Wed Jun 7 20:26:24 PDT 2023
Hi Dmitry,
On 2023/6/6 23:38, Dmitry Rokosov wrote:
> [ EXTERNAL EMAIL ]
>
> 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 at amlogic.com> wrote:
>>
>>> Add the peripherals clock controller driver in the s4 SoC family.
>>>
>>> Signed-off-by: Yu Tu <yu.tu at 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 so much for your comments.
>
> --
> Thank you,
> Dmitry
More information about the linux-amlogic
mailing list