mbox series

[00/17] HSI2, UFS & UFS phy support for Tensor GS101

Message ID 20240404122559.898930-1-peter.griffin@linaro.org
Headers show
Series HSI2, UFS & UFS phy support for Tensor GS101 | expand

Message

Peter Griffin April 4, 2024, 12:25 p.m. UTC
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

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?

I'm also open to suggestions on how else to detect and set G4 if others
have a better idea. It looks like MTK and QCOM drivers both use a version
field, hence the proposal above.

fyi I'm out of office until Monday 12th April, so I will deal with any
review feedback upon my return :-)

For anyone wishing to try out the upstream kernel on their Pixel 6 device
you can find the README on how to build / flash the kernel here
https://git.codelinaro.org/linaro/googlelt/pixelscripts

kind regards,

Peter

Peter Griffin (17):
  dt-bindings: clock: google,gs101-clock:  add HSI2 clock management
    unit
  dt-bindings: soc: google: exynos-sysreg: add dedicated hsi2 sysreg
    compatible
  dt-bindings: ufs: exynos-ufs: Add gs101 compatible
  dt-bindings: phy: samsung,ufs-phy: Add dedicated gs101-ufs-phy
    compatible
  arm64: dts: exynos: gs101: enable cmu-hsi2 clock controller
  arm64: dts: exynos: gs101: Add the hsi2 sysreg node
  arm64: dts: exynos: gs101: Add ufs, ufs-phy and ufs regulator dt nodes
  clk: samsung: gs101: add support for cmu_hsi2
  phy: samsung-ufs: use exynos_get_pmu_regmap_by_phandle() to obtain PMU
    regmap
  phy: samsung-ufs: ufs: Add SoC callbacks for calibration and clk data
    recovery
  phy: samsung-ufs: ufs: Add support for gs101 UFS phy tuning
  scsi: ufs: host: ufs-exynos: Add EXYNOS_UFS_OPT_UFSPR_SECURE option
  scsi: ufs: host: ufs-exynos: add EXYNOS_UFS_OPT_TIMER_TICK_SELECT
    option
  scsi: ufs: host: ufs-exynos: allow max frequencies up to 267Mhz
  scsi: ufs: host: ufs-exynos: add some pa_dbg_ register offsets into
    drvdata
  scsi: ufs: host: ufs-exynos: Add support for Tensor gs101 SoC
  MAINTAINERS: Add phy-gs101-ufs file to Tensor GS101.

 .../bindings/clock/google,gs101-clock.yaml    |  30 +-
 .../bindings/phy/samsung,ufs-phy.yaml         |   1 +
 .../soc/samsung/samsung,exynos-sysreg.yaml    |   2 +
 .../bindings/ufs/samsung,exynos-ufs.yaml      |  51 +-
 MAINTAINERS                                   |   1 +
 .../boot/dts/exynos/google/gs101-oriole.dts   |  17 +
 arch/arm64/boot/dts/exynos/google/gs101.dtsi  |  53 ++
 drivers/clk/samsung/clk-gs101.c               | 558 ++++++++++++++++++
 drivers/phy/samsung/Makefile                  |   1 +
 drivers/phy/samsung/phy-exynos7-ufs.c         |   1 +
 drivers/phy/samsung/phy-exynosautov9-ufs.c    |   1 +
 drivers/phy/samsung/phy-fsd-ufs.c             |   1 +
 drivers/phy/samsung/phy-gs101-ufs.c           | 182 ++++++
 drivers/phy/samsung/phy-samsung-ufs.c         |  21 +-
 drivers/phy/samsung/phy-samsung-ufs.h         |   6 +
 drivers/ufs/host/ufs-exynos.c                 | 197 ++++++-
 drivers/ufs/host/ufs-exynos.h                 |  24 +-
 include/dt-bindings/clock/google,gs101.h      |  63 ++
 18 files changed, 1179 insertions(+), 31 deletions(-)
 create mode 100644 drivers/phy/samsung/phy-gs101-ufs.c

Comments

André Draszik April 5, 2024, 7:04 a.m. UTC | #1
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");
André Draszik April 5, 2024, 7:15 a.m. UTC | #2
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:
André Draszik April 5, 2024, 7:38 a.m. UTC | #3
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>;
Krzysztof Kozlowski April 5, 2024, 7:45 a.m. UTC | #4
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
Krzysztof Kozlowski April 5, 2024, 7:46 a.m. UTC | #5
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
Krzysztof Kozlowski April 5, 2024, 7:49 a.m. UTC | #6
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
Krzysztof Kozlowski April 5, 2024, 7:51 a.m. UTC | #7
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
Krzysztof Kozlowski April 5, 2024, 7:53 a.m. UTC | #8
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
Krzysztof Kozlowski April 5, 2024, 7:57 a.m. UTC | #9
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
Krzysztof Kozlowski April 5, 2024, 7:58 a.m. UTC | #10
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
Krzysztof Kozlowski April 5, 2024, 7:59 a.m. UTC | #11
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
Krzysztof Kozlowski April 5, 2024, 8 a.m. UTC | #12
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
Vinod Koul April 6, 2024, 9:19 a.m. UTC | #13
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,
Alim Akhtar April 8, 2024, 8:30 a.m. UTC | #14
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
Peter Griffin April 16, 2024, 10:28 a.m. UTC | #15
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
Peter Griffin April 16, 2024, 10:29 a.m. UTC | #16
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.
Peter Griffin April 16, 2024, 10:52 a.m. UTC | #17
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
Peter Griffin April 16, 2024, 11:30 a.m. UTC | #18
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
Peter Griffin April 16, 2024, 11:56 a.m. UTC | #19
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
André Draszik April 16, 2024, 12:21 p.m. UTC | #20
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'
Peter Griffin April 16, 2024, 2:33 p.m. UTC | #21
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
Peter Griffin April 18, 2024, 1:20 p.m. UTC | #22
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
>
Krzysztof Kozlowski April 18, 2024, 5:31 p.m. UTC | #23
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
Peter Griffin April 22, 2024, 12:40 p.m. UTC | #24
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