Re: [PATCH v9 5/5] iommu/arm-smmu: Add support for qcom,smmu-v2 variant

2018-04-10 Thread Vivek Gautam 

Hi Yisheng,


On 4/11/2018 6:52 AM, Yisheng Xie wrote:

Hi Tomasz,

On 2018/4/10 21:14, Tomasz Figa wrote:

Hi Yisheng,

Sorry, I think we missed your question here.

On Wed, Mar 28, 2018 at 3:12 PM Yisheng Xie  wrote:


Hi Vivek,
On 2018/3/28 12:37, Vivek Gautam wrote:

Hi Yisheng


On 3/28/2018 6:54 AM, Yisheng Xie wrote:

Hi Vivek,

On 2018/3/13 16:55, Vivek Gautam wrote:

+- power-domains:  Specifiers for power domains required to be

powered on for

+  the SMMU to operate, as per generic power domain

bindings.

+

In this patchset, power-domains is not used right? And you just do the

clock gating,

but not power gating, right?

We are handling the power-domains too. Please see the example in this

binding doc.


I see, but I do not find the point in code of these patchset, do you mean

PMIC(e.g mmcc)

will gate the power domain of SMMU(e.g. MDSS_GDSC of mmcc) when PMIC

suspend?


If respective SoC power domains is registered as a standard genpd PM
domain, then the runtime PM subsystem will take care of power domain
control at runtime suspend and resume.


Get it, thanks for your explain, I should have learned about this.


Sorry, i missed your subsequent question, and Tomasz has explained it now.
Let me know if you have further questions.

regards
Vivek


Thanks
Yisheng


Best regards,
Tomasz

.


--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

Re: [PATCH v9 5/5] iommu/arm-smmu: Add support for qcom,smmu-v2 variant

2018-04-10 Thread Yisheng Xie 
Hi Tomasz,

On 2018/4/10 21:14, Tomasz Figa wrote:
> Hi Yisheng,
> 
> Sorry, I think we missed your question here.
> 
> On Wed, Mar 28, 2018 at 3:12 PM Yisheng Xie  wrote:
> 
>> Hi Vivek,
> 
>> On 2018/3/28 12:37, Vivek Gautam wrote:
>>> Hi Yisheng
>>>
>>>
>>> On 3/28/2018 6:54 AM, Yisheng Xie wrote:
 Hi Vivek,

 On 2018/3/13 16:55, Vivek Gautam wrote:
> +- power-domains:  Specifiers for power domains required to be
> powered on for
> +  the SMMU to operate, as per generic power domain
> bindings.
> +
 In this patchset, power-domains is not used right? And you just do the
> clock gating,
 but not power gating, right?
>>>
>>> We are handling the power-domains too. Please see the example in this
> binding doc.
> 
>> I see, but I do not find the point in code of these patchset, do you mean
> PMIC(e.g mmcc)
>> will gate the power domain of SMMU(e.g. MDSS_GDSC of mmcc) when PMIC
> suspend?
> 
> 
> If respective SoC power domains is registered as a standard genpd PM
> domain, then the runtime PM subsystem will take care of power domain
> control at runtime suspend and resume.
> 

Get it, thanks for your explain, I should have learned about this.

Thanks
Yisheng

> Best regards,
> Tomasz
> 
> .
> 

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

Re: [PATCH v3 2/2] drivers: remove force dma flag from buses

2018-04-10 Thread Bjorn Helgaas 
On Fri, Mar 30, 2018 at 01:24:45PM +0530, Nipun Gupta wrote:
> With each bus implementing its own DMA configuration callback,
> there is no need for bus to explicitly have force_dma in its
> global structure. This patch modifies of_dma_configure API to
> accept an input parameter which specifies if implicit DMA
> configuration is required even when it is not described by the
> firmware.
> 
> Signed-off-by: Nipun Gupta 

Acked-by: Bjorn Helgaas   # PCI parts

> ---
> Changes in v2:
>   - This is a new change suggested by Robin and Christoph
> and is added to the series.
> 
> Changes in v3:
>   - Rebase and changes corresponding to the changes in patch 1/2
> 
>  drivers/amba/bus.c| 1 -
>  drivers/base/platform.c   | 3 +--
>  drivers/bcma/main.c   | 2 +-
>  drivers/dma/qcom/hidma_mgmt.c | 2 +-
>  drivers/gpu/host1x/bus.c  | 5 ++---
>  drivers/of/device.c   | 6 --
>  drivers/of/of_reserved_mem.c  | 2 +-
>  drivers/pci/pci-driver.c  | 3 +--
>  include/linux/device.h| 4 
>  include/linux/of_device.h | 8 ++--
>  10 files changed, 17 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index 867dc2b..abe73c4 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -199,7 +199,6 @@ struct bus_type amba_bustype = {
>   .uevent = amba_uevent,
>   .dma_configure  = platform_dma_configure,
>   .pm = _pm,
> - .force_dma  = true,
>  };
>  
>  static int __init amba_init(void)
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 72fdbf6..cfbc569 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1136,7 +1136,7 @@ int platform_dma_configure(struct device *dev)
>   int ret = 0;
>  
>   if (dev->of_node) {
> - ret = of_dma_configure(dev, dev->of_node);
> + ret = of_dma_configure(dev, dev->of_node, true);
>   } else if (has_acpi_companion(dev)) {
>   attr = acpi_get_dma_attr(to_acpi_device_node(dev->fwnode));
>   if (attr != DEV_DMA_NOT_SUPPORTED)
> @@ -1159,7 +1159,6 @@ struct bus_type platform_bus_type = {
>   .uevent = platform_uevent,
>   .dma_configure  = platform_dma_configure,
>   .pm = _dev_pm_ops,
> - .force_dma  = true,
>  };
>  EXPORT_SYMBOL_GPL(platform_bus_type);
>  
> diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
> index e6986c7..fc1f4ac 100644
> --- a/drivers/bcma/main.c
> +++ b/drivers/bcma/main.c
> @@ -207,7 +207,7 @@ static void bcma_of_fill_device(struct device *parent,
>  
>   core->irq = bcma_of_get_irq(parent, core, 0);
>  
> - of_dma_configure(>dev, node);
> + of_dma_configure(>dev, node, false);
>  }
>  
>  unsigned int bcma_core_irq(struct bcma_device *core, int num)
> diff --git a/drivers/dma/qcom/hidma_mgmt.c b/drivers/dma/qcom/hidma_mgmt.c
> index 000c7019..d64edeb 100644
> --- a/drivers/dma/qcom/hidma_mgmt.c
> +++ b/drivers/dma/qcom/hidma_mgmt.c
> @@ -398,7 +398,7 @@ static int __init hidma_mgmt_of_populate_channels(struct 
> device_node *np)
>   }
>   of_node_get(child);
>   new_pdev->dev.of_node = child;
> - of_dma_configure(_pdev->dev, child);
> + of_dma_configure(_pdev->dev, child, true);
>   /*
>* It is assumed that calling of_msi_configure is safe on
>* platforms with or without MSI support.
> diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> index a9ec99d..b39c1e9 100644
> --- a/drivers/gpu/host1x/bus.c
> +++ b/drivers/gpu/host1x/bus.c
> @@ -317,7 +317,7 @@ static int host1x_device_match(struct device *dev, struct 
> device_driver *drv)
>  static int host1x_dma_configure(struct device *dev)
>  {
>   if (dev->of_node)
> - return of_dma_configure(dev, dev->of_node);
> + return of_dma_configure(dev, dev->of_node, true);
>   return 0;
>  }
>  
> @@ -335,7 +335,6 @@ struct bus_type host1x_bus_type = {
>   .match = host1x_device_match,
>   .dma_configure  = host1x_dma_configure,
>   .pm = _device_pm_ops,
> - .force_dma = true,
>  };
>  
>  static void __host1x_device_del(struct host1x_device *device)
> @@ -424,7 +423,7 @@ static int host1x_device_add(struct host1x *host1x,
>   device->dev.bus = _bus_type;
>   device->dev.parent = host1x->dev;
>  
> - of_dma_configure(>dev, host1x->dev->of_node);
> + of_dma_configure(>dev, host1x->dev->of_node, true);
>  
>   err = host1x_device_parse_dt(device, driver);
>   if (err < 0) {
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index 064c818..33d8551 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -76,6 +76,8 @@ int of_device_add(struct platform_device *ofdev)
>   * of_dma_configure - Setup DMA configuration
>   * @dev: Device to apply DMA configuration
>   * @np:

Re: [PATCH v3 1/2] dma-mapping: move dma configuration to bus infrastructure

2018-04-10 Thread Bjorn Helgaas 
On Fri, Mar 30, 2018 at 01:24:44PM +0530, Nipun Gupta wrote:
> It is bus specific aspect to map a given device on the bus and
> relevant firmware description of its DMA configuration.
> So, this change introduces 'dma_configure' as bus callback
> giving flexibility to busses for implementing its own dma
> configuration function.
> 
> The change eases the addition of new busses w.r.t. adding the dma
> configuration functionality.
> 
> This patch also updates the PCI, Platform, ACPI and host1x bus to
> use new introduced callbacks.

s/dma/DMA/ consistently above.

> Suggested-by: Christoph Hellwig 
> Signed-off-by: Nipun Gupta 
> Reviewed-by: Greg Kroah-Hartman 

Acked-by: Bjorn Helgaas   # PCI parts

I assume you'll merge this via some non-PCI tree.  Let me know if you
need anything else from me.

> ---
>  - The patches are based on the comments on:
>https://patchwork.kernel.org/patch/10259087/
> 
> Changes in v2:
>   - Do not have dma_deconfigure callback
>   - Have '/dma_common_configure/' API to provide a common DMA
> configuration which can be used by busses if it suits them.
>   - Platform and ACPI bus to use '/dma_common_configure/' in
> '/dma_configure/' callback.
>   - Updated commit message
>   - Updated pci_dma_configure API with changes suggested by Robin
> 
> Changes in v3
>   - Move dma_common_configure() within platform_dma_configure() and
> reuse platofrm_dma_configure() for AMBA bus too
>   - Declare 'attr' in pci_dma_configure() inside the else statement
> where it is used.
> 
>  drivers/amba/bus.c  |  4 
>  drivers/base/dma-mapping.c  | 31 ---
>  drivers/base/platform.c | 17 +
>  drivers/gpu/host1x/bus.c|  8 
>  drivers/pci/pci-driver.c| 32 
>  include/linux/device.h  |  4 
>  include/linux/platform_device.h |  2 ++
>  7 files changed, 71 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index 594c228..867dc2b 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> @@ -188,12 +189,15 @@ static int amba_pm_runtime_resume(struct device *dev)
>  /*
>   * Primecells are part of the Advanced Microcontroller Bus Architecture,
>   * so we call the bus "amba".
> + * DMA configuration for platform and AMBA bus is same. So here we reuse
> + * platform's DMA config routine.
>   */
>  struct bus_type amba_bustype = {
>   .name   = "amba",
>   .dev_groups = amba_dev_groups,
>   .match  = amba_match,
>   .uevent = amba_uevent,
> + .dma_configure  = platform_dma_configure,
>   .pm = _pm,
>   .force_dma  = true,
>  };
> diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c
> index 3b11835..fdc1502 100644
> --- a/drivers/base/dma-mapping.c
> +++ b/drivers/base/dma-mapping.c
> @@ -331,36 +331,13 @@ void dma_common_free_remap(void *cpu_addr, size_t size, 
> unsigned long vm_flags)
>  #endif
>  
>  /*
> - * Common configuration to enable DMA API use for a device
> + * enables DMA API use for a device
>   */
> -#include 
> -
>  int dma_configure(struct device *dev)
>  {
> - struct device *bridge = NULL, *dma_dev = dev;
> - enum dev_dma_attr attr;
> - int ret = 0;
> -
> - if (dev_is_pci(dev)) {
> - bridge = pci_get_host_bridge_device(to_pci_dev(dev));
> - dma_dev = bridge;
> - if (IS_ENABLED(CONFIG_OF) && dma_dev->parent &&
> - dma_dev->parent->of_node)
> - dma_dev = dma_dev->parent;
> - }
> -
> - if (dma_dev->of_node) {
> - ret = of_dma_configure(dev, dma_dev->of_node);
> - } else if (has_acpi_companion(dma_dev)) {
> - attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));
> - if (attr != DEV_DMA_NOT_SUPPORTED)
> - ret = acpi_dma_configure(dev, attr);
> - }
> -
> - if (bridge)
> - pci_put_host_bridge_device(bridge);
> -
> - return ret;
> + if (dev->bus->dma_configure)
> + return dev->bus->dma_configure(dev);
> + return 0;
>  }
>  
>  void dma_deconfigure(struct device *dev)
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index f1bf7b3..72fdbf6 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -1130,6 +1130,22 @@ int platform_pm_restore(struct device *dev)
>  
>  #endif /* CONFIG_HIBERNATE_CALLBACKS */
>  
> +int platform_dma_configure(struct device *dev)
> +{
> + enum dev_dma_attr attr;
> + int ret = 0;
> +
> + if (dev->of_node) {
> + ret = of_dma_configure(dev, dev->of_node);
> + } else if (has_acpi_companion(dev)) {
> + attr =

Re: [PATCH 03/37] iommu/sva: Manage process address spaces

2018-04-10 Thread Sinan Kaya 
On 2/12/2018 1:33 PM, Jean-Philippe Brucker wrote:
> +static void io_mm_detach_all_locked(struct iommu_bond *bond)
> +{
> + while (!io_mm_detach_locked(bond));
> +}
> +

I don't remember if I mentioned this before or not but I think this loop
needs a little bit relaxation with yield and maybe an informational message
with might help if wait exceeds some time.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures

2018-04-10 Thread Christoph Hellwig 
On Tue, Apr 10, 2018 at 06:50:04PM +0100, Robin Murphy wrote:
> In the first one, the machine appears to have enough RAM that most of it is 
> beyond the device's 36-bit DMA mask, thus it's fairly likely for the 
> initial direct alloc to come back with something unsuitable.

But we should try a GFP_DMA32 allocation first, so this is a bit
surprising.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures

2018-04-10 Thread Robin Murphy 

On 10/04/18 18:07, Takashi Iwai wrote:

On Tue, 10 Apr 2018 19:06:15 +0200,
Christoph Hellwig wrote:


On Tue, Apr 10, 2018 at 07:05:13PM +0200, Takashi Iwai wrote:

The code refactoring by commit 0176adb00406 ("swiotlb: refactor
coherent buffer allocation") made swiotlb_alloc_buffer() almost always
failing due to a thinko: namely, the function evaluates the
dma_coherent_ok() call incorrectly and dealing as if it's invalid.
This ends up with weird errors like iwlwifi probe failure or amdgpu
screen flickering.

This patch corrects the logic error.


This looks ok, although even hitting this code is a bad sign.  Do you
know why the previous dma_direct_alloc didn't succeed?


That I have no idea, sorry.  I just figured out the regression just
from the dmesg output of 4.16 kernel in bugzilla reports.
Could you take a look at the bugzilla reports there?


In the first one, the machine appears to have enough RAM that most of it 
is beyond the device's 36-bit DMA mask, thus it's fairly likely for the 
initial direct alloc to come back with something unsuitable.


Robin.




thanks,

Takashi


Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088658
Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088902
Fixes: 0176adb00406 ("swiotlb: refactor coherent buffer allocation")
Cc:  # v4.16+
Signed-off-by: Takashi Iwai 
---
  lib/swiotlb.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 47aeb04c1997..de7cc540450f 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -719,7 +719,7 @@ swiotlb_alloc_buffer(struct device *dev, size_t size, 
dma_addr_t *dma_handle,
goto out_warn;
  
  	*dma_handle = __phys_to_dma(dev, phys_addr);

-   if (dma_coherent_ok(dev, *dma_handle, size))
+   if (!dma_coherent_ok(dev, *dma_handle, size))
goto out_unmap;
  
  	memset(phys_to_virt(phys_addr), 0, size);

--
2.16.3

---end quoted text---


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


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

[PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures

2018-04-10 Thread Takashi Iwai 
The code refactoring by commit 0176adb00406 ("swiotlb: refactor
coherent buffer allocation") made swiotlb_alloc_buffer() almost always
failing due to a thinko: namely, the function evaluates the
dma_coherent_ok() call incorrectly and dealing as if it's invalid.
This ends up with weird errors like iwlwifi probe failure or amdgpu
screen flickering.

This patch corrects the logic error.

Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088658
Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088902
Fixes: 0176adb00406 ("swiotlb: refactor coherent buffer allocation")
Cc:  # v4.16+
Signed-off-by: Takashi Iwai 
---
 lib/swiotlb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 47aeb04c1997..de7cc540450f 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -719,7 +719,7 @@ swiotlb_alloc_buffer(struct device *dev, size_t size, 
dma_addr_t *dma_handle,
goto out_warn;
 
*dma_handle = __phys_to_dma(dev, phys_addr);
-   if (dma_coherent_ok(dev, *dma_handle, size))
+   if (!dma_coherent_ok(dev, *dma_handle, size))
goto out_unmap;
 
memset(phys_to_virt(phys_addr), 0, size);
-- 
2.16.3

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

Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures

2018-04-10 Thread Takashi Iwai 
On Tue, 10 Apr 2018 19:06:15 +0200,
Christoph Hellwig wrote:
> 
> On Tue, Apr 10, 2018 at 07:05:13PM +0200, Takashi Iwai wrote:
> > The code refactoring by commit 0176adb00406 ("swiotlb: refactor
> > coherent buffer allocation") made swiotlb_alloc_buffer() almost always
> > failing due to a thinko: namely, the function evaluates the
> > dma_coherent_ok() call incorrectly and dealing as if it's invalid.
> > This ends up with weird errors like iwlwifi probe failure or amdgpu
> > screen flickering.
> > 
> > This patch corrects the logic error.
> 
> This looks ok, although even hitting this code is a bad sign.  Do you
> know why the previous dma_direct_alloc didn't succeed?

That I have no idea, sorry.  I just figured out the regression just
from the dmesg output of 4.16 kernel in bugzilla reports.
Could you take a look at the bugzilla reports there?


thanks,

Takashi

> > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088658
> > Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088902
> > Fixes: 0176adb00406 ("swiotlb: refactor coherent buffer allocation")
> > Cc:  # v4.16+
> > Signed-off-by: Takashi Iwai 
> > ---
> >  lib/swiotlb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> > index 47aeb04c1997..de7cc540450f 100644
> > --- a/lib/swiotlb.c
> > +++ b/lib/swiotlb.c
> > @@ -719,7 +719,7 @@ swiotlb_alloc_buffer(struct device *dev, size_t size, 
> > dma_addr_t *dma_handle,
> > goto out_warn;
> >  
> > *dma_handle = __phys_to_dma(dev, phys_addr);
> > -   if (dma_coherent_ok(dev, *dma_handle, size))
> > +   if (!dma_coherent_ok(dev, *dma_handle, size))
> > goto out_unmap;
> >  
> > memset(phys_to_virt(phys_addr), 0, size);
> > -- 
> > 2.16.3
> ---end quoted text---
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] swiotlb: Fix unexpected swiotlb_alloc_coherent() failures

2018-04-10 Thread Christoph Hellwig 
On Tue, Apr 10, 2018 at 07:05:13PM +0200, Takashi Iwai wrote:
> The code refactoring by commit 0176adb00406 ("swiotlb: refactor
> coherent buffer allocation") made swiotlb_alloc_buffer() almost always
> failing due to a thinko: namely, the function evaluates the
> dma_coherent_ok() call incorrectly and dealing as if it's invalid.
> This ends up with weird errors like iwlwifi probe failure or amdgpu
> screen flickering.
> 
> This patch corrects the logic error.

This looks ok, although even hitting this code is a bad sign.  Do you
know why the previous dma_direct_alloc didn't succeed?

> 
> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088658
> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1088902
> Fixes: 0176adb00406 ("swiotlb: refactor coherent buffer allocation")
> Cc:  # v4.16+
> Signed-off-by: Takashi Iwai 
> ---
>  lib/swiotlb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index 47aeb04c1997..de7cc540450f 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -719,7 +719,7 @@ swiotlb_alloc_buffer(struct device *dev, size_t size, 
> dma_addr_t *dma_handle,
>   goto out_warn;
>  
>   *dma_handle = __phys_to_dma(dev, phys_addr);
> - if (dma_coherent_ok(dev, *dma_handle, size))
> + if (!dma_coherent_ok(dev, *dma_handle, size))
>   goto out_unmap;
>  
>   memset(phys_to_virt(phys_addr), 0, size);
> -- 
> 2.16.3
---end quoted text---
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v9 5/5] iommu/arm-smmu: Add support for qcom, smmu-v2 variant

2018-04-10 Thread Tomasz Figa 
Hi Yisheng,

Sorry, I think we missed your question here.

On Wed, Mar 28, 2018 at 3:12 PM Yisheng Xie  wrote:

> Hi Vivek,

> On 2018/3/28 12:37, Vivek Gautam wrote:
> > Hi Yisheng
> >
> >
> > On 3/28/2018 6:54 AM, Yisheng Xie wrote:
> >> Hi Vivek,
> >>
> >> On 2018/3/13 16:55, Vivek Gautam wrote:
> >>> +- power-domains:  Specifiers for power domains required to be
powered on for
> >>> +  the SMMU to operate, as per generic power domain
bindings.
> >>> +
> >> In this patchset, power-domains is not used right? And you just do the
clock gating,
> >> but not power gating, right?
> >
> > We are handling the power-domains too. Please see the example in this
binding doc.

> I see, but I do not find the point in code of these patchset, do you mean
PMIC(e.g mmcc)
> will gate the power domain of SMMU(e.g. MDSS_GDSC of mmcc) when PMIC
suspend?


If respective SoC power domains is registered as a standard genpd PM
domain, then the runtime PM subsystem will take care of power domain
control at runtime suspend and resume.

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

Re: [PATCH 2/2] iommu/rockchip: make clock handling optional

2018-04-10 Thread Robin Murphy 

On 10/04/18 10:26, Heiko Stuebner wrote:

iommu clocks are optional, so the driver should not fail if they are not
present. Instead just set the number of clocks to 0, which the clk-blk APIs
can handle just fine.

Fixes: f2e3a5f557ad ("iommu/rockchip: Control clocks needed to access the 
IOMMU")
Signed-off-by: Heiko Stuebner 
---
  drivers/iommu/rockchip-iommu.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 5fc8656c60f9..7215c683cb8f 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -1176,8 +1176,12 @@ static int rk_iommu_probe(struct platform_device *pdev)
iommu->clocks[i].id = rk_iommu_clocks[i];
  
  	err = devm_clk_bulk_get(iommu->dev, iommu->num_clocks, iommu->clocks);

-   if (err)
-   return err;
+   if (err) {
+   if (err == -ENOENT)
+   iommu->num_clocks = 0;
+   else
+   return err;
+   }


Nit: this might be a bit nicer as:

if (err == -ENOENT)
iommu->num_clocks = 0;
else if (err)
return err;

Either way,

Reviewed-by: Robin Murphy 

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

Re: [PATCH 1/2] dt-bindings: iommu/rockchip: Make clock properties optional

2018-04-10 Thread Robin Murphy 

On 10/04/18 10:26, Heiko Stuebner wrote:

Rockchip IOMMUs are used without explicit clock handling for 4 years
now, so we should keep compatibility with old devicetrees if possible.
Therefore make iommu clocks optional.


Do we need to touch the binding itself? Obviously the driver has to 
treat clocks as optional in existing DTs (and I feel a bit dumb now for 
managing to overlook that in review), but the binding effectively only 
covers future DTs, and I'd assume we want to encourage the clocks to be 
correctly specified there.


Robin.


Fixes: 8fa9eb39c614 ("dt-bindings: iommu/rockchip: Add clock property")
Signed-off-by: Heiko Stuebner 
---
  Documentation/devicetree/bindings/iommu/rockchip,iommu.txt | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt 
b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
index 6ecefea1c6f9..25bfad987513 100644
--- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
@@ -14,13 +14,13 @@ Required properties:
  "single-master" device, and needs no additional 
information
  to associate with its master device.  See:
  Documentation/devicetree/bindings/iommu/iommu.txt
+
+Optional properties:
  - clocks  : A list of clocks required for the IOMMU to be accessible 
by
  the host CPU.
  - clock-names : Should contain the following:
"iface" - Main peripheral bus clock (PCLK/HCL) (required)
"aclk"  - AXI bus clock (required)
-
-Optional properties:
  - rockchip,disable-mmu-reset : Don't use the mmu reset operation.
   Some mmu instances may produce unexpected results
   when the reset operation is used.


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

[PATCH 1/2] dt-bindings: iommu/rockchip: Make clock properties optional

2018-04-10 Thread Heiko Stuebner 
Rockchip IOMMUs are used without explicit clock handling for 4 years
now, so we should keep compatibility with old devicetrees if possible.
Therefore make iommu clocks optional.

Fixes: 8fa9eb39c614 ("dt-bindings: iommu/rockchip: Add clock property")
Signed-off-by: Heiko Stuebner 
---
 Documentation/devicetree/bindings/iommu/rockchip,iommu.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt 
b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
index 6ecefea1c6f9..25bfad987513 100644
--- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
+++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
@@ -14,13 +14,13 @@ Required properties:
 "single-master" device, and needs no additional information
 to associate with its master device.  See:
 Documentation/devicetree/bindings/iommu/iommu.txt
+
+Optional properties:
 - clocks  : A list of clocks required for the IOMMU to be accessible by
 the host CPU.
 - clock-names : Should contain the following:
"iface" - Main peripheral bus clock (PCLK/HCL) (required)
"aclk"  - AXI bus clock (required)
-
-Optional properties:
 - rockchip,disable-mmu-reset : Don't use the mmu reset operation.
   Some mmu instances may produce unexpected results
   when the reset operation is used.
-- 
2.16.2

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

[PATCH 0/2] iommu/rockchip: fix clock handling to not break old dts

2018-04-10 Thread Heiko Stuebner 
The newly added clock handling makes iommu clocks required, thus breaking
devicetrees from the last 4 years.

This series fixes the binding and driver to work without clocks being
present, like it did before and should therefore probably go in as fix
on top of the clock-handling patch.


Heiko Stuebner (2):
  dt-bindings: iommu/rockchip: Make clock properties optional
  iommu/rockchip: make clock handling optional

 Documentation/devicetree/bindings/iommu/rockchip,iommu.txt | 4 ++--
 drivers/iommu/rockchip-iommu.c | 8 ++--
 2 files changed, 8 insertions(+), 4 deletions(-)

-- 
2.16.2

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

Re: [PATCH] base: dma-mapping: Postpone cpu addr translation on mmap()

2018-04-10 Thread jacopo mondi 
Hi Christoph,

On Mon, Apr 09, 2018 at 10:52:51AM -0700, Christoph Hellwig wrote:
> On Mon, Apr 09, 2018 at 06:59:08PM +0200, Jacopo Mondi wrote:
> > I'm still a bit puzzled on what happens if dma_mmap_from_dev_coherent() 
> > fails.
> > Does a dma_mmap_from_dev_coherent() failure guarantee anyhow that the
> > successive virt_to_page() isn't problematic as it is today?
> > Or is it the
> > if (off < count && user_count <= (count - off))
> > check that makes the translation safe?
>
> It doesn't.  I think one major issue is that we should not simply fall
> to dma_common_mmap if no method is required, but need every instance of
> dma_map_ops to explicitly opt into an mmap method that is known to work.

I see.. this patch thus just postpones the problem...

>
> >  #ifndef CONFIG_ARCH_NO_COHERENT_DMA_MMAP
> > unsigned long user_count = vma_pages(vma);
> > unsigned long count = PAGE_ALIGN(size) >> PAGE_SHIFT;
> > -   unsigned long pfn = page_to_pfn(virt_to_page(cpu_addr));
> > unsigned long off = vma->vm_pgoff;
> > +   unsigned long pfn;
> >
> > vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> >
> > @@ -235,6 +235,7 @@ int dma_common_mmap(struct device *dev, struct 
> > vm_area_struct *vma,
> > return ret;
> >
> > if (off < count && user_count <= (count - off)) {
> > +   pfn = page_to_pfn(virt_to_page(cpu_addr));
> > ret = remap_pfn_range(vma, vma->vm_start,
> >   pfn + off,
> >   user_count << PAGE_SHIFT,
>
> Why not:
>
>   ret = remap_pfn_range(vma, vma->vm_start,
>   page_to_pfn(virt_to_page(cpu_addr)) + off,
>
> and save the temp variable?

Sure, it's better... Should I send a v2 or considering your above
comment this patch is just a mitigation and should be ditched in
favour of a proper solution (which requires a much more considerable amount
of work though)?

Thanks
   j


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