Message ID | 20240404122559.898930-1-peter.griffin@linaro.org |
---|---|
Headers | show |
Series | HSI2, UFS & UFS phy support for Tensor GS101 | expand |
On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > This allows us to obtain a PMU regmap that is created by the exynos-pmu > driver. Platforms such as gs101 require exynos-pmu created regmap to > issue SMC calls for PMU register accesses. Existing platforms still get > a MMIO regmap as before. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > drivers/phy/samsung/phy-samsung-ufs.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/phy/samsung/phy-samsung-ufs.c b/drivers/phy/samsung/phy-samsung-ufs.c > index 183c88e3d1ec..c567efafc30f 100644 > --- a/drivers/phy/samsung/phy-samsung-ufs.c > +++ b/drivers/phy/samsung/phy-samsung-ufs.c > @@ -18,6 +18,7 @@ > #include <linux/phy/phy.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > +#include <linux/soc/samsung/exynos-pmu.h> You can now drop the include of linux/mfd/syscon.h Once done, feel free to add Reviewed-by: André Draszik <andre.draszik@linaro.org> > > #include "phy-samsung-ufs.h" > > @@ -255,8 +256,8 @@ static int samsung_ufs_phy_probe(struct platform_device *pdev) > goto out; > } > > - phy->reg_pmu = syscon_regmap_lookup_by_phandle( > - dev->of_node, "samsung,pmu-syscon"); > + phy->reg_pmu = exynos_get_pmu_regmap_by_phandle(dev->of_node, > + "samsung,pmu-syscon"); > if (IS_ERR(phy->reg_pmu)) { > err = PTR_ERR(phy->reg_pmu); > dev_err(dev, "failed syscon remap for pmu\n");
Hi Pete, On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > Add dt schema documentation and clock IDs for the High Speed Interface > 2 (HSI2) clock management unit. This CMU feeds high speed interfaces > such as PCIe and UFS. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > .../bindings/clock/google,gs101-clock.yaml | 30 +++++++++++++++++-- > 1 file changed, 28 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101- > clock.yaml > index 1d2bcea41c85..a202fd5d1ead 100644 > --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > @@ -32,14 +32,15 @@ properties: > - google,gs101-cmu-misc > - google,gs101-cmu-peric0 > - google,gs101-cmu-peric1 > + - google,gs101-cmu-hsi2 Can you keep this alphabetical and add hsi before misc please. > > clocks: > minItems: 1 > - maxItems: 3 > + maxItems: 5 > > clock-names: > minItems: 1 > - maxItems: 3 > + maxItems: 5 > > "#clock-cells": > const: 1 > @@ -112,6 +113,31 @@ allOf: > - const: bus > - const: ip > > + - if: > + properties: > + compatible: > + contains: > + enum: > + - google,gs101-cmu-hsi2 this block should also come before misc please. Once done, feel free to add Reviewed-by: André Draszik <andre.draszik@linaro.org> > + > + then: > + properties: > + clocks: > + items: > + - description: External reference clock (24.576 MHz) > + - description: High Speed Interface bus clock (from CMU_TOP) > + - description: High Speed Interface pcie clock (from CMU_TOP) > + - description: High Speed Interface ufs clock (from CMU_TOP) > + - description: High Speed Interface mmc clock (from CMU_TOP) > + > + clock-names: > + items: > + - const: oscclk > + - const: bus > + - const: pcie > + - const: ufs_embd > + - const: mmc_card > + > additionalProperties: false > > examples:
On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > Enable the cmu_hsi2 clock management unit. It feeds some of > the high speed interfaces such as PCIe and UFS. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > arch/arm64/boot/dts/exynos/google/gs101.dtsi | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > index eddb6b326fde..38ac4fb1397e 100644 > --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi > +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > @@ -1253,6 +1253,18 @@ pinctrl_hsi1: pinctrl@11840000 { > interrupts = <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH 0>; > }; > > + cmu_hsi2: clock-controller@14400000 { > + compatible = "google,gs101-cmu-hsi2"; > + reg = <0x14400000 0x4000>; > + #clock-cells = <1>; > + clocks = <&ext_24_5m>, > + <&cmu_top CLK_DOUT_CMU_HSI2_BUS>, > + <&cmu_top CLK_DOUT_CMU_HSI2_PCIE>, > + <&cmu_top CLK_DOUT_CMU_HSI2_UFS_EMBD>, > + <&cmu_top CLK_DOUT_CMU_HSI2_MMC_CARD>; > + clock-names = "oscclk", "bus", "pcie", "ufs_embd", "mmc_card"; > + }; This doesn't build because you didn't add the clock ids in the binding patch. Other than that, Reviewed-by: André Draszik <andre.draszik@linaro.org> > + > pinctrl_hsi2: pinctrl@14440000 { > compatible = "google,gs101-pinctrl"; > reg = <0x14440000 0x00001000>;
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 05/04/2024 09:15, André Draszik wrote: > Hi Pete, > > On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: >> Add dt schema documentation and clock IDs for the High Speed Interface >> 2 (HSI2) clock management unit. This CMU feeds high speed interfaces >> such as PCIe and UFS. >> >> Signed-off-by: Peter Griffin <peter.griffin@linaro.org> >> --- >> .../bindings/clock/google,gs101-clock.yaml | 30 +++++++++++++++++-- >> 1 file changed, 28 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101- >> clock.yaml >> index 1d2bcea41c85..a202fd5d1ead 100644 >> --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >> +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >> @@ -32,14 +32,15 @@ properties: >> - google,gs101-cmu-misc >> - google,gs101-cmu-peric0 >> - google,gs101-cmu-peric1 >> + - google,gs101-cmu-hsi2 > > Can you keep this alphabetical and add hsi before misc please. >> >> clocks: >> minItems: 1 >> - maxItems: 3 >> + maxItems: 5 >> >> clock-names: >> minItems: 1 >> - maxItems: 3 >> + maxItems: 5 >> >> "#clock-cells": >> const: 1 >> @@ -112,6 +113,31 @@ allOf: >> - const: bus >> - const: ip >> >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - google,gs101-cmu-hsi2 > > this block should also come before misc please. > > Once done, feel free to add Yes, please, ack for both. Best regards, Krzysztof
On 04/04/2024 14:25, Peter Griffin wrote: > Add dedicated google,gs101-ufs compatible for Google Tensor gs101 > SoC. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > .../bindings/ufs/samsung,exynos-ufs.yaml | 51 +++++++++++++++---- > 1 file changed, 42 insertions(+), 9 deletions(-) > > diff --git a/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml b/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > index b2b509b3944d..898da6c0e94f 100644 > --- a/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > +++ b/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > @@ -12,12 +12,10 @@ maintainers: > description: | > Each Samsung UFS host controller instance should have its own node. > > -allOf: > - - $ref: ufs-common.yaml > - > properties: > compatible: > enum: > + - google,gs101-ufs > - samsung,exynos7-ufs > - samsung,exynosautov9-ufs > - samsung,exynosautov9-ufs-vh > @@ -38,14 +36,12 @@ properties: > - const: ufsp > > clocks: > - items: > - - description: ufs link core clock > - - description: unipro main clock > + minItems: 2 > + maxItems: 5 Keep here minItems and: + - description: ufs link core clock + - description: unipro main clock + - description: fmp clock + - description: ufs aclk clock + - description: ufs pclk clock > > clock-names: > - items: > - - const: core_clk > - - const: sclk_unipro_main > + minItems: 2 > + maxItems: 5 Similarly here > > phys: > maxItems: 1 > @@ -72,6 +68,43 @@ required: > - clocks > - clock-names > > +allOf: > + - $ref: ufs-common.yaml > + - if: > + properties: > + compatible: > + contains: > + const: google,gs101-ufs > + > + then: > + properties: > + clocks: Enough is: minItems: 5 > + items: and drop the items since they are defined in top-level. Your original code is correct, but with my approach we keep the list synced between variants, at least part of the list. If another variant appears, then maybe it will go back to your approach, but maybe we can still have the same clocks and their order. > + - description: ufs link core clock > + - description: unipro main clock > + - description: fmp clock > + - description: ufs aclk clock > + - description: ufs pclk clock > + > + clock-names: minItems: 5 > + items: > + - const: core_clk > + - const: sclk_unipro_main > + - const: fmp > + - const: ufs_aclk > + - const: ufs_pclk > + else: > + properties: > + clocks: maxItems: 2 > + items: > + - description: ufs link core clock > + - description: unipro main clock > + > + clock-names: maxItems: 2 > + items: > + - const: core_clk > + - const: sclk_unipro_main > + > unevaluatedProperties: false > > examples: Best regards, Krzysztof
On 04/04/2024 14:25, Peter Griffin wrote: > Enable the cmu_hsi2 clock management unit. It feeds some of > the high speed interfaces such as PCIe and UFS. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > arch/arm64/boot/dts/exynos/google/gs101.dtsi | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > Was it really compiled? Best regards, Krzysztof
On 04/04/2024 14:25, Peter Griffin wrote: > Enable the ufs controller, ufs phy and ufs regulator in device tree. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > .../boot/dts/exynos/google/gs101-oriole.dts | 17 +++++++++ > arch/arm64/boot/dts/exynos/google/gs101.dtsi | 35 +++++++++++++++++++ If you wish you can split DTSI and DTS into separate patches. Up to you. > 2 files changed, 52 insertions(+) > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > index 6be15e990b65..986eb5c9898a 100644 > --- a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > +++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > @@ -53,6 +53,14 @@ button-power { > wakeup-source; > }; > }; > + > + ufs_0_fixed_vcc_reg: regulator-0 { > + compatible = "regulator-fixed"; > + regulator-name = "ufs-vcc"; > + gpio = <&gpp0 1 0>; Use defines for GPIO flags, but more important: are you sure this is not coming from a PMIC? What's the voltage? It looks like a stub for missing PMIC, because UFS voltages are usually provided by PMIC. > + regulator-boot-on; > + enable-active-high; > + }; > }; > > &ext_24_5m { > @@ -106,6 +114,15 @@ &serial_0 { > status = "okay"; > }; > > +&ufs_0 { > + status = "okay"; > + vcc-supply = <&ufs_0_fixed_vcc_reg>; > +}; > + > +&ufs_0_phy { > + status = "okay"; > +}; > + > &usi_uart { > samsung,clkreq-on; /* needed for UART mode */ > status = "okay"; > diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > index 608369cec47b..9c94829bf14c 100644 > --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi > +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > @@ -1277,6 +1277,41 @@ pinctrl_hsi2: pinctrl@14440000 { > interrupts = <GIC_SPI 503 IRQ_TYPE_LEVEL_HIGH 0>; > }; > > + ufs_0_phy: phy@17e04000 { > + compatible = "google,gs101-ufs-phy"; > + reg = <0x14704000 0x3000>; > + reg-names = "phy-pma"; > + samsung,pmu-syscon = <&pmu_system_controller>; > + #phy-cells = <0>; > + clocks = <&ext_24_5m>; > + clock-names = "ref_clk"; > + status = "disabled"; > + }; > + > + ufs_0: ufs@14700000 { > + compatible = "google,gs101-ufs"; > + Drop blank line. > + reg = <0x14700000 0x200>, > + <0x14701100 0x200>, > + <0x14780000 0xa000>, > + <0x14600000 0x100>; Best regards, Krzysztof
On 04/04/2024 14:25, Peter Griffin wrote: > Add the m-phy tuning values for gs101 UFS phy and SoC callbacks > gs101_phy_wait_for_calibration() and gs101_phy_wait_for_cdr_lock(). > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 04/04/2024 14:25, Peter Griffin wrote: > This option is intended to be set for SoCs that have HCI_V2P1_CTRL > register and can select their tick source via IA_TICK_SEL bit. > > Source clock selection for timer tick > 0x0 = Bus clock (aclk) > 0x1 = Function clock (mclk) > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 04/04/2024 14:25, Peter Griffin wrote: > This allows these registers to be at different offsets or not > exist at all on some SoCs variants. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > drivers/ufs/host/ufs-exynos.c | 38 ++++++++++++++++++++++++----------- > drivers/ufs/host/ufs-exynos.h | 6 +++++- > 2 files changed, 31 insertions(+), 13 deletions(-) Acked-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
On 04/04/2024 14:25, Peter Griffin wrote: > Add the newly created ufs phy for GS101 to MAINTAINERS. > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 491d48f7c2fa..48ac9bd64f22 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9256,6 +9256,7 @@ S: Maintained > F: Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > F: arch/arm64/boot/dts/exynos/google/ > F: drivers/clk/samsung/clk-gs101.c > +F: drivers/phy/samsung/phy-gs101-ufs.c This could go also via phy-tree: Acked-by: Krzysztof Kozlowski <krzk@kernel.org> 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é, Thanks for the review feedback. On Fri, 5 Apr 2024 at 08:15, André Draszik <andre.draszik@linaro.org> wrote: > > Hi Pete, > > On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > > Add dt schema documentation and clock IDs for the High Speed Interface > > 2 (HSI2) clock management unit. This CMU feeds high speed interfaces > > such as PCIe and UFS. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > --- > > .../bindings/clock/google,gs101-clock.yaml | 30 +++++++++++++++++-- > > 1 file changed, 28 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101- > > clock.yaml > > index 1d2bcea41c85..a202fd5d1ead 100644 > > --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > > +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > > @@ -32,14 +32,15 @@ properties: > > - google,gs101-cmu-misc > > - google,gs101-cmu-peric0 > > - google,gs101-cmu-peric1 > > + - google,gs101-cmu-hsi2 > > Can you keep this alphabetical and add hsi before misc please. Will fix > > > > clocks: > > minItems: 1 > > - maxItems: 3 > > + maxItems: 5 > > > > clock-names: > > minItems: 1 > > - maxItems: 3 > > + maxItems: 5 > > > > "#clock-cells": > > const: 1 > > @@ -112,6 +113,31 @@ allOf: > > - const: bus > > - const: ip > > > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - google,gs101-cmu-hsi2 > > this block should also come before misc please. Will fix > > Once done, feel free to add > > Reviewed-by: André Draszik <andre.draszik@linaro.org> Thanks! regards, Pete
Hi Krzysztof, Thanks for the review. On Fri, 5 Apr 2024 at 08:49, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 04/04/2024 14:25, Peter Griffin wrote: > > Add dedicated google,gs101-ufs compatible for Google Tensor gs101 > > SoC. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > --- > > .../bindings/ufs/samsung,exynos-ufs.yaml | 51 +++++++++++++++---- > > 1 file changed, 42 insertions(+), 9 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml b/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > > index b2b509b3944d..898da6c0e94f 100644 > > --- a/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > > +++ b/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml > > @@ -12,12 +12,10 @@ maintainers: > > description: | > > Each Samsung UFS host controller instance should have its own node. > > > > -allOf: > > - - $ref: ufs-common.yaml > > - > > properties: > > compatible: > > enum: > > + - google,gs101-ufs > > - samsung,exynos7-ufs > > - samsung,exynosautov9-ufs > > - samsung,exynosautov9-ufs-vh > > @@ -38,14 +36,12 @@ properties: > > - const: ufsp > > > > clocks: > > - items: > > - - description: ufs link core clock > > - - description: unipro main clock > > + minItems: 2 > > + maxItems: 5 > > Keep here minItems and: > > + - description: ufs link core clock > + - description: unipro main clock > + - description: fmp clock > + - description: ufs aclk clock > + - description: ufs pclk clock > > > > > clock-names: > > - items: > > - - const: core_clk > > - - const: sclk_unipro_main > > + minItems: 2 > > + maxItems: 5 > > Similarly here > > > > > phys: > > maxItems: 1 > > @@ -72,6 +68,43 @@ required: > > - clocks > > - clock-names > > > > +allOf: > > + - $ref: ufs-common.yaml > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: google,gs101-ufs > > + > > + then: > > + properties: > > + clocks: > > Enough is: > minItems: 5 > > > + items: > > and drop the items since they are defined in top-level. > > Your original code is correct, but with my approach we keep the list > synced between variants, at least part of the list. If another variant > appears, then maybe it will go back to your approach, but maybe we can > still have the same clocks and their order. Will update like you suggest in v2. Thanks, Peter
Hi André, Thanks for the review. On Fri, 5 Apr 2024 at 08:38, André Draszik <andre.draszik@linaro.org> wrote: > > On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > > Enable the cmu_hsi2 clock management unit. It feeds some of > > the high speed interfaces such as PCIe and UFS. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > --- > > arch/arm64/boot/dts/exynos/google/gs101.dtsi | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > index eddb6b326fde..38ac4fb1397e 100644 > > --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > @@ -1253,6 +1253,18 @@ pinctrl_hsi1: pinctrl@11840000 { > > interrupts = <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH 0>; > > }; > > > > + cmu_hsi2: clock-controller@14400000 { > > + compatible = "google,gs101-cmu-hsi2"; > > + reg = <0x14400000 0x4000>; > > + #clock-cells = <1>; > > + clocks = <&ext_24_5m>, > > + <&cmu_top CLK_DOUT_CMU_HSI2_BUS>, > > + <&cmu_top CLK_DOUT_CMU_HSI2_PCIE>, > > + <&cmu_top CLK_DOUT_CMU_HSI2_UFS_EMBD>, > > + <&cmu_top CLK_DOUT_CMU_HSI2_MMC_CARD>; > > + clock-names = "oscclk", "bus", "pcie", "ufs_embd", "mmc_card"; > > + }; > > This doesn't build because you didn't add the clock ids in the binding patch. These clock IDs are for cmu_top, not cmu_hsi2. They were added as part of the initial gs101/Oriole upstream support series in the following commit commit 0a910f1606384a5886a045e36b1fc80a7fa6706b Author: Peter Griffin <peter.griffin@linaro.org> Date: Sat Dec 9 23:30:48 2023 +0000 dt-bindings: clock: Add Google gs101 clock management unit bindings Provide dt-schema documentation for Google gs101 SoC clock controller. Currently this adds support for cmu_top, cmu_misc and cmu_apm. Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org> Signed-off-by: Peter Griffin <peter.griffin@linaro.org> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Link: https://lore.kernel.org/r/20231209233106.147416-3-peter.griffin@linaro.org Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> regards, Peter
Hi Pete, On Tue, 2024-04-16 at 12:56 +0100, Peter Griffin wrote: > Hi André, > > Thanks for the review. > > On Fri, 5 Apr 2024 at 08:38, André Draszik <andre.draszik@linaro.org> wrote: > > > > On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > > > Enable the cmu_hsi2 clock management unit. It feeds some of > > > the high speed interfaces such as PCIe and UFS. > > > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > > --- > > > arch/arm64/boot/dts/exynos/google/gs101.dtsi | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > > index eddb6b326fde..38ac4fb1397e 100644 > > > --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > > +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > > @@ -1253,6 +1253,18 @@ pinctrl_hsi1: pinctrl@11840000 { > > > interrupts = <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH 0>; > > > }; > > > > > > + cmu_hsi2: clock-controller@14400000 { > > > + compatible = "google,gs101-cmu-hsi2"; > > > + reg = <0x14400000 0x4000>; > > > + #clock-cells = <1>; > > > + clocks = <&ext_24_5m>, > > > + <&cmu_top CLK_DOUT_CMU_HSI2_BUS>, > > > + <&cmu_top CLK_DOUT_CMU_HSI2_PCIE>, > > > + <&cmu_top CLK_DOUT_CMU_HSI2_UFS_EMBD>, > > > + <&cmu_top CLK_DOUT_CMU_HSI2_MMC_CARD>; > > > + clock-names = "oscclk", "bus", "pcie", "ufs_embd", "mmc_card"; > > > + }; > > > > This doesn't build because you didn't add the clock ids in the binding patch. > > These clock IDs are for cmu_top, not cmu_hsi2. Right. I replied to the wrong patch. Sorry for that. It is patch 7 that uses clock ids that are only added in patch 8. The clock ids from patch 8 in include/dt-bindings/clock/google,gs101.h should be added in patch 1 instead. Cheers, Andre'
Hi André, On Tue, 16 Apr 2024 at 13:21, André Draszik <andre.draszik@linaro.org> wrote: > > Hi Pete, > > On Tue, 2024-04-16 at 12:56 +0100, Peter Griffin wrote: > > Hi André, > > > > Thanks for the review. > > > > On Fri, 5 Apr 2024 at 08:38, André Draszik <andre.draszik@linaro.org> wrote: > > > > > > On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > > > > Enable the cmu_hsi2 clock management unit. It feeds some of > > > > the high speed interfaces such as PCIe and UFS. > > > > > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > > > --- > > > > arch/arm64/boot/dts/exynos/google/gs101.dtsi | 12 ++++++++++++ > > > > 1 file changed, 12 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > > > index eddb6b326fde..38ac4fb1397e 100644 > > > > --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > > > +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > > > @@ -1253,6 +1253,18 @@ pinctrl_hsi1: pinctrl@11840000 { > > > > interrupts = <GIC_SPI 471 IRQ_TYPE_LEVEL_HIGH 0>; > > > > }; > > > > > > > > + cmu_hsi2: clock-controller@14400000 { > > > > + compatible = "google,gs101-cmu-hsi2"; > > > > + reg = <0x14400000 0x4000>; > > > > + #clock-cells = <1>; > > > > + clocks = <&ext_24_5m>, > > > > + <&cmu_top CLK_DOUT_CMU_HSI2_BUS>, > > > > + <&cmu_top CLK_DOUT_CMU_HSI2_PCIE>, > > > > + <&cmu_top CLK_DOUT_CMU_HSI2_UFS_EMBD>, > > > > + <&cmu_top CLK_DOUT_CMU_HSI2_MMC_CARD>; > > > > + clock-names = "oscclk", "bus", "pcie", "ufs_embd", "mmc_card"; > > > > + }; > > > > > > This doesn't build because you didn't add the clock ids in the binding patch. > > > > These clock IDs are for cmu_top, not cmu_hsi2. > > Right. I replied to the wrong patch. Sorry for that. It is patch 7 that > uses clock ids that are only added in patch 8. The clock ids from patch 8 > in include/dt-bindings/clock/google,gs101.h should be added in patch 1 > instead. Ah I see, thanks for the clarification. I'll fix that in v2. Thanks, Pete
Hi Krzysztof, Thanks for your review feedback. On Fri, 5 Apr 2024 at 08:53, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 04/04/2024 14:25, Peter Griffin wrote: > > Enable the ufs controller, ufs phy and ufs regulator in device tree. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > --- > > .../boot/dts/exynos/google/gs101-oriole.dts | 17 +++++++++ > > arch/arm64/boot/dts/exynos/google/gs101.dtsi | 35 +++++++++++++++++++ > > If you wish you can split DTSI and DTS into separate patches. Up to you. Thanks for the heads up > > > 2 files changed, 52 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > > index 6be15e990b65..986eb5c9898a 100644 > > --- a/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > > +++ b/arch/arm64/boot/dts/exynos/google/gs101-oriole.dts > > @@ -53,6 +53,14 @@ button-power { > > wakeup-source; > > }; > > }; > > + > > + ufs_0_fixed_vcc_reg: regulator-0 { > > + compatible = "regulator-fixed"; > > + regulator-name = "ufs-vcc"; > > + gpio = <&gpp0 1 0>; > > Use defines for GPIO flags, Will fix in v2 > but more important: are you sure this is not > coming from a PMIC? What's the voltage? It looks like a stub for missing > PMIC, because UFS voltages are usually provided by PMIC. UFS vcc is 1.2v. The gpio signal from gs101 SoC is called BOOTLD0, and it is connected to slave pmic (S2MPG11) UFS_EN signal which is a gpio enabled voltage rail for UFS (LDO8S). The downstream driver code declares the UFS supply as regulator-fixed even though it has a fully featured regulator driver for the pmic, with the LDO8S regulator exposed. Checking the DT for the pmic the min and max volt are different, so using regulator-fixed seems wrong for downstream. s_ldo8_reg: LDO8S { regulator-name = "S2MPG11_LDO8"; regulator-min-microvolt = <1127800>; regulator-max-microvolt = <1278600>; regulator-always-on; regulator-initial-mode = <SEC_OPMODE_SUSPEND>; channel-mux-selection = <0x28>; schematic-name = "L8S_UFS_VCCQ"; subsys-name = "UFS"; }; So I think you're correct this is a stub pending full pmic support. I propose in v2 to add a comment similar to what we have in exynos850-e850-96.dts today above the regulator-fixed node like /* TODO: Remove this once PMIC is implemented */? regards, Peter. > > > + regulator-boot-on; > > + enable-active-high; > > + }; > > }; > > > > &ext_24_5m { > > @@ -106,6 +114,15 @@ &serial_0 { > > status = "okay"; > > }; > > > > +&ufs_0 { > > + status = "okay"; > > + vcc-supply = <&ufs_0_fixed_vcc_reg>; > > +}; > > + > > +&ufs_0_phy { > > + status = "okay"; > > +}; > > + > > &usi_uart { > > samsung,clkreq-on; /* needed for UART mode */ > > status = "okay"; > > diff --git a/arch/arm64/boot/dts/exynos/google/gs101.dtsi b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > index 608369cec47b..9c94829bf14c 100644 > > --- a/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > +++ b/arch/arm64/boot/dts/exynos/google/gs101.dtsi > > @@ -1277,6 +1277,41 @@ pinctrl_hsi2: pinctrl@14440000 { > > interrupts = <GIC_SPI 503 IRQ_TYPE_LEVEL_HIGH 0>; > > }; > > > > + ufs_0_phy: phy@17e04000 { > > + compatible = "google,gs101-ufs-phy"; > > + reg = <0x14704000 0x3000>; > > + reg-names = "phy-pma"; > > + samsung,pmu-syscon = <&pmu_system_controller>; > > + #phy-cells = <0>; > > + clocks = <&ext_24_5m>; > > + clock-names = "ref_clk"; > > + status = "disabled"; > > + }; > > + > > + ufs_0: ufs@14700000 { > > + compatible = "google,gs101-ufs"; > > + > > Drop blank line. > > > + reg = <0x14700000 0x200>, > > + <0x14701100 0x200>, > > + <0x14780000 0xa000>, > > + <0x14600000 0x100>; > > > Best regards, > Krzysztof >
On 18/04/2024 15:20, Peter Griffin wrote: > > s_ldo8_reg: LDO8S { > regulator-name = "S2MPG11_LDO8"; > regulator-min-microvolt = <1127800>; > regulator-max-microvolt = <1278600>; > regulator-always-on; > regulator-initial-mode = <SEC_OPMODE_SUSPEND>; > channel-mux-selection = <0x28>; > schematic-name = "L8S_UFS_VCCQ"; > subsys-name = "UFS"; > }; > > So I think you're correct this is a stub pending full pmic support. I > propose in v2 to add a comment similar to what we have in > exynos850-e850-96.dts today above the regulator-fixed node like /* > TODO: Remove this once PMIC is implemented */? > Sounds good. Best regards, Krzysztof
Hi André, Thanks for the review feedback. On Fri, 5 Apr 2024 at 08:04, André Draszik <andre.draszik@linaro.org> wrote: > > On Thu, 2024-04-04 at 13:25 +0100, Peter Griffin wrote: > > This allows us to obtain a PMU regmap that is created by the exynos-pmu > > driver. Platforms such as gs101 require exynos-pmu created regmap to > > issue SMC calls for PMU register accesses. Existing platforms still get > > a MMIO regmap as before. > > > > Signed-off-by: Peter Griffin <peter.griffin@linaro.org> > > --- > > drivers/phy/samsung/phy-samsung-ufs.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/phy/samsung/phy-samsung-ufs.c b/drivers/phy/samsung/phy-samsung-ufs.c > > index 183c88e3d1ec..c567efafc30f 100644 > > --- a/drivers/phy/samsung/phy-samsung-ufs.c > > +++ b/drivers/phy/samsung/phy-samsung-ufs.c > > @@ -18,6 +18,7 @@ > > #include <linux/phy/phy.h> > > #include <linux/platform_device.h> > > #include <linux/regmap.h> > > +#include <linux/soc/samsung/exynos-pmu.h> > > You can now drop the include of linux/mfd/syscon.h I'll send a followup patch for this. Thanks, Peter