LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Pratyush Yadav <p.yadav@ti.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: Nikhil Devshatwar <nikhil.nd@ti.com>,
	Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Chunfeng Yun <chunfeng.yun@mediatek.com>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Peter Chen <peter.chen@nxp.com>, <linux-kernel@vger.kernel.org>,
	<linux-phy@lists.infradead.org>
Subject: Re: [PATCH v3 2/7] phy: cdns-dphy: Prepare for Rx support
Date: Wed, 11 Aug 2021 23:09:50 +0530	[thread overview]
Message-ID: <20210811173948.cezk6zu366c3huhw@ti.com> (raw)
In-Reply-To: <YQ0MZqI36bz1Jp87@matsya>

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 <p.yadav@ti.com>
> > ---
> > 
> > (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.

  reply	other threads:[~2021-08-11 17:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-24 18:41 [PATCH v3 0/7] Rx mode support for Cadence DPHY Pratyush Yadav
2021-06-24 18:41 ` [PATCH v3 1/7] phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes Pratyush Yadav
2021-08-06 11:11   ` Paul Kocialkowski
2021-08-11 17:36     ` Pratyush Yadav
2021-06-24 18:41 ` [PATCH v3 2/7] phy: cdns-dphy: Prepare for Rx support Pratyush Yadav
2021-08-06 10:18   ` Vinod Koul
2021-08-11 17:39     ` Pratyush Yadav [this message]
2021-06-24 18:41 ` [PATCH v3 3/7] phy: cdns-dphy: Allow setting mode Pratyush Yadav
2021-06-24 18:41 ` [PATCH v3 4/7] phy: cdns-dphy: Add Rx support Pratyush Yadav
2021-08-06 11:15   ` Vinod Koul
2021-08-11 17:33     ` Pratyush Yadav
2021-06-24 18:41 ` [PATCH v3 5/7] phy: dt-bindings: Convert Cadence DPHY binding to YAML Pratyush Yadav
2021-06-24 18:41 ` [PATCH v3 6/7] phy: dt-bindings: cdns,dphy: make clocks optional Pratyush Yadav
2021-06-24 18:41 ` [PATCH v3 7/7] phy: dt-bindings: cdns,dphy: add power-domains property Pratyush Yadav

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=20210811173948.cezk6zu366c3huhw@ti.com \
    --to=p.yadav@ti.com \
    --cc=chunfeng.yun@mediatek.com \
    --cc=kishon@ti.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=nikhil.nd@ti.com \
    --cc=paul.kocialkowski@bootlin.com \
    --cc=peter.chen@nxp.com \
    --cc=tomi.valkeinen@ideasonboard.com \
    --cc=vigneshr@ti.com \
    --cc=vkoul@kernel.org \
    --subject='Re: [PATCH v3 2/7] phy: cdns-dphy: Prepare for Rx support' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).