[PATCH v2 3/6] phy: samsung: ufs: support secondary ufs phy

[Krzysztof Kozlowski]

On 02/06/2022 07:32, Chanho Park wrote:
> To support secondary ufs phy device, we need to get an offset for phy
> isolation from the syscon DT node. If the first index argument of the
> node is existing, we can read the offset value and set it as isol->offset.
> To allow this assignment, we need to change the field so the isol data
> needs to be allocated and copied from drvdata.
> 
> Signed-off-by: Chanho Park <chanho61.park at samsung.com>
> ---
>  drivers/phy/samsung/phy-samsung-ufs.c | 14 +++++++++++++-
>  drivers/phy/samsung/phy-samsung-ufs.h |  2 +-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/samsung/phy-samsung-ufs.c b/drivers/phy/samsung/phy-samsung-ufs.c
> index b7ddb91a02be..7716b6dc955b 100644
> --- a/drivers/phy/samsung/phy-samsung-ufs.c
> +++ b/drivers/phy/samsung/phy-samsung-ufs.c
> @@ -288,6 +288,7 @@ static int samsung_ufs_phy_probe(struct platform_device *pdev)
>  	struct phy *gen_phy;
>  	struct phy_provider *phy_provider;
>  	const struct samsung_ufs_phy_drvdata *drvdata;
> +	u32 isol_offset;
>  	int err = 0;
>  
>  	match = of_match_node(samsung_ufs_phy_match, dev->of_node);
> @@ -324,11 +325,22 @@ static int samsung_ufs_phy_probe(struct platform_device *pdev)
>  		goto out;
>  	}
>  
> +	phy->isol = devm_kzalloc(dev, sizeof(struct pmu_isol), GFP_KERNEL);

1. Looks like devm_kmemdup
2. sizeof(*variable), not sizeof(struct)

3. and actually you can simplify all that by storing struct pmu_isol
directly in struct samsung_ufs_phy, not as pointer.

After all that storing drvdata in samsung_ufs_phy does not make any
sense - only one field is left still used (has_symbol_clk), so this
should be simplified here as well.

> +	if (!phy->isol) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +
>  	drvdata = match->data;
>  	phy->dev = dev;
>  	phy->drvdata = drvdata;
>  	phy->cfg = drvdata->cfg;
> -	phy->isol = &drvdata->isol;
> +	memcpy(phy->isol, &drvdata->isol, sizeof(struct pmu_isol));
> +
> +	if (!of_property_read_u32_index(dev->of_node, "samsung,pmu-syscon", 1,
> +					&isol_offset))
> +		phy->isol->offset = isol_offset;
> +
>  	phy->lane_cnt = PHY_DEF_LANE_CNT;
>  
>  	phy_set_drvdata(gen_phy, phy);
> diff --git a/drivers/phy/samsung/phy-samsung-ufs.h b/drivers/phy/samsung/phy-samsung-ufs.h
> index 8c3696b3c5ec..d3e1c2016b27 100644
> --- a/drivers/phy/samsung/phy-samsung-ufs.h
> +++ b/drivers/phy/samsung/phy-samsung-ufs.h
> @@ -122,7 +122,7 @@ struct samsung_ufs_phy {
>  	struct clk *rx1_symbol_clk;
>  	const struct samsung_ufs_phy_drvdata *drvdata;
>  	const struct samsung_ufs_phy_cfg **cfg;
> -	const struct pmu_isol *isol;
> +	struct pmu_isol *isol;
>  	u8 lane_cnt;
>  	int ufs_phy_state;
>  	enum phy_mode mode;


Best regards,
Krzysztof