On Wed, Sep 07, 2016 at 01:42:41PM +0200, Marcin Niestroj wrote: > On 03.09.2016 19:53, Dmitry Torokhov wrote: > >On Fri, Sep 02, 2016 at 12:35:26PM +0200, Marcin Niestroj wrote: > >>This driver enables us to use tps65217's power button as KEY_POWER on > >>am335x boards (directly connected button in chiliboard, accessible pin > >>via expansion header in beaglebone). This patch has been tested with > >>chiliboard. > >> > >>Signed-off-by: Marcin Niestroj <m.niestroj@xxxxxxxxxxxxxxxx> > >>Acked-by: Rob Herring <robh@xxxxxxxxxx> > > > >Was it produces by soing s/65218/65217 over > >drivers/input/misc/tps65217-pwrbutton.c? I honestly can't see much > >difference between the 2. Can they be probably be combined? > > I didn't think of that earlier. Below is a patch that adds support for > tps65217 power button in tps65218-pwrbutton.c. Is this approach ok for > you? If yes, I will send new patch version with it. > > Additionally I guess the file should be renamed to tps6521x-pwrbutton.c > and config option changed to CONFIG_INPUT_TPS6521X_PWRBUTTON? We could but I do not think it is that important. > > diff --git a/drivers/input/misc/tps65218-pwrbutton.c > b/drivers/input/misc/tps65218-pwrbutton.c > index a39b626..19d2bb1 100644 > --- a/drivers/input/misc/tps65218-pwrbutton.c > +++ b/drivers/input/misc/tps65218-pwrbutton.c > @@ -1,8 +1,9 @@ > /* > - * Texas Instruments' TPS65218 Power Button Input Driver > + * Texas Instruments' TPS65217 and TPS65218 Power Button Input Driver > * > * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com/ > * Author: Felipe Balbi <balbi@xxxxxx> > + * Author: Marcin Niestroj <m.niestroj@xxxxxxxxxxxxxxxx> > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2 as > @@ -18,31 +19,73 @@ > #include <linux/input.h> > #include <linux/interrupt.h> > #include <linux/kernel.h> > +#include <linux/mfd/tps65217.h> > #include <linux/mfd/tps65218.h> > #include <linux/module.h> > #include <linux/of.h> > #include <linux/platform_device.h> > #include <linux/slab.h> > > -struct tps65218_pwrbutton { > +struct tps6521x_data { > + unsigned int reg_status; > + unsigned int pb_mask; > + const char *name; > +}; > + > +static const struct tps6521x_data tps65217_data = { > + .reg_status = TPS65217_REG_STATUS, > + .pb_mask = TPS65217_STATUS_PB, > + .name = "tps65217_pwrbutton", > +}; > + > +static const struct tps6521x_data tps65218_data = { > + .reg_status = TPS65218_REG_STATUS, > + .pb_mask = TPS65218_STATUS_PB_STATE, > + .name = "tps65218_pwrbutton", > +}; > + > +struct tps6521x_pwrbutton { > struct device *dev; > - struct tps65218 *tps; > + void *tps; > struct input_dev *idev; > + const struct tps6521x_data *data; > + char phys[32]; > }; > > -static irqreturn_t tps65218_pwr_irq(int irq, void *_pwr) > +static const struct of_device_id of_tps6521x_pwr_match[] = { > + { .compatible = "ti,tps65217-pwrbutton", .data = &tps65217_data }, > + { .compatible = "ti,tps65218-pwrbutton", .data = &tps65218_data }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, of_tps6521x_pwr_match); > + > +static int tps6521x_reg_read(struct tps6521x_pwrbutton *pwr, > unsigned int reg, > + unsigned int *val) > { > - struct tps65218_pwrbutton *pwr = _pwr; > + if (pwr->data == &tps65217_data) > + return tps65217_reg_read((struct tps65217 *) pwr->tps, > + reg, val); > + else if (pwr->data == &tps65218_data) > + return tps65218_reg_read((struct tps65218 *) pwr->tps, > + reg, val); This is step in right direction, but not quite there yet. You could add the reg_read pointer to your tps6521x_data and avoid doing if/else/case, or, maybe better yet, have registers accessible via regmap. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
