On 18/10/2019 17:23, Segher Boessenkool wrote:
On Fri, Oct 18, 2019 at 04:53:26PM +0100, Mark Cave-Ayland wrote:
On 18/10/2019 10:52, Segher Boessenkool wrote:
On Fri, Oct 18, 2019 at 08:48:53AM +0100, Mark Cave-Ayland wrote:
Otherwise the compiler may not understand that a read from the assigned buffer is comming from memory-mapped IO space and fail to update accordingly.
I think you should not tell the compiler that whatever writes to ch writes the same value multiple times in sequence, instead?
Can you explain a bit more about what you mean? The issue seems to be related to this:
static volatile unsigned char *kbd_dev;
void kbd_init(phys_addr_t base) { kbd_dev = (unsigned char *)ofmem_map_io(base, 2 * 4); kbd_dev += 4; }
... ...
unsigned char keyboard_readdata(void) { volatile unsigned char ch;
while (!keyboard_dataready()) { } do { ch = kbd_dev[2] & 0xff; ...}
Without the volatile marking on ch it seems with optimisation enabled that ch gets optimised into a register causing strange intermittent keyboard behaviour. I guess since kbd_dev is a pointer to something of unknown size, the compiler doesn't understand that kbd_dev[2] should also be considered volatile?
No, the compiler understands that fine.
How is ch *used*?
[ It's a lot better to treat I/O as I/O, accessed via access functions, than to treat it as memory, exposed in C as some variables. I/O is not memory, and you cannot access it exactly the same way, normally. But that aside. ]
It's used to return a single character read from the serial port (along with some special key detection and keycode translation). The complete code looks like this:
unsigned char keyboard_readdata(void) { volatile unsigned char ch;
while (!keyboard_dataready()) { }
do { ch = kbd_dev[2] & 0xff; if (ch == 99) shiftstate |= 1; else if (ch == 110) shiftstate |= 2; else if (ch == 227) shiftstate &= ~1; else if (ch == 238) shiftstate &= ~2; //printk("getch: %d\n", ch); } // If release, wait for key press while ((ch & 0x80) == 0x80 || ch == 238 || ch == 227); //printk("getch rel: %d\n", ch); ch &= 0x7f; if (shiftstate) ch = sunkbd_keycode_shifted[ch]; else ch = sunkbd_keycode[ch]; //printk("getch xlate: %d\n", ch);
return ch; }
The kbd_dev pointer is allocated dynamically when mapping the physical device address.
ATB,
Mark.