[PATCH 0/5] omap assembly fixes for CONFIG_DEBUG_RODATA

STALE3922d SUPERSEDED

Revision v1 of 2 in this series.

11 messages, 4 authors, 2016-01-19 · open the first message on its own page

[PATCH 0/5] omap assembly fixes for CONFIG_DEBUG_RODATA

From: tony@atomide.com (Tony Lindgren)
Date: 2016-01-19 17:48:49

Hi all,

Here are read-only data fixes for omaps discovered few weeks ago
with "[PATCH v2] ARM: mm: flip priority of CONFIG_DEBUG_RODATA"
that caused issues during deeper idle states on omaps [0].

Please review and test if possible. I'd like to merge these fixes
during v4.5-rc cycle to remove these blockers for making it possible
to have CONFIG_DEBUG_RODATA enabled by default.

Regards,

Tony


[0] https://lkml.org/lkml/2015/12/2/908

Tony Lindgren (5):
  ARM: OMAP2+: Fix wait_dll_lock_timed for rodata
  ARM: OMAP2+: Fix l2dis_3630 for rodata
  ARM: OMAP2+: Fix save_secure_ram_context for rodata
  ARM: OMAP2+: Fix l2_inv_api_params for rodata
  ARM: OMAP2+: Fix ppa_zero_params and ppa_por_params for rodata

 arch/arm/mach-omap2/sleep34xx.S | 60 ++++++++++++++++++++---------------------
 arch/arm/mach-omap2/sleep44xx.S | 25 +++++++++++------
 2 files changed, 47 insertions(+), 38 deletions(-)

-- 
2.7.0.rc3

[PATCH 1/5] ARM: OMAP2+: Fix wait_dll_lock_timed for rodata

From: tony@atomide.com (Tony Lindgren)
Date: 2016-01-19 17:48:50

We don't want to be writing to .text so it can be set rodata.
Fix error "Unable to handle kernel paging request at virtual address
c012396c" in wait_dll_lock_timed if CONFIG_DEBUG_RODATA is selected.

As these counters are for debugging only and unused, we can just
remove them.

Cc: Kees Cook <redacted>
Cc: Laura Abbott <redacted>
Cc: Nicolas Pitre <redacted>
Cc: Nishanth Menon <redacted>
Cc: Richard Woodruff <redacted>
Cc: Russell King <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/sleep34xx.S | 22 ----------------------
 1 file changed, 22 deletions(-)
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index eafd120..787cfda 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -289,12 +289,6 @@ wait_sdrc_ready:
 	bic	r5, r5, #0x40
 	str	r5, [r4]
 
-/*
- * PC-relative stores lead to undefined behaviour in Thumb-2: use a r7 as a
- * base instead.
- * Be careful not to clobber r7 when maintaing this code.
- */
-
 is_dll_in_lock_mode:
 	/* Is dll in lock mode? */
 	ldr	r4, sdrc_dlla_ctrl
@@ -302,11 +296,7 @@ is_dll_in_lock_mode:
 	tst	r5, #0x4
 	bne	exit_nonoff_modes	@ Return if locked
 	/* wait till dll locks */
-	adr	r7, kick_counter
 wait_dll_lock_timed:
-	ldr	r4, wait_dll_lock_counter
-	add	r4, r4, #1
-	str	r4, [r7, #wait_dll_lock_counter - kick_counter]
 	ldr	r4, sdrc_dlla_status
 	/* Wait 20uS for lock */
 	mov	r6, #8
@@ -330,9 +320,6 @@ kick_dll:
 	orr	r6, r6, #(1<<3)		@ enable dll
 	str	r6, [r4]
 	dsb
-	ldr	r4, kick_counter
-	add	r4, r4, #1
-	str	r4, [r7]		@ kick_counter
 	b	wait_dll_lock_timed
 
 exit_nonoff_modes:
@@ -360,15 +347,6 @@ sdrc_dlla_status:
 	.word	SDRC_DLLA_STATUS_V
 sdrc_dlla_ctrl:
 	.word	SDRC_DLLA_CTRL_V
-	/*
-	 * When exporting to userspace while the counters are in SRAM,
-	 * these 2 words need to be@the end to facilitate retrival!
-	 */
-kick_counter:
-	.word	0
-wait_dll_lock_counter:
-	.word	0
-
 ENTRY(omap3_do_wfi_sz)
 	.word	. - omap3_do_wfi
 
-- 
2.7.0.rc3

[PATCH 2/5] ARM: OMAP2+: Fix l2dis_3630 for rodata

From: tony@atomide.com (Tony Lindgren)
Date: 2016-01-19 17:48:51

We don't want to write to .text section. Let's move l2dis_3630
to .data and access it via a pointer.

Cc: Kees Cook <redacted>
Cc: Laura Abbott <redacted>
Cc: Nicolas Pitre <redacted>
Cc: Nishanth Menon <redacted>
Cc: Richard Woodruff <redacted>
Cc: Russell King <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/sleep34xx.S | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index 787cfda..f7c7bf8 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -86,7 +86,9 @@ ENTRY(enable_omap3630_toggle_l2_on_restore)
 	stmfd	sp!, {lr}	@ save registers on stack
 	/* Setup so that we will disable and enable l2 */
 	mov	r1, #0x1
-	adrl	r2, l2dis_3630	@ may be too distant for plain adr
+	adrl	r3, l2dis_3630_offset	@ may be too distant for plain adr
+	ldr	r2, [r3]
+	add	r2, r2, r3
 	str	r1, [r2]
 	ldmfd	sp!, {pc}	@ restore regs and return
 ENDPROC(enable_omap3630_toggle_l2_on_restore)
@@ -415,7 +417,9 @@ ENTRY(omap3_restore)
 	cmp	r2, #0x0	@ Check if target power state was OFF or RET
 	bne	logic_l1_restore
 
-	ldr	r0, l2dis_3630
+	adr	r1, l2dis_3630_offset
+	ldr	r0, [r1]
+	add	r0, r0, r1
 	cmp	r0, #0x1	@ should we disable L2 on 3630?
 	bne	skipl2dis
 	mrc	p15, 0, r0, c1, c0, 1
@@ -484,7 +488,9 @@ l2_inv_gp:
 	mov	r12, #0x2
 	smc	#0			@ Call SMI monitor (smieq)
 logic_l1_restore:
-	ldr	r1, l2dis_3630
+	adr	r0, l2dis_3630_offset
+	ldr	r1, [r0]
+	add	r1, r1, r0
 	cmp	r1, #0x1		@ Test if L2 re-enable needed on 3630
 	bne	skipl2reen
 	mrc	p15, 0, r1, c1, c0, 1
@@ -513,6 +519,10 @@ control_stat:
 	.word	CONTROL_STAT
 control_mem_rta:
 	.word	CONTROL_MEM_RTA_CTRL
+l2dis_3630_offset:
+	.long	l2dis_3630 - .
+
+	.data
 l2dis_3630:
 	.word	0
 
-- 
2.7.0.rc3

[PATCH 3/5] ARM: OMAP2+: Fix save_secure_ram_context for rodata

From: tony@atomide.com (Tony Lindgren)
Date: 2016-01-19 17:48:52

We don't want to write to .text and we can move save_secure_ram_context
into .data as it all gets copied into SRAM anyways.

Cc: Kees Cook <redacted>
Cc: Laura Abbott <redacted>
Cc: Nicolas Pitre <redacted>
Cc: Nishanth Menon <redacted>
Cc: Richard Woodruff <redacted>
Cc: Russell King <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/sleep34xx.S | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index f7c7bf8..042e4fb 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -93,8 +93,12 @@ ENTRY(enable_omap3630_toggle_l2_on_restore)
 	ldmfd	sp!, {pc}	@ restore regs and return
 ENDPROC(enable_omap3630_toggle_l2_on_restore)
 
-	.text
-/* Function to call rom code to save secure ram context */
+/*
+ * Function to call rom code to save secure ram context, This gets
+ * relocated to SRAM, so it can be all in .data section. Otherwise
+ * we need to initialize api_params separately.
+ */
+	.data
 	.align	3
 ENTRY(save_secure_ram_context)
 	stmfd	sp!, {r4 - r11, lr}	@ save registers on stack
@@ -128,6 +132,8 @@ ENDPROC(save_secure_ram_context)
 ENTRY(save_secure_ram_context_sz)
 	.word	. - save_secure_ram_context
 
+	.text
+
 /*
  * ======================
  * == Idle entry point ==
-- 
2.7.0.rc3

[PATCH 4/5] ARM: OMAP2+: Fix l2_inv_api_params for rodata

From: tony@atomide.com (Tony Lindgren)
Date: 2016-01-19 17:48:53

We don't want to write to .text, so let's move l2_inv_api_params
to .data and access it via a pointer.

Cc: Kees Cook <redacted>
Cc: Laura Abbott <redacted>
Cc: Nicolas Pitre <redacted>
Cc: Nishanth Menon <redacted>
Cc: Richard Woodruff <redacted>
Cc: Russell King <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/sleep34xx.S | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index 042e4fb..7b12c6b 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -437,12 +437,14 @@ skipl2dis:
 	and	r1, #0x700
 	cmp	r1, #0x300
 	beq	l2_inv_gp
+	adr	r0, l2_inv_api_params_offset	@ r3 points to dummy parameters
+	ldr	r3, [r0]
+	add	r3, r3, r0
 	mov	r0, #40			@ set service ID for PPA
 	mov	r12, r0			@ copy secure Service ID in r12
 	mov	r1, #0			@ set task id for ROM code in r1
 	mov	r2, #4			@ set some flags in r2, r6
 	mov	r6, #0xff
-	adr	r3, l2_inv_api_params	@ r3 points to dummy parameters
 	dsb				@ data write barrier
 	dmb				@ data memory barrier
 	smc	#1			@ call SMI monitor (smi #1)
@@ -476,8 +478,8 @@ skipl2dis:
 	b	logic_l1_restore
 
 	.align
-l2_inv_api_params:
-	.word	0x1, 0x00
+l2_inv_api_params_offset:
+	.long	l2_inv_api_params - .
 l2_inv_gp:
 	/* Execute smi to invalidate L2 cache */
 	mov r12, #0x1			@ set up to invalidate L2
@@ -532,6 +534,10 @@ l2dis_3630_offset:
 l2dis_3630:
 	.word	0
 
+	.data
+l2_inv_api_params:
+	.word	0x1, 0x00
+
 /*
  * Internal functions
  */
-- 
2.7.0.rc3

[PATCH 5/5] ARM: OMAP2+: Fix ppa_zero_params and ppa_por_params for rodata

From: tony@atomide.com (Tony Lindgren)
Date: 2016-01-19 17:48:54

We don't want to write to .text so let's move ppa_zero_params and
ppa_por_params to .data and access them via pointers.

Note that I have not been able to test as we I don't have a HS
omap4 to test with. The code has been changed in similar way as
for omap3 though.

Cc: Kees Cook <redacted>
Cc: Laura Abbott <redacted>
Cc: Nicolas Pitre <redacted>
Cc: Nishanth Menon <redacted>
Cc: Richard Woodruff <redacted>
Cc: Russell King <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/sleep44xx.S | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-omap2/sleep44xx.S b/arch/arm/mach-omap2/sleep44xx.S
index 9b09d85..74ab3d1 100644
--- a/arch/arm/mach-omap2/sleep44xx.S
+++ b/arch/arm/mach-omap2/sleep44xx.S
@@ -29,12 +29,6 @@
 	dsb
 .endm
 
-ppa_zero_params:
-	.word		0x0
-
-ppa_por_params:
-	.word		1, 0
-
 #ifdef CONFIG_ARCH_OMAP4
 
 /*
@@ -266,7 +260,9 @@ ENTRY(omap4_cpu_resume)
 	beq	skip_ns_smp_enable
 ppa_actrl_retry:
 	mov     r0, #OMAP4_PPA_CPU_ACTRL_SMP_INDEX
-	adr	r3, ppa_zero_params		@ Pointer to parameters
+	adr	r1, ppa_zero_params_offset	@ Pointer to parameters
+	ldr	r3, [r1]
+	add	r3, r3, r1
 	mov	r1, #0x0			@ Process ID
 	mov	r2, #0x4			@ Flag
 	mov	r6, #0xff
@@ -303,7 +299,9 @@ skip_ns_smp_enable:
 	ldr     r0, =OMAP4_PPA_L2_POR_INDEX
 	ldr     r1, =OMAP44XX_SAR_RAM_BASE
 	ldr     r4, [r1, #L2X0_PREFETCH_CTRL_OFFSET]
-	adr     r3, ppa_por_params
+	adr     r1, ppa_por_params_offset
+	ldr	r3, [r1]
+	add	r3, r3, r1
 	str     r4, [r3, #0x04]
 	mov	r1, #0x0			@ Process ID
 	mov	r2, #0x4			@ Flag
@@ -328,6 +326,8 @@ skip_l2en:
 #endif
 
 	b	cpu_resume			@ Jump to generic resume
+ppa_por_params_offset:
+	.long	ppa_por_params - .
 ENDPROC(omap4_cpu_resume)
 #endif	/* CONFIG_ARCH_OMAP4 */
 
@@ -380,4 +380,13 @@ ENTRY(omap_do_wfi)
 	nop
 
 	ldmfd	sp!, {pc}
+ppa_zero_params_offset:
+	.long	ppa_zero_params - .
 ENDPROC(omap_do_wfi)
+
+	.data
+ppa_zero_params:
+	.word		0
+
+ppa_por_params:
+	.word		1, 0
-- 
2.7.0.rc3

[PATCH 2/5] ARM: OMAP2+: Fix l2dis_3630 for rodata

From: Nicolas Pitre <hidden>
Date: 2016-01-19 18:10:09

On Tue, 19 Jan 2016, Tony Lindgren wrote:
quoted hunk
We don't want to write to .text section. Let's move l2dis_3630
to .data and access it via a pointer.

Cc: Kees Cook <redacted>
Cc: Laura Abbott <redacted>
Cc: Nicolas Pitre <redacted>
Cc: Nishanth Menon <redacted>
Cc: Richard Woodruff <redacted>
Cc: Russell King <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/sleep34xx.S | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index 787cfda..f7c7bf8 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -86,7 +86,9 @@ ENTRY(enable_omap3630_toggle_l2_on_restore)
 	stmfd	sp!, {lr}	@ save registers on stack
 	/* Setup so that we will disable and enable l2 */
 	mov	r1, #0x1
-	adrl	r2, l2dis_3630	@ may be too distant for plain adr
+	adrl	r3, l2dis_3630_offset	@ may be too distant for plain adr
+	ldr	r2, [r3]
+	add	r2, r2, r3
 	str	r1, [r2]
You could combine the add with the following str:

	str	r1, [r2, r3]
quoted hunk
 	ldmfd	sp!, {pc}	@ restore regs and return
 ENDPROC(enable_omap3630_toggle_l2_on_restore)
@@ -415,7 +417,9 @@ ENTRY(omap3_restore)
 	cmp	r2, #0x0	@ Check if target power state was OFF or RET
 	bne	logic_l1_restore
 
-	ldr	r0, l2dis_3630
+	adr	r1, l2dis_3630_offset
+	ldr	r0, [r1]
+	add	r0, r0, r1
 	cmp	r0, #0x1	@ should we disable L2 on 3630?
This looks wrong. You're testing the first bit of the address for 
l2dis_3630 rather than its content.
quoted hunk
 	bne	skipl2dis
 	mrc	p15, 0, r0, c1, c0, 1
@@ -484,7 +488,9 @@ l2_inv_gp:
 	mov	r12, #0x2
 	smc	#0			@ Call SMI monitor (smieq)
 logic_l1_restore:
-	ldr	r1, l2dis_3630
+	adr	r0, l2dis_3630_offset
+	ldr	r1, [r0]
+	add	r1, r1, r0
 	cmp	r1, #0x1		@ Test if L2 re-enable needed on 3630
Ditto here.
quoted hunk
 	bne	skipl2reen
 	mrc	p15, 0, r1, c1, c0, 1
@@ -513,6 +519,10 @@ control_stat:
 	.word	CONTROL_STAT
 control_mem_rta:
 	.word	CONTROL_MEM_RTA_CTRL
+l2dis_3630_offset:
+	.long	l2dis_3630 - .
+
+	.data
 l2dis_3630:
 	.word	0
 
-- 
2.7.0.rc3

[PATCH 0/5] omap assembly fixes for CONFIG_DEBUG_RODATA

From: Kees Cook <hidden>
Date: 2016-01-19 18:12:07

On Tue, Jan 19, 2016 at 9:48 AM, Tony Lindgren [off-list ref] wrote:
Hi all,

Here are read-only data fixes for omaps discovered few weeks ago
with "[PATCH v2] ARM: mm: flip priority of CONFIG_DEBUG_RODATA"
that caused issues during deeper idle states on omaps [0].

Please review and test if possible. I'd like to merge these fixes
during v4.5-rc cycle to remove these blockers for making it possible
to have CONFIG_DEBUG_RODATA enabled by default.
Yay! :) Thanks for working on this.

-Kees
Regards,

Tony


[0] https://lkml.org/lkml/2015/12/2/908

Tony Lindgren (5):
  ARM: OMAP2+: Fix wait_dll_lock_timed for rodata
  ARM: OMAP2+: Fix l2dis_3630 for rodata
  ARM: OMAP2+: Fix save_secure_ram_context for rodata
  ARM: OMAP2+: Fix l2_inv_api_params for rodata
  ARM: OMAP2+: Fix ppa_zero_params and ppa_por_params for rodata

 arch/arm/mach-omap2/sleep34xx.S | 60 ++++++++++++++++++++---------------------
 arch/arm/mach-omap2/sleep44xx.S | 25 +++++++++++------
 2 files changed, 47 insertions(+), 38 deletions(-)

--
2.7.0.rc3


-- 
Kees Cook
Chrome OS & Brillo Security

[PATCH 2/5] ARM: OMAP2+: Fix l2dis_3630 for rodata

From: tony@atomide.com (Tony Lindgren)
Date: 2016-01-19 18:40:46

* Nicolas Pitre [off-list ref] [160119 10:10]:
On Tue, 19 Jan 2016, Tony Lindgren wrote:
quoted
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -86,7 +86,9 @@ ENTRY(enable_omap3630_toggle_l2_on_restore)
 	stmfd	sp!, {lr}	@ save registers on stack
 	/* Setup so that we will disable and enable l2 */
 	mov	r1, #0x1
-	adrl	r2, l2dis_3630	@ may be too distant for plain adr
+	adrl	r3, l2dis_3630_offset	@ may be too distant for plain adr
+	ldr	r2, [r3]
+	add	r2, r2, r3
 	str	r1, [r2]
You could combine the add with the following str:
OK.
	str	r1, [r2, r3]
quoted
 	ldmfd	sp!, {pc}	@ restore regs and return
 ENDPROC(enable_omap3630_toggle_l2_on_restore)
@@ -415,7 +417,9 @@ ENTRY(omap3_restore)
 	cmp	r2, #0x0	@ Check if target power state was OFF or RET
 	bne	logic_l1_restore
 
-	ldr	r0, l2dis_3630
+	adr	r1, l2dis_3630_offset
+	ldr	r0, [r1]
+	add	r0, r0, r1
 	cmp	r0, #0x1	@ should we disable L2 on 3630?
This looks wrong. You're testing the first bit of the address for 
l2dis_3630 rather than its content.
Ugh yup a ldr missing missing there. Will check the whole series
and repost. Thanks for checking!

Regards,

Tony

[PATCH 3/5] ARM: OMAP2+: Fix save_secure_ram_context for rodata

From: Sergei Shtylyov <hidden>
Date: 2016-01-19 18:45:51

Hello.

On 01/19/2016 08:48 PM, Tony Lindgren wrote:
quoted hunk
We don't want to write to .text and we can move save_secure_ram_context
into .data as it all gets copied into SRAM anyways.

Cc: Kees Cook <redacted>
Cc: Laura Abbott <redacted>
Cc: Nicolas Pitre <redacted>
Cc: Nishanth Menon <redacted>
Cc: Richard Woodruff <redacted>
Cc: Russell King <redacted>
Cc: Tero Kristo <redacted>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
  arch/arm/mach-omap2/sleep34xx.S | 10 ++++++++--
  1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
index f7c7bf8..042e4fb 100644
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -93,8 +93,12 @@ ENTRY(enable_omap3630_toggle_l2_on_restore)
  	ldmfd	sp!, {pc}	@ restore regs and return
  ENDPROC(enable_omap3630_toggle_l2_on_restore)

-	.text
-/* Function to call rom code to save secure ram context */
+/*
+ * Function to call rom code to save secure ram context, This gets
    s/,/./?
quoted hunk
+ * relocated to SRAM, so it can be all in .data section. Otherwise
+ * we need to initialize api_params separately.
+ */
+	.data
  	.align	3
  ENTRY(save_secure_ram_context)
  	stmfd	sp!, {r4 - r11, lr}	@ save registers on stack
[...]

MBR, Sergei

[PATCH 3/5] ARM: OMAP2+: Fix save_secure_ram_context for rodata

From: tony@atomide.com (Tony Lindgren)
Date: 2016-01-19 21:57:45

* Sergei Shtylyov [off-list ref] [160119 10:46]:
quoted
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -93,8 +93,12 @@ ENTRY(enable_omap3630_toggle_l2_on_restore)
 	ldmfd	sp!, {pc}	@ restore regs and return
 ENDPROC(enable_omap3630_toggle_l2_on_restore)

-	.text
-/* Function to call rom code to save secure ram context */
+/*
+ * Function to call rom code to save secure ram context, This gets
   s/,/./?
Will fix thanks.

Tony
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help