* [PATCH] clk: clk_set_parent: skip any operation if current and new parents are equal @ 2025-10-03 10:37 Michael Grzeschik 2025-10-06 11:43 ` Ahmad Fatoum 0 siblings, 1 reply; 5+ messages in thread From: Michael Grzeschik @ 2025-10-03 10:37 UTC (permalink / raw) To: barebox Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> --- drivers/clk/clk.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 89a007a12c5..ac5b83cf8b4 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -302,6 +302,9 @@ int clk_set_parent(struct clk *clk, struct clk *newparent) if (IS_ERR(newparent)) return PTR_ERR(newparent); + if (newparent == curparent) + return 0; + if (!clk->num_parents) return -EINVAL; if (!clk->ops->set_parent) -- 2.47.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clk: clk_set_parent: skip any operation if current and new parents are equal 2025-10-03 10:37 [PATCH] clk: clk_set_parent: skip any operation if current and new parents are equal Michael Grzeschik @ 2025-10-06 11:43 ` Ahmad Fatoum 2025-10-06 20:53 ` Michael Grzeschik 0 siblings, 1 reply; 5+ messages in thread From: Ahmad Fatoum @ 2025-10-06 11:43 UTC (permalink / raw) To: Michael Grzeschik, barebox Hi, On 10/3/25 12:37 PM, Michael Grzeschik wrote: > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> While I agree this makes sense, I appreciate some background. Did you run into problems? Or is it just something you noticed while reading the code? A prime location for that background would be the empty commit message ;) Cheers, Ahmad > --- > drivers/clk/clk.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 89a007a12c5..ac5b83cf8b4 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -302,6 +302,9 @@ int clk_set_parent(struct clk *clk, struct clk *newparent) > if (IS_ERR(newparent)) > return PTR_ERR(newparent); > > + if (newparent == curparent) > + return 0; > + > if (!clk->num_parents) > return -EINVAL; > if (!clk->ops->set_parent) -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clk: clk_set_parent: skip any operation if current and new parents are equal 2025-10-06 11:43 ` Ahmad Fatoum @ 2025-10-06 20:53 ` Michael Grzeschik 2025-10-07 11:50 ` Sascha Hauer 0 siblings, 1 reply; 5+ messages in thread From: Michael Grzeschik @ 2025-10-06 20:53 UTC (permalink / raw) To: Ahmad Fatoum; +Cc: barebox [-- Attachment #1: Type: text/plain, Size: 1921 bytes --] On Mon, Oct 06, 2025 at 01:43:47PM +0200, Ahmad Fatoum wrote: >On 10/3/25 12:37 PM, Michael Grzeschik wrote: >> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > >While I agree this makes sense, I appreciate some background. Did you >run into problems? Or is it just something you noticed while reading the >code? A prime location for that background would be the empty commit >message ;) Yes this happend when using the video drivers. Somehow some drivers tried to reparent the video_pll to the same parent that it already had. clk: failed to reparent video_pll to osc_24m: -22 clk: failed to reparent video_pll to osc_24m: -22 I did not check which one it was. Since this call came twice, it looked right to me to fix this on this obvious location. Michael >> --- >> drivers/clk/clk.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c >> index 89a007a12c5..ac5b83cf8b4 100644 >> --- a/drivers/clk/clk.c >> +++ b/drivers/clk/clk.c >> @@ -302,6 +302,9 @@ int clk_set_parent(struct clk *clk, struct clk *newparent) >> if (IS_ERR(newparent)) >> return PTR_ERR(newparent); >> >> + if (newparent == curparent) >> + return 0; >> + >> if (!clk->num_parents) >> return -EINVAL; >> if (!clk->ops->set_parent) > >-- >Pengutronix e.K. | | >Steuerwalder Str. 21 | http://www.pengutronix.de/ | >31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | >Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clk: clk_set_parent: skip any operation if current and new parents are equal 2025-10-06 20:53 ` Michael Grzeschik @ 2025-10-07 11:50 ` Sascha Hauer 2025-10-07 11:57 ` Sascha Hauer 0 siblings, 1 reply; 5+ messages in thread From: Sascha Hauer @ 2025-10-07 11:50 UTC (permalink / raw) To: Michael Grzeschik; +Cc: Ahmad Fatoum, barebox On Mon, Oct 06, 2025 at 10:53:02PM +0200, Michael Grzeschik wrote: > On Mon, Oct 06, 2025 at 01:43:47PM +0200, Ahmad Fatoum wrote: > > On 10/3/25 12:37 PM, Michael Grzeschik wrote: > > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > > > > While I agree this makes sense, I appreciate some background. Did you > > run into problems? Or is it just something you noticed while reading the > > code? A prime location for that background would be the empty commit > > message ;) > > Yes this happend when using the video drivers. Somehow some drivers > tried to reparent the video_pll to the same parent that it already had. > > clk: failed to reparent video_pll to osc_24m: -22 > clk: failed to reparent video_pll to osc_24m: -22 If it's the same parent it already has, why does it fail with -EINVAL then? Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] clk: clk_set_parent: skip any operation if current and new parents are equal 2025-10-07 11:50 ` Sascha Hauer @ 2025-10-07 11:57 ` Sascha Hauer 0 siblings, 0 replies; 5+ messages in thread From: Sascha Hauer @ 2025-10-07 11:57 UTC (permalink / raw) To: Michael Grzeschik; +Cc: Ahmad Fatoum, barebox On Tue, Oct 07, 2025 at 01:50:28PM +0200, Sascha Hauer wrote: > On Mon, Oct 06, 2025 at 10:53:02PM +0200, Michael Grzeschik wrote: > > On Mon, Oct 06, 2025 at 01:43:47PM +0200, Ahmad Fatoum wrote: > > > On 10/3/25 12:37 PM, Michael Grzeschik wrote: > > > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de> > > > > > > While I agree this makes sense, I appreciate some background. Did you > > > run into problems? Or is it just something you noticed while reading the > > > code? A prime location for that background would be the empty commit > > > message ;) > > > > Yes this happend when using the video drivers. Somehow some drivers > > tried to reparent the video_pll to the same parent that it already had. > > > > clk: failed to reparent video_pll to osc_24m: -22 > > clk: failed to reparent video_pll to osc_24m: -22 > > If it's the same parent it already has, why does it fail with -EINVAL > then? Ah, probably because of: if (!clk->ops->set_parent) return -EINVAL; So the clk only has a single parent, thus no set_parent op. Your patch is indeed correct. Please update the commit message with this information. Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-10-07 11:58 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-10-03 10:37 [PATCH] clk: clk_set_parent: skip any operation if current and new parents are equal Michael Grzeschik 2025-10-06 11:43 ` Ahmad Fatoum 2025-10-06 20:53 ` Michael Grzeschik 2025-10-07 11:50 ` Sascha Hauer 2025-10-07 11:57 ` Sascha Hauer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox