Re: [PATCH] iommu: rockchip: fix building without CONFIG_OF

Hi Amd, Thanks for your patch. On 04/04/2018 06:23 PM, Arnd Bergmann wrote: We get a build error when compiling the iommu driver without CONFIG_OF: drivers/iommu/rockchip-iommu.c: In function 'rk_iommu_of_xlate': drivers/iommu/rockchip-iommu.c:1101:2: error: implicit declaration of function

Re: [PATCH v8 11/14] iommu/rockchip: Use OF_IOMMU to attach devices automatically

Hi Daniel, Thanks for your reply. On 03/26/2018 02:31 PM, Daniel Kurtz wrote: >+struct rk_iommudata { >+ struct rk_iommu *iommu; >+}; Why do we need this struct? Can't we just assign a pointer to struct rk_iommu directly to dev->archdata.iommu? hmmm, i was trying to add more device

Re: [PATCH v7 13/14] iommu/rockchip: Add runtime PM support

Hi Tomasz, Thanks for your reply. On 03/06/2018 06:07 PM, Tomasz Figa wrote: Hi Jeffy, It looks like I missed some details of how runtime PM enable works before, so we might be able to simplify things. Sorry for not getting things right earlier hmm, right, the enable state should be the

Re: [RESEND PATCH v6 09/14] dt-bindings: iommu/rockchip: Add clock property

Hi Rob, Thanks for your reply. On 03/06/2018 10:27 AM, Rob Herring wrote: On Thu, Mar 01, 2018 at 06:18:32PM +0800, Jeffy Chen wrote: Add clock property, since we are going to control clocks in rockchip iommu driver. Signed-off-by: Jeffy Chen --- Changes in v6:

Re: [RESEND PATCH v6 13/14] iommu/rockchip: Add runtime PM support

Hi Tomasz, Thanks for your reply. On 03/05/2018 09:49 PM, Tomasz Figa wrote: > struct rk_iommudata { >+ struct device_link *link; /* runtime PM link from IOMMU to master */ Kerneldoc comment for the struct could be added instead. i saw this in the kerneldoc: * An MMU device exists

Re: [RESEND PATCH v6 14/14] iommu/rockchip: Support sharing IOMMU between masters

Hi Robin, On 03/01/2018 07:03 PM, Robin Murphy wrote: +static struct iommu_group *rk_iommu_device_group(struct device *dev) +{ +struct rk_iommu *iommu; + +iommu = rk_iommu_from_dev(dev); + +return iommu->group; Oops, seems I overlooked this in my previous review - it should

Re: [PATCH v5 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi Robin, Thanks for your reply. On 02/28/2018 11:06 PM, Robin Murphy wrote: On 28/02/18 13:00, JeffyChen wrote: Hi Robin, Thanks for your reply. On 02/28/2018 12:59 AM, Robin Murphy wrote: the rockchip IOMMU is part of the master block in hardware, so it needs to control the master's

Re: [PATCH v5 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi Robin, Thanks for your reply. On 02/28/2018 12:59 AM, Robin Murphy wrote: the rockchip IOMMU is part of the master block in hardware, so it needs to control the master's power domain and some of the master's clocks when access it's registers. and the number of clocks needed here, might be

Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi guys, On 01/25/2018 06:24 PM, JeffyChen wrote: On 01/25/2018 05:42 PM, Randy Li wrote: confirmed with Simon, there might be some iommus don't have a pd, and We use the pd to control the NIU node(not on upstream), without a pd or fake pd, none of the platform would work. after talked

Re: [PATCH v5 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi guys, On 02/01/2018 07:19 PM, JeffyChen wrote: diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt index 2098f7732264..33dd853359fa 100644 --- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt +++ b

Re: [PATCH v5 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi Rob, Thanks for your reply. On 01/31/2018 09:50 PM, Rob Herring wrote: On Wed, Jan 31, 2018 at 1:52 AM, Tomasz Figa wrote: Hi Rob, On Wed, Jan 31, 2018 at 2:05 AM, Rob Herring wrote: On Wed, Jan 24, 2018 at 06:35:11PM +0800, Jeffy Chen wrote: From:

Re: [PATCH v5 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi Robin, Thanks for your reply. On 01/24/2018 09:49 PM, Robin Murphy wrote: +Optional properties: +- clocks : A list of master clocks requires for the IOMMU to be accessible s/requires/required/ ok + by the host CPU. The number of clocks depends on the master +

Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi Randy, On 01/22/2018 10:15 AM, JeffyChen wrote: Hi Randy, On 01/22/2018 09:18 AM, Randy Li wrote: Also the power domain driver could manage the clocks as well, I would suggest to use pm_runtime_*. actually the clocks required by pm domain may not be the same as what we want to control

Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi Randy, On 01/22/2018 09:18 AM, Randy Li wrote: Also the power domain driver could manage the clocks as well, I would suggest to use pm_runtime_*. actually the clocks required by pm domain may not be the same as what we want to control here, there might be some clocks only be needed when

Re: [PATCH v4 07/13] ARM: dts: rockchip: add clocks in vop iommu nodes

Hi Tomasz, Thanks for your reply. On 01/19/2018 11:23 AM, Tomasz Figa wrote: On Thu, Jan 18, 2018 at 8:52 PM, Jeffy Chen wrote: Add clocks in vop iommu nodes, since we are going to control clocks in rockchip iommu driver. Signed-off-by: Jeffy Chen

Re: [PATCH] iommu/of: Only do IOMMU lookup for available ones

Hi Will, Thanks for your reply. On 01/18/2018 10:41 PM, Will Deacon wrote: > >Makes sense to me, but I'd like to have an OK from Robin or Will (added >to Cc) before applying this. I don't think this patch makes a lot of sense in isolation: the SMMU drivers themselves will likely still probe,

Re: [PATCH v4 08/13] iommu/rockchip: Control clocks needed to access the IOMMU

Hi Robin, On 01/18/2018 08:27 PM, Robin Murphy wrote: Is it worth using the clk_bulk_*() APIs for this? At a glance, most of the code being added here appears to duplicate what those functions already do (but I'm no clk API expert, for sure). right, i think it's doable, the clk_bulk APIs are

Re: [PATCH v4 04/13] iommu/rockchip: Fix error handling in attach

Hi Robin, On 01/18/2018 09:23 PM, Robin Murphy wrote: @@ -837,7 +837,7 @@ static int rk_iommu_attach_device(struct iommu_domain *domain, ret = rk_iommu_enable_paging(iommu); if (ret) -return ret; +goto err_disable_stall; spin_lock_irqsave(_domain->iommus_lock,

Re: [PATCH v4 05/13] iommu/rockchip: Use iopoll helpers to wait for hardware

Hi Robin, On 01/18/2018 09:09 PM, Robin Murphy wrote: -#define FORCE_RESET_TIMEOUT100/* ms */ +#define FORCE_RESET_TIMEOUT10/* us */ +#define POLL_TIMEOUT1000/* us */ Nit: the callsites look a bit odd with the combination of POLL_TIMEOUT and the magic number

Re: [PATCH v4 00/13] iommu/rockchip: Use OF_IOMMU

Hi Joerg, Thanks for your reply. On 01/18/2018 08:44 PM, Joerg Roedel wrote: On Thu, Jan 18, 2018 at 07:52:38PM +0800, Jeffy Chen wrote: Jeffy Chen (9): iommu/rockchip: Prohibit unbind and remove iommu/rockchip: Fix error handling in probe iommu/rockchip: Request irqs in

Re: [PATCH v3 01/12] iommu/rockchip: Prohibiat unbind and remove

Hi Tomasz, Thanks for your reply. and just found i forgot to add iommu clocks for other rockchip platforms(rk3399 already has that)...will also do that in the next version. On 01/18/2018 12:17 PM, Tomasz Figa wrote: On Thu, Jan 18, 2018 at 12:25 AM, Jeffy Chen

Re: [PATCH v2 13/13] iommu/rockchip: Support sharing IOMMU between masters

Hi Robin, On 01/17/2018 09:00 PM, Robin Murphy wrote: On 16/01/18 13:25, Jeffy Chen wrote: There would be some masters sharing the same IOMMU device. Put them in the same iommu group and share the same iommu domain. Signed-off-by: Jeffy Chen --- Changes in v2:

Re: [PATCH v2 09/13] iommu/rockchip: Use iommu_group_get_for_dev() for add_device

On 01/17/2018 08:47 PM, JeffyChen wrote: +static struct iommu_group *rk_iommu_device_group(struct device *dev) +{ +struct iommu_group *group; +int ret; + +group = iommu_group_get(dev); +if (!group) { This check is pointless - if dev->iommu_group were non-NULL you wouldn't h

Re: [PATCH v2 09/13] iommu/rockchip: Use iommu_group_get_for_dev() for add_device

Hi Robin, On 01/17/2018 08:31 PM, Robin Murphy wrote: On 16/01/18 13:25, Jeffy Chen wrote: IOMMU drivers are supposed to call this function instead of manually creating a group in their .add_device callback. This behavior is not strictly required by ARM DMA mapping implementation, but ARM64

Re: [PATCH v2 01/13] iommu/rockchip: Request irqs in rk_iommu_probe()

Hi Robin, On 01/17/2018 08:18 PM, Robin Murphy wrote: @@ -91,7 +92,6 @@ struct rk_iommu { void __iomem **bases; int num_mmu; int *irq; Nit: irq seems to be redundant now as well. oops, will fix it. -int num_irq; bool reset_disabled; struct iommu_device

Re: [PATCH v2 05/13] iommu/rockchip: Fix error handling in init

Hi Robin, Thanks for your reply. On 01/17/2018 07:36 PM, Robin Murphy wrote: On 17/01/18 05:26, Tomasz Figa wrote: On Tue, Jan 16, 2018 at 10:25 PM, Jeffy Chen wrote: It's hard to undo bus_set_iommu() in the error path, so move it to the end of rk_iommu_probe().

Re: [PATCH v2 12/13] iommu/rockchip: Add runtime PM support

Hi Tomasz, On 01/17/2018 03:52 PM, JeffyChen wrote: Hi Tomasz, On 01/17/2018 03:38 PM, Tomasz Figa wrote: >>Don't we need to check here (and in _shutdown() too) if we have a >>domain attached? > >hmmm, right, the startup might been called by resume, so should check &

Re: [PATCH v2 12/13] iommu/rockchip: Add runtime PM support

Hi Tomasz, On 01/17/2018 03:38 PM, Tomasz Figa wrote: >>Don't we need to check here (and in _shutdown() too) if we have a >>domain attached? > >hmmm, right, the startup might been called by resume, so should check >iommu->domain here. > >but the shutdown would be called at the end of detach or

Re: [PATCH v2 11/13] iommu/rockchip: Use OF_IOMMU to attach devices automatically

On 01/17/2018 03:30 PM, Tomasz Figa wrote: >but it's possible the probe failed after calling iommu_device_set_fwnode, so >i'll try to add some checks here, and maybe adjust probe() to prevent it >too. I think iommu_device_set_fwnode() is not enough for of_iommu_xlate() to find the IOMMU. The

Re: [PATCH v2 01/13] iommu/rockchip: Request irqs in rk_iommu_probe()

Hi Tomasz, On 01/17/2018 03:16 PM, Tomasz Figa wrote: >> >>This lacks consistency. of_irq_count() is used for counting, but >>platform_get_irq() is used for getting. Either platform_ or of_ API >>should be used for both and I'd lean for platform_, since it's more >>general. > >hmmm, right, i

Re: [PATCH v2 12/13] iommu/rockchip: Add runtime PM support

Hi Tomasz, Thanks for your reply. On 01/17/2018 02:20 PM, Tomasz Figa wrote: On Tue, Jan 16, 2018 at 10:25 PM, Jeffy Chen wrote: When the power domain is powered off, the IOMMU cannot be accessed and register programming must be deferred until the power domain

Re: [PATCH v2 11/13] iommu/rockchip: Use OF_IOMMU to attach devices automatically

Hi Tomasz, Thanks for your reply. On 01/17/2018 01:44 PM, Tomasz Figa wrote: On Tue, Jan 16, 2018 at 10:25 PM, Jeffy Chen wrote: Converts the rockchip-iommu driver to use the OF_IOMMU infrastructure, which allows attaching master devices to their IOMMUs

Re: [PATCH v2 02/13] iommu/rockchip: Suppress unbinding

Hi Tomasz, On 01/17/2018 01:32 PM, Tomasz Figa wrote: On Wed, Jan 17, 2018 at 1:23 PM, Tomasz Figa wrote: On Tue, Jan 16, 2018 at 10:25 PM, Jeffy Chen wrote: It's not safe to unbind rockchip IOMMU driver. Might be good to explain why it is

Re: [PATCH v2 05/13] iommu/rockchip: Fix error handling in init

Hi Tomasz, On 01/17/2018 01:26 PM, Tomasz Figa wrote: On Tue, Jan 16, 2018 at 10:25 PM, Jeffy Chen wrote: It's hard to undo bus_set_iommu() in the error path, so move it to the end of rk_iommu_probe(). Does this work fine now? I remember we used to need this

Re: [PATCH v2 01/13] iommu/rockchip: Request irqs in rk_iommu_probe()

Hi Tomasz, Thanks for your reply. On 01/17/2018 12:21 PM, Tomasz Figa wrote: Hi Jeffy, Thanks for the patch. Please see my comments inline. On Tue, Jan 16, 2018 at 10:25 PM, Jeffy Chen wrote: Please add patch description. ok, will do. Suggested-by: Robin

Re: [PATCH 1/9] iommu/of: Drop early initialisation hooks

Hi Robin, Thnaks for your reply. On 01/11/2018 08:24 PM, Robin Murphy wrote: Hi Jeffy, On 11/01/18 11:14, JeffyChen wrote: Hi Marek, Thanks for your reply. On 01/11/2018 05:40 PM, Marek Szyprowski wrote: Hi Jeffy, On 2018-01-11 09:22, Jeffy Chen wrote: With the probe-deferral mechanism

Re: [PATCH 2/9] iommu/rockchip: Fix error handling in attach

Hi Robin, thanks for your reply. On 01/11/2018 11:47 PM, Robin Murphy wrote: +for (i = 0; i < iommu->num_irq; i++) { +ret = devm_request_irq(iommu->dev, iommu->irq[i], rk_iommu_irq, + IRQF_SHARED, dev_name(dev), iommu); Why aren't we simply requesting the

Re: [PATCH 1/9] iommu/of: Drop early initialisation hooks

Hi Marek, Thanks for your reply. On 01/11/2018 05:40 PM, Marek Szyprowski wrote: Hi Jeffy, On 2018-01-11 09:22, Jeffy Chen wrote: With the probe-deferral mechanism, early initialisation hooks are no longer needed. Suggested-by: Robin Murphy Signed-off-by: Jeffy Chen