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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YcRvL85x9Q1dkvRn@matsya>
Date:   Thu, 23 Dec 2021 18:14:31 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Li Chen <lchen@...arella.com>
Cc:     Kishon Vijay Abraham I <kishon@...com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Swapnil Jakhade <sjakhade@...ence.com>,
        "linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [EXT] Re: [PATCH] Revert "phy: cadence-torrent: Do not configure
 SERDES if it's already configured"

On 23-12-21, 06:27, Li Chen wrote:
> Hi, Vinod
> 
> > -----Original Message-----
> > From: Vinod Koul [mailto:vkoul@...nel.org]
> > Sent: Thursday, December 23, 2021 1:27 PM
> > To: Li Chen
> > Cc: Kishon Vijay Abraham I; Philipp Zabel; Swapnil Jakhade; linux-
> > phy@...ts.infradead.org; linux-kernel@...r.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@...arella.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