From: Bo Gan <ganboing@gmail.com>
To: dongxuyang@eswincomputing.com, mturquette@baylibre.com,
sboyd@kernel.org, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
troy.mitchell@linux.dev, bmasney@redhat.com
Cc: ningyu@eswincomputing.com, linmin@eswincomputing.com,
huangyifeng@eswincomputing.com, pinkesh.vaghela@einfochips.com,
marcel@ziswiler.com
Subject: Re: [PATCH v13 0/3] Add driver support for ESWIN eic700 SoC clock controller
Date: Tue, 24 Feb 2026 00:46:13 -0800 [thread overview]
Message-ID: <ae2358d7-7e82-49fe-8a03-195487a7b010@gmail.com> (raw)
In-Reply-To: <20260214101421.228-1-dongxuyang@eswincomputing.com>
Hi ESWIN folks,
I'm testing your patchset on
https://github.com/ganboing/linux-eic77/tree/eic77-integration-test-clk-v13
It's applied on top of Samuel's PMA v3 patshset, plus device-tree patches
from your integration test branch. It's pretty similar to what Marcel did,
but I just want to have a separate verification. What I found is that the
kernel can successfully boot with clk_ignore_unused (eMMC/eth working),
but hangs without. It stuck at
[ 3.257141] clk: Disabling unused clocks
I'm pretty sure that some clocks that weren't supposed to get disabled got
turned off. Can you validate if that's a bug in the clock driver, or my
device-tree just didn't link all clocks. One thing I noticed is that there
is no CLK_IS_CRITICAL in your code, so it's highly likely that DDR PLL or
others were turned off unintentionally. Not a clock driver expert, but I'd
expect the kernel should work fine without clk_ignore_unused.
Bo
On 2/14/26 02:14, dongxuyang@eswincomputing.com wrote:
> From: Xuyang Dong <dongxuyang@eswincomputing.com>
>
> The link [1] provides the official documentation for the EIC7700. Section 3.2
> covers the clock subsystem.
>
> [1] https://www.sifive.com/document-file/eic7700x-datasheet
>
> Updates:
> Change in v13:
> - Updated driver file
> - Modify commit subject from "clock: eswin: Add eic7700 clock driver"
> to "clk: eswin: Add eic7700 clock driver".
> - Use parent rate to calculate PLL clock rate.
> - Use GENMASK() to define PLL mask macros and use FIELD_PREP() to simplify
> bitwise operations.
> - Remove shift and width references for pll enable, refdiv, fbdiv, frac and postdiv.
> (Use only the PLL macros to match the functionality)
> - Set postdiv1 and postdiv2 in clk_pll_set_rate function.
>
> - Link to v11: https://lore.kernel.org/all/20260213094112.115-1-dongxuyang@eswincomputing.com/
>
> Change in v12:
> - Updated driver file
> - Add CLK_MUX_ROUND_CLOSEST to the mux clocks since they need a more
> accurate clock frequency.
> - Change 'rem' from u64 to u32 for function 'eswin_calc_pll' and
> function 'clk_pll_recalc_rate'.
> - Change 'rate' from u64 to unsigned long for function 'clk_pll_recalc_rate'.
> - Fix build error with u64 divisor for function `clk_pll_set_rate'.
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202602111424.mhTAakDX-lkp@intel.com/
>
> - Link to v11: https://lore.kernel.org/all/20260210095008.726-1-dongxuyang@eswincomputing.com/
>
> Change in v11:
> - Updated driver file
> - Add "Tested-by: Marcel Ziswiler <marcel@ziswiler.com> # ebc77" tag.
> - Fix build error in function eswin_clk_register_clks.
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202602060520.p4Hg35Ja-lkp@intel.com/
> - Link to v10: https://lore.kernel.org/all/20260205093322.1030-1-dongxuyang@eswincomputing.com/
>
> Changes in v10:
> - Updated driver file
> - Add a private clock divider API named 'eswin_register_clkdiv' to
> register divider clocks with private flag.
> Define 'ESWIN_PRIV_DIV_MIN_2' private flag for registering the clock
> dividers whose division ratio start from 2.
> Implement the private set_rate, recalc_rate, and determine_rate accordingly.
> - Add CLK_DIVIDER_ALLOW_ZERO flag to the clock dividers whose division
> ratio start from 0.
> - Replace CONFIG_ARCH_ESWIN with CONFIG_COMMON_CLK_ESWIN in the Makefile.
> - Modify the help description of COMMON_CLK_EIC7700.
> - Move register offset definitions from 'clk-eic7700.h' to 'clk-eic7700.c' and
> remove the 'clk-eic7700.h'. Remove '_CTRL' from the offset names.
> - Remove all unused headers.
> - Use devm_platform_ioremap_resource() instead of devm_of_iomap().
> - Export the functions from clk.c as symbols.
> - Use readl_poll_timeout().
> - Use 'clk_parent_data' and '.hw' instead of string parent names.
> - Rename the header file from clk.h to common.h.
> - Rename macros from EIC7700_* to ESWIN_* in common.h.
> - Add a new function, 'eswin_clk_register_clks', which can register
> divider, mux, gate, and fixed-factor clocks based on their types.
> - Add structure 'eswin_clk_info' to manage the clocks that need to be registered
> in a specific order.
> - Add macros 'ESWIN_*_TYPE' to define divider, mux, gate, and fixed-factor clocks
> with type. Add enum 'eswin_clk_type' for these types.
> - Remove 'eswin_clk_register_mux_tbl'. Use 'eswin_clk_register_mux' to register
> mux clocks with or without table.
> - Add xtal24m as the parent clock of the PLL.
> - Change 2025 to 2026 in all files.
>
> - Link to v9: https://lore.kernel.org/all/20251229105844.1089-1-dongxuyang@eswincomputing.com/
>
> Changes in v9:
> - Updated driver file
> - Checked return values of eswin_clk_register_*() in the probe function.
> - Removed binding IDs check from pll functions in clk.c, as these functions
> were exclusively used by pll clocks and did not require ID validation.
> - The PLL structure has been extended with max_rate and min_rate fields to
> explicitly define the frequency operating range of the PLL. These limits
> were now utilized by the clk_pll_determine_rate() function.
> - Removed __clk_lookup() in clk_pll_set_rate. Added clk_notifier support
> in clk-eic7700.c.
> - Added five previously missing clocks from [1].
>
> - Link to v8: https://lore.kernel.org/all/20251113013637.1109-1-dongxuyang@eswincomputing.com/
>
> Changes in v8:
> - Updated YAML file
> - Added "Acked-by: Troy Mitchell <troy.mitchell@linux.dev>"
> - Updated driver file
> - Changed Kconfig from bool to tristate.
> - Updated MAINTAINERS file
> - Added "ESWIN SOC SERIES CLOCK DRIVER"
>
> - Link to v7: https://lore.kernel.org/all/20251023071658.455-1-dongxuyang@eswincomputing.com/
>
> Changes in v7:
> - Updated YAML file
> - Added "Acked-by: Conor Dooley <conor.dooley@microchip.com>" for bindings.
> - Updated driver file
> - Added description for clk of eswin_calc_pll().
> - Added macro EIC7700_MUX_TBL to manage mux clock-tree.
> - Added eswin_clk_register_mux_tbl() to register mux clocks with
> discontinuous parent indexes.
>
> - Link to v6: https://lore.kernel.org/all/20251009092029.140-1-dongxuyang@eswincomputing.com/
>
> Changes in v6:
> - Removed config option patch dependency from cover letter, because the patch
> was applied.
> - Updated YAML file
> - Added an oscillator as the clock input, named xtal24m.
> - Added clocks property.
> - Updated driver file
> - Replaced fixed_rate_clk_xtal_24m with xtal24m.
> - Dropped fixed_rate_clk_xtal_24m from driver. Because clock xtal24m was
> registered by fixed-clock as oscillator.
>
> - Link to v5: https://lore.kernel.org/all/20250923084637.1223-1-dongxuyang@eswincomputing.com/
>
> Changes in v5:
> - Removed vendor prefix patch dependency from cover letter, because the patch
> was applied.
> - Updated YAML file
> - Placed the required after all properties.
> - Removed patternProperties. Also removed compatible of eswin,pll-clock,
> eswin,mux-clock, eswin,divider-clock and eswin,gate-clock as we have moved
> clock tree from DTS to Linux driver.
> - Removed the clock tree from DTS. Used clock-controller to manage all
> clock. Removed all child nodes in clock-controller.
> - Removed '#address-cells' and '#size-cells' properties, because the clock
> controller did not need to define these properties.
> - Removed eic7700-clocks.dtsi.
> - Added dt-bindings header for clock IDs. Because used the IDs to register
> clocks.
> - Updated driver file
> - Modified the commit for clock driver. Dropped indentation in commit.
> - Removed CLK_OF_DECLARE(). Used *clk_hw_register* to register clocks. Used
> devm_of_clk_add_hw_provider.
> - Dropped singletons.
> - Checked the value right after obtaining it.
> - Removed the definitions of macro frequency in clk.h like CLK_FREQ_24M.
> - Modified description of help in Kconfig.
> - Added COMPILE_TEST. Added COMMON_CLK_ESWIN for clk.o. And added
> "select COMMON_CLK_ESWIN" for clk-eic7700.c. Without COMMON_CLK_EIC7700,
> clk.c could not be compiled.
> - Used .determined_rate.
> - Added macro definitions of EIC7700_DIV, EIC7700_FIXED, EIC7700_FACTOR,
> EIC7700_MUX and EIC7700_PLL to manage clock tree.
> - Added clk-eic7700.h to place eic7700 SoC clock registers.
> - Removed refdiv_val and postdiv1_val from clk_pll_recalc_rate(). Because
> these values were unused.
>
> - Link to v4: https://lore.kernel.org/all/20250815093539.975-1-dongxuyang@eswincomputing.com/
>
> Changes in v4:
> - Updated YAML file
> - Changed name from cpu-default-frequency to cpu-default-freq-hz.
> - Dropped $ref of cpu-default-frequency.
> - Added cpu-default-frequency for required.
> - Removed cpu-default-frequency in updated file, because there was no
> need to add cpu-default-frequency.
> - Moved DIVIDER to DIV.
> - Arranged the IDs in order.
> - Dropped EIC7700_NR_CLKS.
> - Removed dt-bindings eswin,eic7700-clock.h. Because IDs was not used,
> and used clock device nodes.
> - According to the updated driver codes, the YAML has been updated.
> - Updated driver file
> - Remove undocumented parameters "cpu_no_boost_1_6ghz" and
> "cpu-default-frequency".
> - Modified the comment and used the correct Linux coding style.
> - Removed codes of voltage, because it was not the clock driver.
> - Updated the formula of clock frequency calculation. Removed the logic
> that only used register selection.
> - Used CLK_OF_DECLARE() to register clocks. Registered pll-clock,
> mux-clock, divider-clock, and gate-clock in clk-eic7700.c.
> The specific implementation of clock registration was in clk.c.
> - Added eic7700-clocks.dtsi.
> - Moved device information to DTS. Put all clocks' node in the
> eic7700-clocks.dtsi.
>
> - Link to v3: https://lore.kernel.org/all/20250624103212.287-1-dongxuyang@eswincomputing.com/
>
> Changes in v3:
> - Update example, drop child node and add '#clock-cells' to the parent
> node.
> - Change parent node from sys-crg to clock-controller for this yaml.
> - Drop "syscon", "simple-mfd" to clear warnings/errors by using "make
> dt_binding_check". And these are not necessary.
> - Add "cpu-default-frequency" definition in yaml for "undocumented ABI".
> - Drop Reviewed-by, this is misunderstanding. We have not received such
> an email.
> - Link to v2: https://lore.kernel.org/all/20250523090747.1830-1-dongxuyang@eswincomputing.com/
>
> Changes in v2:
> - Update example, drop child node.
> - Clear warnings/errors for using "make dt_binding_check".
> - Change to the correct format.
> - Drop some non-stanard code.
> - Use dev_err_probe() in probe functions.
> - Link to v1: https://lore.kernel.org/all/20250514002233.187-1-dongxuyang@eswincomputing.com/
>
> Xuyang Dong (3):
> dt-bindings: clock: eswin: Documentation for eic7700 SoC
> clk: eswin: Add eic7700 clock driver
> MAINTAINERS: Add entry for ESWIN EIC7700 clock driver
>
> .../bindings/clock/eswin,eic7700-clock.yaml | 46 +
> MAINTAINERS | 8 +
> drivers/clk/Kconfig | 1 +
> drivers/clk/Makefile | 1 +
> drivers/clk/eswin/Kconfig | 15 +
> drivers/clk/eswin/Makefile | 8 +
> drivers/clk/eswin/clk-eic7700.c | 1357 +++++++++++++++++
> drivers/clk/eswin/clk.c | 591 +++++++
> drivers/clk/eswin/common.h | 341 +++++
> .../dt-bindings/clock/eswin,eic7700-clock.h | 285 ++++
> 10 files changed, 2653 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/eswin,eic7700-clock.yaml
> create mode 100644 drivers/clk/eswin/Kconfig
> create mode 100644 drivers/clk/eswin/Makefile
> create mode 100644 drivers/clk/eswin/clk-eic7700.c
> create mode 100644 drivers/clk/eswin/clk.c
> create mode 100644 drivers/clk/eswin/common.h
> create mode 100644 include/dt-bindings/clock/eswin,eic7700-clock.h
>
> --
> 2.43.0
>
next prev parent reply other threads:[~2026-02-24 8:48 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-14 10:14 [PATCH v13 0/3] Add driver support for ESWIN eic700 SoC clock controller dongxuyang
2026-02-14 10:15 ` [PATCH v13 1/3] dt-bindings: clock: eswin: Documentation for eic7700 SoC dongxuyang
2026-02-14 10:15 ` [PATCH v13 2/3] clk: eswin: Add eic7700 clock driver dongxuyang
2026-02-16 17:17 ` Brian Masney
2026-02-17 1:09 ` Brian Masney
2026-02-19 13:14 ` Marcel Ziswiler
2026-02-25 6:55 ` Xuyang Dong
2026-02-25 13:29 ` Brian Masney
2026-02-14 10:15 ` [PATCH v13 3/3] MAINTAINERS: Add entry for ESWIN EIC7700 " dongxuyang
2026-02-24 8:46 ` Bo Gan [this message]
2026-02-24 9:09 ` Re: [PATCH v13 0/3] Add driver support for ESWIN eic700 SoC clock controller Xuyang Dong
2026-02-24 11:33 ` Brian Masney
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=ae2358d7-7e82-49fe-8a03-195487a7b010@gmail.com \
--to=ganboing@gmail.com \
--cc=bmasney@redhat.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dongxuyang@eswincomputing.com \
--cc=huangyifeng@eswincomputing.com \
--cc=krzk+dt@kernel.org \
--cc=linmin@eswincomputing.com \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcel@ziswiler.com \
--cc=mturquette@baylibre.com \
--cc=ningyu@eswincomputing.com \
--cc=pinkesh.vaghela@einfochips.com \
--cc=robh@kernel.org \
--cc=sboyd@kernel.org \
--cc=troy.mitchell@linux.dev \
/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