* [PATCH] iommu/of: Only do IOMMU lookup for available ones
@ 2018-01-03 6:09 Jeffy Chen
2018-01-08 11:45 ` Robin Murphy
2018-01-17 13:28 ` Joerg Roedel
0 siblings, 2 replies; 8+ messages in thread
From: Jeffy Chen @ 2018-01-03 6:09 UTC (permalink / raw)
To: linux-kernel; +Cc: jcliang, tfiga, Jeffy Chen, iommu, Joerg Roedel
The for_each_matching_node_and_match() would return every matching
nodes including unavailable ones.
It's pointless to init unavailable IOMMUs, so add a sanity check to
avoid that.
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
drivers/iommu/of_iommu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 50947ebb6d17..6f7456caa30d 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -240,6 +240,9 @@ static int __init of_iommu_init(void)
for_each_matching_node_and_match(np, matches, &match) {
const of_iommu_init_fn init_fn = match->data;
+ if (!of_device_is_available(np))
+ continue;
+
if (init_fn && init_fn(np))
pr_err("Failed to initialise IOMMU %pOF\n", np);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] iommu/of: Only do IOMMU lookup for available ones
@ 2018-01-08 11:45 ` Robin Murphy
0 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2018-01-08 11:45 UTC (permalink / raw)
To: Jeffy Chen, linux-kernel; +Cc: jcliang, iommu
Hi Jeffy,
On 03/01/18 06:09, Jeffy Chen wrote:
> The for_each_matching_node_and_match() would return every matching
> nodes including unavailable ones.
>
> It's pointless to init unavailable IOMMUs, so add a sanity check to
> avoid that.
Even better would be to clean up the last remaining init_fn user and get
rid of the whole business. With the probe-deferral mechanism, early
initialisation hooks are no longer needed, and the IOMMU_OF_DECLARE
section really only remains as a way of detecting builtin drivers long
before their registration initcalls run.
Robin.
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> drivers/iommu/of_iommu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 50947ebb6d17..6f7456caa30d 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -240,6 +240,9 @@ static int __init of_iommu_init(void)
> for_each_matching_node_and_match(np, matches, &match) {
> const of_iommu_init_fn init_fn = match->data;
>
> + if (!of_device_is_available(np))
> + continue;
> +
> if (init_fn && init_fn(np))
> pr_err("Failed to initialise IOMMU %pOF\n", np);
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] iommu/of: Only do IOMMU lookup for available ones
@ 2018-01-08 11:45 ` Robin Murphy
0 siblings, 0 replies; 8+ messages in thread
From: Robin Murphy @ 2018-01-08 11:45 UTC (permalink / raw)
To: Jeffy Chen, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: jcliang-F7+t8E8rja9g9hUCZPvPmw,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Hi Jeffy,
On 03/01/18 06:09, Jeffy Chen wrote:
> The for_each_matching_node_and_match() would return every matching
> nodes including unavailable ones.
>
> It's pointless to init unavailable IOMMUs, so add a sanity check to
> avoid that.
Even better would be to clean up the last remaining init_fn user and get
rid of the whole business. With the probe-deferral mechanism, early
initialisation hooks are no longer needed, and the IOMMU_OF_DECLARE
section really only remains as a way of detecting builtin drivers long
before their registration initcalls run.
Robin.
> Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> ---
>
> drivers/iommu/of_iommu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 50947ebb6d17..6f7456caa30d 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -240,6 +240,9 @@ static int __init of_iommu_init(void)
> for_each_matching_node_and_match(np, matches, &match) {
> const of_iommu_init_fn init_fn = match->data;
>
> + if (!of_device_is_available(np))
> + continue;
> +
> if (init_fn && init_fn(np))
> pr_err("Failed to initialise IOMMU %pOF\n", np);
> }
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/of: Only do IOMMU lookup for available ones
2018-01-03 6:09 [PATCH] iommu/of: Only do IOMMU lookup for available ones Jeffy Chen
2018-01-08 11:45 ` Robin Murphy
@ 2018-01-17 13:28 ` Joerg Roedel
2018-01-18 14:41 ` Will Deacon
1 sibling, 1 reply; 8+ messages in thread
From: Joerg Roedel @ 2018-01-17 13:28 UTC (permalink / raw)
To: Jeffy Chen; +Cc: linux-kernel, jcliang, tfiga, iommu, Robin Murphy, Will Deacon
On Wed, Jan 03, 2018 at 02:09:20PM +0800, Jeffy Chen wrote:
> The for_each_matching_node_and_match() would return every matching
> nodes including unavailable ones.
>
> It's pointless to init unavailable IOMMUs, so add a sanity check to
> avoid that.
>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> drivers/iommu/of_iommu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 50947ebb6d17..6f7456caa30d 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -240,6 +240,9 @@ static int __init of_iommu_init(void)
> for_each_matching_node_and_match(np, matches, &match) {
> const of_iommu_init_fn init_fn = match->data;
>
> + if (!of_device_is_available(np))
> + continue;
> +
Makes sense to me, but I'd like to have an OK from Robin or Will (added
to Cc) before applying this.
Joerg
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] iommu/of: Only do IOMMU lookup for available ones
@ 2018-01-18 14:41 ` Will Deacon
0 siblings, 0 replies; 8+ messages in thread
From: Will Deacon @ 2018-01-18 14:41 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jeffy Chen, linux-kernel, jcliang, tfiga, iommu, Robin Murphy
On Wed, Jan 17, 2018 at 02:28:08PM +0100, Joerg Roedel wrote:
> On Wed, Jan 03, 2018 at 02:09:20PM +0800, Jeffy Chen wrote:
> > The for_each_matching_node_and_match() would return every matching
> > nodes including unavailable ones.
> >
> > It's pointless to init unavailable IOMMUs, so add a sanity check to
> > avoid that.
> >
> > Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> > ---
> >
> > drivers/iommu/of_iommu.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 50947ebb6d17..6f7456caa30d 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -240,6 +240,9 @@ static int __init of_iommu_init(void)
> > for_each_matching_node_and_match(np, matches, &match) {
> > const of_iommu_init_fn init_fn = match->data;
> >
> > + if (!of_device_is_available(np))
> > + continue;
> > +
>
> 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, and it's unclear what we should about
DMA when an IOMMU is not deemed to be available. See:
https://patchwork.kernel.org/patch/9681211/
Jeffy -- are you solving a real issue here, or is this just an attempt at
some cleanup?
Will
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] iommu/of: Only do IOMMU lookup for available ones
@ 2018-01-18 14:41 ` Will Deacon
0 siblings, 0 replies; 8+ messages in thread
From: Will Deacon @ 2018-01-18 14:41 UTC (permalink / raw)
To: Joerg Roedel
Cc: Jeffy Chen, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
jcliang-F7+t8E8rja9g9hUCZPvPmw,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
On Wed, Jan 17, 2018 at 02:28:08PM +0100, Joerg Roedel wrote:
> On Wed, Jan 03, 2018 at 02:09:20PM +0800, Jeffy Chen wrote:
> > The for_each_matching_node_and_match() would return every matching
> > nodes including unavailable ones.
> >
> > It's pointless to init unavailable IOMMUs, so add a sanity check to
> > avoid that.
> >
> > Signed-off-by: Jeffy Chen <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > ---
> >
> > drivers/iommu/of_iommu.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 50947ebb6d17..6f7456caa30d 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -240,6 +240,9 @@ static int __init of_iommu_init(void)
> > for_each_matching_node_and_match(np, matches, &match) {
> > const of_iommu_init_fn init_fn = match->data;
> >
> > + if (!of_device_is_available(np))
> > + continue;
> > +
>
> 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, and it's unclear what we should about
DMA when an IOMMU is not deemed to be available. See:
https://patchwork.kernel.org/patch/9681211/
Jeffy -- are you solving a real issue here, or is this just an attempt at
some cleanup?
Will
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] iommu/of: Only do IOMMU lookup for available ones
@ 2018-01-18 14:58 ` JeffyChen
0 siblings, 0 replies; 8+ messages in thread
From: JeffyChen @ 2018-01-18 14:58 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel
Cc: linux-kernel, jcliang, tfiga, iommu, Robin Murphy
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, and it's unclear what we should about
> DMA when an IOMMU is not deemed to be available. See:
>
> https://patchwork.kernel.org/patch/9681211/
>
> Jeffy -- are you solving a real issue here, or is this just an attempt at
> some cleanup?
hmmm, it's just a cleanup, but it seems like we already removed this
of_iommu_init_fn in:
b0c560f7d8a4 iommu: Clean up of_iommu_init_fn
so this patch can be disregarded too :)
>
> Will
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] iommu/of: Only do IOMMU lookup for available ones
@ 2018-01-18 14:58 ` JeffyChen
0 siblings, 0 replies; 8+ messages in thread
From: JeffyChen @ 2018-01-18 14:58 UTC (permalink / raw)
To: Will Deacon, Joerg Roedel
Cc: jcliang-F7+t8E8rja9g9hUCZPvPmw,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
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, and it's unclear what we should about
> DMA when an IOMMU is not deemed to be available. See:
>
> https://patchwork.kernel.org/patch/9681211/
>
> Jeffy -- are you solving a real issue here, or is this just an attempt at
> some cleanup?
hmmm, it's just a cleanup, but it seems like we already removed this
of_iommu_init_fn in:
b0c560f7d8a4 iommu: Clean up of_iommu_init_fn
so this patch can be disregarded too :)
>
> Will
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-01-18 14:58 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-03 6:09 [PATCH] iommu/of: Only do IOMMU lookup for available ones Jeffy Chen
2018-01-08 11:45 ` Robin Murphy
2018-01-08 11:45 ` Robin Murphy
2018-01-17 13:28 ` Joerg Roedel
2018-01-18 14:41 ` Will Deacon
2018-01-18 14:41 ` Will Deacon
2018-01-18 14:58 ` JeffyChen
2018-01-18 14:58 ` JeffyChen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.