Re: [PATCH v7 3/5] memory: mediatek: Add SMI driver

2016-01-19 Thread Yong Wu 
On Mon, 2016-01-18 at 11:11 +0100, Matthias Brugger wrote:
> 
> On 18/12/15 09:09, Yong Wu wrote:
> > This patch add SMI(Smart Multimedia Interface) driver. This driver
> > is responsible to enable/disable iommu and control the power domain
> > and clocks of each local arbiter.
> >
> > Signed-off-by: Yong Wu 

[...]

> > +int mtk_smi_larb_get(struct device *larbdev)
> > +{
> > +   struct mtk_smi_larb *larb = dev_get_drvdata(larbdev);
> > +   struct mtk_smi *common = dev_get_drvdata(larb->smi_common_dev);
> > +   int ret;
> > +
> > +   /* Enable the smi-common's power and clocks */
> > +   ret = mtk_smi_enable(common);
> > +   if (ret)
> > +   return ret;
> > +
> > +   /* Enable the larb's power and clocks */
> > +   ret = mtk_smi_enable(>smi);
> > +   if (ret) {
> > +   mtk_smi_disable(common);
> > +   return ret;
> > +   }
> > +
> > +   /* Configure the iommu info */
> > +   if (larb->mmu)
> > +   writel(*larb->mmu, larb->base + SMI_LARB_MMU_EN);
> 
> Can there be a larb that does not have a mmu id defined?
> Phandles for the iommu need always to have a mtk_m4u_id defined, so I 
> don't see the case where this can happen?

  No. In current IC(mt8173), All the larbs have the mmu info register.

> Why did you changed this value to a pointer and added the if?

  I changed it to a pointer in order to support
iommu_attach_device/iommu_detach_device dynamically.

  In our v6, there is a interface called mtk_smi_config_port in which
the iommu(M4U) could tell SMI which ports should enable iommu.
  In this version, we use the additional data of component to finish
this job(the third parameter of the mtk_smi_larb_bind). then we could
delete that interface.

  larb->mmu is a pointer that point to the mmu in "struct mtk_smi_iommu
smi_imu" of "struct mtk_iommu_data". This "smi_imu" is used to record
that which ports has already enabled iommu for each larb and it will be
updated during the iommu_attach_device/iommu_detach_device.
 
   Why add the if?
  -> This pointer may be null in the mtk_smi_larb_unbind.
  I'm also not sure when it will enter mtk_smi_larb_unbind, maybe while
the iommu device is removed, then it call component_master_del, or the
larb device itself is removed, or some other error case in component.
but there really is a path it will be null, so I add this *if*.
   If you think it is unnecessary, I can delete it.

> 
> > +
> > +   return 0;
> > +}
> > +
> > +void mtk_smi_larb_put(struct device *larbdev)
> > +{
> > +   struct mtk_smi_larb *larb = dev_get_drvdata(larbdev);
> > +   struct mtk_smi *common = dev_get_drvdata(larb->smi_common_dev);
> > +
> 
> Would it make sense to write SMI_LARB_MMU_EN here again to eventually 
> turn off mmus?

I think no. As you know, there may be some modules in one larb, so we
cann't write 0 in SMI_LARB_MMU_EN here to turn off mmus for all modules.

mtk_smi_larb_get/mtk_smi_larb_put mainly enable/disable the power domain
and clocks for this local arbiter. And the larb is a unit here, SMI
cann’t detect which module calls mtk_smi_larb_put and disable his
special iommu bits. so do mtk_smi_larb_get.
 
I know that the module’s special iommu bit wasn’t wrote to 0 here, but
if this module would like to work again, he have to call
mtk_smi_larb_get again, and currently all the multimedia modules always
works via iommu, we don’t need to write 0 here.


> Other then this, the driver looks fine to me.

Thanks very much for your reply. I could send the new version if we get
a conclusion for the two issue above here, like you really don't like
that *if*.

> 
> Regards,
> Matthias
> 
> > +   mtk_smi_disable(>smi);
> > +   mtk_smi_disable(common);
> > +}
> > +
> > +static int
> > +mtk_smi_larb_bind(struct device *dev, struct device *master, void *data)
> > +{
> > +   struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> > +   struct mtk_smi_iommu *smi_iommu = data;
> > +   unsigned int i;
> > +
> > +   for (i = 0; i < smi_iommu->larb_nr; i++) {
> > +   if (dev == smi_iommu->larb_imu[i].dev) {
> > +   larb->mmu = _iommu->larb_imu[i].mmu;
> > +   return 0;
> > +   }
> > +   }
> > +   return -ENODEV;
> > +}
> > +
> > +static void
> > +mtk_smi_larb_unbind(struct device *dev, struct device *master, void *data)
> > +{
> > +   struct mtk_smi_larb *larb = dev_get_drvdata(dev);
> > +
> > +   larb->mmu = NULL;
> > +}
> > +
> > +static const struct component_ops mtk_smi_larb_component_ops = {
> > +   .bind = mtk_smi_larb_bind,
> > +   .unbind = mtk_smi_larb_unbind,
> > +};

[...]


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v1] iommu/rockchip: reconstruct to support multi slaves

2016-01-19 Thread Shunqian Zheng 
From: ZhengShunQian 

There are some IPs, such as video encoder/decoder, contains 2 slave iommus,
one for reading and the other for writing. They share the same irq and
clock with master.

This patch reconstructs to support this case by making them share the same
Page Directory, Page Tables and even the register operations.
That means every instruction to the reading MMU registers would be
duplicated to the writing MMU and vice versa.

Signed-off-by: ZhengShunQian 
---
 drivers/iommu/rockchip-iommu.c | 214 ++---
 1 file changed, 135 insertions(+), 79 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index ebf0adb..a6f593a 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -86,7 +86,8 @@ struct rk_iommu_domain {
 
 struct rk_iommu {
struct device *dev;
-   void __iomem *base;
+   void __iomem **bases;
+   int num_mmu;
int irq;
struct list_head node; /* entry in rk_iommu_domain.iommus */
struct iommu_domain *domain; /* domain to which iommu is attached */
@@ -271,47 +272,70 @@ static u32 rk_iova_page_offset(dma_addr_t iova)
return (u32)(iova & RK_IOVA_PAGE_MASK) >> RK_IOVA_PAGE_SHIFT;
 }
 
-static u32 rk_iommu_read(struct rk_iommu *iommu, u32 offset)
+static u32 rk_iommu_read(void __iomem *base, u32 offset)
 {
-   return readl(iommu->base + offset);
+   return readl(base + offset);
 }
 
-static void rk_iommu_write(struct rk_iommu *iommu, u32 offset, u32 value)
+static void rk_iommu_write(void __iomem *base, u32 offset, u32 value)
 {
-   writel(value, iommu->base + offset);
+   writel(value, base + offset);
 }
 
 static void rk_iommu_command(struct rk_iommu *iommu, u32 command)
 {
-   writel(command, iommu->base + RK_MMU_COMMAND);
+   int i;
+
+   for (i = 0; i < iommu->num_mmu; i++)
+   writel(command, iommu->bases[i] + RK_MMU_COMMAND);
 }
 
+static void rk_iommu_base_command(void __iomem *base, u32 command)
+{
+   writel(command, base + RK_MMU_COMMAND);
+}
 static void rk_iommu_zap_lines(struct rk_iommu *iommu, dma_addr_t iova,
   size_t size)
 {
+   int i;
+
dma_addr_t iova_end = iova + size;
/*
 * TODO(djkurtz): Figure out when it is more efficient to shootdown the
 * entire iotlb rather than iterate over individual iovas.
 */
-   for (; iova < iova_end; iova += SPAGE_SIZE)
-   rk_iommu_write(iommu, RK_MMU_ZAP_ONE_LINE, iova);
+   for (i = 0; i < iommu->num_mmu; i++)
+   for (; iova < iova_end; iova += SPAGE_SIZE)
+   rk_iommu_write(iommu->bases[i], RK_MMU_ZAP_ONE_LINE, 
iova);
 }
 
 static bool rk_iommu_is_stall_active(struct rk_iommu *iommu)
 {
-   return rk_iommu_read(iommu, RK_MMU_STATUS) & RK_MMU_STATUS_STALL_ACTIVE;
+   bool active = true;
+   int i;
+
+   for (i = 0; i < iommu->num_mmu; i++)
+   active &= rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
+   RK_MMU_STATUS_STALL_ACTIVE;
+
+   return active;
 }
 
 static bool rk_iommu_is_paging_enabled(struct rk_iommu *iommu)
 {
-   return rk_iommu_read(iommu, RK_MMU_STATUS) &
-RK_MMU_STATUS_PAGING_ENABLED;
+   bool enable = true;
+   int i;
+
+   for (i = 0; i < iommu->num_mmu; i++)
+   enable &= rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
+   RK_MMU_STATUS_PAGING_ENABLED;
+
+   return enable;
 }
 
 static int rk_iommu_enable_stall(struct rk_iommu *iommu)
 {
-   int ret;
+   int ret, i;
 
if (rk_iommu_is_stall_active(iommu))
return 0;
@@ -324,15 +348,16 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu)
 
ret = rk_wait_for(rk_iommu_is_stall_active(iommu), 1);
if (ret)
-   dev_err(iommu->dev, "Enable stall request timed out, status: 
%#08x\n",
-   rk_iommu_read(iommu, RK_MMU_STATUS));
+   for (i = 0; i < iommu->num_mmu; i++)
+   dev_err(iommu->dev, "Enable stall request timed out, 
status: %#08x\n",
+   rk_iommu_read(iommu->bases[i], RK_MMU_STATUS));
 
return ret;
 }
 
 static int rk_iommu_disable_stall(struct rk_iommu *iommu)
 {
-   int ret;
+   int ret, i;
 
if (!rk_iommu_is_stall_active(iommu))
return 0;
@@ -341,15 +366,16 @@ static int rk_iommu_disable_stall(struct rk_iommu *iommu)
 
ret = rk_wait_for(!rk_iommu_is_stall_active(iommu), 1);
if (ret)
-   dev_err(iommu->dev, "Disable stall request timed out, status: 
%#08x\n",
-   rk_iommu_read(iommu, RK_MMU_STATUS));
+   for (i = 0; i < iommu->num_mmu; i++)
+   dev_err(iommu->dev, "Disable stall

[git pull] IOMMU Updates for Linux v4.5

2016-01-19 Thread Joerg Roedel 
Hi Linus,

The following changes since commit afd2ff9b7e1b367172f18ba7f693dfb62bdcb2dc:

  Linux 4.4 (2016-01-10 15:01:32 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git 
tags/iommu-updates-v4.5

for you to fetch changes up to 32704253dc008dfedead71da016b00d10cd1854f:

  Merge branches 's390', 'arm/renesas', 'arm/msm', 'arm/shmobile', 'arm/smmu', 
'x86/amd' and 'x86/vt-d' into next (2016-01-19 15:30:43 +0100)



IOMMU Updates for Linux v4.5

The updates include:

* Small code cleanups in the AMD IOMMUv2 driver

* Scalability improvements for the DMA-API implementation of the
  AMD IOMMU driver. This is just a starting point, but already
  showed some good improvements in my tests.

* Removal of the unused Renesas IPMMU/IPMMUI driver

* Updates for ARM-SMMU include:

* Some fixes to get the driver working nicely on
  Broadcom hardware

* A change to the io-pgtable API to indicate the unit in
  which to flush (all callers converted, with Ack from
  Laurent)

* Use of devm_* for allocating/freeing the SMMUv3
  buffers

* Some other small fixes and improvements for other drivers


Dan Carpenter (1):
  iommu/amd: Remove an unneeded condition

Geert Uytterhoeven (1):
  iommu/shmobile: Remove unused Renesas IPMMU/IPMMUI driver

Joerg Roedel (28):
  iommu/amd: Correctly set flags for handle_mm_fault call
  iommu/amd: Cleanup error handling in do_fault()
  Merge branch 'for-joerg/arm-smmu/updates' of 
git://git.kernel.org/.../will/linux into arm/smmu
  iommu/amd: Warn only once on unexpected pte value
  iommu/amd: Move 'struct dma_ops_domain' definition to amd_iommu.c
  iommu/amd: Introduce bitmap_lock in struct aperture_range
  iommu/amd: Flush IOMMU TLB on __map_single error path
  iommu/amd: Flush the IOMMU TLB before the addresses are freed
  iommu/amd: Pass correct shift to iommu_area_alloc()
  iommu/amd: Add dma_ops_aperture_alloc() function
  iommu/amd: Move aperture_range.offset to another cache-line
  iommu/amd: Retry address allocation within one aperture
  iommu/amd: Flush iommu tlb in dma_ops_aperture_alloc()
  iommu/amd: Remove 'start' parameter from dma_ops_area_alloc
  iommu/amd: Rename dma_ops_domain->next_address to next_index
  iommu/amd: Flush iommu tlb in dma_ops_free_addresses
  iommu/amd: Iterate over all aperture ranges in dma_ops_area_alloc
  iommu/amd: Remove need_flush from struct dma_ops_domain
  iommu/amd: Optimize dma_ops_free_addresses
  iommu/amd: Allocate new aperture ranges in dma_ops_alloc_addresses
  iommu/amd: Build io page-tables with cmpxchg64
  iommu/amd: Initialize new aperture range before making it visible
  iommu/amd: Relax locking in dma_ops path
  iommu/amd: Make dma_ops_domain->next_index percpu
  iommu/amd: Use trylock to aquire bitmap_lock
  iommu/amd: Preallocate dma_ops apertures based on dma_mask
  iommu/vt-d: Fix up error handling in alloc_iommu
  Merge branches 's390', 'arm/renesas', 'arm/msm', 'arm/shmobile', 
'arm/smmu', 'x86/amd' and 'x86/vt-d' into next

Julia Lawall (1):
  iommu/amd: Constify mmu_notifier_ops structures

Magnus Damm (1):
  iommu/ipmmu-vmsa: Include SoC part number in DT binding docs

Markus Elfring (1):
  iommu/arm-smmu: Delete an unnecessary check before free_io_pgtable_ops()

Nicholas Krause (1):
  iommu/vt-d: Check the return value of iommu_device_create()

Peng Fan (1):
  iommu/arm-smmu: Correct group reference count

Prem Mallappa (2):
  iommu/arm-smmu: Fix write to GERRORN register
  iommu/arm-smmu: Use STE.S1STALLD only when supported

Robin Murphy (4):
  iommu/io-pgtable-arm: Avoid dereferencing bogus PTEs
  iommu/io-pgtable: Indicate granule for TLB maintenance
  iommu/arm-smmu: Invalidate TLBs properly
  iommu/io-pgtable: Make io_pgtable_ops_to_pgtable() macro common

Sebastian Ott (1):
  iommu/s390: Fix sparse warnings

Thierry Reding (1):
  iommu/msm: Use platform_register/unregister_drivers()

Will Deacon (5):
  iommu/arm-smmu: Remove #define for non-existent PRIQ_0_OF field
  iommu/arm-smmu: Convert DMA buffer allocations to the managed API
  iommu/arm-smmu: Use incoming shareability attributes in bypass mode
  iommu/arm-smmu: Handle unknown CERROR values gracefully
  iommu/io-pgtable-arm: Ensure we free the final level on teardown

 .../bindings/iommu/renesas,ipmmu-vmsa.txt  |  12 +-
 drivers/iommu/Kconfig  |  75 
 drivers/iommu/Makefile |   2 -
 drivers/iommu/amd_iommu.c  | 396