Re: [EXT] Re: [PATCH] Revert "phy: cadence-torrent: Do not configure SERDES if it's already configured"

[Vinod Koul <vkoul@kernel.org>]

On 23-12-21, 06:27, Li Chen wrote:
> Hi, Vinod
> 
> > -----Original Message-----
> > From: Vinod Koul [mailto:vkoul@kernel.org]
> > Sent: Thursday, December 23, 2021 1:27 PM
> > To: Li Chen
> > Cc: Kishon Vijay Abraham I; Philipp Zabel; Swapnil Jakhade; linux-
> > phy@lists.infradead.org; linux-kernel@vger.kernel.org; Dan Carpenter
> > Subject: [EXT] Re: [PATCH] Revert "phy: cadence-torrent: Do not configure
> > SERDES if it's already configured"
> > 
> > On 26-11-21, 05:06, Li Chen wrote:
> > > This reverts commit
> > > b69d39f68419("phy: cadence-torrent: Do not configure SERDES if it's already
> > configured")
> > 
> > space between commit id and open brace...
> > 
> > >
> > > our soc will hang on any regmap field read before reset.
> > 
> > okay, in this case the right fix would be to keep track of reset in SW
> > and still skip reset if it is already configured?
> > 
> 
> I should be grateful if you would give me more details of reset in SW.

Store the reset state in a driver variable reset and use that for
finding already_configured rather than reading a hw value

> 
> > >
> > > Signed-off-by: Li Chen <lchen@ambarella.com>
> > > ---
> > >  drivers/phy/cadence/phy-cadence-torrent.c | 31 +++++++----------------
> > >  1 file changed, 9 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/phy/cadence/phy-cadence-torrent.c
> > b/drivers/phy/cadence/phy-cadence-torrent.c
> > > index 415ace64adc5c..e57e0b1523aff 100644
> > > --- a/drivers/phy/cadence/phy-cadence-torrent.c
> > > +++ b/drivers/phy/cadence/phy-cadence-torrent.c
> > > @@ -2031,11 +2031,6 @@ static int cdns_torrent_noop_phy_on(struct phy
> > *phy)
> > >  	return 0;
> > >  }
> > >
> > > -static const struct phy_ops noop_ops = {
> > > -	.power_on	= cdns_torrent_noop_phy_on,
> > > -	.owner		= THIS_MODULE,
> > > -};
> > > -
> > >  static
> > >  int cdns_torrent_phy_configure_multilink(struct cdns_torrent_phy *cdns_phy)
> > >  {
> > > @@ -2282,7 +2277,6 @@ static int cdns_torrent_phy_probe(struct
> > platform_device *pdev)
> > >  	struct device_node *child;
> > >  	int ret, subnodes, node = 0, i;
> > >  	u32 total_num_lanes = 0;
> > > -	int already_configured;
> > >  	u8 init_dp_regmap = 0;
> > >  	u32 phy_type;
> > >
> > > @@ -2321,20 +2315,16 @@ static int cdns_torrent_phy_probe(struct
> > platform_device *pdev)
> > >  	if (ret)
> > >  		return ret;
> > >
> > > -	regmap_field_read(cdns_phy->phy_pma_cmn_ctrl_1,
> > &already_configured);
> > > -
> > > -	if (!already_configured) {
> > > -		ret = cdns_torrent_reset(cdns_phy);
> > > -		if (ret)
> > > -			goto clk_cleanup;
> > > +	ret = cdns_torrent_reset(cdns_phy);
> > > +	if (ret)
> > > +		goto clk_cleanup;
> > >
> > > -		ret = cdns_torrent_clk(cdns_phy);
> > > -		if (ret)
> > > -			goto clk_cleanup;
> > > +	ret = cdns_torrent_clk(cdns_phy);
> > > +	if (ret)
> > > +		goto clk_cleanup;
> > >
> > >  		/* Enable APB */
> > > -		reset_control_deassert(cdns_phy->apb_rst);
> > > -	}
> > > +	reset_control_deassert(cdns_phy->apb_rst);
> > >
> > >  	for_each_available_child_of_node(dev->of_node, child) {
> > >  		struct phy *gphy;
> > > @@ -2404,10 +2394,7 @@ static int cdns_torrent_phy_probe(struct
> > platform_device *pdev)
> > >  		of_property_read_u32(child, "cdns,ssc-mode",
> > >  				     &cdns_phy->phys[node].ssc_mode);
> > >
> > > -		if (!already_configured)
> > > -			gphy = devm_phy_create(dev, child,
> > &cdns_torrent_phy_ops);
> > > -		else
> > > -			gphy = devm_phy_create(dev, child, &noop_ops);
> > > +		gphy = devm_phy_create(dev, child, &cdns_torrent_phy_ops);
> > >  		if (IS_ERR(gphy)) {
> > >  			ret = PTR_ERR(gphy);
> > >  			goto put_child;
> > > @@ -2490,7 +2477,7 @@ static int cdns_torrent_phy_probe(struct
> > platform_device *pdev)
> > >  		goto put_lnk_rst;
> > >  	}
> > >
> > > -	if (cdns_phy->nsubnodes > 1 && !already_configured) {
> > > +	if (cdns_phy->nsubnodes > 1) {
> > >  		ret = cdns_torrent_phy_configure_multilink(cdns_phy);
> > >  		if (ret)
> > >  			goto put_lnk_rst;
> > > --
> > > 2.33.1
> > >
> > >
> > >
> > **************************************************************
> > ********
> > > This email and attachments contain Ambarella Proprietary and/or Confidential
> > Information and is intended solely for the use of the individual(s) to whom it is
> > addressed. Any unauthorized review, use, disclosure, distribute, copy, or print is
> > prohibited. If you are not an intended recipient, please contact the sender by
> > reply email and destroy all copies of the original message. Thank you.

You do realize this is not OK in public emails... Pls fix

> > 
> > Aha!!!
> > 
> > Okay destroyed this now!
> > 
> > --
> > ~Vinod
> > 
> > ##############################################################
> > ########
> > This EXTERNAL email has been scanned by Proofpoint Email Protect service.
> 
> Regards,
> Li

-- 
~Vinod

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy