linux-kernel - Re: [PATCH v2] mmc: pxamci: Simplify pxamci_probe() error handling using devm APIs

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]

Message-ID: <pvid2ycmgbkbmegnnzwl4hyev6e2smusxk5olkuqxfwxzykz2e@jlvolirolrxl>
Date: Tue, 21 Oct 2025 10:31:25 +0200
From: Uwe Kleine-König <u.kleine-koenig@...libre.com>
To: Rakuram Eswaran <rakuram.e96@...il.com>
Cc: chenhuacai@...nel.org, dan.carpenter@...aro.org, 
	david.hunter.linux@...il.com, khalid@...nel.org, linux-kernel-mentees@...ts.linux.dev, 
	linux-kernel@...r.kernel.org, linux-mmc@...r.kernel.org, lkp@...el.com, 
	skhan@...uxfoundation.org, ulf.hansson@...aro.org, zhoubinbin@...ngson.cn
Subject: Re: [PATCH v2] mmc: pxamci: Simplify pxamci_probe() error handling
 using devm APIs

Hello Rakuram,

On Tue, Oct 21, 2025 at 12:02:07AM +0530, Rakuram Eswaran wrote:
> On Thu, 16 Oct 2025 at 14:20, Uwe Kleine-König <u.kleine-koenig@...libre.com> wrote:
> > On Wed, Oct 15, 2025 at 12:16:57AM +0530, Rakuram Eswaran wrote:
> Sorry for the delayed reply as I was in vacation. 

I didn't hold my breath :-O

> Ah, got it — I’ll drop the clk_get_rate() comment since it was only a reminder
> from your WIP suggestion.
> 
> Just to confirm, are you referring to adding a call to clk_prepare_enable()
> before clk_get_rate()? I can move the clk_get_rate() call after
> clk_prepare_enable(), or drop the comment entirely.
> 
> If my understanding is correct, I’ll keep v3 focused on the current set of
> fixes and handle the clk_get_rate() precondition (by moving it after
> clk_prepare_enable()) in a follow-up patch. That should keep the scope of each
> change clean and review-friendly.
> 
> > > -out:
> > > -     if (host->dma_chan_rx)
> > > -             dma_release_channel(host->dma_chan_rx);
> > > -     if (host->dma_chan_tx)
> > > -             dma_release_channel(host->dma_chan_tx);
> >
> > I was lazy in my prototype patch and didn't drop the calls to
> > dma_release_channel() in pxamci_remove(). For a proper patch this is
> > required though.
> >
> > To continue the quest: Now that I looked at pxamci_remove(): `mmc` is
> > always non-NULL, so the respective check can be dropped.
> >
> 
> Understood. Since pxamci_remove() is only called after successful allocation
> and initialization in probe(), mmc will always be a valid pointer. I’ll drop
> the if (mmc) check in v3 as it can never be NULL in normal operation, and
> remove the dma_release_channel() calls as well.

Yes, I suggest to make restructuring .remote a separate patch. (But
removing dma_release_channel belongs into the patch that introduces devm
to allocate the dma channels.)
 
> I’ve prepared a preview of the v3 patch incorporating your previous comments.
> Before sending it out formally, I wanted to share it with you to confirm that
> the updates look good — especially the cleanup changes in pxamci_remove() and
> the dropped clk_get_rate() comment.
> 
> static void pxamci_remove(struct platform_device *pdev)
> {
> 	struct mmc_host *mmc = platform_get_drvdata(pdev);
> 	struct pxamci_host *host = mmc_priv(mmc);
> 
> 	mmc_remove_host(mmc);
> 
> 	if (host->pdata && host->pdata->exit)
> 		host->pdata->exit(&pdev->dev, mmc);
> 
> 	pxamci_stop_clock(host);
> 	writel(TXFIFO_WR_REQ|RXFIFO_RD_REQ|CLK_IS_OFF|STOP_CMD|
> 			END_CMD_RES|PRG_DONE|DATA_TRAN_DONE,
> 			host->base + MMC_I_MASK);
> 
> 	dmaengine_terminate_all(host->dma_chan_rx);
> 	dmaengine_terminate_all(host->dma_chan_tx);
> }

Looks right.

Best regards
Uwe

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)