2015-01-04 0:34 GMT+09:00 Nishanth Menon <nm@xxxxxx>: > On 15:40-20150103, Tomasz Figa wrote: >> Hi Nishanth, >> >> 2015-01-03 2:43 GMT+09:00 Nishanth Menon <nm@xxxxxx>: >> > AM437x generation of processors support programming the PL310 L2Cache >> > controller's address filter start and end registers using a secure >> > montior service. >> >> typo: s/montior/monitor/ >> >> [snip] > > Uggh.. yes indeed. I will post a v3 updating the comments. If the > following is ok. >> >> > + base = omap4_get_l2cache_base(); >> > + filter_start = (reg == L310_ADDR_FILTER_START) ? val : >> > + readl_relaxed(base + L310_ADDR_FILTER_START); >> > + filter_end = (reg == L310_ADDR_FILTER_END) ? val : >> > + readl_relaxed(base + L310_ADDR_FILTER_END); >> > + omap_smc1_2(AM43X_MON_L2X0_SETFILTER_INDEX, filter_start, >> > + filter_end); >> > + return; >> >> I don't have any significant comments about this patch in particular, >> but just noticed that you need to do read-backs here (and the typo >> thanks to the spell checker of my mailing app). Maybe you should >> consider switching to the .configure() API I introduced in my series? >> This would let you get rid of the hardcoded static mapping. > > Yeah, I have two choices there.. Either I provide the fundamental > write function for the generic l2c code to use OR I provide a > duplicate of resultant l2c_configure(aux write) + l2c310_configure. > > To allow for reuse of improvements or anything like errata > implementations in the future, OMAP L2C implementation has chosen to provide the > low level code and allow the higherlevel configure/write/whatever of the > future to stay in arch/arm/mm/cache-l2x0.c. The write_sec operation is > not too complicated enough to warrant a replication of l2c310_configure. > > So, I prefer the current implementation than providing a .configure > handler for outer_cache.configure from SoC level. > > Let me know if anyone has a strong objection to this. Well, what l2c310_configure() does after my series is just writing the registers. If they cannot be written normally (without some tricks such as reading back other registers) then IMHO a separate function should be provided. This is becomes possible after patch 3/8 (ARM: l2c: Add interface to ask hypervisor to configure L2C) and what is used on Exynos which also updates multiple registers in single SMC calls. You can find an example of use in patch 6/8 (ARM: EXYNOS: Add .write_sec outer cache callback for L2C-310). What's even more interesting is that approaches similar to the one currently used on OMAP had been NAKed, when proposed for Exynos and this is why we have the solution proposed by my patches. Note that .write_sec() callback is still used for L2X0_CTRL and L2X0_DEBUG_CTRL registers, because there might be a need to write them separately (e.g. to disable the controller and to perform debug operations/workarounds when the controller is already enabled). Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html