linux-kernel - Re: [PATCH 5/5] omap mmc: Add new omap hsmmc controller for 2430 and 34xx

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: <20081230083624.GD7850@atomide.com>
Date:	Tue, 30 Dec 2008 10:36:26 +0200
From:	Tony Lindgren <tony@...mide.com>
To:	Pierre Ossman <drzeus-mmc@...eus.cx>
Cc:	linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.arm.linux.org.uk,
	linux-omap@...r.kernel.org,
	Madhusudhan Chikkature <madhu.cr@...com>,
	David Brownell <david-b@...bell.net>
Subject: Re: [PATCH 5/5] omap mmc: Add new omap hsmmc controller for 2430
	and 34xx

* Tony Lindgren <tony@...mide.com> [081229 18:56]:
> Hi,
> 
> * Pierre Ossman <drzeus-mmc@...eus.cx> [081221 18:17]:
> > On Sun, 07 Dec 2008 13:51:39 -0800
> > Tony Lindgren <tony@...mide.com> wrote:
> > 
> > > +	/*
> > > +	 * Unlike OMAP1 controller, the cmdtype does not seem to be based on
> > > +	 * ac, bc, adtc, bcr. Only CMD12 needs a val of 0x3, rest 0x0.
> > > +	 */
> > > +	if (cmd->opcode == 12)
> > > +		cmdtype = 0x3;
> > 
> > Isn't it more likely that it needs 0x3 for any commands that ends an
> > open ended transfer? I.e. every time cmd == mrq.stop.
> 
> OK, changed and still works :)
> 
> > > +	host->dbclk = clk_get(&pdev->dev, "mmchsdb_fck");
> > > +	/*
> > > +	 * MMC can still work without debounce clock.
> > > +	 */
> > > +	if (IS_ERR(host->dbclk))
> > > +		dev_dbg(mmc_dev(host->mmc), "Failed to get debounce clock\n");
> > 
> > Perhaps a warning should be printed though, as there could be some
> > problems caused by trying to init a card that is still bouncing.
> 
> OK
> 
> > > +static int omap_mmc_remove(struct platform_device *pdev)
> > > +{
> > > +	struct mmc_omap_host *host = platform_get_drvdata(pdev);
> > > +	struct resource *res;
> > > +	u16 vdd = 0;
> > > +
> > > +	if (!(OMAP_HSMMC_READ(host->base, HCTL) & SDVSDET)) {
> > > +	/*
> > > +	 * Set the vdd back to 3V,
> > > +	 * applicable for dual volt support.
> > > +	 */
> > > +		vdd = fls(host->mmc->ocr_avail) - 1;
> > > +		if (omap_mmc_switch_opcond(host, vdd) != 0)
> > > +			host->mmc->ios.vdd = vdd;
> > > +	}
> > > +
> > > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > +	if (res)
> > > +		release_mem_region(res->start, res->end - res->start + 1);
> > > +
> > > +	platform_set_drvdata(pdev, NULL);
> > > +	if (host) {
> > > +		mmc_remove_host(host->mmc);
> > 
> > You can't go fiddling with the voltage and removing the MMIO until
> > you've made sure that there is no more activity on the slot.
> 
> Changed in the attached patch. I've changed the code to reset
> the controller voltage setting after powering down the socket
> instead of trying to do it in two earlier places..
> 
> Do you have any better suggestions for this? Basically it looks
> like the controller may fail to detect the next card after unplugging
> a 1.8V card unless the controller is reset to 3V.
> 
> > Fix the above issues and you can let it go via Russell with a:
> > 
> > Acked-by: Pierre Ossman <drzeus@...eus.cx>
> 
> Thank you for your comments. Care to take a one more look at the
> attached patch before I add your Ack? I also fixed the remaining
> checkpatch warnings.

Here's one more version that leaves out the unnecessary TWL4030
dependency in the Kconfig as pointed out by David Brownell.

> Regards,
> 
> Tony

View attachment "hsmmc-v3.patch" of type "text/x-diff" (33685 bytes)