lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <03e501db30fc$916cf390$b446dab0$@samsung.com>
Date: Thu, 7 Nov 2024 15:35:23 +0530
From: "Alim Akhtar" <alim.akhtar@...sung.com>
To: "'Sowon Na'" <sowon.na@...sung.com>, <robh@...nel.org>,
	<krzk@...nel.org>, <conor+dt@...nel.org>, <vkoul@...nel.org>,
	<kishon@...nel.org>
Cc: <krzk+dt@...nel.org>, <linux-kernel@...r.kernel.org>,
	<devicetree@...r.kernel.org>, <linux-samsung-soc@...r.kernel.org>
Subject: RE: [PATCH 2/3] phy: samsung-ufs: support exynosauto ufs phy driver

Hello Sowon

> -----Original Message-----
> From: Sowon Na <sowon.na@...sung.com>
> Sent: Thursday, November 7, 2024 9:45 AM
> To: robh@...nel.org; krzk@...nel.org; conor+dt@...nel.org;
> vkoul@...nel.org; alim.akhtar@...sung.com; kishon@...nel.org
> Cc: krzk+dt@...nel.org; linux-kernel@...r.kernel.org;
> devicetree@...r.kernel.org; linux-samsung-soc@...r.kernel.org;
> sowon.na@...sung.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@...sung.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



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ