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] [day] [month] [year] [list]
Date:	Tue, 18 Aug 2009 12:10:25 -0500
From:	"Madhusudhan" <madhu.cr@...com>
To:	"'Adrian Hunter'" <adrian.hunter@...ia.com>
Cc:	<akpm@...ux-foundation.org>, <linux-kernel@...r.kernel.org>,
	<linux-omap@...r.kernel.org>,
	<linux-arm-kernel@...ts.arm.linux.org.uk>
Subject: RE: [PATCH] omap_hsmmc: Fix for the db clock failure message



> -----Original Message-----
> From: linux-omap-owner@...r.kernel.org [mailto:linux-omap-
> owner@...r.kernel.org] On Behalf Of Madhusudhan
> Sent: Tuesday, August 18, 2009 10:27 AM
> To: 'Adrian Hunter'
> Cc: akpm@...ux-foundation.org; linux-kernel@...r.kernel.org; linux-
> omap@...r.kernel.org; linux-arm-kernel@...ts.arm.linux.org.uk
> Subject: RE: [PATCH] omap_hsmmc: Fix for the db clock failure message
> 
> 
> 
> > -----Original Message-----
> > From: Adrian Hunter [mailto:adrian.hunter@...ia.com]
> > Sent: Tuesday, August 18, 2009 1:53 AM
> > To: Madhusudhan Chikkature
> > Cc: akpm@...ux-foundation.org; linux-kernel@...r.kernel.org; linux-
> > omap@...r.kernel.org; linux-arm-kernel@...ts.arm.linux.org.uk
> > Subject: Re: [PATCH] omap_hsmmc: Fix for the db clock failure message
> >
> > Madhusudhan Chikkature wrote:
> > > This patch applies on top of the series "[PATCH V2 0/32] mmc and
> > omap_hsmmc
> > > patches" posted by Adrian Hunter.
> > >
> > > -------------------------------------------------
> > >
> > >
> > >
> > > This patch removes the error message "Failed to get debounce clock.."
> > printed
> > > out by the HSMMC driver on OMAP3430. The debounce clock needs to be
> > handled only
> > > for OMAP2430.
> >
> > I have a suggestion that may make it tidier.
> >
> > What about renaming host->dbclk_enabled to host->got_dbclk and leaving
> it
> > alone
> > after it is set in the probe function.  Then create a macro to use in
> > conditions:
> >
> > #define have_dbclk(host) (cpu_is_omap2430() && (host)->got_dbclk)
> >
> > So the following:
> >
> > -	if (host->dbclk_enabled)
> > +	if (cpu_is_omap2430() && host->dbclk_enabled) {
> >  		clk_disable(host->dbclk);
> > +		host->dbclk_enabled = 0;
> > +	}
> >
> > becomes:
> >
> > -	if (host->dbclk_enabled)
> > +	if (have_dbclk(host))
> >  		clk_disable(host->dbclk);
> >
> > Or alternatively, forget the macro, and let the code always compile in:
> >
> > -	if (host->dbclk_enabled)
> > +	if (host->got_dbclk)
> >  		clk_disable(host->dbclk);
> >
> In both the cases how about the enable path? We want to enable the
> debounce
> clock only for OMAP2430, Right? Otherwise we will still end up throwing an
> error message for 3430 if we check the result of clk_enable for db clock.
> 
> Regards,
> Madhu
> 
I got your point after looking at the patch again. I guess you want to set
the "host->got_dbclk" only once in probe if the clk_get succeeds for db clk
and handle clk_enable/disable only based on got_dbclk. That should be fine.
I will resubmit the patch.

Regards,
Madhu
> >
> > >
> > > Signed-off-by: Madhusudhan Chikkature <madhu.cr@...com>
> > > ---
> > >  drivers/mmc/host/omap_hsmmc.c |   63 +++++++++++++++++++++++++++-----
> --
> > --------
> > >  1 file changed, 41 insertions(+), 22 deletions(-)
> > >
> > > Index: linux-2.6/drivers/mmc/host/omap_hsmmc.c
> > > ===================================================================
> > > --- linux-2.6.orig/drivers/mmc/host/omap_hsmmc.c
> > > +++ linux-2.6/drivers/mmc/host/omap_hsmmc.c
> > > @@ -735,8 +735,10 @@ static int omap_hsmmc_switch_opcond(stru
> > >  	/* Disable the clocks */
> > >  	clk_disable(host->fclk);
> > >  	clk_disable(host->iclk);
> > > -	if (host->dbclk_enabled)
> > > +	if (cpu_is_omap2430() && host->dbclk_enabled) {
> > >  		clk_disable(host->dbclk);
> > > +		host->dbclk_enabled = 0;
> > > +	}
> > >
> > >  	/* Turn the power off */
> > >  	ret = mmc_slot(host).set_power(host->dev, host->slot_id, 0, 0);
> > > @@ -746,9 +748,14 @@ static int omap_hsmmc_switch_opcond(stru
> > >  		ret = mmc_slot(host).set_power(host->dev, host->slot_id, 1,
> > >  					       vdd);
> > >  	clk_enable(host->iclk);
> > > -	if (host->dbclk_enabled)
> > > -		clk_enable(host->dbclk);
> > >  	clk_enable(host->fclk);
> > > +	if (cpu_is_omap2430() && !host->dbclk_enabled) {
> > > +		if (clk_enable(host->dbclk) != 0)
> > > +			dev_dbg(mmc_dev(host->mmc), "Enabling debounce"
> > > +						" clk failed\n");
> > > +		else
> > > +			host->dbclk_enabled = 1;
> > > +	}
> > >
> > >  	if (ret != 0)
> > >  		goto err;
> > > @@ -1697,18 +1704,21 @@ static int __init omap_hsmmc_probe(struc
> > >  		goto err1;
> > >  	}
> > >
> > > -	host->dbclk = clk_get(&pdev->dev, "mmchsdb_fck");
> > > -	/*
> > > -	 * MMC can still work without debounce clock.
> > > -	 */
> > > -	if (IS_ERR(host->dbclk))
> > > -		dev_warn(mmc_dev(host->mmc), "Failed to get debounce
> > clock\n");
> > > -	else
> > > -		if (clk_enable(host->dbclk) != 0)
> > > -			dev_dbg(mmc_dev(host->mmc), "Enabling debounce"
> > > -							" clk failed\n");
> > > +	if (cpu_is_omap2430()) {
> > > +		host->dbclk = clk_get(&pdev->dev, "mmchsdb_fck");
> > > +		/*
> > > +		 * MMC can still work without debounce clock.
> > > +		 */
> > > +		if (IS_ERR(host->dbclk))
> > > +			dev_warn(mmc_dev(host->mmc),
> > > +				"Failed to get debounce clock\n");
> > >  		else
> > > -			host->dbclk_enabled = 1;
> > > +			if (clk_enable(host->dbclk) != 0)
> > > +				dev_dbg(mmc_dev(host->mmc), "Enabling
> debounce"
> > > +							" clk failed\n");
> > > +			else
> > > +				host->dbclk_enabled = 1;
> > > +	}
> > >
> > >  	/* Since we do only SG emulation, we can have as many segs
> > >  	 * as we want. */
> > > @@ -1825,8 +1835,9 @@ err_irq:
> > >  	clk_disable(host->iclk);
> > >  	clk_put(host->fclk);
> > >  	clk_put(host->iclk);
> > > -	if (host->dbclk_enabled) {
> > > -		clk_disable(host->dbclk);
> > > +	if (cpu_is_omap2430()) {
> > > +		if (host->dbclk_enabled)
> > > +			clk_disable(host->dbclk);
> > >  		clk_put(host->dbclk);
> > >  	}
> > >
> > > @@ -1859,8 +1870,9 @@ static int omap_hsmmc_remove(struct plat
> > >  		clk_disable(host->iclk);
> > >  		clk_put(host->fclk);
> > >  		clk_put(host->iclk);
> > > -		if (host->dbclk_enabled) {
> > > -			clk_disable(host->dbclk);
> > > +		if (cpu_is_omap2430()) {
> > > +			if (host->dbclk_enabled)
> > > +				clk_disable(host->dbclk);
> > >  			clk_put(host->dbclk);
> > >  		}
> > >
> > > @@ -1910,8 +1922,10 @@ static int omap_hsmmc_suspend(struct pla
> > >  				OMAP_HSMMC_READ(host->base, HCTL) & ~SDBP);
> > >  			mmc_host_disable(host->mmc);
> > >  			clk_disable(host->iclk);
> > > -			if (host->dbclk_enabled)
> > > +			if (cpu_is_omap2430() && host->dbclk_enabled) {
> > >  				clk_disable(host->dbclk);
> > > +				host->dbclk_enabled = 0;
> > > +			}
> > >  		} else {
> > >  			host->suspended = 0;
> > >  			if (host->pdata->resume) {
> > > @@ -1942,14 +1956,19 @@ static int omap_hsmmc_resume(struct plat
> > >  		if (ret)
> > >  			goto clk_en_err;
> > >
> > > -		if (host->dbclk_enabled)
> > > -			clk_enable(host->dbclk);
> > > -
> > >  		if (mmc_host_enable(host->mmc) != 0) {
> > >  			clk_disable(host->iclk);
> > >  			goto clk_en_err;
> > >  		}
> > >
> > > +		if (cpu_is_omap2430() && !host->dbclk_enabled) {
> > > +			if (clk_enable(host->dbclk) != 0)
> > > +				dev_dbg(mmc_dev(host->mmc), "Enabling
> debounce"
> > > +							" clk failed\n");
> > > +			else
> > > +				host->dbclk_enabled = 1;
> > > +		}
> > > +
> > >  		omap_hsmmc_conf_bus_power(host);
> > >
> > >  		if (host->pdata->resume) {
> > >
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-omap"
> in
> > > the body of a message to majordomo@...r.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > >
> >
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ