Re: [PATCH v7 0/3] Add driver support for ESWIN EIC7700 HSP clock and reset generator

["Xuyang Dong" <dongxuyang@eswincomputing.com>]

> 
> From: Xuyang Dong <dongxuyang@eswincomputing.com>
> 
> Add support for the ESWIN EIC7700 HSP (high-speed peripherals). The drivers
> provide basic functionality to manage and control the clock and reset
> signals for EIC7700 HSP, including mmc, USB, ethernet, SATA and DMAC.
> 
> The clock and reset registers are mapped to overlapping I/O address ranges.
> This causes a resource conflict when two drivers attempt to request the
> same region. Use the auxiliary device framework: the main driver
> allocates the shared register region and passes it to auxiliary
> devices, avoiding resource contention and duplicate remapping.
> 
> Features:
> Implements support for the ESWIN EIC7700 HSP clock and reset controller.
> Provide API to manage clock and reset signals for the EIC7700 HSP.
> 
> Supported chips:
> ESWIN EIC7700 series SoC.
> 
> Test:
> Test this patch on the Sifive HiFive Premier P550 (which used the EIC7700
> SoC), include USB and other peripherals. All the drivers of these modules
> use the clock module and reset module.
> 

Hi Stephen and Michael,

Just a gentle ping on this thread.
v7 now has Reviewed-by tags from Brian for the clock part, and from 
Philipp for the reset part. Do you have any further comments on v7?

Best regards,
Xuyang Dong

> Updates:
>   Changes in v7:
>   - Clock driver:
>     - Add the 'CLK_IGNORE_UNUSED' flag to all gate clocks; otherwise,
>       board bring-up fails unless 'clk_ignore_unused' is passed on the
>       kernel command line.
>     - Sashiko's comments: not addressed in this version 
>       (see email for explanation)
> 
>   - Link to v6: https://lore.kernel.org/all/20260519081431.1424-1-dongxuyang@eswincomputing.com/
> 
>   Changes in v6:
>   - Clock driver:
>     - Add 'select REGMAP_MMIO' for Kconfig entry (Sashiko review of v5).
>   - Reset driver:
>     - Add <linux/module.h> (Sashiko review of v5).
> 
>   - Link to v5: https://lore.kernel.org/all/20260514114212.903-1-dongxuyang@eswincomputing.com/
> 
>   Changes in v5:
>   - Clock driver:
>     - Add check for regmap_read().
>     - Add "Reviewed-by: Brian Masney <bmasney@redhat.com>".
> 
>   - Link to v4: https://lore.kernel.org/all/20260512020432.671-1-dongxuyang@eswincomputing.com/
> 
>   Changes in v4:
>   - Clock driver:
>     - Remove "Reviewed-by: Benoît Monin <benoit.monin@bootlin.com>" and
>       "Reviewed-by: Brian Masney <bmasney@redhat.com>", because the clock
>       driver has been updated.
>     - Remove inclusion of io.h.
>     - Add struct regmap to eic7700_hsp_clk_gate.
>       Replace 'void __iomem *reg' with 'unsigned int reg'.
>       Replace 'void __iomem *ref_reg' with 'unsigned int ref_reg'.
>       Replace long with int for 'offset' and 'ref_offset'.
>       Remove 'spinlock_t *lock'.
>       Apply the same changes to hsp_clk_register_gate().
>     - Remove the structure eic7700_hsp_regmap_lock, and the functions
>       eic7700_hsp_regmap_lock() and eic7700_hsp_regmap_unlock().
>     - Remove the 'guard(spinlock_irqsave)(gate->lock)' in
>       hsp_clk_gate_endisable().
>     - Replace readl() and writel() with regmap_assign_bits() in
>       hsp_clk_gate_endisable().
>     - Change the parameter enable from int to bool.
>     - Replace readl() with regmap_read() in hsp_clk_gate_is_enabled().
>     - Remove the lock_ctx variable.
>     - Move eic7700_hsp_regmap_config from inside the probe function to global
>       scope.
>       Remove '.lock', '.unlock' and 'lock_arg'.
>       Add '.fast_io = true' and '.use_raw_spinlock = true'.
>   - Reset driver:
>     - Add "Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>" for reset.
>     - Drop the temporary variable, just return regmap_assign_bits(...) directly.
>     - Replace -EINVAL with -ENODEV for dev_get_regmap() check.
>     - Drop the '_dt'.
> 
>   - Link to v3: https://lore.kernel.org/all/20260423090904.2108-1-dongxuyang@eswincomputing.com/
> 
>   Changes in v3:
>   - Bindings:
>     - Added "Acked-by: Conor Dooley <conor.dooley@microchip.com>" for bindings.
>   - Clock driver:
>     - Remove 'gate_flags'.
>     - Add __acquires for eic7700_hsp_regmap_lock() and add __releases for
>       eic7700_hsp_regmap_unlock().
>     - Move writel(USB_REF_XTAL24M, gate->ref_reg) into enable. Because this
>       is only used for USB gate clock on the enable path. And modify the
>       comments.
>     - Simplify to: 'return !!(readl(gate->reg) & BIT(gate->bit_idx));'.
>     - Drop const from eic7700_hsp_regmap_config.
>     - Declare eic7700_hsp_regmap_config as a regular variable at the top.
> 
>   - Link to v2: https://lore.kernel.org/all/20260420093929.1895-1-dongxuyang@eswincomputing.com/
> 
>   Changes in v2:
>   - Bindings:
>     - Remove "hsp_" from clock-names.
>     - Replace "eswin,eic7700-clock.yaml" and "eswin,eic7700-hspcrg.yaml" with
>       "eswin,eic7700*".
>     - Replace "eswin,eic7700-clock.h" and "eswin,eic7700-hspcrg.h" with
>       "eswin,eic7700*".
>   - Clock driver:
>     - Use guard(spinlock_irqsave)(gate->lock) instead of spin_lock_irqsave()
>       and remove spin_unlock_irqrestore().
>     - Remove the newline in function hsp_clk_gate_is_enabled().
>     - Use struct clk_init_data init = {}.
>     - Replace 'static struct clk_parent_data' with
>       'static const struct clk_parent_data'.
>     - Change '.fw_name' to '.index', because the function
>       eswin_clk_register_fixed_factor() uses .index.
>     - The structures of clocks should use static struct. When registering a clock,
>       the 'hw' field in the structure will be assigned.
>     - Remove __force.
>     - Create the regmap in the clock driver and remove (__force void*)data->base.
>       The reset driver uses dev_get_regmap() to get the regmap from the clock.
>     - Move 'const struct regmap_config eic7700_hsp_regmap_config' from reset
>       driver to clock driver.
>     - The USB clock gate (hsp_clk_gate_endisable) and the reset driver both
>       perform read-modify-write cycles on registers 0x800 and 0x900. Use
>       custom regmap lock callbacks so that regmap operations hold data->lock
>       with IRQs disabled, the same lock the clock gate path uses, preventing
>       concurrent RMW races on those shared registers.
>     - Change to 'ret = eswin_clk_register_fixed_factor(dev, eic7700_hsp_factor_clks,'.
>       The next line will be over 80 characters and under 100 characters.
>   - Reset driver:
>     - Remove 'depends on COMMON_CLK_EIC7700_HSP' and 'default COMMON_CLK_EIC7700_HSP'.
>     - Use regmap_assign_bits() in assert and deassert functions.
>     - Remove eic7700_hsp_reset_reset().
>     - The clock driver creates the regmap, and the reset driver uses dev_get_regmap().
>     - Remove of_reset_n_cells.
> 
>   - Link to v1: https://lore.kernel.org/all/20260403093459.612-1-dongxuyang@eswincomputing.com/
> 
> Xuyang Dong (3):
>   dt-bindings: clock: Add ESWIN eic7700 HSP clock and reset generator
>   clk: eswin: Add eic7700 HSP clock driver
>   reset: eswin: Add eic7700 HSP reset driver
> 
>  .../bindings/clock/eswin,eic7700-hspcrg.yaml  |  63 ++++
>  MAINTAINERS                                   |   5 +-
>  drivers/clk/eswin/Kconfig                     |  13 +
>  drivers/clk/eswin/Makefile                    |   1 +
>  drivers/clk/eswin/clk-eic7700-hsp.c           | 345 ++++++++++++++++++
>  drivers/reset/Kconfig                         |  11 +
>  drivers/reset/Makefile                        |   1 +
>  drivers/reset/reset-eic7700-hsp.c             | 113 ++++++
>  .../dt-bindings/clock/eswin,eic7700-hspcrg.h  |  33 ++
>  .../dt-bindings/reset/eswin,eic7700-hspcrg.h  |  21 ++
>  10 files changed, 604 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/eswin,eic7700-hspcrg.yaml
>  create mode 100644 drivers/clk/eswin/clk-eic7700-hsp.c
>  create mode 100644 drivers/reset/reset-eic7700-hsp.c
>  create mode 100644 include/dt-bindings/clock/eswin,eic7700-hspcrg.h
>  create mode 100644 include/dt-bindings/reset/eswin,eic7700-hspcrg.h
> 
> --
> 2.34.1