Re: [RFC PATCH 1/2] watchdog: imx2: fix hang-up on boot for i.MX21, i.MX27 and i.MX31 SoCs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/10/2016 11:28 AM, Magnus Lilja wrote:
Hi,

On 26 September 2016 at 15:02, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On 09/25/2016 05:39 PM, Vladimir Zapolskiy wrote:

Power down counter enable/disable bit switch is located in WMCR
register, but watchdog controllers found on legacy i.MX21, i.MX27 and
i.MX31 SoCs don't have this register. As a result of writing data to
the non-existing register on driver probe the SoC hangs up, to fix the
problem add more OF compatible strings and on this basis get
information about availability of the WMCR register.

Fixes: 5fe65ce7ccbb ("watchdog: imx2_wdt: Disable power down counter on
boot")
Signed-off-by: Vladimir Zapolskiy <vz@xxxxxxxxx>
---
 drivers/watchdog/imx2_wdt.c | 47
+++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 62f346b..b6763e0 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -29,6 +29,7 @@
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/watchdog.h>
@@ -57,6 +58,10 @@

 #define WDOG_SEC_TO_COUNT(s)   ((s * 2 - 1) << 8)

+struct imx2_wdt_data {
+       bool has_pdc;
+};
+
 struct imx2_wdt_device {
        struct clk *clk;
        struct regmap *regmap;
@@ -64,6 +69,8 @@ struct imx2_wdt_device {
        bool ext_reset;
 };

+static const struct of_device_id imx2_wdt_dt_ids[];
+
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started
(default="
@@ -200,10 +207,13 @@ static const struct regmap_config
imx2_wdt_regmap_config = {

 static int __init imx2_wdt_probe(struct platform_device *pdev)
 {
+       const struct of_device_id *of_id;
+       const struct imx2_wdt_data *data;
        struct imx2_wdt_device *wdev;
        struct watchdog_device *wdog;
        struct resource *res;
        void __iomem *base;
+       bool has_pdc;
        int ret;
        u32 val;

@@ -261,12 +271,24 @@ static int __init imx2_wdt_probe(struct
platform_device *pdev)
                set_bit(WDOG_HW_RUNNING, &wdog->status);
        }

+       if (pdev->dev.of_node) {
+               of_id = of_match_device(imx2_wdt_dt_ids, &pdev->dev);
+               if (!of_id)
+                       return -ENODEV;
+
+               data = of_id->data;
+               has_pdc = data->has_pdc;
+       } else {
+               has_pdc = false;
+       }
+
        /*
         * Disable the watchdog power down counter at boot. Otherwise the
power
         * down counter will pull down the #WDOG interrupt line for one
clock
         * cycle.
         */
-       regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);
+       if (has_pdc)
+               regmap_write(wdev->regmap, IMX2_WDT_WMCR, 0);

        ret = watchdog_register_device(wdog);
        if (ret) {
@@ -363,8 +385,29 @@ static int imx2_wdt_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(imx2_wdt_pm_ops, imx2_wdt_suspend,
                         imx2_wdt_resume);

+static const struct imx2_wdt_data imx21_wdt_data = {
+       .has_pdc = false,
+};
+
+static const struct imx2_wdt_data imx25_wdt_data = {
+       .has_pdc = true,
+};
+
 static const struct of_device_id imx2_wdt_dt_ids[] = {
-       { .compatible = "fsl,imx21-wdt", },
+       { .compatible = "fsl,imx21-wdt",  .data = &imx21_wdt_data },


Please just specify the flag directly, as in
                                        .data = (void *) true
or, if you prefer, use defines.
                                        .data = (void *) IMX_SUPPORTS_PDC

Then, in above code:
                has_pdc = (bool) of_id->data;

Guenter

Has this patch (or an updated one) been merged in any tree? I've
tested it on a i.MX31 board with positive result.


I had asked for a change, and I don't recall seeing v2. So, at least as far
as I know, the answer is no.

Guenter

--
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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux