| Message ID | 20170324084240.23251-1-sre@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 24 Mar 2017, Sebastian Reichel wrote: > CPCAP can sense if IRQ is currently set or not. This > functionality is required for a few subdevices, such > as the power button and usb phy modules. > > Acked-by: Tony Lindgren <tony@atomide.com> > Signed-off-by: Sebastian Reichel <sre@kernel.org> > > --- > Hi Lee, > > I hope this is fine with you. I tried to come up > with a sensible macro for reg, but everything I > came up with actually reduced readability. > > -- Sebastian > > Changes since PATCHv3: > - add explicit extern to function definition > - use BIT macro for mask variable > - avoid magic numbers > --- > drivers/mfd/motorola-cpcap.c | 28 ++++++++++++++++++++++++++++ > include/linux/mfd/motorola-cpcap.h | 2 ++ > 2 files changed, 30 insertions(+) > > diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c > index 6aeada7d7ce5..c80997a377a1 100644 > --- a/drivers/mfd/motorola-cpcap.c > +++ b/drivers/mfd/motorola-cpcap.c > @@ -23,6 +23,8 @@ > > #define CPCAP_NR_IRQ_REG_BANKS 6 > #define CPCAP_NR_IRQ_CHIPS 3 > +#define CPCAP_REGISTER_SIZE 4 > +#define CPCAP_REGISTER_BITS 16 > > struct cpcap_ddata { > struct spi_device *spi; > @@ -32,6 +34,32 @@ struct cpcap_ddata { > struct regmap *regmap; > }; > > +static int cpcap_sense_irq(struct regmap *regmap, int irq) > +{ > + int regnum = irq / CPCAP_REGISTER_BITS; > + int mask = BIT(irq % CPCAP_REGISTER_BITS); > + int reg = CPCAP_REG_INTS1 + (regnum * CPCAP_REGISTER_SIZE); > + int err, val; > + > + if (reg < CPCAP_REG_INTS1 || reg > CPCAP_REG_INTS4) > + return -EINVAL; > + > + err = regmap_read(regmap, reg, &val); > + if (err) > + return err; > + > + return !!(val & mask); > +} > + > +int cpcap_sense_virq(struct regmap *regmap, int virq) > +{ > + struct regmap_irq_chip_data *d = irq_get_chip_data(virq); > + int base = regmap_irq_chip_get_base(d); What base is this? Could it be used to avoid some calculations in cpcap_sense_irq()? > + return cpcap_sense_irq(regmap, virq - base); > +} > +EXPORT_SYMBOL_GPL(cpcap_sense_virq); > + > static int cpcap_check_revision(struct cpcap_ddata *cpcap) > { > u16 vendor, rev; > diff --git a/include/linux/mfd/motorola-cpcap.h b/include/linux/mfd/motorola-cpcap.h > index b4031c2b2214..793aa695faa0 100644 > --- a/include/linux/mfd/motorola-cpcap.h > +++ b/include/linux/mfd/motorola-cpcap.h > @@ -290,3 +290,5 @@ static inline int cpcap_get_vendor(struct device *dev, > > return 0; > } > + > +extern int cpcap_sense_virq(struct regmap *regmap, int virq);
Hi Lee, On Tue, Mar 28, 2017 at 11:27:16AM +0100, Lee Jones wrote: > > +int cpcap_sense_virq(struct regmap *regmap, int virq) > > +{ > > + struct regmap_irq_chip_data *d = irq_get_chip_data(virq); > > + int base = regmap_irq_chip_get_base(d); > > What base is this? This function takes an Linux irq number. That number is a virtual irq number, which has nothing to do with the hardware. For example a platform could look like this: linux irq device base device irq --------------------------------------------- 0-63 SoC 0 0-63 64-127 SoC-GPIO 64 0-63 128-191 CPCAP 128 0-63 For the above example the function would map 128-191 to 0-63 as used by the device. > Could it be used to avoid some calculations in cpcap_sense_irq()? No. > > + return cpcap_sense_irq(regmap, virq - base); > > +} > > +EXPORT_SYMBOL_GPL(cpcap_sense_virq); -- Sebastian
On Tue, 28 Mar 2017, Sebastian Reichel wrote: > Hi Lee, > > On Tue, Mar 28, 2017 at 11:27:16AM +0100, Lee Jones wrote: > > > +int cpcap_sense_virq(struct regmap *regmap, int virq) > > > +{ > > > + struct regmap_irq_chip_data *d = irq_get_chip_data(virq); > > > + int base = regmap_irq_chip_get_base(d); > > > > What base is this? > > This function takes an Linux irq number. That number is a virtual > irq number, which has nothing to do with the hardware. For example > a platform could look like this: > > linux irq device base device irq > --------------------------------------------- > 0-63 SoC 0 0-63 > 64-127 SoC-GPIO 64 0-63 > 128-191 CPCAP 128 0-63 > > For the above example the function would map 128-191 to 0-63 as > used by the device. In which case may I suggest that you rename the variable, since 'base' is commonly used in the kernel to mean base of a particular IP's memory block. > > Could it be used to avoid some calculations in cpcap_sense_irq()? > > No. > > > > + return cpcap_sense_irq(regmap, virq - base); > > > +} > > > +EXPORT_SYMBOL_GPL(cpcap_sense_virq); > > -- Sebastian
diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c index 6aeada7d7ce5..c80997a377a1 100644 --- a/drivers/mfd/motorola-cpcap.c +++ b/drivers/mfd/motorola-cpcap.c @@ -23,6 +23,8 @@ #define CPCAP_NR_IRQ_REG_BANKS 6 #define CPCAP_NR_IRQ_CHIPS 3 +#define CPCAP_REGISTER_SIZE 4 +#define CPCAP_REGISTER_BITS 16 struct cpcap_ddata { struct spi_device *spi; @@ -32,6 +34,32 @@ struct cpcap_ddata { struct regmap *regmap; }; +static int cpcap_sense_irq(struct regmap *regmap, int irq) +{ + int regnum = irq / CPCAP_REGISTER_BITS; + int mask = BIT(irq % CPCAP_REGISTER_BITS); + int reg = CPCAP_REG_INTS1 + (regnum * CPCAP_REGISTER_SIZE); + int err, val; + + if (reg < CPCAP_REG_INTS1 || reg > CPCAP_REG_INTS4) + return -EINVAL; + + err = regmap_read(regmap, reg, &val); + if (err) + return err; + + return !!(val & mask); +} + +int cpcap_sense_virq(struct regmap *regmap, int virq) +{ + struct regmap_irq_chip_data *d = irq_get_chip_data(virq); + int base = regmap_irq_chip_get_base(d); + + return cpcap_sense_irq(regmap, virq - base); +} +EXPORT_SYMBOL_GPL(cpcap_sense_virq); + static int cpcap_check_revision(struct cpcap_ddata *cpcap) { u16 vendor, rev; diff --git a/include/linux/mfd/motorola-cpcap.h b/include/linux/mfd/motorola-cpcap.h index b4031c2b2214..793aa695faa0 100644 --- a/include/linux/mfd/motorola-cpcap.h +++ b/include/linux/mfd/motorola-cpcap.h @@ -290,3 +290,5 @@ static inline int cpcap_get_vendor(struct device *dev, return 0; } + +extern int cpcap_sense_virq(struct regmap *regmap, int virq);