Re: [PATCH kernel v9 17/32] powerpc/powernv: Implement accessor to TCE entry

David Gibson
2015-04-30T00:38:30Z

[-- Attachment #1: Type: text/plain, Size: 2494 bytes --]

On Wed, Apr 29, 2015 at 07:02:17PM +1000, Alexey Kardashevskiy wrote:
> On 04/29/2015 02:04 PM, David Gibson wrote:
> >On Sat, Apr 25, 2015 at 10:14:41PM +1000, Alexey Kardashevskiy wrote:
> >>This replaces direct accesses to TCE table with a helper which
> >>returns an TCE entry address. This does not make difference now but will
> >>when multi-level TCE tables get introduces.
> >>
> >>No change in behavior is expected.
> >>
> >>Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> >
> >Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> >
> >
> >>---
> >>Changes:
> >>v9:
> >>* new patch in the series to separate this mechanical change from
> >>functional changes; this is not right before
> >>"powerpc/powernv: Implement multilevel TCE tables" but here in order
> >>to let the next patch - "powerpc/iommu/powernv: Release replaced TCE" -
> >>use pnv_tce() and avoid changing the same code twice
> >>---
> >> arch/powerpc/platforms/powernv/pci.c | 34 +++++++++++++++++++++-------------
> >> 1 file changed, 21 insertions(+), 13 deletions(-)
> >>
> >>diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> >>index 84b4ea4..ba75aa5 100644
> >>--- a/arch/powerpc/platforms/powernv/pci.c
> >>+++ b/arch/powerpc/platforms/powernv/pci.c
> >>@@ -572,38 +572,46 @@ struct pci_ops pnv_pci_ops = {
> >> .write = pnv_pci_write_config,
> >> };
> >>
> >>+static __be64 *pnv_tce(struct iommu_table *tbl, long idx)
> >>+{
> >>+ __be64 *tmp = ((__be64 *)tbl->it_base);
> >>+
> >>+ return tmp + idx;
> >>+}
> >>+
> >> int pnv_tce_build(struct iommu_table *tbl, long index, long npages,
> >> unsigned long uaddr, enum dma_data_direction direction,
> >> struct dma_attrs *attrs)
> >> {
> >> u64 proto_tce = iommu_direction_to_tce_perm(direction);
> >>- __be64 *tcep;
> >>- u64 rpn;
> >>+ u64 rpn = __pa(uaddr) >> tbl->it_page_shift;
> >
> >I guess this was a problem in the existing code, not this patch. But
> >"uaddr" is a really bad name (and unsigned long is a bad type) for
> >what must actually be a kernel linear mapping address.
>
>
> Yes and may be one day I'll clean this up. s/uaddr/linear/ and s/hwaddr/hpa/
> are the first things to do globally but not in this patchset.

Ok.

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]