On Tue, 07 Jul 2015, Vaibhav Hiremath wrote:
> On Tuesday 07 July 2015 04:10 PM, Lee Jones wrote:
> >On Tue, 07 Jul 2015, Vaibhav Hiremath wrote:
> >>On Tuesday 07 July 2015 12:59 PM, Lee Jones wrote:
> >>>On Mon, 29 Jun 2015, Vaibhav Hiremath wrote:
> >>>
> >>>>As per the spec, bit 1 (INT_CLEAR_MODE) of reg addr 0xe
> >>>>(page 0) controls the method of clearing interrupt
> >>>>status of 88pm800 family of devices;
> >>>>
> >>>> 0: clear on read
> >>>> 1: clear on write
> >>>>
> >>>>If pdata is not coming from board file, then set the
> >>>>default irq clear method to "irq clear on write"
> >>>>
> >>>>Also, as suggested by "Lee Jones" renaming variable field
> >>>>to appropriate name.
> >>>>
> >>>>Signed-off-by: Zhao Ye <zhaoy@xxxxxxxxxxx>
> >>>>Signed-off-by: Vaibhav Hiremath <vaibhav.hiremath@xxxxxxxxxx>
> >>>>---
> >>>> drivers/mfd/88pm800.c | 15 ++++++++++-----
> >>>> include/linux/mfd/88pm80x.h | 10 ++++++++--
> >>>> 2 files changed, 18 insertions(+), 7 deletions(-)
> >>>>
> >>>>diff --git a/drivers/mfd/88pm800.c b/drivers/mfd/88pm800.c
> >>>>index d495737..66347be 100644
> >>>>--- a/drivers/mfd/88pm800.c
> >>>>+++ b/drivers/mfd/88pm800.c
> >>>>@@ -374,7 +374,7 @@ static int device_irq_init_800(struct pm80x_chip *chip)
> >>>> {
> >>>> struct regmap *map = chip->regmap;
> >>>> unsigned long flags = IRQF_ONESHOT;
> >>>>- int data, mask, ret = -EINVAL;
> >>>>+ int irq_clr_mode, mask, ret = -EINVAL;
> >>>>
> >>>> if (!map || !chip->irq) {
> >>>> dev_err(chip->dev, "incorrect parameters\n");
> >>>>@@ -382,15 +382,16 @@ static int device_irq_init_800(struct pm80x_chip *chip)
> >>>> }
> >>>>
> >>>> /*
> >>>>- * irq_mode defines the way of clearing interrupt. it's read-clear by
> >>>>- * default.
> >>>>+ * irq_clr_on_wr defines the way of clearing interrupt by
> >>>>+ * read/write(0/1). It's read-clear by default.
> >>>> */
> >>>> mask =
> >>>> PM800_WAKEUP2_INV_INT | PM800_WAKEUP2_INT_CLEAR |
> >>>> PM800_WAKEUP2_INT_MASK;
> >>>>
> >>>>- data = PM800_WAKEUP2_INT_CLEAR;
> >>>>- ret = regmap_update_bits(map, PM800_WAKEUP2, mask, data);
> >>>>+ irq_clr_mode = chip->irq_clr_method == PM800_IRQ_CLR_ON_WRITE ?
> >>>>+ PM800_WAKEUP2_INT_WRITE_CLEAR : PM800_WAKEUP2_INT_READ_CLEAR;
> >>>>+ ret = regmap_update_bits(map, PM800_WAKEUP2, mask, irq_clr_mode);
> >>>
> >>>What's stopping you just passing PM800_WAKEUP2_INT_WRITE_CLEAR or
> >>>PM800_WAKEUP2_INT_READ_CLEAR from pdata? Then you can use the value
> >>>directly without all of this faffing about.
> >>>
> >>> regmap_update_bits(map, PM800_WAKEUP2, mask, pdata->irq_clr_mode);
> >>>
> >>
> >>Because "irq_clr_method" is of boolean type.
> >>And macros which you are referring to is,
> >>
> >>#define PM800_WAKEUP2_INT_READ_CLEAR (0 << 1)
> >>#define PM800_WAKEUP2_INT_WRITE_CLEAR (1 << 1)
> >>
> >>
> >>And also, I feel it is more cleaner approach with the current code as
> >>register definition and userflag are maintained separately.
> >
> >I see your point, although it's a shame we have to have this code in
> >its place.
> >
> >One thing I think you can do though is rid chip->irq_clr_method, just
> >use the one you already have in pdata.
> >
>
> Looking at the current code,
> Yes, this can be done, but I have to do some more changes around it,
> to make code cleaner,
>
> change the signature of
>
> static int device_irq_init_800(struct pm80x_chip *chip)
>
> TO
>
> static int device_irq_init_800(struct pm80x_chip *chip, struct
> pm80x_platform_data *pdata)
>
>
> and then only use pdata->irq_clr_method.
>
>
> How do you want to get this inside? V6 version? or separate patch?
>
> I have one more cleanup patch in the queue, which I am planning to
> submit today, if you are ok then I can submit along with that.
Ideally not. Don't you save the 'struct device' into *chip? You
should use that to fetch the pdata, like:
pdata = dev_get_platdata(chip->dev);
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
