linux-kernel - Re: [PATCH 9/9] remoteproc/wkup_m3: Use reset control driver if available

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: <20201026130238.GA5639@atomide.com>
Date:   Mon, 26 Oct 2020 15:02:38 +0200
From:   Tony Lindgren <tony@...mide.com>
To:     Philipp Zabel <p.zabel@...gutronix.de>
Cc:     linux-omap@...r.kernel.org, "Andrew F . Davis" <afd@...com>,
        Dave Gerlach <d-gerlach@...com>,
        Faiz Abbas <faiz_abbas@...com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Grygorii Strashko <grygorii.strashko@...com>,
        Keerthy <j-keerthy@...com>, Nishanth Menon <nm@...com>,
        Peter Ujfalusi <peter.ujfalusi@...com>,
        Roger Quadros <rogerq@...com>, Suman Anna <s-anna@...com>,
        Tero Kristo <t-kristo@...com>, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-remoteproc@...r.kernel.org,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        Stephen Boyd <sboyd@...nel.org>, linux-clk@...r.kernel.org
Subject: Re: [PATCH 9/9] remoteproc/wkup_m3: Use reset control driver if
 available

Hi,

* Philipp Zabel <p.zabel@...gutronix.de> [201026 11:35]:
> On Mon, 2020-10-26 at 13:10 +0200, Tony Lindgren wrote:
> > @@ -57,6 +60,9 @@ static int wkup_m3_rproc_start(struct rproc *rproc)
> >  	struct device *dev = &pdev->dev;
> >  	struct wkup_m3_platform_data *pdata = dev_get_platdata(dev);
> >  
> > +	if (wkupm3->rsts)
> 
> No need for this check, reset_control_deassert() just returns 0 if the
> rstc parameter is NULL.
> 
> > +		return reset_control_deassert(wkupm3->rsts);
> > +
> >  	if (pdata->deassert_reset(pdev, pdata->reset_name)) {
> >  		dev_err(dev, "Unable to reset wkup_m3!\n");
> >  		return -ENODEV;
> > @@ -72,6 +78,9 @@ static int wkup_m3_rproc_stop(struct rproc *rproc)
> >  	struct device *dev = &pdev->dev;
> >  	struct wkup_m3_platform_data *pdata = dev_get_platdata(dev);
> >  
> > +	if (wkupm3->rsts)
> 
> Same as above.

OK great.

> > +		return reset_control_assert(wkupm3->rsts);
> > +
> >  	if (pdata->assert_reset(pdev, pdata->reset_name)) {
> >  		dev_err(dev, "Unable to assert reset of wkup_m3!\n");
> >  		return -ENODEV;
> > @@ -132,12 +141,6 @@ static int wkup_m3_rproc_probe(struct platform_device *pdev)
> >  	int ret;
> >  	int i;
> >  
> > -	if (!(pdata && pdata->deassert_reset && pdata->assert_reset &&
> > -	      pdata->reset_name)) {
> > -		dev_err(dev, "Platform data missing!\n");
> > -		return -ENODEV;
> > -	}
> > -
> >  	ret = of_property_read_string(dev->of_node, "ti,pm-firmware",
> >  				      &fw_name);
> >  	if (ret) {
> > @@ -165,6 +168,17 @@ static int wkup_m3_rproc_probe(struct platform_device *pdev)
> >  	wkupm3->rproc = rproc;
> >  	wkupm3->pdev = pdev;
> >  
> > +	wkupm3->rsts = devm_reset_control_get_optional_shared(dev, "rstctrl");
> > +	if (PTR_ERR_OR_ZERO(wkupm3->rsts)) {
> 
> Please properly return errors. rsts will be NULL if the optional rstctrl
> reset is not specified:
> 
> 	if (IS_ERR(wkump3->rsts))
> 		return PTR_ERR(wkump3->rsts);

OK thanks will do.

> 	if (!wkump3->rsts) {
> > +		if (!(pdata && pdata->deassert_reset && pdata->assert_reset &&
> > +		      pdata->reset_name)) {
> > +			dev_err(dev, "Platform data missing!\n");
> > +			ret = -ENODEV;
> > +			goto err_put_rproc;
> > +		}
> > +		wkupm3->rsts = NULL;
> 
> I assume this will later be dropped with the platform data support?

Yes once also am437x has been updated to probe with dts data only we
can drop the custom callbacks.

Regards,

Tony