Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1240329 > unrolled thread

Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API Christoph Hellwig <hch@infradead.org> - 2015-10-06 12:00 +0200
    Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API Nishanth Aravamudan <nacc@linux.vnet.ibm.com> - 2015-10-12 18:10 +0200
    Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API Nishanth Aravamudan <nacc@linux.vnet.ibm.com> - 2015-10-12 23:10 +0200
      Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API Nishanth Aravamudan <nacc@linux.vnet.ibm.com> - 2015-10-14 17:40 +0200
        Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API Christoph Hellwig <hch@infradead.org> - 2015-10-14 17:50 +0200
          Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API Nishanth Aravamudan <nacc@linux.vnet.ibm.com> - 2015-10-16 01:00 +0200

#1240329 — Re: [PATCH 1/5 v2] dma-mapping: add generic dma_get_page_shift API

Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
with an #ifndef in common code?

Also not all architectures use dma-mapping-common.h yet, so you either
need to update all of those as well, or just add the #ifndef directly
to linux/dma-mapping.h.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1244861

On 06.10.2015 [02:51:36 -0700], Christoph Hellwig wrote:
> Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
> with an #ifndef in common code?

I suppose we could do that -- I wasn't sure if the macro would be
palatable.

> Also not all architectures use dma-mapping-common.h yet, so you either
> need to update all of those as well, or just add the #ifndef directly
> to linux/dma-mapping.h.

Ok, I will take a look at that and spin up a v3.

Thanks for the feedback!

-Nish

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

[toc] | [prev] | [next] | [standalone]

#1245139

On 06.10.2015 [02:51:36 -0700], Christoph Hellwig wrote:
> Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
> with an #ifndef in common code?

On Power, since it's technically variable, we'd need a function. So are
you suggesting define'ing it to a function just on Power and leaving it
a constant elsewhere?

I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.

-Nish

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

[toc] | [prev] | [next] | [standalone]

#1246911

Hi Christoph,

On 12.10.2015 [14:06:51 -0700], Nishanth Aravamudan wrote:
> On 06.10.2015 [02:51:36 -0700], Christoph Hellwig wrote:
> > Do we need a function here or can we just have a IOMMU_PAGE_SHIFT define
> > with an #ifndef in common code?
> 
> On Power, since it's technically variable, we'd need a function. So are
> you suggesting define'ing it to a function just on Power and leaving it
> a constant elsewhere?
> 
> I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.

Sorry, I should have been more specific -- I'm ready to spin out a v3,
with a sparc-specific function.

Are you ok with leaving it a function for now (the only caller is in
NVMe obviously).

-Nish

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

[toc] | [prev] | [next] | [standalone]

#1246922

Hi Nishanth,

sorry for the late reply.

> > On Power, since it's technically variable, we'd need a function. So are
> > you suggesting define'ing it to a function just on Power and leaving it
> > a constant elsewhere?
> > 
> > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> 
> Sorry, I should have been more specific -- I'm ready to spin out a v3,
> with a sparc-specific function.
> 
> Are you ok with leaving it a function for now (the only caller is in
> NVMe obviously).


I guess we do indeed need a function then.  I'll take a look at your
patch, but as long you found a way to avoid adding too much boilerplate
code it should be fine.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248238

On 14.10.2015 [08:42:51 -0700], Christoph Hellwig wrote:
> Hi Nishanth,
> 
> sorry for the late reply.
> 
> > > On Power, since it's technically variable, we'd need a function. So are
> > > you suggesting define'ing it to a function just on Power and leaving it
> > > a constant elsewhere?
> > > 
> > > I noticed that sparc has a IOMMU_PAGE_SHIFT already, fwiw.
> > 
> > Sorry, I should have been more specific -- I'm ready to spin out a v3,
> > with a sparc-specific function.
> > 
> > Are you ok with leaving it a function for now (the only caller is in
> > NVMe obviously).
> 
> 
> I guess we do indeed need a function then.  I'll take a look at your
> patch, but as long you found a way to avoid adding too much boilerplate
> code it should be fine.

Ok, so I've got the moved function (include/linux/dma-mapping.h instead
of dma-mapping-common.h) ready to go, which should only involve changing
the first patch in the series. But I'm really mystified by what to do
for sparc, which defines IOMMU_PAGE_SHIFT and IO_PAGE_SHIFT in
arch/sparc/kernel/iommu_common.h.

1) Which constant reflects the value we mean for this function on sparc?
I assume it should be IOMMU_PAGE_SHIFT, but they are the same value and
I want to make sure I get the semantics right.

2) Where would I put sparc's definition of dma_get_page_shift()? Should
it be in a asm/dma-mapping.h? Should we move some of the constants from
arch/sparc/kernel/iommu_common.h to
arch/sparc/include/asm/iommu_common.h and then #include that in
asm/dma-mapping.h?

Dave M., any opinions/insights? Essentially, this helper function
assists the NVMe driver in determining what page size it should use to
satisfy both the device and IOMMU's requirements. Maybe I misunderstand
the constants on sparc and PAGE_SHIFT is fine there too?

-Nish

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web