* Re: [PATCH v2] Input: atmel_mxt_ts - fix -Wunused-const-variable
From: Dmitry Torokhov @ 2019-07-01 8:19 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Huckleberry, nick, linux-input, LKML, clang-built-linux
In-Reply-To: <CAKwvOd=252Ak-VQ20XtsGaRXEfraxtNTNjhjYfdrsWv_7OHsoQ@mail.gmail.com>
On Thu, Jun 13, 2019 at 11:26:41AM -0700, Nick Desaulniers wrote:
> On Thu, Jun 13, 2019 at 11:24 AM 'Nathan Huckleberry' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> > Changes from v1 -> v2
> > * Moved definition of mxt_video_fops into existing ifdef
>
> Thanks for the v2.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Applied, thank you.
>
> > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > @@ -256,16 +256,6 @@ enum v4l_dbg_inputs {
> > MXT_V4L_INPUT_MAX,
> > };
> >
> > -static const struct v4l2_file_operations mxt_video_fops = {
> > - .owner = THIS_MODULE,
> > - .open = v4l2_fh_open,
> > - .release = vb2_fop_release,
> > - .unlocked_ioctl = video_ioctl2,
> > - .read = vb2_fop_read,
> > - .mmap = vb2_fop_mmap,
> > - .poll = vb2_fop_poll,
> > -};
> > -
> > enum mxt_suspend_mode {
> > MXT_SUSPEND_DEEP_SLEEP = 0,
> > MXT_SUSPEND_T9_CTRL = 1,
> > @@ -2218,6 +2208,16 @@ static int mxt_init_t7_power_cfg(struct mxt_data *data)
> > }
> >
> > #ifdef CONFIG_TOUCHSCREEN_ATMEL_MXT_T37
> > +static const struct v4l2_file_operations mxt_video_fops = {
> > + .owner = THIS_MODULE,
> > + .open = v4l2_fh_open,
> > + .release = vb2_fop_release,
> > + .unlocked_ioctl = video_ioctl2,
> > + .read = vb2_fop_read,
> > + .mmap = vb2_fop_mmap,
> > + .poll = vb2_fop_poll,
> > +};
> > +
>
> --
> Thanks,
> ~Nick Desaulniers
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: apple: Fix stuck function keys when using FN
From: Benjamin Tissoires @ 2019-07-01 8:32 UTC (permalink / raw)
To: João Moreno; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <CAHxFc3QC147B6j4pBztjK7stLgCveeYhJWojai_SbKNbnpC9yw@mail.gmail.com>
Hi João,
On Sun, Jun 30, 2019 at 10:15 PM João Moreno <mail@joaomoreno.com> wrote:
>
> Hi Jiri & Benjamin,
>
> Let me know if you need something else to get this patch moving forward. This
> fixes an issue I hit daily, it would be great to get it fixed.
Sorry for the delay, I am very busy with internal corporate stuff, and
I tried setting up a new CI system at home, and instead of spending a
couple of ours, I am down to 2 weeks of hard work, without possibility
to switch to the new right now :(
Anyway.
>
> Thanks.
>
> On Mon, 10 Jun 2019 at 23:31, Joao Moreno <mail@joaomoreno.com> wrote:
> >
> > This fixes an issue in which key down events for function keys would be
> > repeatedly emitted even after the user has raised the physical key. For
> > example, the driver fails to emit the F5 key up event when going through
> > the following steps:
> > - fnmode=1: hold FN, hold F5, release FN, release F5
> > - fnmode=2: hold F5, hold FN, release F5, release FN
Ouch :/
> >
> > The repeated F5 key down events can be easily verified using xev.
> >
> > Signed-off-by: Joao Moreno <mail@joaomoreno.com>
> > ---
> > drivers/hid/hid-apple.c | 21 +++++++++++----------
> > 1 file changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> > index 1cb41992aaa1..81867a6fa047 100644
> > --- a/drivers/hid/hid-apple.c
> > +++ b/drivers/hid/hid-apple.c
> > @@ -205,20 +205,21 @@ static int hidinput_apple_event(struct hid_device *hid, struct input_dev *input,
> > trans = apple_find_translation (table, usage->code);
> >
> > if (trans) {
> > - if (test_bit(usage->code, asc->pressed_fn))
> > - do_translate = 1;
> > - else if (trans->flags & APPLE_FLAG_FKEY)
> > - do_translate = (fnmode == 2 && asc->fn_on) ||
> > - (fnmode == 1 && !asc->fn_on);
> > + int fn_on = value ? asc->fn_on :
> > + test_bit(usage->code, asc->pressed_fn);
> > +
> > + if (!value)
> > + clear_bit(usage->code, asc->pressed_fn);
> > + else if (asc->fn_on)
> > + set_bit(usage->code, asc->pressed_fn);
I have the feeling that this is not the correct fix here.
I might be wrong, but the following sequence might also mess up the
driver state, depending on how the reports are emitted:
- hold FN, hold F4, hold F5, release F4, release FN, release F5
The reason is that the driver only considers you have one key pressed
with the modifier, and as the code changed its state based on the last
value.
IMO a better fix would:
- keep the existing `trans` mapping lookout
- whenever a `trans` mapping gets found:
* get both translated and non-translated currently reported values
(`test_bit(keycode, input_dev->key)`)
* if one of them is set to true, then consider the keycode to be the
one of the key (no matter fn_on)
-> deal with `value` with the corrected keycode
* if the key was not pressed:
-> chose the keycode based on `fn_on` and `fnmode` states
and report the key press event
This should remove the nasty pressed_fn state which depends on the
other pressed keys.
Cheers,
Benjamin
> > +
> > + if (trans->flags & APPLE_FLAG_FKEY)
> > + do_translate = (fnmode == 2 && fn_on) ||
> > + (fnmode == 1 && !fn_on);
> > else
> > do_translate = asc->fn_on;
> >
> > if (do_translate) {
> > - if (value)
> > - set_bit(usage->code, asc->pressed_fn);
> > - else
> > - clear_bit(usage->code, asc->pressed_fn);
> > -
> > input_event(input, usage->type, trans->to,
> > value);
> >
> > --
> > 2.19.1
> >
^ permalink raw reply
* Re: [PATCH 3/3] video: fbdev: don't print error message on framebuffer_alloc() failure
From: Benjamin Tissoires @ 2019-07-01 8:37 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: linux-fbdev, Jiri Kosina, lkml, dri-devel, Bruno Prémont,
open list:HID CORE LAYER
In-Reply-To: <3da197ed-a701-2aa7-d775-2bdbe9deab4a@samsung.com>
Hi Bartlomiej,
On Fri, Jun 14, 2019 at 4:52 PM Bartlomiej Zolnierkiewicz
<b.zolnierkie@samsung.com> wrote:
>
> framebuffer_alloc() can fail only on kzalloc() memory allocation
> failure and since kzalloc() will print error message in such case
> we can omit printing extra error message in drivers (which BTW is
> what the majority of framebuffer_alloc() users is doing already).
>
> Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
> drivers/hid/hid-picolcd_fb.c | 4 +---
> drivers/video/fbdev/amifb.c | 4 +---
> drivers/video/fbdev/arkfb.c | 4 +---
> drivers/video/fbdev/atmel_lcdfb.c | 4 +---
> drivers/video/fbdev/aty/aty128fb.c | 5 ++---
> drivers/video/fbdev/aty/atyfb_base.c | 10 ++++------
> drivers/video/fbdev/aty/radeon_base.c | 2 --
> drivers/video/fbdev/chipsfb.c | 1 -
> drivers/video/fbdev/cirrusfb.c | 5 +----
> drivers/video/fbdev/da8xx-fb.c | 1 -
> drivers/video/fbdev/efifb.c | 1 -
> drivers/video/fbdev/grvga.c | 4 +---
> drivers/video/fbdev/gxt4500.c | 5 ++---
> drivers/video/fbdev/hyperv_fb.c | 4 +---
> drivers/video/fbdev/i740fb.c | 4 +---
> drivers/video/fbdev/imsttfb.c | 5 +----
> drivers/video/fbdev/intelfb/intelfbdrv.c | 5 ++---
> drivers/video/fbdev/jz4740_fb.c | 4 +---
> drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 5 +----
> drivers/video/fbdev/mbx/mbxfb.c | 4 +---
> drivers/video/fbdev/omap/omapfb_main.c | 2 --
> drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 6 +-----
> drivers/video/fbdev/platinumfb.c | 5 ++---
> drivers/video/fbdev/pmag-aa-fb.c | 4 +---
> drivers/video/fbdev/pmag-ba-fb.c | 4 +---
> drivers/video/fbdev/pmagb-b-fb.c | 4 +---
> drivers/video/fbdev/pvr2fb.c | 6 +-----
> drivers/video/fbdev/riva/fbdev.c | 1 -
> drivers/video/fbdev/s3c-fb.c | 4 +---
> drivers/video/fbdev/s3fb.c | 4 +---
> drivers/video/fbdev/sh_mobile_lcdcfb.c | 8 ++------
> drivers/video/fbdev/sm501fb.c | 4 +---
> drivers/video/fbdev/sm712fb.c | 1 -
> drivers/video/fbdev/smscufx.c | 4 +---
> drivers/video/fbdev/ssd1307fb.c | 4 +---
> drivers/video/fbdev/sunxvr1000.c | 1 -
> drivers/video/fbdev/sunxvr2500.c | 1 -
> drivers/video/fbdev/sunxvr500.c | 1 -
> drivers/video/fbdev/tgafb.c | 4 +---
> drivers/video/fbdev/udlfb.c | 4 +---
> drivers/video/fbdev/via/viafbdev.c | 6 +-----
> drivers/video/fbdev/vt8623fb.c | 4 +---
> 42 files changed, 40 insertions(+), 123 deletions(-)
>
> Index: b/drivers/hid/hid-picolcd_fb.c
> ===================================================================
> --- a/drivers/hid/hid-picolcd_fb.c
> +++ b/drivers/hid/hid-picolcd_fb.c
> @@ -522,10 +522,8 @@ int picolcd_init_framebuffer(struct pico
> sizeof(struct fb_deferred_io) +
> sizeof(struct picolcd_fb_data) +
> PICOLCDFB_SIZE, dev);
> - if (info == NULL) {
> - dev_err(dev, "failed to allocate a framebuffer\n");
> + if (!info)
> goto err_nomem;
> - }
It would have been better to split this change as the HID and fbdev
are different trees.
However, I do not expect a conflict here (there hasn't been updates of
hid-picolcd_fb.c in a while), so feel free to take this patch through
the fbdev tree with my:
Acked-By: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
>
> info->fbdefio = info->par;
> *info->fbdefio = picolcd_fb_defio;
> Index: b/drivers/video/fbdev/amifb.c
> ===================================================================
> --- a/drivers/video/fbdev/amifb.c
> +++ b/drivers/video/fbdev/amifb.c
> @@ -3554,10 +3554,8 @@ static int __init amifb_probe(struct pla
> custom.dmacon = DMAF_ALL | DMAF_MASTER;
>
> info = framebuffer_alloc(sizeof(struct amifb_par), &pdev->dev);
> - if (!info) {
> - dev_err(&pdev->dev, "framebuffer_alloc failed\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> strcpy(info->fix.id, "Amiga ");
> info->fix.visual = FB_VISUAL_PSEUDOCOLOR;
> Index: b/drivers/video/fbdev/arkfb.c
> ===================================================================
> --- a/drivers/video/fbdev/arkfb.c
> +++ b/drivers/video/fbdev/arkfb.c
> @@ -954,10 +954,8 @@ static int ark_pci_probe(struct pci_dev
>
> /* Allocate and fill driver data structure */
> info = framebuffer_alloc(sizeof(struct arkfb_info), &(dev->dev));
> - if (! info) {
> - dev_err(&(dev->dev), "cannot allocate memory\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> mutex_init(&par->open_lock);
> Index: b/drivers/video/fbdev/atmel_lcdfb.c
> ===================================================================
> --- a/drivers/video/fbdev/atmel_lcdfb.c
> +++ b/drivers/video/fbdev/atmel_lcdfb.c
> @@ -1053,10 +1053,8 @@ static int __init atmel_lcdfb_probe(stru
>
> ret = -ENOMEM;
> info = framebuffer_alloc(sizeof(struct atmel_lcdfb_info), dev);
> - if (!info) {
> - dev_err(dev, "cannot allocate memory\n");
> + if (!info)
> goto out;
> - }
>
> sinfo = info->par;
> sinfo->pdev = pdev;
> Index: b/drivers/video/fbdev/aty/aty128fb.c
> ===================================================================
> --- a/drivers/video/fbdev/aty/aty128fb.c
> +++ b/drivers/video/fbdev/aty/aty128fb.c
> @@ -2102,10 +2102,9 @@ static int aty128_probe(struct pci_dev *
>
> /* We have the resources. Now virtualize them */
> info = framebuffer_alloc(sizeof(struct aty128fb_par), &pdev->dev);
> - if (info == NULL) {
> - printk(KERN_ERR "aty128fb: can't alloc fb_info_aty128\n");
> + if (!info)
> goto err_free_mmio;
> - }
> +
> par = info->par;
>
> info->pseudo_palette = par->pseudo_palette;
> Index: b/drivers/video/fbdev/aty/atyfb_base.c
> ===================================================================
> --- a/drivers/video/fbdev/aty/atyfb_base.c
> +++ b/drivers/video/fbdev/aty/atyfb_base.c
> @@ -3550,10 +3550,9 @@ static int atyfb_pci_probe(struct pci_de
>
> /* Allocate framebuffer */
> info = framebuffer_alloc(sizeof(struct atyfb_par), &pdev->dev);
> - if (!info) {
> - PRINTKE("atyfb_pci_probe() can't alloc fb_info\n");
> + if (!info)
> return -ENOMEM;
> - }
> +
> par = info->par;
> par->bus_type = PCI;
> info->fix = atyfb_fix;
> @@ -3643,10 +3642,9 @@ static int __init atyfb_atari_probe(void
> }
>
> info = framebuffer_alloc(sizeof(struct atyfb_par), NULL);
> - if (!info) {
> - PRINTKE("atyfb_atari_probe() can't alloc fb_info\n");
> + if (!info)
> return -ENOMEM;
> - }
> +
> par = info->par;
>
> info->fix = atyfb_fix;
> Index: b/drivers/video/fbdev/aty/radeon_base.c
> ===================================================================
> --- a/drivers/video/fbdev/aty/radeon_base.c
> +++ b/drivers/video/fbdev/aty/radeon_base.c
> @@ -2294,8 +2294,6 @@ static int radeonfb_pci_register(struct
>
> info = framebuffer_alloc(sizeof(struct radeonfb_info), &pdev->dev);
> if (!info) {
> - printk (KERN_ERR "radeonfb (%s): could not allocate memory\n",
> - pci_name(pdev));
> ret = -ENOMEM;
> goto err_disable;
> }
> Index: b/drivers/video/fbdev/chipsfb.c
> ===================================================================
> --- a/drivers/video/fbdev/chipsfb.c
> +++ b/drivers/video/fbdev/chipsfb.c
> @@ -366,7 +366,6 @@ static int chipsfb_pci_init(struct pci_d
>
> p = framebuffer_alloc(0, &dp->dev);
> if (p == NULL) {
> - dev_err(&dp->dev, "Cannot allocate framebuffer structure\n");
> rc = -ENOMEM;
> goto err_disable;
> }
> Index: b/drivers/video/fbdev/cirrusfb.c
> ===================================================================
> --- a/drivers/video/fbdev/cirrusfb.c
> +++ b/drivers/video/fbdev/cirrusfb.c
> @@ -2093,7 +2093,6 @@ static int cirrusfb_pci_register(struct
>
> info = framebuffer_alloc(sizeof(struct cirrusfb_info), &pdev->dev);
> if (!info) {
> - printk(KERN_ERR "cirrusfb: could not allocate memory\n");
> ret = -ENOMEM;
> goto err_out;
> }
> @@ -2206,10 +2205,8 @@ static int cirrusfb_zorro_register(struc
> struct cirrusfb_info *cinfo;
>
> info = framebuffer_alloc(sizeof(struct cirrusfb_info), &z->dev);
> - if (!info) {
> - printk(KERN_ERR "cirrusfb: could not allocate memory\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> zcl = (const struct zorrocl *)ent->driver_data;
> btype = zcl->type;
> Index: b/drivers/video/fbdev/da8xx-fb.c
> ===================================================================
> --- a/drivers/video/fbdev/da8xx-fb.c
> +++ b/drivers/video/fbdev/da8xx-fb.c
> @@ -1400,7 +1400,6 @@ static int fb_probe(struct platform_devi
> da8xx_fb_info = framebuffer_alloc(sizeof(struct da8xx_fb_par),
> &device->dev);
> if (!da8xx_fb_info) {
> - dev_dbg(&device->dev, "Memory allocation failed for fb_info\n");
> ret = -ENOMEM;
> goto err_pm_runtime_disable;
> }
> Index: b/drivers/video/fbdev/efifb.c
> ===================================================================
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -448,7 +448,6 @@ static int efifb_probe(struct platform_d
>
> info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
> if (!info) {
> - pr_err("efifb: cannot allocate framebuffer\n");
> err = -ENOMEM;
> goto err_release_mem;
> }
> Index: b/drivers/video/fbdev/grvga.c
> ===================================================================
> --- a/drivers/video/fbdev/grvga.c
> +++ b/drivers/video/fbdev/grvga.c
> @@ -341,10 +341,8 @@ static int grvga_probe(struct platform_d
> char *options = NULL, *mode_opt = NULL;
>
> info = framebuffer_alloc(sizeof(struct grvga_par), &dev->dev);
> - if (!info) {
> - dev_err(&dev->dev, "framebuffer_alloc failed\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> /* Expecting: "grvga: modestring, [addr:<framebuffer physical address>], [size:<framebuffer size>]
> *
> Index: b/drivers/video/fbdev/gxt4500.c
> ===================================================================
> --- a/drivers/video/fbdev/gxt4500.c
> +++ b/drivers/video/fbdev/gxt4500.c
> @@ -642,10 +642,9 @@ static int gxt4500_probe(struct pci_dev
> }
>
> info = framebuffer_alloc(sizeof(struct gxt4500_par), &pdev->dev);
> - if (!info) {
> - dev_err(&pdev->dev, "gxt4500: cannot alloc FB info record\n");
> + if (!info)
> goto err_free_fb;
> - }
> +
> par = info->par;
> cardtype = ent->driver_data;
> par->refclk_ps = cardinfo[cardtype].refclk_ps;
> Index: b/drivers/video/fbdev/hyperv_fb.c
> ===================================================================
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -771,10 +771,8 @@ static int hvfb_probe(struct hv_device *
> int ret;
>
> info = framebuffer_alloc(sizeof(struct hvfb_par), &hdev->device);
> - if (!info) {
> - pr_err("No memory for framebuffer info\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> par->info = info;
> Index: b/drivers/video/fbdev/i740fb.c
> ===================================================================
> --- a/drivers/video/fbdev/i740fb.c
> +++ b/drivers/video/fbdev/i740fb.c
> @@ -1005,10 +1005,8 @@ static int i740fb_probe(struct pci_dev *
> u8 *edid;
>
> info = framebuffer_alloc(sizeof(struct i740fb_par), &(dev->dev));
> - if (!info) {
> - dev_err(&(dev->dev), "cannot allocate framebuffer\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> mutex_init(&par->open_lock);
> Index: b/drivers/video/fbdev/imsttfb.c
> ===================================================================
> --- a/drivers/video/fbdev/imsttfb.c
> +++ b/drivers/video/fbdev/imsttfb.c
> @@ -1477,11 +1477,8 @@ static int imsttfb_probe(struct pci_dev
> printk(KERN_ERR "imsttfb: no OF node for pci device\n");
>
> info = framebuffer_alloc(sizeof(struct imstt_par), &pdev->dev);
> -
> - if (!info) {
> - printk(KERN_ERR "imsttfb: Can't allocate memory\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
>
> Index: b/drivers/video/fbdev/intelfb/intelfbdrv.c
> ===================================================================
> --- a/drivers/video/fbdev/intelfb/intelfbdrv.c
> +++ b/drivers/video/fbdev/intelfb/intelfbdrv.c
> @@ -491,10 +491,9 @@ static int intelfb_pci_register(struct p
> }
>
> info = framebuffer_alloc(sizeof(struct intelfb_info), &pdev->dev);
> - if (!info) {
> - ERR_MSG("Could not allocate memory for intelfb_info.\n");
> + if (!info)
> return -ENOMEM;
> - }
> +
> if (fb_alloc_cmap(&info->cmap, 256, 1) < 0) {
> ERR_MSG("Could not allocate cmap for intelfb_info.\n");
> goto err_out_cmap;
> Index: b/drivers/video/fbdev/jz4740_fb.c
> ===================================================================
> --- a/drivers/video/fbdev/jz4740_fb.c
> +++ b/drivers/video/fbdev/jz4740_fb.c
> @@ -544,10 +544,8 @@ static int jzfb_probe(struct platform_de
> }
>
> fb = framebuffer_alloc(sizeof(struct jzfb), &pdev->dev);
> - if (!fb) {
> - dev_err(&pdev->dev, "Failed to allocate framebuffer device\n");
> + if (!fb)
> return -ENOMEM;
> - }
>
> fb->fbops = &jzfb_ops;
> fb->flags = FBINFO_DEFAULT;
> Index: b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> ===================================================================
> --- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> +++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
> @@ -684,10 +684,8 @@ static int of_platform_mb862xx_probe(str
> }
>
> info = framebuffer_alloc(sizeof(struct mb862xxfb_par), dev);
> - if (info == NULL) {
> - dev_err(dev, "cannot allocate framebuffer\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> par->info = info;
> @@ -1009,7 +1007,6 @@ static int mb862xx_pci_probe(struct pci_
>
> info = framebuffer_alloc(sizeof(struct mb862xxfb_par), dev);
> if (!info) {
> - dev_err(dev, "framebuffer alloc failed\n");
> ret = -ENOMEM;
> goto dis_dev;
> }
> Index: b/drivers/video/fbdev/mbx/mbxfb.c
> ===================================================================
> --- a/drivers/video/fbdev/mbx/mbxfb.c
> +++ b/drivers/video/fbdev/mbx/mbxfb.c
> @@ -899,10 +899,8 @@ static int mbxfb_probe(struct platform_d
> }
>
> fbi = framebuffer_alloc(sizeof(struct mbxfb_info), &dev->dev);
> - if (fbi == NULL) {
> - dev_err(&dev->dev, "framebuffer_alloc failed\n");
> + if (!fbi)
> return -ENOMEM;
> - }
>
> mfbi = fbi->par;
> fbi->pseudo_palette = mfbi->pseudo_palette;
> Index: b/drivers/video/fbdev/omap/omapfb_main.c
> ===================================================================
> --- a/drivers/video/fbdev/omap/omapfb_main.c
> +++ b/drivers/video/fbdev/omap/omapfb_main.c
> @@ -1515,8 +1515,6 @@ static int planes_init(struct omapfb_dev
> fbi = framebuffer_alloc(sizeof(struct omapfb_plane_struct),
> fbdev->dev);
> if (fbi == NULL) {
> - dev_err(fbdev->dev,
> - "unable to allocate memory for plane info\n");
> planes_cleanup(fbdev);
> return -ENOMEM;
> }
> Index: b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> ===================================================================
> --- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> @@ -1892,12 +1892,8 @@ static int omapfb_create_framebuffers(st
>
> fbi = framebuffer_alloc(sizeof(struct omapfb_info),
> fbdev->dev);
> -
> - if (fbi == NULL) {
> - dev_err(fbdev->dev,
> - "unable to allocate memory for plane info\n");
> + if (!fbi)
> return -ENOMEM;
> - }
>
> clear_fb_info(fbi);
>
> Index: b/drivers/video/fbdev/platinumfb.c
> ===================================================================
> --- a/drivers/video/fbdev/platinumfb.c
> +++ b/drivers/video/fbdev/platinumfb.c
> @@ -538,10 +538,9 @@ static int platinumfb_probe(struct platf
> dev_info(&odev->dev, "Found Apple Platinum video hardware\n");
>
> info = framebuffer_alloc(sizeof(*pinfo), &odev->dev);
> - if (info == NULL) {
> - dev_err(&odev->dev, "Failed to allocate fbdev !\n");
> + if (!info)
> return -ENOMEM;
> - }
> +
> pinfo = info->par;
>
> if (of_address_to_resource(dp, 0, &pinfo->rsrc_reg) ||
> Index: b/drivers/video/fbdev/pmag-aa-fb.c
> ===================================================================
> --- a/drivers/video/fbdev/pmag-aa-fb.c
> +++ b/drivers/video/fbdev/pmag-aa-fb.c
> @@ -165,10 +165,8 @@ static int pmagaafb_probe(struct device
> int err;
>
> info = framebuffer_alloc(sizeof(struct aafb_par), dev);
> - if (!info) {
> - printk(KERN_ERR "%s: Cannot allocate memory\n", dev_name(dev));
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> dev_set_drvdata(dev, info);
> Index: b/drivers/video/fbdev/pmag-ba-fb.c
> ===================================================================
> --- a/drivers/video/fbdev/pmag-ba-fb.c
> +++ b/drivers/video/fbdev/pmag-ba-fb.c
> @@ -150,10 +150,8 @@ static int pmagbafb_probe(struct device
> int err;
>
> info = framebuffer_alloc(sizeof(struct pmagbafb_par), dev);
> - if (!info) {
> - printk(KERN_ERR "%s: Cannot allocate memory\n", dev_name(dev));
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> dev_set_drvdata(dev, info);
> Index: b/drivers/video/fbdev/pmagb-b-fb.c
> ===================================================================
> --- a/drivers/video/fbdev/pmagb-b-fb.c
> +++ b/drivers/video/fbdev/pmagb-b-fb.c
> @@ -257,10 +257,8 @@ static int pmagbbfb_probe(struct device
> int err;
>
> info = framebuffer_alloc(sizeof(struct pmagbbfb_par), dev);
> - if (!info) {
> - printk(KERN_ERR "%s: Cannot allocate memory\n", dev_name(dev));
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> dev_set_drvdata(dev, info);
> Index: b/drivers/video/fbdev/pvr2fb.c
> ===================================================================
> --- a/drivers/video/fbdev/pvr2fb.c
> +++ b/drivers/video/fbdev/pvr2fb.c
> @@ -1069,12 +1069,8 @@ static int __init pvr2fb_init(void)
> #endif
>
> fb_info = framebuffer_alloc(sizeof(struct pvr2fb_par), NULL);
> -
> - if (!fb_info) {
> - printk(KERN_ERR "Failed to allocate memory for fb_info\n");
> + if (!fb_info)
> return -ENOMEM;
> - }
> -
>
> currentpar = fb_info->par;
>
> Index: b/drivers/video/fbdev/riva/fbdev.c
> ===================================================================
> --- a/drivers/video/fbdev/riva/fbdev.c
> +++ b/drivers/video/fbdev/riva/fbdev.c
> @@ -1902,7 +1902,6 @@ static int rivafb_probe(struct pci_dev *
>
> info = framebuffer_alloc(sizeof(struct riva_par), &pd->dev);
> if (!info) {
> - printk (KERN_ERR PFX "could not allocate memory\n");
> ret = -ENOMEM;
> goto err_ret;
> }
> Index: b/drivers/video/fbdev/s3c-fb.c
> ===================================================================
> --- a/drivers/video/fbdev/s3c-fb.c
> +++ b/drivers/video/fbdev/s3c-fb.c
> @@ -1189,10 +1189,8 @@ static int s3c_fb_probe_win(struct s3c_f
>
> fbinfo = framebuffer_alloc(sizeof(struct s3c_fb_win) +
> palette_size * sizeof(u32), sfb->dev);
> - if (!fbinfo) {
> - dev_err(sfb->dev, "failed to allocate framebuffer\n");
> + if (!fbinfo)
> return -ENOMEM;
> - }
>
> windata = sfb->pdata->win[win_no];
> initmode = *sfb->pdata->vtiming;
> Index: b/drivers/video/fbdev/s3fb.c
> ===================================================================
> --- a/drivers/video/fbdev/s3fb.c
> +++ b/drivers/video/fbdev/s3fb.c
> @@ -1128,10 +1128,8 @@ static int s3_pci_probe(struct pci_dev *
>
> /* Allocate and fill driver data structure */
> info = framebuffer_alloc(sizeof(struct s3fb_info), &(dev->dev));
> - if (!info) {
> - dev_err(&(dev->dev), "cannot allocate memory\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> mutex_init(&par->open_lock);
> Index: b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> ===================================================================
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> @@ -1644,10 +1644,8 @@ sh_mobile_lcdc_overlay_fb_init(struct sh
>
> /* Allocate and initialize the frame buffer device. */
> info = framebuffer_alloc(0, priv->dev);
> - if (info == NULL) {
> - dev_err(priv->dev, "unable to allocate fb_info\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> ovl->info = info;
>
> @@ -2138,10 +2136,8 @@ sh_mobile_lcdc_channel_fb_init(struct sh
> * list and allocate the color map.
> */
> info = framebuffer_alloc(0, priv->dev);
> - if (info == NULL) {
> - dev_err(priv->dev, "unable to allocate fb_info\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> ch->info = info;
>
> Index: b/drivers/video/fbdev/sm501fb.c
> ===================================================================
> --- a/drivers/video/fbdev/sm501fb.c
> +++ b/drivers/video/fbdev/sm501fb.c
> @@ -1868,10 +1868,8 @@ static int sm501fb_probe_one(struct sm50
> }
>
> fbi = framebuffer_alloc(sizeof(struct sm501fb_par), info->dev);
> - if (fbi == NULL) {
> - dev_err(info->dev, "cannot allocate %s framebuffer\n", name);
> + if (!fbi)
> return -ENOMEM;
> - }
>
> par = fbi->par;
> par->info = info;
> Index: b/drivers/video/fbdev/sm712fb.c
> ===================================================================
> --- a/drivers/video/fbdev/sm712fb.c
> +++ b/drivers/video/fbdev/sm712fb.c
> @@ -1538,7 +1538,6 @@ static int smtcfb_pci_probe(struct pci_d
>
> info = framebuffer_alloc(sizeof(*sfb), &pdev->dev);
> if (!info) {
> - dev_err(&pdev->dev, "framebuffer_alloc failed\n");
> err = -ENOMEM;
> goto failed_free;
> }
> Index: b/drivers/video/fbdev/smscufx.c
> ===================================================================
> --- a/drivers/video/fbdev/smscufx.c
> +++ b/drivers/video/fbdev/smscufx.c
> @@ -1653,10 +1653,8 @@ static int ufx_usb_probe(struct usb_inte
>
> /* allocates framebuffer driver structure, not framebuffer memory */
> info = framebuffer_alloc(0, &usbdev->dev);
> - if (!info) {
> - dev_err(dev->gdev, "framebuffer_alloc failed\n");
> + if (!info)
> goto e_nomem;
> - }
>
> dev->info = info;
> info->par = dev;
> Index: b/drivers/video/fbdev/ssd1307fb.c
> ===================================================================
> --- a/drivers/video/fbdev/ssd1307fb.c
> +++ b/drivers/video/fbdev/ssd1307fb.c
> @@ -556,10 +556,8 @@ static int ssd1307fb_probe(struct i2c_cl
> }
>
> info = framebuffer_alloc(sizeof(struct ssd1307fb_par), &client->dev);
> - if (!info) {
> - dev_err(&client->dev, "Couldn't allocate framebuffer.\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> par->info = info;
> Index: b/drivers/video/fbdev/sunxvr1000.c
> ===================================================================
> --- a/drivers/video/fbdev/sunxvr1000.c
> +++ b/drivers/video/fbdev/sunxvr1000.c
> @@ -121,7 +121,6 @@ static int gfb_probe(struct platform_dev
>
> info = framebuffer_alloc(sizeof(struct gfb_info), &op->dev);
> if (!info) {
> - printk(KERN_ERR "gfb: Cannot allocate fb_info\n");
> err = -ENOMEM;
> goto err_out;
> }
> Index: b/drivers/video/fbdev/sunxvr2500.c
> ===================================================================
> --- a/drivers/video/fbdev/sunxvr2500.c
> +++ b/drivers/video/fbdev/sunxvr2500.c
> @@ -132,7 +132,6 @@ static int s3d_pci_register(struct pci_d
>
> info = framebuffer_alloc(sizeof(struct s3d_info), &pdev->dev);
> if (!info) {
> - printk(KERN_ERR "s3d: Cannot allocate fb_info\n");
> err = -ENOMEM;
> goto err_disable;
> }
> Index: b/drivers/video/fbdev/sunxvr500.c
> ===================================================================
> --- a/drivers/video/fbdev/sunxvr500.c
> +++ b/drivers/video/fbdev/sunxvr500.c
> @@ -272,7 +272,6 @@ static int e3d_pci_register(struct pci_d
>
> info = framebuffer_alloc(sizeof(struct e3d_info), &pdev->dev);
> if (!info) {
> - printk(KERN_ERR "e3d: Cannot allocate fb_info\n");
> err = -ENOMEM;
> goto err_disable;
> }
> Index: b/drivers/video/fbdev/tgafb.c
> ===================================================================
> --- a/drivers/video/fbdev/tgafb.c
> +++ b/drivers/video/fbdev/tgafb.c
> @@ -1416,10 +1416,8 @@ static int tgafb_register(struct device
>
> /* Allocate the fb and par structures. */
> info = framebuffer_alloc(sizeof(struct tga_par), dev);
> - if (!info) {
> - printk(KERN_ERR "tgafb: Cannot allocate memory\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> dev_set_drvdata(dev, info);
> Index: b/drivers/video/fbdev/udlfb.c
> ===================================================================
> --- a/drivers/video/fbdev/udlfb.c
> +++ b/drivers/video/fbdev/udlfb.c
> @@ -1689,10 +1689,8 @@ static int dlfb_usb_probe(struct usb_int
>
> /* allocates framebuffer driver structure, not framebuffer memory */
> info = framebuffer_alloc(0, &dlfb->udev->dev);
> - if (!info) {
> - dev_err(&dlfb->udev->dev, "framebuffer_alloc failed\n");
> + if (!info)
> goto error;
> - }
>
> dlfb->info = info;
> info->par = dlfb;
> Index: b/drivers/video/fbdev/via/viafbdev.c
> ===================================================================
> --- a/drivers/video/fbdev/via/viafbdev.c
> +++ b/drivers/video/fbdev/via/viafbdev.c
> @@ -1756,10 +1756,8 @@ int via_fb_pci_probe(struct viafb_dev *v
> viafbinfo = framebuffer_alloc(viafb_par_length +
> ALIGN(sizeof(struct viafb_shared), BITS_PER_LONG/8),
> &vdev->pdev->dev);
> - if (!viafbinfo) {
> - printk(KERN_ERR"Could not allocate memory for viafb_info.\n");
> + if (!viafbinfo)
> return -ENOMEM;
> - }
>
> viaparinfo = (struct viafb_par *)viafbinfo->par;
> viaparinfo->shared = viafbinfo->par + viafb_par_length;
> @@ -1834,8 +1832,6 @@ int via_fb_pci_probe(struct viafb_dev *v
> viafbinfo1 = framebuffer_alloc(viafb_par_length,
> &vdev->pdev->dev);
> if (!viafbinfo1) {
> - printk(KERN_ERR
> - "allocate the second framebuffer struct error\n");
> rc = -ENOMEM;
> goto out_fb_release;
> }
> Index: b/drivers/video/fbdev/vt8623fb.c
> ===================================================================
> --- a/drivers/video/fbdev/vt8623fb.c
> +++ b/drivers/video/fbdev/vt8623fb.c
> @@ -669,10 +669,8 @@ static int vt8623_pci_probe(struct pci_d
>
> /* Allocate and fill driver data structure */
> info = framebuffer_alloc(sizeof(struct vt8623fb_info), &(dev->dev));
> - if (! info) {
> - dev_err(&(dev->dev), "cannot allocate memory\n");
> + if (!info)
> return -ENOMEM;
> - }
>
> par = info->par;
> mutex_init(&par->open_lock);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH] drm: bridge: DRM_SIL_SII8620 should depend on, not select INPUT
From: Andrzej Hajda @ 2019-07-01 9:26 UTC (permalink / raw)
To: Randy Dunlap, dri-devel, LKML, Ronald Tschalär
Cc: Inki Dae, Laurent Pinchart, linux-input@vger.kernel.org
In-Reply-To: <a7edece4-fec4-5811-27a9-ca6c275a4c40@samsung.com>
On 01.07.2019 11:23, Andrzej Hajda wrote:
> On 01.07.2019 05:39, Randy Dunlap wrote:
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> A single driver should not enable (select) an entire subsystem,
>> such as INPUT, so change the 'select' to "depends on".
>>
>> Fixes: d6abe6df706c ("drm/bridge: sil_sii8620: do not have a dependency of RC_CORE")
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Cc: Inki Dae <inki.dae@samsung.com>
>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
>> Cc: dri-devel@lists.freedesktop.org
>> ---
>> Linus has written this a couple of times in the last 15 years or so,
>> but my search fu cannot find it. And there are a few drivers in the
>> kernel tree that do this, but we shouldn't be adding more that do so.
>
> The proper solution has been already posted [1], but it has not been
> applied yet to input tree?
>
> Randy are there chances your patchset will appear in ML in near future,
> or should I merge your sii8620 patch alone?
Ups, I used wrong surname, I meant Ronald, added him input ML to cc.
Regards
Andrzej
>
>
> [1]:
> https://lore.kernel.org/lkml/20190419081926.13567-2-ronald@innovation.ch/
>
>
> Regards
>
> Andrzej
>
>
>
>> drivers/gpu/drm/bridge/Kconfig | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> --- lnx-52-rc7.orig/drivers/gpu/drm/bridge/Kconfig
>> +++ lnx-52-rc7/drivers/gpu/drm/bridge/Kconfig
>> @@ -83,10 +83,9 @@ config DRM_PARADE_PS8622
>>
>> config DRM_SIL_SII8620
>> tristate "Silicon Image SII8620 HDMI/MHL bridge"
>> - depends on OF
>> + depends on OF && INPUT
>> select DRM_KMS_HELPER
>> imply EXTCON
>> - select INPUT
>> select RC_CORE
>> help
>> Silicon Image SII8620 HDMI/MHL bridge chip driver.
>>
>>
>>
>>
^ permalink raw reply
* Re: [PATCH v3] HID: sb0540: add support for Creative SB0540 IR receivers
From: Benjamin Tissoires @ 2019-07-01 9:45 UTC (permalink / raw)
To: Bastien Nocera
Cc: open list:HID CORE LAYER, lkml, Jiri Kosina, Bastien Nocera
In-Reply-To: <20190626140618.8944-1-hadess@hadess.net>
Hi Bastien,
On Wed, Jun 26, 2019 at 4:07 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> From: Bastien Nocera <bnocera@redhat.com>
>
> Add a new hid driver for the Creative SB0540 IR receiver. This receiver
> is usually coupled with an RM-1500 or an RM-1800 remote control.
>
> The scrollwheels on the RM-1800 remote are not bound, as they are
> labelled for specific audio controls that don't usually exist on most
> systems. They can be remapped using standard Linux keyboard
> remapping tools.
Much better commit message :)
Thanks!
I have a few nitpicks however
>
> Signed-off-by: Bastien Nocera <bnocera@redhat.com>
> ---
> drivers/hid/Kconfig | 9 ++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-creative-sb0540.c | 254 ++++++++++++++++++++++++++++++
> drivers/hid/hid-ids.h | 1 +
> 4 files changed, 265 insertions(+)
> create mode 100644 drivers/hid/hid-creative-sb0540.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 3872e03d9a59..f16c4bd822e4 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -273,6 +273,15 @@ config HID_CP2112
> and gpiochip to expose these functions of the CP2112. The
> customizable USB descriptor fields are exposed as sysfs attributes.
>
> +config HID_CREATIVE_SB0540
> + tristate "Creative SB0540 infrared receiver"
> + depends on USB_HID
> + ---help---
checkpatch.pl complains here that new help texts should use "help" not
"---help---"
And BTW, checkpatch.pl complains *a lot*, so please fix most of the
warnings in the next submission (not sure you can go to 0 for a new
driver, but you should be able to get rid of most of them).
> + Support for Creative infrared SB0540-compatible remote controls, such
> + as the RM-1500 and RM-1800 remotes.
> +
> + Say Y here if you want support for Creative SB0540 infrared receiver.
> +
> config HID_CYPRESS
> tristate "Cypress mouse and barcode readers"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index cc5d827c9164..1ad662fe37b6 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_HID_ALPS) += hid-alps.o
> obj-$(CONFIG_HID_ACRUX) += hid-axff.o
> obj-$(CONFIG_HID_APPLE) += hid-apple.o
> obj-$(CONFIG_HID_APPLEIR) += hid-appleir.o
> +obj-$(CONFIG_HID_CREATIVE_SB0540) += hid-creative-sb0540.c
> obj-$(CONFIG_HID_ASUS) += hid-asus.o
> obj-$(CONFIG_HID_AUREAL) += hid-aureal.o
> obj-$(CONFIG_HID_BELKIN) += hid-belkin.o
> diff --git a/drivers/hid/hid-creative-sb0540.c b/drivers/hid/hid-creative-sb0540.c
> new file mode 100644
> index 000000000000..a94542cbdd33
> --- /dev/null
> +++ b/drivers/hid/hid-creative-sb0540.c
> @@ -0,0 +1,254 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * HID driver for the Creative SB0540 receiver
> + *
> + * Copyright (C) 2019 Red Hat Inc. All Rights Reserved
> + *
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include "hid-ids.h"
> +
> +MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
> +MODULE_DESCRIPTION("HID Creative SB0540 receiver");
> +MODULE_LICENSE("GPL");
> +
> +static const unsigned short creative_sb0540_key_table[] = {
> + KEY_POWER,
> + KEY_RESERVED, /* text: 24bit */
> + KEY_RESERVED, /* 24bit wheel up */
> + KEY_RESERVED, /* 24bit wheel down */
> + KEY_RESERVED, /* text: CMSS */
> + KEY_RESERVED, /* CMSS wheel Up */
> + KEY_RESERVED, /* CMSS wheel Down */
> + KEY_RESERVED, /* text: EAX */
> + KEY_RESERVED, /* EAX wheel up */
> + KEY_RESERVED, /* EAX wheel down */
> + KEY_RESERVED, /* text: 3D Midi */
> + KEY_RESERVED, /* 3D Midi wheel up */
> + KEY_RESERVED, /* 3D Midi wheel down */
> + KEY_MUTE,
> + KEY_VOLUMEUP,
> + KEY_VOLUMEDOWN,
> + KEY_UP,
> + KEY_LEFT,
> + KEY_RIGHT,
> + KEY_REWIND,
> + KEY_OK,
> + KEY_FASTFORWARD,
> + KEY_DOWN,
> + KEY_AGAIN, /* text: Return, symbol: Jump to */
> + KEY_PLAY, /* text: Start */
> + KEY_ESC, /* text: Cancel */
> + KEY_RECORD,
> + KEY_OPTION,
> + KEY_MENU, /* text: Display */
> + KEY_PREVIOUS,
> + KEY_PLAYPAUSE,
> + KEY_NEXT,
> + KEY_SLOW,
> + KEY_STOP,
> + KEY_NUMERIC_1,
> + KEY_NUMERIC_2,
> + KEY_NUMERIC_3,
> + KEY_NUMERIC_4,
> + KEY_NUMERIC_5,
> + KEY_NUMERIC_6,
> + KEY_NUMERIC_7,
> + KEY_NUMERIC_8,
> + KEY_NUMERIC_9,
> + KEY_NUMERIC_0
> +};
> +
> +/* Codes and keys from lirc's
> + * remotes/creative/lircd.conf.alsa_usb
> + * order and size must match creative_sb0540_key_table[] above */
Please follow the kernel coding style:
/*
* Codes and keys from lirc's
* remotes/creative/lircd.conf.alsa_usb
* order and size must match creative_sb0540_key_table[] above
*/
And same for all the multi-line comments.
> +static const unsigned short creative_sb0540_codes[] = {
> + 0x619E,
> + 0x916E,
> + 0x926D,
> + 0x936C,
> + 0x718E,
> + 0x946B,
> + 0x956A,
> + 0x8C73,
> + 0x9669,
> + 0x9768,
> + 0x9867,
> + 0x9966,
> + 0x9A65,
> + 0x6E91,
> + 0x629D,
> + 0x639C,
> + 0x7B84,
> + 0x6B94,
> + 0x728D,
> + 0x8778,
> + 0x817E,
> + 0x758A,
> + 0x8D72,
> + 0x8E71,
> + 0x8877,
> + 0x7C83,
> + 0x738C,
> + 0x827D,
> + 0x7689,
> + 0x7F80,
> + 0x7986,
> + 0x7A85,
> + 0x7D82,
> + 0x857A,
> + 0x8B74,
> + 0x8F70,
> + 0x906F,
> + 0x8A75,
> + 0x847B,
> + 0x7887,
> + 0x8976,
> + 0x837C,
> + 0x7788,
> + 0x807F
> +};
> +
> +struct creative_sb0540 {
> + struct input_dev *input_dev;
> + struct hid_device *hid;
> + unsigned short keymap[ARRAY_SIZE(creative_sb0540_key_table)];
> +};
> +
> +static inline u64 reverse(u64 data, int bits)
> +{
> + int i;
> + u64 c;
> +
> + c = 0;
> + for (i = 0; i < bits; i++) {
> + c |= (u64) (((data & (((u64) 1) << i)) ? 1 : 0)) << (bits - 1 - i);
> + }
> + return (c);
> +}
> +
> +static int get_key(struct creative_sb0540 *creative_sb0540, u64 keycode)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(creative_sb0540_codes); i++) {
> + if (creative_sb0540_codes[i] == keycode)
> + return creative_sb0540->keymap[i];
> + }
> +
> + return 0;
> +
> +}
> +
> +static int creative_sb0540_raw_event(struct hid_device *hid, struct hid_report *report,
> + u8 *data, int len)
> +{
> + struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
> + u64 code, main_code;
> + int key;
> +
> + if (len != 6)
> + goto out;
> +
> + /* From daemons/hw_hiddev.c sb0540_rec() in lirc */
> + code = reverse(data[5], 8);
> + main_code = (code << 8) + ((~code) & 0xff);
> +
> + /* Flip to get values in the same format as
> + * remotes/creative/lircd.conf.alsa_usb in lirc */
> + main_code = ((main_code & 0xff) << 8) + ((main_code & 0xff00) >> 8);
> +
> + key = get_key(creative_sb0540, main_code);
> + if (key == 0 || key == KEY_RESERVED) {
> + hid_err(hid, "Could not get a key for main_code %llX\n", main_code);
> + goto out;
I don't really like the got out here.
Why not changing the condition above to
if (key && key != KEY_RESERVED) and include the block below in the true case?
> + }
> +
> + input_report_key(creative_sb0540->input_dev, key, 1);
> + input_report_key(creative_sb0540->input_dev, key, 0);
> + input_sync(creative_sb0540->input_dev);
> +
> +out:
> + /* let hidraw and hiddev handle the report */
> + return 0;
> +}
> +
> +static int creative_sb0540_input_configured(struct hid_device *hid,
> + struct hid_input *hidinput)
> +{
> + struct input_dev *input_dev = hidinput->input;
> + struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
> + int i;
> +
> + creative_sb0540->input_dev = input_dev;
> +
> + input_dev->keycode = creative_sb0540->keymap;
> + input_dev->keycodesize = sizeof(unsigned short);
> + input_dev->keycodemax = ARRAY_SIZE(creative_sb0540->keymap);
> +
> + input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
> +
> + memcpy(creative_sb0540->keymap, creative_sb0540_key_table, sizeof(creative_sb0540->keymap));
> + for (i = 0; i < ARRAY_SIZE(creative_sb0540_key_table); i++)
> + set_bit(creative_sb0540->keymap[i], input_dev->keybit);
> + clear_bit(KEY_RESERVED, input_dev->keybit);
> +
> + return 0;
> +}
> +
> +static int creative_sb0540_input_mapping(struct hid_device *hid,
> + struct hid_input *hi, struct hid_field *field,
> + struct hid_usage *usage, unsigned long **bit, int *max)
> +{
Please add a comment here that we are remapping them ourselves, so we
can skip the hid-input processing.
> + return -1;
> +}
> +
> +static int creative_sb0540_probe(struct hid_device *hid, const struct hid_device_id *id)
> +{
> + int ret;
> + struct creative_sb0540 *creative_sb0540;
> +
> + creative_sb0540 = devm_kzalloc(&hid->dev, sizeof(struct creative_sb0540), GFP_KERNEL);
> + if (!creative_sb0540)
> + return -ENOMEM;
> +
> + creative_sb0540->hid = hid;
> +
> + /* force input as some remotes bypass the input registration */
> + hid->quirks |= HID_QUIRK_HIDINPUT_FORCE;
Does this still applies to your remote?
Cheers,
Benjamin
> +
> + hid_set_drvdata(hid, creative_sb0540);
> +
> + ret = hid_parse(hid);
> + if (ret) {
> + hid_err(hid, "parse failed\n");
> + return ret;
> + }
> +
> + ret = hid_hw_start(hid, HID_CONNECT_DEFAULT);
> + if (ret) {
> + hid_err(hid, "hw start failed\n");
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static const struct hid_device_id creative_sb0540_devices[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB0540) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, creative_sb0540_devices);
> +
> +static struct hid_driver creative_sb0540_driver = {
> + .name = "creative-sb0540",
> + .id_table = creative_sb0540_devices,
> + .raw_event = creative_sb0540_raw_event,
> + .input_configured = creative_sb0540_input_configured,
> + .probe = creative_sb0540_probe,
> + .input_mapping = creative_sb0540_input_mapping,
> +};
> +module_hid_driver(creative_sb0540_driver);
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 826324997686..206b7065da86 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -312,6 +312,7 @@
> #define USB_VENDOR_ID_CREATIVELABS 0x041e
> #define USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51 0x322c
> #define USB_DEVICE_ID_PRODIKEYS_PCMIDI 0x2801
> +#define USB_DEVICE_ID_CREATIVE_SB0540 0x3100
>
> #define USB_VENDOR_ID_CVTOUCH 0x1ff7
> #define USB_DEVICE_ID_CVTOUCH_SCREEN 0x0013
> --
> 2.21.0
>
^ permalink raw reply
* [PATCH v4] HID: sb0540: add support for Creative SB0540 IR receivers
From: Bastien Nocera @ 2019-07-01 10:08 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Bastien Nocera
From: Bastien Nocera <bnocera@redhat.com>
Add a new hid driver for the Creative SB0540 IR receiver. This receiver
is usually coupled with an RM-1500 or an RM-1800 remote control.
The scrollwheels on the RM-1800 remote are not bound, as they are
labelled for specific audio controls that don't usually exist on most
systems. They can be remapped using standard Linux keyboard
remapping tools.
Signed-off-by: Bastien Nocera <bnocera@redhat.com>
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 9 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-creative-sb0540.c | 268 ++++++++++++++++++++++++++++++
drivers/hid/hid-ids.h | 1 +
5 files changed, 285 insertions(+)
create mode 100644 drivers/hid/hid-creative-sb0540.c
diff --git a/MAINTAINERS b/MAINTAINERS
index d0ed735994a5..6fc022d152c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4227,6 +4227,12 @@ S: Maintained
F: Documentation/filesystems/cramfs.txt
F: fs/cramfs/
+CREATIVE SB0540
+M: Bastien Nocera <hadess@hadess.net>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-creative-sb0540.c
+
CRYPTO API
M: Herbert Xu <herbert@gondor.apana.org.au>
M: "David S. Miller" <davem@davemloft.net>
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 3872e03d9a59..a70999f9c639 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -273,6 +273,15 @@ config HID_CP2112
and gpiochip to expose these functions of the CP2112. The
customizable USB descriptor fields are exposed as sysfs attributes.
+config HID_CREATIVE_SB0540
+ tristate "Creative SB0540 infrared receiver"
+ depends on USB_HID
+ help
+ Support for Creative infrared SB0540-compatible remote controls, such
+ as the RM-1500 and RM-1800 remotes.
+
+ Say Y here if you want support for Creative SB0540 infrared receiver.
+
config HID_CYPRESS
tristate "Cypress mouse and barcode readers"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index cc5d827c9164..1ad662fe37b6 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_HID_ALPS) += hid-alps.o
obj-$(CONFIG_HID_ACRUX) += hid-axff.o
obj-$(CONFIG_HID_APPLE) += hid-apple.o
obj-$(CONFIG_HID_APPLEIR) += hid-appleir.o
+obj-$(CONFIG_HID_CREATIVE_SB0540) += hid-creative-sb0540.c
obj-$(CONFIG_HID_ASUS) += hid-asus.o
obj-$(CONFIG_HID_AUREAL) += hid-aureal.o
obj-$(CONFIG_HID_BELKIN) += hid-belkin.o
diff --git a/drivers/hid/hid-creative-sb0540.c b/drivers/hid/hid-creative-sb0540.c
new file mode 100644
index 000000000000..6b7c81ccf310
--- /dev/null
+++ b/drivers/hid/hid-creative-sb0540.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID driver for the Creative SB0540 receiver
+ *
+ * Copyright (C) 2019 Red Hat Inc. All Rights Reserved
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include "hid-ids.h"
+
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_DESCRIPTION("HID Creative SB0540 receiver");
+MODULE_LICENSE("GPL");
+
+static const unsigned short creative_sb0540_key_table[] = {
+ KEY_POWER,
+ KEY_RESERVED, /* text: 24bit */
+ KEY_RESERVED, /* 24bit wheel up */
+ KEY_RESERVED, /* 24bit wheel down */
+ KEY_RESERVED, /* text: CMSS */
+ KEY_RESERVED, /* CMSS wheel Up */
+ KEY_RESERVED, /* CMSS wheel Down */
+ KEY_RESERVED, /* text: EAX */
+ KEY_RESERVED, /* EAX wheel up */
+ KEY_RESERVED, /* EAX wheel down */
+ KEY_RESERVED, /* text: 3D Midi */
+ KEY_RESERVED, /* 3D Midi wheel up */
+ KEY_RESERVED, /* 3D Midi wheel down */
+ KEY_MUTE,
+ KEY_VOLUMEUP,
+ KEY_VOLUMEDOWN,
+ KEY_UP,
+ KEY_LEFT,
+ KEY_RIGHT,
+ KEY_REWIND,
+ KEY_OK,
+ KEY_FASTFORWARD,
+ KEY_DOWN,
+ KEY_AGAIN, /* text: Return, symbol: Jump to */
+ KEY_PLAY, /* text: Start */
+ KEY_ESC, /* text: Cancel */
+ KEY_RECORD,
+ KEY_OPTION,
+ KEY_MENU, /* text: Display */
+ KEY_PREVIOUS,
+ KEY_PLAYPAUSE,
+ KEY_NEXT,
+ KEY_SLOW,
+ KEY_STOP,
+ KEY_NUMERIC_1,
+ KEY_NUMERIC_2,
+ KEY_NUMERIC_3,
+ KEY_NUMERIC_4,
+ KEY_NUMERIC_5,
+ KEY_NUMERIC_6,
+ KEY_NUMERIC_7,
+ KEY_NUMERIC_8,
+ KEY_NUMERIC_9,
+ KEY_NUMERIC_0
+};
+
+/*
+ * Codes and keys from lirc's
+ * remotes/creative/lircd.conf.alsa_usb
+ * order and size must match creative_sb0540_key_table[] above
+ */
+static const unsigned short creative_sb0540_codes[] = {
+ 0x619E,
+ 0x916E,
+ 0x926D,
+ 0x936C,
+ 0x718E,
+ 0x946B,
+ 0x956A,
+ 0x8C73,
+ 0x9669,
+ 0x9768,
+ 0x9867,
+ 0x9966,
+ 0x9A65,
+ 0x6E91,
+ 0x629D,
+ 0x639C,
+ 0x7B84,
+ 0x6B94,
+ 0x728D,
+ 0x8778,
+ 0x817E,
+ 0x758A,
+ 0x8D72,
+ 0x8E71,
+ 0x8877,
+ 0x7C83,
+ 0x738C,
+ 0x827D,
+ 0x7689,
+ 0x7F80,
+ 0x7986,
+ 0x7A85,
+ 0x7D82,
+ 0x857A,
+ 0x8B74,
+ 0x8F70,
+ 0x906F,
+ 0x8A75,
+ 0x847B,
+ 0x7887,
+ 0x8976,
+ 0x837C,
+ 0x7788,
+ 0x807F
+};
+
+struct creative_sb0540 {
+ struct input_dev *input_dev;
+ struct hid_device *hid;
+ unsigned short keymap[ARRAY_SIZE(creative_sb0540_key_table)];
+};
+
+static inline u64 reverse(u64 data, int bits)
+{
+ int i;
+ u64 c;
+
+ c = 0;
+ for (i = 0; i < bits; i++) {
+ c |= (u64) (((data & (((u64) 1) << i)) ? 1 : 0))
+ << (bits - 1 - i);
+ }
+ return (c);
+}
+
+static int get_key(struct creative_sb0540 *creative_sb0540, u64 keycode)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(creative_sb0540_codes); i++) {
+ if (creative_sb0540_codes[i] == keycode)
+ return creative_sb0540->keymap[i];
+ }
+
+ return 0;
+
+}
+
+static int creative_sb0540_raw_event(struct hid_device *hid,
+ struct hid_report *report, u8 *data, int len)
+{
+ struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
+ u64 code, main_code;
+ int key;
+
+ if (len != 6)
+ goto out;
+
+ /* From daemons/hw_hiddev.c sb0540_rec() in lirc */
+ code = reverse(data[5], 8);
+ main_code = (code << 8) + ((~code) & 0xff);
+
+ /*
+ * Flip to get values in the same format as
+ * remotes/creative/lircd.conf.alsa_usb in lirc
+ */
+ main_code = ((main_code & 0xff) << 8) +
+ ((main_code & 0xff00) >> 8);
+
+ key = get_key(creative_sb0540, main_code);
+ if (key == 0 || key == KEY_RESERVED) {
+ hid_err(hid, "Could not get a key for main_code %llX\n",
+ main_code);
+ return 0;
+ }
+
+ input_report_key(creative_sb0540->input_dev, key, 1);
+ input_report_key(creative_sb0540->input_dev, key, 0);
+ input_sync(creative_sb0540->input_dev);
+
+ /* let hidraw and hiddev handle the report */
+ return 0;
+}
+
+static int creative_sb0540_input_configured(struct hid_device *hid,
+ struct hid_input *hidinput)
+{
+ struct input_dev *input_dev = hidinput->input;
+ struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
+ int i;
+
+ creative_sb0540->input_dev = input_dev;
+
+ input_dev->keycode = creative_sb0540->keymap;
+ input_dev->keycodesize = sizeof(unsigned short);
+ input_dev->keycodemax = ARRAY_SIZE(creative_sb0540->keymap);
+
+ input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
+
+ memcpy(creative_sb0540->keymap, creative_sb0540_key_table,
+ sizeof(creative_sb0540->keymap));
+ for (i = 0; i < ARRAY_SIZE(creative_sb0540_key_table); i++)
+ set_bit(creative_sb0540->keymap[i], input_dev->keybit);
+ clear_bit(KEY_RESERVED, input_dev->keybit);
+
+ return 0;
+}
+
+static int creative_sb0540_input_mapping(struct hid_device *hid,
+ struct hid_input *hi, struct hid_field *field,
+ struct hid_usage *usage, unsigned long **bit, int *max)
+{
+ /*
+ * We are remapping the keys ourselves, so ignore the hid-input
+ * keymap processing.
+ */
+ return -1;
+}
+
+static int creative_sb0540_probe(struct hid_device *hid,
+ const struct hid_device_id *id)
+{
+ int ret;
+ struct creative_sb0540 *creative_sb0540;
+
+ creative_sb0540 = devm_kzalloc(&hid->dev,
+ sizeof(struct creative_sb0540), GFP_KERNEL);
+
+ if (!creative_sb0540)
+ return -ENOMEM;
+
+ creative_sb0540->hid = hid;
+
+ /* force input as some remotes bypass the input registration */
+ hid->quirks |= HID_QUIRK_HIDINPUT_FORCE;
+
+ hid_set_drvdata(hid, creative_sb0540);
+
+ ret = hid_parse(hid);
+ if (ret) {
+ hid_err(hid, "parse failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hid, HID_CONNECT_DEFAULT);
+ if (ret) {
+ hid_err(hid, "hw start failed\n");
+ return ret;
+ }
+
+ return ret;
+}
+
+static const struct hid_device_id creative_sb0540_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB0540) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, creative_sb0540_devices);
+
+static struct hid_driver creative_sb0540_driver = {
+ .name = "creative-sb0540",
+ .id_table = creative_sb0540_devices,
+ .raw_event = creative_sb0540_raw_event,
+ .input_configured = creative_sb0540_input_configured,
+ .probe = creative_sb0540_probe,
+ .input_mapping = creative_sb0540_input_mapping,
+};
+module_hid_driver(creative_sb0540_driver);
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 826324997686..206b7065da86 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -312,6 +312,7 @@
#define USB_VENDOR_ID_CREATIVELABS 0x041e
#define USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51 0x322c
#define USB_DEVICE_ID_PRODIKEYS_PCMIDI 0x2801
+#define USB_DEVICE_ID_CREATIVE_SB0540 0x3100
#define USB_VENDOR_ID_CVTOUCH 0x1ff7
#define USB_DEVICE_ID_CVTOUCH_SCREEN 0x0013
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v3] HID: sb0540: add support for Creative SB0540 IR receivers
From: Bastien Nocera @ 2019-07-01 10:10 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: open list:HID CORE LAYER, lkml, Jiri Kosina, Bastien Nocera
In-Reply-To: <CAO-hwJJDf68gqgJZHQtAdjZcEzuL+LbOJD=OYHtod1xN6G+GjA@mail.gmail.com>
On Mon, 2019-07-01 at 11:45 +0200, Benjamin Tissoires wrote:
> Hi Bastien,
>
> On Wed, Jun 26, 2019 at 4:07 PM Bastien Nocera <hadess@hadess.net>
> wrote:
> > From: Bastien Nocera <bnocera@redhat.com>
> >
> > Add a new hid driver for the Creative SB0540 IR receiver. This
> > receiver
> > is usually coupled with an RM-1500 or an RM-1800 remote control.
> >
> > The scrollwheels on the RM-1800 remote are not bound, as they are
> > labelled for specific audio controls that don't usually exist on
> > most
> > systems. They can be remapped using standard Linux keyboard
> > remapping tools.
>
> Much better commit message :)
> Thanks!
>
> I have a few nitpicks however
Most of the checkpatch.pl warnings are now fixed. I ignored the one
about writing docs for the symbol (I have no idea what it wants, looks
like a false positive) and the too long line for the USB device match.
<snip>
> > + /* force input as some remotes bypass the input
> > registration */
> > + hid->quirks |= HID_QUIRK_HIDINPUT_FORCE;
>
> Does this still applies to your remote?
Yes, you mentioned it in your original review as well, and it was
necessary.
v4's been sent.
Thanks!
^ permalink raw reply
* Re: [PATCH v4] HID: sb0540: add support for Creative SB0540 IR receivers
From: Benjamin Tissoires @ 2019-07-01 10:15 UTC (permalink / raw)
To: Bastien Nocera
Cc: open list:HID CORE LAYER, lkml, Jiri Kosina, Bastien Nocera
In-Reply-To: <20190701100819.6032-1-hadess@hadess.net>
On Mon, Jul 1, 2019 at 12:08 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> From: Bastien Nocera <bnocera@redhat.com>
>
> Add a new hid driver for the Creative SB0540 IR receiver. This receiver
> is usually coupled with an RM-1500 or an RM-1800 remote control.
>
> The scrollwheels on the RM-1800 remote are not bound, as they are
> labelled for specific audio controls that don't usually exist on most
> systems. They can be remapped using standard Linux keyboard
> remapping tools.
>
> Signed-off-by: Bastien Nocera <bnocera@redhat.com>
> ---
> MAINTAINERS | 6 +
> drivers/hid/Kconfig | 9 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-creative-sb0540.c | 268 ++++++++++++++++++++++++++++++
> drivers/hid/hid-ids.h | 1 +
> 5 files changed, 285 insertions(+)
> create mode 100644 drivers/hid/hid-creative-sb0540.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d0ed735994a5..6fc022d152c8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4227,6 +4227,12 @@ S: Maintained
> F: Documentation/filesystems/cramfs.txt
> F: fs/cramfs/
>
> +CREATIVE SB0540
> +M: Bastien Nocera <hadess@hadess.net>
> +L: linux-input@vger.kernel.org
> +S: Maintained
> +F: drivers/hid/hid-creative-sb0540.c
> +
> CRYPTO API
> M: Herbert Xu <herbert@gondor.apana.org.au>
> M: "David S. Miller" <davem@davemloft.net>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 3872e03d9a59..a70999f9c639 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -273,6 +273,15 @@ config HID_CP2112
> and gpiochip to expose these functions of the CP2112. The
> customizable USB descriptor fields are exposed as sysfs attributes.
>
> +config HID_CREATIVE_SB0540
> + tristate "Creative SB0540 infrared receiver"
> + depends on USB_HID
> + help
> + Support for Creative infrared SB0540-compatible remote controls, such
> + as the RM-1500 and RM-1800 remotes.
> +
> + Say Y here if you want support for Creative SB0540 infrared receiver.
> +
> config HID_CYPRESS
> tristate "Cypress mouse and barcode readers"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index cc5d827c9164..1ad662fe37b6 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_HID_ALPS) += hid-alps.o
> obj-$(CONFIG_HID_ACRUX) += hid-axff.o
> obj-$(CONFIG_HID_APPLE) += hid-apple.o
> obj-$(CONFIG_HID_APPLEIR) += hid-appleir.o
> +obj-$(CONFIG_HID_CREATIVE_SB0540) += hid-creative-sb0540.c
I forgot to mention that sparse was complaining about:
scripts/Makefile.build:283: target 'drivers/hid/hid-creative-sb0540.c'
doesn't match the target pattern
And it turns out your line should read `hid-creative-sb0540.o` not
`hid-creative-sb0540.c`
Cheers,
Benjamin
> obj-$(CONFIG_HID_ASUS) += hid-asus.o
> obj-$(CONFIG_HID_AUREAL) += hid-aureal.o
> obj-$(CONFIG_HID_BELKIN) += hid-belkin.o
> diff --git a/drivers/hid/hid-creative-sb0540.c b/drivers/hid/hid-creative-sb0540.c
> new file mode 100644
> index 000000000000..6b7c81ccf310
> --- /dev/null
> +++ b/drivers/hid/hid-creative-sb0540.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * HID driver for the Creative SB0540 receiver
> + *
> + * Copyright (C) 2019 Red Hat Inc. All Rights Reserved
> + *
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include "hid-ids.h"
> +
> +MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
> +MODULE_DESCRIPTION("HID Creative SB0540 receiver");
> +MODULE_LICENSE("GPL");
> +
> +static const unsigned short creative_sb0540_key_table[] = {
> + KEY_POWER,
> + KEY_RESERVED, /* text: 24bit */
> + KEY_RESERVED, /* 24bit wheel up */
> + KEY_RESERVED, /* 24bit wheel down */
> + KEY_RESERVED, /* text: CMSS */
> + KEY_RESERVED, /* CMSS wheel Up */
> + KEY_RESERVED, /* CMSS wheel Down */
> + KEY_RESERVED, /* text: EAX */
> + KEY_RESERVED, /* EAX wheel up */
> + KEY_RESERVED, /* EAX wheel down */
> + KEY_RESERVED, /* text: 3D Midi */
> + KEY_RESERVED, /* 3D Midi wheel up */
> + KEY_RESERVED, /* 3D Midi wheel down */
> + KEY_MUTE,
> + KEY_VOLUMEUP,
> + KEY_VOLUMEDOWN,
> + KEY_UP,
> + KEY_LEFT,
> + KEY_RIGHT,
> + KEY_REWIND,
> + KEY_OK,
> + KEY_FASTFORWARD,
> + KEY_DOWN,
> + KEY_AGAIN, /* text: Return, symbol: Jump to */
> + KEY_PLAY, /* text: Start */
> + KEY_ESC, /* text: Cancel */
> + KEY_RECORD,
> + KEY_OPTION,
> + KEY_MENU, /* text: Display */
> + KEY_PREVIOUS,
> + KEY_PLAYPAUSE,
> + KEY_NEXT,
> + KEY_SLOW,
> + KEY_STOP,
> + KEY_NUMERIC_1,
> + KEY_NUMERIC_2,
> + KEY_NUMERIC_3,
> + KEY_NUMERIC_4,
> + KEY_NUMERIC_5,
> + KEY_NUMERIC_6,
> + KEY_NUMERIC_7,
> + KEY_NUMERIC_8,
> + KEY_NUMERIC_9,
> + KEY_NUMERIC_0
> +};
> +
> +/*
> + * Codes and keys from lirc's
> + * remotes/creative/lircd.conf.alsa_usb
> + * order and size must match creative_sb0540_key_table[] above
> + */
> +static const unsigned short creative_sb0540_codes[] = {
> + 0x619E,
> + 0x916E,
> + 0x926D,
> + 0x936C,
> + 0x718E,
> + 0x946B,
> + 0x956A,
> + 0x8C73,
> + 0x9669,
> + 0x9768,
> + 0x9867,
> + 0x9966,
> + 0x9A65,
> + 0x6E91,
> + 0x629D,
> + 0x639C,
> + 0x7B84,
> + 0x6B94,
> + 0x728D,
> + 0x8778,
> + 0x817E,
> + 0x758A,
> + 0x8D72,
> + 0x8E71,
> + 0x8877,
> + 0x7C83,
> + 0x738C,
> + 0x827D,
> + 0x7689,
> + 0x7F80,
> + 0x7986,
> + 0x7A85,
> + 0x7D82,
> + 0x857A,
> + 0x8B74,
> + 0x8F70,
> + 0x906F,
> + 0x8A75,
> + 0x847B,
> + 0x7887,
> + 0x8976,
> + 0x837C,
> + 0x7788,
> + 0x807F
> +};
> +
> +struct creative_sb0540 {
> + struct input_dev *input_dev;
> + struct hid_device *hid;
> + unsigned short keymap[ARRAY_SIZE(creative_sb0540_key_table)];
> +};
> +
> +static inline u64 reverse(u64 data, int bits)
> +{
> + int i;
> + u64 c;
> +
> + c = 0;
> + for (i = 0; i < bits; i++) {
> + c |= (u64) (((data & (((u64) 1) << i)) ? 1 : 0))
> + << (bits - 1 - i);
> + }
> + return (c);
> +}
> +
> +static int get_key(struct creative_sb0540 *creative_sb0540, u64 keycode)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(creative_sb0540_codes); i++) {
> + if (creative_sb0540_codes[i] == keycode)
> + return creative_sb0540->keymap[i];
> + }
> +
> + return 0;
> +
> +}
> +
> +static int creative_sb0540_raw_event(struct hid_device *hid,
> + struct hid_report *report, u8 *data, int len)
> +{
> + struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
> + u64 code, main_code;
> + int key;
> +
> + if (len != 6)
> + goto out;
> +
> + /* From daemons/hw_hiddev.c sb0540_rec() in lirc */
> + code = reverse(data[5], 8);
> + main_code = (code << 8) + ((~code) & 0xff);
> +
> + /*
> + * Flip to get values in the same format as
> + * remotes/creative/lircd.conf.alsa_usb in lirc
> + */
> + main_code = ((main_code & 0xff) << 8) +
> + ((main_code & 0xff00) >> 8);
> +
> + key = get_key(creative_sb0540, main_code);
> + if (key == 0 || key == KEY_RESERVED) {
> + hid_err(hid, "Could not get a key for main_code %llX\n",
> + main_code);
> + return 0;
> + }
> +
> + input_report_key(creative_sb0540->input_dev, key, 1);
> + input_report_key(creative_sb0540->input_dev, key, 0);
> + input_sync(creative_sb0540->input_dev);
> +
> + /* let hidraw and hiddev handle the report */
> + return 0;
> +}
> +
> +static int creative_sb0540_input_configured(struct hid_device *hid,
> + struct hid_input *hidinput)
> +{
> + struct input_dev *input_dev = hidinput->input;
> + struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
> + int i;
> +
> + creative_sb0540->input_dev = input_dev;
> +
> + input_dev->keycode = creative_sb0540->keymap;
> + input_dev->keycodesize = sizeof(unsigned short);
> + input_dev->keycodemax = ARRAY_SIZE(creative_sb0540->keymap);
> +
> + input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
> +
> + memcpy(creative_sb0540->keymap, creative_sb0540_key_table,
> + sizeof(creative_sb0540->keymap));
> + for (i = 0; i < ARRAY_SIZE(creative_sb0540_key_table); i++)
> + set_bit(creative_sb0540->keymap[i], input_dev->keybit);
> + clear_bit(KEY_RESERVED, input_dev->keybit);
> +
> + return 0;
> +}
> +
> +static int creative_sb0540_input_mapping(struct hid_device *hid,
> + struct hid_input *hi, struct hid_field *field,
> + struct hid_usage *usage, unsigned long **bit, int *max)
> +{
> + /*
> + * We are remapping the keys ourselves, so ignore the hid-input
> + * keymap processing.
> + */
> + return -1;
> +}
> +
> +static int creative_sb0540_probe(struct hid_device *hid,
> + const struct hid_device_id *id)
> +{
> + int ret;
> + struct creative_sb0540 *creative_sb0540;
> +
> + creative_sb0540 = devm_kzalloc(&hid->dev,
> + sizeof(struct creative_sb0540), GFP_KERNEL);
> +
> + if (!creative_sb0540)
> + return -ENOMEM;
> +
> + creative_sb0540->hid = hid;
> +
> + /* force input as some remotes bypass the input registration */
> + hid->quirks |= HID_QUIRK_HIDINPUT_FORCE;
> +
> + hid_set_drvdata(hid, creative_sb0540);
> +
> + ret = hid_parse(hid);
> + if (ret) {
> + hid_err(hid, "parse failed\n");
> + return ret;
> + }
> +
> + ret = hid_hw_start(hid, HID_CONNECT_DEFAULT);
> + if (ret) {
> + hid_err(hid, "hw start failed\n");
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static const struct hid_device_id creative_sb0540_devices[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB0540) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, creative_sb0540_devices);
> +
> +static struct hid_driver creative_sb0540_driver = {
> + .name = "creative-sb0540",
> + .id_table = creative_sb0540_devices,
> + .raw_event = creative_sb0540_raw_event,
> + .input_configured = creative_sb0540_input_configured,
> + .probe = creative_sb0540_probe,
> + .input_mapping = creative_sb0540_input_mapping,
> +};
> +module_hid_driver(creative_sb0540_driver);
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 826324997686..206b7065da86 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -312,6 +312,7 @@
> #define USB_VENDOR_ID_CREATIVELABS 0x041e
> #define USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51 0x322c
> #define USB_DEVICE_ID_PRODIKEYS_PCMIDI 0x2801
> +#define USB_DEVICE_ID_CREATIVE_SB0540 0x3100
>
> #define USB_VENDOR_ID_CVTOUCH 0x1ff7
> #define USB_DEVICE_ID_CVTOUCH_SCREEN 0x0013
> --
> 2.21.0
>
^ permalink raw reply
* [PATCH v5] HID: sb0540: add support for Creative SB0540 IR receivers
From: Bastien Nocera @ 2019-07-01 10:20 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires, Bastien Nocera
From: Bastien Nocera <bnocera@redhat.com>
Add a new hid driver for the Creative SB0540 IR receiver. This receiver
is usually coupled with an RM-1500 or an RM-1800 remote control.
The scrollwheels on the RM-1800 remote are not bound, as they are
labelled for specific audio controls that don't usually exist on most
systems. They can be remapped using standard Linux keyboard
remapping tools.
Signed-off-by: Bastien Nocera <bnocera@redhat.com>
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 9 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-creative-sb0540.c | 268 ++++++++++++++++++++++++++++++
drivers/hid/hid-ids.h | 1 +
5 files changed, 285 insertions(+)
create mode 100644 drivers/hid/hid-creative-sb0540.c
diff --git a/MAINTAINERS b/MAINTAINERS
index d0ed735994a5..6fc022d152c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4227,6 +4227,12 @@ S: Maintained
F: Documentation/filesystems/cramfs.txt
F: fs/cramfs/
+CREATIVE SB0540
+M: Bastien Nocera <hadess@hadess.net>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-creative-sb0540.c
+
CRYPTO API
M: Herbert Xu <herbert@gondor.apana.org.au>
M: "David S. Miller" <davem@davemloft.net>
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 3872e03d9a59..a70999f9c639 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -273,6 +273,15 @@ config HID_CP2112
and gpiochip to expose these functions of the CP2112. The
customizable USB descriptor fields are exposed as sysfs attributes.
+config HID_CREATIVE_SB0540
+ tristate "Creative SB0540 infrared receiver"
+ depends on USB_HID
+ help
+ Support for Creative infrared SB0540-compatible remote controls, such
+ as the RM-1500 and RM-1800 remotes.
+
+ Say Y here if you want support for Creative SB0540 infrared receiver.
+
config HID_CYPRESS
tristate "Cypress mouse and barcode readers"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index cc5d827c9164..0c03308cfb08 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_HID_ALPS) += hid-alps.o
obj-$(CONFIG_HID_ACRUX) += hid-axff.o
obj-$(CONFIG_HID_APPLE) += hid-apple.o
obj-$(CONFIG_HID_APPLEIR) += hid-appleir.o
+obj-$(CONFIG_HID_CREATIVE_SB0540) += hid-creative-sb0540.o
obj-$(CONFIG_HID_ASUS) += hid-asus.o
obj-$(CONFIG_HID_AUREAL) += hid-aureal.o
obj-$(CONFIG_HID_BELKIN) += hid-belkin.o
diff --git a/drivers/hid/hid-creative-sb0540.c b/drivers/hid/hid-creative-sb0540.c
new file mode 100644
index 000000000000..6b7c81ccf310
--- /dev/null
+++ b/drivers/hid/hid-creative-sb0540.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID driver for the Creative SB0540 receiver
+ *
+ * Copyright (C) 2019 Red Hat Inc. All Rights Reserved
+ *
+ */
+
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include "hid-ids.h"
+
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_DESCRIPTION("HID Creative SB0540 receiver");
+MODULE_LICENSE("GPL");
+
+static const unsigned short creative_sb0540_key_table[] = {
+ KEY_POWER,
+ KEY_RESERVED, /* text: 24bit */
+ KEY_RESERVED, /* 24bit wheel up */
+ KEY_RESERVED, /* 24bit wheel down */
+ KEY_RESERVED, /* text: CMSS */
+ KEY_RESERVED, /* CMSS wheel Up */
+ KEY_RESERVED, /* CMSS wheel Down */
+ KEY_RESERVED, /* text: EAX */
+ KEY_RESERVED, /* EAX wheel up */
+ KEY_RESERVED, /* EAX wheel down */
+ KEY_RESERVED, /* text: 3D Midi */
+ KEY_RESERVED, /* 3D Midi wheel up */
+ KEY_RESERVED, /* 3D Midi wheel down */
+ KEY_MUTE,
+ KEY_VOLUMEUP,
+ KEY_VOLUMEDOWN,
+ KEY_UP,
+ KEY_LEFT,
+ KEY_RIGHT,
+ KEY_REWIND,
+ KEY_OK,
+ KEY_FASTFORWARD,
+ KEY_DOWN,
+ KEY_AGAIN, /* text: Return, symbol: Jump to */
+ KEY_PLAY, /* text: Start */
+ KEY_ESC, /* text: Cancel */
+ KEY_RECORD,
+ KEY_OPTION,
+ KEY_MENU, /* text: Display */
+ KEY_PREVIOUS,
+ KEY_PLAYPAUSE,
+ KEY_NEXT,
+ KEY_SLOW,
+ KEY_STOP,
+ KEY_NUMERIC_1,
+ KEY_NUMERIC_2,
+ KEY_NUMERIC_3,
+ KEY_NUMERIC_4,
+ KEY_NUMERIC_5,
+ KEY_NUMERIC_6,
+ KEY_NUMERIC_7,
+ KEY_NUMERIC_8,
+ KEY_NUMERIC_9,
+ KEY_NUMERIC_0
+};
+
+/*
+ * Codes and keys from lirc's
+ * remotes/creative/lircd.conf.alsa_usb
+ * order and size must match creative_sb0540_key_table[] above
+ */
+static const unsigned short creative_sb0540_codes[] = {
+ 0x619E,
+ 0x916E,
+ 0x926D,
+ 0x936C,
+ 0x718E,
+ 0x946B,
+ 0x956A,
+ 0x8C73,
+ 0x9669,
+ 0x9768,
+ 0x9867,
+ 0x9966,
+ 0x9A65,
+ 0x6E91,
+ 0x629D,
+ 0x639C,
+ 0x7B84,
+ 0x6B94,
+ 0x728D,
+ 0x8778,
+ 0x817E,
+ 0x758A,
+ 0x8D72,
+ 0x8E71,
+ 0x8877,
+ 0x7C83,
+ 0x738C,
+ 0x827D,
+ 0x7689,
+ 0x7F80,
+ 0x7986,
+ 0x7A85,
+ 0x7D82,
+ 0x857A,
+ 0x8B74,
+ 0x8F70,
+ 0x906F,
+ 0x8A75,
+ 0x847B,
+ 0x7887,
+ 0x8976,
+ 0x837C,
+ 0x7788,
+ 0x807F
+};
+
+struct creative_sb0540 {
+ struct input_dev *input_dev;
+ struct hid_device *hid;
+ unsigned short keymap[ARRAY_SIZE(creative_sb0540_key_table)];
+};
+
+static inline u64 reverse(u64 data, int bits)
+{
+ int i;
+ u64 c;
+
+ c = 0;
+ for (i = 0; i < bits; i++) {
+ c |= (u64) (((data & (((u64) 1) << i)) ? 1 : 0))
+ << (bits - 1 - i);
+ }
+ return (c);
+}
+
+static int get_key(struct creative_sb0540 *creative_sb0540, u64 keycode)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(creative_sb0540_codes); i++) {
+ if (creative_sb0540_codes[i] == keycode)
+ return creative_sb0540->keymap[i];
+ }
+
+ return 0;
+
+}
+
+static int creative_sb0540_raw_event(struct hid_device *hid,
+ struct hid_report *report, u8 *data, int len)
+{
+ struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
+ u64 code, main_code;
+ int key;
+
+ if (len != 6)
+ goto out;
+
+ /* From daemons/hw_hiddev.c sb0540_rec() in lirc */
+ code = reverse(data[5], 8);
+ main_code = (code << 8) + ((~code) & 0xff);
+
+ /*
+ * Flip to get values in the same format as
+ * remotes/creative/lircd.conf.alsa_usb in lirc
+ */
+ main_code = ((main_code & 0xff) << 8) +
+ ((main_code & 0xff00) >> 8);
+
+ key = get_key(creative_sb0540, main_code);
+ if (key == 0 || key == KEY_RESERVED) {
+ hid_err(hid, "Could not get a key for main_code %llX\n",
+ main_code);
+ return 0;
+ }
+
+ input_report_key(creative_sb0540->input_dev, key, 1);
+ input_report_key(creative_sb0540->input_dev, key, 0);
+ input_sync(creative_sb0540->input_dev);
+
+ /* let hidraw and hiddev handle the report */
+ return 0;
+}
+
+static int creative_sb0540_input_configured(struct hid_device *hid,
+ struct hid_input *hidinput)
+{
+ struct input_dev *input_dev = hidinput->input;
+ struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
+ int i;
+
+ creative_sb0540->input_dev = input_dev;
+
+ input_dev->keycode = creative_sb0540->keymap;
+ input_dev->keycodesize = sizeof(unsigned short);
+ input_dev->keycodemax = ARRAY_SIZE(creative_sb0540->keymap);
+
+ input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
+
+ memcpy(creative_sb0540->keymap, creative_sb0540_key_table,
+ sizeof(creative_sb0540->keymap));
+ for (i = 0; i < ARRAY_SIZE(creative_sb0540_key_table); i++)
+ set_bit(creative_sb0540->keymap[i], input_dev->keybit);
+ clear_bit(KEY_RESERVED, input_dev->keybit);
+
+ return 0;
+}
+
+static int creative_sb0540_input_mapping(struct hid_device *hid,
+ struct hid_input *hi, struct hid_field *field,
+ struct hid_usage *usage, unsigned long **bit, int *max)
+{
+ /*
+ * We are remapping the keys ourselves, so ignore the hid-input
+ * keymap processing.
+ */
+ return -1;
+}
+
+static int creative_sb0540_probe(struct hid_device *hid,
+ const struct hid_device_id *id)
+{
+ int ret;
+ struct creative_sb0540 *creative_sb0540;
+
+ creative_sb0540 = devm_kzalloc(&hid->dev,
+ sizeof(struct creative_sb0540), GFP_KERNEL);
+
+ if (!creative_sb0540)
+ return -ENOMEM;
+
+ creative_sb0540->hid = hid;
+
+ /* force input as some remotes bypass the input registration */
+ hid->quirks |= HID_QUIRK_HIDINPUT_FORCE;
+
+ hid_set_drvdata(hid, creative_sb0540);
+
+ ret = hid_parse(hid);
+ if (ret) {
+ hid_err(hid, "parse failed\n");
+ return ret;
+ }
+
+ ret = hid_hw_start(hid, HID_CONNECT_DEFAULT);
+ if (ret) {
+ hid_err(hid, "hw start failed\n");
+ return ret;
+ }
+
+ return ret;
+}
+
+static const struct hid_device_id creative_sb0540_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB0540) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, creative_sb0540_devices);
+
+static struct hid_driver creative_sb0540_driver = {
+ .name = "creative-sb0540",
+ .id_table = creative_sb0540_devices,
+ .raw_event = creative_sb0540_raw_event,
+ .input_configured = creative_sb0540_input_configured,
+ .probe = creative_sb0540_probe,
+ .input_mapping = creative_sb0540_input_mapping,
+};
+module_hid_driver(creative_sb0540_driver);
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 826324997686..206b7065da86 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -312,6 +312,7 @@
#define USB_VENDOR_ID_CREATIVELABS 0x041e
#define USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51 0x322c
#define USB_DEVICE_ID_PRODIKEYS_PCMIDI 0x2801
+#define USB_DEVICE_ID_CREATIVE_SB0540 0x3100
#define USB_VENDOR_ID_CVTOUCH 0x1ff7
#define USB_DEVICE_ID_CVTOUCH_SCREEN 0x0013
--
2.21.0
^ permalink raw reply related
* Re: [PATCH v4] HID: sb0540: add support for Creative SB0540 IR receivers
From: Bastien Nocera @ 2019-07-01 10:20 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: open list:HID CORE LAYER, lkml, Jiri Kosina, Bastien Nocera
In-Reply-To: <CAO-hwJJaj6f216_H=UFO0CEp+ZHRWwvhTO9FCPqdeimEwE6p0Q@mail.gmail.com>
On Mon, 2019-07-01 at 12:15 +0200, Benjamin Tissoires wrote:
>
<snip>
> I forgot to mention that sparse was complaining about:
>
> scripts/Makefile.build:283: target 'drivers/hid/hid-creative-
> sb0540.c'
> doesn't match the target pattern
>
> And it turns out your line should read `hid-creative-sb0540.o` not
> `hid-creative-sb0540.c`
It does show that I didn't fancy making my office warmer with a full
kernel compile, right?
v5 sent with the fix, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] Input: edt-ft5x06 - simplify event reporting code
From: Andy Shevchenko @ 2019-07-01 10:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Marco Felsch, Benoit Parrot,
Linux Kernel Mailing List
In-Reply-To: <20190630070552.GA91171@dtor-ws>
On Sun, Jun 30, 2019 at 10:05 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Sun, Jun 23, 2019 at 10:59:18AM +0300, Andy Shevchenko wrote:
> > On Sun, Jun 23, 2019 at 9:31 AM Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > >
> > > Now that input_mt_report_slot_state() returns true if slot is active we no
> > > longer need a temporary for the slot state.
> > > - down = type != TOUCH_EVENT_UP;
> > >
> > > input_mt_slot(tsdata->input, id);
> > > - input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, down);
> >
> > > + if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER,
> > > + type != TOUCH_EVENT_UP))
> >
> > Can't we simple do somethink like
> > - down = type != TOUCH_EVENT_UP;
> > + down = input_mt_report_slot_state(tsdata->input,
> > MT_TOOL_FINGER, type != TOUCH_EVENT_UP);
>
> Why though? The temporary was needed so we did not have to repeat the
> expression for "contact down" condition, and now we do not need it. The
> whole change was done so that we cab remove the temporary...
I see. Thanks for explanation.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 3/3] video: fbdev: don't print error message on framebuffer_alloc() failure
From: Bartlomiej Zolnierkiewicz @ 2019-07-01 15:07 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: linux-fbdev, Jiri Kosina, lkml, dri-devel, Bruno Prémont,
open list:HID CORE LAYER
In-Reply-To: <CAO-hwJJFKQEwEiDe1M6gP5D_yf0BxyzaJH-kxwxqkCVo52WK6g@mail.gmail.com>
On 7/1/19 10:37 AM, Benjamin Tissoires wrote:
> Hi Bartlomiej,
Hi Benjamin,
> On Fri, Jun 14, 2019 at 4:52 PM Bartlomiej Zolnierkiewicz
> <b.zolnierkie@samsung.com> wrote:
>>
>> framebuffer_alloc() can fail only on kzalloc() memory allocation
>> failure and since kzalloc() will print error message in such case
>> we can omit printing extra error message in drivers (which BTW is
>> what the majority of framebuffer_alloc() users is doing already).
>>
>> Cc: "Bruno Prémont" <bonbons@linux-vserver.org>
>> Cc: Jiri Kosina <jikos@kernel.org>
>> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
>> ---
>> drivers/hid/hid-picolcd_fb.c | 4 +---
>> drivers/video/fbdev/amifb.c | 4 +---
>> drivers/video/fbdev/arkfb.c | 4 +---
>> drivers/video/fbdev/atmel_lcdfb.c | 4 +---
>> drivers/video/fbdev/aty/aty128fb.c | 5 ++---
>> drivers/video/fbdev/aty/atyfb_base.c | 10 ++++------
>> drivers/video/fbdev/aty/radeon_base.c | 2 --
>> drivers/video/fbdev/chipsfb.c | 1 -
>> drivers/video/fbdev/cirrusfb.c | 5 +----
>> drivers/video/fbdev/da8xx-fb.c | 1 -
>> drivers/video/fbdev/efifb.c | 1 -
>> drivers/video/fbdev/grvga.c | 4 +---
>> drivers/video/fbdev/gxt4500.c | 5 ++---
>> drivers/video/fbdev/hyperv_fb.c | 4 +---
>> drivers/video/fbdev/i740fb.c | 4 +---
>> drivers/video/fbdev/imsttfb.c | 5 +----
>> drivers/video/fbdev/intelfb/intelfbdrv.c | 5 ++---
>> drivers/video/fbdev/jz4740_fb.c | 4 +---
>> drivers/video/fbdev/mb862xx/mb862xxfbdrv.c | 5 +----
>> drivers/video/fbdev/mbx/mbxfb.c | 4 +---
>> drivers/video/fbdev/omap/omapfb_main.c | 2 --
>> drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 6 +-----
>> drivers/video/fbdev/platinumfb.c | 5 ++---
>> drivers/video/fbdev/pmag-aa-fb.c | 4 +---
>> drivers/video/fbdev/pmag-ba-fb.c | 4 +---
>> drivers/video/fbdev/pmagb-b-fb.c | 4 +---
>> drivers/video/fbdev/pvr2fb.c | 6 +-----
>> drivers/video/fbdev/riva/fbdev.c | 1 -
>> drivers/video/fbdev/s3c-fb.c | 4 +---
>> drivers/video/fbdev/s3fb.c | 4 +---
>> drivers/video/fbdev/sh_mobile_lcdcfb.c | 8 ++------
>> drivers/video/fbdev/sm501fb.c | 4 +---
>> drivers/video/fbdev/sm712fb.c | 1 -
>> drivers/video/fbdev/smscufx.c | 4 +---
>> drivers/video/fbdev/ssd1307fb.c | 4 +---
>> drivers/video/fbdev/sunxvr1000.c | 1 -
>> drivers/video/fbdev/sunxvr2500.c | 1 -
>> drivers/video/fbdev/sunxvr500.c | 1 -
>> drivers/video/fbdev/tgafb.c | 4 +---
>> drivers/video/fbdev/udlfb.c | 4 +---
>> drivers/video/fbdev/via/viafbdev.c | 6 +-----
>> drivers/video/fbdev/vt8623fb.c | 4 +---
>> 42 files changed, 40 insertions(+), 123 deletions(-)
>>
>> Index: b/drivers/hid/hid-picolcd_fb.c
>> ===================================================================
>> --- a/drivers/hid/hid-picolcd_fb.c
>> +++ b/drivers/hid/hid-picolcd_fb.c
>> @@ -522,10 +522,8 @@ int picolcd_init_framebuffer(struct pico
>> sizeof(struct fb_deferred_io) +
>> sizeof(struct picolcd_fb_data) +
>> PICOLCDFB_SIZE, dev);
>> - if (info == NULL) {
>> - dev_err(dev, "failed to allocate a framebuffer\n");
>> + if (!info)
>> goto err_nomem;
>> - }
>
> It would have been better to split this change as the HID and fbdev
> are different trees.
Ah, there are no modifications to framebuffer_alloc() itself so changes
are independent. I should have noticed that earlier, sorry about that..
> However, I do not expect a conflict here (there hasn't been updates of
> hid-picolcd_fb.c in a while), so feel free to take this patch through
> the fbdev tree with my:
> Acked-By: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Thank you!
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [Patch 1/1] Input: edt-ft5x06 - disable irq handling during suspend
From: Benoit Parrot @ 2019-07-01 20:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Andy Shevchenko, Henrik Rydberg, Marco Felsch, Andy Shevchenko,
linux-input, Linux Kernel Mailing List
In-Reply-To: <20190701073233.GA172968@dtor-ws>
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote on Mon [2019-Jul-01 00:32:33 -0700]:
> On Mon, Jun 24, 2019 at 07:24:57AM -0500, Benoit Parrot wrote:
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote on Sat [2019-Jun-22 22:59:40 -0700]:
> > > On Sat, Jun 22, 2019 at 01:37:10PM +0300, Andy Shevchenko wrote:
> > > > On Fri, Jun 21, 2019 at 9:53 PM Benoit Parrot <bparrot@ti.com> wrote:
> > > > >
> > > > > As a wakeup source when the system is in suspend there is little point
> > > > > trying to access a register across the i2c bus as it is probably still
> > > > > inactive. We need to prevent the irq handler from being called during
> > > > > suspend.
> > > > >
> > > >
> > > > Hmm... But how OS will know what the event to handle afterwards?
> > > > I mean shouldn't we guarantee somehow the delivery of the event to the
> > > > input, in this case, subsystem followed by corresponding user space?
> > >
> > > If we are using level interrupts then it will work OK, however it is
> > > really easy to lose edge here, as replaying disabled edge triggered
> > > interrupts is not really reliable.
> > >
> > > Benoit, what kind of interrupt do you use in your system?
> >
> > Dmitry,
> >
> > On our systems we currently used edge trigger. One example is available in
> > mainline: arch/arm/boot/dts/am437x-sk-evm.dts
> > 632: interrupts = <31 IRQ_TYPE_EDGE_FALLING>;
>
> Does your device still work if you switch to level-triggered interrupt?
That would depend on the device. But for instance on am437x, in order for
GPIO IRQ to be detected as a wake up event they need to be edge triggered.
>
> Regarding your patch I am uncomfortable with disabling interrupts if
> interrupt is edge-triggered, as replaying edge interrupts after enabling
> is not very reliable. So we should either only disable interrupt if it
> is level-triggered, or make sure we read and process data from the
> device after re-enabling interrupt to rearm it. We'll need to make sure
> suspend does not race with interrupt handler than and also make sure we
> handle case when device does not actually has data to report.
I am still not sure who would consume these events. Upon waking up from
suspend it would take a while for user-space to be ready to consume these
events, and by that time there may have been quite a few of them.
We are currently missing those events anyways, no?
I mean the i2c read operation during suspend is failing anyways, which
means that particular event is already missed.
Regards,
Benoit
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* [PATCH 0/2] Support for buttons on newer MS Surface devices
From: Maximilian Luz @ 2019-07-02 0:37 UTC (permalink / raw)
Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
Hans de Goede, Chen Yu, Darren Hart, Andy Shevchenko,
Benjamin Tissoires, Maximilian Luz
This series adds suport for power and volume buttons on 5th and 6th
generation Microsoft Surface devices. Specifically, it adds support for
the power-button on the Surface Laptop 1 and Laptop 2, as well as
support for power- and (on-device) volume-buttons on the Surface Pro 5
(2017), Pro 6, and Book 2.
These devices use the same MSHW0040 device as on the Surface Pro 4,
however, whereas the Pro 4 uses an ACPI notify handler, the newer
devices use GPIO interrupts to signal these events.
The first patch of this series ensures that the surfacepro3_button
driver, used for MSHW0040 on the Pro 4, does not probe for the newer
devices. The second patch adapts soc_button_array to implement the
actual button support.
I think the changes to soc_button_array in the second patch warrant a
thorough review. I've tried to make things a bit more generic to be able
to integrate arbitrary ACPI GPIO power-/volume-button devices more
easily, I'm not sure if there may be reasons against this.
These patches have also been tested on various Surface devices via the
github.com/jakeday/linux-surface patchset.
Changes since v1:
- [PATCH 1/2] platform: Fix device check for surfacepro3_button
No changes.
- [PATCH 2/2] input: soc_button_array for newer surface devices
Ensure the patch compiles without CONFIG_ACPI.
Maximilian Luz (2):
platform: Fix device check for surfacepro3_button
input: soc_button_array for newer surface devices
drivers/input/misc/soc_button_array.c | 145 ++++++++++++++++++++--
drivers/platform/x86/surfacepro3_button.c | 38 ++++++
2 files changed, 171 insertions(+), 12 deletions(-)
--
2.22.0
^ permalink raw reply
* [PATCH v2 1/2] platform: Fix device check for surfacepro3_button
From: Maximilian Luz @ 2019-07-02 0:37 UTC (permalink / raw)
Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
Hans de Goede, Chen Yu, Darren Hart, Andy Shevchenko,
Benjamin Tissoires, Maximilian Luz
In-Reply-To: <20190702003740.75970-1-luzmaximilian@gmail.com>
Do not use the surfacepro3_button driver on newer Microsoft Surface
models, only use it on the Surface Pro 3 and 4. Newer models (5th, 6th
and possibly future generations) use the same device as the Surface Pro
4 to represent their volume and power buttons (MSHW0040), but their
acutal implementation is significantly different. This patch ensures
that the surfacepro3_button driver is only used on the Pro 3 and 4
models, allowing a different driver to bind on other models.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
drivers/platform/x86/surfacepro3_button.c | 38 +++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c
index 47c6d000465a..0e2c7dfafd9f 100644
--- a/drivers/platform/x86/surfacepro3_button.c
+++ b/drivers/platform/x86/surfacepro3_button.c
@@ -20,6 +20,12 @@
#define SURFACE_BUTTON_OBJ_NAME "VGBI"
#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro 3/4 Buttons"
+#define MSHW0040_DSM_REVISION 0x01
+#define MSHW0040_DSM_GET_OMPR 0x02 // get OEM Platform Revision
+static const guid_t MSHW0040_DSM_UUID =
+ GUID_INIT(0x6fd05c69, 0xcde3, 0x49f4, 0x95, 0xed, 0xab, 0x16, 0x65,
+ 0x49, 0x80, 0x35);
+
#define SURFACE_BUTTON_NOTIFY_TABLET_MODE 0xc8
#define SURFACE_BUTTON_NOTIFY_PRESS_POWER 0xc6
@@ -142,6 +148,34 @@ static int surface_button_resume(struct device *dev)
}
#endif
+/*
+ * Surface Pro 4 and Surface Book 2 / Surface Pro 2017 use the same device
+ * ID (MSHW0040) for the power/volume buttons. Make sure this is the right
+ * device by checking for the _DSM method and OEM Platform Revision.
+ */
+static int surface_button_check_MSHW0040(struct acpi_device *dev)
+{
+ acpi_handle handle = dev->handle;
+ union acpi_object *result;
+ u64 oem_platform_rev = 0;
+
+ // get OEM platform revision
+ result = acpi_evaluate_dsm_typed(handle, &MSHW0040_DSM_UUID,
+ MSHW0040_DSM_REVISION,
+ MSHW0040_DSM_GET_OMPR,
+ NULL, ACPI_TYPE_INTEGER);
+
+ if (result) {
+ oem_platform_rev = result->integer.value;
+ ACPI_FREE(result);
+ }
+
+ dev_dbg(&dev->dev, "OEM Platform Revision %llu\n", oem_platform_rev);
+
+ return oem_platform_rev == 0 ? 0 : -ENODEV;
+}
+
+
static int surface_button_add(struct acpi_device *device)
{
struct surface_button *button;
@@ -154,6 +188,10 @@ static int surface_button_add(struct acpi_device *device)
strlen(SURFACE_BUTTON_OBJ_NAME)))
return -ENODEV;
+ error = surface_button_check_MSHW0040(device);
+ if (error)
+ return error;
+
button = kzalloc(sizeof(struct surface_button), GFP_KERNEL);
if (!button)
return -ENOMEM;
--
2.22.0
^ permalink raw reply related
* [PATCH v2 2/2] input: soc_button_array for newer surface devices
From: Maximilian Luz @ 2019-07-02 0:37 UTC (permalink / raw)
Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
Hans de Goede, Chen Yu, Darren Hart, Andy Shevchenko,
Benjamin Tissoires, Maximilian Luz
In-Reply-To: <20190702003740.75970-1-luzmaximilian@gmail.com>
Power and volume button support for 5th and 6th genration Microsoft
Surface devices via soc_button_array.
Note that these devices use the same MSHW0040 device as on the Surface
Pro 4, however the implementation is different (GPIOs vs. ACPI
notifications). Thus some checking is required to ensure we only load
this driver on the correct devices.
Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
---
drivers/input/misc/soc_button_array.c | 145 +++++++++++++++++++++++---
1 file changed, 133 insertions(+), 12 deletions(-)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 5e59f8e57f8e..8bd6909f8d1f 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -25,6 +25,17 @@ struct soc_button_info {
bool wakeup;
};
+/**
+ * struct soc_device_data - driver data for different device types
+ * @button_info: specifications of buttons, if NULL specification is assumed to
+ * be present in _DSD
+ * @check: device-specific check (NULL means all will be accepted)
+ */
+struct soc_device_data {
+ struct soc_button_info *button_info;
+ int (*check)(struct device *dev);
+};
+
/*
* Some of the buttons like volume up/down are auto repeat, while others
* are not. To support both, we register two platform devices, and put
@@ -310,6 +321,7 @@ static int soc_button_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
const struct acpi_device_id *id;
+ struct soc_device_data *device_data;
struct soc_button_info *button_info;
struct soc_button_data *priv;
struct platform_device *pd;
@@ -320,18 +332,20 @@ static int soc_button_probe(struct platform_device *pdev)
if (!id)
return -ENODEV;
- if (!id->driver_data) {
+ device_data = (struct soc_device_data *)id->driver_data;
+ if (device_data && device_data->check) {
+ error = device_data->check(dev);
+ if (error)
+ return error;
+ }
+
+ if (device_data && device_data->button_info) {
+ button_info = (struct soc_button_info *)
+ device_data->button_info;
+ } else {
button_info = soc_button_get_button_info(dev);
if (IS_ERR(button_info))
return PTR_ERR(button_info);
- } else {
- button_info = (struct soc_button_info *)id->driver_data;
- }
-
- error = gpiod_count(dev, NULL);
- if (error < 0) {
- dev_dbg(dev, "no GPIO attached, ignoring...\n");
- return -ENODEV;
}
priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
@@ -357,12 +371,32 @@ static int soc_button_probe(struct platform_device *pdev)
if (!priv->children[0] && !priv->children[1])
return -ENODEV;
- if (!id->driver_data)
+ if (!device_data || !device_data->button_info)
devm_kfree(dev, button_info);
return 0;
}
+
+static int soc_device_check_generic(struct device *dev)
+{
+ int gpios;
+
+ gpios = gpiod_count(dev, NULL);
+ if (gpios < 0) {
+ dev_dbg(dev, "no GPIO attached, ignoring...\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static struct soc_device_data soc_device_ACPI0011 = {
+ .button_info = NULL,
+ .check = soc_device_check_generic,
+};
+
+
/*
* Definition of buttons on the tablet. The ACPI index of each button
* is defined in section 2.8.7.2 of "Windows ACPI Design Guide for SoC
@@ -377,9 +411,96 @@ static struct soc_button_info soc_button_PNP0C40[] = {
{ }
};
+static struct soc_device_data soc_device_PNP0C40 = {
+ .button_info = soc_button_PNP0C40,
+ .check = soc_device_check_generic,
+};
+
+
+/*
+ * Special device check for Surface Book 2 and Surface Pro (2017).
+ * Both, the Surface Pro 4 (surfacepro3_button.c) and the above mentioned
+ * devices use MSHW0040 for power and volume buttons, however the way they
+ * have to be addressed differs. Make sure that we only load this drivers
+ * for the correct devices by checking the OEM Platform Revision provided by
+ * the _DSM method.
+ */
+#define MSHW0040_DSM_REVISION 0x01
+#define MSHW0040_DSM_GET_OMPR 0x02 // get OEM Platform Revision
+static const guid_t MSHW0040_DSM_UUID =
+ GUID_INIT(0x6fd05c69, 0xcde3, 0x49f4, 0x95, 0xed, 0xab, 0x16, 0x65,
+ 0x49, 0x80, 0x35);
+
+#ifdef CONFIG_ACPI
+
+static int soc_device_check_MSHW0040(struct device *dev)
+{
+ acpi_handle handle = ACPI_HANDLE(dev);
+ union acpi_object *result;
+ u64 oem_platform_rev = 0;
+ int gpios;
+
+ // get OEM platform revision
+ result = acpi_evaluate_dsm_typed(handle, &MSHW0040_DSM_UUID,
+ MSHW0040_DSM_REVISION,
+ MSHW0040_DSM_GET_OMPR, NULL,
+ ACPI_TYPE_INTEGER);
+
+ if (result) {
+ oem_platform_rev = result->integer.value;
+ ACPI_FREE(result);
+ }
+
+ if (oem_platform_rev == 0)
+ return -ENODEV;
+
+ dev_dbg(dev, "OEM Platform Revision %llu\n", oem_platform_rev);
+
+ /*
+ * We are _really_ expecting GPIOs here. If we do not get any, this
+ * means the GPIO driver has not been loaded yet (which can happen).
+ * Try again later.
+ */
+ gpios = gpiod_count(dev, NULL);
+ if (gpios < 0)
+ return -EAGAIN;
+
+ return 0;
+}
+
+#else /* CONFIG_ACPI */
+
+static int soc_device_check_MSHW0040(struct device *dev)
+{
+ return -ENODEV;
+}
+
+#endif /* CONFIG_ACPI */
+
+/*
+ * Button infos for Microsoft Surface Book 2 and Surface Pro (2017).
+ * Obtained from DSDT/testing.
+ */
+static struct soc_button_info soc_button_MSHW0040[] = {
+ { "power", 0, EV_KEY, KEY_POWER, false, true },
+ { "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false },
+ { "volume_down", 4, EV_KEY, KEY_VOLUMEDOWN, true, false },
+ { }
+};
+
+static struct soc_device_data soc_device_MSHW0040 = {
+ .button_info = soc_button_MSHW0040,
+ .check = soc_device_check_MSHW0040,
+};
+
+
static const struct acpi_device_id soc_button_acpi_match[] = {
- { "PNP0C40", (unsigned long)soc_button_PNP0C40 },
- { "ACPI0011", 0 },
+ { "PNP0C40", (unsigned long)&soc_device_PNP0C40 },
+ { "ACPI0011", (unsigned long)&soc_device_ACPI0011 },
+
+ /* Microsoft Surface Devices (5th and 6th generation) */
+ { "MSHW0040", (unsigned long)&soc_device_MSHW0040 },
+
{ }
};
--
2.22.0
^ permalink raw reply related
* Re: [PATCH 0/2] Support for buttons on newer MS Surface devices
From: Maximilian Luz @ 2019-07-02 0:43 UTC (permalink / raw)
Cc: Linux Kernel Mailing List, linux-input, Platform Driver,
Dmitry Torokhov, Hans de Goede, Chen Yu, Darren Hart,
Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <CAHp75VcSDvjnS57mS2HyEvUyBRGv68yxXt7wCbJHK3pw98UiOg@mail.gmail.com>
On 6/29/19 4:18 PM, Andy Shevchenko wrote:
> Pushed to my review and testing queue, thanks!
Sorry for my rookie mistake of not checking that this works without
CONFIG_ACPI. I have updated and re-sent the patches to fix this.
Maximilian
^ permalink raw reply
* Re: [PATCH v2 1/2] platform: Fix device check for surfacepro3_button
From: Yu Chen @ 2019-07-02 1:14 UTC (permalink / raw)
To: Maximilian Luz
Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
Hans de Goede, Darren Hart, Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <20190702003740.75970-2-luzmaximilian@gmail.com>
Hi,
On Tue, Jul 02, 2019 at 02:37:39AM +0200, Maximilian Luz wrote:
> Do not use the surfacepro3_button driver on newer Microsoft Surface
> models, only use it on the Surface Pro 3 and 4. Newer models (5th, 6th
> and possibly future generations) use the same device as the Surface Pro
> 4 to represent their volume and power buttons (MSHW0040), but their
> acutal implementation is significantly different. This patch ensures
> that the surfacepro3_button driver is only used on the Pro 3 and 4
> models, allowing a different driver to bind on other models.
>
This method overall looks ok to me.
> Signed-off-by: Maximilian Luz <luzmaximilian@gmail.com>
> ---
> drivers/platform/x86/surfacepro3_button.c | 38 +++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c
> index 47c6d000465a..0e2c7dfafd9f 100644
> --- a/drivers/platform/x86/surfacepro3_button.c
> +++ b/drivers/platform/x86/surfacepro3_button.c
> @@ -20,6 +20,12 @@
> #define SURFACE_BUTTON_OBJ_NAME "VGBI"
> #define SURFACE_BUTTON_DEVICE_NAME "Surface Pro 3/4 Buttons"
>
> +#define MSHW0040_DSM_REVISION 0x01
> +#define MSHW0040_DSM_GET_OMPR 0x02 // get OEM Platform Revision
> +static const guid_t MSHW0040_DSM_UUID =
> + GUID_INIT(0x6fd05c69, 0xcde3, 0x49f4, 0x95, 0xed, 0xab, 0x16, 0x65,
> + 0x49, 0x80, 0x35);
> +
> #define SURFACE_BUTTON_NOTIFY_TABLET_MODE 0xc8
>
> #define SURFACE_BUTTON_NOTIFY_PRESS_POWER 0xc6
> @@ -142,6 +148,34 @@ static int surface_button_resume(struct device *dev)
> }
> #endif
>
> +/*
> + * Surface Pro 4 and Surface Book 2 / Surface Pro 2017 use the same device
> + * ID (MSHW0040) for the power/volume buttons. Make sure this is the right
> + * device by checking for the _DSM method and OEM Platform Revision.
> + */
> +static int surface_button_check_MSHW0040(struct acpi_device *dev)
> +{
> + acpi_handle handle = dev->handle;
> + union acpi_object *result;
> + u64 oem_platform_rev = 0;
> +
> + // get OEM platform revision
> + result = acpi_evaluate_dsm_typed(handle, &MSHW0040_DSM_UUID,
> + MSHW0040_DSM_REVISION,
> + MSHW0040_DSM_GET_OMPR,
> + NULL, ACPI_TYPE_INTEGER);
> +
Does it mean, only 5th, 6th and newer platforms have OEM platform revision?
3rd/4th will get NULL result? Or the opposite?
> + if (result) {
> + oem_platform_rev = result->integer.value;
> + ACPI_FREE(result);
> + }
> +
> + dev_dbg(&dev->dev, "OEM Platform Revision %llu\n", oem_platform_rev);
> +
> + return oem_platform_rev == 0 ? 0 : -ENODEV;
if 3rd/4th do not have this oem rev information while 5th/newer have,
why the latter returns NODEV(it actually has this info)?
> +}
> +
> +
> static int surface_button_add(struct acpi_device *device)
> {
> struct surface_button *button;
> @@ -154,6 +188,10 @@ static int surface_button_add(struct acpi_device *device)
> strlen(SURFACE_BUTTON_OBJ_NAME)))
> return -ENODEV;
>
> + error = surface_button_check_MSHW0040(device);
> + if (error)
> + return error;
> +
ditto, 3rd/4th get error=0?
> button = kzalloc(sizeof(struct surface_button), GFP_KERNEL);
> if (!button)
> return -ENOMEM;
> --
> 2.22.0
>
Best,
Yu
^ permalink raw reply
* Re: [PATCH v2 1/2] platform: Fix device check for surfacepro3_button
From: Maximilian Luz @ 2019-07-02 1:25 UTC (permalink / raw)
To: Yu Chen
Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
Hans de Goede, Darren Hart, Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <20190702011443.GA19902@chenyu-office.sh.intel.com>
On 7/2/19 3:14 AM, Yu Chen wrote:
> On Tue, Jul 02, 2019 at 02:37:39AM +0200, Maximilian Luz wrote:
>> +/*
>> + * Surface Pro 4 and Surface Book 2 / Surface Pro 2017 use the same device
>> + * ID (MSHW0040) for the power/volume buttons. Make sure this is the right
>> + * device by checking for the _DSM method and OEM Platform Revision.
>> + */
>> +static int surface_button_check_MSHW0040(struct acpi_device *dev)
>> +{
>> + acpi_handle handle = dev->handle;
>> + union acpi_object *result;
>> + u64 oem_platform_rev = 0;
>> +
>> + // get OEM platform revision
>> + result = acpi_evaluate_dsm_typed(handle, &MSHW0040_DSM_UUID,
>> + MSHW0040_DSM_REVISION,
>> + MSHW0040_DSM_GET_OMPR,
>> + NULL, ACPI_TYPE_INTEGER);
>> +
> Does it mean, only 5th, 6th and newer platforms have OEM platform revision?
> 3rd/4th will get NULL result? Or the opposite?
Correct, from my testing (with limited sample size) and AML code: 5th
and 6th generation devices have a non-zero OEM platform revision,
whereas 3rd and 4th gen. devices do not have any (i.e. result will be
NULL).
>> + if (result) {
>> + oem_platform_rev = result->integer.value;
>> + ACPI_FREE(result);
>> + }
>> +
>> + dev_dbg(&dev->dev, "OEM Platform Revision %llu\n", oem_platform_rev);
>> +
>> + return oem_platform_rev == 0 ? 0 : -ENODEV;
> if 3rd/4th do not have this oem rev information while 5th/newer have,
> why the latter returns NODEV(it actually has this info)?
Since we always expect a non-zero platform revision (for 5th/6th gen.),
we can initialize it to zero and use that as "unknown"/"not available".
So if it can not be determined, we return NODEV.
>> +}
Cheers,
Maximilian
^ permalink raw reply
* Re: [PATCH v2 1/2] platform: Fix device check for surfacepro3_button
From: Maximilian Luz @ 2019-07-02 1:33 UTC (permalink / raw)
To: Yu Chen
Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
Hans de Goede, Darren Hart, Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <91349d00-e7e2-887b-45e5-4689a401aa2f@gmail.com>
On 7/2/19 3:25 AM, Maximilian Luz wrote:
> On 7/2/19 3:14 AM, Yu Chen wrote:
>> On Tue, Jul 02, 2019 at 02:37:39AM +0200, Maximilian Luz wrote:
>>> +/*
>>> + * Surface Pro 4 and Surface Book 2 / Surface Pro 2017 use the same device
>>> + * ID (MSHW0040) for the power/volume buttons. Make sure this is the right
>>> + * device by checking for the _DSM method and OEM Platform Revision.
>>> + */
>>> +static int surface_button_check_MSHW0040(struct acpi_device *dev)
>>> +{
>>> + acpi_handle handle = dev->handle;
>>> + union acpi_object *result;
>>> + u64 oem_platform_rev = 0;
>>> +
>>> + // get OEM platform revision
>>> + result = acpi_evaluate_dsm_typed(handle, &MSHW0040_DSM_UUID,
>>> + MSHW0040_DSM_REVISION,
>>> + MSHW0040_DSM_GET_OMPR,
>>> + NULL, ACPI_TYPE_INTEGER);
>>> +
>> Does it mean, only 5th, 6th and newer platforms have OEM platform revision?
>> 3rd/4th will get NULL result? Or the opposite?
>
> Correct, from my testing (with limited sample size) and AML code: 5th
> and 6th generation devices have a non-zero OEM platform revision,
> whereas 3rd and 4th gen. devices do not have any (i.e. result will be
> NULL).
>
>>> + if (result) {
>>> + oem_platform_rev = result->integer.value;
>>> + ACPI_FREE(result);
>>> + }
>>> +
>>> + dev_dbg(&dev->dev, "OEM Platform Revision %llu\n", oem_platform_rev);
>>> +
>>> + return oem_platform_rev == 0 ? 0 : -ENODEV;
>> if 3rd/4th do not have this oem rev information while 5th/newer have,
>> why the latter returns NODEV(it actually has this info)?
>
> Since we always expect a non-zero platform revision (for 5th/6th gen.),
> we can initialize it to zero and use that as "unknown"/"not available".
> So if it can not be determined, we return NODEV.
Sorry, small mistake here: If it can be determined (i.e. is 5th or 6th
gen.) then we return NODEV. Not the other way around.
Also to clarify on your last question:
On 7/2/19 3:14 AM, Yu Chen wrote:
>> static int surface_button_add(struct acpi_device *device)
>> {
>> struct surface_button *button;
>> @@ -154,6 +188,10 @@ static int surface_button_add(struct acpi_device *device)
>> strlen(SURFACE_BUTTON_OBJ_NAME)))
>> return -ENODEV;
>>
>> + error = surface_button_check_MSHW0040(device);
>> + if (error)
>> + return error;
>> +
> ditto, 3rd/4th get error=0?
You are correct.
Maximilian
^ permalink raw reply
* Re: [PATCH v2 1/2] platform: Fix device check for surfacepro3_button
From: Yu Chen @ 2019-07-02 1:57 UTC (permalink / raw)
To: Maximilian Luz
Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
Hans de Goede, Darren Hart, Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <d7e17f54-4c33-fa8d-be03-9e507da8e495@gmail.com>
On Tue, Jul 02, 2019 at 03:33:20AM +0200, Maximilian Luz wrote:
> On 7/2/19 3:25 AM, Maximilian Luz wrote:
> > On 7/2/19 3:14 AM, Yu Chen wrote:
> > > On Tue, Jul 02, 2019 at 02:37:39AM +0200, Maximilian Luz wrote:
> > > > +/*
> > > > + * Surface Pro 4 and Surface Book 2 / Surface Pro 2017 use the same device
> > > > + * ID (MSHW0040) for the power/volume buttons. Make sure this is the right
> > > > + * device by checking for the _DSM method and OEM Platform Revision.
> > > > + */
> > > > +static int surface_button_check_MSHW0040(struct acpi_device *dev)
> > > > +{
> > > > + acpi_handle handle = dev->handle;
> > > > + union acpi_object *result;
> > > > + u64 oem_platform_rev = 0;
> > > > +
> > > > + // get OEM platform revision
> > > > + result = acpi_evaluate_dsm_typed(handle, &MSHW0040_DSM_UUID,
> > > > + MSHW0040_DSM_REVISION,
> > > > + MSHW0040_DSM_GET_OMPR,
> > > > + NULL, ACPI_TYPE_INTEGER);
> > > > +
> > > Does it mean, only 5th, 6th and newer platforms have OEM platform revision?
> > > 3rd/4th will get NULL result? Or the opposite?
> >
> > Correct, from my testing (with limited sample size) and AML code: 5th
> > and 6th generation devices have a non-zero OEM platform revision,
> > whereas 3rd and 4th gen. devices do not have any (i.e. result will be
> > NULL).
> >
> > > > + if (result) {
> > > > + oem_platform_rev = result->integer.value;
> > > > + ACPI_FREE(result);
> > > > + }
> > > > +
> > > > + dev_dbg(&dev->dev, "OEM Platform Revision %llu\n", oem_platform_rev);
> > > > +
> > > > + return oem_platform_rev == 0 ? 0 : -ENODEV;
> > > if 3rd/4th do not have this oem rev information while 5th/newer have,
> > > why the latter returns NODEV(it actually has this info)?
> >
> > Since we always expect a non-zero platform revision (for 5th/6th gen.),
> > we can initialize it to zero and use that as "unknown"/"not available".
> > So if it can not be determined, we return NODEV.
>
> Sorry, small mistake here: If it can be determined (i.e. is 5th or 6th
> gen.) then we return NODEV. Not the other way around.
>
How about using a boolean, according to the function name, if a mshw0040 revison
is detected then returns true other wise false. Other than that,
Acked-by: Chen Yu <yu.c.chen@intel.com>
Best,
Chenyu
> Also to clarify on your last question:
>
> On 7/2/19 3:14 AM, Yu Chen wrote:
> > > static int surface_button_add(struct acpi_device *device)
> > > {
> > > struct surface_button *button;
> > > @@ -154,6 +188,10 @@ static int surface_button_add(struct acpi_device *device)
> > > strlen(SURFACE_BUTTON_OBJ_NAME)))
> > > return -ENODEV;
> > > + error = surface_button_check_MSHW0040(device);
> > > + if (error)
> > > + return error;
> > > +
> > ditto, 3rd/4th get error=0?
>
> You are correct.
>
> Maximilian
^ permalink raw reply
* Re: [PATCH v2 1/2] platform: Fix device check for surfacepro3_button
From: Maximilian Luz @ 2019-07-02 2:04 UTC (permalink / raw)
To: Yu Chen
Cc: linux-kernel, linux-input, platform-driver-x86, Dmitry Torokhov,
Hans de Goede, Darren Hart, Andy Shevchenko, Benjamin Tissoires
In-Reply-To: <20190702015717.GA20346@chenyu-office.sh.intel.com>
On 7/2/19 3:57 AM, Yu Chen wrote:
> How about using a boolean, according to the function name, if a mshw0040 revison
> is detected then returns true other wise false. Other than that,
> Acked-by: Chen Yu <yu.c.chen@intel.com>
I can change that if you want me to. Just thought this might be a bit
more flexible in case we ever have to adapt the check for future device
generations.
Thanks,
Maximilian
^ permalink raw reply
* Reminder: 1 open syzbot bug in hid subsystem
From: Eric Biggers @ 2019-07-02 5:04 UTC (permalink / raw)
To: linux-input, Jiri Kosina, Benjamin Tissoires; +Cc: linux-kernel, syzkaller-bugs
[This email was generated by a script. Let me know if you have any suggestions
to make it better, or if you want it re-generated with the latest status.]
Of the currently open syzbot reports against the upstream kernel, I've manually
marked 1 of them as possibly being a bug in the hid subsystem.
If you believe this bug is no longer valid, please close the syzbot report by
sending a '#syz fix', '#syz dup', or '#syz invalid' command in reply to the
original thread, as explained at https://goo.gl/tpsmEJ#status
If you believe I misattributed this bug to the hid subsystem, please let me
know, and if possible forward the report to the correct people or mailing list.
Here is the bug:
--------------------------------------------------------------------------------
Title: INFO: task hung in fsnotify_connector_destroy_workfn (2)
Last occurred: 10 days ago
Reported: 290 days ago
Branches: Mainline and others
Dashboard link: https://syzkaller.appspot.com/bug?id=d6011f00f49a2253c15a60ac102b2ea79e3ee8de
Original thread: https://lkml.kernel.org/lkml/0000000000006364200575dfc280@google.com/T/#u
This bug has a syzkaller reproducer only.
The original thread for this bug received 7 replies; the last was 279 days ago.
If you fix this bug, please add the following tag to the commit:
Reported-by: syzbot+6fb572170402d311dd39@syzkaller.appspotmail.com
If you send any email or patch for this bug, please consider replying to the
original thread. For the git send-email command to use, or tips on how to reply
if the thread isn't in your mailbox, see the "Reply instructions" at
https://lkml.kernel.org/r/0000000000006364200575dfc280@google.com
^ permalink raw reply
* Re: [PATCH 2/4] input: keyboard/mouse/touchscreen/misc: Use dev_get_drvdata()
From: Fuqian Huang @ 2019-07-02 8:18 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
NXP Linux Team, Vladimir Zapolskiy, Sylvain Lemieux,
Laxman Dewangan, Thierry Reding, Jonathan Hunter, Olof Johansson,
H Hartley Sweeten, Arnd Bergmann, Enrico Weigelt, Thomas Gleixner,
Andy Shevchenko, Kate Stewart, Florian Fainelli, Anson Huang
In-Reply-To: <20190701075255.GD172968@dtor-ws>
I am not an expert on this. I just write a coccinelle script to search
this kind of misuse and fix it in a naive way.
Could you tell me about how to use the proper bus accessors? Then I
will fix it up and resend a v2 patch set.
Thanks.
Dmitry Torokhov <dmitry.torokhov@gmail.com> 於 2019年7月1日週一 下午3:52寫道：
>
> Hi Fuqian,
>
> On Mon, Jul 01, 2019 at 11:23:12AM +0800, Fuqian Huang wrote:
> > Using dev_get_drvdata directly.
> >
>
> I prefer using proper bus accessors.
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v5] HID: sb0540: add support for Creative SB0540 IR receivers
From: Benjamin Tissoires @ 2019-07-02 8:29 UTC (permalink / raw)
To: Bastien Nocera
Cc: open list:HID CORE LAYER, lkml, Jiri Kosina, Bastien Nocera
In-Reply-To: <20190701102010.6611-1-hadess@hadess.net>
On Mon, Jul 1, 2019 at 12:20 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> From: Bastien Nocera <bnocera@redhat.com>
>
> Add a new hid driver for the Creative SB0540 IR receiver. This receiver
> is usually coupled with an RM-1500 or an RM-1800 remote control.
>
> The scrollwheels on the RM-1800 remote are not bound, as they are
> labelled for specific audio controls that don't usually exist on most
> systems. They can be remapped using standard Linux keyboard
> remapping tools.
>
> Signed-off-by: Bastien Nocera <bnocera@redhat.com>
> ---
> MAINTAINERS | 6 +
> drivers/hid/Kconfig | 9 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-creative-sb0540.c | 268 ++++++++++++++++++++++++++++++
> drivers/hid/hid-ids.h | 1 +
> 5 files changed, 285 insertions(+)
> create mode 100644 drivers/hid/hid-creative-sb0540.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d0ed735994a5..6fc022d152c8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4227,6 +4227,12 @@ S: Maintained
> F: Documentation/filesystems/cramfs.txt
> F: fs/cramfs/
>
> +CREATIVE SB0540
> +M: Bastien Nocera <hadess@hadess.net>
> +L: linux-input@vger.kernel.org
> +S: Maintained
> +F: drivers/hid/hid-creative-sb0540.c
> +
> CRYPTO API
> M: Herbert Xu <herbert@gondor.apana.org.au>
> M: "David S. Miller" <davem@davemloft.net>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 3872e03d9a59..a70999f9c639 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -273,6 +273,15 @@ config HID_CP2112
> and gpiochip to expose these functions of the CP2112. The
> customizable USB descriptor fields are exposed as sysfs attributes.
>
> +config HID_CREATIVE_SB0540
> + tristate "Creative SB0540 infrared receiver"
> + depends on USB_HID
> + help
> + Support for Creative infrared SB0540-compatible remote controls, such
> + as the RM-1500 and RM-1800 remotes.
> +
> + Say Y here if you want support for Creative SB0540 infrared receiver.
> +
> config HID_CYPRESS
> tristate "Cypress mouse and barcode readers"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index cc5d827c9164..0c03308cfb08 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_HID_ALPS) += hid-alps.o
> obj-$(CONFIG_HID_ACRUX) += hid-axff.o
> obj-$(CONFIG_HID_APPLE) += hid-apple.o
> obj-$(CONFIG_HID_APPLEIR) += hid-appleir.o
> +obj-$(CONFIG_HID_CREATIVE_SB0540) += hid-creative-sb0540.o
> obj-$(CONFIG_HID_ASUS) += hid-asus.o
> obj-$(CONFIG_HID_AUREAL) += hid-aureal.o
> obj-$(CONFIG_HID_BELKIN) += hid-belkin.o
> diff --git a/drivers/hid/hid-creative-sb0540.c b/drivers/hid/hid-creative-sb0540.c
> new file mode 100644
> index 000000000000..6b7c81ccf310
> --- /dev/null
> +++ b/drivers/hid/hid-creative-sb0540.c
> @@ -0,0 +1,268 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * HID driver for the Creative SB0540 receiver
> + *
> + * Copyright (C) 2019 Red Hat Inc. All Rights Reserved
> + *
> + */
> +
> +#include <linux/device.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include "hid-ids.h"
> +
> +MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
> +MODULE_DESCRIPTION("HID Creative SB0540 receiver");
> +MODULE_LICENSE("GPL");
> +
> +static const unsigned short creative_sb0540_key_table[] = {
> + KEY_POWER,
> + KEY_RESERVED, /* text: 24bit */
> + KEY_RESERVED, /* 24bit wheel up */
> + KEY_RESERVED, /* 24bit wheel down */
> + KEY_RESERVED, /* text: CMSS */
> + KEY_RESERVED, /* CMSS wheel Up */
> + KEY_RESERVED, /* CMSS wheel Down */
> + KEY_RESERVED, /* text: EAX */
> + KEY_RESERVED, /* EAX wheel up */
> + KEY_RESERVED, /* EAX wheel down */
> + KEY_RESERVED, /* text: 3D Midi */
> + KEY_RESERVED, /* 3D Midi wheel up */
> + KEY_RESERVED, /* 3D Midi wheel down */
> + KEY_MUTE,
> + KEY_VOLUMEUP,
> + KEY_VOLUMEDOWN,
> + KEY_UP,
> + KEY_LEFT,
> + KEY_RIGHT,
> + KEY_REWIND,
> + KEY_OK,
> + KEY_FASTFORWARD,
> + KEY_DOWN,
> + KEY_AGAIN, /* text: Return, symbol: Jump to */
> + KEY_PLAY, /* text: Start */
> + KEY_ESC, /* text: Cancel */
> + KEY_RECORD,
> + KEY_OPTION,
> + KEY_MENU, /* text: Display */
> + KEY_PREVIOUS,
> + KEY_PLAYPAUSE,
> + KEY_NEXT,
> + KEY_SLOW,
> + KEY_STOP,
> + KEY_NUMERIC_1,
> + KEY_NUMERIC_2,
> + KEY_NUMERIC_3,
> + KEY_NUMERIC_4,
> + KEY_NUMERIC_5,
> + KEY_NUMERIC_6,
> + KEY_NUMERIC_7,
> + KEY_NUMERIC_8,
> + KEY_NUMERIC_9,
> + KEY_NUMERIC_0
> +};
> +
> +/*
> + * Codes and keys from lirc's
> + * remotes/creative/lircd.conf.alsa_usb
> + * order and size must match creative_sb0540_key_table[] above
> + */
> +static const unsigned short creative_sb0540_codes[] = {
> + 0x619E,
> + 0x916E,
> + 0x926D,
> + 0x936C,
> + 0x718E,
> + 0x946B,
> + 0x956A,
> + 0x8C73,
> + 0x9669,
> + 0x9768,
> + 0x9867,
> + 0x9966,
> + 0x9A65,
> + 0x6E91,
> + 0x629D,
> + 0x639C,
> + 0x7B84,
> + 0x6B94,
> + 0x728D,
> + 0x8778,
> + 0x817E,
> + 0x758A,
> + 0x8D72,
> + 0x8E71,
> + 0x8877,
> + 0x7C83,
> + 0x738C,
> + 0x827D,
> + 0x7689,
> + 0x7F80,
> + 0x7986,
> + 0x7A85,
> + 0x7D82,
> + 0x857A,
> + 0x8B74,
> + 0x8F70,
> + 0x906F,
> + 0x8A75,
> + 0x847B,
> + 0x7887,
> + 0x8976,
> + 0x837C,
> + 0x7788,
> + 0x807F
> +};
> +
> +struct creative_sb0540 {
> + struct input_dev *input_dev;
> + struct hid_device *hid;
> + unsigned short keymap[ARRAY_SIZE(creative_sb0540_key_table)];
> +};
> +
> +static inline u64 reverse(u64 data, int bits)
> +{
> + int i;
> + u64 c;
> +
> + c = 0;
> + for (i = 0; i < bits; i++) {
> + c |= (u64) (((data & (((u64) 1) << i)) ? 1 : 0))
> + << (bits - 1 - i);
> + }
> + return (c);
> +}
> +
> +static int get_key(struct creative_sb0540 *creative_sb0540, u64 keycode)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(creative_sb0540_codes); i++) {
> + if (creative_sb0540_codes[i] == keycode)
> + return creative_sb0540->keymap[i];
> + }
> +
> + return 0;
> +
> +}
> +
> +static int creative_sb0540_raw_event(struct hid_device *hid,
> + struct hid_report *report, u8 *data, int len)
> +{
> + struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
> + u64 code, main_code;
> + int key;
> +
> + if (len != 6)
> + goto out;
drivers/hid/hid-creative-sb0540.c: In function 'creative_sb0540_raw_event':
drivers/hid/hid-creative-sb0540.c:157:3: error: label 'out' used but not defined
157 | goto out;
| ^~~~
It would have been nice to at least try to compile it in a tree.
You don't need to compile the whole tree: just clone it, apply your
patch and then `make -j4 M=drivers/hid`
Cheers,
Benjamin
> +
> + /* From daemons/hw_hiddev.c sb0540_rec() in lirc */
> + code = reverse(data[5], 8);
> + main_code = (code << 8) + ((~code) & 0xff);
> +
> + /*
> + * Flip to get values in the same format as
> + * remotes/creative/lircd.conf.alsa_usb in lirc
> + */
> + main_code = ((main_code & 0xff) << 8) +
> + ((main_code & 0xff00) >> 8);
> +
> + key = get_key(creative_sb0540, main_code);
> + if (key == 0 || key == KEY_RESERVED) {
> + hid_err(hid, "Could not get a key for main_code %llX\n",
> + main_code);
> + return 0;
> + }
> +
> + input_report_key(creative_sb0540->input_dev, key, 1);
> + input_report_key(creative_sb0540->input_dev, key, 0);
> + input_sync(creative_sb0540->input_dev);
> +
> + /* let hidraw and hiddev handle the report */
> + return 0;
> +}
> +
> +static int creative_sb0540_input_configured(struct hid_device *hid,
> + struct hid_input *hidinput)
> +{
> + struct input_dev *input_dev = hidinput->input;
> + struct creative_sb0540 *creative_sb0540 = hid_get_drvdata(hid);
> + int i;
> +
> + creative_sb0540->input_dev = input_dev;
> +
> + input_dev->keycode = creative_sb0540->keymap;
> + input_dev->keycodesize = sizeof(unsigned short);
> + input_dev->keycodemax = ARRAY_SIZE(creative_sb0540->keymap);
> +
> + input_dev->evbit[0] = BIT(EV_KEY) | BIT(EV_REP);
> +
> + memcpy(creative_sb0540->keymap, creative_sb0540_key_table,
> + sizeof(creative_sb0540->keymap));
> + for (i = 0; i < ARRAY_SIZE(creative_sb0540_key_table); i++)
> + set_bit(creative_sb0540->keymap[i], input_dev->keybit);
> + clear_bit(KEY_RESERVED, input_dev->keybit);
> +
> + return 0;
> +}
> +
> +static int creative_sb0540_input_mapping(struct hid_device *hid,
> + struct hid_input *hi, struct hid_field *field,
> + struct hid_usage *usage, unsigned long **bit, int *max)
> +{
> + /*
> + * We are remapping the keys ourselves, so ignore the hid-input
> + * keymap processing.
> + */
> + return -1;
> +}
> +
> +static int creative_sb0540_probe(struct hid_device *hid,
> + const struct hid_device_id *id)
> +{
> + int ret;
> + struct creative_sb0540 *creative_sb0540;
> +
> + creative_sb0540 = devm_kzalloc(&hid->dev,
> + sizeof(struct creative_sb0540), GFP_KERNEL);
> +
> + if (!creative_sb0540)
> + return -ENOMEM;
> +
> + creative_sb0540->hid = hid;
> +
> + /* force input as some remotes bypass the input registration */
> + hid->quirks |= HID_QUIRK_HIDINPUT_FORCE;
> +
> + hid_set_drvdata(hid, creative_sb0540);
> +
> + ret = hid_parse(hid);
> + if (ret) {
> + hid_err(hid, "parse failed\n");
> + return ret;
> + }
> +
> + ret = hid_hw_start(hid, HID_CONNECT_DEFAULT);
> + if (ret) {
> + hid_err(hid, "hw start failed\n");
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static const struct hid_device_id creative_sb0540_devices[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB0540) },
> + { }
> +};
> +MODULE_DEVICE_TABLE(hid, creative_sb0540_devices);
> +
> +static struct hid_driver creative_sb0540_driver = {
> + .name = "creative-sb0540",
> + .id_table = creative_sb0540_devices,
> + .raw_event = creative_sb0540_raw_event,
> + .input_configured = creative_sb0540_input_configured,
> + .probe = creative_sb0540_probe,
> + .input_mapping = creative_sb0540_input_mapping,
> +};
> +module_hid_driver(creative_sb0540_driver);
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 826324997686..206b7065da86 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -312,6 +312,7 @@
> #define USB_VENDOR_ID_CREATIVELABS 0x041e
> #define USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51 0x322c
> #define USB_DEVICE_ID_PRODIKEYS_PCMIDI 0x2801
> +#define USB_DEVICE_ID_CREATIVE_SB0540 0x3100
>
> #define USB_VENDOR_ID_CVTOUCH 0x1ff7
> #define USB_DEVICE_ID_CVTOUCH_SCREEN 0x0013
> --
> 2.21.0
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox