| Message ID | 20240730145610.2177627-1-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,1/1] watchdog: imx7ulp_wdt: move post_rcs_wait into struct imx_wdt_hw_feature | expand |
On Tue, Jul 30, 2024 at 10:56:10AM -0400, Frank Li wrote: > Move post_rcs_wait into struct imx_wdt_hw_feature to simple code logic for > difference compatible string. > > i.MX93 watchdog needn't wait 2.5 clocks after RCS is done. So needn't set > post_rcs_wait. > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Alice Guo <alice.guo@nxp.com> > Reviewed-by: Ye Li <ye.li@nxp.com> > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- Ping? Frank > Change from v4 to v5 > - move compatible string 8ulp under 7ulp > Chagne from v3 to v4: > - Go back to v2 according to Guenter's feedback > Change from v2 to v3: > - Set post_rcs_wait to false explicitly to maintain code consistency > - Add Guenter review tag. > Change from v1 to v2: > - Combine to one patch > --- > drivers/watchdog/imx7ulp_wdt.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c > index 94914a22daff7..0f13a30533574 100644 > --- a/drivers/watchdog/imx7ulp_wdt.c > +++ b/drivers/watchdog/imx7ulp_wdt.c > @@ -55,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" > > struct imx_wdt_hw_feature { > bool prescaler_enable; > + bool post_rcs_wait; > u32 wdog_clock_rate; > }; > > @@ -62,7 +63,6 @@ struct imx7ulp_wdt_device { > struct watchdog_device wdd; > void __iomem *base; > struct clk *clk; > - bool post_rcs_wait; > bool ext_reset; > const struct imx_wdt_hw_feature *hw; > }; > @@ -95,7 +95,7 @@ static int imx7ulp_wdt_wait_rcs(struct imx7ulp_wdt_device *wdt) > ret = -ETIMEDOUT; > > /* Wait 2.5 clocks after RCS done */ > - if (wdt->post_rcs_wait) > + if (wdt->hw->post_rcs_wait) > usleep_range(wait_min, wait_min + 2000); > > return ret; > @@ -334,15 +334,6 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev) > /* The WDOG may need to do external reset through dedicated pin */ > imx7ulp_wdt->ext_reset = of_property_read_bool(dev->of_node, "fsl,ext-reset-output"); > > - imx7ulp_wdt->post_rcs_wait = true; > - if (of_device_is_compatible(dev->of_node, > - "fsl,imx8ulp-wdt")) { > - dev_info(dev, "imx8ulp wdt probe\n"); > - imx7ulp_wdt->post_rcs_wait = false; > - } else { > - dev_info(dev, "imx7ulp wdt probe\n"); > - } > - > wdog = &imx7ulp_wdt->wdd; > wdog->info = &imx7ulp_wdt_info; > wdog->ops = &imx7ulp_wdt_ops; > @@ -403,6 +394,12 @@ static const struct dev_pm_ops imx7ulp_wdt_pm_ops = { > static const struct imx_wdt_hw_feature imx7ulp_wdt_hw = { > .prescaler_enable = false, > .wdog_clock_rate = 1000, > + .post_rcs_wait = true, > +}; > + > +static const struct imx_wdt_hw_feature imx8ulp_wdt_hw = { > + .prescaler_enable = false, > + .wdog_clock_rate = 1000, > }; > > static const struct imx_wdt_hw_feature imx93_wdt_hw = { > @@ -411,8 +408,8 @@ static const struct imx_wdt_hw_feature imx93_wdt_hw = { > }; > > static const struct of_device_id imx7ulp_wdt_dt_ids[] = { > - { .compatible = "fsl,imx8ulp-wdt", .data = &imx7ulp_wdt_hw, }, > { .compatible = "fsl,imx7ulp-wdt", .data = &imx7ulp_wdt_hw, }, > + { .compatible = "fsl,imx8ulp-wdt", .data = &imx8ulp_wdt_hw, }, > { .compatible = "fsl,imx93-wdt", .data = &imx93_wdt_hw, }, > { /* sentinel */ } > }; > -- > 2.34.1 >
On Thu, Aug 29, 2024 at 06:00:51PM -0400, Frank Li wrote: > On Tue, Jul 30, 2024 at 10:56:10AM -0400, Frank Li wrote: > > Move post_rcs_wait into struct imx_wdt_hw_feature to simple code logic for > > difference compatible string. > > > > i.MX93 watchdog needn't wait 2.5 clocks after RCS is done. So needn't set > > post_rcs_wait. > > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > Signed-off-by: Alice Guo <alice.guo@nxp.com> > > Reviewed-by: Ye Li <ye.li@nxp.com> > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- Wim Van Sebroeck: Could you please take care this patch? Frank > > Ping? > > Frank > > > Change from v4 to v5 > > - move compatible string 8ulp under 7ulp > > Chagne from v3 to v4: > > - Go back to v2 according to Guenter's feedback > > Change from v2 to v3: > > - Set post_rcs_wait to false explicitly to maintain code consistency > > - Add Guenter review tag. > > Change from v1 to v2: > > - Combine to one patch > > --- > > drivers/watchdog/imx7ulp_wdt.c | 21 +++++++++------------ > > 1 file changed, 9 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c > > index 94914a22daff7..0f13a30533574 100644 > > --- a/drivers/watchdog/imx7ulp_wdt.c > > +++ b/drivers/watchdog/imx7ulp_wdt.c > > @@ -55,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" > > > > struct imx_wdt_hw_feature { > > bool prescaler_enable; > > + bool post_rcs_wait; > > u32 wdog_clock_rate; > > }; > > > > @@ -62,7 +63,6 @@ struct imx7ulp_wdt_device { > > struct watchdog_device wdd; > > void __iomem *base; > > struct clk *clk; > > - bool post_rcs_wait; > > bool ext_reset; > > const struct imx_wdt_hw_feature *hw; > > }; > > @@ -95,7 +95,7 @@ static int imx7ulp_wdt_wait_rcs(struct imx7ulp_wdt_device *wdt) > > ret = -ETIMEDOUT; > > > > /* Wait 2.5 clocks after RCS done */ > > - if (wdt->post_rcs_wait) > > + if (wdt->hw->post_rcs_wait) > > usleep_range(wait_min, wait_min + 2000); > > > > return ret; > > @@ -334,15 +334,6 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev) > > /* The WDOG may need to do external reset through dedicated pin */ > > imx7ulp_wdt->ext_reset = of_property_read_bool(dev->of_node, "fsl,ext-reset-output"); > > > > - imx7ulp_wdt->post_rcs_wait = true; > > - if (of_device_is_compatible(dev->of_node, > > - "fsl,imx8ulp-wdt")) { > > - dev_info(dev, "imx8ulp wdt probe\n"); > > - imx7ulp_wdt->post_rcs_wait = false; > > - } else { > > - dev_info(dev, "imx7ulp wdt probe\n"); > > - } > > - > > wdog = &imx7ulp_wdt->wdd; > > wdog->info = &imx7ulp_wdt_info; > > wdog->ops = &imx7ulp_wdt_ops; > > @@ -403,6 +394,12 @@ static const struct dev_pm_ops imx7ulp_wdt_pm_ops = { > > static const struct imx_wdt_hw_feature imx7ulp_wdt_hw = { > > .prescaler_enable = false, > > .wdog_clock_rate = 1000, > > + .post_rcs_wait = true, > > +}; > > + > > +static const struct imx_wdt_hw_feature imx8ulp_wdt_hw = { > > + .prescaler_enable = false, > > + .wdog_clock_rate = 1000, > > }; > > > > static const struct imx_wdt_hw_feature imx93_wdt_hw = { > > @@ -411,8 +408,8 @@ static const struct imx_wdt_hw_feature imx93_wdt_hw = { > > }; > > > > static const struct of_device_id imx7ulp_wdt_dt_ids[] = { > > - { .compatible = "fsl,imx8ulp-wdt", .data = &imx7ulp_wdt_hw, }, > > { .compatible = "fsl,imx7ulp-wdt", .data = &imx7ulp_wdt_hw, }, > > + { .compatible = "fsl,imx8ulp-wdt", .data = &imx8ulp_wdt_hw, }, > > { .compatible = "fsl,imx93-wdt", .data = &imx93_wdt_hw, }, > > { /* sentinel */ } > > }; > > -- > > 2.34.1 > >
Hi Frank, > On Thu, Aug 29, 2024 at 06:00:51PM -0400, Frank Li wrote: > > On Tue, Jul 30, 2024 at 10:56:10AM -0400, Frank Li wrote: > > > Move post_rcs_wait into struct imx_wdt_hw_feature to simple code logic for > > > difference compatible string. > > > > > > i.MX93 watchdog needn't wait 2.5 clocks after RCS is done. So needn't set > > > post_rcs_wait. > > > > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> > > > Signed-off-by: Alice Guo <alice.guo@nxp.com> > > > Reviewed-by: Ye Li <ye.li@nxp.com> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > --- > > Wim Van Sebroeck: > > Could you please take care this patch? > It's in Linux-watchdog-next since tuesday. Kind regards, Wim.
diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c index 94914a22daff7..0f13a30533574 100644 --- a/drivers/watchdog/imx7ulp_wdt.c +++ b/drivers/watchdog/imx7ulp_wdt.c @@ -55,6 +55,7 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" struct imx_wdt_hw_feature { bool prescaler_enable; + bool post_rcs_wait; u32 wdog_clock_rate; }; @@ -62,7 +63,6 @@ struct imx7ulp_wdt_device { struct watchdog_device wdd; void __iomem *base; struct clk *clk; - bool post_rcs_wait; bool ext_reset; const struct imx_wdt_hw_feature *hw; }; @@ -95,7 +95,7 @@ static int imx7ulp_wdt_wait_rcs(struct imx7ulp_wdt_device *wdt) ret = -ETIMEDOUT; /* Wait 2.5 clocks after RCS done */ - if (wdt->post_rcs_wait) + if (wdt->hw->post_rcs_wait) usleep_range(wait_min, wait_min + 2000); return ret; @@ -334,15 +334,6 @@ static int imx7ulp_wdt_probe(struct platform_device *pdev) /* The WDOG may need to do external reset through dedicated pin */ imx7ulp_wdt->ext_reset = of_property_read_bool(dev->of_node, "fsl,ext-reset-output"); - imx7ulp_wdt->post_rcs_wait = true; - if (of_device_is_compatible(dev->of_node, - "fsl,imx8ulp-wdt")) { - dev_info(dev, "imx8ulp wdt probe\n"); - imx7ulp_wdt->post_rcs_wait = false; - } else { - dev_info(dev, "imx7ulp wdt probe\n"); - } - wdog = &imx7ulp_wdt->wdd; wdog->info = &imx7ulp_wdt_info; wdog->ops = &imx7ulp_wdt_ops; @@ -403,6 +394,12 @@ static const struct dev_pm_ops imx7ulp_wdt_pm_ops = { static const struct imx_wdt_hw_feature imx7ulp_wdt_hw = { .prescaler_enable = false, .wdog_clock_rate = 1000, + .post_rcs_wait = true, +}; + +static const struct imx_wdt_hw_feature imx8ulp_wdt_hw = { + .prescaler_enable = false, + .wdog_clock_rate = 1000, }; static const struct imx_wdt_hw_feature imx93_wdt_hw = { @@ -411,8 +408,8 @@ static const struct imx_wdt_hw_feature imx93_wdt_hw = { }; static const struct of_device_id imx7ulp_wdt_dt_ids[] = { - { .compatible = "fsl,imx8ulp-wdt", .data = &imx7ulp_wdt_hw, }, { .compatible = "fsl,imx7ulp-wdt", .data = &imx7ulp_wdt_hw, }, + { .compatible = "fsl,imx8ulp-wdt", .data = &imx8ulp_wdt_hw, }, { .compatible = "fsl,imx93-wdt", .data = &imx93_wdt_hw, }, { /* sentinel */ } };