From: "Alim Akhtar" <alim.akhtar@samsung.com>
To: "'Sowon Na'" <sowon.na@samsung.com>, <robh@kernel.org>,
<krzk@kernel.org>, <conor+dt@kernel.org>, <vkoul@kernel.org>,
<kishon@kernel.org>
Cc: <krzk+dt@kernel.org>, <linux-kernel@vger.kernel.org>,
<devicetree@vger.kernel.org>, <linux-samsung-soc@vger.kernel.org>
Subject: RE: [PATCH 2/3] phy: samsung-ufs: support exynosauto ufs phy driver
Date: Thu, 7 Nov 2024 15:35:23 +0530 [thread overview]
Message-ID: <03e501db30fc$916cf390$b446dab0$@samsung.com> (raw)
In-Reply-To: <20241107041509.824457-4-sowon.na@samsung.com>
Hello Sowon
> -----Original Message-----
> From: Sowon Na <sowon.na@samsung.com>
> Sent: Thursday, November 7, 2024 9:45 AM
> To: robh@kernel.org; krzk@kernel.org; conor+dt@kernel.org;
> vkoul@kernel.org; alim.akhtar@samsung.com; kishon@kernel.org
> Cc: krzk+dt@kernel.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
> sowon.na@samsung.com
> Subject: [PATCH 2/3] phy: samsung-ufs: support exynosauto ufs phy driver
>
s/exynosauto/ exynosautov920
let's be specific as we have v9 variant as well.
> Support phy-exynosautov920-ufs driver for ExynosAutov920 series SoCs,
> using "samsung,exynosautov920-ufs-phy" compatible.
>
s/ ExynosAutov920/ exynosautov920
May be just: Add support for exynosautov920 ufs phy driver
using "samsung,exynosautov920-ufs-phy" compatible is obvious
> Signed-off-by: Sowon Na <sowon.na@samsung.com>
> ---
> drivers/phy/samsung/Makefile | 1 +
> drivers/phy/samsung/phy-exynosautov920-ufs.c | 159
> +++++++++++++++++++
> drivers/phy/samsung/phy-samsung-ufs.c | 9 +-
> drivers/phy/samsung/phy-samsung-ufs.h | 4 +
> 4 files changed, 170 insertions(+), 3 deletions(-) create mode 100644
> drivers/phy/samsung/phy-exynosautov920-ufs.c
>
> diff --git a/drivers/phy/samsung/Makefile b/drivers/phy/samsung/Makefile
> index fea1f96d0e43..342682638a87 100644
> --- a/drivers/phy/samsung/Makefile
> +++ b/drivers/phy/samsung/Makefile
> @@ -7,6 +7,7 @@ phy-exynos-ufs-y += phy-gs101-ufs.o
> phy-exynos-ufs-y += phy-samsung-ufs.o
> phy-exynos-ufs-y += phy-exynos7-ufs.o
> phy-exynos-ufs-y += phy-exynosautov9-ufs.o
> +phy-exynos-ufs-y += phy-exynosautov920-ufs.o
> phy-exynos-ufs-y += phy-fsd-ufs.o
> obj-$(CONFIG_PHY_SAMSUNG_USB2) += phy-exynos-
> usb2.o
> phy-exynos-usb2-y += phy-samsung-usb2.o
> diff --git a/drivers/phy/samsung/phy-exynosautov920-ufs.c
> b/drivers/phy/samsung/phy-exynosautov920-ufs.c
> new file mode 100644
> index 000000000000..8f4a94e13781
> --- /dev/null
> +++ b/drivers/phy/samsung/phy-exynosautov920-ufs.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * UFS PHY driver data for Samsung EXYNOSAUTO v920 SoC
Let be consistent with the naming, may be EXYNOSAUTOV920
> + *
> + * Copyright (C) 2024 Samsung Electronics Co., Ltd.
> + */
> +
> +#include "phy-samsung-ufs.h"
> +
> +#define EXYNOSAUTOV920_EMBEDDED_COMBO_PHY_CTRL
> 0x708
> +#define EXYNOSAUTOV920_EMBEDDED_COMBO_PHY_CTRL_MASK
> 0x1
> +#define EXYNOSAUTOV920_EMBEDDED_COMBO_PHY_CTRL_EN
> BIT(0)
> +#define EXYNOSAUTOV920_EMBEDDED_COMBO_PHY_CDR_LOCK_STATUS
> 0x5e
> +
> +#define PHY_TRSV_REG_CFG_AUTOV920(o, v, d) \
> + PHY_TRSV_REG_CFG_OFFSET(o, v, d, 0x200)
No magic number please, add a macro for 0x200
May be PHY_EXYNOSAUTOV920_LANE_OFFSET
> +
> +/* Calibration for phy initialization */ static const struct
> +samsung_ufs_phy_cfg exynosautov920_pre_init_cfg[] = {
> + PHY_COMN_REG_CFG(0x29, 0x22, PWR_MODE_ANY),
> + PHY_COMN_REG_CFG(0x43, 0x10, PWR_MODE_ANY),
> + PHY_COMN_REG_CFG(0x3C, 0x14, PWR_MODE_ANY),
> + PHY_COMN_REG_CFG(0x46, 0x48, PWR_MODE_ANY),
> + PHY_COMN_REG_CFG(0x04, 0x95, PWR_MODE_ANY),
> + PHY_COMN_REG_CFG(0x06, 0x30, PWR_MODE_ANY),
> +
> + PHY_TRSV_REG_CFG_AUTOV920(0x200, 0x00, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x201, 0x06, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x202, 0x06, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x203, 0x0A, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x204, 0x00, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x205, 0x10, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x207, 0x0C, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2E1, 0xC0, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x22D, 0xF8, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x234, 0x60, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x238, 0x13, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x239, 0x48, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x23A, 0x01, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x23B, 0x29, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x23C, 0x2A, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x23D, 0x01, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x23E, 0x14, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x23F, 0x13, PWR_MODE_ANY),
> +
> + PHY_TRSV_REG_CFG_AUTOV920(0x240, 0x4A, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x243, 0x40, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x244, 0x02, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x25D, 0x00, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x25E, 0x3F, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x25F, 0xFF, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x26F, 0xF0, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x273, 0x33, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x274, 0x50, PWR_MODE_ANY),
> +
> + PHY_TRSV_REG_CFG_AUTOV920(0x284, 0x02, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x285, 0x02, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2A2, 0x04, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x27D, 0x01, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2FA, 0x01, PWR_MODE_ANY),
> +
> + PHY_TRSV_REG_CFG_AUTOV920(0x286, 0x03, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x287, 0x03, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x288, 0x03, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x289, 0x03, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2B3, 0x04, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2B6, 0x0B, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2B7, 0x0B, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2B8, 0x0B, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2B9, 0x0B, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2BA, 0x0B, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2BB, 0x06, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2BC, 0x06, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2BD, 0x06, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x2BE, 0x06, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x34B, 0x01, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x34C, 0x24, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x34D, 0x23, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x34E, 0x45, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x34F, 0x00, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x350, 0x31, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x351, 0x00, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x352, 0x02, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x353, 0x00, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x354, 0x01, PWR_MODE_ANY),
> +
> + PHY_COMN_REG_CFG(0x43, 0x18, PWR_MODE_ANY),
> + PHY_COMN_REG_CFG(0x43, 0x00, PWR_MODE_ANY),
> +
> + END_UFS_PHY_CFG,
> +};
> +
> +/* Calibration for HS mode series A/B */ static const struct
> +samsung_ufs_phy_cfg exynosautov920_pre_pwr_hs_cfg[] = {
> + PHY_TRSV_REG_CFG_AUTOV920(0x369, 0x11, PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x246, 0x03, PWR_MODE_ANY),
> +
> + END_UFS_PHY_CFG,
> +};
> +
> +static const struct samsung_ufs_phy_cfg
> exynosautov920_post_pwr_hs_cfg[] = {
> + END_UFS_PHY_CFG,
> +};
> +
> +#define DELAY_IN_US 40
> +#define RETRY_CNT 100
> +#define MASK 0x8
> +int samsung_ufs_phy_wait_cdr_lock(struct phy *phy, u8 lane) {
This is specific to v920, so rename the function as exynosautov920_ufs_*
> + struct samsung_ufs_phy *ufs_phy = get_samsung_ufs_phy(phy);
> + u32 reg, i;
> +
> + struct samsung_ufs_phy_cfg cfg[4] = {
> + PHY_TRSV_REG_CFG_AUTOV920(0x222, 0x10,
> PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x222, 0x18,
> PWR_MODE_ANY),
> + PHY_TRSV_REG_CFG_AUTOV920(0x246, 0x01,
> PWR_MODE_ANY),
> + END_UFS_PHY_CFG,
> + };
> +
> + for (i = 0; i < RETRY_CNT; i++) {
> + udelay(DELAY_IN_US);
> +
> + reg = readl(ufs_phy->reg_pma + 0xCE4 + (0x800 * lane));
Please have macro for the offsets
> + if ((reg & MASK) == MASK)
MASK is very generic, can we have some meaningful name for better readability and understanding
> + break;
How you are handling error here, what will happen if CDR lock fails?
> +
> + udelay(DELAY_IN_US);
> +
> + samsung_ufs_phy_config(ufs_phy, &cfg[0], lane);
> + samsung_ufs_phy_config(ufs_phy, &cfg[1], lane);
> + }
> +
> + samsung_ufs_phy_config(ufs_phy, &cfg[2], lane);
> +
> + return 0;
> +}
> +
> +static const struct samsung_ufs_phy_cfg
> *exynosautov920_ufs_phy_cfgs[CFG_TAG_MAX] = {
> + [CFG_PRE_INIT] = exynosautov920_pre_init_cfg,
> + [CFG_PRE_PWR_HS] = exynosautov920_pre_pwr_hs_cfg,
> + [CFG_POST_PWR_HS] = exynosautov920_post_pwr_hs_cfg,
> +};
> +
> +static const char * const exynosautov920_ufs_phy_clks[] = {
> + "ref_clk",
> +};
> +
> +const struct samsung_ufs_phy_drvdata exynosautov920_ufs_phy = {
> + .cfgs = exynosautov920_ufs_phy_cfgs,
> + .isol = {
> + .offset = EXYNOSAUTOV920_EMBEDDED_COMBO_PHY_CTRL,
> + .mask =
> EXYNOSAUTOV920_EMBEDDED_COMBO_PHY_CTRL_MASK,
> + .en =
> EXYNOSAUTOV920_EMBEDDED_COMBO_PHY_CTRL_EN,
> + },
> + .clk_list = exynosautov920_ufs_phy_clks,
> + .num_clks = ARRAY_SIZE(exynosautov920_ufs_phy_clks),
> + .cdr_lock_status_offset =
> EXYNOSAUTOV920_EMBEDDED_COMBO_PHY_CDR_LOCK_STATUS,
> + .wait_for_cdr = samsung_ufs_phy_wait_cdr_lock, };
> diff --git a/drivers/phy/samsung/phy-samsung-ufs.c
> b/drivers/phy/samsung/phy-samsung-ufs.c
> index 6c5d41552649..c13fe149bc75 100644
> --- a/drivers/phy/samsung/phy-samsung-ufs.c
> +++ b/drivers/phy/samsung/phy-samsung-ufs.c
> @@ -28,9 +28,9 @@
>
> #define PHY_DEF_LANE_CNT 1
>
> -static void samsung_ufs_phy_config(struct samsung_ufs_phy *phy,
> - const struct samsung_ufs_phy_cfg *cfg,
> - u8 lane)
> +void samsung_ufs_phy_config(struct samsung_ufs_phy *phy,
> + const struct samsung_ufs_phy_cfg *cfg,
> + u8 lane)
> {
> enum {LANE_0, LANE_1}; /* lane index */
>
> @@ -323,6 +323,9 @@ static const struct of_device_id
> samsung_ufs_phy_match[] = {
> }, {
> .compatible = "samsung,exynosautov9-ufs-phy",
> .data = &exynosautov9_ufs_phy,
> + }, {
> + .compatible = "samsung,exynosautov920-ufs-phy",
> + .data = &exynosautov920_ufs_phy,
> }, {
> .compatible = "tesla,fsd-ufs-phy",
> .data = &fsd_ufs_phy,
> diff --git a/drivers/phy/samsung/phy-samsung-ufs.h
> b/drivers/phy/samsung/phy-samsung-ufs.h
> index 9b7deef6e10f..b7a8ec6bcafa 100644
> --- a/drivers/phy/samsung/phy-samsung-ufs.h
> +++ b/drivers/phy/samsung/phy-samsung-ufs.h
> @@ -143,9 +143,13 @@ static inline void samsung_ufs_phy_ctrl_isol( }
>
> int samsung_ufs_phy_wait_for_lock_acq(struct phy *phy, u8 lane);
> +int samsung_ufs_phy_wait_cdr_lock(struct phy *phy, u8 lane); void
> +samsung_ufs_phy_config(struct samsung_ufs_phy *phy,
> + const struct samsung_ufs_phy_cfg *cfg, u8 lane);
>
> extern const struct samsung_ufs_phy_drvdata exynos7_ufs_phy; extern
> const struct samsung_ufs_phy_drvdata exynosautov9_ufs_phy;
> +extern const struct samsung_ufs_phy_drvdata exynosautov920_ufs_phy;
> extern const struct samsung_ufs_phy_drvdata fsd_ufs_phy; extern const
> struct samsung_ufs_phy_drvdata tensor_gs101_ufs_phy;
>
> --
> 2.45.2
next prev parent reply other threads:[~2024-11-07 10:05 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20241107041510epcas2p28a5506042eeca912da5b665a04c3a6f0@epcas2p2.samsung.com>
2024-11-07 4:15 ` [PATCH 0/3] Support exynosautov920 ufs phy driver Sowon Na
2024-11-07 4:15 ` Sowon Na
2024-11-07 4:15 ` [PATCH 1/3] dt-bindings: phy: Add ExynosAutov920 UFS PHY bindings Sowon Na
2024-11-07 11:04 ` Krzysztof Kozlowski
2024-11-07 11:10 ` Alim Akhtar
2024-11-07 4:15 ` [PATCH 2/3] phy: samsung-ufs: support exynosauto ufs phy driver Sowon Na
2024-11-07 10:05 ` Alim Akhtar [this message]
2024-11-07 11:04 ` Krzysztof Kozlowski
2024-11-07 4:15 ` [PATCH 3/3] arm64: dts: exynosautov920: add ufs phy for exynosautov920 SoC Sowon Na
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='03e501db30fc$916cf390$b446dab0$@samsung.com' \
--to=alim.akhtar@samsung.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=kishon@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=krzk@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=robh@kernel.org \
--cc=sowon.na@samsung.com \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.