From: Liang Yang <liang.yang@amlogic.com>
To: Neil Armstrong <narmstrong@baylibre.com>,
Jerome Brunet <jbrunet@baylibre.com>,
Kevin Hilman <khilman@baylibre.com>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>,
<linux-clk@vger.kernel.org>
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
Jianxin Pan <jianxin.pan@amlogic.com>,
Victor Wan <victor.wan@amlogic.com>,
XianWei Zhao <xianwei.zhao@amlogic.com>,
Kelvin Zhang <kelvin.zhang@amlogic.com>,
BiChao Zheng <bichao.zheng@amlogic.com>,
YongHui Yu <yonghui.yu@amlogic.com>,
<linux-arm-kernel@lists.infradead.org>,
<linux-amlogic@lists.infradead.org>,
<linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>
Subject: Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support
Date: Wed, 26 Jan 2022 17:08:42 +0800 [thread overview]
Message-ID: <6eb4f247-367b-d460-6314-fc94ccd00b89@amlogic.com> (raw)
In-Reply-To: <f5a429f2-ffbc-ea03-810a-45a0f90959a2@baylibre.com>
Hi Neil,
On 2022/1/25 22:54, Neil Armstrong wrote:
> [ EXTERNAL EMAIL ]
>
> Hi Liang,
>
> On 21/01/2022 08:45, Liang Yang wrote:
>> This driver will add a MMC clock controller driver support.
>> The original idea about adding a clock controller is during the
>> discussion in the NAND driver mainline effort[1].
>>
>> This driver is tested in the S400 board (AXG platform) with NAND driver.
>
> Thanks a lot for providing a fixed and updated version of this serie.
>
> After some chat with Jerome and Kevin, it seems the way the eMMC clock reuse
> for NAND was designed nearly 4 years doesn't look accurate anymore.
>
> Having a separate clk driver designed to replace the eMMC node when NAND is
> used on the board seems over engineered.
>
> Actually having the clock code you add in this serie _but_ directly in
> the NAND looks far better, and more coherent since having Linux runtime
> detection of eMMC vs NAND will never happen and even this serie required
> some DT modification from the bootloader.
>
> I'll let Jerome or Kevin add more details if they want, but I think you should resurrect
> the work you pushed in [1] & [2] but:
> - passing the eMMC clk registers as a third "reg" cell
Does it just need to define a 'reg' resource in NFC node and not
'syscon' here?
> - passing the same "clocks" phandle as the eMMC node
> - adding the eMMC clock code in the NAND driver directly
>
> I'm open to discussions if you consider the current approach is still superior.
I don't have persuasive ideas, but really it shares the common clock
implementation for both NFC and EMMC. and we don't need to paste the
same code in NFC and EMMC.
>
> Thanks,
> Neil
>
> [1] https://lore.kernel.org/r/20220106033130.37623-1-liang.yang@amlogic.com
> [2] https://lore.kernel.org/r/20220106032504.23310-1-liang.yang@amlogic.com
>
>>
>> Changes since v9 [10]
>> - use clk_parent_data instead of parent_names
>>
>> Changes since v8 [9]
>> - use MESON_SCLK_ONE_BASED instead of CLK_DIVIDER_ONE_BASED
>> - use struct_size to caculate onecell_data
>> - add clk-phase-delay.h
>> - define CLK_DELAY_STEP_PS_GX and CLK_DELAY_STEP_PS_AXG
>>
>> Changes since v7 [8]
>> - move meson_clk_get_phase_delay_data() from header to driver
>> - CONFIG sclk-div with COMMON_CLK_AMLOGIC instead of COMMON_CLK_AMLOGIC_AUDIO
>> - remove onecell date and ID for internal MUX clk
>> - use helper for functions for ONE_BASED in sclk-div
>> - add ONE_BASED support for duty cycle
>>
>> Changes since v6 [7]:
>> - add one based support for sclk divier
>> - alloc sclk in probe for multiple instance
>> - fix coding styles
>>
>> Changes since v5 [6]:
>> - remove divider ops with .init and use sclk_div instead
>> - drop CLK_DIVIDER_ROUND_CLOSEST in mux and div
>> - drop the useless type cast
>>
>> Changes since v4 [5]:
>> - use struct parm in phase delay driver
>> - remove 0 delay releted part in phase delay driver
>> - don't rebuild the parent name once again
>> - add divider ops with .init
>>
>> Changes since v3 [4]:
>> - separate clk-phase-delay driver
>> - replace clk_get_rate() with clk_hw_get_rate()
>> - collect Rob's R-Y
>> - drop 'meson-' prefix from compatible string
>>
>> Changes since v2 [3]:
>> - squash dt-binding clock-id patch
>> - update license
>> - fix alignment
>> - construct a clk register helper() function
>>
>> Changes since v1 [2]:
>> - implement phase clock
>> - update compatible name
>> - adjust file name
>> - divider probe() into small functions, and re-use them
>>
>> [1] https://lkml.kernel.org/r/20180628090034.0637a062@xps13
>> [2] https://lkml.kernel.org/r/20180703145716.31860-1-yixun.lan@amlogic.com
>> [3] https://lkml.kernel.org/r/20180710163658.6175-1-yixun.lan@amlogic.com
>> [4] https://lkml.kernel.org/r/20180712211244.11428-1-yixun.lan@amlogic.com
>> [5] https://lkml.kernel.org/r/20180809070724.11935-4-yixun.lan@amlogic.com
>> [6] https://lkml.kernel.org/r/1539839245-13793-1-git-send-email-jianxin.pan@amlogic.com
>> [7] https://lkml.kernel.org/r/1541089855-19356-1-git-send-email-jianxin.pan@amlogic.com
>> [8] https://lkml.kernel.org/r/1544457877-51301-1-git-send-email-jianxin.pan@amlogic.com
>> [9] https://lkml.kernel.org/r/1545063850-21504-1-git-send-email-jianxin.pan@amlogic.com
>> [10] https://lore.kernel.org/all/20220113115745.45826-1-liang.yang@amlogic.com/
>> Liang Yang (4):
>> clk: meson: add one based divider support for sclk
>> clk: meson: add emmc sub clock phase delay driver
>> clk: meson: add DT documentation for emmc clock controller
>> clk: meson: add sub MMC clock controller driver
>>
>> .../bindings/clock/amlogic,mmc-clkc.yaml | 64 ++++
>> drivers/clk/meson/Kconfig | 18 ++
>> drivers/clk/meson/Makefile | 2 +
>> drivers/clk/meson/clk-phase-delay.c | 69 ++++
>> drivers/clk/meson/clk-phase-delay.h | 20 ++
>> drivers/clk/meson/mmc-clkc.c | 302 ++++++++++++++++++
>> drivers/clk/meson/sclk-div.c | 59 ++--
>> drivers/clk/meson/sclk-div.h | 3 +
>> include/dt-bindings/clock/amlogic,mmc-clkc.h | 14 +
>> 9 files changed, 529 insertions(+), 22 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,mmc-clkc.yaml
>> create mode 100644 drivers/clk/meson/clk-phase-delay.c
>> create mode 100644 drivers/clk/meson/clk-phase-delay.h
>> create mode 100644 drivers/clk/meson/mmc-clkc.c
>> create mode 100644 include/dt-bindings/clock/amlogic,mmc-clkc.h
>>
>
> .
next prev parent reply other threads:[~2022-01-26 9:08 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-21 7:45 [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support Liang Yang
2022-01-21 7:45 ` [PATCH v10 1/4] clk: meson: add one based divider support for sclk Liang Yang
2022-01-21 7:45 ` [PATCH v10 2/4] clk: meson: add emmc sub clock phase delay driver Liang Yang
2022-01-21 7:45 ` [PATCH v10 3/4] clk: meson: add DT documentation for emmc clock controller Liang Yang
2022-01-21 7:45 ` [PATCH v10 4/4] clk: meson: add sub MMC clock controller driver Liang Yang
2022-01-25 14:54 ` [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support Neil Armstrong
2022-01-26 9:08 ` Liang Yang [this message]
2022-01-26 9:14 ` Jerome Brunet
2022-01-26 9:42 ` Liang Yang
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=6eb4f247-367b-d460-6314-fc94ccd00b89@amlogic.com \
--to=liang.yang@amlogic.com \
--cc=bichao.zheng@amlogic.com \
--cc=devicetree@vger.kernel.org \
--cc=jbrunet@baylibre.com \
--cc=jianxin.pan@amlogic.com \
--cc=kelvin.zhang@amlogic.com \
--cc=khilman@baylibre.com \
--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=narmstrong@baylibre.com \
--cc=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
--cc=victor.wan@amlogic.com \
--cc=xianwei.zhao@amlogic.com \
--cc=yonghui.yu@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).