[GIT PULL] fixes for tidspbridge 2.6.37-rc1

[GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Felipe Contreras]

Hi Greg,

The situation of tidspbridge driver on staging has been pretty sad,
basically, it has never worked. This is a step backwards from the
previous situation where it was clear which branch to use to get it
working. Unfortunately, the ARM and OMAP trees haven't made it easy,
as changes in those trees have broken it even further.

My proposal to get this fixed is:

1) Revert back the iommu changes

Presumably, in order to get the migration to omap iommu working many
dependencies are needed, which haven't been merged yet. And at least I
have never seen it working, although Fernando claims to have the right
set of patches to do so. I say merging this was premature.

I have reverted back those changes in:
 git://gitorious.org/~felipec/linux-omap/felipec.git fc-dsp-iommu-revert-v37-rc1

That would have been enough for 2.6.36, but...

2) Apply the fixes for recent omap control changes

On 2.6.37-rc1, omap platform internals changed, so the build is broken
again. Paul Walmsley provided some reference patches that I have
modified slightly.

3) Fix for memblock

Changes in memblock also broke this driver, one patch of mine should
fix that, as well as prepare it for imminent changes from the ARM
tree.

All the changes are in:
 git://gitorious.org/~felipec/linux-omap/felipec.git fc-dsp-fix-v37-rc1

With this, the driver should work on 2.6.37, and would be the first
time it does so on staging.

I didn't see any point in sending the patches to review the reverts,
I'm going to send the rest of the patches on top of the branch
fc-dsp-iommu-revert-v37-rc1.

These are the changes since v2.6.37-rc1:

Felipe Contreras (13):
      Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
      Revert "staging: tidspbridge - remove dmm custom module"
      Revert "staging: tidspbridge - deprecate
reserve/unreserve_memory funtions"
      Revert "staging: tidspbridge - remove reserved memory clean up"
      Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
      Revert "staging: tidspbridge - move all iommu related code to a new file"
      Revert "staging: tidspbridge - remove hw directory"
      Revert "staging: tidspbridge - fix mmufault support"
      Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
      Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap
to a proper name"
      Revert "staging: tidspbridge - move shared memory iommu maps to
tiomap3430.c"
      Revert "staging: tidspbridge: replace iommu custom for
opensource implementation"
      omap: dsp: remove shm from normal memory

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

Cheers.

-- 
Felipe Contreras

[PATCH 0/5] omap: dsp: fixes for 2.6.37-rc1

[Felipe Contreras]

Hi,

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.

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.

Cheers.

Felipe Contreras (1):
  omap: dsp: remove shm from normal memory

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

 arch/arm/mach-omap2/control.c                 |   49 +++++++++++++++++++++++++
 arch/arm/mach-omap2/control.h                 |   10 +++---
 arch/arm/mach-omap2/dsp.c                     |    4 ++
 arch/arm/mach-omap2/pm34xx.c                  |    6 ++--
 arch/arm/plat-omap/devices.c                  |    4 ++-
 arch/arm/plat-omap/include/plat/dsp.h         |   10 +++++
 drivers/staging/tidspbridge/core/tiomap3430.c |   13 +++----
 7 files changed, 80 insertions(+), 16 deletions(-)

[1] git://gitorious.org/~felipec/linux-omap/felipec.git fc-dsp-iommu-revert-v37-rc1

-- 
1.7.3.2.3.gf8529

[PATCH 1/5] omap: dsp: remove shm from normal memory

[Felipe Contreras]

Also, don't be picky about the location, which incidentally fixes the
build since MEMBLOCK_REAL_LIMIT is gone on 2.6.37.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/plat-omap/devices.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
index 6f42a18..fc81912 100644
--- a/arch/arm/plat-omap/devices.c
+++ b/arch/arm/plat-omap/devices.c
@@ -284,12 +284,14 @@ void __init omap_dsp_reserve_sdram_memblock(void)
 	if (!size)
 		return;
 
-	paddr = __memblock_alloc_base(size, SZ_1M, MEMBLOCK_REAL_LIMIT);
+	paddr = memblock_alloc(size, SZ_1M);
 	if (!paddr) {
 		pr_err("%s: failed to reserve %x bytes\n",
 				__func__, size);
 		return;
 	}
+	memblock_free(paddr, size);
+	memblock_remove(paddr, size);
 
 	omap_dsp_phys_mempool_base = paddr;
 }
-- 
1.7.3.2.3.gf8529

[PATCH 2/5] omap: control: add functions for DSP boot

[Felipe Contreras]

From: Paul Walmsley <paul@pwsan.com>

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.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/control.c         |   49 +++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/control.h         |   10 +++---
 arch/arm/plat-omap/include/plat/dsp.h |    6 ++++
 3 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 1fa3294..808abeb 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -209,6 +209,55 @@ void omap4_ctrl_pad_writel(u32 val, u16 offset)
 	__raw_writel(val, OMAP4_CTRL_PAD_REGADDR(offset));
 }
 
+/* OMAP DSP control functions */
+
+/**
+ * omap_ctrl_set_dsp_bootaddr - set the DSP's boot address
+ * @pa: DSP boot address (in physical memory)
+ *
+ * Set the DSP's boot address.  This is an address in physical memory.
+ * No return value.  XXX The TRM claims that this is an "index to a
+ * 4kByte page".  If so, why is the bitfield 21 bits wide, rather than
+ * 20?
+ */
+void omap_ctrl_set_dsp_bootaddr(u32 pa)
+{
+	if (!(cpu_is_omap2430() || cpu_is_omap34xx())) {
+		WARN(1, "control: %s: not supported on this SoC\n", __func__);
+		return;
+	}
+
+	WARN(pa & ~OMAP_CTRL_DSP_BOOTADDR_MASK,
+	     "control: %s: invalid DSP boot address %08x\n", __func__, pa);
+
+	omap_ctrl_writel(pa, OMAP243X_CONTROL_IVA2_BOOTADDR);
+}
+
+/**
+ * omap_ctrl_set_dsp_bootmode - set the DSP's boot mode
+ * @mode: DSP boot mode (described below)
+ *
+ * Sets the DSP boot mode - see OMAP3 TRM revision ZH section 7.4.7.4
+ * "IVA2.2 Boot Registers".  Known values of @mode include 0, to boot
+ * directly to the address supplied by omap2_ctrl_set_dsp_bootaddr();
+ * 1, to boot to the DSP's ROM code and idle, waiting for interrupts;
+ * 2, to boot to the DSP's ROM code and spin in an idle loop; 3, to
+ * copy the user's bootstrap code from the DSP's internal memory and
+ * execute it (XXX how does the DSP know where to copy from?); and 4,
+ * to execute the DSP ROM code's context restore code.  No return
+ * value.
+ */
+void omap_ctrl_set_dsp_bootmode(u8 mode)
+{
+	if (!(cpu_is_omap2430() || cpu_is_omap34xx())) {
+		WARN(1, "control: %s: not supported on this SoC\n", __func__);
+		return;
+	}
+
+	omap_ctrl_writel(mode, OMAP243X_CONTROL_IVA2_BOOTMOD);
+}
+
+
 #if defined(CONFIG_ARCH_OMAP3) && defined(CONFIG_PM)
 /*
  * Clears the scratchpad contents in case of cold boot-
diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
index b6c6b7c..105ee00 100644
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -258,11 +258,6 @@
 /* CONTROL_PROG_IO1 bits */
 #define OMAP3630_PRG_SDMMC1_SPEEDCTRL	(1 << 20)
 
-/* CONTROL_IVA2_BOOTMOD bits */
-#define OMAP3_IVA2_BOOTMOD_SHIFT	0
-#define OMAP3_IVA2_BOOTMOD_MASK		(0xf << 0)
-#define OMAP3_IVA2_BOOTMOD_IDLE		(0x1 << 0)
-
 /* CONTROL_PADCONF_X bits */
 #define OMAP3_PADCONF_WAKEUPEVENT0	(1 << 15)
 #define OMAP3_PADCONF_WAKEUPENABLE0	(1 << 14)
@@ -330,6 +325,8 @@
 #define		FEAT_NEON		0
 #define		FEAT_NEON_NONE		1
 
+/* DSP booting-related constants */
+#define OMAP_CTRL_DSP_BOOTADDR_MASK	0xfffffc00
 
 #ifndef __ASSEMBLY__
 #ifdef CONFIG_ARCH_OMAP2PLUS
@@ -351,6 +348,9 @@ extern u32 omap3_arm_context[128];
 extern void omap3_control_save_context(void);
 extern void omap3_control_restore_context(void);
 
+extern void omap_ctrl_set_dsp_bootaddr(u32 pa);
+extern void omap_ctrl_set_dsp_bootmode(u8 mode);
+
 #else
 #define omap_ctrl_base_get()		0
 #define omap_ctrl_readb(x)		0
diff --git a/arch/arm/plat-omap/include/plat/dsp.h b/arch/arm/plat-omap/include/plat/dsp.h
index 9c604b3..079691d 100644
--- a/arch/arm/plat-omap/include/plat/dsp.h
+++ b/arch/arm/plat-omap/include/plat/dsp.h
@@ -28,4 +28,10 @@ extern void omap_dsp_reserve_sdram_memblock(void);
 static inline void omap_dsp_reserve_sdram_memblock(void) { }
 #endif
 
+#define OMAP_DSP_BOOTMODE_USER		0
+#define OMAP_DSP_BOOTMODE_IDLE		1
+#define OMAP_DSP_BOOTMODE_BUSYLOOP		2
+#define OMAP_DSP_BOOTMODE_COPY		3
+#define OMAP_DSP_BOOTMODE_RESTORE		4
+
 #endif
-- 
1.7.3.2.3.gf8529

[PATCH 3/5] omap: pm: use control functions in DSP reset code

[Felipe Contreras]

From: Paul Walmsley <paul@pwsan.com>

Update the DSP reset code in pm34xx.c to use one of the new SCM DSP boot
control functions, omap_ctrl_set_dsp_bootmode().

This reset code should be moved out to a separate function to be called
by the hwmod reset process at some point.  Also, 2430 should be
initializing the DSP in a similar fashion.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/pm34xx.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 75c0cd1..86e7e99 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -37,6 +37,7 @@
 #include <plat/prcm.h>
 #include <plat/gpmc.h>
 #include <plat/dma.h>
+#include <plat/dsp.h>
 
 #include <asm/tlbflush.h>
 
@@ -614,6 +615,7 @@ static struct platform_suspend_ops omap_pm_ops = {
  * function forces the IVA2 into idle state so it can go
  * into retention/off and thus allow full-chip retention/off.
  *
+ * XXX This should be handled by the hwmod.
  **/
 static void __init omap3_iva_idle(void)
 {
@@ -635,9 +637,7 @@ static void __init omap3_iva_idle(void)
 	cm_write_mod_reg(OMAP3430_CM_FCLKEN_IVA2_EN_IVA2_MASK,
 			 OMAP3430_IVA2_MOD, CM_FCLKEN);
 
-	/* Set IVA2 boot mode to 'idle' */
-	omap_ctrl_writel(OMAP3_IVA2_BOOTMOD_IDLE,
-			 OMAP343X_CONTROL_IVA2_BOOTMOD);
+	omap_ctrl_set_dsp_bootmode(OMAP_DSP_BOOTMODE_IDLE);
 
 	/* Un-reset IVA2 */
 	prm_write_mod_reg(0, OMAP3430_IVA2_MOD, OMAP2_RM_RSTCTRL);
-- 
1.7.3.2.3.gf8529

[PATCH 4/5] omap: dsp: add boot control functions

[Felipe Contreras]

From: Paul Walmsley <paul@pwsan.com>

Would be needed to avoid using SCM directly.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/dsp.c             |    4 ++++
 arch/arm/plat-omap/include/plat/dsp.h |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/dsp.c b/arch/arm/mach-omap2/dsp.c
index 6feeeae..6a21ac6 100644
--- a/arch/arm/mach-omap2/dsp.c
+++ b/arch/arm/mach-omap2/dsp.c
@@ -12,6 +12,7 @@
  */
 
 #include <linux/platform_device.h>
+#include "control.h"
 #include "prm.h"
 #include "cm.h"
 #ifdef CONFIG_BRIDGE_DVFS
@@ -31,6 +32,9 @@ static struct omap_dsp_platform_data omap_dsp_pdata __initdata = {
 	.cpu_set_freq = omap_pm_cpu_set_freq,
 	.cpu_get_freq = omap_pm_cpu_get_freq,
 #endif
+	.set_dsp_bootaddr = omap_ctrl_set_dsp_bootaddr,
+	.set_dsp_bootmode = omap_ctrl_set_dsp_bootmode,
+
 	.dsp_prm_read = prm_read_mod_reg,
 	.dsp_prm_write = prm_write_mod_reg,
 	.dsp_prm_rmw_bits = prm_rmw_mod_reg_bits,
diff --git a/arch/arm/plat-omap/include/plat/dsp.h b/arch/arm/plat-omap/include/plat/dsp.h
index 079691d..b2cf15b 100644
--- a/arch/arm/plat-omap/include/plat/dsp.h
+++ b/arch/arm/plat-omap/include/plat/dsp.h
@@ -10,7 +10,11 @@ struct omap_dsp_platform_data {
 	unsigned long (*cpu_get_freq) (void);
 	unsigned long mpu_speed[6];
 
+	void (*set_dsp_bootaddr)(u32 pa);
+	void (*set_dsp_bootmode)(u8 mode);
+
 	/* functions to write and read PRCM registers */
+	/* XXX None of this should be here */
 	void (*dsp_prm_write)(u32, s16 , u16);
 	u32 (*dsp_prm_read)(s16 , u16);
 	u32 (*dsp_prm_rmw_bits)(u32, u32, s16, s16);
-- 
1.7.3.2.3.gf8529

[PATCH 5/5] staging: tidspbridge: use boot control functions

[Felipe Contreras]

From: Paul Walmsley <paul@pwsan.com>

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.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 drivers/staging/tidspbridge/core/tiomap3430.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430.c b/drivers/staging/tidspbridge/core/tiomap3430.c
index cb38743..84de0c2 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430.c
@@ -23,7 +23,7 @@
 #include <dspbridge/host_os.h>
 #include <linux/mm.h>
 #include <linux/mmzone.h>
-#include <plat/control.h>
+#include <plat/dsp.h>
 
 /*  ----------------------------------- DSP/BIOS Bridge */
 #include <dspbridge/dbdefs.h>
@@ -412,19 +412,18 @@ static int bridge_brd_start(struct bridge_dev_context *dev_ctxt,
 
 		/* Assert RST1 i.e only the RST only for DSP megacell */
 		if (!status) {
+			u8 bootmode;
 			(*pdata->dsp_prm_rmw_bits)(OMAP3430_RST1_IVA2_MASK,
 					OMAP3430_RST1_IVA2_MASK, OMAP3430_IVA2_MOD,
 					OMAP2_RM_RSTCTRL);
 			/* Mask address with 1K for compatibility */
-			__raw_writel(dsp_addr & OMAP3_IVA2_BOOTADDR_MASK,
-					OMAP343X_CTRL_REGADDR(
-					OMAP343X_CONTROL_IVA2_BOOTADDR));
+			dsp_addr &= OMAP3_IVA2_BOOTADDR_MASK;
+			(*pdata->set_dsp_bootaddr)(dsp_addr);
 			/*
 			 * Set bootmode to self loop if dsp_debug flag is true
 			 */
-			__raw_writel((dsp_debug) ? OMAP3_IVA2_BOOTMOD_IDLE : 0,
-					OMAP343X_CTRL_REGADDR(
-					OMAP343X_CONTROL_IVA2_BOOTMOD));
+			bootmode = dsp_debug ? OMAP_DSP_BOOTMODE_IDLE : 0;
+			(*pdata->set_dsp_bootmode)(bootmode);
 		}
 	}
 	if (!status) {
-- 
1.7.3.2.3.gf8529

Re: [PATCH 0/5] omap: dsp: fixes for 2.6.37-rc1

[Greg KH]

On Sun, Nov 07, 2010 at 11:43:24PM +0200, Felipe Contreras wrote:
> Hi,
> 
> 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.
> 
> 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.

I can't take these through the staging tree unless I get an ack from the
omap maintainers.

And as you are adding new infrastructure to the core kernel, this really
needed to be there before .37-rc1, I don't think it's acceptable to
merge this late in the development cycle.

thanks,

greg k-h

Re: [PATCH 0/5] omap: dsp: fixes for 2.6.37-rc1

[Felipe Contreras]

On Mon, Nov 8, 2010 at 12:02 AM, Greg KH <greg@kroah.com> wrote:
> On Sun, Nov 07, 2010 at 11:43:24PM +0200, 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.
>>
>> 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.
>
> I can't take these through the staging tree unless I get an ack from the
> omap maintainers.
>
> And as you are adding new infrastructure to the core kernel, this really
> needed to be there before .37-rc1, I don't think it's acceptable to
> merge this late in the development cycle.

Isn't that up to omap maintainers to decide?

Anyway, there's an alternative that doesn't require omap to ack:

--- a/drivers/staging/tidspbridge/core/tiomap3430.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430.c
@@ -23,7 +23,6 @@
 #include <dspbridge/host_os.h>
 #include <linux/mm.h>
 #include <linux/mmzone.h>
-#include <plat/control.h>

 /*  ----------------------------------- DSP/BIOS Bridge */
 #include <dspbridge/dbdefs.h>
@@ -74,6 +73,18 @@
 #define PAGES_II_LVL_TABLE   512
 #define PHYS_TO_PAGE(phys)      pfn_to_page((phys) >> PAGE_SHIFT)

+/*
+ * This is a totally ugly layer violation, but needed until
+ * omap_ctrl_set_dsp_boot*() are provided.
+ */
+#define OMAP3_IVA2_BOOTMOD_IDLE 1
+#define OMAP2_CONTROL_GENERAL 0x270
+#define OMAP343X_CONTROL_IVA2_BOOTADDR (OMAP2_CONTROL_GENERAL + 0x0190)
+#define OMAP343X_CONTROL_IVA2_BOOTMOD (OMAP2_CONTROL_GENERAL + 0x0194)
+
+#define OMAP343X_CTRL_REGADDR(reg) \
+       OMAP2_L4_IO_ADDRESS(OMAP343X_CTRL_BASE + (reg))
+
 /* Forward Declarations: */
 static int bridge_brd_monitor(struct bridge_dev_context *dev_ctxt);
 static int bridge_brd_read(struct bridge_dev_context *dev_ctxt,

-- 
Felipe Contreras

Re: [PATCH 0/5] omap: dsp: fixes for 2.6.37-rc1

[Tony Lindgren]

* Felipe Contreras <felipe.contreras@gmail.com> [101107 14:38]:
> On Mon, Nov 8, 2010 at 12:02 AM, Greg KH <greg@kroah.com> wrote:
> > On Sun, Nov 07, 2010 at 11:43:24PM +0200, 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.
> >>
> >> 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.
> >
> > I can't take these through the staging tree unless I get an ack from the
> > omap maintainers.
> >
> > And as you are adding new infrastructure to the core kernel, this really
> > needed to be there before .37-rc1, I don't think it's acceptable to
> > merge this late in the development cycle.
> 
> Isn't that up to omap maintainers to decide?

Thanks for putting together all these patches, that already helps
people working on it a great deal.

However, these patches are in the no-no category for -rc cycle, they're
in the "fixes for features that never worked in the first place"
category. Linus will not take it and I don't want to take it. We just
have to follow the standard Linux development cycle.
 
> Anyway, there's an alternative that doesn't require omap to ack:
> 
> --- a/drivers/staging/tidspbridge/core/tiomap3430.c
> +++ b/drivers/staging/tidspbridge/core/tiomap3430.c
> @@ -23,7 +23,6 @@
>  #include <dspbridge/host_os.h>
>  #include <linux/mm.h>
>  #include <linux/mmzone.h>
> -#include <plat/control.h>
> 
>  /*  ----------------------------------- DSP/BIOS Bridge */
>  #include <dspbridge/dbdefs.h>
> @@ -74,6 +73,18 @@
>  #define PAGES_II_LVL_TABLE   512
>  #define PHYS_TO_PAGE(phys)      pfn_to_page((phys) >> PAGE_SHIFT)
> 
> +/*
> + * This is a totally ugly layer violation, but needed until
> + * omap_ctrl_set_dsp_boot*() are provided.
> + */
> +#define OMAP3_IVA2_BOOTMOD_IDLE 1
> +#define OMAP2_CONTROL_GENERAL 0x270
> +#define OMAP343X_CONTROL_IVA2_BOOTADDR (OMAP2_CONTROL_GENERAL + 0x0190)
> +#define OMAP343X_CONTROL_IVA2_BOOTMOD (OMAP2_CONTROL_GENERAL + 0x0194)
> +
> +#define OMAP343X_CTRL_REGADDR(reg) \
> +       OMAP2_L4_IO_ADDRESS(OMAP343X_CTRL_BASE + (reg))
> +
>  /* Forward Declarations: */
>  static int bridge_brd_monitor(struct bridge_dev_context *dev_ctxt);
>  static int bridge_brd_read(struct bridge_dev_context *dev_ctxt,

If this makes the dspbridge usable, that sounds like the way to go
for the -rc cycle.

But I don't think this the only patch needed then?

Regards,

Tony

Re: [PATCH 0/5] omap: dsp: fixes for 2.6.37-rc1

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [101108 08:37]:
> * Felipe Contreras <felipe.contreras@gmail.com> [101107 14:38]:
> 
> Thanks for putting together all these patches, that already helps
> people working on it a great deal.
> 
> However, these patches are in the no-no category for -rc cycle, they're
> in the "fixes for features that never worked in the first place"
> category. Linus will not take it and I don't want to take it. We just
> have to follow the standard Linux development cycle.
>  
> > Anyway, there's an alternative that doesn't require omap to ack:
> > 
> > --- a/drivers/staging/tidspbridge/core/tiomap3430.c
> > +++ b/drivers/staging/tidspbridge/core/tiomap3430.c
> > @@ -23,7 +23,6 @@
> >  #include <dspbridge/host_os.h>
> >  #include <linux/mm.h>
> >  #include <linux/mmzone.h>
> > -#include <plat/control.h>
> > 
> >  /*  ----------------------------------- DSP/BIOS Bridge */
> >  #include <dspbridge/dbdefs.h>
> > @@ -74,6 +73,18 @@
> >  #define PAGES_II_LVL_TABLE   512
> >  #define PHYS_TO_PAGE(phys)      pfn_to_page((phys) >> PAGE_SHIFT)
> > 
> > +/*
> > + * This is a totally ugly layer violation, but needed until
> > + * omap_ctrl_set_dsp_boot*() are provided.
> > + */
> > +#define OMAP3_IVA2_BOOTMOD_IDLE 1
> > +#define OMAP2_CONTROL_GENERAL 0x270
> > +#define OMAP343X_CONTROL_IVA2_BOOTADDR (OMAP2_CONTROL_GENERAL + 0x0190)
> > +#define OMAP343X_CONTROL_IVA2_BOOTMOD (OMAP2_CONTROL_GENERAL + 0x0194)
> > +
> > +#define OMAP343X_CTRL_REGADDR(reg) \
> > +       OMAP2_L4_IO_ADDRESS(OMAP343X_CTRL_BASE + (reg))
> > +
> >  /* Forward Declarations: */
> >  static int bridge_brd_monitor(struct bridge_dev_context *dev_ctxt);
> >  static int bridge_brd_read(struct bridge_dev_context *dev_ctxt,
> 
> If this makes the dspbridge usable, that sounds like the way to go
> for the -rc cycle.
> 
> But I don't think this the only patch needed then?

I guess it would be this fix + omap: dsp: remove shm from normal memory +
revert some drivers/staging/tidspbridge patches?

It's still quite a bit of patches for the -rc cycle..

If you want, I can keep merging your branch into linux-omap master
branch for testing while we're waiting for the next merge window. 

Regards,

Tony

Re: [PATCH 0/5] omap: dsp: fixes for 2.6.37-rc1

[Felipe Contreras]

On Mon, Nov 8, 2010 at 9:16 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Tony Lindgren <tony@atomide.com> [101108 08:37]:
>> * Felipe Contreras <felipe.contreras@gmail.com> [101107 14:38]:
>>
>> Thanks for putting together all these patches, that already helps
>> people working on it a great deal.
>>
>> However, these patches are in the no-no category for -rc cycle, they're
>> in the "fixes for features that never worked in the first place"
>> category. Linus will not take it and I don't want to take it. We just
>> have to follow the standard Linux development cycle.
>>
>> > Anyway, there's an alternative that doesn't require omap to ack:
>> >
>> > --- a/drivers/staging/tidspbridge/core/tiomap3430.c
>> > +++ b/drivers/staging/tidspbridge/core/tiomap3430.c
>> > @@ -23,7 +23,6 @@
>> >  #include <dspbridge/host_os.h>
>> >  #include <linux/mm.h>
>> >  #include <linux/mmzone.h>
>> > -#include <plat/control.h>
>> >
>> >  /*  ----------------------------------- DSP/BIOS Bridge */
>> >  #include <dspbridge/dbdefs.h>
>> > @@ -74,6 +73,18 @@
>> >  #define PAGES_II_LVL_TABLE   512
>> >  #define PHYS_TO_PAGE(phys)      pfn_to_page((phys) >> PAGE_SHIFT)
>> >
>> > +/*
>> > + * This is a totally ugly layer violation, but needed until
>> > + * omap_ctrl_set_dsp_boot*() are provided.
>> > + */
>> > +#define OMAP3_IVA2_BOOTMOD_IDLE 1
>> > +#define OMAP2_CONTROL_GENERAL 0x270
>> > +#define OMAP343X_CONTROL_IVA2_BOOTADDR (OMAP2_CONTROL_GENERAL + 0x0190)
>> > +#define OMAP343X_CONTROL_IVA2_BOOTMOD (OMAP2_CONTROL_GENERAL + 0x0194)
>> > +
>> > +#define OMAP343X_CTRL_REGADDR(reg) \
>> > +       OMAP2_L4_IO_ADDRESS(OMAP343X_CTRL_BASE + (reg))
>> > +
>> >  /* Forward Declarations: */
>> >  static int bridge_brd_monitor(struct bridge_dev_context *dev_ctxt);
>> >  static int bridge_brd_read(struct bridge_dev_context *dev_ctxt,
>>
>> If this makes the dspbridge usable, that sounds like the way to go
>> for the -rc cycle.
>>
>> But I don't think this the only patch needed then?
>
> I guess it would be this fix + omap: dsp: remove shm from normal memory +
> revert some drivers/staging/tidspbridge patches?
>
> It's still quite a bit of patches for the -rc cycle..

Fine, then all you need to do is ack one patch:
http://article.gmane.org/gmane.linux.ports.arm.omap/45076

I sent this 20 days ago, it's 3 lines of diff, and it fixes a break
issue that was introduced on 2.6.37-rc1. This would not have made
sense before .37 because of the memblock changes that just got in. You
should have picked this one for the .37 merge window, there was
nothing preventing that. But it's still not too late because this is
not new code, it's a fix.

But you don't need to pick this one, all you need to do is ack it.

The rest is for Greg to decide.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Felipe Contreras]

On Sun, Nov 7, 2010 at 11:36 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> The situation of tidspbridge driver on staging has been pretty sad,
> basically, it has never worked. This is a step backwards from the
> previous situation where it was clear which branch to use to get it
> working. Unfortunately, the ARM and OMAP trees haven't made it easy,
> as changes in those trees have broken it even further.

Here's another try, this one has only one patch that needs omap
maintainers to ack.

 git://gitorious.org/~felipec/linux-omap/felipec.git fc-dsp-fix-v37-rc1-min

These are the changes since v2.6.37-rc1:

Felipe Contreras (14):
      Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
      Revert "staging: tidspbridge - remove dmm custom module"
      Revert "staging: tidspbridge - deprecate
reserve/unreserve_memory funtions"
      Revert "staging: tidspbridge - remove reserved memory clean up"
      Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
      Revert "staging: tidspbridge - move all iommu related code to a new file"
      Revert "staging: tidspbridge - remove hw directory"
      Revert "staging: tidspbridge - fix mmufault support"
      Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
      Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap
to a proper name"
      Revert "staging: tidspbridge - move shared memory iommu maps to
tiomap3430.c"
      Revert "staging: tidspbridge: replace iommu custom for
opensource implementation"
      staging: tidspbridge: hack to make it compile on v37-rc1
      omap: dsp: remove shm from normal memory

-- 
Felipe Contreras

Re: [PATCH 0/5] omap: dsp: fixes for 2.6.37-rc1

[Tony Lindgren]

* Felipe Contreras <felipe.contreras@gmail.com> [101108 14:47]:
> On Mon, Nov 8, 2010 at 9:16 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Tony Lindgren <tony@atomide.com> [101108 08:37]:
> >> * Felipe Contreras <felipe.contreras@gmail.com> [101107 14:38]:
> >>
> >> Thanks for putting together all these patches, that already helps
> >> people working on it a great deal.
> >>
> >> However, these patches are in the no-no category for -rc cycle, they're
> >> in the "fixes for features that never worked in the first place"
> >> category. Linus will not take it and I don't want to take it. We just
> >> have to follow the standard Linux development cycle.
> >>
> >> > Anyway, there's an alternative that doesn't require omap to ack:
> >> >
> >> > --- a/drivers/staging/tidspbridge/core/tiomap3430.c
> >> > +++ b/drivers/staging/tidspbridge/core/tiomap3430.c
> >> > @@ -23,7 +23,6 @@
> >> >  #include <dspbridge/host_os.h>
> >> >  #include <linux/mm.h>
> >> >  #include <linux/mmzone.h>
> >> > -#include <plat/control.h>
> >> >
> >> >  /*  ----------------------------------- DSP/BIOS Bridge */
> >> >  #include <dspbridge/dbdefs.h>
> >> > @@ -74,6 +73,18 @@
> >> >  #define PAGES_II_LVL_TABLE   512
> >> >  #define PHYS_TO_PAGE(phys)      pfn_to_page((phys) >> PAGE_SHIFT)
> >> >
> >> > +/*
> >> > + * This is a totally ugly layer violation, but needed until
> >> > + * omap_ctrl_set_dsp_boot*() are provided.
> >> > + */
> >> > +#define OMAP3_IVA2_BOOTMOD_IDLE 1
> >> > +#define OMAP2_CONTROL_GENERAL 0x270
> >> > +#define OMAP343X_CONTROL_IVA2_BOOTADDR (OMAP2_CONTROL_GENERAL + 0x0190)
> >> > +#define OMAP343X_CONTROL_IVA2_BOOTMOD (OMAP2_CONTROL_GENERAL + 0x0194)
> >> > +
> >> > +#define OMAP343X_CTRL_REGADDR(reg) \
> >> > +       OMAP2_L4_IO_ADDRESS(OMAP343X_CTRL_BASE + (reg))
> >> > +
> >> >  /* Forward Declarations: */
> >> >  static int bridge_brd_monitor(struct bridge_dev_context *dev_ctxt);
> >> >  static int bridge_brd_read(struct bridge_dev_context *dev_ctxt,
> >>
> >> If this makes the dspbridge usable, that sounds like the way to go
> >> for the -rc cycle.
> >>
> >> But I don't think this the only patch needed then?
> >
> > I guess it would be this fix + omap: dsp: remove shm from normal memory +
> > revert some drivers/staging/tidspbridge patches?
> >
> > It's still quite a bit of patches for the -rc cycle..
> 
> Fine, then all you need to do is ack one patch:
> http://article.gmane.org/gmane.linux.ports.arm.omap/45076
> 
> I sent this 20 days ago, it's 3 lines of diff, and it fixes a break
> issue that was introduced on 2.6.37-rc1. This would not have made
> sense before .37 because of the memblock changes that just got in. You
> should have picked this one for the .37 merge window, there was
> nothing preventing that. But it's still not too late because this is
> not new code, it's a fix.

Yes I agree that's a fix now that the memblock code is in.
 
> But you don't need to pick this one, all you need to do is ack it.

OK, will reply with my ack the same patch in this thread.
 
> The rest is for Greg to decide.

Yes it's up to Greg now. 

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] omap: dsp: remove shm from normal memory

[Tony Lindgren]

* Felipe Contreras <felipe.contreras@gmail.com> [101107 13:34]:
> Also, don't be picky about the location, which incidentally fixes the
> build since MEMBLOCK_REAL_LIMIT is gone on 2.6.37.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

This is a valid fix for 2.6.37 -rc cycle now that the memblock code
is in and should get merged with other drivers/staging/tidspbridge
related fixes.

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  arch/arm/plat-omap/devices.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/devices.c b/arch/arm/plat-omap/devices.c
> index 6f42a18..fc81912 100644
> --- a/arch/arm/plat-omap/devices.c
> +++ b/arch/arm/plat-omap/devices.c
> @@ -284,12 +284,14 @@ void __init omap_dsp_reserve_sdram_memblock(void)
>  	if (!size)
>  		return;
>  
> -	paddr = __memblock_alloc_base(size, SZ_1M, MEMBLOCK_REAL_LIMIT);
> +	paddr = memblock_alloc(size, SZ_1M);
>  	if (!paddr) {
>  		pr_err("%s: failed to reserve %x bytes\n",
>  				__func__, size);
>  		return;
>  	}
> +	memblock_free(paddr, size);
> +	memblock_remove(paddr, size);
>  
>  	omap_dsp_phys_mempool_base = paddr;
>  }
> -- 
> 1.7.3.2.3.gf8529
> 

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Tony Lindgren]

* Felipe Contreras <felipe.contreras@gmail.com> [101108 14:51]:
> On Sun, Nov 7, 2010 at 11:36 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> > The situation of tidspbridge driver on staging has been pretty sad,
> > basically, it has never worked. This is a step backwards from the
> > previous situation where it was clear which branch to use to get it
> > working. Unfortunately, the ARM and OMAP trees haven't made it easy,
> > as changes in those trees have broken it even further.

Hmm to me it looks like these reverts you are suggesting are
for drivers/staging/tidspbridge, not for arch/arm/* :)
 
> Here's another try, this one has only one patch that needs omap
> maintainers to ack.

Acked that memblock patch it in this thread.

Tony

Re: [PATCH 1/5] omap: dsp: remove shm from normal memory

[Greg KH]

On Mon, Nov 08, 2010 at 04:53:21PM -0800, Tony Lindgren wrote:
> * Felipe Contreras <felipe.contreras@gmail.com> [101107 13:34]:
> > Also, don't be picky about the location, which incidentally fixes the
> > build since MEMBLOCK_REAL_LIMIT is gone on 2.6.37.
> > 
> > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> 
> This is a valid fix for 2.6.37 -rc cycle now that the memblock code
> is in and should get merged with other drivers/staging/tidspbridge
> related fixes.
> 
> Acked-by: Tony Lindgren <tony@atomide.com>

Ok, I'll take this in the staging-linus tree, thanks for the ack.

Felipe, I'll pull in your other reverts as well thanks for doing them.
I hope to get to them by the end of the week, lots of work to do due to
being gone last week...

thanks,

greg k-h

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Arnd Bergmann]

On Tuesday 09 November 2010, Felipe Contreras wrote:
> Felipe Contreras (14):
>       Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
>       Revert "staging: tidspbridge - remove dmm custom module"
>       Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
>       Revert "staging: tidspbridge - remove reserved memory clean up"
>       Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
>       Revert "staging: tidspbridge - move all iommu related code to a new file"
>       Revert "staging: tidspbridge - remove hw directory"
>       Revert "staging: tidspbridge - fix mmufault support"
>       Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
>       Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
>       Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
>       Revert "staging: tidspbridge: replace iommu custom for opensource implementation"

That adds quite a lot of crap back in that was removed by Fernando earlier:

 44 files changed, 3733 insertions(+), 847 deletions(-)

It may have been premature to merge the patches as you say, but now that
they are in, I'd vote for giving Fernando a chance to fix up any damage
that was done in the process rather than just reverting all the useful
changes.

	Arnd

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Guzman Lugo, Fernando]

On Tue, Nov 9, 2010 at 10:29 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 09 November 2010, Felipe Contreras wrote:
>> Felipe Contreras (14):
>>       Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
>>       Revert "staging: tidspbridge - remove dmm custom module"
>>       Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
>>       Revert "staging: tidspbridge - remove reserved memory clean up"
>>       Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
>>       Revert "staging: tidspbridge - move all iommu related code to a new file"
>>       Revert "staging: tidspbridge - remove hw directory"
>>       Revert "staging: tidspbridge - fix mmufault support"
>>       Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
>>       Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
>>       Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
>>       Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
>
> That adds quite a lot of crap back in that was removed by Fernando earlier:
>
>  44 files changed, 3733 insertions(+), 847 deletions(-)
>
> It may have been premature to merge the patches as you say, but now that
> they are in, I'd vote for giving Fernando a chance to fix up any damage
> that was done in the process rather than just reverting all the useful
> changes.
>
>        Arnd

tidspbridge iommu change are working fine all the patches and few fixes after
that are alredy sent. what breaks tidspbridge, is the unmerged
dependencies in linux omap
tree, specifically the iommu module patches and the SG patch. if the
dependencies are merged tidspbridge work perfectly I don't need to fix
anything. As the dependencies are not merge yet it is ok to revert and
once push once all the dependencies are merge. I am not familiar with
that so I don't know how much time can it take.

Regards,
Fernando.

>

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Greg KH]

On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
> On Tuesday 09 November 2010, Felipe Contreras wrote:
> > Felipe Contreras (14):
> >       Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
> >       Revert "staging: tidspbridge - remove dmm custom module"
> >       Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
> >       Revert "staging: tidspbridge - remove reserved memory clean up"
> >       Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
> >       Revert "staging: tidspbridge - move all iommu related code to a new file"
> >       Revert "staging: tidspbridge - remove hw directory"
> >       Revert "staging: tidspbridge - fix mmufault support"
> >       Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
> >       Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
> >       Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
> >       Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
> 
> That adds quite a lot of crap back in that was removed by Fernando earlier:
> 
>  44 files changed, 3733 insertions(+), 847 deletions(-)
> 
> It may have been premature to merge the patches as you say, but now that
> they are in, I'd vote for giving Fernando a chance to fix up any damage
> that was done in the process rather than just reverting all the useful
> changes.

In looking at this further, I agree.

Felipe, are all of these really needing to be reverted?  How about
picking out the functional changes that need to be resolved instead of
just rolling back everything that has been done here.  Surely not all of
these are wrong, right?

thanks,

greg k-h

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Guzman Lugo, Fernando]

On Tue, Nov 9, 2010 at 10:55 AM, Greg KH <greg@kroah.com> wrote:
> On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
>> On Tuesday 09 November 2010, Felipe Contreras wrote:
>> > Felipe Contreras (14):
>> >       Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
>> >       Revert "staging: tidspbridge - remove dmm custom module"
>> >       Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
>> >       Revert "staging: tidspbridge - remove reserved memory clean up"
>> >       Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
>> >       Revert "staging: tidspbridge - move all iommu related code to a new file"
>> >       Revert "staging: tidspbridge - remove hw directory"
>> >       Revert "staging: tidspbridge - fix mmufault support"
>> >       Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
>> >       Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
>> >       Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
>> >       Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
>>
>> That adds quite a lot of crap back in that was removed by Fernando earlier:
>>
>>  44 files changed, 3733 insertions(+), 847 deletions(-)
>>
>> It may have been premature to merge the patches as you say, but now that
>> they are in, I'd vote for giving Fernando a chance to fix up any damage
>> that was done in the process rather than just reverting all the useful
>> changes.
>
> In looking at this further, I agree.
>
> Felipe, are all of these really needing to be reverted?  How about
> picking out the functional changes that need to be resolved instead of
> just rolling back everything that has been done here.  Surely not all of
> these are wrong, right?

Patches are _NOT_ wrong, missing dependencies break the bridge.
Without that dependencies the first patch of the set won't work and
all other patches have dependency on the first one, so all of them
need to be reverted.

Regards,
Fernando.

>
> thanks,
>
> greg k-h
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Greg KH]

On Tue, Nov 09, 2010 at 11:04:18AM -0600, Guzman Lugo, Fernando wrote:
> On Tue, Nov 9, 2010 at 10:55 AM, Greg KH <greg@kroah.com> wrote:
> > On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
> >> On Tuesday 09 November 2010, Felipe Contreras wrote:
> >> > Felipe Contreras (14):
> >> >       Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
> >> >       Revert "staging: tidspbridge - remove dmm custom module"
> >> >       Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
> >> >       Revert "staging: tidspbridge - remove reserved memory clean up"
> >> >       Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
> >> >       Revert "staging: tidspbridge - move all iommu related code to a new file"
> >> >       Revert "staging: tidspbridge - remove hw directory"
> >> >       Revert "staging: tidspbridge - fix mmufault support"
> >> >       Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
> >> >       Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
> >> >       Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
> >> >       Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
> >>
> >> That adds quite a lot of crap back in that was removed by Fernando earlier:
> >>
> >>  44 files changed, 3733 insertions(+), 847 deletions(-)
> >>
> >> It may have been premature to merge the patches as you say, but now that
> >> they are in, I'd vote for giving Fernando a chance to fix up any damage
> >> that was done in the process rather than just reverting all the useful
> >> changes.
> >
> > In looking at this further, I agree.
> >
> > Felipe, are all of these really needing to be reverted?  How about
> > picking out the functional changes that need to be resolved instead of
> > just rolling back everything that has been done here.  Surely not all of
> > these are wrong, right?
> 
> Patches are _NOT_ wrong, missing dependencies break the bridge.
> Without that dependencies the first patch of the set won't work and
> all other patches have dependency on the first one, so all of them
> need to be reverted.

How about hand-reverting only the wrong patch, so the other work isn't
lost?  I'd much prefer that.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Tony Lindgren]

* Guzman Lugo, Fernando <fernando.lugo@ti.com> [101109 08:36]:
> 
> tidspbridge iommu change are working fine all the patches and few fixes after
> that are alredy sent. what breaks tidspbridge, is the unmerged
> dependencies in linux omap tree, specifically the iommu module patches
> and the SG patch.

Care to post a series of the missing patches listed above?

That way we can at least start merging those into linux-omap for
testing while waiting for the next merge window.

Tony

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Guzman Lugo, Fernando]

On Tue, Nov 9, 2010 at 11:25 AM, Greg KH <greg@kroah.com> wrote:
> On Tue, Nov 09, 2010 at 11:04:18AM -0600, Guzman Lugo, Fernando wrote:
>> On Tue, Nov 9, 2010 at 10:55 AM, Greg KH <greg@kroah.com> wrote:
>> > On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
>> >> On Tuesday 09 November 2010, Felipe Contreras wrote:
>> >> > Felipe Contreras (14):
>> >> >       Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
>> >> >       Revert "staging: tidspbridge - remove dmm custom module"
>> >> >       Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
>> >> >       Revert "staging: tidspbridge - remove reserved memory clean up"
>> >> >       Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
>> >> >       Revert "staging: tidspbridge - move all iommu related code to a new file"
>> >> >       Revert "staging: tidspbridge - remove hw directory"
>> >> >       Revert "staging: tidspbridge - fix mmufault support"
>> >> >       Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
>> >> >       Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
>> >> >       Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
>> >> >       Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
>> >>
>> >> That adds quite a lot of crap back in that was removed by Fernando earlier:
>> >>
>> >>  44 files changed, 3733 insertions(+), 847 deletions(-)
>> >>
>> >> It may have been premature to merge the patches as you say, but now that
>> >> they are in, I'd vote for giving Fernando a chance to fix up any damage
>> >> that was done in the process rather than just reverting all the useful
>> >> changes.
>> >
>> > In looking at this further, I agree.
>> >
>> > Felipe, are all of these really needing to be reverted?  How about
>> > picking out the functional changes that need to be resolved instead of
>> > just rolling back everything that has been done here.  Surely not all of
>> > these are wrong, right?
>>
>> Patches are _NOT_ wrong, missing dependencies break the bridge.
>> Without that dependencies the first patch of the set won't work and
>> all other patches have dependency on the first one, so all of them
>> need to be reverted.
>
> How about hand-reverting only the wrong patch, so the other work isn't
> lost?  I'd much prefer that.

Unfortunately any of the iommu migration patches will work correctly
without the dependencies on iommu module patches. There are some
patches which cleanup the code, but thanks to the iommu migrations the
files can disappear complete other wise I need to check and only clean
what is not needed and leave what the old custom implementation is
using, which will need a lot of rework in the patches. According with
Felipe Contreras it is very easy reverting and pushing after. I don't
like the idea of reverting all iommu patches, however looks the
easiest solution. Unless the dependencies patches can be merged the
would be the best solution.

Regards,
Fernando.

>
> thanks,
>
> greg k-h
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Guzman Lugo, Fernando]

On Tue, Nov 9, 2010 at 11:35 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Guzman Lugo, Fernando <fernando.lugo@ti.com> [101109 08:36]:
>>
>> tidspbridge iommu change are working fine all the patches and few fixes after
>> that are alredy sent. what breaks tidspbridge, is the unmerged
>> dependencies in linux omap tree, specifically the iommu module patches
>> and the SG patch.
>
> Care to post a series of the missing patches listed above?

Here are the missing patches:

Fernando Guzman Lugo (4):
  iovmm: no gap checking for fixed address
  iovmm: add superpages support to fixed da address
  iovmm: replace __iounmap with omap_iounmap
  iommu: create new api to set valid da range

and
  scatterlist: define SG chain for arm architecture


Regards,
Fernando.



>
> That way we can at least start merging those into linux-omap for
> testing while waiting for the next merge window.
>
> Tony
>

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Greg KH]

On Tue, Nov 09, 2010 at 11:49:30AM -0600, Guzman Lugo, Fernando wrote:
> On Tue, Nov 9, 2010 at 11:25 AM, Greg KH <greg@kroah.com> wrote:
> > On Tue, Nov 09, 2010 at 11:04:18AM -0600, Guzman Lugo, Fernando wrote:
> >> On Tue, Nov 9, 2010 at 10:55 AM, Greg KH <greg@kroah.com> wrote:
> >> > On Tue, Nov 09, 2010 at 05:29:17PM +0100, Arnd Bergmann wrote:
> >> >> On Tuesday 09 November 2010, Felipe Contreras wrote:
> >> >> > Felipe Contreras (14):
> >> >> >       Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
> >> >> >       Revert "staging: tidspbridge - remove dmm custom module"
> >> >> >       Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
> >> >> >       Revert "staging: tidspbridge - remove reserved memory clean up"
> >> >> >       Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
> >> >> >       Revert "staging: tidspbridge - move all iommu related code to a new file"
> >> >> >       Revert "staging: tidspbridge - remove hw directory"
> >> >> >       Revert "staging: tidspbridge - fix mmufault support"
> >> >> >       Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
> >> >> >       Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
> >> >> >       Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
> >> >> >       Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
> >> >>
> >> >> That adds quite a lot of crap back in that was removed by Fernando earlier:
> >> >>
> >> >>  44 files changed, 3733 insertions(+), 847 deletions(-)
> >> >>
> >> >> It may have been premature to merge the patches as you say, but now that
> >> >> they are in, I'd vote for giving Fernando a chance to fix up any damage
> >> >> that was done in the process rather than just reverting all the useful
> >> >> changes.
> >> >
> >> > In looking at this further, I agree.
> >> >
> >> > Felipe, are all of these really needing to be reverted?  How about
> >> > picking out the functional changes that need to be resolved instead of
> >> > just rolling back everything that has been done here.  Surely not all of
> >> > these are wrong, right?
> >>
> >> Patches are _NOT_ wrong, missing dependencies break the bridge.
> >> Without that dependencies the first patch of the set won't work and
> >> all other patches have dependency on the first one, so all of them
> >> need to be reverted.
> >
> > How about hand-reverting only the wrong patch, so the other work isn't
> > lost?  I'd much prefer that.
> 
> Unfortunately any of the iommu migration patches will work correctly
> without the dependencies on iommu module patches. There are some
> patches which cleanup the code, but thanks to the iommu migrations the
> files can disappear complete other wise I need to check and only clean
> what is not needed and leave what the old custom implementation is
> using, which will need a lot of rework in the patches. According with
> Felipe Contreras it is very easy reverting and pushing after.

If it is easy to revert and push later, then the "revert just this
piece" should be done now.

Seriously, I'm getting very confused here, and am very annoyed by the
whole thing.

Here's what I don't like:
  - the original driver didn't even seem to work properly
  - people sent me patches they never tested and broke things even worse
  - some people have no respect for the omap maintainers and what they
    think about things, or even basic knowledge of the kernel
    development cycle.
  - I do not have this hardware so I can't test anything.

So, from now on, I'm not taking ANYONES patches for this driver unless
it gets an ack from the driver maintainer, Omar Luna.

Actually, no, I'm not going to take any patch unless it _comes from_
Omar.  Omar, please work to queue up patches and test them, and then
send them to me for merging.

Any questions?

If anyone doesn't like this because they feel that the current driver is
broken, well, I can easily solve that by just deleting the whole thing
from the tree right now.  Would that be a better idea?

Ugh, what a mess...

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Tony Lindgren]

* Guzman Lugo, Fernando <fernando.lugo@ti.com> [101109 09:43]:
> On Tue, Nov 9, 2010 at 11:35 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Guzman Lugo, Fernando <fernando.lugo@ti.com> [101109 08:36]:
> >>
> >> tidspbridge iommu change are working fine all the patches and few fixes after
> >> that are alredy sent. what breaks tidspbridge, is the unmerged
> >> dependencies in linux omap tree, specifically the iommu module patches
> >> and the SG patch.
> >
> > Care to post a series of the missing patches listed above?
> 
> Here are the missing patches:
> 
> Fernando Guzman Lugo (4):
>   iovmm: no gap checking for fixed address
>   iovmm: add superpages support to fixed da address
>   iovmm: replace __iounmap with omap_iounmap
>   iommu: create new api to set valid da range

Yeah this is stuff for Hiroshi to look at and queue for
2.6.28. No way we can merge these now.
 
> and
>   scatterlist: define SG chain for arm architecture

And this we need to test carefully it's not something we
can just merge.

Has this been tested to work with omap MMC drivers?

I'm not at all convinced the drivers can deal with
chained SG lists.. This may not show up with light
testing, the SG lists can be very small in most cases.

So this would have to be tested to make sure the
the chained SG list handled properly. The same goes
for other omap drivers that may be using SG.

Regards,

Tony

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Ramirez Luna, Omar]

Hi,

> Seriously, I'm getting very confused here, and am very annoyed by the
> whole thing.
>
> Here's what I don't like:
>  - the original driver didn't even seem to work properly
>  - people sent me patches they never tested and broke things even worse
>  - some people have no respect for the omap maintainers and what they
>    think about things, or even basic knowledge of the kernel
>    development cycle.
>  - I do not have this hardware so I can't test anything.
>
> So, from now on, I'm not taking ANYONES patches for this driver unless
> it gets an ack from the driver maintainer, Omar Luna.
>
> Actually, no, I'm not going to take any patch unless it _comes from_
> Omar.  Omar, please work to queue up patches and test them, and then
> send them to me for merging.
>
> Any questions?
>
> If anyone doesn't like this because they feel that the current driver is
> broken, well, I can easily solve that by just deleting the whole thing
> from the tree right now.  Would that be a better idea?

I'm fine to queue the patches.

For now, yes, the driver is/has-been broken, and everybody has been
trying for the last two cycles to fix some dependencies (and
unfortunately we do it at the last minute or even after it can't be
done anymore, *sometimes*)... I have been kindly educated by both LO
and staging tree maintainers that it is just a bad practice to hurry
things up and not comply with the development standards.

If 2.6.36 or .37 tidspbridge is needed I'll keep each branch with
their dependent patches in d.o-z (TI's git server), if anybody
(Felipe) wants to keep their own branches I'm fine with that I can
sync with them privately to hear any concerns or something else.

I'll create the branches and sent an announce to both lists.

All dependencies need to be acked long before 2.6.38-rc1 and it is the
responsibility of the senders to track their acceptance.

Thanks for your suggestions and bearing with this situation, I'll
follow them and solve your dislikes about it (the hardware part it
might not be possible :/).

BTW, the testing environment for tidspbridge will be gst-dsp, any
stress testsuites will be run in addition, I'll detail more of it in
the upcoming mail.

Regards,

Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Felipe Contreras]

On Tue, Nov 9, 2010 at 7:52 PM, Guzman Lugo, Fernando
<fernando.lugo@ti.com> wrote:
> On Tue, Nov 9, 2010 at 11:35 AM, Tony Lindgren <tony@atomide.com> wrote:
>> * Guzman Lugo, Fernando <fernando.lugo@ti.com> [101109 08:36]:
>>>
>>> tidspbridge iommu change are working fine all the patches and few fixes after
>>> that are alredy sent. what breaks tidspbridge, is the unmerged
>>> dependencies in linux omap tree, specifically the iommu module patches
>>> and the SG patch.
>>
>> Care to post a series of the missing patches listed above?
>
> Here are the missing patches:
>
> Fernando Guzman Lugo (4):
>  iovmm: no gap checking for fixed address
>  iovmm: add superpages support to fixed da address
>  iovmm: replace __iounmap with omap_iounmap
>  iommu: create new api to set valid da range
>
> and
>  scatterlist: define SG chain for arm architecture

Those are not the only patches needed, you would also need:
 omap: iommu: make iva2 iommu selectable
 staging: tidspbridge: enable iva2 iommu

And the one that actually uses the new API for the da range which
hasn't even been submitted to the mailing list.

Personally, I haven't seen the new iommu code working correctly.

-- 
Felipe Contreras

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Felipe Contreras]

On Tue, Nov 9, 2010 at 7:58 PM, Greg KH <greg@kroah.com> wrote:
> If it is easy to revert and push later, then the "revert just this
> piece" should be done now.
>
> Seriously, I'm getting very confused here, and am very annoyed by the
> whole thing.

With good reason.

> Here's what I don't like:
>  - the original driver didn't even seem to work properly

It _was_ working properly, when it was a separate branch, but the
merge to staging wasn't done correctly, so it has never worked there.

>  - people sent me patches they never tested and broke things even worse

Part of the blame goes to TI. Even if you have the hardware, it's not
straightforward to test this. I have struggled to improve the
situation on user-space with the gst-dsp and dsp-tools projects, which
btw have not received support from TI, but apparently they were not
used to test this (neither was the "official" solution from TI which
is much harder to set up).

>  - some people have no respect for the omap maintainers and what they
>    think about things, or even basic knowledge of the kernel
>    development cycle.

I don't think this is the case. The opinion of the OMAP maintainers is
valuable in order to cleanup the mess that is this driver. But _first_
it has to work. Most people are not developers, they just want to use
this driver.

>  - I do not have this hardware so I can't test anything.
>
> So, from now on, I'm not taking ANYONES patches for this driver unless
> it gets an ack from the driver maintainer, Omar Luna.
>
> Actually, no, I'm not going to take any patch unless it _comes from_
> Omar.  Omar, please work to queue up patches and test them, and then
> send them to me for merging.
>
> Any questions?

You mean after this pull, or should Omar re-send this pull request?

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Felipe Contreras]

On Tue, Nov 9, 2010 at 6:29 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday 09 November 2010, Felipe Contreras wrote:
>> Felipe Contreras (14):
>>       Revert "staging: tidspbridge - update Kconfig to select IOMMU module"
>>       Revert "staging: tidspbridge - remove dmm custom module"
>>       Revert "staging: tidspbridge - deprecate reserve/unreserve_memory funtions"
>>       Revert "staging: tidspbridge - remove reserved memory clean up"
>>       Revert "staging: tidspbridge: remove dw_dmmu_base from cfg_hostres struct"
>>       Revert "staging: tidspbridge - move all iommu related code to a new file"
>>       Revert "staging: tidspbridge - remove hw directory"
>>       Revert "staging: tidspbridge - fix mmufault support"
>>       Revert "staging: tidspbridge - remove custom mmu code from tiomap3430.c"
>>       Revert "staging: tidspbridge - rename bridge_brd_mem_map/unmap to a proper name"
>>       Revert "staging: tidspbridge - move shared memory iommu maps to tiomap3430.c"
>>       Revert "staging: tidspbridge: replace iommu custom for opensource implementation"
>
> That adds quite a lot of crap back in that was removed by Fernando earlier:
>
>  44 files changed, 3733 insertions(+), 847 deletions(-)
>
> It may have been premature to merge the patches as you say, but now that
> they are in, I'd vote for giving Fernando a chance to fix up any damage
> that was done in the process rather than just reverting all the useful
> changes.

The changes from Fernando require changes in other trees: arm, omap.
These cannot get into 2.6.37, and I have my doubts they would get into
2.6.38, as I haven't seen those ack'ed yet.

Moreover, I have applied all the patches Fernando has sent, and I
still haven't seen this driver working with the new iommu code.

So, sure, these patches are good, we (Nokia), requested them years
ago, but right now there's no way to get this driver working with them
on 2.6.37. I would rather have a working driver for once on a vanilla
kernel. It is really tiring to point people to different places to get
working code depending on which week they ask.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Greg KH]

On Tue, Nov 09, 2010 at 11:53:29PM +0200, Felipe Contreras wrote:
> On Tue, Nov 9, 2010 at 7:58 PM, Greg KH <greg@kroah.com> wrote:
> > If it is easy to revert and push later, then the "revert just this
> > piece" should be done now.
> >
> > Seriously, I'm getting very confused here, and am very annoyed by the
> > whole thing.
> 
> With good reason.
> 
> > Here's what I don't like:
> >  - the original driver didn't even seem to work properly
> 
> It _was_ working properly, when it was a separate branch, but the
> merge to staging wasn't done correctly, so it has never worked there.

Well, blame the people who sent it to me then :)

> >  - people sent me patches they never tested and broke things even worse
> 
> Part of the blame goes to TI. Even if you have the hardware, it's not
> straightforward to test this. I have struggled to improve the
> situation on user-space with the gst-dsp and dsp-tools projects, which
> btw have not received support from TI, but apparently they were not
> used to test this (neither was the "official" solution from TI which
> is much harder to set up).
> 
> >  - some people have no respect for the omap maintainers and what they
> >    think about things, or even basic knowledge of the kernel
> >    development cycle.
> 
> I don't think this is the case. The opinion of the OMAP maintainers is
> valuable in order to cleanup the mess that is this driver. But _first_
> it has to work. Most people are not developers, they just want to use
> this driver.
> 
> >  - I do not have this hardware so I can't test anything.
> >
> > So, from now on, I'm not taking ANYONES patches for this driver unless
> > it gets an ack from the driver maintainer, Omar Luna.
> >
> > Actually, no, I'm not going to take any patch unless it _comes from_
> > Omar.  Omar, please work to queue up patches and test them, and then
> > send them to me for merging.
> >
> > Any questions?
> 
> You mean after this pull, or should Omar re-send this pull request?

I'm not pulling _anything_ at the moment for this driver.

I want to see patches, not a pull request please, as these need to be
reviewed very closely.

thanks,

greg k-h

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Felipe Contreras]

On Tue, Nov 9, 2010 at 8:32 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Guzman Lugo, Fernando <fernando.lugo@ti.com> [101109 09:43]:
>> On Tue, Nov 9, 2010 at 11:35 AM, Tony Lindgren <tony@atomide.com> wrote:
>> > * Guzman Lugo, Fernando <fernando.lugo@ti.com> [101109 08:36]:
>> >>
>> >> tidspbridge iommu change are working fine all the patches and few fixes after
>> >> that are alredy sent. what breaks tidspbridge, is the unmerged
>> >> dependencies in linux omap tree, specifically the iommu module patches
>> >> and the SG patch.
>> >
>> > Care to post a series of the missing patches listed above?
>>
>> Here are the missing patches:
>>
>> Fernando Guzman Lugo (4):
>>   iovmm: no gap checking for fixed address
>>   iovmm: add superpages support to fixed da address
>>   iovmm: replace __iounmap with omap_iounmap
>>   iommu: create new api to set valid da range
>
> Yeah this is stuff for Hiroshi to look at and queue for
> 2.6.28. No way we can merge these now.

Right, and he hasn't ack'ed them yet. So there's a chance they don't
get into 2.6.38 either.

>> and
>>   scatterlist: define SG chain for arm architecture
>
> And this we need to test carefully it's not something we
> can just merge.
>
> Has this been tested to work with omap MMC drivers?
>
> I'm not at all convinced the drivers can deal with
> chained SG lists.. This may not show up with light
> testing, the SG lists can be very small in most cases.
>
> So this would have to be tested to make sure the
> the chained SG list handled properly. The same goes
> for other omap drivers that may be using SG.

And the change doesn't affect OMAP only, it's for all ARM platforms.

The proposal from Russell was to do it only on OMAP, but I haven't
seen a patch that does that yet. Hopefully this would go into 2.6.38,
but again, it might not.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Felipe Contreras]

On Wed, Nov 10, 2010 at 12:10 AM, Greg KH <greg@kroah.com> wrote:
> On Tue, Nov 09, 2010 at 11:53:29PM +0200, Felipe Contreras wrote:
>> You mean after this pull, or should Omar re-send this pull request?
>
> I'm not pulling _anything_ at the moment for this driver.
>
> I want to see patches, not a pull request please, as these need to be
> reviewed very closely.

It's a revert from a non-working state to a working state, there's not
much to review there. So if Omar sends the patches with my and his
s-o-b, would you apply the patches?

Getting these patches in one way or the other seems to be the only way
we can get this driver to work on 2.6.37.

-- 
Felipe Contreras

Re: [GIT PULL] fixes for tidspbridge 2.6.37-rc1

[Greg KH]

On Wed, Nov 10, 2010 at 12:39:47AM +0200, Felipe Contreras wrote:
> On Wed, Nov 10, 2010 at 12:10 AM, Greg KH <greg@kroah.com> wrote:
> > On Tue, Nov 09, 2010 at 11:53:29PM +0200, Felipe Contreras wrote:
> >> You mean after this pull, or should Omar re-send this pull request?
> >
> > I'm not pulling _anything_ at the moment for this driver.
> >
> > I want to see patches, not a pull request please, as these need to be
> > reviewed very closely.
> 
> It's a revert from a non-working state to a working state, there's not
> much to review there. So if Omar sends the patches with my and his
> s-o-b, would you apply the patches?

Was my previous email about not taking anything unless it went through
Omar not specific enough?

Re: [PATCH 2/5] omap: control: add functions for DSP boot

[Paul Walmsley]

Hello Felipe,

On Sun, 7 Nov 2010, Felipe Contreras wrote:

> From: Paul Walmsley <paul@pwsan.com>
> 
> 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.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

I see that you modified my original patch, but it seems that you also 
dropped my Signed-off-by:.  This is a tricky situation, to be sure.  
Probably the best thing to do in these cases if you don't wish to ask me 
to review the modified patch,  is to keep my original Signed-off-by: -- 
since I wrote the patch, originally -- and then, immediately before your 
Signed-off-by:, to add a bracketed note saying what you changed, e.g.,

[felipe.contreras@gmail.com: dropped some changes, etc.]

That way the provenance of the patch is clear.

regards

- Paul

Re: [PATCH 2/5] omap: control: add functions for DSP boot

[Felipe Contreras]

On Thu, Nov 11, 2010 at 2:00 AM, Paul Walmsley <paul@pwsan.com> wrote:
> Hello Felipe,
>
> On Sun, 7 Nov 2010, Felipe Contreras wrote:
>
>> From: Paul Walmsley <paul@pwsan.com>
>>
>> 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.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> I see that you modified my original patch, but it seems that you also
> dropped my Signed-off-by:.

Yes, because you didn't s-o-b my changes. I got that complaint from Catalin.

> This is a tricky situation, to be sure.
> Probably the best thing to do in these cases if you don't wish to ask me
> to review the modified patch,  is to keep my original Signed-off-by: --
> since I wrote the patch, originally -- and then, immediately before your
> Signed-off-by:, to add a bracketed note saying what you changed, e.g.,
>
> [felipe.contreras@gmail.com: dropped some changes, etc.]
>
> That way the provenance of the patch is clear.

Ok. I was hoping you would review the patch and put your s-o-b.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/5] omap: control: add functions for DSP boot

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1179 bytes --]

Hello Felipe,

On Thu, 11 Nov 2010, Felipe Contreras wrote:

> On Thu, Nov 11, 2010 at 2:00 AM, Paul Walmsley <paul@pwsan.com> wrote:
> >
> > I see that you modified my original patch, but it seems that you also
> > dropped my Signed-off-by:.
> 
> Yes, because you didn't s-o-b my changes. I got that complaint from Catalin.

I have reviewed my paul@pwsan.com inbox. I didn't see any request from you 
to review the changes.  But perhaps it was inadvertently dropped at some 
point in the mail path between you and I.

> > This is a tricky situation, to be sure.
> > Probably the best thing to do in these cases if you don't wish to ask me
> > to review the modified patch,  is to keep my original Signed-off-by: --
> > since I wrote the patch, originally -- and then, immediately before your
> > Signed-off-by:, to add a bracketed note saying what you changed, e.g.,
> >
> > [felipe.contreras@gmail.com: dropped some changes, etc.]
> >
> > That way the provenance of the patch is clear.
> 
> Ok. I was hoping you would review the patch and put your s-o-b.

I have reviewed your updated patch; you are welcome to re-add my 
Signed-off-by:.


- Paul

Re: [PATCH 0/5] omap: dsp: fixes for 2.6.37-rc1

[Paul Walmsley]

Hello Felipe

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?

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.

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.

regards

- Paul

Re: [PATCH 0/5] omap: dsp: fixes for 2.6.37-rc1

[Felipe Contreras]

Hi,

On Wed, Dec 15, 2010 at 8:00 AM, Paul Walmsley <paul@pwsan.com> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html