| Message ID | 20240711-wdt-v1-2-8955a9e05ba0@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | watchdog: imx7ulp_wdt: needn't wait 2.5 clocks after RCS is done for iMX93 | expand |
On 7/11/24 15:41, Frank Li wrote: > From: Alice Guo <alice.guo@nxp.com> > > i.MX93 watchdog needn't wait 2.5 clocks after RCS is done. So set > post_rcs_wait to false for "fsl,imx93-wdt". > > 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> > --- > drivers/watchdog/imx7ulp_wdt.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c > index 904b9f1873856..3a75a6f98f8f0 100644 > --- a/drivers/watchdog/imx7ulp_wdt.c > +++ b/drivers/watchdog/imx7ulp_wdt.c > @@ -405,7 +405,6 @@ static const struct imx_wdt_hw_feature imx8ulp_wdt_hw = { > static const struct imx_wdt_hw_feature imx93_wdt_hw = { > .prescaler_enable = true, > .wdog_clock_rate = 125, > - .post_rcs_wait = true, > }; > > static const struct of_device_id imx7ulp_wdt_dt_ids[] = { > Introducing that flag in the previous patch just to remove it here doesn't make sense to me, sorry. What the two changes do together is to disable post_rcs_wait for iMX93. That is a single logical change, and it can and should be done in a single patch. If you do that by moving the flag into imx_wdt_hw_feature or by adding another of_device_is_compatible() is your call. Guenter
On Thu, Jul 11, 2024 at 03:55:52PM -0700, Guenter Roeck wrote: > On 7/11/24 15:41, Frank Li wrote: > > From: Alice Guo <alice.guo@nxp.com> > > > > i.MX93 watchdog needn't wait 2.5 clocks after RCS is done. So set > > post_rcs_wait to false for "fsl,imx93-wdt". > > > > 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> > > --- > > drivers/watchdog/imx7ulp_wdt.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c > > index 904b9f1873856..3a75a6f98f8f0 100644 > > --- a/drivers/watchdog/imx7ulp_wdt.c > > +++ b/drivers/watchdog/imx7ulp_wdt.c > > @@ -405,7 +405,6 @@ static const struct imx_wdt_hw_feature imx8ulp_wdt_hw = { > > static const struct imx_wdt_hw_feature imx93_wdt_hw = { > > .prescaler_enable = true, > > .wdog_clock_rate = 125, > > - .post_rcs_wait = true, > > }; > > static const struct of_device_id imx7ulp_wdt_dt_ids[] = { > > > Introducing that flag in the previous patch just to remove it here doesn't > make sense to me, sorry. Some maintainer want create function equal patch first if just code restructure/re-originzed. Then add additional change base on it. Of course, I can squash to one if you like. Frank > > What the two changes do together is to disable post_rcs_wait for iMX93. > That is a single logical change, and it can and should be done in a > single patch. If you do that by moving the flag into imx_wdt_hw_feature > or by adding another of_device_is_compatible() is your call. > > Guenter >
On 7/11/24 18:39, Frank Li wrote: > On Thu, Jul 11, 2024 at 03:55:52PM -0700, Guenter Roeck wrote: >> On 7/11/24 15:41, Frank Li wrote: >>> From: Alice Guo <alice.guo@nxp.com> >>> >>> i.MX93 watchdog needn't wait 2.5 clocks after RCS is done. So set >>> post_rcs_wait to false for "fsl,imx93-wdt". >>> >>> 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> >>> --- >>> drivers/watchdog/imx7ulp_wdt.c | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c >>> index 904b9f1873856..3a75a6f98f8f0 100644 >>> --- a/drivers/watchdog/imx7ulp_wdt.c >>> +++ b/drivers/watchdog/imx7ulp_wdt.c >>> @@ -405,7 +405,6 @@ static const struct imx_wdt_hw_feature imx8ulp_wdt_hw = { >>> static const struct imx_wdt_hw_feature imx93_wdt_hw = { >>> .prescaler_enable = true, >>> .wdog_clock_rate = 125, >>> - .post_rcs_wait = true, >>> }; >>> static const struct of_device_id imx7ulp_wdt_dt_ids[] = { >>> >> Introducing that flag in the previous patch just to remove it here doesn't >> make sense to me, sorry. > > Some maintainer want create function equal patch first if just code > restructure/re-originzed. Then add additional change base on it. > In general I would ask you to do that as well, but not if patch 1/2 introduces a change and patch 2/2 does nothing but to remove part of the change introduced in patch 1/2. Guenter
diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c index 904b9f1873856..3a75a6f98f8f0 100644 --- a/drivers/watchdog/imx7ulp_wdt.c +++ b/drivers/watchdog/imx7ulp_wdt.c @@ -405,7 +405,6 @@ static const struct imx_wdt_hw_feature imx8ulp_wdt_hw = { static const struct imx_wdt_hw_feature imx93_wdt_hw = { .prescaler_enable = true, .wdog_clock_rate = 125, - .post_rcs_wait = true, }; static const struct of_device_id imx7ulp_wdt_dt_ids[] = {