diff mbox series

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

Message ID 20210705102318.102999-3-chanho61.park@samsung.com
State Superseded
Headers show
Series Support exynosauto ufs phy driver | expand

Commit Message

Chanho Park July 5, 2021, 10:23 a.m. UTC 
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

Comments

Krzysztof Kozlowski July 5, 2021, 12:29 p.m. UTC | #1 
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
Krzysztof Kozlowski July 5, 2021, 12:35 p.m. UTC | #2 
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
Chanho Park July 5, 2021, 11:57 p.m. UTC | #3 
> >        - 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
diff mbox series

Patch

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_ */