| Message ID | 1376718381-16924-1-git-send-email-shc_work@mail.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Alexander, On Sat, Aug 17, 2013 at 09:46:20AM +0400, Alexander Shiyan wrote: > This patch is a preparation mc13xxx powerbutton driver to support > MC13892 and support the probe through the DT. As this patch already mention by itself, it's doing to much at once. And by looking at it, it realy does more than it tells here. Can you rework that into seperate readable tasks. In this patch you nearly rewrite the whole file. That runs the review impossible. > Signed-off-by: Alexander Shiyan <shc_work@mail.ru> > --- > arch/arm/mach-imx/mach-mx31moboard.c | 9 +- > drivers/input/misc/mc13783-pwrbutton.c | 332 +++++++++++++-------------------- > include/linux/mfd/mc13xxx.h | 28 +-- > 3 files changed, 151 insertions(+), 218 deletions(-) > > diff --git a/arch/arm/mach-imx/mach-mx31moboard.c b/arch/arm/mach-imx/mach-mx31moboard.c > index 6f424ec..2a8aa43 100644 > --- a/arch/arm/mach-imx/mach-mx31moboard.c > +++ b/arch/arm/mach-imx/mach-mx31moboard.c > @@ -276,9 +276,12 @@ static struct mc13xxx_leds_platform_data moboard_leds = { > }; > > static struct mc13xxx_buttons_platform_data moboard_buttons = { > - .b1on_flags = MC13783_BUTTON_DBNC_750MS | MC13783_BUTTON_ENABLE | > - MC13783_BUTTON_POL_INVERT, > - .b1on_key = KEY_POWER, > + .buttons[0] = { > + .keycode = KEY_POWER, > + .flags = MC13XXX_BUTTON_ENABLE | > + MC13XXX_BUTTON_DBNC_750MS | > + MC13XXX_BUTTON_POL_INVERT, > + }, > }; > > static struct mc13xxx_codec_platform_data moboard_codec = { > diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c > index d0277a7..aaaacef 100644 > --- a/drivers/input/misc/mc13783-pwrbutton.c > +++ b/drivers/input/misc/mc13783-pwrbutton.c > @@ -24,248 +24,176 @@ > #include <linux/kernel.h> > #include <linux/errno.h> > #include <linux/input.h> > -#include <linux/interrupt.h> > #include <linux/platform_device.h> > #include <linux/mfd/mc13783.h> > -#include <linux/sched.h> > -#include <linux/slab.h> > - > -struct mc13783_pwrb { > - struct input_dev *pwr; > - struct mc13xxx *mc13783; > -#define MC13783_PWRB_B1_POL_INVERT (1 << 0) > -#define MC13783_PWRB_B2_POL_INVERT (1 << 1) > -#define MC13783_PWRB_B3_POL_INVERT (1 << 2) > - int flags; > - unsigned short keymap[3]; > + > +struct mc13xxx_button_def { > + unsigned int irq; > + unsigned int sense_bit; > }; > > -#define MC13783_REG_INTERRUPT_SENSE_1 5 > -#define MC13783_IRQSENSE1_ONOFD1S (1 << 3) > -#define MC13783_IRQSENSE1_ONOFD2S (1 << 4) > -#define MC13783_IRQSENSE1_ONOFD3S (1 << 5) > +struct mc13xxx_pwrb_devtype { > + struct mc13xxx_button_def btn_def[MAX13XXX_NUM_BUTTONS]; > +}; > > -#define MC13783_REG_POWER_CONTROL_2 15 > -#define MC13783_POWER_CONTROL_2_ON1BDBNC 4 > -#define MC13783_POWER_CONTROL_2_ON2BDBNC 6 > -#define MC13783_POWER_CONTROL_2_ON3BDBNC 8 > -#define MC13783_POWER_CONTROL_2_ON1BRSTEN (1 << 1) > -#define MC13783_POWER_CONTROL_2_ON2BRSTEN (1 << 2) > -#define MC13783_POWER_CONTROL_2_ON3BRSTEN (1 << 3) > +struct mc13xxx_pwrb { > + struct mc13xxx_pwrb_devtype *devtype; > + unsigned int enabled; > + unsigned int inverted; > + u16 btn_code[MAX13XXX_NUM_BUTTONS]; > + struct input_dev *input; > + struct mc13xxx *mc13xxx; > +}; > > -static irqreturn_t button_irq(int irq, void *_priv) > -{ > - struct mc13783_pwrb *priv = _priv; > - int val; > - > - mc13xxx_irq_ack(priv->mc13783, irq); > - mc13xxx_reg_read(priv->mc13783, MC13783_REG_INTERRUPT_SENSE_1, &val); > - > - switch (irq) { > - case MC13783_IRQ_ONOFD1: > - val = val & MC13783_IRQSENSE1_ONOFD1S ? 1 : 0; > - if (priv->flags & MC13783_PWRB_B1_POL_INVERT) > - val ^= 1; > - input_report_key(priv->pwr, priv->keymap[0], val); > - break; > - > - case MC13783_IRQ_ONOFD2: > - val = val & MC13783_IRQSENSE1_ONOFD2S ? 1 : 0; > - if (priv->flags & MC13783_PWRB_B2_POL_INVERT) > - val ^= 1; > - input_report_key(priv->pwr, priv->keymap[1], val); > - break; > - > - case MC13783_IRQ_ONOFD3: > - val = val & MC13783_IRQSENSE1_ONOFD3S ? 1 : 0; > - if (priv->flags & MC13783_PWRB_B3_POL_INVERT) > - val ^= 1; > - input_report_key(priv->pwr, priv->keymap[2], val); > - break; > - } > +#define MC13XXX_REG_INTERRUPT_SENSE_1 5 > +#define MC13XXX_REG_POWER_CONTROL_2 15 > > - input_sync(priv->pwr); > +static irqreturn_t mc13xxx_pwrbutton_irq(int irq, void *data) > +{ > + struct mc13xxx_pwrb *priv = data; > + unsigned int i, val; > + > + mc13xxx_irq_ack(priv->mc13xxx, irq); > + mc13xxx_reg_read(priv->mc13xxx, MC13XXX_REG_INTERRUPT_SENSE_1, &val); > + > + for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) > + if (irq == priv->devtype->btn_def[i].irq) { > + val = !!(val & priv->devtype->btn_def[i].sense_bit); > + if (priv->inverted & BIT(i)) > + val = !val; > + input_report_key(priv->input, priv->btn_code[i], val); > + input_sync(priv->input); > + break; > + } > > return IRQ_HANDLED; > } > > -static int mc13783_pwrbutton_probe(struct platform_device *pdev) > +static int __init mc13xxx_pwrbutton_probe(struct platform_device *pdev) > { > - const struct mc13xxx_buttons_platform_data *pdata; > - struct mc13xxx *mc13783 = dev_get_drvdata(pdev->dev.parent); > - struct input_dev *pwr; > - struct mc13783_pwrb *priv; > - int err = 0; > - int reg = 0; > - > - pdata = dev_get_platdata(&pdev->dev); > + struct mc13xxx_buttons_platform_data *pdata = > + dev_get_platdata(&pdev->dev); > + struct mc13xxx *mc13xxx = dev_get_drvdata(pdev->dev.parent); > + struct mc13xxx_pwrb_devtype *devtype = > + (struct mc13xxx_pwrb_devtype *)pdev->id_entry->driver_data; > + struct mc13xxx_pwrb *priv; > + int i, reg = 0, ret = -EINVAL; > + > if (!pdata) { > - dev_err(&pdev->dev, "missing platform data\n"); > + dev_err(&pdev->dev, "Missing platform data\n"); > return -ENODEV; > } > > - pwr = input_allocate_device(); > - if (!pwr) { > - dev_dbg(&pdev->dev, "Can't allocate power button\n"); > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) > return -ENOMEM; > - } > - > - priv = kzalloc(sizeof(*priv), GFP_KERNEL); > - if (!priv) { > - err = -ENOMEM; > - dev_dbg(&pdev->dev, "Can't allocate power button\n"); > - goto free_input_dev; > - } > - > - reg |= (pdata->b1on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC; > - reg |= (pdata->b2on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC; > - reg |= (pdata->b3on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC; > - > - priv->pwr = pwr; > - priv->mc13783 = mc13783; > > - mc13xxx_lock(mc13783); > - > - if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) { > - priv->keymap[0] = pdata->b1on_key; > - if (pdata->b1on_key != KEY_RESERVED) > - __set_bit(pdata->b1on_key, pwr->keybit); > - > - if (pdata->b1on_flags & MC13783_BUTTON_POL_INVERT) > - priv->flags |= MC13783_PWRB_B1_POL_INVERT; > + priv->input = devm_input_allocate_device(&pdev->dev); > + if (!priv->input) > + return -ENOMEM; > > - if (pdata->b1on_flags & MC13783_BUTTON_RESET_EN) > - reg |= MC13783_POWER_CONTROL_2_ON1BRSTEN; > + priv->mc13xxx = mc13xxx; > + priv->devtype = devtype; > + platform_set_drvdata(pdev, priv); > > - err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD1, > - button_irq, "b1on", priv); > - if (err) { > - dev_dbg(&pdev->dev, "Can't request irq\n"); > - goto free_priv; > - } > + for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) { > + u16 code, invert, reset, debounce; > + > + if (!(pdata->buttons[i].flags & MC13XXX_BUTTON_ENABLE)) > + continue; > + code = pdata->buttons[i].keycode; > + invert = !!(pdata->buttons[i].flags & > + MC13XXX_BUTTON_POL_INVERT); > + reset = !!(pdata->buttons[i].flags & > + MC13XXX_BUTTON_RESET_EN); > + debounce = pdata->buttons[i].flags; > + > + priv->btn_code[i] = code; > + if (code != KEY_RESERVED) > + __set_bit(code, priv->input->keybit); > + > + priv->enabled |= BIT(i); > + priv->inverted |= invert << i; > + reg |= reset << (i + 1); > + reg |= (debounce & 0x03) << (4 + i * 2); > } > > - if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) { > - priv->keymap[1] = pdata->b2on_key; > - if (pdata->b2on_key != KEY_RESERVED) > - __set_bit(pdata->b2on_key, pwr->keybit); > - > - if (pdata->b2on_flags & MC13783_BUTTON_POL_INVERT) > - priv->flags |= MC13783_PWRB_B2_POL_INVERT; > - > - if (pdata->b2on_flags & MC13783_BUTTON_RESET_EN) > - reg |= MC13783_POWER_CONTROL_2_ON2BRSTEN; > - > - err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD2, > - button_irq, "b2on", priv); > - if (err) { > - dev_dbg(&pdev->dev, "Can't request irq\n"); > - goto free_irq_b1; > + mc13xxx_lock(mc13xxx); > + > + mc13xxx_reg_rmw(mc13xxx, MC13XXX_REG_POWER_CONTROL_2, 0x3fe, reg); > + > + for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) > + if (priv->enabled & BIT(i)) { > + ret = mc13xxx_irq_request(mc13xxx, > + devtype->btn_def[i].irq, > + mc13xxx_pwrbutton_irq, NULL, > + priv); > + if (ret) { > + dev_err(&pdev->dev, "Can't request IRQ%i: %i\n", > + devtype->btn_def[i].irq, ret); > + priv->enabled &= ~BIT(i); > + } > } > - } > > - if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) { > - priv->keymap[2] = pdata->b3on_key; > - if (pdata->b3on_key != KEY_RESERVED) > - __set_bit(pdata->b3on_key, pwr->keybit); > + mc13xxx_unlock(mc13xxx); > > - if (pdata->b3on_flags & MC13783_BUTTON_POL_INVERT) > - priv->flags |= MC13783_PWRB_B3_POL_INVERT; > + priv->input->name = "mc13xxx_pwrbutton"; > + priv->input->phys = "mc13xxx_pwrbutton/input0"; > + priv->input->id.bustype = BUS_HOST; > + priv->input->id.vendor = 0x0001; > + priv->input->id.product = 0x0001; > + priv->input->id.version = 0x0100; > + priv->input->keycode = priv->btn_code; > + priv->input->keycodemax = ARRAY_SIZE(priv->btn_code); > + priv->input->keycodesize = sizeof(priv->btn_code[0]); > + __set_bit(EV_KEY, priv->input->evbit); > > - if (pdata->b3on_flags & MC13783_BUTTON_RESET_EN) > - reg |= MC13783_POWER_CONTROL_2_ON3BRSTEN; > - > - err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD3, > - button_irq, "b3on", priv); > - if (err) { > - dev_dbg(&pdev->dev, "Can't request irq: %d\n", err); > - goto free_irq_b2; > - } > - } > + input_set_drvdata(priv->input, priv); > > - mc13xxx_reg_rmw(mc13783, MC13783_REG_POWER_CONTROL_2, 0x3FE, reg); > + ret = input_register_device(priv->input); > + if (ret) > + dev_err(&pdev->dev, "Can't register input device: %i\n", ret); > > - mc13xxx_unlock(mc13783); > - > - pwr->name = "mc13783_pwrbutton"; > - pwr->phys = "mc13783_pwrbutton/input0"; > - pwr->dev.parent = &pdev->dev; > - > - pwr->keycode = priv->keymap; > - pwr->keycodemax = ARRAY_SIZE(priv->keymap); > - pwr->keycodesize = sizeof(priv->keymap[0]); > - __set_bit(EV_KEY, pwr->evbit); > - > - err = input_register_device(pwr); > - if (err) { > - dev_dbg(&pdev->dev, "Can't register power button: %d\n", err); > - goto free_irq; > - } > - > - platform_set_drvdata(pdev, priv); > - > - return 0; > - > -free_irq: > - mc13xxx_lock(mc13783); > - > - if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) > - mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD3, priv); > - > -free_irq_b2: > - if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) > - mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD2, priv); > - > -free_irq_b1: > - if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) > - mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD1, priv); > - > -free_priv: > - mc13xxx_unlock(mc13783); > - kfree(priv); > - > -free_input_dev: > - input_free_device(pwr); > - > - return err; > + return ret; > } > > -static int mc13783_pwrbutton_remove(struct platform_device *pdev) > +static int mc13xxx_pwrbutton_remove(struct platform_device *pdev) > { > - struct mc13783_pwrb *priv = platform_get_drvdata(pdev); > - const struct mc13xxx_buttons_platform_data *pdata; > - > - pdata = dev_get_platdata(&pdev->dev); > - > - mc13xxx_lock(priv->mc13783); > - > - if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) > - mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD3, priv); > - if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) > - mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD2, priv); > - if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) > - mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD1, priv); > + struct mc13xxx_pwrb *priv = platform_get_drvdata(pdev); > + int i; > > - mc13xxx_unlock(priv->mc13783); > - > - input_unregister_device(priv->pwr); > - kfree(priv); > + mc13xxx_lock(priv->mc13xxx); > + for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) > + if (priv->enabled & BIT(i)) > + mc13xxx_irq_free(priv->mc13xxx, > + priv->devtype->btn_def[i].irq, priv); > + mc13xxx_unlock(priv->mc13xxx); > > return 0; > } > > -static struct platform_driver mc13783_pwrbutton_driver = { > - .probe = mc13783_pwrbutton_probe, > - .remove = mc13783_pwrbutton_remove, > - .driver = { > - .name = "mc13783-pwrbutton", > +static const struct mc13xxx_pwrb_devtype mc13783_pwrb_devtype = { > + .btn_def[0] = { MC13783_IRQ_ONOFD1, BIT(3) }, > + .btn_def[1] = { MC13783_IRQ_ONOFD2, BIT(4) }, > + .btn_def[2] = { MC13783_IRQ_ONOFD3, BIT(5) } > +}; > + > +static const struct platform_device_id mc13xxx_pwrbutton_id_table[] = { > + { "mc13783-pwrbutton", (kernel_ulong_t)&mc13783_pwrb_devtype }, > + { } > +}; > +MODULE_DEVICE_TABLE(platform, mc13xxx_pwrbutton_id_table); > + > +static struct platform_driver mc13xxx_pwrbutton_driver = { > + .driver = { > + .name = "mc13xxx-pwrbutton", > .owner = THIS_MODULE, > }, > + .remove = mc13xxx_pwrbutton_remove, > + .id_table = mc13xxx_pwrbutton_id_table, > }; > +module_platform_driver_probe(mc13xxx_pwrbutton_driver, mc13xxx_pwrbutton_probe); > > -module_platform_driver(mc13783_pwrbutton_driver); > - > -MODULE_ALIAS("platform:mc13783-pwrbutton"); > MODULE_DESCRIPTION("MC13783 Power Button"); > MODULE_LICENSE("GPL v2"); > MODULE_AUTHOR("Philippe Retornaz"); > diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h > index 41ed592..b895538 100644 > --- a/include/linux/mfd/mc13xxx.h > +++ b/include/linux/mfd/mc13xxx.h > @@ -142,20 +142,22 @@ struct mc13xxx_leds_platform_data { > u32 led_control[MAX_LED_CONTROL_REGS]; > }; > > +#define MAX13XXX_NUM_BUTTONS 3 > + > +struct mc13xxx_button { > + u16 keycode; > + unsigned int flags; > +#define MC13XXX_BUTTON_DBNC_0MS 0 > +#define MC13XXX_BUTTON_DBNC_30MS 1 > +#define MC13XXX_BUTTON_DBNC_150MS 2 > +#define MC13XXX_BUTTON_DBNC_750MS 3 > +#define MC13XXX_BUTTON_ENABLE (1 << 2) > +#define MC13XXX_BUTTON_POL_INVERT (1 << 3) > +#define MC13XXX_BUTTON_RESET_EN (1 << 4) > +}; > + > struct mc13xxx_buttons_platform_data { > -#define MC13783_BUTTON_DBNC_0MS 0 > -#define MC13783_BUTTON_DBNC_30MS 1 > -#define MC13783_BUTTON_DBNC_150MS 2 > -#define MC13783_BUTTON_DBNC_750MS 3 > -#define MC13783_BUTTON_ENABLE (1 << 2) > -#define MC13783_BUTTON_POL_INVERT (1 << 3) > -#define MC13783_BUTTON_RESET_EN (1 << 4) > - int b1on_flags; > - unsigned short b1on_key; > - int b2on_flags; > - unsigned short b2on_key; > - int b3on_flags; > - unsigned short b3on_key; > + struct mc13xxx_button buttons[MAX13XXX_NUM_BUTTONS]; > }; > > struct mc13xxx_ts_platform_data { > -- > 1.8.1.5 > >
On Sat, 17 Aug 2013 08:20:12 +0200 Michael Grzeschik <mgr@pengutronix.de> wrote: > On Sat, Aug 17, 2013 at 09:46:20AM +0400, Alexander Shiyan wrote: > > This patch is a preparation mc13xxx powerbutton driver to support > > MC13892 and support the probe through the DT. > > As this patch already mention by itself, it's doing to much at once. > And by looking at it, it realy does more than it tells here. > > Can you rework that into seperate readable tasks. In this patch you > nearly rewrite the whole file. That runs the review impossible. All changes in this patch are addicted to each other, can not be divided. As an alternative, I can do all as a new driver. Thanks.
On Sat, Aug 17, 2013 at 10:53:05AM +0400, Alexander Shiyan wrote: > On Sat, 17 Aug 2013 08:20:12 +0200 > Michael Grzeschik <mgr@pengutronix.de> wrote: > > > On Sat, Aug 17, 2013 at 09:46:20AM +0400, Alexander Shiyan wrote: > > > This patch is a preparation mc13xxx powerbutton driver to support > > > MC13892 and support the probe through the DT. > > > > As this patch already mention by itself, it's doing to much at once. > > And by looking at it, it realy does more than it tells here. > > > > Can you rework that into seperate readable tasks. In this patch you > > nearly rewrite the whole file. That runs the review impossible. > > All changes in this patch are addicted to each other, can not be divided. I find this hard to believe. Here are some changes that can be separated easily: - Rename defines from MC13XXX_BUTTON_ to MC13783_BUTTON_ - convert module_platform_driver to module_platform_driver_probe, why do you change this anyway? - convert from kzalloc to devm_kzalloc - rewrite error message from "missing platform data" to "Missing platform data" Drop these changes or make them separate patches and your original patch would be smaller already. Sascha
diff --git a/arch/arm/mach-imx/mach-mx31moboard.c b/arch/arm/mach-imx/mach-mx31moboard.c index 6f424ec..2a8aa43 100644 --- a/arch/arm/mach-imx/mach-mx31moboard.c +++ b/arch/arm/mach-imx/mach-mx31moboard.c @@ -276,9 +276,12 @@ static struct mc13xxx_leds_platform_data moboard_leds = { }; static struct mc13xxx_buttons_platform_data moboard_buttons = { - .b1on_flags = MC13783_BUTTON_DBNC_750MS | MC13783_BUTTON_ENABLE | - MC13783_BUTTON_POL_INVERT, - .b1on_key = KEY_POWER, + .buttons[0] = { + .keycode = KEY_POWER, + .flags = MC13XXX_BUTTON_ENABLE | + MC13XXX_BUTTON_DBNC_750MS | + MC13XXX_BUTTON_POL_INVERT, + }, }; static struct mc13xxx_codec_platform_data moboard_codec = { diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c index d0277a7..aaaacef 100644 --- a/drivers/input/misc/mc13783-pwrbutton.c +++ b/drivers/input/misc/mc13783-pwrbutton.c @@ -24,248 +24,176 @@ #include <linux/kernel.h> #include <linux/errno.h> #include <linux/input.h> -#include <linux/interrupt.h> #include <linux/platform_device.h> #include <linux/mfd/mc13783.h> -#include <linux/sched.h> -#include <linux/slab.h> - -struct mc13783_pwrb { - struct input_dev *pwr; - struct mc13xxx *mc13783; -#define MC13783_PWRB_B1_POL_INVERT (1 << 0) -#define MC13783_PWRB_B2_POL_INVERT (1 << 1) -#define MC13783_PWRB_B3_POL_INVERT (1 << 2) - int flags; - unsigned short keymap[3]; + +struct mc13xxx_button_def { + unsigned int irq; + unsigned int sense_bit; }; -#define MC13783_REG_INTERRUPT_SENSE_1 5 -#define MC13783_IRQSENSE1_ONOFD1S (1 << 3) -#define MC13783_IRQSENSE1_ONOFD2S (1 << 4) -#define MC13783_IRQSENSE1_ONOFD3S (1 << 5) +struct mc13xxx_pwrb_devtype { + struct mc13xxx_button_def btn_def[MAX13XXX_NUM_BUTTONS]; +}; -#define MC13783_REG_POWER_CONTROL_2 15 -#define MC13783_POWER_CONTROL_2_ON1BDBNC 4 -#define MC13783_POWER_CONTROL_2_ON2BDBNC 6 -#define MC13783_POWER_CONTROL_2_ON3BDBNC 8 -#define MC13783_POWER_CONTROL_2_ON1BRSTEN (1 << 1) -#define MC13783_POWER_CONTROL_2_ON2BRSTEN (1 << 2) -#define MC13783_POWER_CONTROL_2_ON3BRSTEN (1 << 3) +struct mc13xxx_pwrb { + struct mc13xxx_pwrb_devtype *devtype; + unsigned int enabled; + unsigned int inverted; + u16 btn_code[MAX13XXX_NUM_BUTTONS]; + struct input_dev *input; + struct mc13xxx *mc13xxx; +}; -static irqreturn_t button_irq(int irq, void *_priv) -{ - struct mc13783_pwrb *priv = _priv; - int val; - - mc13xxx_irq_ack(priv->mc13783, irq); - mc13xxx_reg_read(priv->mc13783, MC13783_REG_INTERRUPT_SENSE_1, &val); - - switch (irq) { - case MC13783_IRQ_ONOFD1: - val = val & MC13783_IRQSENSE1_ONOFD1S ? 1 : 0; - if (priv->flags & MC13783_PWRB_B1_POL_INVERT) - val ^= 1; - input_report_key(priv->pwr, priv->keymap[0], val); - break; - - case MC13783_IRQ_ONOFD2: - val = val & MC13783_IRQSENSE1_ONOFD2S ? 1 : 0; - if (priv->flags & MC13783_PWRB_B2_POL_INVERT) - val ^= 1; - input_report_key(priv->pwr, priv->keymap[1], val); - break; - - case MC13783_IRQ_ONOFD3: - val = val & MC13783_IRQSENSE1_ONOFD3S ? 1 : 0; - if (priv->flags & MC13783_PWRB_B3_POL_INVERT) - val ^= 1; - input_report_key(priv->pwr, priv->keymap[2], val); - break; - } +#define MC13XXX_REG_INTERRUPT_SENSE_1 5 +#define MC13XXX_REG_POWER_CONTROL_2 15 - input_sync(priv->pwr); +static irqreturn_t mc13xxx_pwrbutton_irq(int irq, void *data) +{ + struct mc13xxx_pwrb *priv = data; + unsigned int i, val; + + mc13xxx_irq_ack(priv->mc13xxx, irq); + mc13xxx_reg_read(priv->mc13xxx, MC13XXX_REG_INTERRUPT_SENSE_1, &val); + + for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) + if (irq == priv->devtype->btn_def[i].irq) { + val = !!(val & priv->devtype->btn_def[i].sense_bit); + if (priv->inverted & BIT(i)) + val = !val; + input_report_key(priv->input, priv->btn_code[i], val); + input_sync(priv->input); + break; + } return IRQ_HANDLED; } -static int mc13783_pwrbutton_probe(struct platform_device *pdev) +static int __init mc13xxx_pwrbutton_probe(struct platform_device *pdev) { - const struct mc13xxx_buttons_platform_data *pdata; - struct mc13xxx *mc13783 = dev_get_drvdata(pdev->dev.parent); - struct input_dev *pwr; - struct mc13783_pwrb *priv; - int err = 0; - int reg = 0; - - pdata = dev_get_platdata(&pdev->dev); + struct mc13xxx_buttons_platform_data *pdata = + dev_get_platdata(&pdev->dev); + struct mc13xxx *mc13xxx = dev_get_drvdata(pdev->dev.parent); + struct mc13xxx_pwrb_devtype *devtype = + (struct mc13xxx_pwrb_devtype *)pdev->id_entry->driver_data; + struct mc13xxx_pwrb *priv; + int i, reg = 0, ret = -EINVAL; + if (!pdata) { - dev_err(&pdev->dev, "missing platform data\n"); + dev_err(&pdev->dev, "Missing platform data\n"); return -ENODEV; } - pwr = input_allocate_device(); - if (!pwr) { - dev_dbg(&pdev->dev, "Can't allocate power button\n"); + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) return -ENOMEM; - } - - priv = kzalloc(sizeof(*priv), GFP_KERNEL); - if (!priv) { - err = -ENOMEM; - dev_dbg(&pdev->dev, "Can't allocate power button\n"); - goto free_input_dev; - } - - reg |= (pdata->b1on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC; - reg |= (pdata->b2on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC; - reg |= (pdata->b3on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC; - - priv->pwr = pwr; - priv->mc13783 = mc13783; - mc13xxx_lock(mc13783); - - if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) { - priv->keymap[0] = pdata->b1on_key; - if (pdata->b1on_key != KEY_RESERVED) - __set_bit(pdata->b1on_key, pwr->keybit); - - if (pdata->b1on_flags & MC13783_BUTTON_POL_INVERT) - priv->flags |= MC13783_PWRB_B1_POL_INVERT; + priv->input = devm_input_allocate_device(&pdev->dev); + if (!priv->input) + return -ENOMEM; - if (pdata->b1on_flags & MC13783_BUTTON_RESET_EN) - reg |= MC13783_POWER_CONTROL_2_ON1BRSTEN; + priv->mc13xxx = mc13xxx; + priv->devtype = devtype; + platform_set_drvdata(pdev, priv); - err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD1, - button_irq, "b1on", priv); - if (err) { - dev_dbg(&pdev->dev, "Can't request irq\n"); - goto free_priv; - } + for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) { + u16 code, invert, reset, debounce; + + if (!(pdata->buttons[i].flags & MC13XXX_BUTTON_ENABLE)) + continue; + code = pdata->buttons[i].keycode; + invert = !!(pdata->buttons[i].flags & + MC13XXX_BUTTON_POL_INVERT); + reset = !!(pdata->buttons[i].flags & + MC13XXX_BUTTON_RESET_EN); + debounce = pdata->buttons[i].flags; + + priv->btn_code[i] = code; + if (code != KEY_RESERVED) + __set_bit(code, priv->input->keybit); + + priv->enabled |= BIT(i); + priv->inverted |= invert << i; + reg |= reset << (i + 1); + reg |= (debounce & 0x03) << (4 + i * 2); } - if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) { - priv->keymap[1] = pdata->b2on_key; - if (pdata->b2on_key != KEY_RESERVED) - __set_bit(pdata->b2on_key, pwr->keybit); - - if (pdata->b2on_flags & MC13783_BUTTON_POL_INVERT) - priv->flags |= MC13783_PWRB_B2_POL_INVERT; - - if (pdata->b2on_flags & MC13783_BUTTON_RESET_EN) - reg |= MC13783_POWER_CONTROL_2_ON2BRSTEN; - - err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD2, - button_irq, "b2on", priv); - if (err) { - dev_dbg(&pdev->dev, "Can't request irq\n"); - goto free_irq_b1; + mc13xxx_lock(mc13xxx); + + mc13xxx_reg_rmw(mc13xxx, MC13XXX_REG_POWER_CONTROL_2, 0x3fe, reg); + + for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) + if (priv->enabled & BIT(i)) { + ret = mc13xxx_irq_request(mc13xxx, + devtype->btn_def[i].irq, + mc13xxx_pwrbutton_irq, NULL, + priv); + if (ret) { + dev_err(&pdev->dev, "Can't request IRQ%i: %i\n", + devtype->btn_def[i].irq, ret); + priv->enabled &= ~BIT(i); + } } - } - if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) { - priv->keymap[2] = pdata->b3on_key; - if (pdata->b3on_key != KEY_RESERVED) - __set_bit(pdata->b3on_key, pwr->keybit); + mc13xxx_unlock(mc13xxx); - if (pdata->b3on_flags & MC13783_BUTTON_POL_INVERT) - priv->flags |= MC13783_PWRB_B3_POL_INVERT; + priv->input->name = "mc13xxx_pwrbutton"; + priv->input->phys = "mc13xxx_pwrbutton/input0"; + priv->input->id.bustype = BUS_HOST; + priv->input->id.vendor = 0x0001; + priv->input->id.product = 0x0001; + priv->input->id.version = 0x0100; + priv->input->keycode = priv->btn_code; + priv->input->keycodemax = ARRAY_SIZE(priv->btn_code); + priv->input->keycodesize = sizeof(priv->btn_code[0]); + __set_bit(EV_KEY, priv->input->evbit); - if (pdata->b3on_flags & MC13783_BUTTON_RESET_EN) - reg |= MC13783_POWER_CONTROL_2_ON3BRSTEN; - - err = mc13xxx_irq_request(mc13783, MC13783_IRQ_ONOFD3, - button_irq, "b3on", priv); - if (err) { - dev_dbg(&pdev->dev, "Can't request irq: %d\n", err); - goto free_irq_b2; - } - } + input_set_drvdata(priv->input, priv); - mc13xxx_reg_rmw(mc13783, MC13783_REG_POWER_CONTROL_2, 0x3FE, reg); + ret = input_register_device(priv->input); + if (ret) + dev_err(&pdev->dev, "Can't register input device: %i\n", ret); - mc13xxx_unlock(mc13783); - - pwr->name = "mc13783_pwrbutton"; - pwr->phys = "mc13783_pwrbutton/input0"; - pwr->dev.parent = &pdev->dev; - - pwr->keycode = priv->keymap; - pwr->keycodemax = ARRAY_SIZE(priv->keymap); - pwr->keycodesize = sizeof(priv->keymap[0]); - __set_bit(EV_KEY, pwr->evbit); - - err = input_register_device(pwr); - if (err) { - dev_dbg(&pdev->dev, "Can't register power button: %d\n", err); - goto free_irq; - } - - platform_set_drvdata(pdev, priv); - - return 0; - -free_irq: - mc13xxx_lock(mc13783); - - if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) - mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD3, priv); - -free_irq_b2: - if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) - mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD2, priv); - -free_irq_b1: - if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) - mc13xxx_irq_free(mc13783, MC13783_IRQ_ONOFD1, priv); - -free_priv: - mc13xxx_unlock(mc13783); - kfree(priv); - -free_input_dev: - input_free_device(pwr); - - return err; + return ret; } -static int mc13783_pwrbutton_remove(struct platform_device *pdev) +static int mc13xxx_pwrbutton_remove(struct platform_device *pdev) { - struct mc13783_pwrb *priv = platform_get_drvdata(pdev); - const struct mc13xxx_buttons_platform_data *pdata; - - pdata = dev_get_platdata(&pdev->dev); - - mc13xxx_lock(priv->mc13783); - - if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) - mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD3, priv); - if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) - mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD2, priv); - if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) - mc13xxx_irq_free(priv->mc13783, MC13783_IRQ_ONOFD1, priv); + struct mc13xxx_pwrb *priv = platform_get_drvdata(pdev); + int i; - mc13xxx_unlock(priv->mc13783); - - input_unregister_device(priv->pwr); - kfree(priv); + mc13xxx_lock(priv->mc13xxx); + for (i = 0; i < MAX13XXX_NUM_BUTTONS; i++) + if (priv->enabled & BIT(i)) + mc13xxx_irq_free(priv->mc13xxx, + priv->devtype->btn_def[i].irq, priv); + mc13xxx_unlock(priv->mc13xxx); return 0; } -static struct platform_driver mc13783_pwrbutton_driver = { - .probe = mc13783_pwrbutton_probe, - .remove = mc13783_pwrbutton_remove, - .driver = { - .name = "mc13783-pwrbutton", +static const struct mc13xxx_pwrb_devtype mc13783_pwrb_devtype = { + .btn_def[0] = { MC13783_IRQ_ONOFD1, BIT(3) }, + .btn_def[1] = { MC13783_IRQ_ONOFD2, BIT(4) }, + .btn_def[2] = { MC13783_IRQ_ONOFD3, BIT(5) } +}; + +static const struct platform_device_id mc13xxx_pwrbutton_id_table[] = { + { "mc13783-pwrbutton", (kernel_ulong_t)&mc13783_pwrb_devtype }, + { } +}; +MODULE_DEVICE_TABLE(platform, mc13xxx_pwrbutton_id_table); + +static struct platform_driver mc13xxx_pwrbutton_driver = { + .driver = { + .name = "mc13xxx-pwrbutton", .owner = THIS_MODULE, }, + .remove = mc13xxx_pwrbutton_remove, + .id_table = mc13xxx_pwrbutton_id_table, }; +module_platform_driver_probe(mc13xxx_pwrbutton_driver, mc13xxx_pwrbutton_probe); -module_platform_driver(mc13783_pwrbutton_driver); - -MODULE_ALIAS("platform:mc13783-pwrbutton"); MODULE_DESCRIPTION("MC13783 Power Button"); MODULE_LICENSE("GPL v2"); MODULE_AUTHOR("Philippe Retornaz"); diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h index 41ed592..b895538 100644 --- a/include/linux/mfd/mc13xxx.h +++ b/include/linux/mfd/mc13xxx.h @@ -142,20 +142,22 @@ struct mc13xxx_leds_platform_data { u32 led_control[MAX_LED_CONTROL_REGS]; }; +#define MAX13XXX_NUM_BUTTONS 3 + +struct mc13xxx_button { + u16 keycode; + unsigned int flags; +#define MC13XXX_BUTTON_DBNC_0MS 0 +#define MC13XXX_BUTTON_DBNC_30MS 1 +#define MC13XXX_BUTTON_DBNC_150MS 2 +#define MC13XXX_BUTTON_DBNC_750MS 3 +#define MC13XXX_BUTTON_ENABLE (1 << 2) +#define MC13XXX_BUTTON_POL_INVERT (1 << 3) +#define MC13XXX_BUTTON_RESET_EN (1 << 4) +}; + struct mc13xxx_buttons_platform_data { -#define MC13783_BUTTON_DBNC_0MS 0 -#define MC13783_BUTTON_DBNC_30MS 1 -#define MC13783_BUTTON_DBNC_150MS 2 -#define MC13783_BUTTON_DBNC_750MS 3 -#define MC13783_BUTTON_ENABLE (1 << 2) -#define MC13783_BUTTON_POL_INVERT (1 << 3) -#define MC13783_BUTTON_RESET_EN (1 << 4) - int b1on_flags; - unsigned short b1on_key; - int b2on_flags; - unsigned short b2on_key; - int b3on_flags; - unsigned short b3on_key; + struct mc13xxx_button buttons[MAX13XXX_NUM_BUTTONS]; }; struct mc13xxx_ts_platform_data {
This patch is a preparation mc13xxx powerbutton driver to support MC13892 and support the probe through the DT. Signed-off-by: Alexander Shiyan <shc_work@mail.ru> --- arch/arm/mach-imx/mach-mx31moboard.c | 9 +- drivers/input/misc/mc13783-pwrbutton.c | 332 +++++++++++++-------------------- include/linux/mfd/mc13xxx.h | 28 +-- 3 files changed, 151 insertions(+), 218 deletions(-)