On 29/06/2019 14.02, Philippe Mathieu-Daudé wrote:
> On 6/28/19 8:15 PM, Thomas Huth wrote:
>> It is likely still quite incomplete (e.g. mouse and interrupts are not
>> implemented yet), but it is good enough for keyboard input at the firmware
>> monitor.
[...]
>> +
>> +static uint32_t kbd_read_word(void *opaque, hwaddr addr)
>> +{
>> + qemu_log_mask(LOG_UNIMP, "NeXT kbd read word %"HWADDR_PRIx"\n", addr);
>
> This sounds odd... a bus working in 32-bit/8-bit but not 16...
I guess the bus is working in 16-bit, too, it's just that the firmware
(and the OS) does not access it in that mode, so neither Bryce nor I
spent time implementing this. It's an "UNIMP" log entry ... so whenever
we see it during runtime, we still can add it.
>> + return 0;
>> +}
[...]
>> +static void kbd_write_byte(void *opaque, hwaddr addr, uint32_t val)
>> +{
>> + qemu_log_mask(LOG_UNIMP, "NeXT kbd write byte %"HWADDR_PRIx"\n", addr);
>> +}
>> +static void kbd_write_word(void *opaque, hwaddr addr, uint32_t val)
>> +{
>> + qemu_log_mask(LOG_UNIMP, "NeXT kbd write addr %"HWADDR_PRIx"\n", addr);
>> +}
>> +static void kbd_write_long(void *opaque, hwaddr addr, uint32_t val)
>> +{
>> + qemu_log_mask(LOG_UNIMP, "NeXT kbd write long %"HWADDR_PRIx"\n", addr);
>> +}
>> +
>> +static uint64_t kbd_readfn(void *opaque, hwaddr addr, unsigned size)
>> +{
>> + switch (size) {
>> + case 1:
>> + return kbd_read_byte(opaque, addr);
>> + case 2:
>> + return kbd_read_word(opaque, addr);
>> + case 4:
>> + return kbd_read_long(opaque, addr);
>> + default:
>> + g_assert_not_reached();
>> + }
>> +}
>> +
>> +static void kbd_writefn(void *opaque, hwaddr addr, uint64_t value,
>> + unsigned size)
>> +{
>> + switch (size) {
>> + case 1:
>> + kbd_write_byte(opaque, addr, value);
>> + break;
>> + case 2:
>> + kbd_write_word(opaque, addr, value);
>> + break;
>> + case 4:
>> + kbd_write_long(opaque, addr, value);
>> + break;
>> + default:
>> + g_assert_not_reached();
>> + }
>
> Well, you can replace this by:
>
> qemu_log_mask(LOG_UNIMP,
> "NeXT kbd write size:%u 0x%"HWADDR_PRIx"\n",
> size, addr);
>
> and kill the kbd_write_*() functions and the assert (never
> reached since .valid.max_access_size = 4).
Good idea, I'll do it in v3.
>> +}
>> +
>> +static const MemoryRegionOps kbd_ops = {
>> + .read = kbd_readfn,
>> + .write = kbd_writefn,
>> + .valid.min_access_size = 1,
>> + .valid.max_access_size = 4,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static void nextkbd_event(void *opaque, int ch)
>> +{
>> + /*
>> + * Will want to set vars for caps/num lock
>> + * if (ch & 0x80) -> key release
>> + * there's also e0 escaped scancodes that might need to be handled
>> + */
>> + queue_code(opaque, ch);
>> +}
>> +
>> +static const unsigned char next_keycodes[128] = {
>> + 0x00, 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x50, 0x4F,
>> + 0x4E, 0x1E, 0x1F, 0x20, 0x1D, 0x1C, 0x1B, 0x00,
>> + 0x42, 0x43, 0x44, 0x45, 0x48, 0x47, 0x46, 0x06,
>> + 0x07, 0x08, 0x00, 0x00, 0x2A, 0x00, 0x39, 0x3A,
>> + 0x3B, 0x3C, 0x3D, 0x40, 0x3F, 0x3E, 0x2D, 0x2C,
>> + 0x2B, 0x26, 0x00, 0x00, 0x31, 0x32, 0x33, 0x34,
>> + 0x35, 0x37, 0x36, 0x2e, 0x2f, 0x30, 0x00, 0x00,
>> + 0x00, 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
>> + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0
>> +};
>> +
>> +static void queue_code(void *opaque, int code)
>> +{
>> + NextKBDState *s = NEXTKBD(opaque);
>> + KBDQueue *q = &s->queue;
>> + int key = code & 0x7F;
>
> Here 0x7f is ARRAY_SIZE(next_keycodes) - 1, ok.
> Isn't it the KD_KEYMASK definition from above then?
I guess so ... I'll use it in v3.
>> + int release = code & 0x80;
>> + static int ext;
>> +
>> + if (code == 0xE0) {
>> + ext = 1;
>> + }
>> +
>> + if (code == 0x2A || code == 0x1D || code == 0x36) {
>> + if (code == 0x2A) {
>> + s->shift = KD_LSHIFT;
>> + } else if (code == 0x36) {
>> + s->shift = KD_RSHIFT;
>> + ext = 0;
>> + } else if (code == 0x1D && !ext) {
>> + s->shift = KD_LCOMM;
>> + } else if (code == 0x1D && ext) {
>> + ext = 0;
>> + s->shift = KD_RCOMM;
>> + }
>> + return;
>> + } else if (code == (0x2A | 0x80) || code == (0x1D | 0x80) ||
>> + code == (0x36 | 0x80)) {
>> + s->shift = 0;
>> + return;
>> + }
>> +
>> + if (q->count >= KBD_QUEUE_SIZE) {
>> + return;
>> + }
>> +
>> + q->data[q->wptr] = next_keycodes[key] | release;
>> +
>> + if (++q->wptr == KBD_QUEUE_SIZE) {
>> + q->wptr = 0;
>> + }
>> +
>> + q->count++;
>> +
>> + /*
>> + * might need to actually trigger the NeXT irq, but as the keyboard
>> works
>> + * at the moment, I'll worry about it later
>> + */
>> + /* s->update_irq(s->update_arg, 1); */
>> +}
>> +
>> +static void nextkbd_reset(DeviceState *dev)
>> +{
>> + NextKBDState *nks = NEXTKBD(dev);
>> +
>> + memset(&nks->queue, 0, sizeof(KBDQueue));
>> + nks->shift = 0;
>> +}
>> +
>> +static void nextkbd_realize(DeviceState *dev, Error **errp)
>> +{
>> + NextKBDState *s = NEXTKBD(dev);
>> +
>> + memory_region_init_io(&s->mr, OBJECT(dev), &kbd_ops, s, "next.kbd",
>> 0x1000);
>> +
>> + qemu_add_kbd_event_handler(nextkbd_event, s);
>> +}
>> +
>> +static void nextkbd_class_init(ObjectClass *oc, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(oc);
>> +
>> + set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
>> + dc->realize = nextkbd_realize;
>> + dc->reset = nextkbd_reset;
>> +}
>> +
>> +static const TypeInfo nextkbd_info = {
>> + .name = TYPE_NEXTKBD,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(NextKBDState),
>> + .class_init = nextkbd_class_init,
>> +};
>> +
>> +static void nextkbd_register_types(void)
>> +{
>> + type_register_static(&nextkbd_info);
>> +}
>> +
>> +type_init(nextkbd_register_types)
>> +
>> +void nextkbd_init(void)
>> +{
>> + DeviceState *dev;
>> + NextKBDState *nks;
>> +
>> + dev = qdev_create(NULL, TYPE_NEXTKBD);
>> + qdev_init_nofail(dev);
>> +
>> + nks = NEXTKBD(dev);
>> + memory_region_add_subregion(get_system_memory(), 0x200e000, &nks->mr);
>
> 0x200e000 -> 0x0200e000 :)
Ok.
>> +}
>
> Again, nextkbd_init() is board-specific code.
> Could you move this function there?
Will do.
Thanks,
Thomas
