2015-01-04 1:45 GMT+09:00 Nishanth Menon <nm@xxxxxx>: > On 01/03/2015 10:16 AM, Tomasz Figa wrote: >> >> 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). > > > > we dont have a machine descriptor for configure instead we overide the > logic(in you case after firmware load, in OMAP case, I need to figure out). > my point being unlike the exynos configure code, OMAP code will look exactly > like current pl310_configure-2 lines of code which really does not benefit > anything. > > > Thinking again, in fact, i'd rather drop this series than have to do a > duplicated configure code(and force a resultant maintenance for the future) > in OMAP code since none of OMAP4 or AM437x series need these patches. > Interest for this series was non-mandatory, but just to be complete from SoC > definition point of view. > > Let me know which way you guys want me to go. Right, dropping this series would definitely solve the the read-back issue. :) After all, if you don't need to override the latencies in the kernel (i.e. have sane firmware, unlike certain Exynos boards ;)), then I don't see a point of having this feature. 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
