Hi, On Wed, Dec 15, 2010 at 8:00 AM, Paul Walmsley <paul@xxxxxxxxx> wrote: > I am reviewing these patches for merging, and would appreciate your help. > > On Sun, 7 Nov 2010, Felipe Contreras wrote: > >> Paul already sent these, but I did some minor modifications, mostly to minimize >> the amount of changes. >> >> Also, I'm re-sending my memblock patch so that the driver can actually be >> compiled. > > It looks like the memblock change has been applied already. > >> With these, and reverting the iommu patches[1], the driver should be working. I >> don't see a more straight-forward way to achieve that. >> >> Paul Walmsley (4): >> Â omap: control: add functions for DSP boot >> Â omap: pm: use control functions in DSP reset code >> Â omap: dsp: add boot control functions >> Â staging: tidspbridge: use boot control functions > > It seems that we still need to apply these. ÂCould you please rebase these > against the 'integration-2.6.38-20101214-013' tag available from > git://git.pwsan.com/linux-integration? Ok, will do. > Also, a few changes are needed in the patches themselves. ÂHere are two > that I noticed: > > First, please add my Signed-off-by: on the first > patch, "omap: control: add functions for DSP boot", as discussed > previously. Yes, I haven't got around to do so. I thought I still had time before .38 merge window. > Second, I notice that you wiped out almost my entire original commit > message from "omap: dsp: add boot control functions", and did not add a > note in brackets explaining what you did or why. ÂI strongly object to > this. ÂThe commit message is as much a part of that patch as the code is > and I would appreciate it if you would either add it back in, or add a > bracketed note next to your Signed-off-by: explaining why the commit > message needed to be removed. Huh? Before: mach-omap2/control: --- Add two functions for OMAP2430/OMAP3 IVA2 DSP boot control. These registers wound up in the System Control Module. Other kernel code that wishes to control the DSP's boot process should now use these functions to do so; subsequent patches implement this in the two in-tree users of these functions. This patch is functionally untested; that is for the DSP/Bridge programmers to do. --- tidspbridge/core/tiomap3430.c: tidspbridge/include/dspbridge/host_os.h: --- DSPBridge currently tries to __raw_writel() to the System Control Module to control the DSP boot process. This is a layering violation; this is SoC-specific code, and belongs in a file in arch/arm/mach-omap2/*. None of those CM and PRM register accesses through struct platform_data belong under drivers/. (Nor would they belong in DSP platform init code in arch/arm/mach-omap2/* - such code must use the clock, clockdomain, powerdomain, omap_device, and omap_hwmod layers that are provided for this purpose.) The immediate problem, however, is that after commit 346a5c890a55495c718394b74214be1de9303cf6, this code can no longer compile. So to fix this immediate problem, convert the DSP boot control code to use platform_data function pointers. The DSPBridge-on-OMAP3 people also need to implement a file in arch/arm/mach-omap2/ to populate the platform_data function pointers. Such a file does not yet exist in the mainline tree, so it's unlikely that DSPBridge is usable in the mainline until this is done. --- After: mach-omap2/control: --- Add two functions for OMAP2430/OMAP3 IVA2 DSP boot control. These registers wound up in the System Control Module. Other kernel code that wishes to control the DSP's boot process should now use these functions to do so; subsequent patches implement this in the two in-tree users of these functions. --- mach-omap2/dsp: --- Would be needed to avoid using SCM directly. --- tidspbridge/core/tiomap3430.c: --- Use the new functions from SCM layer instead of handling registers directly with __raw_writel, as explained in: http://marc.info/?l=linux-omap&m=128779652429922&w=2 This fixes the build on 2.6.37 since control.h is not available for drivers any more. --- I thought it was evident that patch 1 adds some functions, and says those functions should be used to modify SCM registers, patch 2 adds callbacks to them, and patch 3 uses those callbacks instead of directly modifying the registers. I could add "Such access is a layer violation; it should be handled by the platform control (arch/arm/mach-omap2/control.c)". -- Felipe Contreras -- 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
