linux-kernel - Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 01 Dec 2010 12:20:20 -0800
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Daniel Walker <dwalker@...eaurora.org>,
	Tony Lindgren <tony@...mide.com>
CC:	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	Arnd Bergmann <arnd@...db.de>,
	Nicolas Pitre <nico@...xnic.net>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Mike Frysinger <vapier@...too.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Randy Dunlap <randy.dunlap@...cle.com>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support

On 12/01/2010 10:54 AM, Daniel Walker wrote:
> Are you talking about __dcc_getstatus only? I don't think adding
> volatile is going to hurt anything, if not having it causes problems.
>

Just marking __dcc_getstatus volatile gives me

00000038 <hvc_dcc_get_chars>:
  38:   ee10fe11        mrc     14, 0, pc, cr0, cr1, {0}
  3c:   1afffffd        bne     38 <hvc_dcc_get_chars>
  40:   ee103e15        mrc     14, 0, r3, cr0, cr5, {0}
  44:   e3a00000        mov     r0, #0  ; 0x0
  48:   e6ef3073        uxtb    r3, r3
  4c:   ea000004        b       64 <hvc_dcc_get_chars+0x2c>
  50:   ee10ce11        mrc     14, 0, ip, cr0, cr1, {0}
  54:   e31c0101        tst     ip, #1073741824 ; 0x40000000
  58:   012fff1e        bxeq    lr
  5c:   e7c13000        strb    r3, [r1, r0]
  60:   e2800001        add     r0, r0, #1      ; 0x1
  64:   e1500002        cmp     r0, r2
  68:   bafffff8        blt     50 <hvc_dcc_get_chars+0x18>
  6c:   e12fff1e        bx      lr

Seems the compiler keeps the value of __dcc_getchar() in r3 for the
duration of the loop. So we need to mark that one volatile too. I don't
think __dcc_putchar() needs to be marked volatile but it probably
doesn't hurt.

>
> We could maybe drop the looping for TX, but RX has no C based looping
> even tho for v7 it's recommended that we loop (presumably for v6 it's
> not recommended).
>

Definitely for TX since it seems like a redundant loop, but I agree RX
code has changed. Instead of

If RX buffer full
Poll for RX buffer full
Read character from RX buffer

we would have

If RX buffer full
Read character from RX buffer

which doesn't seem all that different assuming the RX buffer doesn't go
from full to empty between the If and Poll steps. Hopefully Tony knows more.

> Like this?
>
> 	for (i = 0; i < count; ++i) {
>
> 		if (__dcc_getstatus() & DCC_STATUS_RX)
> 			buf[i] = __dcc_getchar();
> 		else
> 			break;
> 	}
>
> It's a micro clean up I guess ..

Yes, it's much clearer that way.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/