Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-18 Thread Laurent Pinchart
Hi Arnd,

On Wednesday 17 December 2014 16:56:53 Arnd Bergmann wrote:
 On Wednesday 17 December 2014 15:53:25 Lucas Stach wrote:
  Am Mittwoch, den 17.12.2014, 15:27 +0100 schrieb Arnd Bergmann:
  On Wednesday 17 December 2014 01:24:42 Laurent Pinchart wrote:
  If we forbid the IOMMU driver from being compiled as a module can't we
  just rely on deferred probing ? The bus master driver will just be
  reprobed after the IOMMU gets probed, like for other devices.
  
  This could fail in case the IOMMU device permanently fails to probe.
  There would be something very wrong in the system in that case,
  Enabling the bus masters totally transparently without IOMMU support
  could not be such a good idea.
  
  I believe in the majority of cases today, the IOMMU is entirely
  optional. There are valid reasons for not including the IOMMU driver in
  the kernel, e.g. when you know that all the machines with that driver
  can DMA to all of their RAM and you want to avoid the overhead of IOTLB
  misses.
  
  This is similar to the problems we discussed with the componentized
  device framework and in the end everybody agreed on a simple rule:
  if the device node is enabled in the DT there must be a driver bound to
  it before other devices depending on this node can be probed.
  
  If we apply the same logic to the IOMMU situation we get two
  possibilities to allow bypassing the IOMMU:
  1. completely disable the IOMMU node in the DT
  2. leave node enabled in DT, but add a bypass property
  
  Obviously the second option still requires to have the IOMMU driver
  available, but is more flexible. Right now everything depends on the
  IOMMU being in passthrough mode at reset with no driver bound. If a
  device comes around where this isn't the case thing will break with the
  current assumptions or the first option.
  
  Having the IOMMU node enabled in DT with no driver available to the
  kernel seems like an invalid configuration which should be expected to
  break. Exactly the same thing as with componentized devices...
 
 One differences here is that for the IOMMU, we should generally be able
 to fall back to swiotlb

Or to nothing at all if the device can DMA to the allocated memory directly.

 (currently only on ARM64, not ARM32, until someone adds support). I can see
 your point regarding machines that have a mandatory IOMMU with no fallback
 when there is no driver, but we can support them by making the iommu driver
 selected through Kconfig for that platform, while still allowing other
 platforms to work with drivers left out of the kernel.

The question is how to tell the kernel not to wait for an IOMMU that will 
never be there. Would a kernel command line argument be an acceptable solution 
or do we need something more automatic ?

-- 
Regards,

Laurent Pinchart

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-18 Thread Arnd Bergmann
On Thursday 18 December 2014 22:36:14 Laurent Pinchart wrote:
 
  (currently only on ARM64, not ARM32, until someone adds support). I can see
  your point regarding machines that have a mandatory IOMMU with no fallback
  when there is no driver, but we can support them by making the iommu driver
  selected through Kconfig for that platform, while still allowing other
  platforms to work with drivers left out of the kernel.
 
 The question is how to tell the kernel not to wait for an IOMMU that will 
 never be there. Would a kernel command line argument be an acceptable 
 solution 
 or do we need something more automatic ?

I would hope that we can find an automatic solution without relying on
the command line.

Unfortunately, I also remembered one case in support of what Lucas mentioned
and that would break this: There is at least one SoC that uses cache-coherent
DMA only when the IOMMU (ARM SMMU IIRC)is enabled, but is non-coherent
otherwise. We can't really express that in DT, so we actually do rely on
the IOMMU driver to be present on this machine when any iommus properties
are used, as they would always come in combination with dma-coherent
properties that are otherwise wrong.

We can still enforce this by requiring the smmu driver to be built into
the kernel on this platform, but simply saying that the device cannot
support DMA as long as there is an iommus property but no driver for
it does make a lot of sense.

Note that there are more than two ways that the kernel could treat
the situation of probing a device with a valid iommus reference but
no driver loaded for the iommu:

a) assume all iommu drivers are initialized early, so use linear or
   swiotlb dma_map_ops, and probe the driver normally. This breaks
   the scenario with conditionally coherent devices, and requires doing
   the iommu init early
b) assume all iommu drivers are initialized early, so disallow any DMA
   by setting a zero dma_mask but allow the driver to be probed using
   polling I/O mode (useful for slow devices like UART or SPI)
   This breaks devices that require DMA but can fall back to linear
   or swiotlb mappings, and requires doing the iommu init early.
c) defer probing until an iommu driver is gets initialized. This breaks
   both the cases where we could fall back to a sane behavior without
   iommu, but it would let us use a proper driver with regular power
   management etc.

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-17 Thread Arnd Bergmann
On Wednesday 17 December 2014 01:24:42 Laurent Pinchart wrote:
 
 If we forbid the IOMMU driver from being compiled as a module can't we just 
 rely on deferred probing ? The bus master driver will just be reprobed after 
 the IOMMU gets probed, like for other devices.
 
 This could fail in case the IOMMU device permanently fails to probe. There 
 would be something very wrong in the system in that case, Enabling the bus 
 masters totally transparently without IOMMU support could not be such a good 
 idea.

I believe in the majority of cases today, the IOMMU is entirely optional.
There are valid reasons for not including the IOMMU driver in the kernel,
e.g. when you know that all the machines with that driver can DMA to
all of their RAM and you want to avoid the overhead of IOTLB misses.

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-17 Thread Laurent Pinchart
Hi Arnd,

On Wednesday 17 December 2014 15:27:36 Arnd Bergmann wrote:
 On Wednesday 17 December 2014 01:24:42 Laurent Pinchart wrote:
  If we forbid the IOMMU driver from being compiled as a module can't we
  just rely on deferred probing ? The bus master driver will just be
  reprobed after the IOMMU gets probed, like for other devices.
  
  This could fail in case the IOMMU device permanently fails to probe. There
  would be something very wrong in the system in that case, Enabling the bus
  masters totally transparently without IOMMU support could not be such a
  good idea.
 
 I believe in the majority of cases today, the IOMMU is entirely optional.
 There are valid reasons for not including the IOMMU driver in the kernel,
 e.g. when you know that all the machines with that driver can DMA to
 all of their RAM and you want to avoid the overhead of IOTLB misses.

Should that really be controlled by compiling the IOMMU driver out, wouldn't 
it be better to disable the IOMMU devices in DT in that case ?

-- 
Regards,

Laurent Pinchart

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-17 Thread Lucas Stach
Am Mittwoch, den 17.12.2014, 15:27 +0100 schrieb Arnd Bergmann:
 On Wednesday 17 December 2014 01:24:42 Laurent Pinchart wrote:
  
  If we forbid the IOMMU driver from being compiled as a module can't we just 
  rely on deferred probing ? The bus master driver will just be reprobed 
  after 
  the IOMMU gets probed, like for other devices.
  
  This could fail in case the IOMMU device permanently fails to probe. There 
  would be something very wrong in the system in that case, Enabling the bus 
  masters totally transparently without IOMMU support could not be such a 
  good 
  idea.
 
 I believe in the majority of cases today, the IOMMU is entirely optional.
 There are valid reasons for not including the IOMMU driver in the kernel,
 e.g. when you know that all the machines with that driver can DMA to
 all of their RAM and you want to avoid the overhead of IOTLB misses.
 
This is similar to the problems we discussed with the componentized
device framework and in the end everybody agreed on a simple rule:
if the device node is enabled in the DT there must be a driver bound to
it before other devices depending on this node can be probed.

If we apply the same logic to the IOMMU situation we get two
possibilities to allow bypassing the IOMMU:
1. completely disable the IOMMU node in the DT
2. leave node enabled in DT, but add a bypass property

Obviously the second option still requires to have the IOMMU driver
available, but is more flexible. Right now everything depends on the
IOMMU being in passthrough mode at reset with no driver bound. If a
device comes around where this isn't the case thing will break with the
current assumptions or the first option.

Having the IOMMU node enabled in DT with no driver available to the
kernel seems like an invalid configuration which should be expected to
break. Exactly the same thing as with componentized devices...

Regards,
Lucas

-- 
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-17 Thread Arnd Bergmann
On Wednesday 17 December 2014 16:39:02 Laurent Pinchart wrote:
 
 On Wednesday 17 December 2014 15:27:36 Arnd Bergmann wrote:
  On Wednesday 17 December 2014 01:24:42 Laurent Pinchart wrote:
   If we forbid the IOMMU driver from being compiled as a module can't we
   just rely on deferred probing ? The bus master driver will just be
   reprobed after the IOMMU gets probed, like for other devices.
   
   This could fail in case the IOMMU device permanently fails to probe. There
   would be something very wrong in the system in that case, Enabling the bus
   masters totally transparently without IOMMU support could not be such a
   good idea.
  
  I believe in the majority of cases today, the IOMMU is entirely optional.
  There are valid reasons for not including the IOMMU driver in the kernel,
  e.g. when you know that all the machines with that driver can DMA to
  all of their RAM and you want to avoid the overhead of IOTLB misses.
 
 Should that really be controlled by compiling the IOMMU driver out, wouldn't 
 it be better to disable the IOMMU devices in DT in that case ?

It's a policy decision that should only depend on the user. Modifying the
DT is wrong here IMHO because the device is still connected to the IOMMU
in hardware and we should correctly represent that.

You can normally set 'iommu=off' or 'iommu=force' on the command line
to force it on or off rather than letting the IOMMU driver decide whether
the device turns it on. Not enabling the IOMMU at all should really just
behave the same way as 'iommu=off', anything else would be very confusing.

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-17 Thread Arnd Bergmann
On Wednesday 17 December 2014 15:53:25 Lucas Stach wrote:
 Am Mittwoch, den 17.12.2014, 15:27 +0100 schrieb Arnd Bergmann:
  On Wednesday 17 December 2014 01:24:42 Laurent Pinchart wrote:
   
   If we forbid the IOMMU driver from being compiled as a module can't we 
   just 
   rely on deferred probing ? The bus master driver will just be reprobed 
   after 
   the IOMMU gets probed, like for other devices.
   
   This could fail in case the IOMMU device permanently fails to probe. 
   There 
   would be something very wrong in the system in that case, Enabling the 
   bus 
   masters totally transparently without IOMMU support could not be such a 
   good 
   idea.
  
  I believe in the majority of cases today, the IOMMU is entirely optional.
  There are valid reasons for not including the IOMMU driver in the kernel,
  e.g. when you know that all the machines with that driver can DMA to
  all of their RAM and you want to avoid the overhead of IOTLB misses.
  
 This is similar to the problems we discussed with the componentized
 device framework and in the end everybody agreed on a simple rule:
 if the device node is enabled in the DT there must be a driver bound to
 it before other devices depending on this node can be probed.
 
 If we apply the same logic to the IOMMU situation we get two
 possibilities to allow bypassing the IOMMU:
 1. completely disable the IOMMU node in the DT
 2. leave node enabled in DT, but add a bypass property
 
 Obviously the second option still requires to have the IOMMU driver
 available, but is more flexible. Right now everything depends on the
 IOMMU being in passthrough mode at reset with no driver bound. If a
 device comes around where this isn't the case thing will break with the
 current assumptions or the first option.
 
 Having the IOMMU node enabled in DT with no driver available to the
 kernel seems like an invalid configuration which should be expected to
 break. Exactly the same thing as with componentized devices...

One differences here is that for the IOMMU, we should generally be able
to fall back to swiotlb (currently only on ARM64, not ARM32, until
someone adds support). I can see your point regarding machines that
have a mandatory IOMMU with no fallback when there is no driver, but
we can support them by making the iommu driver selected through Kconfig
for that platform, while still allowing other platforms to work with
drivers left out of the kernel.

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-17 Thread Laurent Pinchart
Hi Arnd,

On Wednesday 17 December 2014 16:41:33 Arnd Bergmann wrote:
 On Wednesday 17 December 2014 16:39:02 Laurent Pinchart wrote:
  On Wednesday 17 December 2014 15:27:36 Arnd Bergmann wrote:
   On Wednesday 17 December 2014 01:24:42 Laurent Pinchart wrote:
If we forbid the IOMMU driver from being compiled as a module can't we
just rely on deferred probing ? The bus master driver will just be
reprobed after the IOMMU gets probed, like for other devices.

This could fail in case the IOMMU device permanently fails to probe.
There
would be something very wrong in the system in that case, Enabling the
bus
masters totally transparently without IOMMU support could not be such
a
good idea.
   
   I believe in the majority of cases today, the IOMMU is entirely
   optional.
   There are valid reasons for not including the IOMMU driver in the
   kernel,
   e.g. when you know that all the machines with that driver can DMA to
   all of their RAM and you want to avoid the overhead of IOTLB misses.
  
  Should that really be controlled by compiling the IOMMU driver out,
  wouldn't it be better to disable the IOMMU devices in DT in that case ?
 
 It's a policy decision that should only depend on the user. Modifying the
 DT is wrong here IMHO because the device is still connected to the IOMMU
 in hardware and we should correctly represent that.

I was thinking about setting status = disabled on the IOMMU nodes, not 
removing the IOMMU references in the bus master nodes.

 You can normally set 'iommu=off' or 'iommu=force' on the command line
 to force it on or off rather than letting the IOMMU driver decide whether
 the device turns it on. Not enabling the IOMMU at all should really just
 behave the same way as 'iommu=off', anything else would be very confusing.

Setting iommu=off sounds fine to me. The IOMMU core would then return a no 
IOMMU present error instead of -EPROBE_DEFER, and probing of the bus masters 
would continue without IOMMU support.

-- 
Regards,

Laurent Pinchart

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-17 Thread Arnd Bergmann
On Wednesday 17 December 2014 18:02:51 Laurent Pinchart wrote:
 On Wednesday 17 December 2014 16:41:33 Arnd Bergmann wrote:
  On Wednesday 17 December 2014 16:39:02 Laurent Pinchart wrote:
   On Wednesday 17 December 2014 15:27:36 Arnd Bergmann wrote:
On Wednesday 17 December 2014 01:24:42 Laurent Pinchart wrote:
 If we forbid the IOMMU driver from being compiled as a module can't we
 just rely on deferred probing ? The bus master driver will just be
 reprobed after the IOMMU gets probed, like for other devices.
 
 This could fail in case the IOMMU device permanently fails to probe.
 There
 would be something very wrong in the system in that case, Enabling the
 bus
 masters totally transparently without IOMMU support could not be such
 a
 good idea.

I believe in the majority of cases today, the IOMMU is entirely
optional.
There are valid reasons for not including the IOMMU driver in the
kernel,
e.g. when you know that all the machines with that driver can DMA to
all of their RAM and you want to avoid the overhead of IOTLB misses.
   
   Should that really be controlled by compiling the IOMMU driver out,
   wouldn't it be better to disable the IOMMU devices in DT in that case ?
  
  It's a policy decision that should only depend on the user. Modifying the
  DT is wrong here IMHO because the device is still connected to the IOMMU
  in hardware and we should correctly represent that.
 
 I was thinking about setting status = disabled on the IOMMU nodes, not 
 removing the IOMMU references in the bus master nodes.

But that still requires a modification of the DT. The hardware is the
same, so I don't see why we should update the dtb based on kernel
configuration.

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-17 Thread Laurent Pinchart
Hi Arnd,

On Wednesday 17 December 2014 22:58:47 Arnd Bergmann wrote:
 On Wednesday 17 December 2014 18:02:51 Laurent Pinchart wrote:
  On Wednesday 17 December 2014 16:41:33 Arnd Bergmann wrote:
  On Wednesday 17 December 2014 16:39:02 Laurent Pinchart wrote:
  On Wednesday 17 December 2014 15:27:36 Arnd Bergmann wrote:
  On Wednesday 17 December 2014 01:24:42 Laurent Pinchart wrote:
  If we forbid the IOMMU driver from being compiled as a module
  can't we just rely on deferred probing ? The bus master driver
  will just be reprobed after the IOMMU gets probed, like for other
  devices.
  
  This could fail in case the IOMMU device permanently fails to
  probe. There would be something very wrong in the system in that
  case, Enabling the bus masters totally transparently without IOMMU
  support could not be such a good idea.
  
  I believe in the majority of cases today, the IOMMU is entirely
  optional. There are valid reasons for not including the IOMMU driver
  in the kernel, e.g. when you know that all the machines with that
  driver can DMA to all of their RAM and you want to avoid the
  overhead of IOTLB misses.
  
  Should that really be controlled by compiling the IOMMU driver out,
  wouldn't it be better to disable the IOMMU devices in DT in that case
  ?
  
  It's a policy decision that should only depend on the user. Modifying
  the DT is wrong here IMHO because the device is still connected to the
  IOMMU in hardware and we should correctly represent that.
  
  I was thinking about setting status = disabled on the IOMMU nodes, not
  removing the IOMMU references in the bus master nodes.
 
 But that still requires a modification of the DT. The hardware is the
 same, so I don't see why we should update the dtb based on kernel
 configuration.

It wouldn't be the first time we encode configuration information in DT, but I 
agree it's not an optimal solution. Setting iommu=off on the kernel command 
line is better, and should be easy to implement.

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-16 Thread Marek Szyprowski

Hello,

On 2014-12-15 19:19, Laurent Pinchart wrote:

On Monday 15 December 2014 18:13:23 Will Deacon wrote:

On Mon, Dec 15, 2014 at 05:53:48PM +, Laurent Pinchart wrote:

On Monday 15 December 2014 17:43:02 Will Deacon wrote:

On Mon, Dec 15, 2014 at 05:27:30PM +, Laurent Pinchart wrote:

On Monday 15 December 2014 17:17:00 Will Deacon wrote:

Creating the platform device manually for the IOMMU is indeed
grotty, but I don't really understand why it's needed. Interrupt
controllers, for example, seem to get by without one.

There's several reasons, one of the most compelling ones I can think
of at the moment is runtime PM. IRQ controllers close to the CPU use
CPU PM notifiers instead. Note that IRQ controllers that are further
away from the CPU (such as GPIO-based IRQ controllers) are real
platform devices and use runtime PM.

Ok, that's a good point, but the IOMMU will still probe later anyway,
right?

That depends on the driver implementation, using OF node match an IOMMU
driver doesn't have to register a struct driver. Assuming we require
IOMMU drivers to register a struct driver, a platform device should then
be probed at a later time.

However, if we wait until the IOMMU gets probed to initialize runtime PM
and make it functional, we'll be back in square one if the IOMMU gets
probed after the bus master, as the bus master could start issuing bus
requests at probe time with the IOMMU not powered yet.

True, but couldn't the early init code do enough to get the thing
functional? That said, I'm showing my ignorance here as I'm not familiar
with the PM code (and the software models I have for the SMMU clearly don't
implement anything in this regard).

We're reaching the limits of my knowledge as well. If the IOMMU is in a power
domain different than the bus masters the driver would at least need to ensure
that the power domain is turned on, which might be a bit hackish without
runtime PM.


In case of Exynos SoC both IOMMU and master device are in the same power 
domain.
During iommu_attach() call driver needs to have power domain enabled, 
but power

domain driver is probed from arch_initcall(). I wanted to move power domain
initialization earlier to ensure that it will happen before IOMMU driver 
probe,
that not really a problem. Right now it usually works, because power 
domains are

enabled by bootloader.

However please not that I would really like to have something merged. The
discussion about iommu controllers lasts for about 2 years and we still 
don't

have ANYTHING working in mainline, so lets merge the of_iommu glue and then
check how the remaining issues can be solved.

Best regards
--
Marek Szyprowski, PhD
Samsung RD Institute Poland

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


Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-16 Thread Arnd Bergmann
On Monday 15 December 2014 18:13:23 Will Deacon wrote:
IOMMUs are not as low-level as system interrupt controllers or system
clocks. I'm beginning to agree with Thierry that they should be treated
as normal platform devices as they're not required earlier than probe
time of their bus master devices.
   
   Well, I think you'd have to propose patches for discussion since I'm
   certainly not wed to the current approach; I just want something that
   allows of_{dma,iommu}_configure to run with the information it needs.
  
  Do we need of_dma_configure() to run when the device is created, or could 
  we 
  postpone it to just before probe time ?
 
 I'm not sure I can answer that one... Arnd?

I believe we could postpone it to probe time, but I'd rather not.
The way I see the arguments in favor, we have mainly cosmetic arguments:

- Doing it at probe time would eliminate the ugly section magic hack
- iommu drivers could be implemented as loadable modules with platform
  drivers, for consistency with most other drivers

On the other hand, I see:

- DMA configuration is traditionally done at device creation time, and
  changing that is more likely to introduce bugs than leaving it
  where it is.
- On a lot of machines, the IOMMU is optional, and the probe function
  cannot know the difference between an IOMMU driver that is left out
  of the kernel and one that will be initialized later, using a fixed
  entry point for initializing the IOMMU makes the behavior consistent

There is a third option in theory, which is to only enable the IOMMU
as part of dma_set_mask(). I've done that in the past on powerpc, but
the new approach seems cleaner.

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


Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-16 Thread Laurent Pinchart
Hi Arnd,

On Tuesday 16 December 2014 12:40:28 Arnd Bergmann wrote:
 On Monday 15 December 2014 18:13:23 Will Deacon wrote:
  IOMMUs are not as low-level as system interrupt controllers or
  system clocks. I'm beginning to agree with Thierry that they should
  be treated as normal platform devices as they're not required
  earlier than probe time of their bus master devices.
  
  Well, I think you'd have to propose patches for discussion since I'm
  certainly not wed to the current approach; I just want something that
  allows of_{dma,iommu}_configure to run with the information it needs.
  
  Do we need of_dma_configure() to run when the device is created, or
  could we postpone it to just before probe time ?
  
  I'm not sure I can answer that one... Arnd?
 
 I believe we could postpone it to probe time, but I'd rather not.
 The way I see the arguments in favor, we have mainly cosmetic arguments:
 
 - Doing it at probe time would eliminate the ugly section magic hack
 - iommu drivers could be implemented as loadable modules with platform
   drivers, for consistency with most other drivers

The main argument, from my point of view, is that handling IOMMUs are normal 
platform devices allow using all the kernel infrastructure that depends on a 
struct device. The dev_* print helpers are nice to have, but what would make a 
big difference is the ability to use runtime PM.

 On the other hand, I see:
 
 - DMA configuration is traditionally done at device creation time, and
   changing that is more likely to introduce bugs than leaving it
   where it is.
 - On a lot of machines, the IOMMU is optional, and the probe function
   cannot know the difference between an IOMMU driver that is left out
   of the kernel and one that will be initialized later, using a fixed
   entry point for initializing the IOMMU makes the behavior consistent

I'm not advocating for IOMMU support being built as a loadable module. It 
might be nice from a development point of view, but that's about it.

 There is a third option in theory, which is to only enable the IOMMU
 as part of dma_set_mask(). I've done that in the past on powerpc, but
 the new approach seems cleaner.

-- 
Regards,

Laurent Pinchart

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-16 Thread Arnd Bergmann
On Tuesday 16 December 2014 14:07:11 Laurent Pinchart wrote:
 Hi Arnd,
 
 On Tuesday 16 December 2014 12:40:28 Arnd Bergmann wrote:
  On Monday 15 December 2014 18:13:23 Will Deacon wrote:
   IOMMUs are not as low-level as system interrupt controllers or
   system clocks. I'm beginning to agree with Thierry that they should
   be treated as normal platform devices as they're not required
   earlier than probe time of their bus master devices.
   
   Well, I think you'd have to propose patches for discussion since I'm
   certainly not wed to the current approach; I just want something that
   allows of_{dma,iommu}_configure to run with the information it needs.
   
   Do we need of_dma_configure() to run when the device is created, or
   could we postpone it to just before probe time ?
   
   I'm not sure I can answer that one... Arnd?
  
  I believe we could postpone it to probe time, but I'd rather not.
  The way I see the arguments in favor, we have mainly cosmetic arguments:
  
  - Doing it at probe time would eliminate the ugly section magic hack
  - iommu drivers could be implemented as loadable modules with platform
drivers, for consistency with most other drivers
 
 The main argument, from my point of view, is that handling IOMMUs are normal 
 platform devices allow using all the kernel infrastructure that depends on a 
 struct device. The dev_* print helpers are nice to have, but what would make 
 a 
 big difference is the ability to use runtime PM.

Right, I agree that would be nice.

  On the other hand, I see:
  
  - DMA configuration is traditionally done at device creation time, and
changing that is more likely to introduce bugs than leaving it
where it is.
  - On a lot of machines, the IOMMU is optional, and the probe function
cannot know the difference between an IOMMU driver that is left out
of the kernel and one that will be initialized later, using a fixed
entry point for initializing the IOMMU makes the behavior consistent
 
 I'm not advocating for IOMMU support being built as a loadable module. It 
 might be nice from a development point of view, but that's about it.

We'd still have to deal with deferred probing, or with ordering the iommu
initcalls before the dma master initcalls in some other way, so the
problem is not that different from loadable modules. Do you have an idea
for how to solve that?

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


Re: [Linaro-mm-sig] [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-16 Thread Laurent Pinchart
Hi Arnd,

On Tuesday 16 December 2014 13:10:59 Arnd Bergmann wrote:
 On Tuesday 16 December 2014 14:07:11 Laurent Pinchart wrote:
  On Tuesday 16 December 2014 12:40:28 Arnd Bergmann wrote:
   On Monday 15 December 2014 18:13:23 Will Deacon wrote:
IOMMUs are not as low-level as system interrupt controllers or
system clocks. I'm beginning to agree with Thierry that they should
be treated as normal platform devices as they're not required
earlier than probe time of their bus master devices.

Well, I think you'd have to propose patches for discussion since I'm
certainly not wed to the current approach; I just want something
that allows of_{dma,iommu}_configure to run with the information it
needs.

Do we need of_dma_configure() to run when the device is created, or
could we postpone it to just before probe time ?

I'm not sure I can answer that one... Arnd?
   
   I believe we could postpone it to probe time, but I'd rather not.
   The way I see the arguments in favor, we have mainly cosmetic arguments:
   
   - Doing it at probe time would eliminate the ugly section magic hack
   - iommu drivers could be implemented as loadable modules with platform
 drivers, for consistency with most other drivers
  
  The main argument, from my point of view, is that handling IOMMUs are
  normal platform devices allow using all the kernel infrastructure that
  depends on a struct device. The dev_* print helpers are nice to have, but
  what would make a big difference is the ability to use runtime PM.
 
 Right, I agree that would be nice.
 
   On the other hand, I see:
   
   - DMA configuration is traditionally done at device creation time, and
 changing that is more likely to introduce bugs than leaving it
 where it is.
   
   - On a lot of machines, the IOMMU is optional, and the probe function
 cannot know the difference between an IOMMU driver that is left out
 of the kernel and one that will be initialized later, using a fixed
 entry point for initializing the IOMMU makes the behavior consistent
  
  I'm not advocating for IOMMU support being built as a loadable module. It
  might be nice from a development point of view, but that's about it.
 
 We'd still have to deal with deferred probing, or with ordering the iommu
 initcalls before the dma master initcalls in some other way, so the
 problem is not that different from loadable modules. Do you have an idea
 for how to solve that?

If we forbid the IOMMU driver from being compiled as a module can't we just 
rely on deferred probing ? The bus master driver will just be reprobed after 
the IOMMU gets probed, like for other devices.

This could fail in case the IOMMU device permanently fails to probe. There 
would be something very wrong in the system in that case, Enabling the bus 
masters totally transparently without IOMMU support could not be such a good 
idea.

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-15 Thread Thierry Reding
On Sun, Dec 14, 2014 at 02:45:36PM +0200, Laurent Pinchart wrote:
 Hi Marek,
 
 Thank you for the patch.
 
 On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
  This patch introduces IOMMU_OF_DECLARE-based initialization to the
  driver, which replaces subsys_initcall-based procedure.
  exynos_iommu_of_setup ensures that each sysmmu controller is probed
  before its master device.
  
  Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
  ---
   drivers/iommu/exynos-iommu.c | 28 +++-
   1 file changed, 27 insertions(+), 1 deletion(-)
  
  diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
  index cd28dc09db39..88f9afe641a0 100644
  --- a/drivers/iommu/exynos-iommu.c
  +++ b/drivers/iommu/exynos-iommu.c
 
 [snip]
 
  @@ -1125,4 +1134,21 @@ err_reg_driver:
  kmem_cache_destroy(lv2table_kmem_cache);
  return ret;
   }
  -subsys_initcall(exynos_iommu_init);
  +
  +static int __init exynos_iommu_of_setup(struct device_node *np)
  +{
  +   struct platform_device *pdev;
  +
  +   if (!init_done)
  +   exynos_iommu_init();
  +
  +   pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
  +   if (IS_ERR(pdev))
  +   return PTR_ERR(pdev);
 
 If we end up having to create the IOMMU platform devices from within the 
 drivers, the introduction of IOMMU_OF_DECLARE starts to feel like a 
 workaround 
 to me. I wonder whether it wouldn't then be better to let the driver core 
 instantiate the IOMMU platform device from DT as for all other devices, and 
 use device notifiers to defer probe of the bus masters until the required 
 IOMMU(s) are registered.

Notifiers don't work very well for this. Notifier blocks are supposed to
return a very limited number of values, so sneaking in a -EPROBE_DEFER
isn't likely to work out very well.

This was in fact one of Hiroshi's proposals over a year ago and got
refused because of those reasons. The next solution was to introduce a
function, not very much unlike the of_iommu_configure() that would be
called in the core prior to calling into the driver's -probe() callback
so that it could handle this at probe time (as opposed to device
creation time). That way the core can easily defer probe if the IOMMU is
not there yet. At the same time it can simply use the driver model
without requiring per-architecture hacks or workarounds.

Note that there is really no need for any of this configuration or
initialization to happen at device creation time. Drivers won't be able
to use the IOMMU or DMA APIs until their .probe(), so handling this any
earlier is completely unnecessary.

Thierry


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

Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-15 Thread Will Deacon
On Sun, Dec 14, 2014 at 12:45:36PM +, Laurent Pinchart wrote:
 On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
  This patch introduces IOMMU_OF_DECLARE-based initialization to the
  driver, which replaces subsys_initcall-based procedure.
  exynos_iommu_of_setup ensures that each sysmmu controller is probed
  before its master device.
  
  Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
  ---
   drivers/iommu/exynos-iommu.c | 28 +++-
   1 file changed, 27 insertions(+), 1 deletion(-)
  
  diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
  index cd28dc09db39..88f9afe641a0 100644
  --- a/drivers/iommu/exynos-iommu.c
  +++ b/drivers/iommu/exynos-iommu.c
 
 [snip]
 
  @@ -1125,4 +1134,21 @@ err_reg_driver:
  kmem_cache_destroy(lv2table_kmem_cache);
  return ret;
   }
  -subsys_initcall(exynos_iommu_init);
  +
  +static int __init exynos_iommu_of_setup(struct device_node *np)
  +{
  +   struct platform_device *pdev;
  +
  +   if (!init_done)
  +   exynos_iommu_init();
  +
  +   pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
  +   if (IS_ERR(pdev))
  +   return PTR_ERR(pdev);
 
 If we end up having to create the IOMMU platform devices from within the 
 drivers, the introduction of IOMMU_OF_DECLARE starts to feel like a 
 workaround 
 to me. I wonder whether it wouldn't then be better to let the driver core 
 instantiate the IOMMU platform device from DT as for all other devices, and 
 use device notifiers to defer probe of the bus masters until the required 
 IOMMU(s) are registered.
 
 Will, what's your opinion on that ?

Creating the platform device manually for the IOMMU is indeed grotty, but I
don't really understand why it's needed. Interrupt controllers, for example,
seem to get by without one.

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


Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-15 Thread Laurent Pinchart
Hi Will,

On Monday 15 December 2014 17:17:00 Will Deacon wrote:
 On Sun, Dec 14, 2014 at 12:45:36PM +, Laurent Pinchart wrote:
  On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
   This patch introduces IOMMU_OF_DECLARE-based initialization to the
   driver, which replaces subsys_initcall-based procedure.
   exynos_iommu_of_setup ensures that each sysmmu controller is probed
   before its master device.
   
   Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
   ---
   
drivers/iommu/exynos-iommu.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
   
   diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
   index cd28dc09db39..88f9afe641a0 100644
   --- a/drivers/iommu/exynos-iommu.c
   +++ b/drivers/iommu/exynos-iommu.c
  
  [snip]
  
   @@ -1125,4 +1134,21 @@ err_reg_driver:
 kmem_cache_destroy(lv2table_kmem_cache);
 return ret;

}
   
   -subsys_initcall(exynos_iommu_init);
   +
   +static int __init exynos_iommu_of_setup(struct device_node *np)
   +{
   + struct platform_device *pdev;
   +
   + if (!init_done)
   + exynos_iommu_init();
   +
   + pdev = of_platform_device_create(np, NULL,
   platform_bus_type.dev_root);
   + if (IS_ERR(pdev))
   + return PTR_ERR(pdev);
  
  If we end up having to create the IOMMU platform devices from within the
  drivers, the introduction of IOMMU_OF_DECLARE starts to feel like a
  workaround to me. I wonder whether it wouldn't then be better to let the
  driver core instantiate the IOMMU platform device from DT as for all
  other devices, and use device notifiers to defer probe of the bus masters
  until the required IOMMU(s) are registered.
  
  Will, what's your opinion on that ?
 
 Creating the platform device manually for the IOMMU is indeed grotty, but I
 don't really understand why it's needed. Interrupt controllers, for example,
 seem to get by without one.

There's several reasons, one of the most compelling ones I can think of at the 
moment is runtime PM. IRQ controllers close to the CPU use CPU PM notifiers 
instead. Note that IRQ controllers that are further away from the CPU (such as 
GPIO-based IRQ controllers) are real platform devices and use runtime PM.

IOMMUs are not as low-level as system interrupt controllers or system clocks. 
I'm beginning to agree with Thierry that they should be treated as normal 
platform devices as they're not required earlier than probe time of their bus 
master devices.

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-15 Thread Will Deacon
On Mon, Dec 15, 2014 at 05:27:30PM +, Laurent Pinchart wrote:
 On Monday 15 December 2014 17:17:00 Will Deacon wrote:
  On Sun, Dec 14, 2014 at 12:45:36PM +, Laurent Pinchart wrote:
   On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
+static int __init exynos_iommu_of_setup(struct device_node *np)
+{
+   struct platform_device *pdev;
+
+   if (!init_done)
+   exynos_iommu_init();
+
+   pdev = of_platform_device_create(np, NULL,
platform_bus_type.dev_root);
+   if (IS_ERR(pdev))
+   return PTR_ERR(pdev);
   
   If we end up having to create the IOMMU platform devices from within the
   drivers, the introduction of IOMMU_OF_DECLARE starts to feel like a
   workaround to me. I wonder whether it wouldn't then be better to let the
   driver core instantiate the IOMMU platform device from DT as for all
   other devices, and use device notifiers to defer probe of the bus masters
   until the required IOMMU(s) are registered.
   
   Will, what's your opinion on that ?
  
  Creating the platform device manually for the IOMMU is indeed grotty, but I
  don't really understand why it's needed. Interrupt controllers, for example,
  seem to get by without one.
 
 There's several reasons, one of the most compelling ones I can think of at 
 the 
 moment is runtime PM. IRQ controllers close to the CPU use CPU PM notifiers 
 instead. Note that IRQ controllers that are further away from the CPU (such 
 as 
 GPIO-based IRQ controllers) are real platform devices and use runtime PM.

Ok, that's a good point, but the IOMMU will still probe later anyway, right?

 IOMMUs are not as low-level as system interrupt controllers or system clocks. 
 I'm beginning to agree with Thierry that they should be treated as normal 
 platform devices as they're not required earlier than probe time of their bus 
 master devices.

Well, I think you'd have to propose patches for discussion since I'm
certainly not wed to the current approach; I just want something that
allows of_{dma,iommu}_configure to run with the information it needs.

The usual answer is `we should model buses properly', but that's really
not practical afaict.

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


Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-15 Thread Will Deacon
On Mon, Dec 15, 2014 at 05:53:48PM +, Laurent Pinchart wrote:
 Hi Will,

Hello again :)

 On Monday 15 December 2014 17:43:02 Will Deacon wrote:
  On Mon, Dec 15, 2014 at 05:27:30PM +, Laurent Pinchart wrote:
   On Monday 15 December 2014 17:17:00 Will Deacon wrote:
Creating the platform device manually for the IOMMU is indeed grotty,
but I don't really understand why it's needed. Interrupt controllers,
for example, seem to get by without one.
   
   There's several reasons, one of the most compelling ones I can think of at
   the moment is runtime PM. IRQ controllers close to the CPU use CPU PM
   notifiers instead. Note that IRQ controllers that are further away from
   the CPU (such as GPIO-based IRQ controllers) are real platform devices
   and use runtime PM.
 
  Ok, that's a good point, but the IOMMU will still probe later anyway, right?
 
 That depends on the driver implementation, using OF node match an IOMMU 
 driver 
 doesn't have to register a struct driver. Assuming we require IOMMU drivers 
 to 
 register a struct driver, a platform device should then be probed at a later 
 time.
 
 However, if we wait until the IOMMU gets probed to initialize runtime PM and 
 make it functional, we'll be back in square one if the IOMMU gets probed 
 after 
 the bus master, as the bus master could start issuing bus requests at probe 
 time with the IOMMU not powered yet.

True, but couldn't the early init code do enough to get the thing
functional? That said, I'm showing my ignorance here as I'm not familiar
with the PM code (and the software models I have for the SMMU clearly don't
implement anything in this regard).

   IOMMUs are not as low-level as system interrupt controllers or system
   clocks. I'm beginning to agree with Thierry that they should be treated
   as normal platform devices as they're not required earlier than probe
   time of their bus master devices.
  
  Well, I think you'd have to propose patches for discussion since I'm
  certainly not wed to the current approach; I just want something that
  allows of_{dma,iommu}_configure to run with the information it needs.
 
 Do we need of_dma_configure() to run when the device is created, or could we 
 postpone it to just before probe time ?

I'm not sure I can answer that one... Arnd?

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


Re: [PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-12-14 Thread Laurent Pinchart
Hi Marek,

Thank you for the patch.

On Wednesday 19 November 2014 12:15:47 Marek Szyprowski wrote:
 This patch introduces IOMMU_OF_DECLARE-based initialization to the
 driver, which replaces subsys_initcall-based procedure.
 exynos_iommu_of_setup ensures that each sysmmu controller is probed
 before its master device.
 
 Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
 ---
  drivers/iommu/exynos-iommu.c | 28 +++-
  1 file changed, 27 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
 index cd28dc09db39..88f9afe641a0 100644
 --- a/drivers/iommu/exynos-iommu.c
 +++ b/drivers/iommu/exynos-iommu.c

[snip]

 @@ -1125,4 +1134,21 @@ err_reg_driver:
   kmem_cache_destroy(lv2table_kmem_cache);
   return ret;
  }
 -subsys_initcall(exynos_iommu_init);
 +
 +static int __init exynos_iommu_of_setup(struct device_node *np)
 +{
 + struct platform_device *pdev;
 +
 + if (!init_done)
 + exynos_iommu_init();
 +
 + pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
 + if (IS_ERR(pdev))
 + return PTR_ERR(pdev);

If we end up having to create the IOMMU platform devices from within the 
drivers, the introduction of IOMMU_OF_DECLARE starts to feel like a workaround 
to me. I wonder whether it wouldn't then be better to let the driver core 
instantiate the IOMMU platform device from DT as for all other devices, and 
use device notifiers to defer probe of the bus masters until the required 
IOMMU(s) are registered.

Will, what's your opinion on that ?

 +
 + of_iommu_set_ops(np, exynos_iommu_ops);
 + return 0;
 +}
 +
 +IOMMU_OF_DECLARE(exynos_iommu_of, samsung,exynos-sysmmu,
 +  exynos_iommu_of_setup);

-- 
Regards,

Laurent Pinchart

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


[PATCH v3 18/19] iommu: exynos: init from dt-specific callback instead of initcall

2014-11-19 Thread Marek Szyprowski
This patch introduces IOMMU_OF_DECLARE-based initialization to the
driver, which replaces subsys_initcall-based procedure.
exynos_iommu_of_setup ensures that each sysmmu controller is probed
before its master device.

Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
 drivers/iommu/exynos-iommu.c | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index cd28dc09db39..88f9afe641a0 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -13,16 +13,21 @@
 #endif
 
 #include linux/clk.h
+#include linux/dma-mapping.h
 #include linux/err.h
 #include linux/io.h
 #include linux/iommu.h
 #include linux/interrupt.h
 #include linux/list.h
+#include linux/of.h
+#include linux/of_iommu.h
+#include linux/of_platform.h
 #include linux/platform_device.h
 #include linux/pm_runtime.h
 #include linux/slab.h
 
 #include asm/cacheflush.h
+#include asm/dma-iommu.h
 #include asm/pgtable.h
 
 typedef u32 sysmmu_iova_t;
@@ -1084,6 +1089,8 @@ static const struct iommu_ops exynos_iommu_ops = {
.pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
 };
 
+static int init_done;
+
 static int __init exynos_iommu_init(void)
 {
int ret;
@@ -1116,6 +1123,8 @@ static int __init exynos_iommu_init(void)
goto err_set_iommu;
}
 
+   init_done = true;
+
return 0;
 err_set_iommu:
kmem_cache_free(lv2table_kmem_cache, zero_lv2_table);
@@ -1125,4 +1134,21 @@ err_reg_driver:
kmem_cache_destroy(lv2table_kmem_cache);
return ret;
 }
-subsys_initcall(exynos_iommu_init);
+
+static int __init exynos_iommu_of_setup(struct device_node *np)
+{
+   struct platform_device *pdev;
+
+   if (!init_done)
+   exynos_iommu_init();
+
+   pdev = of_platform_device_create(np, NULL, platform_bus_type.dev_root);
+   if (IS_ERR(pdev))
+   return PTR_ERR(pdev);
+
+   of_iommu_set_ops(np, exynos_iommu_ops);
+   return 0;
+}
+
+IOMMU_OF_DECLARE(exynos_iommu_of, samsung,exynos-sysmmu,
+exynos_iommu_of_setup);
-- 
1.9.2

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