2021-06-24 18:42:28

by Pratyush Yadav

[permalink] [raw]
Subject: [PATCH v3 2/7] phy: cdns-dphy: Prepare for Rx support

The Rx programming sequence differs from the Tx programming sequence.
Currently only Tx mode is supported. Move all the Tx related parts into
a set of Tx-specific hooks that are then called by the main PHY
framework hooks. This way when Rx support is added all that is needed to
be done is to plug in the Rx hooks.

The clocks "psm" and "pll_ref" are not used by the Rx path so make them
optional in the probe and then check if they exist in the power_on()
hook.

Signed-off-by: Pratyush Yadav <[email protected]>
---

(no changes since v1)

drivers/phy/cadence/cdns-dphy.c | 140 ++++++++++++++++++++++++--------
1 file changed, 104 insertions(+), 36 deletions(-)

diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
index ba042e39cfaf..8656f2102a91 100644
--- a/drivers/phy/cadence/cdns-dphy.c
+++ b/drivers/phy/cadence/cdns-dphy.c
@@ -75,6 +75,11 @@ struct cdns_dphy;
struct cdns_dphy_ops {
int (*probe)(struct cdns_dphy *dphy);
void (*remove)(struct cdns_dphy *dphy);
+ int (*power_on)(struct cdns_dphy *dphy);
+ int (*power_off)(struct cdns_dphy *dphy);
+ int (*validate)(struct cdns_dphy *dphy, enum phy_mode mode, int submode,
+ union phy_configure_opts *opts);
+ int (*configure)(struct cdns_dphy *dphy, union phy_configure_opts *opts);
void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
enum cdns_dphy_clk_lane_cfg cfg);
@@ -86,12 +91,18 @@ struct cdns_dphy_ops {
struct cdns_dphy {
struct cdns_dphy_cfg cfg;
void __iomem *regs;
+ struct device *dev;
struct clk *psm_clk;
struct clk *pll_ref_clk;
const struct cdns_dphy_ops *ops;
struct phy *phy;
};

+struct cdns_dphy_driver_data {
+ const struct cdns_dphy_ops *tx;
+ const struct cdns_dphy_ops *rx;
+};
+
static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
struct cdns_dphy_cfg *cfg,
struct phy_configure_opts_mipi_dphy *opts,
@@ -199,20 +210,9 @@ static void cdns_dphy_ref_set_psm_div(struct cdns_dphy *dphy, u8 div)
dphy->regs + DPHY_PSM_CFG);
}

-/*
- * This is the reference implementation of DPHY hooks. Specific integration of
- * this IP may have to re-implement some of them depending on how they decided
- * to wire things in the SoC.
- */
-static const struct cdns_dphy_ops ref_dphy_ops = {
- .get_wakeup_time_ns = cdns_dphy_ref_get_wakeup_time_ns,
- .set_pll_cfg = cdns_dphy_ref_set_pll_cfg,
- .set_psm_div = cdns_dphy_ref_set_psm_div,
-};
-
-static int cdns_dphy_config_from_opts(struct phy *phy,
- struct phy_configure_opts_mipi_dphy *opts,
- struct cdns_dphy_cfg *cfg)
+static int cdns_dphy_tx_config_from_opts(struct phy *phy,
+ struct phy_configure_opts_mipi_dphy *opts,
+ struct cdns_dphy_cfg *cfg)
{
struct cdns_dphy *dphy = phy_get_drvdata(phy);
unsigned int dsi_hfp_ext = 0;
@@ -232,24 +232,13 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
return 0;
}

-static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
- union phy_configure_opts *opts)
+static int cdns_dphy_tx_configure(struct cdns_dphy *dphy,
+ union phy_configure_opts *opts)
{
struct cdns_dphy_cfg cfg = { 0 };
-
- if (mode != PHY_MODE_MIPI_DPHY)
- return -EINVAL;
-
- return cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
-}
-
-static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
-{
- struct cdns_dphy *dphy = phy_get_drvdata(phy);
- struct cdns_dphy_cfg cfg = { 0 };
int ret;

- ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
+ ret = cdns_dphy_tx_config_from_opts(dphy->phy, &opts->mipi_dphy, &cfg);
if (ret)
return ret;

@@ -279,9 +268,21 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
return 0;
}

-static int cdns_dphy_power_on(struct phy *phy)
+static int cdns_dphy_tx_validate(struct cdns_dphy *dphy, enum phy_mode mode,
+ int submode, union phy_configure_opts *opts)
{
- struct cdns_dphy *dphy = phy_get_drvdata(phy);
+ struct cdns_dphy_cfg cfg = { 0 };
+
+ if (submode != PHY_MIPI_DPHY_SUBMODE_TX)
+ return -EINVAL;
+
+ return cdns_dphy_tx_config_from_opts(dphy->phy, &opts->mipi_dphy, &cfg);
+}
+
+static int cdns_dphy_tx_power_on(struct cdns_dphy *dphy)
+{
+ if (!dphy->psm_clk || !dphy->pll_ref_clk)
+ return -EINVAL;

clk_prepare_enable(dphy->psm_clk);
clk_prepare_enable(dphy->pll_ref_clk);
@@ -293,16 +294,77 @@ static int cdns_dphy_power_on(struct phy *phy)
return 0;
}

-static int cdns_dphy_power_off(struct phy *phy)
+static int cdns_dphy_tx_power_off(struct cdns_dphy *dphy)
{
- struct cdns_dphy *dphy = phy_get_drvdata(phy);
-
clk_disable_unprepare(dphy->pll_ref_clk);
clk_disable_unprepare(dphy->psm_clk);

return 0;
}

+static const struct cdns_dphy_ops tx_ref_dphy_ops = {
+ .power_on = cdns_dphy_tx_power_on,
+ .power_off = cdns_dphy_tx_power_off,
+ .validate = cdns_dphy_tx_validate,
+ .configure = cdns_dphy_tx_configure,
+ .get_wakeup_time_ns = cdns_dphy_ref_get_wakeup_time_ns,
+ .set_pll_cfg = cdns_dphy_ref_set_pll_cfg,
+ .set_psm_div = cdns_dphy_ref_set_psm_div,
+};
+
+/*
+ * This is the reference implementation of DPHY hooks. Specific integration of
+ * this IP may have to re-implement some of them depending on how they decided
+ * to wire things in the SoC.
+ */
+static const struct cdns_dphy_driver_data ref_dphy_ops = {
+ .tx = &tx_ref_dphy_ops,
+};
+
+static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
+ union phy_configure_opts *opts)
+{
+ struct cdns_dphy *dphy = phy_get_drvdata(phy);
+
+ if (mode != PHY_MODE_MIPI_DPHY)
+ return -EINVAL;
+
+ if (dphy->ops->validate)
+ return dphy->ops->validate(dphy, mode, submode, opts);
+
+ return 0;
+}
+
+static int cdns_dphy_power_on(struct phy *phy)
+{
+ struct cdns_dphy *dphy = phy_get_drvdata(phy);
+
+ if (dphy->ops->power_on)
+ return dphy->ops->power_on(dphy);
+
+ return 0;
+}
+
+static int cdns_dphy_power_off(struct phy *phy)
+{
+ struct cdns_dphy *dphy = phy_get_drvdata(phy);
+
+ if (dphy->ops->power_off)
+ return dphy->ops->power_off(dphy);
+
+ return 0;
+}
+
+static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
+{
+ struct cdns_dphy *dphy = phy_get_drvdata(phy);
+
+ if (dphy->ops->configure)
+ return dphy->ops->configure(dphy, opts);
+
+ return 0;
+}
+
static const struct phy_ops cdns_dphy_ops = {
.configure = cdns_dphy_configure,
.validate = cdns_dphy_validate,
@@ -314,14 +376,20 @@ static int cdns_dphy_probe(struct platform_device *pdev)
{
struct phy_provider *phy_provider;
struct cdns_dphy *dphy;
+ const struct cdns_dphy_driver_data *ddata;
int ret;

dphy = devm_kzalloc(&pdev->dev, sizeof(*dphy), GFP_KERNEL);
if (!dphy)
return -ENOMEM;
dev_set_drvdata(&pdev->dev, dphy);
+ dphy->dev = &pdev->dev;

- dphy->ops = of_device_get_match_data(&pdev->dev);
+ ddata = of_device_get_match_data(&pdev->dev);
+ if (!ddata)
+ return -EINVAL;
+
+ dphy->ops = ddata->tx;
if (!dphy->ops)
return -EINVAL;

@@ -329,11 +397,11 @@ static int cdns_dphy_probe(struct platform_device *pdev)
if (IS_ERR(dphy->regs))
return PTR_ERR(dphy->regs);

- dphy->psm_clk = devm_clk_get(&pdev->dev, "psm");
+ dphy->psm_clk = devm_clk_get_optional(dphy->dev, "psm");
if (IS_ERR(dphy->psm_clk))
return PTR_ERR(dphy->psm_clk);

- dphy->pll_ref_clk = devm_clk_get(&pdev->dev, "pll_ref");
+ dphy->pll_ref_clk = devm_clk_get_optional(dphy->dev, "pll_ref");
if (IS_ERR(dphy->pll_ref_clk))
return PTR_ERR(dphy->pll_ref_clk);

--
2.30.0


2021-08-06 14:40:52

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] phy: cdns-dphy: Prepare for Rx support

On 25-06-21, 00:11, Pratyush Yadav wrote:
> The Rx programming sequence differs from the Tx programming sequence.
> Currently only Tx mode is supported. Move all the Tx related parts into
> a set of Tx-specific hooks that are then called by the main PHY
> framework hooks. This way when Rx support is added all that is needed to
> be done is to plug in the Rx hooks.
>
> The clocks "psm" and "pll_ref" are not used by the Rx path so make them
> optional in the probe and then check if they exist in the power_on()
> hook.
>
> Signed-off-by: Pratyush Yadav <[email protected]>
> ---
>
> (no changes since v1)
>
> drivers/phy/cadence/cdns-dphy.c | 140 ++++++++++++++++++++++++--------
> 1 file changed, 104 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> index ba042e39cfaf..8656f2102a91 100644
> --- a/drivers/phy/cadence/cdns-dphy.c
> +++ b/drivers/phy/cadence/cdns-dphy.c
> @@ -75,6 +75,11 @@ struct cdns_dphy;
> struct cdns_dphy_ops {
> int (*probe)(struct cdns_dphy *dphy);
> void (*remove)(struct cdns_dphy *dphy);
> + int (*power_on)(struct cdns_dphy *dphy);
> + int (*power_off)(struct cdns_dphy *dphy);
> + int (*validate)(struct cdns_dphy *dphy, enum phy_mode mode, int submode,
> + union phy_configure_opts *opts);
> + int (*configure)(struct cdns_dphy *dphy, union phy_configure_opts *opts);
> void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
> void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
> enum cdns_dphy_clk_lane_cfg cfg);
> @@ -86,12 +91,18 @@ struct cdns_dphy_ops {
> struct cdns_dphy {
> struct cdns_dphy_cfg cfg;
> void __iomem *regs;
> + struct device *dev;
> struct clk *psm_clk;
> struct clk *pll_ref_clk;
> const struct cdns_dphy_ops *ops;
> struct phy *phy;
> };
>
> +struct cdns_dphy_driver_data {
> + const struct cdns_dphy_ops *tx;
> + const struct cdns_dphy_ops *rx;
> +};
> +
> static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
> struct cdns_dphy_cfg *cfg,
> struct phy_configure_opts_mipi_dphy *opts,
> @@ -199,20 +210,9 @@ static void cdns_dphy_ref_set_psm_div(struct cdns_dphy *dphy, u8 div)
> dphy->regs + DPHY_PSM_CFG);
> }
>
> -/*
> - * This is the reference implementation of DPHY hooks. Specific integration of
> - * this IP may have to re-implement some of them depending on how they decided
> - * to wire things in the SoC.
> - */
> -static const struct cdns_dphy_ops ref_dphy_ops = {
> - .get_wakeup_time_ns = cdns_dphy_ref_get_wakeup_time_ns,
> - .set_pll_cfg = cdns_dphy_ref_set_pll_cfg,
> - .set_psm_div = cdns_dphy_ref_set_psm_div,
> -};
> -
> -static int cdns_dphy_config_from_opts(struct phy *phy,
> - struct phy_configure_opts_mipi_dphy *opts,
> - struct cdns_dphy_cfg *cfg)
> +static int cdns_dphy_tx_config_from_opts(struct phy *phy,
> + struct phy_configure_opts_mipi_dphy *opts,
> + struct cdns_dphy_cfg *cfg)
> {
> struct cdns_dphy *dphy = phy_get_drvdata(phy);
> unsigned int dsi_hfp_ext = 0;
> @@ -232,24 +232,13 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
> return 0;
> }
>
> -static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
> - union phy_configure_opts *opts)
> +static int cdns_dphy_tx_configure(struct cdns_dphy *dphy,
> + union phy_configure_opts *opts)
> {
> struct cdns_dphy_cfg cfg = { 0 };
> -
> - if (mode != PHY_MODE_MIPI_DPHY)
> - return -EINVAL;
> -
> - return cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
> -}
> -
> -static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> -{
> - struct cdns_dphy *dphy = phy_get_drvdata(phy);
> - struct cdns_dphy_cfg cfg = { 0 };
> int ret;
>
> - ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
> + ret = cdns_dphy_tx_config_from_opts(dphy->phy, &opts->mipi_dphy, &cfg);
> if (ret)
> return ret;
>
> @@ -279,9 +268,21 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> return 0;
> }
>
> -static int cdns_dphy_power_on(struct phy *phy)
> +static int cdns_dphy_tx_validate(struct cdns_dphy *dphy, enum phy_mode mode,
> + int submode, union phy_configure_opts *opts)
> {
> - struct cdns_dphy *dphy = phy_get_drvdata(phy);
> + struct cdns_dphy_cfg cfg = { 0 };
> +
> + if (submode != PHY_MIPI_DPHY_SUBMODE_TX)
> + return -EINVAL;
> +
> + return cdns_dphy_tx_config_from_opts(dphy->phy, &opts->mipi_dphy, &cfg);
> +}
> +
> +static int cdns_dphy_tx_power_on(struct cdns_dphy *dphy)
> +{
> + if (!dphy->psm_clk || !dphy->pll_ref_clk)
> + return -EINVAL;
>
> clk_prepare_enable(dphy->psm_clk);
> clk_prepare_enable(dphy->pll_ref_clk);
> @@ -293,16 +294,77 @@ static int cdns_dphy_power_on(struct phy *phy)
> return 0;
> }
>
> -static int cdns_dphy_power_off(struct phy *phy)
> +static int cdns_dphy_tx_power_off(struct cdns_dphy *dphy)
> {
> - struct cdns_dphy *dphy = phy_get_drvdata(phy);
> -
> clk_disable_unprepare(dphy->pll_ref_clk);
> clk_disable_unprepare(dphy->psm_clk);
>
> return 0;
> }
>
> +static const struct cdns_dphy_ops tx_ref_dphy_ops = {
> + .power_on = cdns_dphy_tx_power_on,
> + .power_off = cdns_dphy_tx_power_off,
> + .validate = cdns_dphy_tx_validate,
> + .configure = cdns_dphy_tx_configure,
> + .get_wakeup_time_ns = cdns_dphy_ref_get_wakeup_time_ns,
> + .set_pll_cfg = cdns_dphy_ref_set_pll_cfg,
> + .set_psm_div = cdns_dphy_ref_set_psm_div,
> +};
> +
> +/*
> + * This is the reference implementation of DPHY hooks. Specific integration of
> + * this IP may have to re-implement some of them depending on how they decided
> + * to wire things in the SoC.
> + */
> +static const struct cdns_dphy_driver_data ref_dphy_ops = {
> + .tx = &tx_ref_dphy_ops,
> +};
> +
> +static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
> + union phy_configure_opts *opts)
> +{
> + struct cdns_dphy *dphy = phy_get_drvdata(phy);
> +
> + if (mode != PHY_MODE_MIPI_DPHY)
> + return -EINVAL;
> +
> + if (dphy->ops->validate)
> + return dphy->ops->validate(dphy, mode, submode, opts);
> +
> + return 0;
> +}
> +
> +static int cdns_dphy_power_on(struct phy *phy)
> +{
> + struct cdns_dphy *dphy = phy_get_drvdata(phy);
> +
> + if (dphy->ops->power_on)
> + return dphy->ops->power_on(dphy);
> +
> + return 0;
> +}
> +
> +static int cdns_dphy_power_off(struct phy *phy)
> +{
> + struct cdns_dphy *dphy = phy_get_drvdata(phy);
> +
> + if (dphy->ops->power_off)
> + return dphy->ops->power_off(dphy);
> +
> + return 0;
> +}

why do you need a dphy specific power_on/off only to call internal
ops..? If there is no additional logic lets drop this and continue the
phy ops for this

--
~Vinod

2021-08-11 17:44:24

by Pratyush Yadav

[permalink] [raw]
Subject: Re: [PATCH v3 2/7] phy: cdns-dphy: Prepare for Rx support

On 06/08/21 03:48PM, Vinod Koul wrote:
> On 25-06-21, 00:11, Pratyush Yadav wrote:
> > The Rx programming sequence differs from the Tx programming sequence.
> > Currently only Tx mode is supported. Move all the Tx related parts into
> > a set of Tx-specific hooks that are then called by the main PHY
> > framework hooks. This way when Rx support is added all that is needed to
> > be done is to plug in the Rx hooks.
> >
> > The clocks "psm" and "pll_ref" are not used by the Rx path so make them
> > optional in the probe and then check if they exist in the power_on()
> > hook.
> >
> > Signed-off-by: Pratyush Yadav <[email protected]>
> > ---
> >
> > (no changes since v1)
> >
> > drivers/phy/cadence/cdns-dphy.c | 140 ++++++++++++++++++++++++--------
> > 1 file changed, 104 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/phy/cadence/cdns-dphy.c b/drivers/phy/cadence/cdns-dphy.c
> > index ba042e39cfaf..8656f2102a91 100644
> > --- a/drivers/phy/cadence/cdns-dphy.c
> > +++ b/drivers/phy/cadence/cdns-dphy.c
> > @@ -75,6 +75,11 @@ struct cdns_dphy;
> > struct cdns_dphy_ops {
> > int (*probe)(struct cdns_dphy *dphy);
> > void (*remove)(struct cdns_dphy *dphy);
> > + int (*power_on)(struct cdns_dphy *dphy);
> > + int (*power_off)(struct cdns_dphy *dphy);
> > + int (*validate)(struct cdns_dphy *dphy, enum phy_mode mode, int submode,
> > + union phy_configure_opts *opts);
> > + int (*configure)(struct cdns_dphy *dphy, union phy_configure_opts *opts);
> > void (*set_psm_div)(struct cdns_dphy *dphy, u8 div);
> > void (*set_clk_lane_cfg)(struct cdns_dphy *dphy,
> > enum cdns_dphy_clk_lane_cfg cfg);
> > @@ -86,12 +91,18 @@ struct cdns_dphy_ops {
> > struct cdns_dphy {
> > struct cdns_dphy_cfg cfg;
> > void __iomem *regs;
> > + struct device *dev;
> > struct clk *psm_clk;
> > struct clk *pll_ref_clk;
> > const struct cdns_dphy_ops *ops;
> > struct phy *phy;
> > };
> >
> > +struct cdns_dphy_driver_data {
> > + const struct cdns_dphy_ops *tx;
> > + const struct cdns_dphy_ops *rx;
> > +};
> > +
> > static int cdns_dsi_get_dphy_pll_cfg(struct cdns_dphy *dphy,
> > struct cdns_dphy_cfg *cfg,
> > struct phy_configure_opts_mipi_dphy *opts,
> > @@ -199,20 +210,9 @@ static void cdns_dphy_ref_set_psm_div(struct cdns_dphy *dphy, u8 div)
> > dphy->regs + DPHY_PSM_CFG);
> > }
> >
> > -/*
> > - * This is the reference implementation of DPHY hooks. Specific integration of
> > - * this IP may have to re-implement some of them depending on how they decided
> > - * to wire things in the SoC.
> > - */
> > -static const struct cdns_dphy_ops ref_dphy_ops = {
> > - .get_wakeup_time_ns = cdns_dphy_ref_get_wakeup_time_ns,
> > - .set_pll_cfg = cdns_dphy_ref_set_pll_cfg,
> > - .set_psm_div = cdns_dphy_ref_set_psm_div,
> > -};
> > -
> > -static int cdns_dphy_config_from_opts(struct phy *phy,
> > - struct phy_configure_opts_mipi_dphy *opts,
> > - struct cdns_dphy_cfg *cfg)
> > +static int cdns_dphy_tx_config_from_opts(struct phy *phy,
> > + struct phy_configure_opts_mipi_dphy *opts,
> > + struct cdns_dphy_cfg *cfg)
> > {
> > struct cdns_dphy *dphy = phy_get_drvdata(phy);
> > unsigned int dsi_hfp_ext = 0;
> > @@ -232,24 +232,13 @@ static int cdns_dphy_config_from_opts(struct phy *phy,
> > return 0;
> > }
> >
> > -static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
> > - union phy_configure_opts *opts)
> > +static int cdns_dphy_tx_configure(struct cdns_dphy *dphy,
> > + union phy_configure_opts *opts)
> > {
> > struct cdns_dphy_cfg cfg = { 0 };
> > -
> > - if (mode != PHY_MODE_MIPI_DPHY)
> > - return -EINVAL;
> > -
> > - return cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
> > -}
> > -
> > -static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> > -{
> > - struct cdns_dphy *dphy = phy_get_drvdata(phy);
> > - struct cdns_dphy_cfg cfg = { 0 };
> > int ret;
> >
> > - ret = cdns_dphy_config_from_opts(phy, &opts->mipi_dphy, &cfg);
> > + ret = cdns_dphy_tx_config_from_opts(dphy->phy, &opts->mipi_dphy, &cfg);
> > if (ret)
> > return ret;
> >
> > @@ -279,9 +268,21 @@ static int cdns_dphy_configure(struct phy *phy, union phy_configure_opts *opts)
> > return 0;
> > }
> >
> > -static int cdns_dphy_power_on(struct phy *phy)
> > +static int cdns_dphy_tx_validate(struct cdns_dphy *dphy, enum phy_mode mode,
> > + int submode, union phy_configure_opts *opts)
> > {
> > - struct cdns_dphy *dphy = phy_get_drvdata(phy);
> > + struct cdns_dphy_cfg cfg = { 0 };
> > +
> > + if (submode != PHY_MIPI_DPHY_SUBMODE_TX)
> > + return -EINVAL;
> > +
> > + return cdns_dphy_tx_config_from_opts(dphy->phy, &opts->mipi_dphy, &cfg);
> > +}
> > +
> > +static int cdns_dphy_tx_power_on(struct cdns_dphy *dphy)
> > +{
> > + if (!dphy->psm_clk || !dphy->pll_ref_clk)
> > + return -EINVAL;
> >
> > clk_prepare_enable(dphy->psm_clk);
> > clk_prepare_enable(dphy->pll_ref_clk);
> > @@ -293,16 +294,77 @@ static int cdns_dphy_power_on(struct phy *phy)
> > return 0;
> > }
> >
> > -static int cdns_dphy_power_off(struct phy *phy)
> > +static int cdns_dphy_tx_power_off(struct cdns_dphy *dphy)
> > {
> > - struct cdns_dphy *dphy = phy_get_drvdata(phy);
> > -
> > clk_disable_unprepare(dphy->pll_ref_clk);
> > clk_disable_unprepare(dphy->psm_clk);
> >
> > return 0;
> > }
> >
> > +static const struct cdns_dphy_ops tx_ref_dphy_ops = {
> > + .power_on = cdns_dphy_tx_power_on,
> > + .power_off = cdns_dphy_tx_power_off,
> > + .validate = cdns_dphy_tx_validate,
> > + .configure = cdns_dphy_tx_configure,
> > + .get_wakeup_time_ns = cdns_dphy_ref_get_wakeup_time_ns,
> > + .set_pll_cfg = cdns_dphy_ref_set_pll_cfg,
> > + .set_psm_div = cdns_dphy_ref_set_psm_div,
> > +};
> > +
> > +/*
> > + * This is the reference implementation of DPHY hooks. Specific integration of
> > + * this IP may have to re-implement some of them depending on how they decided
> > + * to wire things in the SoC.
> > + */
> > +static const struct cdns_dphy_driver_data ref_dphy_ops = {
> > + .tx = &tx_ref_dphy_ops,
> > +};
> > +
> > +static int cdns_dphy_validate(struct phy *phy, enum phy_mode mode, int submode,
> > + union phy_configure_opts *opts)
> > +{
> > + struct cdns_dphy *dphy = phy_get_drvdata(phy);
> > +
> > + if (mode != PHY_MODE_MIPI_DPHY)
> > + return -EINVAL;
> > +
> > + if (dphy->ops->validate)
> > + return dphy->ops->validate(dphy, mode, submode, opts);
> > +
> > + return 0;
> > +}
> > +
> > +static int cdns_dphy_power_on(struct phy *phy)
> > +{
> > + struct cdns_dphy *dphy = phy_get_drvdata(phy);
> > +
> > + if (dphy->ops->power_on)
> > + return dphy->ops->power_on(dphy);
> > +
> > + return 0;
> > +}
> > +
> > +static int cdns_dphy_power_off(struct phy *phy)
> > +{
> > + struct cdns_dphy *dphy = phy_get_drvdata(phy);
> > +
> > + if (dphy->ops->power_off)
> > + return dphy->ops->power_off(dphy);
> > +
> > + return 0;
> > +}
>
> why do you need a dphy specific power_on/off only to call internal
> ops..? If there is no additional logic lets drop this and continue the
> phy ops for this

I don't clearly understand what you are trying to say. The internal ops
(dphy->ops) are DPHY-specific. So for example, you might have different
power on/off sequences for Rx and Tx mode DPHYs. You would then
implement different power_on() and power_off() hooks for those and store
them in dphy->ops. How else would you handle this?

>
> --
> ~Vinod

--
Regards,
Pratyush Yadav
Texas Instruments Inc.