Hi Geert, On Mon, Jan 23, 2017 at 9:50 PM, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > Hi Magnus, > > On Mon, Jan 23, 2017 at 1:12 PM, Magnus Damm <magnus.damm@xxxxxxxxx> wrote: >> From: Magnus Damm <damm+renesas@xxxxxxxxxxxxx> >> >> Match on r8a7795 ES2 and enable a certain DMA controller. >> In other cases the IPMMU driver remains disabled. >> >> Signed-off-by: Magnus Damm <damm+renesas@xxxxxxxxxxxxx> >> --- >> >> drivers/iommu/ipmmu-vmsa.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> --- 0001/drivers/iommu/ipmmu-vmsa.c >> +++ work/drivers/iommu/ipmmu-vmsa.c 2017-01-23 20:57:02.620607110 +0900 >> @@ -23,6 +23,7 @@ >> #include <linux/platform_device.h> >> #include <linux/sizes.h> >> #include <linux/slab.h> >> +#include <linux/sys_soc.h> >> >> #if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA) >> #include <asm/dma-iommu.h> >> @@ -1002,16 +1003,39 @@ static void ipmmu_domain_free_dma(struct >> } >> } >> >> +static const struct soc_device_attribute r8a7795es2[] = { >> + { .soc_id = "r8a7795", .revision = "ES2.*" }, >> + { /* sentinel */ } >> +}; >> + >> +static int ipmmu_slave_whitelist(struct device *dev) >> +{ >> + /* Opt-in slave devices based on SoC and ES version */ >> + if (soc_device_match(r8a7795es2)) { >> + if (!strcmp(dev_name(dev), "e7310000.dma-controller")) >> + return 0; >> + } > > I have two comments about the construct above: > 1. IPMMU will be disabled on all non-r8a7795 SoCs. > Is that what you want? Sort of. This patch is just an example to stir up some discussion about this topic. I realize this code as-is changes R-Car Gen2 behavior (that is merged upstream) so perhaps we should keep devices enabled for those SoCs. > 2. Usually we match on the old broken versions instead (e.g. against > "ES1.*"), as (1) it marks more clearly support for old SoCs, and > (2) it makes it easier to remove the check later when these > old SoCs are deemed extinct later. Right, if I understand correctly then you're saying opt-out might be better instead of opt-in. In case of IPMMU and R-Car Gen3 I believe it might be less work to use opt-in rather than excluding not-yet-working stuff. =) With this series I would like to propose to disconnect the DT integration timing from the enablement of IPMMU support for slave devices. If we can enable the IPMMU in DT early on we can reduce potential out-of-tree IPMMU enablement DT patches. So with the DT bindings fixed and accurate data sheet we can merge DT bits ahead of enablement time. And then use run time logic to determine what to enable based on test results. As you are aware, currently we have used the presence of "iommus" in DT to determine if a device is going to be enabled or not. So if the IPMMU Kconfig bits enable the IPMMU driver and the "iommus" DT property tie a certain slave device to the IPMMU then we will make use of IPMMU for a certain device. Currently we assume it will work on all ES versions that use that particular DTB. However ES specific hardware errata together with a wide range of ES versions for r8a7795 and r8a7796 (and whatever SoCs and ES versions that comes next) makes it difficult to use DT like above to enable stuff seemingly on one ES version without potentially breaking other ES versions. I would like to share DT files between the ES versions as much as possible but still only enable IPMMU support for devices that are known to work. Let me know if you think it makes sense to enable DT in a different way than my proposal. I'll have a look at putting the white list code in ->xlate() instead of ->add_device(). Thanks for your comments! Cheers, / magnus