Messages in this thread | | | Date | Wed, 26 Jan 2022 17:42:17 +0800 | Subject | Re: [PATCH v10 0/4] clk: meson: add a sub EMMC clock controller support | From | Liang Yang <> |
| |
Hi Jerome,
On 2022/1/26 17:14, Jerome Brunet wrote: > [ EXTERNAL EMAIL ] > > > On Wed 26 Jan 2022 at 17:08, Liang Yang <liang.yang@amlogic.com> wrote: > >> 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? > > Yes > >>> - 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. > > You don't need to copy everything. If I understood correctly, all the > Rx/Tx should not be needed. Yes, there is some duplication as it stands but > it allows to avoid coupling the MMC and NAND driver. We can still think > about optimizing things later on. Let's get something simply working > first. ok. i will do it. thank you. > >> >>> 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 >>>> >>> . > > .
| |