| Message ID | 639fd177d5ddd4ce80df45f9d774cc971b286ef9.1430393194.git.stwiss.opensource@diasemi.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Steve, On Thu, Apr 30, 2015 at 1:26 PM, S Twiss <stwiss.opensource@diasemi.com> wrote: > Add MFD support for the DA9063 OnKey driver Thanks for your patch! > --- a/drivers/mfd/da9063-core.c > +++ b/drivers/mfd/da9063-core.c > @@ -60,6 +60,7 @@ static struct resource da9063_rtc_resources[] = { > > static struct resource da9063_onkey_resources[] = { > { > + .name = "ONKEY", > .start = DA9063_IRQ_ONKEY, > .end = DA9063_IRQ_ONKEY, > .flags = IORESOURCE_IRQ, > @@ -97,6 +98,7 @@ static const struct mfd_cell da9063_devs[] = { > .name = DA9063_DRVNAME_ONKEY, > .num_resources = ARRAY_SIZE(da9063_onkey_resources), > .resources = da9063_onkey_resources, > + .of_compatible = "dlg,da9063-onkey", > }, > { > .name = DA9063_DRVNAME_RTC, > @@ -109,12 +111,64 @@ static const struct mfd_cell da9063_devs[] = { > }, > }; > > +static int da9063_clear_fault_log(struct da9063 *da9063) > +{ > + int ret = 0; > + int fault_log = 0; > + > + ret = regmap_read(da9063->regmap, DA9063_REG_FAULT_LOG, &fault_log); > + if (ret < 0) { > + dev_err(da9063->dev, "Cannot read FAULT_LOG.\n"); > + return -EIO; > + } > + > + if (fault_log) { > + if (fault_log & DA9063_TWD_ERROR) > + dev_dbg(da9063->dev, > + "Fault log entry detected: DA9063_TWD_ERROR\n"); > + if (fault_log & DA9063_POR) > + dev_dbg(da9063->dev, > + "Fault log entry detected: DA9063_POR\n"); > + if (fault_log & DA9063_VDD_FAULT) > + dev_dbg(da9063->dev, > + "Fault log entry detected: DA9063_VDD_FAULT\n"); > + if (fault_log & DA9063_VDD_START) > + dev_dbg(da9063->dev, > + "Fault log entry detected: DA9063_VDD_START\n"); > + if (fault_log & DA9063_TEMP_CRIT) > + dev_dbg(da9063->dev, > + "Fault log entry detected: DA9063_TEMP_CRIT\n"); > + if (fault_log & DA9063_KEY_RESET) > + dev_dbg(da9063->dev, > + "Fault log entry detected: DA9063_KEY_RESET\n"); > + if (fault_log & DA9063_NSHUTDOWN) > + dev_dbg(da9063->dev, > + "Fault log entry detected: DA9063_NSHUTDOWN\n"); > + if (fault_log & DA9063_WAIT_SHUT) > + dev_dbg(da9063->dev, > + "Fault log entry detected: DA9063_WAIT_SHUT\n"); > + } > + > + ret = regmap_write(da9063->regmap, > + DA9063_REG_FAULT_LOG, > + fault_log); > + if (ret < 0) > + dev_err(da9063->dev, > + "Cannot reset FAULT_LOG values %d\n", ret); > + > + return ret; > +} > + > int da9063_device_init(struct da9063 *da9063, unsigned int irq) > { > struct da9063_pdata *pdata = da9063->dev->platform_data; > int model, variant_id, variant_code; > int ret; > > + ret = da9063_clear_fault_log(da9063); > + if (ret < 0) > + dev_err(da9063->dev, "Cannot clear fault log\n"); > + > if (pdata) { > da9063->flags = pdata->flags; > da9063->irq_base = pdata->irq_base; The code above doesn't seem to match the patch description. Can you please explain why it is needed? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04 May 2015 08:47, Geert Uytterhoeven wrote: > On Thu, Apr 30, 2015 at 1:26 PM, S Twiss <stwiss.opensource@diasemi.com> > wrote: > > Add MFD support for the DA9063 OnKey driver > > Thanks for your patch! > > > +static int da9063_clear_fault_log(struct da9063 *da9063) > > +{ > > + int ret = 0; > > + int fault_log = 0; > > + > > + ret = regmap_read(da9063->regmap, DA9063_REG_FAULT_LOG, > &fault_log); > > + if (ret < 0) { > > + dev_err(da9063->dev, "Cannot read FAULT_LOG.\n"); > > + return -EIO; > > + } > > + > > + if (fault_log) { > > + if (fault_log & DA9063_TWD_ERROR) > > + dev_dbg(da9063->dev, > > + "Fault log entry detected: DA9063_TWD_ERROR\n"); > > + if (fault_log & DA9063_POR) > > + dev_dbg(da9063->dev, > > + "Fault log entry detected: DA9063_POR\n"); > > + if (fault_log & DA9063_VDD_FAULT) > > + dev_dbg(da9063->dev, > > + "Fault log entry detected: DA9063_VDD_FAULT\n"); > > + if (fault_log & DA9063_VDD_START) > > + dev_dbg(da9063->dev, > > + "Fault log entry detected: DA9063_VDD_START\n"); > > + if (fault_log & DA9063_TEMP_CRIT) > > + dev_dbg(da9063->dev, > > + "Fault log entry detected: DA9063_TEMP_CRIT\n"); > > + if (fault_log & DA9063_KEY_RESET) > > + dev_dbg(da9063->dev, > > + "Fault log entry detected: DA9063_KEY_RESET\n"); > > + if (fault_log & DA9063_NSHUTDOWN) > > + dev_dbg(da9063->dev, > > + "Fault log entry detected: DA9063_NSHUTDOWN\n"); > > + if (fault_log & DA9063_WAIT_SHUT) > > + dev_dbg(da9063->dev, > > + "Fault log entry detected: DA9063_WAIT_SHUT\n"); > > + } > > + > > + ret = regmap_write(da9063->regmap, > > + DA9063_REG_FAULT_LOG, > > + fault_log); > > + if (ret < 0) > > + dev_err(da9063->dev, > > + "Cannot reset FAULT_LOG values %d\n", ret); > > + > > + return ret; > > +} > > + > > int da9063_device_init(struct da9063 *da9063, unsigned int irq) > > { > > struct da9063_pdata *pdata = da9063->dev->platform_data; > > int model, variant_id, variant_code; > > int ret; > > > > + ret = da9063_clear_fault_log(da9063); > > + if (ret < 0) > > + dev_err(da9063->dev, "Cannot clear fault log\n"); > > + > > if (pdata) { > > da9063->flags = pdata->flags; > > da9063->irq_base = pdata->irq_base; > > The code above doesn't seem to match the patch description. > Can you please explain why it is needed? > Hi Geert, Yes, at first it does seem that the fault-log clearing function is unrelated to the "MFD support for the DA9063 OnKey driver", but there is an important connection that makes it essential for the OnKey driver in this case. I have made some explanation of the OnKey's operation in this thread here: https://lkml.org/lkml/2015/4/29/406 But I only described the OnKey's point-of-view for case (4) of the "OnKey operation" Case (4): the long-long press and no key release -- the hardware power-cut when software is unable to respond (for whatever reason). In this case, if the software was not responsive and a hardware shutdown of the device was chosen, the FAULT_LOG would persist with information that would be accessible when the device was restarted: it would be possible to take action the next time the device was turned on again (maybe to trigger some mitigation exercise in the bootloader -- e.g. say to complete memory checks or put the device into a safe mode). However this mitigation exercise and clearance of the fault-log cannot be counted on outside the Linux kernel and so the clearance function da9063_clear_fault_log () has been done here. If this clearance function did not exist, then after a hardware shutdown (due to S/W failure), the next time the device is restarted the FAULT_LOG would persist with values from the previous error. Regards, Steve
Hi Steve, On Tue, May 5, 2015 at 10:36 AM, Opensource [Steve Twiss] <stwiss.opensource@diasemi.com> wrote: > On 04 May 2015 08:47, Geert Uytterhoeven wrote: >> On Thu, Apr 30, 2015 at 1:26 PM, S Twiss <stwiss.opensource@diasemi.com> >> wrote: >> > Add MFD support for the DA9063 OnKey driver >> > +static int da9063_clear_fault_log(struct da9063 *da9063) >> > +{ >> > +} >> > + >> > int da9063_device_init(struct da9063 *da9063, unsigned int irq) >> > { >> > struct da9063_pdata *pdata = da9063->dev->platform_data; >> > int model, variant_id, variant_code; >> > int ret; >> > >> > + ret = da9063_clear_fault_log(da9063); >> > + if (ret < 0) >> > + dev_err(da9063->dev, "Cannot clear fault log\n"); >> > + >> > if (pdata) { >> > da9063->flags = pdata->flags; >> > da9063->irq_base = pdata->irq_base; >> >> The code above doesn't seem to match the patch description. >> Can you please explain why it is needed? > > Yes, at first it does seem that the fault-log clearing function is unrelated to the > "MFD support for the DA9063 OnKey driver", but there is an important connection > that makes it essential for the OnKey driver in this case. > > I have made some explanation of the OnKey's operation in this thread here: > https://lkml.org/lkml/2015/4/29/406 > > But I only described the OnKey's point-of-view for case (4) of the "OnKey operation" > Case (4): the long-long press and no key release -- the hardware power-cut when > software is unable to respond (for whatever reason). > > In this case, if the software was not responsive and a hardware shutdown of the > device was chosen, the FAULT_LOG would persist with information that would be > accessible when the device was restarted: it would be possible to take action the > next time the device was turned on again (maybe to trigger some mitigation exercise > in the bootloader -- e.g. say to complete memory checks or put the device into a > safe mode). > > However this mitigation exercise and clearance of the fault-log cannot be counted on > outside the Linux kernel and so the clearance function da9063_clear_fault_log () has > been done here. > > If this clearance function did not exist, then after a hardware shutdown (due to S/W > failure), the next time the device is restarted the FAULT_LOG would persist with values > from the previous error. Thanks for your explanation! Please add (some parts of it) to the patch description. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05 May 2015 09:52 Geert Uytterhoeven wrote: > On Tue, May 5, 2015 at 10:36 AM, Opensource [Steve Twiss] > <stwiss.opensource@diasemi.com> wrote: > > On 04 May 2015 08:47, Geert Uytterhoeven wrote: > >> On Thu, Apr 30, 2015 at 1:26 PM, S Twiss <stwiss.opensource@diasemi.com> wrote: > >> > Add MFD support for the DA9063 OnKey driver > >> > +static int da9063_clear_fault_log(struct da9063 *da9063) > >> > +{ > > >> > +} > >> > + > >> > int da9063_device_init(struct da9063 *da9063, unsigned int irq) > >> > { > >> > struct da9063_pdata *pdata = da9063->dev->platform_data; > >> > int model, variant_id, variant_code; > >> > int ret; > >> > > >> > + ret = da9063_clear_fault_log(da9063); > >> > + if (ret < 0) > >> > + dev_err(da9063->dev, "Cannot clear fault log\n"); > >> > + > >> > if (pdata) { > >> > da9063->flags = pdata->flags; > >> > da9063->irq_base = pdata->irq_base; > >> > >> The code above doesn't seem to match the patch description. > >> Can you please explain why it is needed? > > > > Yes, at first it does seem that the fault-log clearing function is unrelated to the > > "MFD support for the DA9063 OnKey driver", but there is an important connection > > that makes it essential for the OnKey driver in this case. > > > > I have made some explanation of the OnKey's operation in this thread here: > > https://lkml.org/lkml/2015/4/29/406 > > > > But I only described the OnKey's point-of-view for case (4) of the "OnKey operation" > > Case (4): the long-long press and no key release -- the hardware power-cut when > > software is unable to respond (for whatever reason). > > > > In this case, if the software was not responsive and a hardware shutdown of the > > device was chosen, the FAULT_LOG would persist with information that would be > > accessible when the device was restarted: it would be possible to take action the > > next time the device was turned on again (maybe to trigger some mitigation exercise > > in the bootloader -- e.g. say to complete memory checks or put the device into a > > safe mode). > > > > However this mitigation exercise and clearance of the fault-log cannot be counted on > > outside the Linux kernel and so the clearance function da9063_clear_fault_log () has > > been done here. > > > > If this clearance function did not exist, then after a hardware shutdown (due to S/W > > failure), the next time the device is restarted the FAULT_LOG would persist with values > > from the previous error. > > Thanks for your explanation! > > Please add (some parts of it) to the patch description. > Thanks Geert, I will do that. I'll wait a while to see if there are any other responses (e.g. OnKey) and then make a new patch set called PATCH V4. Regards, Steve CC: Dmitry Torokhov for information on these MFD changes since they also relate to the OnKey.
diff --git a/drivers/mfd/da9063-core.c b/drivers/mfd/da9063-core.c index facd361..af841c1 100644 --- a/drivers/mfd/da9063-core.c +++ b/drivers/mfd/da9063-core.c @@ -60,6 +60,7 @@ static struct resource da9063_rtc_resources[] = { static struct resource da9063_onkey_resources[] = { { + .name = "ONKEY", .start = DA9063_IRQ_ONKEY, .end = DA9063_IRQ_ONKEY, .flags = IORESOURCE_IRQ, @@ -97,6 +98,7 @@ static const struct mfd_cell da9063_devs[] = { .name = DA9063_DRVNAME_ONKEY, .num_resources = ARRAY_SIZE(da9063_onkey_resources), .resources = da9063_onkey_resources, + .of_compatible = "dlg,da9063-onkey", }, { .name = DA9063_DRVNAME_RTC, @@ -109,12 +111,64 @@ static const struct mfd_cell da9063_devs[] = { }, }; +static int da9063_clear_fault_log(struct da9063 *da9063) +{ + int ret = 0; + int fault_log = 0; + + ret = regmap_read(da9063->regmap, DA9063_REG_FAULT_LOG, &fault_log); + if (ret < 0) { + dev_err(da9063->dev, "Cannot read FAULT_LOG.\n"); + return -EIO; + } + + if (fault_log) { + if (fault_log & DA9063_TWD_ERROR) + dev_dbg(da9063->dev, + "Fault log entry detected: DA9063_TWD_ERROR\n"); + if (fault_log & DA9063_POR) + dev_dbg(da9063->dev, + "Fault log entry detected: DA9063_POR\n"); + if (fault_log & DA9063_VDD_FAULT) + dev_dbg(da9063->dev, + "Fault log entry detected: DA9063_VDD_FAULT\n"); + if (fault_log & DA9063_VDD_START) + dev_dbg(da9063->dev, + "Fault log entry detected: DA9063_VDD_START\n"); + if (fault_log & DA9063_TEMP_CRIT) + dev_dbg(da9063->dev, + "Fault log entry detected: DA9063_TEMP_CRIT\n"); + if (fault_log & DA9063_KEY_RESET) + dev_dbg(da9063->dev, + "Fault log entry detected: DA9063_KEY_RESET\n"); + if (fault_log & DA9063_NSHUTDOWN) + dev_dbg(da9063->dev, + "Fault log entry detected: DA9063_NSHUTDOWN\n"); + if (fault_log & DA9063_WAIT_SHUT) + dev_dbg(da9063->dev, + "Fault log entry detected: DA9063_WAIT_SHUT\n"); + } + + ret = regmap_write(da9063->regmap, + DA9063_REG_FAULT_LOG, + fault_log); + if (ret < 0) + dev_err(da9063->dev, + "Cannot reset FAULT_LOG values %d\n", ret); + + return ret; +} + int da9063_device_init(struct da9063 *da9063, unsigned int irq) { struct da9063_pdata *pdata = da9063->dev->platform_data; int model, variant_id, variant_code; int ret; + ret = da9063_clear_fault_log(da9063); + if (ret < 0) + dev_err(da9063->dev, "Cannot clear fault log\n"); + if (pdata) { da9063->flags = pdata->flags; da9063->irq_base = pdata->irq_base; diff --git a/include/linux/mfd/da9063/pdata.h b/include/linux/mfd/da9063/pdata.h index 95c8742..612383b 100644 --- a/include/linux/mfd/da9063/pdata.h +++ b/include/linux/mfd/da9063/pdata.h @@ -103,6 +103,7 @@ struct da9063; struct da9063_pdata { int (*init)(struct da9063 *da9063); int irq_base; + bool key_power; unsigned flags; struct da9063_regulators_pdata *regulators_pdata; struct led_platform_data *leds_pdata;