Message ID | 20240404122559.898930-1-peter.griffin@linaro.org (mailing list archive) |
---|---|
Headers | show |
Series | HSI2, UFS & UFS phy support for Tensor GS101 | expand |
Hi Pete, Thanks for this! I haven't reviewed this, but one immediate comment... On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > [...] > 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) Can you please keep the #defines alphabetical (hsi before misc). > > /* ---- CMU_TOP ------------------------------------------------------------- */ > > @@ -3409,6 +3410,560 @@ static const struct samsung_cmu_info peric1_cmu_info __initconst = { > .clk_name = "bus", > }; > > +/* ---- CMU_HSI2 ---------------------------------------------------------- */ and this code block should be earlier in the file > [..] > static int __init gs101_cmu_probe(struct platform_device *pdev) > @@ -3432,6 +3987,9 @@ static const struct of_device_id gs101_cmu_of_match[] = { > }, { > .compatible = "google,gs101-cmu-peric1", > .data = &peric1_cmu_info, > + }, { > + .compatible = "google,gs101-cmu-hsi2", > + .data = &hsi2_cmu_info, > }, { and this block should move up > }, > }; > diff --git a/include/dt-bindings/clock/google,gs101.h b/include/dt-bindings/clock/google,gs101.h > index 3dac3577788a..ac239ce6821b 100644 > --- a/include/dt-bindings/clock/google,gs101.h > +++ b/include/dt-bindings/clock/google,gs101.h > @@ -518,4 +518,67 @@ > #define CLK_GOUT_PERIC1_CLK_PERIC1_USI9_USI_CLK 45 > #define CLK_GOUT_PERIC1_SYSREG_PERIC1_PCLK 46 > > +/* CMU_HSI2 */ and all these defines, too. Cheers, Andre'
On 04/04/2024 14:25, Peter Griffin wrote: > Hi folks, > > This series adds support for the High Speed Interface (HSI) 2 clock > management unit, UFS controller and UFS phy calibration/tuning for GS101. > > With this series applied, UFS is now functional! The SKhynix HN8T05BZGKX015 > can be enumerated, partitions mounted etc. This then allows us to move away > from the initramfs rootfs we have been using for development so far. > > The intention is this series will be merged via Krzysztofs Samsung Exynos > tree(s). This series is rebased on next-20240404. > > The series is broadly split into the following parts: > 1) dt-bindings documentation updates > 2) gs101 device tree updates > 3) Prepatory patches for samsung-ufs driver > 4) GS101 ufs-phy support > 5) Prepatory patches for ufs-exynos driver > 6) GS101 ufs-exynos support UFS phy and host should go through their trees. What is the reason/need/requirement to put it into this patchset and merge via Samsung SoC tree? Best regards, Krzysztof
On Thu, 04 Apr 2024 13:25:42 +0100, Peter Griffin wrote: > This series adds support for the High Speed Interface (HSI) 2 clock > management unit, UFS controller and UFS phy calibration/tuning for GS101. > > With this series applied, UFS is now functional! The SKhynix HN8T05BZGKX015 > can be enumerated, partitions mounted etc. This then allows us to move away > from the initramfs rootfs we have been using for development so far. > > [...] Applied, thanks! [04/17] dt-bindings: phy: samsung,ufs-phy: Add dedicated gs101-ufs-phy compatible commit: 724e4fc053fe217d0ed477517ae68db11feab1f5 [09/17] phy: samsung-ufs: use exynos_get_pmu_regmap_by_phandle() to obtain PMU regmap commit: f2c6d0fa197a1558f4ef50162bb87e6644af232d [10/17] phy: samsung-ufs: ufs: Add SoC callbacks for calibration and clk data recovery commit: a4de58a9096b471f9dc1c2bc6bfaa8aa48110c31 [11/17] phy: samsung-ufs: ufs: Add support for gs101 UFS phy tuning commit: c1cf725db1065153459f0deb69bd4d497a5fd183 [17/17] MAINTAINERS: Add phy-gs101-ufs file to Tensor GS101. commit: 0338e1d2f933a4ec7ae96ed1f40c39b899e357d7 Best regards,
Hi Peter > -----Original Message----- > From: Peter Griffin <peter.griffin@linaro.org> > Sent: Thursday, April 4, 2024 5:56 PM > To: 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 > Cc: 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; > andre.draszik@linaro.org; saravanak@google.com; > willmcvicker@google.com; Peter Griffin <peter.griffin@linaro.org> > Subject: [PATCH 00/17] HSI2, UFS & UFS phy support for Tensor GS101 > > Hi folks, > > > Question > ======== > > Currently the link comes up in Gear 3 due to ufshcd_init_host_params() > host_params initialisation. If I update that to use UFS_HS_G4 for negotiation > then the link come up in Gear 4. I propose (in a future patch) to use VER > register offset 0x8 to determine whether to set G4 capability or not (if major > version is >= 3). > > The bitfield of VER register in gs101 docs is > > RSVD [31:16] Reserved > MJR [15:8] Major version number > MNR [7:4] Minor version number > VS [3:0] Version Suffix > > Can anyone confirm if other Exynos platforms supported by this driver have > the same register, and if it conforms to the bitfield described above? > VER (offset 0x8) is standard UFS HCI spec, so all vendor need to have this (unless something really wrong with the HW) Yes, Exynos and FSD SoC has these bitfield implemented. > > 2.44.0.478.gd926399ef9-goog
Hi Krzysztof, On Fri, 5 Apr 2024 at 08:45, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 04/04/2024 14:25, Peter Griffin wrote: > > Hi folks, > > > > This series adds support for the High Speed Interface (HSI) 2 clock > > management unit, UFS controller and UFS phy calibration/tuning for GS101. > > > > With this series applied, UFS is now functional! The SKhynix HN8T05BZGKX015 > > can be enumerated, partitions mounted etc. This then allows us to move away > > from the initramfs rootfs we have been using for development so far. > > > > The intention is this series will be merged via Krzysztofs Samsung Exynos > > tree(s). This series is rebased on next-20240404. > > > > The series is broadly split into the following parts: > > 1) dt-bindings documentation updates > > 2) gs101 device tree updates > > 3) Prepatory patches for samsung-ufs driver > > 4) GS101 ufs-phy support > > 5) Prepatory patches for ufs-exynos driver > > 6) GS101 ufs-exynos support > > UFS phy and host should go through their trees. What is the > reason/need/requirement to put it into this patchset and merge via > Samsung SoC tree? Good point, there is no requirement for it to all go via Samsung SoC tree I will remove that from the cover letter in future versions. I see Vinod has already queued the phy parts of the series which is great :-) regards, Peter
Hi Alim, Thanks for your review feedback. On Mon, 8 Apr 2024 at 09:30, Alim Akhtar <alim.akhtar@samsung.com> wrote: > > Hi Peter > > > -----Original Message----- > > From: Peter Griffin <peter.griffin@linaro.org> > > Sent: Thursday, April 4, 2024 5:56 PM > > To: 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 > > Cc: 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; > > andre.draszik@linaro.org; saravanak@google.com; > > willmcvicker@google.com; Peter Griffin <peter.griffin@linaro.org> > > Subject: [PATCH 00/17] HSI2, UFS & UFS phy support for Tensor GS101 > > > > Hi folks, > > > > > > Question > > ======== > > > > Currently the link comes up in Gear 3 due to ufshcd_init_host_params() > > host_params initialisation. If I update that to use UFS_HS_G4 for > negotiation > > then the link come up in Gear 4. I propose (in a future patch) to use VER > > register offset 0x8 to determine whether to set G4 capability or not (if > major > > version is >= 3). > > > > The bitfield of VER register in gs101 docs is > > > > RSVD [31:16] Reserved > > MJR [15:8] Major version number > > MNR [7:4] Minor version number > > VS [3:0] Version Suffix > > > > Can anyone confirm if other Exynos platforms supported by this driver have > > the same register, and if it conforms to the bitfield described above? > > > > VER (offset 0x8) is standard UFS HCI spec, so all vendor need to have this > (unless something really wrong with the HW) > Yes, Exynos and FSD SoC has these bitfield implemented. Thanks for confirming! I will look to propose something once this series is merged. Peter.
Hi André, On Thu, 4 Apr 2024 at 14:24, André Draszik <andre.draszik@linaro.org> wrote: > > Hi Pete, > > Thanks for this! > > I haven't reviewed this, but one immediate comment... > > On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > > [...] > > 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) > > Can you please keep the #defines alphabetical (hsi before misc). Will fix > > > > > /* ---- CMU_TOP ------------------------------------------------------------- */ > > > > @@ -3409,6 +3410,560 @@ static const struct samsung_cmu_info peric1_cmu_info __initconst = { > > .clk_name = "bus", > > }; > > > > +/* ---- CMU_HSI2 ---------------------------------------------------------- */ > > and this code block should be earlier in the file Will fix > > > [..] > > > static int __init gs101_cmu_probe(struct platform_device *pdev) > > @@ -3432,6 +3987,9 @@ static const struct of_device_id gs101_cmu_of_match[] = { > > }, { > > .compatible = "google,gs101-cmu-peric1", > > .data = &peric1_cmu_info, > > + }, { > > + .compatible = "google,gs101-cmu-hsi2", > > + .data = &hsi2_cmu_info, > > }, { > > and this block should move up Will fix > > > }, > > }; > > diff --git a/include/dt-bindings/clock/google,gs101.h b/include/dt-bindings/clock/google,gs101.h > > index 3dac3577788a..ac239ce6821b 100644 > > --- a/include/dt-bindings/clock/google,gs101.h > > +++ b/include/dt-bindings/clock/google,gs101.h > > @@ -518,4 +518,67 @@ > > #define CLK_GOUT_PERIC1_CLK_PERIC1_USI9_USI_CLK 45 > > #define CLK_GOUT_PERIC1_SYSREG_PERIC1_PCLK 46 > > > > +/* CMU_HSI2 */ > > and all these defines, too. Will fix. regards, Peter