RE: [PATCH] omap_hsmmc: Fix for the db clock failure message

From: Madhusudhan
Date: Tue Aug 18 2009 - 13:11:25 EST

Next message: Mario Limonciello: "Re: [PATCH 2/3] Add rfkill support to compal-laptop"
Previous message: Bruno PrÃmont: "Re: 2.6.31-rc5 regression: Oops when USB Serial disconnected whilein use"
In reply to: Madhusudhan: "RE: [PATCH] omap_hsmmc: Fix for the db clock failure message"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

> -----Original Message-----
> From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Madhusudhan
> Sent: Tuesday, August 18, 2009 10:27 AM
> To: 'Adrian Hunter'
> Cc: akpm@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> omap@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxxxxx
> Subject: RE: [PATCH] omap_hsmmc: Fix for the db clock failure message
>
>
>
> > -----Original Message-----
> > From: Adrian Hunter [mailto:adrian.hunter@xxxxxxxxx]
> > Sent: Tuesday, August 18, 2009 1:53 AM
> > To: Madhusudhan Chikkature
> > Cc: akpm@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> > omap@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxxxxx
> > 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@xxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxx
> > > 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@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Mario Limonciello: "Re: [PATCH 2/3] Add rfkill support to compal-laptop"
Previous message: Bruno PrÃmont: "Re: 2.6.31-rc5 regression: Oops when USB Serial disconnected whilein use"
In reply to: Madhusudhan: "RE: [PATCH] omap_hsmmc: Fix for the db clock failure message"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]