[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 at 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