Re: [PATCH 2/2] iommu/omap: Add support for configuring dsp iommus on DRA7xx

[Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>]

Hi Tony,

On 07/22/2015 12:26 AM, Tony Lindgren wrote:
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [150721 16:58]:
>> --- a/drivers/iommu/omap-iommu.c
>> +++ b/drivers/iommu/omap-iommu.c
>> @@ -26,6 +26,8 @@
>>  #include <linux/of_iommu.h>
>>  #include <linux/of_irq.h>
>>  #include <linux/of_platform.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>>  
>>  #include <asm/cacheflush.h>
>>  
>> @@ -112,6 +114,18 @@ void omap_iommu_restore_ctx(struct device *dev)
>>  }
>>  EXPORT_SYMBOL_GPL(omap_iommu_restore_ctx);
>>  
>> +static void dra7_cfg_dspsys_mmu(struct omap_iommu *obj, bool enable)
>> +{
>> +	u32 val, mask;
>> +
>> +	if (!obj->syscfg)
>> +		return;
>> +
>> +	mask = (1 << (obj->id * DSP_SYS_MMU_CONFIG_EN_SHIFT));
>> +	val = enable ? mask : 0;
>> +	regmap_update_bits(obj->syscfg, DSP_SYS_MMU_CONFIG, mask, val);
>> +}
>> +
>>  static void __iommu_set_twl(struct omap_iommu *obj, bool on)
> 
> I don't like using syscon for tinkering directly with SoC registers.

This is not a SoC-level register, but a register within a sub-module of
the DSP processor sub-system. The DSP_SYSTEM sub-module in general is
described in Section 5.3.3 of the TRM [1], and it implements different
functionalities like the PRCM handshaking, wakeup logic and DSP
subsystem top-level configuration. It is a module present within the DSP
processor sub-system, so can only be accessed when the sub-system is
clocked and the appropriate reset is released.

> We should use some Linux generic framework for configuring these
> bits to avoid nasty dependencies between various hardware modules
> on the SoC.
> 
> What does DSP_SYS_MMU_CONFIG register do? It seems it's probably
> a regulator or a gate clock? If so, it should be set up as a
> regulator or a clock and then the omap-iommu driver can just
> use regulator or clcok framework to request the resource.

No, its neither. It is a control bit that dictates whether the
processor/EDMA addresses go through the respective MMU or not. The
register currently has 4 bits (bit 0 in each nibble), one each for
enabling each MMU and requesting an MMU abort on each. The MMU
integration and enablement notes are detailed in Section 5.3.6 of the
TRM [1], and the DSP_SYS_MMU_CONFIG register layout is in Table 5-28
(Page 1641).

regards
Suman

[1] http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf