From: Peter Griffin <peter.griffin@linaro.org>
To: "André Draszik" <andre.draszik@linaro.org>
Cc: mturquette@baylibre.com, sboyd@kernel.org, robh@kernel.org,
krzk+dt@kernel.org, conor+dt@kernel.org, vkoul@kernel.org,
kishon@kernel.org, alim.akhtar@samsung.com, avri.altman@wdc.com,
bvanassche@acm.org, s.nawrocki@samsung.com,
cw00.choi@samsung.com, jejb@linux.ibm.com,
martin.petersen@oracle.com, chanho61.park@samsung.com,
ebiggers@kernel.org, linux-scsi@vger.kernel.org,
linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
linux-clk@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, tudor.ambarus@linaro.org,
saravanak@google.com, willmcvicker@google.com
Subject: Re: [PATCH 08/17] clk: samsung: gs101: add support for cmu_hsi2
Date: Tue, 23 Apr 2024 18:45:50 +0100 [thread overview]
Message-ID: <CADrjBPrNqJ6FNKZTgVxST1en-hRdyZFmJe42uwerSnDSmgifbg@mail.gmail.com> (raw)
In-Reply-To: <6c2b060b3b32b2da46bafbdc33236c319b6cec62.camel@linaro.org>
Hi André,
On Mon, 8 Apr 2024 at 15:49, André Draszik <andre.draszik@linaro.org> wrote:
>
> Hi Pete,
>
> On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote:
> > CMU_HSI2 is the clock management unit used for the hsi2 block.
> > HSI stands for High Speed Interface and as such it generates
> > clocks for PCIe, UFS and MMC card.
> >
> > This patch adds support for the muxes, dividers, and gates in
> > cmu_hsi2.
> >
> > CLK_GOUT_HSI2_HSI2_CMU_HSI2_PCLK is marked as CLK_IS_CRITICAL
> > as disabling it leads to an immediate system hang.
> >
> > CLK_GOUT_HSI2_SYSREG_HSI2_PCLK is also marked CLK_IS_CRITICAL.
> > A hang is not observed with fine grained clock control, but
> > UFS IP does not function with syscon controlling this clock
> > just around hsi2_sysreg register accesses.
>
> Would it make sense to add this clock to the &ufs_0 node in the DTS
> instead? Seems more natural than a clock that's constantly enabled?
Will add this to ufs node in v2.
>
> > [...]
> >
> > Updated regex for clock name mangling
> > sed \
> > -e 's|^PLL_LOCKTIME_PLL_\([^_]\+\)|fout_\L\1_pll|' \
> > \
> > -e 's|^PLL_CON0_MUX_CLKCMU_\([^_]\+\)_\(.*\)|mout_\L\1_\2|' \
> > -e 's|^PLL_CON0_PLL_\(.*\)|mout_pll_\L\1|' \
> > -e 's|^CLK_CON_MUX_MUX_CLK_\(.*\)|mout_\L\1|' \
> > -e '/^PLL_CON[1-4]_[^_]\+_/d' \
> > -e '/^[^_]\+_CMU_[^_]\+_CONTROLLER_OPTION/d' \
> > -e '/^CLKOUT_CON_BLK_[^_]\+_CMU_[^_]\+_CLKOUT0/d' \
> > \
> > -e 's|_IPCLKPORT||' \
> > -e 's|_RSTNSYNC||' \
> > -e 's|_G4X2_DWC_PCIE_CTL||' \
> > -e 's|_G4X1_DWC_PCIE_CTL||' \
> > -e 's|_PCIE_SUB_CTRL||' \
> > -e 's|_INST_0||g' \
> > -e 's|_LN05LPE||' \
> > -e 's|_TM_WRAPPER||' \
> > -e 's|_SF||' \
> > \
> > -e 's|^CLK_CON_DIV_DIV_CLK_\([^_]\+\)_\(.*\)|dout_\L\1_\2|' \
> > \
> > -e 's|^CLK_CON_BUF_CLKBUF_\([^_]\+\)_\(.*\)|gout_\L\1_\2|' \
> > -e 's|^CLK_CON_GAT_CLK_BLK_\([^_]\+\)_UID_\(.*\)|gout_\L\1_\2|' \
> > -e 's|^gout_[^_]\+_[^_]\+_cmu_\([^_]\+\)_pclk$|gout_\1_\1_pclk|' \
> > -e 's|^CLK_CON_GAT_GOUT_BLK_\([^_]\+\)_UID_\(.*\)|gout_\L\1_\2|' \
> > -e 's|^CLK_CON_GAT_CLK_\([^_]\+\)_\(.*\)|gout_\L\1_clk_\L\1_\2|' \
> > \
> > -e '/^\(DMYQCH\|PCH\|QCH\|QUEUE\)_/d'
>
> Thank you for the updated regex.
>
> > ---
> > drivers/clk/samsung/clk-gs101.c | 558 +++++++++++++++++++++++
> > include/dt-bindings/clock/google,gs101.h | 63 +++
> > 2 files changed, 621 insertions(+)
> >
> > diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c
> > index d065e343a85d..b9f84c7d5c22 100644
> > --- a/drivers/clk/samsung/clk-gs101.c
> > +++ b/drivers/clk/samsung/clk-gs101.c
> > @@ -22,6 +22,7 @@
> > #define CLKS_NR_MISC (CLK_GOUT_MISC_XIU_D_MISC_ACLK + 1)
> > #define CLKS_NR_PERIC0 (CLK_GOUT_PERIC0_SYSREG_PERIC0_PCLK + 1)
> > #define CLKS_NR_PERIC1 (CLK_GOUT_PERIC1_SYSREG_PERIC1_PCLK + 1)
> > +#define CLKS_NR_HSI2 (CLK_GOUT_HSI2_XIU_P_HSI2_ACLK + 1)
> >
> > /* ---- CMU_TOP ------------------------------------------------------------- */
> >
> > @@ -3409,6 +3410,560 @@ static const struct samsung_cmu_info peric1_cmu_info __initconst = {
> > .clk_name = "bus",
> > };
> >
> > +/* ---- CMU_HSI2 ---------------------------------------------------------- */
>
> This comment is shorter that all the other similar comments in this file.
Will fix
>
> > [...]
> > +
> > +PNAME(mout_hsi2_bus_user_p) = { "oscclk", "dout_cmu_hsi2_bus" };
> > +PNAME(mout_hsi2_pcie_user_p) = { "oscclk", "dout_cmu_hsi2_pcie" };
> > +PNAME(mout_hsi2_ufs_embd_user_p) = { "oscclk", "dout_cmu_hsi2_ufs_embd" };
> > +PNAME(mout_hsi2_mmc_card_user_p) = { "oscclk", "dout_cmu_hsi2_mmc_card" };
>
> Can you make these alphabetical, too, please, which would also match their usage
> below:
Will fix
>
> > +
> > +static const struct samsung_mux_clock hsi2_mux_clks[] __initconst = {
> > + MUX(CLK_MOUT_HSI2_BUS_USER, "mout_hsi2_bus_user", mout_hsi2_bus_user_p,
> > + PLL_CON0_MUX_CLKCMU_HSI2_BUS_USER, 4, 1),
> > + MUX(CLK_MOUT_HSI2_MMC_CARD_USER, "mout_hsi2_mmc_card_user",
> > + mout_hsi2_mmc_card_user_p, PLL_CON0_MUX_CLKCMU_HSI2_MMC_CARD_USER,
> > + 4, 1),
> > + MUX(CLK_MOUT_HSI2_PCIE_USER, "mout_hsi2_pcie_user",
> > + mout_hsi2_pcie_user_p, PLL_CON0_MUX_CLKCMU_HSI2_PCIE_USER,
> > + 4, 1),
> > + MUX(CLK_MOUT_HSI2_UFS_EMBD_USER, "mout_hsi2_ufs_embd_user",
> > + mout_hsi2_ufs_embd_user_p, PLL_CON0_MUX_CLKCMU_HSI2_UFS_EMBD_USER,
> > + 4, 1),
> > +};
> > +
> > +static const struct samsung_gate_clock hsi2_gate_clks[] __initconst = {
> > +
>
> Here and below: all these extra empty lines are not needed.
Will fix
>
> > + GATE(CLK_GOUT_HSI2_PCIE_GEN4_1_PCIE_003_PHY_REFCLK_IN,
> > + "gout_hsi2_pcie_gen4_1_pcie_003_phy_refclk_in",
> > + "mout_hsi2_pcie_user",
> > + CLK_CON_GAT_CLK_BLK_HSI2_UID_PCIE_GEN4_1_IPCLKPORT_PCIE_003_PCIE_SUB_CTRL_INST_0_PHY_REFCLK_IN,
> > + 21, 0, 0),
> > +
> > + GATE(CLK_GOUT_HSI2_PCIE_GEN4_1_PCIE_004_PHY_REFCLK_IN,
> > + "gout_hsi2_pcie_gen4_1_pcie_004_phy_refclk_in",
> > + "mout_hsi2_pcie_user",
> > + CLK_CON_GAT_CLK_BLK_HSI2_UID_PCIE_GEN4_1_IPCLKPORT_PCIE_004_PCIE_SUB_CTRL_INST_0_PHY_REFCLK_IN,
> > + 21, 0, 0),
> > +
> > + GATE(CLK_GOUT_HSI2_SSMT_PCIE_IA_GEN4A_1_ACLK,
> > + "gout_hsi2_ssmt_pcie_ia_gen4a_1_aclk",
> > + "mout_hsi2_bus_user",
>
> The two strings fit on the same line.
Will fix
>
> > + CLK_CON_GAT_CLK_BLK_HSI2_UID_SSMT_PCIE_IA_GEN4A_1_IPCLKPORT_ACLK,
> > + 21, 0, 0),
> > +
> > + GATE(CLK_GOUT_HSI2_SSMT_PCIE_IA_GEN4A_1_PCLK,
> > + "gout_hsi2_ssmt_pcie_ia_gen4a_1_pclk",
> > + "mout_hsi2_bus_user",
>
> dito.
Will fix
regards,
Peter
>
> > [...]
> > + /* Disabling this clock makes the system hang. Mark the clock as critical. */
> > + GATE(CLK_GOUT_HSI2_HSI2_CMU_HSI2_PCLK,
> > + "gout_hsi2_hsi2_cmu_hsi2_pclk", "mout_hsi2_bus_user",
> > + CLK_CON_GAT_GOUT_BLK_HSI2_UID_HSI2_CMU_HSI2_IPCLKPORT_PCLK,
> > + 21, CLK_IS_CRITICAL, 0),
>
> I have a similar clock in USB, which also causes a hang if off, I wonder what we
> could do better here.
>
>
> Cheers,
> Andre'
>
next prev parent reply other threads:[~2024-04-23 17:46 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20240404122615epcas5p3812bd7c825bf604fc474bbcdf40d11f6@epcas5p3.samsung.com>
2024-04-04 12:25 ` [PATCH 00/17] HSI2, UFS & UFS phy support for Tensor GS101 Peter Griffin
2024-04-04 12:25 ` [PATCH 01/17] dt-bindings: clock: google,gs101-clock: add HSI2 clock management unit Peter Griffin
2024-04-05 7:15 ` André Draszik
2024-04-05 7:46 ` Krzysztof Kozlowski
2024-04-16 10:52 ` Peter Griffin
2024-04-04 12:25 ` [PATCH 02/17] dt-bindings: soc: google: exynos-sysreg: add dedicated hsi2 sysreg compatible Peter Griffin
2024-04-05 7:19 ` André Draszik
2024-04-10 16:40 ` Rob Herring
2024-04-04 12:25 ` [PATCH 03/17] dt-bindings: ufs: exynos-ufs: Add gs101 compatible Peter Griffin
2024-04-05 7:49 ` Krzysztof Kozlowski
2024-04-16 11:30 ` Peter Griffin
2024-04-04 12:25 ` [PATCH 04/17] dt-bindings: phy: samsung,ufs-phy: Add dedicated gs101-ufs-phy compatible Peter Griffin
2024-04-05 7:50 ` Krzysztof Kozlowski
2024-04-16 11:45 ` Peter Griffin
2024-04-04 12:25 ` [PATCH 05/17] arm64: dts: exynos: gs101: enable cmu-hsi2 clock controller Peter Griffin
2024-04-05 7:38 ` André Draszik
2024-04-16 11:56 ` Peter Griffin
2024-04-16 12:21 ` André Draszik
2024-04-16 14:33 ` Peter Griffin
2024-04-05 7:51 ` Krzysztof Kozlowski
2024-04-04 12:25 ` [PATCH 06/17] arm64: dts: exynos: gs101: Add the hsi2 sysreg node Peter Griffin
2024-04-05 7:33 ` André Draszik
2024-04-16 12:13 ` Peter Griffin
2024-04-04 12:25 ` [PATCH 07/17] arm64: dts: exynos: gs101: Add ufs, ufs-phy and ufs regulator dt nodes Peter Griffin
2024-04-05 7:53 ` Krzysztof Kozlowski
2024-04-18 13:20 ` Peter Griffin
2024-04-18 17:31 ` Krzysztof Kozlowski
2024-04-04 12:25 ` [PATCH 08/17] clk: samsung: gs101: add support for cmu_hsi2 Peter Griffin
2024-04-04 13:24 ` André Draszik
2024-04-22 14:55 ` Peter Griffin
2024-04-04 22:52 ` kernel test robot
2024-04-05 7:23 ` André Draszik
2024-04-05 7:55 ` Krzysztof Kozlowski
2024-04-08 14:49 ` André Draszik
2024-04-23 17:45 ` Peter Griffin [this message]
2024-04-04 12:25 ` [PATCH 09/17] phy: samsung-ufs: use exynos_get_pmu_regmap_by_phandle() to obtain PMU regmap Peter Griffin
2024-04-05 7:04 ` André Draszik
2024-04-22 12:40 ` Peter Griffin
2024-04-10 0:59 ` Alim Akhtar
2024-04-04 12:25 ` [PATCH 10/17] phy: samsung-ufs: ufs: Add SoC callbacks for calibration and clk data recovery Peter Griffin
2024-04-05 7:56 ` Krzysztof Kozlowski
2024-04-17 9:52 ` Dan Carpenter
2024-04-22 12:39 ` Peter Griffin
2024-04-04 12:25 ` [PATCH 11/17] phy: samsung-ufs: ufs: Add support for gs101 UFS phy tuning Peter Griffin
2024-04-05 7:57 ` Krzysztof Kozlowski
2024-04-04 12:25 ` [PATCH 12/17] scsi: ufs: host: ufs-exynos: Add EXYNOS_UFS_OPT_UFSPR_SECURE option Peter Griffin
2024-04-05 7:57 ` Krzysztof Kozlowski
2024-04-09 20:30 ` William McVicker
2024-04-04 12:25 ` [PATCH 13/17] scsi: ufs: host: ufs-exynos: add EXYNOS_UFS_OPT_TIMER_TICK_SELECT option Peter Griffin
2024-04-05 7:58 ` Krzysztof Kozlowski
2024-04-09 20:31 ` William McVicker
2024-04-04 12:25 ` [PATCH 14/17] scsi: ufs: host: ufs-exynos: allow max frequencies up to 267Mhz Peter Griffin
2024-04-05 7:58 ` Krzysztof Kozlowski
2024-04-09 20:32 ` William McVicker
2024-04-04 12:25 ` [PATCH 15/17] scsi: ufs: host: ufs-exynos: add some pa_dbg_ register offsets into drvdata Peter Griffin
2024-04-05 7:59 ` Krzysztof Kozlowski
2024-04-09 20:35 ` William McVicker
2024-04-04 12:25 ` [PATCH 16/17] scsi: ufs: host: ufs-exynos: Add support for Tensor gs101 SoC Peter Griffin
2024-04-05 7:59 ` Krzysztof Kozlowski
2024-04-09 20:36 ` William McVicker
2024-04-04 12:25 ` [PATCH 17/17] MAINTAINERS: Add phy-gs101-ufs file to Tensor GS101 Peter Griffin
2024-04-05 8:00 ` Krzysztof Kozlowski
2024-04-05 7:45 ` [PATCH 00/17] HSI2, UFS & UFS phy support for " Krzysztof Kozlowski
2024-04-16 10:28 ` Peter Griffin
2024-04-06 9:19 ` (subset) " Vinod Koul
2024-04-08 8:30 ` Alim Akhtar
2024-04-16 10:29 ` Peter Griffin
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=CADrjBPrNqJ6FNKZTgVxST1en-hRdyZFmJe42uwerSnDSmgifbg@mail.gmail.com \
--to=peter.griffin@linaro.org \
--cc=alim.akhtar@samsung.com \
--cc=andre.draszik@linaro.org \
--cc=avri.altman@wdc.com \
--cc=bvanassche@acm.org \
--cc=chanho61.park@samsung.com \
--cc=conor+dt@kernel.org \
--cc=cw00.choi@samsung.com \
--cc=devicetree@vger.kernel.org \
--cc=ebiggers@kernel.org \
--cc=jejb@linux.ibm.com \
--cc=kishon@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=mturquette@baylibre.com \
--cc=robh@kernel.org \
--cc=s.nawrocki@samsung.com \
--cc=saravanak@google.com \
--cc=sboyd@kernel.org \
--cc=tudor.ambarus@linaro.org \
--cc=vkoul@kernel.org \
--cc=willmcvicker@google.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 read-only IMAP folder(s) and NNTP newsgroup(s).