All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frank Li <Frank.li@oss.nxp.com>
To: Michal Pecio <michal.pecio@gmail.com>
Cc: frank.li@nxp.com, corbet@lwn.net, dmaengine@vger.kernel.org,
	hch@infradead.org, imx@lists.linux.dev,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, lizhijian@fujitsu.com, mst@redhat.com,
	rdunlap@infradead.org
Subject: Re: [PATCH v2 1/1] docs: dma: correct dma_set_mask() sample code
Date: Mon, 17 Aug 2026 10:50:35 -0500	[thread overview]
Message-ID: <aoMty9Dvt2bnWm74@SMW015318> (raw)
In-Reply-To: <20260816071044.331a2c51.michal.pecio@gmail.com>

On Sun, Aug 16, 2026 at 07:10:44AM +0200, Michal Pecio wrote:
> [You don't often get email from michal.pecio@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Hi,
>
> > There are bunch of codes in driver like
> >
> >        if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)))
> >                dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))
> >
> > Actually it is wrong because if dma_set_mask_and_coherent(64) fails,
> > dma_set_mask_and_coherent(32) will fail for the same reason.
>
> I encountered similar driver code and found it similarly suspicious.
> I arrived here searching for reasons to convince myself (and relevant
> maintainers) that removing this is indeed the right thing to do.
>
> But I have a few remaining questions and remarks.
>
> > And dma_set_mask_and_coherent(64) never returns failure.
>
> No realistic chance of the dev->dma_mask check (below) giving -EIO?

dma_mask is pointer, which already initilized by bus driver before call to
probe.

For example

https://elixir.bootlin.com/linux/v7.1.8/source/drivers/base/platform.c#L634

If you find one, which bus driver have not init it, please high light it.

So far safe check !dev->dma_mask is reduntant.

>
> > According to the definition of dma_set_mask(), it indicates the width
> > of address that device DMA can access. If it can access 64-bit
> > address, it must access 32-bit address inherently. So only need set
> > biggest address width.
> >
> > See below code fragment:
> >
> > dma_set_mask(mask)
> > {
> >       mask = (dma_addr_t)mask;
> >
> >       if (!dev->dma_mask || !dma_supported(dev, mask))
> >               return -EIO;
> >
> >       arch_dma_set_mask(dev, mask);
> >       *dev->dma_mask = mask;
> >       return 0;
> > }
> >
> > dma_supported() will call dma_direct_supported or iommux's
> > dma_supported call back function.
>
> Aapparently, it may also use some 'dma_map_ops' and there is a bunch
> of those spread over drivers/ and arch/. But I gather they are expected
> to behave similarly as the functions named above?

I grep it and checked at that time, all return 1 when >= 32. Now more
powerfull check tools avaible, you can double check it.

>
> > --- a/Documentation/core-api/dma-api-howto.rst
> > +++ b/Documentation/core-api/dma-api-howto.rst
> >
> > +The standard 64-bit addressing device would do something like this::
> > +
> > +     dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))
> > +
> > +dma_set_mask_and_coherent() never return fail when DMA_BIT_MASK(64). Typical
> > +error code like::
> > +
> > +     /* Wrong code */
> > +     if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)))
> > +             dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))
> > +
> > +dma_set_mask_and_coherent() will never return failure when bigger then 32.
> > +So typical code like::
> > +
> > +     /* Recommended code */
> > +     if (support_64bit)
> > +             dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
> > +     else
> > +             dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > +
>
> This text is unclear. Two sentences begin with "Typical error code",
> then some code is quoted, and the sentences are cut abruptly without
> actually making any statement about the code in question.

Sorry, I still not understand what your means.

Anyways, now dt's "dma-ranges" is more userful now.

Frank

>
> Thanks,
> Michal
>

  reply	other threads:[~2026-08-17 15:50 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-01 17:41 [PATCH v2 1/1] docs: dma: correct dma_set_mask() sample code Frank Li
2024-04-02 15:09 ` Christoph Hellwig
2024-04-02 15:11   ` Jonathan Corbet
2024-04-02 15:43 ` Niklas Cassel
2024-04-02 16:08   ` Jonathan Corbet
2026-08-16  5:10 ` Michal Pecio
2026-08-17 15:50   ` Frank Li [this message]
2026-08-17 17:17     ` Michal Pecio
2026-08-17 18:43       ` Frank Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aoMty9Dvt2bnWm74@SMW015318 \
    --to=frank.li@oss.nxp.com \
    --cc=corbet@lwn.net \
    --cc=dmaengine@vger.kernel.org \
    --cc=frank.li@nxp.com \
    --cc=hch@infradead.org \
    --cc=imx@lists.linux.dev \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lizhijian@fujitsu.com \
    --cc=michal.pecio@gmail.com \
    --cc=mst@redhat.com \
    --cc=rdunlap@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.