diff mbox

[U-Boot,26/28] input: Convert i8042 to driver model

Message ID CAEUhbmXK8RjWpn+JkJnhOYrYzvGm_7Cqz_gaZ65nX8sw9UjYZg@mail.gmail.com
State Superseded
Delegated to: Simon Glass
Headers show

Commit Message

Bin Meng Sept. 15, 2015, 6:12 a.m. UTC 
Hi Simon,

On Wed, Sep 9, 2015 at 12:32 PM, Simon Glass <sjg@chromium.org> wrote:
> Adjust this driver to support driver model. The only users are x86 boards
> so this should be safe.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---

After updating QEMU to use the DM i8042 driver below:


The i8042 still does not work. I've applied the same changes to Crown
Bay. i8042 does not work either. Could you please have a look?

>
>  drivers/input/Makefile |   2 +-
>  drivers/input/i8042.c  | 113 ++++++++++++++++++++++++++++++++-----------------
>  include/i8042.h        |   6 ---
>  3 files changed, 76 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/input/Makefile b/drivers/input/Makefile
> index 9388dfe..5f15265 100644
> --- a/drivers/input/Makefile
> +++ b/drivers/input/Makefile
> @@ -7,7 +7,7 @@
>
>  obj-$(CONFIG_DM_KEYBOARD) += keyboard-uclass.o
>
> -obj-$(CONFIG_I8042_KBD) += i8042.o
> +obj-$(CONFIG_I8042_KEYB) += i8042.o
>  obj-$(CONFIG_TEGRA_KEYBOARD) += tegra-kbc.o
>  obj-$(CONFIG_TWL4030_INPUT) += twl4030.o
>  obj-$(CONFIG_CROS_EC_KEYB) += cros_ec_keyb.o
> diff --git a/drivers/input/i8042.c b/drivers/input/i8042.c
> index 5ebb571..3649e2f 100644
> --- a/drivers/input/i8042.c
> +++ b/drivers/input/i8042.c
> @@ -8,8 +8,11 @@
>  /* i8042.c - Intel 8042 keyboard driver routines */
>
>  #include <common.h>
> +#include <dm.h>
> +#include <errno.h>
>  #include <i8042.h>
>  #include <input.h>
> +#include <keyboard.h>
>  #include <asm/io.h>
>
>  /* defines */
> @@ -17,8 +20,9 @@
>  #define out8(p, v)     outb(v, p)
>
>  /* locals */
> -static struct input_config config;
> -static bool extended;
> +struct i8042_kbd_priv {
> +       bool extended;  /* true if an extended keycode is expected next */
> +};
>
>  static unsigned char ext_key_map[] = {
>         0x1c, /* keypad enter */
> @@ -60,12 +64,20 @@ static int kbd_output_full(void)
>         return kbd_timeout != -1;
>  }
>
> -static void kbd_led_set(int flags)
> +/**
> + * check_leds() - Check the keyboard LEDs and update them it needed
> + *
> + * @ret:       Value to return
> + * @return value of @ret
> + */
> +static int i8042_kbd_update_leds(struct udevice *dev, int leds)
>  {
>         kbd_input_empty();
>         out8(I8042_DATA_REG, CMD_SET_KBD_LED);
>         kbd_input_empty();
> -       out8(I8042_DATA_REG, flags & 0x7);
> +       out8(I8042_DATA_REG, leds & 0x7);
> +
> +       return 0;
>  }
>
>  static int kbd_write(int reg, int value)
> @@ -146,6 +158,8 @@ static int kbd_controller_present(void)
>  /*
>   * Implement a weak default function for boards that optionally
>   * need to skip the i8042 initialization.
> + *
> + * TODO(sjg@chromium.org): Use device tree for this?
>   */
>  int __weak board_i8042_skip(void)
>  {
> @@ -192,6 +206,8 @@ int i8042_disable(void)
>
>  static int i8042_kbd_check(struct input_config *input)
>  {
> +       struct i8042_kbd_priv *priv = dev_get_priv(input->dev);
> +
>         if ((in8(I8042_STS_REG) & STATUS_OBF) == 0) {
>                 return 0;
>         } else {
> @@ -203,15 +219,15 @@ static int i8042_kbd_check(struct input_config *input)
>                 if (scan_code == 0xfa) {
>                         return 0;
>                 } else if (scan_code == 0xe0) {
> -                       extended = true;
> +                       priv->extended = true;
>                         return 0;
>                 }
>                 if (scan_code & 0x80) {
>                         scan_code &= 0x7f;
>                         release = true;
>                 }
> -               if (extended) {
> -                       extended = false;
> +               if (priv->extended) {
> +                       priv->extended = false;
>                         for (i = 0; ext_key_map[i]; i++) {
>                                 if (ext_key_map[i] == scan_code) {
>                                         scan_code = 0x60 + i;
> @@ -223,21 +239,23 @@ static int i8042_kbd_check(struct input_config *input)
>                                 return 0;
>                 }
>
> -               input_add_keycode(&config, scan_code, release);
> +               input_add_keycode(input, scan_code, release);
>                 return 1;
>         }
>  }
>
>  /* i8042_kbd_init - reset keyboard and init state flags */
> -int i8042_kbd_init(void)
> +static int i8042_start(struct udevice *dev)
>  {
> +       struct keyboard_priv *uc_priv = dev_get_uclass_priv(dev);
> +       struct input_config *input = &uc_priv->input;
>         int keymap, try;
>         char *penv;
>         int ret;
>
>         if (!kbd_controller_present() || board_i8042_skip()) {
>                 debug("i8042 keyboard controller is not present\n");
> -               return -1;
> +               return -ENOENT;
>         }
>
>         /* Init keyboard device (default US layout) */
> @@ -253,46 +271,65 @@ int i8042_kbd_init(void)
>                         return -1;
>         }
>
> -       ret = input_init(&config, 0, keymap == KBD_GER);
> +       ret = input_add_tables(input, keymap == KBD_GER);
>         if (ret)
>                 return ret;
> -       config.read_keys = i8042_kbd_check;
> -       input_allow_repeats(&config, true);
>
> -       kbd_led_set(NORMAL);
> +       i8042_kbd_update_leds(dev, NORMAL);
> +       debug("%s: started\n", __func__);
>
>         return 0;
>  }
>
>  /**
> - * check_leds() - Check the keyboard LEDs and update them it needed
> + * Set up the i8042 keyboard. This is called by the stdio device handler
>   *
> - * @ret:       Value to return
> - * @return value of @ret
> + * We want to do this init when the keyboard is actually used rather than
> + * at start-up, since keyboard input may not currently be selected.
> + *
> + * Once the keyboard starts there will be a period during which we must
> + * wait for the keyboard to init. We do this only when a key is first
> + * read - see kbd_wait_for_fifo_init().
> + *
> + * @return 0 if ok, -ve on error
>   */
> -static int check_leds(int ret)
> +static int i8042_kbd_probe(struct udevice *dev)
>  {
> -       int leds;
> -
> -       leds = input_leds_changed(&config);
> -       if (leds >= 0)
> -               kbd_led_set(leds);
> +       struct keyboard_priv *uc_priv = dev_get_uclass_priv(dev);
> +       struct stdio_dev *sdev = &uc_priv->sdev;
> +       struct input_config *input = &uc_priv->input;
> +       int ret;
>
> -       return ret;
> -}
> +       /* Register the device. i8042_start() will be called soon */
> +       input->dev = dev;
> +       input->read_keys = i8042_kbd_check;
> +       input_allow_repeats(input, true);
> +       strcpy(sdev->name, "i8042-kbd");
> +       ret = input_stdio_register(sdev);
> +       if (ret) {
> +               debug("%s: input_stdio_register() failed\n", __func__);
> +               return ret;
> +       }
> +       debug("%s: ready\n", __func__);
>
> -/*
> - * i8042_tstc - test if keyboard input is available
> - */
> -int i8042_tstc(struct stdio_dev *dev)
> -{
> -       return check_leds(input_tstc(&config));
> +       return 0;
>  }
>
> -/*
> - * i8042_getc - wait till keyboard input is available
> - */
> -int i8042_getc(struct stdio_dev *dev)
> -{
> -       return check_leds(input_getc(&config));
> -}
> +static const struct keyboard_ops i8042_kbd_ops = {
> +       .start  = i8042_start,
> +       .update_leds    = i8042_kbd_update_leds,
> +};
> +
> +static const struct udevice_id i8042_kbd_ids[] = {
> +       { .compatible = "intel,i8042-keyboard" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(i8042_kbd) = {
> +       .name   = "i8042_kbd",
> +       .id     = UCLASS_KEYBOARD,
> +       .of_match = i8042_kbd_ids,
> +       .probe = i8042_kbd_probe,
> +       .ops    = &i8042_kbd_ops,
> +       .priv_auto_alloc_size = sizeof(struct i8042_kbd_priv),
> +};
> diff --git a/include/i8042.h b/include/i8042.h
> index e0afce1..9723b6a 100644
> --- a/include/i8042.h
> +++ b/include/i8042.h
> @@ -87,10 +87,4 @@ void i8042_flush(void);
>   */
>  int i8042_disable(void);
>
> -struct stdio_dev;
> -
> -int i8042_kbd_init(void);
> -int i8042_tstc(struct stdio_dev *dev);
> -int i8042_getc(struct stdio_dev *dev);
> -
>  #endif /* _I8042_H_ */
> --

Regards,
Bin

Comments

Simon Glass Oct. 18, 2015, 11:17 p.m. UTC | #1 
Hi Bin,

On 15 September 2015 at 00:12, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Wed, Sep 9, 2015 at 12:32 PM, Simon Glass <sjg@chromium.org> wrote:
>> Adjust this driver to support driver model. The only users are x86 boards
>> so this should be safe.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>
> After updating QEMU to use the DM i8042 driver below:
>
> diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts
> index fc74cd0..14782b3 100644
> --- a/arch/x86/dts/qemu-x86_i440fx.dts
> +++ b/arch/x86/dts/qemu-x86_i440fx.dts
> @@ -11,6 +11,7 @@
>  /include/ "skeleton.dtsi"
>  /include/ "serial.dtsi"
>  /include/ "rtc.dtsi"
> +/include/ "keyboard.dtsi"
>
>  / {
>         model = "QEMU x86 (I440FX)";
> diff --git a/include/configs/qemu-x86.h b/include/configs/qemu-x86.h
> index 1b544c1..9daf943 100644
> --- a/include/configs/qemu-x86.h
> +++ b/include/configs/qemu-x86.h
> @@ -30,7 +30,7 @@
>
>  #define CONFIG_PCI_PNP
>
> -#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,vga\0" \
> +#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,i8042-kbd\0" \
>                                         "stdout=serial,vga\0" \
>                                         "stderr=serial,vga\0"
>
>
> The i8042 still does not work. I've applied the same changes to Crown
> Bay. i8042 does not work either. Could you please have a look?
>

It's hard for me to debug this on hardware. I should be able to use
qemu though. Is there anything special needed to make it use that
keyboard?

Regards,
Simon
Simon Glass Oct. 19, 2015, 1:53 a.m. UTC | #2 
Hi Bin,

On 18 October 2015 at 17:17, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 15 September 2015 at 00:12, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Simon,
>>
>> On Wed, Sep 9, 2015 at 12:32 PM, Simon Glass <sjg@chromium.org> wrote:
>>> Adjust this driver to support driver model. The only users are x86 boards
>>> so this should be safe.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>
>> After updating QEMU to use the DM i8042 driver below:
>>
>> diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts
>> index fc74cd0..14782b3 100644
>> --- a/arch/x86/dts/qemu-x86_i440fx.dts
>> +++ b/arch/x86/dts/qemu-x86_i440fx.dts
>> @@ -11,6 +11,7 @@
>>  /include/ "skeleton.dtsi"
>>  /include/ "serial.dtsi"
>>  /include/ "rtc.dtsi"
>> +/include/ "keyboard.dtsi"
>>
>>  / {
>>         model = "QEMU x86 (I440FX)";
>> diff --git a/include/configs/qemu-x86.h b/include/configs/qemu-x86.h
>> index 1b544c1..9daf943 100644
>> --- a/include/configs/qemu-x86.h
>> +++ b/include/configs/qemu-x86.h
>> @@ -30,7 +30,7 @@
>>
>>  #define CONFIG_PCI_PNP
>>
>> -#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,vga\0" \
>> +#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,i8042-kbd\0" \
>>                                         "stdout=serial,vga\0" \
>>                                         "stderr=serial,vga\0"
>>
>>
>> The i8042 still does not work. I've applied the same changes to Crown
>> Bay. i8042 does not work either. Could you please have a look?
>>
>
> It's hard for me to debug this on hardware. I should be able to use
> qemu though. Is there anything special needed to make it use that
> keyboard?

Actually it works for me with qemu.

However it is intermittent, as it was before my series. So there is
something else going on.

Regards,
Simon
Bin Meng Oct. 19, 2015, 3:01 a.m. UTC | #3 
Hi Simon,

On Mon, Oct 19, 2015 at 7:17 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 15 September 2015 at 00:12, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Simon,
>>
>> On Wed, Sep 9, 2015 at 12:32 PM, Simon Glass <sjg@chromium.org> wrote:
>>> Adjust this driver to support driver model. The only users are x86 boards
>>> so this should be safe.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>
>> After updating QEMU to use the DM i8042 driver below:
>>
>> diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts
>> index fc74cd0..14782b3 100644
>> --- a/arch/x86/dts/qemu-x86_i440fx.dts
>> +++ b/arch/x86/dts/qemu-x86_i440fx.dts
>> @@ -11,6 +11,7 @@
>>  /include/ "skeleton.dtsi"
>>  /include/ "serial.dtsi"
>>  /include/ "rtc.dtsi"
>> +/include/ "keyboard.dtsi"
>>
>>  / {
>>         model = "QEMU x86 (I440FX)";
>> diff --git a/include/configs/qemu-x86.h b/include/configs/qemu-x86.h
>> index 1b544c1..9daf943 100644
>> --- a/include/configs/qemu-x86.h
>> +++ b/include/configs/qemu-x86.h
>> @@ -30,7 +30,7 @@
>>
>>  #define CONFIG_PCI_PNP
>>
>> -#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,vga\0" \
>> +#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,i8042-kbd\0" \
>>                                         "stdout=serial,vga\0" \
>>                                         "stderr=serial,vga\0"
>>
>>
>> The i8042 still does not work. I've applied the same changes to Crown
>> Bay. i8042 does not work either. Could you please have a look?
>>
>
> It's hard for me to debug this on hardware. I should be able to use
> qemu though. Is there anything special needed to make it use that
> keyboard?
>

I will have another try (and debug if it is needed) on Crown Bay when
you send the v2 series.

Regards,
Bin
Simon Glass Oct. 19, 2015, 3:07 a.m. UTC | #4 
Hi Bin,

On 18 October 2015 at 21:01, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Mon, Oct 19, 2015 at 7:17 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Bin,
>>
>> On 15 September 2015 at 00:12, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Wed, Sep 9, 2015 at 12:32 PM, Simon Glass <sjg@chromium.org> wrote:
>>>> Adjust this driver to support driver model. The only users are x86 boards
>>>> so this should be safe.
>>>>
>>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>
>>> After updating QEMU to use the DM i8042 driver below:
>>>
>>> diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts
>>> index fc74cd0..14782b3 100644
>>> --- a/arch/x86/dts/qemu-x86_i440fx.dts
>>> +++ b/arch/x86/dts/qemu-x86_i440fx.dts
>>> @@ -11,6 +11,7 @@
>>>  /include/ "skeleton.dtsi"
>>>  /include/ "serial.dtsi"
>>>  /include/ "rtc.dtsi"
>>> +/include/ "keyboard.dtsi"
>>>
>>>  / {
>>>         model = "QEMU x86 (I440FX)";
>>> diff --git a/include/configs/qemu-x86.h b/include/configs/qemu-x86.h
>>> index 1b544c1..9daf943 100644
>>> --- a/include/configs/qemu-x86.h
>>> +++ b/include/configs/qemu-x86.h
>>> @@ -30,7 +30,7 @@
>>>
>>>  #define CONFIG_PCI_PNP
>>>
>>> -#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,vga\0" \
>>> +#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,i8042-kbd\0" \
>>>                                         "stdout=serial,vga\0" \
>>>                                         "stderr=serial,vga\0"
>>>
>>>
>>> The i8042 still does not work. I've applied the same changes to Crown
>>> Bay. i8042 does not work either. Could you please have a look?
>>>
>>
>> It's hard for me to debug this on hardware. I should be able to use
>> qemu though. Is there anything special needed to make it use that
>> keyboard?
>>
>
> I will have another try (and debug if it is needed) on Crown Bay when
> you send the v2 series.

Thanks - also note that I have not fixed the hang on caps lock that
you reported.

Regards,
Simon
diff mbox

Patch

diff --git a/arch/x86/dts/qemu-x86_i440fx.dts b/arch/x86/dts/qemu-x86_i440fx.dts
index fc74cd0..14782b3 100644
--- a/arch/x86/dts/qemu-x86_i440fx.dts
+++ b/arch/x86/dts/qemu-x86_i440fx.dts
@@ -11,6 +11,7 @@ 
 /include/ "skeleton.dtsi"
 /include/ "serial.dtsi"
 /include/ "rtc.dtsi"
+/include/ "keyboard.dtsi"

 / {
        model = "QEMU x86 (I440FX)";
diff --git a/include/configs/qemu-x86.h b/include/configs/qemu-x86.h
index 1b544c1..9daf943 100644
--- a/include/configs/qemu-x86.h
+++ b/include/configs/qemu-x86.h
@@ -30,7 +30,7 @@ 

 #define CONFIG_PCI_PNP

-#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,vga\0" \
+#define CONFIG_STD_DEVICES_SETTINGS    "stdin=serial,i8042-kbd\0" \
                                        "stdout=serial,vga\0" \
                                        "stderr=serial,vga\0"