[PATCH v2 0/7] MIPS: Miscellaneous fixes related to Android Mips emulator

[PATCH v2 0/7] MIPS: Miscellaneous fixes related to Android Mips emulator

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

v1->v2:

    - the patch on PREF usage in memcpy dropped as not needed
    - updated recipient lists using get_maintainer.pl
    - rebased to the latest kernel code

This series contains an assortment of changes necessary for proper
operation of Android emulator for Mips. However, we think that wider
kernel community may benefit from them too.

Aleksandar Markovic (1):
  MIPS: math-emu: Handle zero accumulator case in MADDF and MSUBF
    separately

Lingfeng Yang (1):
  input: goldfish: Fix multitouch event handling

Miodrag Dinic (5):
  MIPS: cmdline: Add support for 'memmap' parameter
  MIPS: build: Fix "-modd-spreg" switch usage when compiling for
    mips32r6
  MIPS: unaligned: Add DSP lwx & lhx missaligned access support
  tty: goldfish: Use streaming DMA for r/w operations on Ranchu
    platforms
  tty: goldfish: Implement support for kernel 'earlycon' parameter

 arch/mips/Makefile                       |   2 +-
 arch/mips/include/uapi/asm/inst.h        |  11 ++
 arch/mips/kernel/setup.c                 |  40 +++++++
 arch/mips/kernel/unaligned.c             | 174 ++++++++++++++++++-------------
 arch/mips/math-emu/dp_maddf.c            |   5 +-
 arch/mips/math-emu/sp_maddf.c            |   5 +-
 drivers/input/keyboard/goldfish_events.c |  33 +++++-
 drivers/tty/Kconfig                      |   3 +
 drivers/tty/goldfish.c                   | 145 ++++++++++++++++++++++++--
 9 files changed, 329 insertions(+), 89 deletions(-)

-- 
2.7.4

[PATCH v2 1/7] MIPS: cmdline: Add support for 'memmap' parameter

[Aleksandar Markovic]

From: Miodrag Dinic <miodrag.dinic@imgtec.com>

Implement support for parsing 'memmap' kernel command line parameter.

This patch covers parsing of the following two formats for 'memmap'
parameter values:

  - nn[KMG]@ss[KMG]
  - nn[KMG]$ss[KMG]

  ([KMG] = K M or G (kilo, mega, giga))

These two allowed formats for parameter value are already documented
in file kernel-parameters.txt in Documentation/admin-guide folder.
Some architectures already support them, but Mips did not prior to
this patch.

Excerpt from Documentation/admin-guide/kernel-parameters.txt:

memmap=nn[KMG]@ss[KMG]
    [KNL] Force usage of a specific region of memory.
    Region of memory to be used is from ss to ss+nn.

memmap=nn[KMG]$ss[KMG]
    Mark specific memory as reserved.
    Region of memory to be reserved is from ss to ss+nn.
    Example: Exclude memory from 0x18690000-0x1869ffff
        memmap=64K$0x18690000
        or
        memmap=0x10000$0x18690000

There is no need to update this documentation file with respect to
this patch.

Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
 arch/mips/kernel/setup.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index ccea90f..5a86da93 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -713,6 +713,46 @@ static int __init early_parse_mem(char *p)
 }
 early_param("mem", early_parse_mem);
 
+static int __init early_parse_memmap(char *p)
+{
+	char *oldp;
+	u64 start_at, mem_size;
+
+	if (!p)
+		return -EINVAL;
+
+	if (!strncmp(p, "exactmap", 8)) {
+		pr_err("\"memmap=exactmap\" invalid on MIPS\n");
+		return 0;
+	}
+
+	oldp = p;
+	mem_size = memparse(p, &p);
+	if (p == oldp)
+		return -EINVAL;
+
+	if (*p == '@') {
+		start_at = memparse(p+1, &p);
+		add_memory_region(start_at, mem_size, BOOT_MEM_RAM);
+	} else if (*p == '#') {
+		pr_err("\"memmap=nn#ss\" (force ACPI data) invalid on MIPS\n");
+		return -EINVAL;
+	} else if (*p == '$') {
+		start_at = memparse(p+1, &p);
+		add_memory_region(start_at, mem_size, BOOT_MEM_RESERVED);
+	} else {
+		pr_err("\"memmap\" invalid format!\n");
+		return -EINVAL;
+	}
+
+	if (*p == '\0') {
+		usermem = 1;
+		return 0;
+	} else
+		return -EINVAL;
+}
+early_param("memmap", early_parse_memmap);
+
 #ifdef CONFIG_PROC_VMCORE
 unsigned long setup_elfcorehdr, setup_elfcorehdr_size;
 static int __init early_parse_elfcorehdr(char *p)
-- 
2.7.4

[PATCH v2 2/7] MIPS: build: Fix "-modd-spreg" switch usage when compiling for mips32r6

[Aleksandar Markovic]

From: Miodrag Dinic <miodrag.dinic@imgtec.com>

Add "-modd-spreg" when compiling the kernel for mips32r6 target.

This makes sure the kernel builds properly even with toolchains that
use "-mno-odd-spreg" by default. This is the case with Android gcc.
Prior to this patch, kernel builds using gcc for Android failed with
following error messages, if target architecture is set to mips32r6:

arch/mips/kernel/r4k_switch.S: Assembler messages:
.../r4k_switch.S:210: Error: float register should be even, was 1
.../r4k_switch.S:212: Error: float register should be even, was 3
.../r4k_switch.S:214: Error: float register should be even, was 5
.../r4k_switch.S:216: Error: float register should be even, was 7
.../r4k_switch.S:218: Error: float register should be even, was 9
.../r4k_switch.S:220: Error: float register should be even, was 11
.../r4k_switch.S:222: Error: float register should be even, was 13
.../r4k_switch.S:224: Error: float register should be even, was 15
.../r4k_switch.S:226: Error: float register should be even, was 17
.../r4k_switch.S:228: Error: float register should be even, was 19
.../r4k_switch.S:230: Error: float register should be even, was 21
.../r4k_switch.S:232: Error: float register should be even, was 23
.../r4k_switch.S:234: Error: float register should be even, was 25
.../r4k_switch.S:236: Error: float register should be even, was 27
.../r4k_switch.S:238: Error: float register should be even, was 29
.../r4k_switch.S:240: Error: float register should be even, was 31
make[2]: *** [arch/mips/kernel/r4k_switch.o] Error 1

Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
 arch/mips/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index bc96c39..1c0ec36 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -160,7 +160,7 @@ cflags-$(CONFIG_CPU_MIPS32_R1)	+= $(call cc-option,-march=mips32,-mips32 -U_MIPS
 			-Wa,-mips32 -Wa,--trap
 cflags-$(CONFIG_CPU_MIPS32_R2)	+= $(call cc-option,-march=mips32r2,-mips32r2 -U_MIPS_ISA -D_MIPS_ISA=_MIPS_ISA_MIPS32) \
 			-Wa,-mips32r2 -Wa,--trap
-cflags-$(CONFIG_CPU_MIPS32_R6)	+= -march=mips32r6 -Wa,--trap
+cflags-$(CONFIG_CPU_MIPS32_R6)	+= -march=mips32r6 -Wa,--trap -modd-spreg
 cflags-$(CONFIG_CPU_MIPS64_R1)	+= $(call cc-option,-march=mips64,-mips64 -U_MIPS_ISA -D_MIPS_ISA=_MIPS_ISA_MIPS64) \
 			-Wa,-mips64 -Wa,--trap
 cflags-$(CONFIG_CPU_MIPS64_R2)	+= $(call cc-option,-march=mips64r2,-mips64r2 -U_MIPS_ISA -D_MIPS_ISA=_MIPS_ISA_MIPS64) \
-- 
2.7.4

[PATCH v2 3/7] MIPS: unaligned: Add DSP lwx & lhx missaligned access support

[Aleksandar Markovic]

From: Miodrag Dinic <miodrag.dinic@imgtec.com>

Add handling of missaligned access for DSP load instructions
lwx & lhx.

Since DSP instructions share SPECIAL3 opcode with other non-DSP
instructions, necessary logic was inserted for distinguishing
between instructions with SPECIAL3 opcode. For that purpose,
the instruction format for DSP instructions is added to
arch/mips/include/uapi/asm/inst.h.

Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtech.com>
---
 arch/mips/include/uapi/asm/inst.h |  11 +++
 arch/mips/kernel/unaligned.c      | 174 ++++++++++++++++++++++----------------
 2 files changed, 111 insertions(+), 74 deletions(-)

diff --git a/arch/mips/include/uapi/asm/inst.h b/arch/mips/include/uapi/asm/inst.h
index b5e46ae..302ad9a 100644
--- a/arch/mips/include/uapi/asm/inst.h
+++ b/arch/mips/include/uapi/asm/inst.h
@@ -755,6 +755,16 @@ struct msa_mi10_format {		/* MSA MI10 */
 	;))))))
 };
 
+struct dsp_format {		/* SPEC3 DSP format instructions */
+	__BITFIELD_FIELD(unsigned int opcode : 6,
+	__BITFIELD_FIELD(unsigned int base : 5,
+	__BITFIELD_FIELD(unsigned int index : 5,
+	__BITFIELD_FIELD(unsigned int rd : 5,
+	__BITFIELD_FIELD(unsigned int op : 5,
+	__BITFIELD_FIELD(unsigned int func : 6,
+	;))))))
+};
+
 struct spec3_format {   /* SPEC3 */
 	__BITFIELD_FIELD(unsigned int opcode:6,
 	__BITFIELD_FIELD(unsigned int rs:5,
@@ -1046,6 +1056,7 @@ union mips_instruction {
 	struct b_format b_format;
 	struct ps_format ps_format;
 	struct v_format v_format;
+	struct dsp_format dsp_format;
 	struct spec3_format spec3_format;
 	struct fb_format fb_format;
 	struct fp0_format fp0_format;
diff --git a/arch/mips/kernel/unaligned.c b/arch/mips/kernel/unaligned.c
index f806ee5..67946bb 100644
--- a/arch/mips/kernel/unaligned.c
+++ b/arch/mips/kernel/unaligned.c
@@ -939,88 +939,114 @@ static void emulate_load_store_insn(struct pt_regs *regs,
 		 * The remaining opcodes are the ones that are really of
 		 * interest.
 		 */
-#ifdef CONFIG_EVA
 	case spec3_op:
-		/*
-		 * we can land here only from kernel accessing user memory,
-		 * so we need to "switch" the address limit to user space, so
-		 * address check can work properly.
-		 */
-		seg = get_fs();
-		set_fs(USER_DS);
-		switch (insn.spec3_format.func) {
-		case lhe_op:
-			if (!access_ok(VERIFY_READ, addr, 2)) {
-				set_fs(seg);
-				goto sigbus;
-			}
-			LoadHWE(addr, value, res);
-			if (res) {
-				set_fs(seg);
-				goto fault;
-			}
-			compute_return_epc(regs);
-			regs->regs[insn.spec3_format.rt] = value;
-			break;
-		case lwe_op:
-			if (!access_ok(VERIFY_READ, addr, 4)) {
-				set_fs(seg);
-				goto sigbus;
+		if (insn.dsp_format.func == lx_op) {
+			switch (insn.dsp_format.op) {
+			case lwx_op:
+				if (!access_ok(VERIFY_READ, addr, 4))
+					goto sigbus;
+				LoadW(addr, value, res);
+				if (res)
+					goto fault;
+				compute_return_epc(regs);
+				regs->regs[insn.dsp_format.rd] = value;
+				break;
+			case lhx_op:
+				if (!access_ok(VERIFY_READ, addr, 2))
+					goto sigbus;
+				LoadHW(addr, value, res);
+				if (res)
+					goto fault;
+				compute_return_epc(regs);
+				regs->regs[insn.dsp_format.rd] = value;
+				break;
+			default:
+				goto sigill;
 			}
+		}
+#ifdef CONFIG_EVA
+		else {
+			/*
+			 * we can land here only from kernel accessing user
+			 * memory, so we need to "switch" the address limit to
+			 * user space, so that address check can work properly.
+			 */
+			seg = get_fs();
+			set_fs(USER_DS);
+			switch (insn.spec3_format.func) {
+			case lhe_op:
+				if (!access_ok(VERIFY_READ, addr, 2)) {
+					set_fs(seg);
+					goto sigbus;
+				}
+				LoadHWE(addr, value, res);
+				if (res) {
+					set_fs(seg);
+					goto fault;
+				}
+				compute_return_epc(regs);
+				regs->regs[insn.spec3_format.rt] = value;
+				break;
+			case lwe_op:
+				if (!access_ok(VERIFY_READ, addr, 4)) {
+					set_fs(seg);
+					goto sigbus;
+				}
 				LoadWE(addr, value, res);
-			if (res) {
-				set_fs(seg);
-				goto fault;
-			}
-			compute_return_epc(regs);
-			regs->regs[insn.spec3_format.rt] = value;
-			break;
-		case lhue_op:
-			if (!access_ok(VERIFY_READ, addr, 2)) {
-				set_fs(seg);
-				goto sigbus;
-			}
-			LoadHWUE(addr, value, res);
-			if (res) {
-				set_fs(seg);
-				goto fault;
-			}
-			compute_return_epc(regs);
-			regs->regs[insn.spec3_format.rt] = value;
-			break;
-		case she_op:
-			if (!access_ok(VERIFY_WRITE, addr, 2)) {
-				set_fs(seg);
-				goto sigbus;
-			}
-			compute_return_epc(regs);
-			value = regs->regs[insn.spec3_format.rt];
-			StoreHWE(addr, value, res);
-			if (res) {
-				set_fs(seg);
-				goto fault;
-			}
-			break;
-		case swe_op:
-			if (!access_ok(VERIFY_WRITE, addr, 4)) {
-				set_fs(seg);
-				goto sigbus;
-			}
-			compute_return_epc(regs);
-			value = regs->regs[insn.spec3_format.rt];
-			StoreWE(addr, value, res);
-			if (res) {
+				if (res) {
+					set_fs(seg);
+					goto fault;
+				}
+				compute_return_epc(regs);
+				regs->regs[insn.spec3_format.rt] = value;
+				break;
+			case lhue_op:
+				if (!access_ok(VERIFY_READ, addr, 2)) {
+					set_fs(seg);
+					goto sigbus;
+				}
+				LoadHWUE(addr, value, res);
+				if (res) {
+					set_fs(seg);
+					goto fault;
+				}
+				compute_return_epc(regs);
+				regs->regs[insn.spec3_format.rt] = value;
+				break;
+			case she_op:
+				if (!access_ok(VERIFY_WRITE, addr, 2)) {
+					set_fs(seg);
+					goto sigbus;
+				}
+				compute_return_epc(regs);
+				value = regs->regs[insn.spec3_format.rt];
+				StoreHWE(addr, value, res);
+				if (res) {
+					set_fs(seg);
+					goto fault;
+				}
+				break;
+			case swe_op:
+				if (!access_ok(VERIFY_WRITE, addr, 4)) {
+					set_fs(seg);
+					goto sigbus;
+				}
+				compute_return_epc(regs);
+				value = regs->regs[insn.spec3_format.rt];
+				StoreWE(addr, value, res);
+				if (res) {
+					set_fs(seg);
+					goto fault;
+				}
+				break;
+			default:
 				set_fs(seg);
-				goto fault;
+				goto sigill;
 			}
-			break;
-		default:
 			set_fs(seg);
-			goto sigill;
 		}
-		set_fs(seg);
-		break;
 #endif
+		break;
 	case lh_op:
 		if (!access_ok(VERIFY_READ, addr, 2))
 			goto sigbus;
-- 
2.7.4

[PATCH v2 4/7] MIPS: math-emu: Handle zero accumulator case in MADDF and MSUBF separately

[Aleksandar Markovic]

From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

If accumulator value is zero, just return the value of previously
calculated product. This brings logic in MADDf/MSUBF implementation
closer to the logic in ADD/SUB case.

Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
 arch/mips/math-emu/dp_maddf.c | 5 ++++-
 arch/mips/math-emu/sp_maddf.c | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/mips/math-emu/dp_maddf.c b/arch/mips/math-emu/dp_maddf.c
index 4a2d03c..caa62f2 100644
--- a/arch/mips/math-emu/dp_maddf.c
+++ b/arch/mips/math-emu/dp_maddf.c
@@ -54,7 +54,7 @@ static union ieee754dp _dp_maddf(union ieee754dp z, union ieee754dp x,
 		return ieee754dp_nanxcpt(z);
 	case IEEE754_CLASS_DNORM:
 		DPDNORMZ;
-	/* QNAN is handled separately below */
+	/* QNAN and ZERO cases are handled separately below */
 	}
 
 	switch (CLPAIR(xc, yc)) {
@@ -210,6 +210,9 @@ static union ieee754dp _dp_maddf(union ieee754dp z, union ieee754dp x,
 	}
 	assert(rm & (DP_HIDDEN_BIT << 3));
 
+	if (zc == IEEE754_CLASS_ZERO)
+		return ieee754dp_format(rs, re, rm);
+
 	/* And now the addition */
 	assert(zm & DP_HIDDEN_BIT);
 
diff --git a/arch/mips/math-emu/sp_maddf.c b/arch/mips/math-emu/sp_maddf.c
index a8cd8b4..c91d5e5 100644
--- a/arch/mips/math-emu/sp_maddf.c
+++ b/arch/mips/math-emu/sp_maddf.c
@@ -54,7 +54,7 @@ static union ieee754sp _sp_maddf(union ieee754sp z, union ieee754sp x,
 		return ieee754sp_nanxcpt(z);
 	case IEEE754_CLASS_DNORM:
 		SPDNORMZ;
-	/* QNAN is handled separately below */
+	/* QNAN and ZERO cases are handled separately below */
 	}
 
 	switch (CLPAIR(xc, yc)) {
@@ -203,6 +203,9 @@ static union ieee754sp _sp_maddf(union ieee754sp z, union ieee754sp x,
 	}
 	assert(rm & (SP_HIDDEN_BIT << 3));
 
+	if (zc == IEEE754_CLASS_ZERO)
+		return ieee754sp_format(rs, re, rm);
+
 	/* And now the addition */
 
 	assert(zm & SP_HIDDEN_BIT);
-- 
2.7.4

[PATCH v2 5/7] input: goldfish: Fix multitouch event handling

[Aleksandar Markovic]

From: Lingfeng Yang <lfy@google.com>

Register Goldfish Events device properly as a multitouch device,
and send SYN_REPORT event in appropriate cases only.

If SYN_REPORT is sent on every single multitouch event, it breaks
the multitouch. The multitouch becomes janky and having to click
2-3 times to do stuff (plus randomly activating notification bars
when not clicking). If these SYN_REPORT events are supressed,
multitouch will work fine, plus the events will have a protocol
that looks nice.

In addition, Goldfish Events device needs to be registerd as a
multitouch device by issuing input_mt_init_slots. Otherwise,
input_handle_abs_event in drivers/input/input.c will silently drop
all ABS_MT_SLOT events, casusing touches with more than one finger
not to work properly.

Signed-off-by: Lingfeng Yang <lfy@google.com>
Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
 drivers/input/keyboard/goldfish_events.c | 33 +++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/goldfish_events.c b/drivers/input/keyboard/goldfish_events.c
index f6e643b..6e0b8bb 100644
--- a/drivers/input/keyboard/goldfish_events.c
+++ b/drivers/input/keyboard/goldfish_events.c
@@ -17,6 +17,7 @@
 #include <linux/interrupt.h>
 #include <linux/types.h>
 #include <linux/input.h>
+#include <linux/input/mt.h>
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -24,6 +25,8 @@
 #include <linux/io.h>
 #include <linux/acpi.h>
 
+#define GOLDFISH_MAX_FINGERS 5
+
 enum {
 	REG_READ        = 0x00,
 	REG_SET_PAGE    = 0x00,
@@ -52,7 +55,22 @@ static irqreturn_t events_interrupt(int irq, void *dev_id)
 	value = __raw_readl(edev->addr + REG_READ);
 
 	input_event(edev->input, type, code, value);
-	input_sync(edev->input);
+
+	/*
+	 * Send an extra (EV_SYN, SYN_REPORT, 0x0) event if a key
+	 * was pressed. Some keyboard device drivers may only send
+	 * the EV_KEY event and not EV_SYN.
+	 *
+	 * Note that sending an extra SYN_REPORT is not necessary
+	 * nor correct protocol with other devices such as
+	 * touchscreens, which will send their own SYN_REPORT's
+	 * when sufficient event information has been collected
+	 * (e.g., for touchscreens, when pressure and X/Y coordinates
+	 * have been received). Hence, we will only send this extra
+	 * SYN_REPORT if type == EV_KEY.
+	 */
+	if (type == EV_KEY)
+		input_sync(edev->input);
 	return IRQ_HANDLED;
 }
 
@@ -155,6 +173,19 @@ static int events_probe(struct platform_device *pdev)
 	input_dev->name = edev->name;
 	input_dev->id.bustype = BUS_HOST;
 
+	/*
+	 * Set the Goldfish Device to be multitouch.
+	 *
+	 * In the Ranchu kernel, there is multitouch-specific code
+	 * for handling ABS_MT_SLOT events (see
+	 * drivers/input/input.c:input_handle_abs_event).
+	 * If we do not issue input_mt_init_slots, the kernel will
+	 * filter out needed ABS_MT_SLOT events when we touch the
+	 * screen in more than one place, preventing multitouch with
+	 * more than one finger from working.
+	 */
+	input_mt_init_slots(input_dev, GOLDFISH_MAX_FINGERS, 0);
+
 	events_import_bits(edev, input_dev->evbit, EV_SYN, EV_MAX);
 	events_import_bits(edev, input_dev->keybit, EV_KEY, KEY_MAX);
 	events_import_bits(edev, input_dev->relbit, EV_REL, REL_MAX);
-- 
2.7.4

[PATCH v2 6/7] tty: goldfish: Use streaming DMA for r/w operations on Ranchu platforms

[Aleksandar Markovic]

From: Miodrag Dinic <miodrag.dinic@imgtec.com>

Implement tty r/w operations using streaming DMA.

Goldfish tty for Ranchu platforms has been modified to use
streaming DMA mappings for read/write operations. This change
eliminates the need for snooping through the TLB in QEMU using
cpu_get_phys_page_debug() which does not guarantee that it will
return the valid va -> pa mapping.

The streaming DMA mapping is implemented using dma_map_single() per
transfer, while dma_unmap_single() is used for unmapping right after
the DMA transfer.

Using DMA API is the proper way for handling r/w transfers and
makes this driver more portable, thus effectively eliminating
the need for virt_to_page() and page_to_phys() conversions.

This change does not affect the old style Goldfish tty behaviour
which is still used by the Goldfish emulator. Version register has
been added and probed to see which platform is running this driver.
Reading from the new GOLDFISH_TTY_VERSION register using the Goldfish
emulator will return 0 and driver will work with virtual addresses.
Whereas if run on Ranchu it returns 1, and thus DMA is used.

Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtech.com>
---
 drivers/tty/goldfish.c | 119 ++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 108 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/goldfish.c b/drivers/tty/goldfish.c
index 996bd47..acd50fa 100644
--- a/drivers/tty/goldfish.c
+++ b/drivers/tty/goldfish.c
@@ -22,6 +22,8 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/goldfish.h>
+#include <linux/mm.h>
+#include <linux/dma-mapping.h>
 
 enum {
 	GOLDFISH_TTY_PUT_CHAR       = 0x00,
@@ -32,6 +34,8 @@ enum {
 	GOLDFISH_TTY_DATA_LEN       = 0x14,
 	GOLDFISH_TTY_DATA_PTR_HIGH  = 0x18,
 
+	GOLDFISH_TTY_VERSION		= 0x20,
+
 	GOLDFISH_TTY_CMD_INT_DISABLE    = 0,
 	GOLDFISH_TTY_CMD_INT_ENABLE     = 1,
 	GOLDFISH_TTY_CMD_WRITE_BUFFER   = 2,
@@ -45,6 +49,8 @@ struct goldfish_tty {
 	u32 irq;
 	int opencount;
 	struct console console;
+	u32 version;
+	struct device *dev;
 };
 
 static DEFINE_MUTEX(goldfish_tty_lock);
@@ -53,24 +59,94 @@ static u32 goldfish_tty_line_count = 8;
 static u32 goldfish_tty_current_line_count;
 static struct goldfish_tty *goldfish_ttys;
 
-static void goldfish_tty_do_write(int line, const char *buf, unsigned count)
+static inline void do_rw_io(struct goldfish_tty *qtty,
+				unsigned long address,
+				unsigned int count,
+				int is_write)
 {
 	unsigned long irq_flags;
-	struct goldfish_tty *qtty = &goldfish_ttys[line];
 	void __iomem *base = qtty->base;
+
 	spin_lock_irqsave(&qtty->lock, irq_flags);
-	gf_write_ptr(buf, base + GOLDFISH_TTY_DATA_PTR,
+	gf_write_ptr((void *)address, base + GOLDFISH_TTY_DATA_PTR,
 				base + GOLDFISH_TTY_DATA_PTR_HIGH);
 	writel(count, base + GOLDFISH_TTY_DATA_LEN);
-	writel(GOLDFISH_TTY_CMD_WRITE_BUFFER, base + GOLDFISH_TTY_CMD);
+
+	if (is_write)
+		writel(GOLDFISH_TTY_CMD_WRITE_BUFFER, base + GOLDFISH_TTY_CMD);
+	else
+		writel(GOLDFISH_TTY_CMD_READ_BUFFER, base + GOLDFISH_TTY_CMD);
+
 	spin_unlock_irqrestore(&qtty->lock, irq_flags);
 }
 
+static inline void goldfish_tty_rw(struct goldfish_tty *qtty,
+				unsigned long addr,
+				unsigned int count,
+				int is_write)
+{
+	dma_addr_t dma_handle;
+	enum dma_data_direction dma_dir;
+
+	dma_dir = (is_write ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
+	if (qtty->version) {
+		/*
+		 * Goldfish TTY for Ranchu platform uses
+		 * physical addresses and DMA for read/write operations
+		 */
+		unsigned long addr_end = addr + count;
+
+		while (addr < addr_end) {
+			unsigned long pg_end = (addr & PAGE_MASK) + PAGE_SIZE;
+			unsigned long next =
+					pg_end < addr_end ? pg_end : addr_end;
+			unsigned long avail = next - addr;
+
+			/*
+			 * Map the buffer's virtual address to the DMA address
+			 * so the buffer can be accessed by the device.
+			 */
+			dma_handle = dma_map_single(qtty->dev,
+						(void *)addr, avail, dma_dir);
+
+			if (dma_mapping_error(qtty->dev, dma_handle)) {
+				dev_err(qtty->dev, "tty: DMA mapping error.\n");
+				return;
+			}
+			do_rw_io(qtty, dma_handle, avail, is_write);
+
+			/*
+			 * Unmap the previously mapped region after
+			 * the completion of the read/write operation.
+			 */
+			dma_unmap_single(qtty->dev, dma_handle, avail, dma_dir);
+
+			addr += avail;
+		}
+	} else {
+		/*
+		 * Old style Goldfish TTY used on the Goldfish platform
+		 * uses virtual addresses.
+		 */
+		do_rw_io(qtty, addr, count, is_write);
+	}
+
+}
+
+static void goldfish_tty_do_write(int line, const char *buf,
+				unsigned int count)
+{
+	struct goldfish_tty *qtty = &goldfish_ttys[line];
+	unsigned long address = (unsigned long)(void *)buf;
+
+	goldfish_tty_rw(qtty, address, count, 1);
+}
+
 static irqreturn_t goldfish_tty_interrupt(int irq, void *dev_id)
 {
 	struct goldfish_tty *qtty = dev_id;
 	void __iomem *base = qtty->base;
-	unsigned long irq_flags;
+	unsigned long address;
 	unsigned char *buf;
 	u32 count;
 
@@ -79,12 +155,10 @@ static irqreturn_t goldfish_tty_interrupt(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	count = tty_prepare_flip_string(&qtty->port, &buf, count);
-	spin_lock_irqsave(&qtty->lock, irq_flags);
-	gf_write_ptr(buf, base + GOLDFISH_TTY_DATA_PTR,
-				base + GOLDFISH_TTY_DATA_PTR_HIGH);
-	writel(count, base + GOLDFISH_TTY_DATA_LEN);
-	writel(GOLDFISH_TTY_CMD_READ_BUFFER, base + GOLDFISH_TTY_CMD);
-	spin_unlock_irqrestore(&qtty->lock, irq_flags);
+
+	address = (unsigned long)(void *)buf;
+	goldfish_tty_rw(qtty, address, count, 0);
+
 	tty_schedule_flip(&qtty->port);
 	return IRQ_HANDLED;
 }
@@ -271,6 +345,29 @@ static int goldfish_tty_probe(struct platform_device *pdev)
 	qtty->port.ops = &goldfish_port_ops;
 	qtty->base = base;
 	qtty->irq = irq;
+	qtty->dev = &pdev->dev;
+
+	/* Goldfish TTY device used by the Goldfish emulator
+	 * should identify itself with 0, forcing the driver
+	 * to use virtual addresses. Goldfish TTY device
+	 * on Ranchu emulator (qemu2) returns 1 here and
+	 * driver will use physical addresses.
+	 */
+	qtty->version = readl(base + GOLDFISH_TTY_VERSION);
+
+	/* Goldfish TTY device on Ranchu emulator (qemu2)
+	 * will use DMA for read/write IO operations.
+	 */
+	if (qtty->version > 0) {
+		/* Initialize dma_mask to 32-bits.
+		 */
+		if (!pdev->dev.dma_mask)
+			pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
+		if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(32))) {
+			dev_err(&pdev->dev, "No suitable DMA available.\n");
+			goto err_create_driver_failed;
+		}
+	}
 
 	writel(GOLDFISH_TTY_CMD_INT_DISABLE, base + GOLDFISH_TTY_CMD);
 
-- 
2.7.4

[PATCH v2 7/7] tty: goldfish: Implement support for kernel 'earlycon' parameter

[Aleksandar Markovic]

From: Miodrag Dinic <miodrag.dinic@imgtec.com>

Add early console functionality to the Goldfish tty driver.

When 'earlycon' kernel command line parameter is used with no options,
the early console is determined by the 'stdout-path' property in device
tree's 'chosen' node. This is illustrated in the following device tree
source example:

Device tree example:

    chosen {
        stdout-path = "/goldfish_tty@1f004000";
    };

    goldfish_tty@1f004000 {
        interrupts = <0xc>;
        reg = <0x1f004000 0x0 0x1000>;
        compatible = "google,goldfish-tty", "generic,goldfish-tty";
    };

Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
 drivers/tty/Kconfig    |  3 +++
 drivers/tty/goldfish.c | 26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/drivers/tty/Kconfig b/drivers/tty/Kconfig
index 9510305..873e0ba 100644
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -392,6 +392,9 @@ config PPC_EARLY_DEBUG_EHV_BC_HANDLE
 config GOLDFISH_TTY
 	tristate "Goldfish TTY Driver"
 	depends on GOLDFISH
+	select SERIAL_CORE
+	select SERIAL_CORE_CONSOLE
+	select SERIAL_EARLYCON
 	help
 	  Console and system TTY driver for the Goldfish virtual platform.
 
diff --git a/drivers/tty/goldfish.c b/drivers/tty/goldfish.c
index acd50fa..22b7ad5 100644
--- a/drivers/tty/goldfish.c
+++ b/drivers/tty/goldfish.c
@@ -1,6 +1,7 @@
 /*
  * Copyright (C) 2007 Google, Inc.
  * Copyright (C) 2012 Intel, Inc.
+ * Copyright (C) 2017 Imagination Technologies Ltd.
  *
  * This software is licensed under the terms of the GNU General Public
  * License version 2, as published by the Free Software Foundation, and
@@ -24,6 +25,7 @@
 #include <linux/goldfish.h>
 #include <linux/mm.h>
 #include <linux/dma-mapping.h>
+#include <linux/serial_core.h>
 
 enum {
 	GOLDFISH_TTY_PUT_CHAR       = 0x00,
@@ -427,6 +429,30 @@ static int goldfish_tty_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static void gf_early_console_putchar(struct uart_port *port, int ch)
+{
+	__raw_writel(ch, port->membase);
+}
+
+static void gf_early_write(struct console *con, const char *s, unsigned int n)
+{
+	struct earlycon_device *dev = con->data;
+
+	uart_console_write(&dev->port, s, n, gf_early_console_putchar);
+}
+
+static int __init gf_earlycon_setup(struct earlycon_device *device,
+					const char *opt)
+{
+	if (!device->port.membase)
+		return -ENODEV;
+
+	device->con->write = gf_early_write;
+	return 0;
+}
+
+OF_EARLYCON_DECLARE(early_gf_tty, "google,goldfish-tty", gf_earlycon_setup);
+
 static const struct of_device_id goldfish_tty_of_match[] = {
 	{ .compatible = "google,goldfish-tty", },
 	{},
-- 
2.7.4

Re: [PATCH v2 0/7] MIPS: Miscellaneous fixes related to Android Mips emulator

[Greg Kroah-Hartman]

On Wed, Jun 28, 2017 at 05:56:24PM +0200, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
> 
> v1->v2:
> 
>     - the patch on PREF usage in memcpy dropped as not needed
>     - updated recipient lists using get_maintainer.pl
>     - rebased to the latest kernel code
> 
> This series contains an assortment of changes necessary for proper
> operation of Android emulator for Mips. However, we think that wider
> kernel community may benefit from them too.

This is nice, thanks for these.

How well does these patches "work" with the recent goldfish
images/kernels that are out there?  I know the goldfish platform has
been revamped a lot recently, and I would not like to see these changes
cause things to break there :)

Also, any chance to get some google reviewers for these changes?  I
don't think you added any to the cc: list, how come?

thanks,

greg k-h

Re: [PATCH v2 5/7] input: goldfish: Fix multitouch event handling

[Dmitry Torokhov]

On Wed, Jun 28, 2017 at 05:56:29PM +0200, Aleksandar Markovic wrote:
> From: Lingfeng Yang <lfy@google.com>
> 
> Register Goldfish Events device properly as a multitouch device,
> and send SYN_REPORT event in appropriate cases only.
> 
> If SYN_REPORT is sent on every single multitouch event, it breaks
> the multitouch. The multitouch becomes janky and having to click
> 2-3 times to do stuff (plus randomly activating notification bars
> when not clicking).

This sounds like a deficiency in protocol handling in userspace. Given
that input core can suppress duplicate events userpsace mught very well
only see one ABS_X followed by SYN_REPORT if Y coordinate did not change
or was suppressed by jitter detection.

> If these SYN_REPORT events are supressed,
> multitouch will work fine, plus the events will have a protocol
> that looks nice.
> 
> In addition, Goldfish Events device needs to be registerd as a
> multitouch device by issuing input_mt_init_slots. Otherwise,
> input_handle_abs_event in drivers/input/input.c will silently drop
> all ABS_MT_SLOT events, casusing touches with more than one finger
> not to work properly.
> 
> Signed-off-by: Lingfeng Yang <lfy@google.com>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
> Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
> ---
>  drivers/input/keyboard/goldfish_events.c | 33 +++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/keyboard/goldfish_events.c b/drivers/input/keyboard/goldfish_events.c
> index f6e643b..6e0b8bb 100644
> --- a/drivers/input/keyboard/goldfish_events.c
> +++ b/drivers/input/keyboard/goldfish_events.c
> @@ -17,6 +17,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/types.h>
>  #include <linux/input.h>
> +#include <linux/input/mt.h>
>  #include <linux/kernel.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> @@ -24,6 +25,8 @@
>  #include <linux/io.h>
>  #include <linux/acpi.h>
>  
> +#define GOLDFISH_MAX_FINGERS 5
> +
>  enum {
>  	REG_READ        = 0x00,
>  	REG_SET_PAGE    = 0x00,
> @@ -52,7 +55,22 @@ static irqreturn_t events_interrupt(int irq, void *dev_id)
>  	value = __raw_readl(edev->addr + REG_READ);
>  
>  	input_event(edev->input, type, code, value);
> -	input_sync(edev->input);
> +
> +	/*
> +	 * Send an extra (EV_SYN, SYN_REPORT, 0x0) event if a key
> +	 * was pressed. Some keyboard device drivers may only send
> +	 * the EV_KEY event and not EV_SYN.

Can they be fixed?

> +	 *
> +	 * Note that sending an extra SYN_REPORT is not necessary
> +	 * nor correct protocol with other devices such as
> +	 * touchscreens, which will send their own SYN_REPORT's
> +	 * when sufficient event information has been collected
> +	 * (e.g., for touchscreens, when pressure and X/Y coordinates
> +	 * have been received). Hence, we will only send this extra
> +	 * SYN_REPORT if type == EV_KEY.
> +	 */
> +	if (type == EV_KEY)
> +		input_sync(edev->input);

Ideally we would not be sending synthetic EV_SYN at all...

>  	return IRQ_HANDLED;
>  }
>  
> @@ -155,6 +173,19 @@ static int events_probe(struct platform_device *pdev)
>  	input_dev->name = edev->name;
>  	input_dev->id.bustype = BUS_HOST;
>  
> +	/*
> +	 * Set the Goldfish Device to be multitouch.
> +	 *
> +	 * In the Ranchu kernel, there is multitouch-specific code
> +	 * for handling ABS_MT_SLOT events (see
> +	 * drivers/input/input.c:input_handle_abs_event).
> +	 * If we do not issue input_mt_init_slots, the kernel will
> +	 * filter out needed ABS_MT_SLOT events when we touch the
> +	 * screen in more than one place, preventing multitouch with
> +	 * more than one finger from working.
> +	 */
> +	input_mt_init_slots(input_dev, GOLDFISH_MAX_FINGERS, 0);

This needs error handling. Also, can the backend communicate number of
slots so the userspace has better idea about the capabilities of the
device?

> +
>  	events_import_bits(edev, input_dev->evbit, EV_SYN, EV_MAX);
>  	events_import_bits(edev, input_dev->keybit, EV_KEY, KEY_MAX);
>  	events_import_bits(edev, input_dev->relbit, EV_REL, REL_MAX);
> -- 
> 2.7.4
> 

Thanks.

-- 
Dmitry

RE: [PATCH v2 0/7] MIPS: Miscellaneous fixes related to Android Mips emulator

[Miodrag Dinic]

Hi Greg,

> How well does these patches "work" with the recent goldfish
> images/kernels that are out there?  I know the goldfish platform has
> been revamped a lot recently, and I would not like to see these changes
> cause things to break there :)

Actually these changes have been in the Googles emulator kernel repo for
a long time and they fix issues found during Android testing :
https://android.googlesource.com/kernel/goldfish.git

So there should not be any regression with them.

> Also, any chance to get some google reviewers for these changes?  I
> don't think you added any to the cc: list, how come?

cc-ing Jin Quian, Bo Hu & Lingfeng Yang from Google.

Kind regards,
Miodrag

________________________________________
From: Greg Kroah-Hartman [gregkh@linuxfoundation.org]
Sent: Wednesday, June 28, 2017 6:30 PM
To: Aleksandar Markovic
Cc: linux-mips@linux-mips.org; Aleksandar Markovic; Douglas Leung; Goran Ferenc; James Hogan; Jiri Slaby; linux-kernel@vger.kernel.org; Miodrag Dinic; Paul Burton; Petar Jovanovic; Raghu Gandham
Subject: Re: [PATCH v2 0/7] MIPS: Miscellaneous fixes related to Android Mips emulator

On Wed, Jun 28, 2017 at 05:56:24PM +0200, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
>
> v1->v2:
>
>     - the patch on PREF usage in memcpy dropped as not needed
>     - updated recipient lists using get_maintainer.pl
>     - rebased to the latest kernel code
>
> This series contains an assortment of changes necessary for proper
> operation of Android emulator for Mips. However, we think that wider
> kernel community may benefit from them too.

This is nice, thanks for these.

How well does these patches "work" with the recent goldfish
images/kernels that are out there?  I know the goldfish platform has
been revamped a lot recently, and I would not like to see these changes
cause things to break there :)

Also, any chance to get some google reviewers for these changes?  I
don't think you added any to the cc: list, how come?

thanks,

greg k-h

RE: [PATCH v2 5/7] input: goldfish: Fix multitouch event handling

[Aleksandar Markovic]

Hello, Dmitry,

Thanks for your valuable review - and sorry for my late response.

For this patch, I am just the submitter, and I would ask Lingfang to
make some additional comments/clarifications regarding architecture of
user-kernel protocols for relevant drivers, and also regarding similar
issues that you brought up.

However, please see my notes (from the point of view of user of this
driver) below, perhaps they can clear some doubts regarding this patch.

> ________________________________________
> From: Dmitry Torokhov [dmitry.torokhov@gmail.com]
> Sent: Thursday, June 29, 2017 11:58 AM
> To: Aleksandar Markovic
> Cc: linux-mips@linux-mips.org; Lingfeng Yang; Miodrag Dinic; Goran Ferenc; Aleksandar Markovic; Douglas Leung; Henrik Rydberg; James Hogan; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Paul Burton; Petar Jovanovic; Raghu Gandham
> Subject: Re: [PATCH v2 5/7] input: goldfish: Fix multitouch event handling
> 
> On Wed, Jun 28, 2017 at 05:56:29PM +0200, Aleksandar Markovic wrote:
> > From: Lingfeng Yang <lfy@google.com>
> >
> > Register Goldfish Events device properly as a multitouch device,
> > and send SYN_REPORT event in appropriate cases only.
> >
> > If SYN_REPORT is sent on every single multitouch event, it breaks
> > the multitouch. The multitouch becomes janky and having to click
> > 2-3 times to do stuff (plus randomly activating notification bars
> > when not clicking).
> 
> This sounds like a deficiency in protocol handling in userspace. Given
> that input core can suppress duplicate events userpsace mught very well
> only see one ABS_X followed by SYN_REPORT if Y coordinate did not change
> or was suppressed by jitter detection.
> 

I can't comment on protocols - I have to deffer these questions to
Lingfeng,

My experiences during integration of Android emulator for Mips
related to this driver is as follows: Without this patch, UI
interaction (even non-multitouch) is so erratic that I would think
majority of users would deem emulator UI non-usable. So, the problem
is severe. With this patch, however, UI is nice and dendy - and,
more than this, we did accross-the-board UI regression tests of 
this path (via CTS), and did not find any regression at all.


> > If these SYN_REPORT events are supressed,
> > multitouch will work fine, plus the events will have a protocol
> > that looks nice.
> >
> > In addition, Goldfish Events device needs to be registerd as a
> > multitouch device by issuing input_mt_init_slots. Otherwise,
> > input_handle_abs_event in drivers/input/input.c will silently drop
> > all ABS_MT_SLOT events, casusing touches with more than one finger
> > not to work properly.
> >
> > Signed-off-by: Lingfeng Yang <lfy@google.com>
> > Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
> > Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
> > Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
> > ---
> >  drivers/input/keyboard/goldfish_events.c | 33 +++++++++++++++++++++++++++++++-
> >  1 file changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/keyboard/goldfish_events.c b/drivers/input/keyboard/goldfish_events.c
> > index f6e643b..6e0b8bb 100644
> > --- a/drivers/input/keyboard/goldfish_events.c
> > +++ b/drivers/input/keyboard/goldfish_events.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/types.h>
> >  #include <linux/input.h>
> > +#include <linux/input/mt.h>
> >  #include <linux/kernel.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/slab.h>
> > @@ -24,6 +25,8 @@
> >  #include <linux/io.h>
> >  #include <linux/acpi.h>
> >
> > +#define GOLDFISH_MAX_FINGERS 5
> > +
> >  enum {
> >       REG_READ        = 0x00,
> >       REG_SET_PAGE    = 0x00,
> > @@ -52,7 +55,22 @@ static irqreturn_t events_interrupt(int irq, void *dev_id)
> >       value = __raw_readl(edev->addr + REG_READ);
> >
> >       input_event(edev->input, type, code, value);
> > -     input_sync(edev->input);
> > +
> > +     /*
> > +      * Send an extra (EV_SYN, SYN_REPORT, 0x0) event if a key
> > +      * was pressed. Some keyboard device drivers may only send
> > +      * the EV_KEY event and not EV_SYN.
> 
> Can they be fixed?
> 
> > +      *
> > +      * Note that sending an extra SYN_REPORT is not necessary
> > +      * nor correct protocol with other devices such as
> > +      * touchscreens, which will send their own SYN_REPORT's
> > +      * when sufficient event information has been collected
> > +      * (e.g., for touchscreens, when pressure and X/Y coordinates
> > +      * have been received). Hence, we will only send this extra
> > +      * SYN_REPORT if type == EV_KEY.
> > +      */
> > +     if (type == EV_KEY)
> > +             input_sync(edev->input);
> 
> Ideally we would not be sending synthetic EV_SYN at all...
> 

My understanding is that this patch aims to minimize synthetic EV_SYNs.
It would've been great if it had eliminated them all, but such
requirement seems to require significant work in multiple drivers.
All that taken into account, this patch looks to me like a good step in
the right direction, and I am asking for its acceptance in its current
form.

> >       return IRQ_HANDLED;
> >  }
> >
> > @@ -155,6 +173,19 @@ static int events_probe(struct platform_device *pdev)
> >       input_dev->name = edev->name;
> >       input_dev->id.bustype = BUS_HOST;
> >
> > +     /*
> > +      * Set the Goldfish Device to be multitouch.
> > +      *
> > +      * In the Ranchu kernel, there is multitouch-specific code
> > +      * for handling ABS_MT_SLOT events (see
> > +      * drivers/input/input.c:input_handle_abs_event).
> > +      * If we do not issue input_mt_init_slots, the kernel will
> > +      * filter out needed ABS_MT_SLOT events when we touch the
> > +      * screen in more than one place, preventing multitouch with
> > +      * more than one finger from working.
> > +      */
> > +     input_mt_init_slots(input_dev, GOLDFISH_MAX_FINGERS, 0);
> 
> This needs error handling. Also, can the backend communicate number of
> slots so the userspace has better idea about the capabilities of the
> device?
> 

Error handling will be fixed. Detecting capabilities sounds like a
good idea, but how about leaving it for a future patch?

> > +
> >       events_import_bits(edev, input_dev->evbit, EV_SYN, EV_MAX);
> >       events_import_bits(edev, input_dev->keybit, EV_KEY, KEY_MAX);
> >       events_import_bits(edev, input_dev->relbit, EV_REL, REL_MAX);
> > --
> > 2.7.4
> >
> 
> Thanks.
> 
> --
> Dmitry

Also, at the end, I would like to point that this patch have already
been in Android kernel "common 4.4" repository for some longish time,
we picked it from there:

https://android.googlesource.com/kernel/common/+/8bf12bc1b78dac6cb4fb7e4fbc0920978d17f5ea

This means that this patch is tested fairly well by now.

Regards,

Aleksandar

RE: [PATCH v2 5/7] input: goldfish: Fix multitouch event handling

[Aleksandar Markovic]

Hello, Dmitry,

Thanks for your valuable review - and sorry for my late response.

For this patch, I am just the submitter, and I would ask Lingfang to
make some additional comments/clarifications regarding architecture of
user-kernel protocols for relevant drivers, and also regarding similar
issues that you brought up.

However, please see my notes (from the point of view of user of this
driver) below, perhaps they can clear some doubts regarding this patch.

> ________________________________________
> From: Dmitry Torokhov [dmitry.torokhov@gmail.com]
> Sent: Thursday, June 29, 2017 11:58 AM
> To: Aleksandar Markovic
> Cc: linux-mips@linux-mips.org; Lingfeng Yang; Miodrag Dinic; Goran Ferenc; Aleksandar Markovic; Douglas Leung; Henrik Rydberg; James Hogan; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Paul Burton; Petar Jovanovic; Raghu Gandham
> Subject: Re: [PATCH v2 5/7] input: goldfish: Fix multitouch event handling
> 
> On Wed, Jun 28, 2017 at 05:56:29PM +0200, Aleksandar Markovic wrote:
> > From: Lingfeng Yang <lfy@google.com>
> >
> > Register Goldfish Events device properly as a multitouch device,
> > and send SYN_REPORT event in appropriate cases only.
> >
> > If SYN_REPORT is sent on every single multitouch event, it breaks
> > the multitouch. The multitouch becomes janky and having to click
> > 2-3 times to do stuff (plus randomly activating notification bars
> > when not clicking).
> 
> This sounds like a deficiency in protocol handling in userspace. Given
> that input core can suppress duplicate events userpsace mught very well
> only see one ABS_X followed by SYN_REPORT if Y coordinate did not change
> or was suppressed by jitter detection.
> 

I can't comment on protocols - I have to deffer these questions to
Lingfeng,

My experiences during integration of Android emulator for Mips
related to this driver is as follows: Without this patch, UI
interaction (even non-multitouch) is so erratic that I would think
majority of users would deem emulator UI non-usable. So, the problem
is severe. With this patch, however, UI is nice and dendy - and,
more than this, we did accross-the-board UI regression tests of 
this path (via CTS), and did not find any regression at all.


> > If these SYN_REPORT events are supressed,
> > multitouch will work fine, plus the events will have a protocol
> > that looks nice.
> >
> > In addition, Goldfish Events device needs to be registerd as a
> > multitouch device by issuing input_mt_init_slots. Otherwise,
> > input_handle_abs_event in drivers/input/input.c will silently drop
> > all ABS_MT_SLOT events, casusing touches with more than one finger
> > not to work properly.
> >
> > Signed-off-by: Lingfeng Yang <lfy@google.com>
> > Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
> > Signed-off-by: Goran Ferenc <goran.ferenc@imgtec.com>
> > Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
> > ---
> >  drivers/input/keyboard/goldfish_events.c | 33 +++++++++++++++++++++++++++++++-
> >  1 file changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/keyboard/goldfish_events.c b/drivers/input/keyboard/goldfish_events.c
> > index f6e643b..6e0b8bb 100644
> > --- a/drivers/input/keyboard/goldfish_events.c
> > +++ b/drivers/input/keyboard/goldfish_events.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/types.h>
> >  #include <linux/input.h>
> > +#include <linux/input/mt.h>
> >  #include <linux/kernel.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/slab.h>
> > @@ -24,6 +25,8 @@
> >  #include <linux/io.h>
> >  #include <linux/acpi.h>
> >
> > +#define GOLDFISH_MAX_FINGERS 5
> > +
> >  enum {
> >       REG_READ        = 0x00,
> >       REG_SET_PAGE    = 0x00,
> > @@ -52,7 +55,22 @@ static irqreturn_t events_interrupt(int irq, void *dev_id)
> >       value = __raw_readl(edev->addr + REG_READ);
> >
> >       input_event(edev->input, type, code, value);
> > -     input_sync(edev->input);
> > +
> > +     /*
> > +      * Send an extra (EV_SYN, SYN_REPORT, 0x0) event if a key
> > +      * was pressed. Some keyboard device drivers may only send
> > +      * the EV_KEY event and not EV_SYN.
> 
> Can they be fixed?
> 
> > +      *
> > +      * Note that sending an extra SYN_REPORT is not necessary
> > +      * nor correct protocol with other devices such as
> > +      * touchscreens, which will send their own SYN_REPORT's
> > +      * when sufficient event information has been collected
> > +      * (e.g., for touchscreens, when pressure and X/Y coordinates
> > +      * have been received). Hence, we will only send this extra
> > +      * SYN_REPORT if type == EV_KEY.
> > +      */
> > +     if (type == EV_KEY)
> > +             input_sync(edev->input);
> 
> Ideally we would not be sending synthetic EV_SYN at all...
> 

My understanding is that this patch aims to minimize synthetic EV_SYNs.
It would've been great if it had eliminated them all, but such
requirement seems to require significant work in multiple drivers.
All that taken into account, this patch looks to me like a good step in
the right direction, and I am asking for its acceptance in its current
form.

> >       return IRQ_HANDLED;
> >  }
> >
> > @@ -155,6 +173,19 @@ static int events_probe(struct platform_device *pdev)
> >       input_dev->name = edev->name;
> >       input_dev->id.bustype = BUS_HOST;
> >
> > +     /*
> > +      * Set the Goldfish Device to be multitouch.
> > +      *
> > +      * In the Ranchu kernel, there is multitouch-specific code
> > +      * for handling ABS_MT_SLOT events (see
> > +      * drivers/input/input.c:input_handle_abs_event).
> > +      * If we do not issue input_mt_init_slots, the kernel will
> > +      * filter out needed ABS_MT_SLOT events when we touch the
> > +      * screen in more than one place, preventing multitouch with
> > +      * more than one finger from working.
> > +      */
> > +     input_mt_init_slots(input_dev, GOLDFISH_MAX_FINGERS, 0);
> 
> This needs error handling. Also, can the backend communicate number of
> slots so the userspace has better idea about the capabilities of the
> device?
> 

Error handling will be fixed. Detecting capabilities sounds like a
good idea, but how about leaving it for a future patch?

> > +
> >       events_import_bits(edev, input_dev->evbit, EV_SYN, EV_MAX);
> >       events_import_bits(edev, input_dev->keybit, EV_KEY, KEY_MAX);
> >       events_import_bits(edev, input_dev->relbit, EV_REL, REL_MAX);
> > --
> > 2.7.4
> >
> 
> Thanks.
> 
> --
> Dmitry

Also, at the end, I would like to point that this patch have already
been in Android kernel "common 4.4" repository for some longish time,
we picked it from there:

https://android.googlesource.com/kernel/common/+/8bf12bc1b78dac6cb4fb7e4fbc0920978d17f5ea

This means that this patch is tested fairly well by now.

Regards,

Aleksandar
From Matt.Redfearn@imgtec.com Fri Jul 21 16:05:32 2017
Received: with ECARTIS (v1.0.0; list linux-mips); Fri, 21 Jul 2017 16:05:38 +0200 (CEST)
Received: from mailapp01.imgtec.com ([195.59.15.196]:49998 "EHLO
        mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by eddie.linux-mips.org
        with ESMTP id S23992466AbdGUOFbyyRXG (ORCPT
        <rfc822;linux-mips@linux-mips.org>); Fri, 21 Jul 2017 16:05:31 +0200
Received: from HHMAIL01.hh.imgtec.org (unknown [10.100.10.19])
        by Forcepoint Email with ESMTPS id EFA6C1B037F65;
        Fri, 21 Jul 2017 15:05:21 +0100 (IST)
Received: from mredfearn-linux.le.imgtec.org (10.150.130.83) by
 HHMAIL01.hh.imgtec.org (10.100.10.21) with Microsoft SMTP Server (TLS) id
 14.3.294.0; Fri, 21 Jul 2017 15:05:25 +0100
From:   Matt Redfearn <matt.redfearn@imgtec.com>
To:     Alexander Viro <viro@zeniv.linux.org.uk>
CC:     "Luis R . Rodriguez" <mcgrof@kernel.org>,
        <linux-mips@linux-mips.org>, Petr Mladek <pmladek@suse.com>,
        Matt Redfearn <matt.redfearn@imgtec.com>,
        <linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: [RFC PATCH] exec: Avoid recursive modprobe for binary format handlers
Date:   Fri, 21 Jul 2017 15:05:20 +0100
Message-ID: <1500645920-28490-1-git-send-email-matt.redfearn@imgtec.com>
X-Mailer: git-send-email 2.7.4
MIME-Version: 1.0
Content-Type: text/plain
X-Originating-IP: [10.150.130.83]
Return-Path: <Matt.Redfearn@imgtec.com>
X-Envelope-To: <"|/home/ecartis/ecartis -s linux-mips"> (uid 0)
X-Orcpt: rfc822;linux-mips@linux-mips.org
Original-Recipient: rfc822;linux-mips@linux-mips.org
X-archive-position: 59175
X-ecartis-version: Ecartis v1.0.0
Sender: linux-mips-bounce@linux-mips.org
Errors-to: linux-mips-bounce@linux-mips.org
X-original-sender: matt.redfearn@imgtec.com
Precedence: bulk
List-help: <mailto:ecartis@linux-mips.org?Subject=help>
List-unsubscribe: <mailto:ecartis@linux-mips.org?subject=unsubscribe%20linux-mips>
List-software: Ecartis version 1.0.0
List-Id: linux-mips <linux-mips.eddie.linux-mips.org>
X-List-ID: linux-mips <linux-mips.eddie.linux-mips.org>
List-subscribe: <mailto:ecartis@linux-mips.org?subject=subscribe%20linux-mips>
List-owner: <mailto:ralf@linux-mips.org>
List-post: <mailto:linux-mips@linux-mips.org>
List-archive: <http://www.linux-mips.org/archives/linux-mips/>
X-list: linux-mips
Content-Length: 2564
Lines: 63

When the kernel does not have a binary format handler for an executable
it is attempting to load, when CONFIG_MODULES is enabled it will attempt
to load a module for that format. If the kernel does not have a binary
format handler for the modprobe executable, this will trigger another
module load. Previously this recursive module loading was caught and an
error message printed informing the user that the executable could not
be executed:

request_module: runaway loop modprobe binfmt-464c
Starting init:/sbin/init exists but couldn't execute it (error -8)

Commit 6d7964a722af ("kmod: throttle kmod thread limit") which was
merged in v4.13-rc1 broke this behaviour since the recursive modprobe is
no longer caught, it just ends up waiting indefinitely for the kmod_wq
wait queue. Hence the kernel appears to hang silently when starting
userspace.

This problem was observed when the binfmt handler for MIPS o32 binaries
is not built in to a 64bit kernel and the root filesystem is o32 ABI.

Catch this by adding a guard to search_binary_handler(). If there is no
binary format handler available to load an exectuable, and the
executable matches modprobe_path, i.e. the userspace helper that would
be executed to load a module, then do not attempt to load the module
since it will just end up here again when it fails to execute. This
actually improves the original behaviour since the "runaway loop"
warning is no longer printed, and we simply get:

Starting init:/sbin/init exists but couldn't execute it (error -8)

Fixes: 6d7964a722af ("kmod: throttle kmod thread limit")
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
---

What we really need to detect is that exec'ing modprobe failed, but
currently it does not get as far as an actual error since it just ends
up stuck waiting for the modprobes to complete, which they never will.
Open to suggestions of a different / better way to fix this.

Thanks,
Matt

---
 fs/exec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index 62175cbcc801..004bb50a01fe 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1644,6 +1644,9 @@ int search_binary_handler(struct linux_binprm *bprm)
 		if (printable(bprm->buf[0]) && printable(bprm->buf[1]) &&
 		    printable(bprm->buf[2]) && printable(bprm->buf[3]))
 			return retval;
+		/* Game over if we need to load a module to execute modprobe */
+		if (strcmp(bprm->filename, modprobe_path) == 0)
+			return retval;
 		if (request_module("binfmt-%04x", *(ushort *)(bprm->buf + 2)) < 0)
 			return retval;
 		need_retry = false;
-- 
2.7.4