[PATCH 0/2] Support exynosauto ufs phy driver

[PATCH 0/2] Support exynosauto ufs phy driver

[Chanho Park]

This patchset introduces ExynosAuto SoC ufs phy driver as Generic PHY
driver framework. Unlike current exynos7 ufs phy driver, this uses 0x50
for offset of PHY_TRSV_CH_OFFSET macro.

Chanho Park (2):
  phy: samsung-ufs: custom PHY_TRSV_CH_OFFSET support
  phy: samsung-ufs: support exynosauto ufs phy driver

 .../bindings/phy/samsung,ufs-phy.yaml         |  1 +
 drivers/phy/samsung/phy-exynosauto-ufs.h      | 70 +++++++++++++++++++
 drivers/phy/samsung/phy-samsung-ufs.c         |  3 +
 drivers/phy/samsung/phy-samsung-ufs.h         |  8 ++-
 4 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100644 drivers/phy/samsung/phy-exynosauto-ufs.h

-- 
2.32.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 1/2] phy: samsung-ufs: custom PHY_TRSV_CH_OFFSET support

[Chanho Park]

To support custom PHY_TSRV_CH_OFFSET for ExynosAuto v9 series SoC,
we can add PHY_TRSV_REG_CFG_OFFSET macro which has an offset field and
let it be used from PHY_TRSV_REG_CFG with the default value.

Signed-off-by: Chanho Park <chanho61.park@samsung.com>
---
 drivers/phy/samsung/phy-samsung-ufs.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/samsung/phy-samsung-ufs.h b/drivers/phy/samsung/phy-samsung-ufs.h
index 5de78710524c..ea1ac38889c1 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.h
+++ b/drivers/phy/samsung/phy-samsung-ufs.h
@@ -24,14 +24,17 @@
 	.id = PHY_COMN_BLK,	\
 }
 
-#define PHY_TRSV_REG_CFG(o, v, d) {	\
+#define PHY_TRSV_REG_CFG_OFFSET(o, v, d, c) {	\
 	.off_0 = PHY_APB_ADDR((o)),	\
-	.off_1 = PHY_APB_ADDR((o) + PHY_TRSV_CH_OFFSET),	\
+	.off_1 = PHY_APB_ADDR((o) + (c)),	\
 	.val = (v),		\
 	.desc = (d),		\
 	.id = PHY_TRSV_BLK,	\
 }
 
+#define PHY_TRSV_REG_CFG(o, v, d)	\
+	PHY_TRSV_REG_CFG_OFFSET(o, v, d, PHY_TRSV_CH_OFFSET)
+
 /* UFS PHY registers */
 #define PHY_PLL_LOCK_STATUS	0x1e
 #define PHY_CDR_LOCK_STATUS	0x5e
-- 
2.32.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 2/2] phy: samsung-ufs: support exynosauto ufs phy driver

[Chanho Park]

This patch adds to support phy-exynosauto-ufs driver for ExynosAuto v9
series SoCs. The patch adds "samsung,exynosauto-ufs-phy" compatible.
Unlike previous exynos ufs phy, the chip uses 0x50 offset as
PHY_TRSV_REG_CFG_OFFSET.

Change-Id: I4ea333d4565af537c26a8876d1b27144eea3c3c6
Signed-off-by: Chanho Park <chanho61.park@samsung.com>
---
 .../bindings/phy/samsung,ufs-phy.yaml         |  1 +
 drivers/phy/samsung/phy-exynosauto-ufs.h      | 70 +++++++++++++++++++
 drivers/phy/samsung/phy-samsung-ufs.c         |  3 +
 drivers/phy/samsung/phy-samsung-ufs.h         |  1 +
 4 files changed, 75 insertions(+)
 create mode 100644 drivers/phy/samsung/phy-exynosauto-ufs.h

diff --git a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
index 636cc501b54f..8e13f8cec6a7 100644
--- a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
@@ -16,6 +16,7 @@ properties:
   compatible:
     enum:
       - samsung,exynos7-ufs-phy
+      - samsung,exynosauto-ufs-phy
 
   reg:
     maxItems: 1
diff --git a/drivers/phy/samsung/phy-exynosauto-ufs.h b/drivers/phy/samsung/phy-exynosauto-ufs.h
new file mode 100644
index 000000000000..3e7d7bb7f0e6
--- /dev/null
+++ b/drivers/phy/samsung/phy-exynosauto-ufs.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * UFS PHY driver data for Samsung EXYNOSAUTO SoC
+ *
+ * Copyright (C) 2021 Samsung Electronics Co., Ltd.
+ */
+#ifndef _PHY_EXYNOSAUTO_UFS_H_
+#define _PHY_EXYNOSAUTO_UFS_H_
+
+#include "phy-samsung-ufs.h"
+
+#define EXYNOSAUTO_EMBEDDED_COMBO_PHY_CTRL	0x728
+#define EXYNOSAUTO_EMBEDDED_COMBO_PHY_CTRL_MASK	0x1
+#define EXYNOSAUTO_EMBEDDED_COMBO_PHY_CTRL_EN	BIT(0)
+
+#define PHY_TRSV_REG_CFG_AUTO(o, v, d)	PHY_TRSV_REG_CFG_OFFSET(o, v, d, 0x50)
+
+/* Calibration for phy initialization */
+static const struct samsung_ufs_phy_cfg exynosauto_pre_init_cfg[] = {
+	PHY_COMN_REG_CFG(0x023, 0x80, PWR_MODE_ANY),
+	PHY_COMN_REG_CFG(0x01d, 0x10, PWR_MODE_ANY),
+
+	PHY_TRSV_REG_CFG_AUTO(0x044, 0xb5, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_AUTO(0x04d, 0x43, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_AUTO(0x05b, 0x20, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_AUTO(0x05e, 0xc0, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_AUTO(0x038, 0x12, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_AUTO(0x059, 0x58, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_AUTO(0x06c, 0x18, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG_AUTO(0x06d, 0x02, PWR_MODE_ANY),
+
+	PHY_COMN_REG_CFG(0x023, 0xc0, PWR_MODE_ANY),
+	PHY_COMN_REG_CFG(0x023, 0x00, PWR_MODE_ANY),
+
+	PHY_TRSV_REG_CFG(0x042, 0x5d, PWR_MODE_ANY),
+	PHY_TRSV_REG_CFG(0x043, 0x80, PWR_MODE_ANY),
+
+	END_UFS_PHY_CFG,
+};
+
+/* Calibration for HS mode series A/B */
+static const struct samsung_ufs_phy_cfg exynosauto_pre_pwr_hs_cfg[] = {
+	PHY_TRSV_REG_CFG(0x032, 0xbc, PWR_MODE_HS_ANY),
+	PHY_TRSV_REG_CFG(0x03c, 0x7f, PWR_MODE_HS_ANY),
+	PHY_TRSV_REG_CFG(0x048, 0xc0, PWR_MODE_HS_ANY),
+
+	PHY_TRSV_REG_CFG(0x04a, 0x00, PWR_MODE_HS_G3_SER_B),
+	PHY_TRSV_REG_CFG(0x04b, 0x10, PWR_MODE_HS_G1_SER_B |
+				      PWR_MODE_HS_G3_SER_B),
+	PHY_TRSV_REG_CFG(0x04d, 0x63, PWR_MODE_HS_G3_SER_B),
+
+	END_UFS_PHY_CFG,
+};
+
+static const struct samsung_ufs_phy_cfg *exynosauto_ufs_phy_cfgs[CFG_TAG_MAX] = {
+	[CFG_PRE_INIT]		= exynosauto_pre_init_cfg,
+	[CFG_PRE_PWR_HS]	= exynosauto_pre_pwr_hs_cfg,
+};
+
+static struct samsung_ufs_phy_drvdata exynosauto_ufs_phy = {
+	.cfg = exynosauto_ufs_phy_cfgs,
+	.isol = {
+		.offset = EXYNOSAUTO_EMBEDDED_COMBO_PHY_CTRL,
+		.mask = EXYNOSAUTO_EMBEDDED_COMBO_PHY_CTRL_MASK,
+		.en = EXYNOSAUTO_EMBEDDED_COMBO_PHY_CTRL_EN,
+	},
+	.has_symbol_clk = 0,
+};
+
+#endif /* _PHY_EXYNOSAUTO_UFS_H_ */
diff --git a/drivers/phy/samsung/phy-samsung-ufs.c b/drivers/phy/samsung/phy-samsung-ufs.c
index dd9ab1519d83..ae58db9228b8 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.c
+++ b/drivers/phy/samsung/phy-samsung-ufs.c
@@ -347,6 +347,9 @@ static const struct of_device_id samsung_ufs_phy_match[] = {
 	{
 		.compatible = "samsung,exynos7-ufs-phy",
 		.data = &exynos7_ufs_phy,
+	}, {
+		.compatible = "samsung,exynosauto-ufs-phy",
+		.data = &exynosauto_ufs_phy,
 	},
 	{},
 };
diff --git a/drivers/phy/samsung/phy-samsung-ufs.h b/drivers/phy/samsung/phy-samsung-ufs.h
index ea1ac38889c1..357876f541b0 100644
--- a/drivers/phy/samsung/phy-samsung-ufs.h
+++ b/drivers/phy/samsung/phy-samsung-ufs.h
@@ -138,5 +138,6 @@ static inline void samsung_ufs_phy_ctrl_isol(
 }
 
 #include "phy-exynos7-ufs.h"
+#include "phy-exynosauto-ufs.h"
 
 #endif /* _PHY_SAMSUNG_UFS_ */
-- 
2.32.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 1/2] phy: samsung-ufs: custom PHY_TRSV_CH_OFFSET support

[Krzysztof Kozlowski]

On 05/07/2021 12:23, Chanho Park wrote:
> To support custom PHY_TSRV_CH_OFFSET for ExynosAuto v9 series SoC,
> we can add PHY_TRSV_REG_CFG_OFFSET macro which has an offset field and
> let it be used from PHY_TRSV_REG_CFG with the default value.
> 
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  drivers/phy/samsung/phy-samsung-ufs.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Hi Chanho,

Thanks for the patches. I think this should be squashed into your next
patch - it does not make much sense as standalone commit.


Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 2/2] phy: samsung-ufs: support exynosauto ufs phy driver

[Krzysztof Kozlowski]

On 05/07/2021 12:23, Chanho Park wrote:
> This patch adds to support phy-exynosauto-ufs driver for ExynosAuto v9
> series SoCs. The patch adds "samsung,exynosauto-ufs-phy" compatible.
> Unlike previous exynos ufs phy, the chip uses 0x50 offset as
> PHY_TRSV_REG_CFG_OFFSET.
> 
> Change-Id: I4ea333d4565af537c26a8876d1b27144eea3c3c6
> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
> ---
>  .../bindings/phy/samsung,ufs-phy.yaml         |  1 +
>  drivers/phy/samsung/phy-exynosauto-ufs.h      | 70 +++++++++++++++++++
>  drivers/phy/samsung/phy-samsung-ufs.c         |  3 +
>  drivers/phy/samsung/phy-samsung-ufs.h         |  1 +
>  4 files changed, 75 insertions(+)
>  create mode 100644 drivers/phy/samsung/phy-exynosauto-ufs.h
> 
> diff --git a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> index 636cc501b54f..8e13f8cec6a7 100644
> --- a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
> @@ -16,6 +16,7 @@ properties:
>    compatible:
>      enum:
>        - samsung,exynos7-ufs-phy
> +      - samsung,exynosauto-ufs-phy

I would prefer real product ID (taken from registers or vendor kernel),
not a marketing name "Auto" because it's too generic, unspecified, too
much as wildcard.

If vendor kernel does not define name or product ID is also not
specific, then it should include a version - so "autov9".

>  
>    reg:
>      maxItems: 1
> diff --git a/drivers/phy/samsung/phy-exynosauto-ufs.h b/drivers/phy/samsung/phy-exynosauto-ufs.h
> new file mode 100644
> index 000000000000..3e7d7bb7f0e6
> --- /dev/null
> +++ b/drivers/phy/samsung/phy-exynosauto-ufs.h
> @@ -0,0 +1,70 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * UFS PHY driver data for Samsung EXYNOSAUTO SoC
> + *
> + * Copyright (C) 2021 Samsung Electronics Co., Ltd.
> + */
> +#ifndef _PHY_EXYNOSAUTO_UFS_H_
> +#define _PHY_EXYNOSAUTO_UFS_H_
> +
> +#include "phy-samsung-ufs.h"
> +
> +#define EXYNOSAUTO_EMBEDDED_COMBO_PHY_CTRL	0x728
> +#define EXYNOSAUTO_EMBEDDED_COMBO_PHY_CTRL_MASK	0x1
> +#define EXYNOSAUTO_EMBEDDED_COMBO_PHY_CTRL_EN	BIT(0)
> +
> +#define PHY_TRSV_REG_CFG_AUTO(o, v, d)	PHY_TRSV_REG_CFG_OFFSET(o, v, d, 0x50)
> +
> +/* Calibration for phy initialization */
> +static const struct samsung_ufs_phy_cfg exynosauto_pre_init_cfg[] = {

I know that Exynos7 UFS did it but it was wrong. Headers should not
define structures. What if a header needs to be included in different
objects/units?

This should be probably a separate file (C file). Something like we did
for Exynos PMU (drivers/soc/samsung/exynos-pmu.h).


Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 2/2] phy: samsung-ufs: support exynosauto ufs phy driver

[Krzysztof Kozlowski]

On 05/07/2021 14:29, Krzysztof Kozlowski wrote:
> On 05/07/2021 12:23, Chanho Park wrote:
>> This patch adds to support phy-exynosauto-ufs driver for ExynosAuto v9
>> series SoCs. The patch adds "samsung,exynosauto-ufs-phy" compatible.
>> Unlike previous exynos ufs phy, the chip uses 0x50 offset as
>> PHY_TRSV_REG_CFG_OFFSET.
>>
>> Change-Id: I4ea333d4565af537c26a8876d1b27144eea3c3c6
>> Signed-off-by: Chanho Park <chanho61.park@samsung.com>
>> ---
>>  .../bindings/phy/samsung,ufs-phy.yaml         |  1 +
>>  drivers/phy/samsung/phy-exynosauto-ufs.h      | 70 +++++++++++++++++++
>>  drivers/phy/samsung/phy-samsung-ufs.c         |  3 +
>>  drivers/phy/samsung/phy-samsung-ufs.h         |  1 +
>>  4 files changed, 75 insertions(+)
>>  create mode 100644 drivers/phy/samsung/phy-exynosauto-ufs.h
>>
>> diff --git a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
>> index 636cc501b54f..8e13f8cec6a7 100644
>> --- a/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
>> +++ b/Documentation/devicetree/bindings/phy/samsung,ufs-phy.yaml
>> @@ -16,6 +16,7 @@ properties:
>>    compatible:
>>      enum:
>>        - samsung,exynos7-ufs-phy
>> +      - samsung,exynosauto-ufs-phy
> 
> I would prefer real product ID (taken from registers or vendor kernel),
> not a marketing name "Auto" because it's too generic, unspecified, too
> much as wildcard.
> 
> If vendor kernel does not define name or product ID is also not
> specific, then it should include a version - so "autov9".
> 

BTW, the previous Exynos Auto SoC was "Exynos Auto 8890", so it already
confuses naming with older designs. It would be actually good to compare
it with other recent Exynos PHYs (Exynos Auto 8890, generic Exynos9xxx
and Exynos980/990) because maybe it's not "Auto" at all.

Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

RE: [PATCH 1/2] phy: samsung-ufs: custom PHY_TRSV_CH_OFFSET support

[Chanho Park]

> Thanks for the patches. I think this should be squashed into your next
> patch - it does not make much sense as standalone commit.
> 

Okay. I'll squash it next patchset.

Thanks.

Best Regards,
Chanho Park


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

RE: [PATCH 2/2] phy: samsung-ufs: support exynosauto ufs phy driver

[Chanho Park]

> >        - samsung,exynos7-ufs-phy
> > +      - samsung,exynosauto-ufs-phy
> 
> I would prefer real product ID (taken from registers or vendor kernel),
> not a marketing name "Auto" because it's too generic, unspecified, too
> much as wildcard.
> 
> If vendor kernel does not define name or product ID is also not specific,
> then it should include a version - so "autov9".

"samsung,exynosautov9-ufs-phy" can be a full name which can describe the SoC.

> I know that Exynos7 UFS did it but it was wrong. Headers should not define
> structures. What if a header needs to be included in different
> objects/units?
> 
> This should be probably a separate file (C file). Something like we did
> for Exynos PMU (drivers/soc/samsung/exynos-pmu.h).

It makes sense to me. I'll move the structures from "phy-exynos7-ufs.h"
to "phy-exynos7-ufs.c" and create "phy-exynosautov9-ufs.c".

Best Regards,
Chanho Park


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy