[PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent()

Brian Masney posted 4 patches 1 week, 3 days ago
[PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent()
Posted by Brian Masney 1 week, 3 days ago
roclk_get_parent() and sclk_get_parent() has the possibility of
returning -EINVAL, however the framework expects this call to always
succeed since the return value is unsigned.

If there is no parent map defined, then the current value programmed in
the hardware is used. Let's use that same value in the case where
-EINVAL is currently returned.

This index is only used by clk_core_get_parent_by_index(), and it
validates that it doesn't overflow the number of available parents.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202512050233.R9hAWsJN-lkp@intel.com/
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
 drivers/clk/microchip/clk-core.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c
index a0163441dfe5c1dfc27dae48e64cf3cb3d6b764f..82f62731fc0ed566b0c6b007381c4f10a2a8a7e7 100644
--- a/drivers/clk/microchip/clk-core.c
+++ b/drivers/clk/microchip/clk-core.c
@@ -283,14 +283,13 @@ static u8 roclk_get_parent(struct clk_hw *hw)
 
 	v = (readl(refo->ctrl_reg) >> REFO_SEL_SHIFT) & REFO_SEL_MASK;
 
-	if (!refo->parent_map)
-		return v;
-
-	for (i = 0; i < clk_hw_get_num_parents(hw); i++)
-		if (refo->parent_map[i] == v)
-			return i;
+	if (refo->parent_map) {
+		for (i = 0; i < clk_hw_get_num_parents(hw); i++)
+			if (refo->parent_map[i] == v)
+				return i;
+	}
 
-	return -EINVAL;
+	return v;
 }
 
 static unsigned long roclk_calc_rate(unsigned long parent_rate,
@@ -817,13 +816,13 @@ static u8 sclk_get_parent(struct clk_hw *hw)
 
 	v = (readl(sclk->mux_reg) >> OSC_CUR_SHIFT) & OSC_CUR_MASK;
 
-	if (!sclk->parent_map)
-		return v;
+	if (sclk->parent_map) {
+		for (i = 0; i < clk_hw_get_num_parents(hw); i++)
+			if (sclk->parent_map[i] == v)
+				return i;
+	}
 
-	for (i = 0; i < clk_hw_get_num_parents(hw); i++)
-		if (sclk->parent_map[i] == v)
-			return i;
-	return -EINVAL;
+	return v;
 }
 
 static int sclk_set_parent(struct clk_hw *hw, u8 index)

-- 
2.52.0
Re: [PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent()
Posted by Claudiu Beznea 1 week, 2 days ago

On 12/5/25 21:46, Brian Masney wrote:
> roclk_get_parent() and sclk_get_parent() has the possibility of
> returning -EINVAL, however the framework expects this call to always
> succeed since the return value is unsigned.
> 
> If there is no parent map defined, then the current value programmed in
> the hardware is used. Let's use that same value in the case where
> -EINVAL is currently returned.
> 
> This index is only used by clk_core_get_parent_by_index(), and it
> validates that it doesn't overflow the number of available parents.
> 
> Reported-by: kernel test robot <lkp@intel.com>

I'm getting this from checkpatch:

Applying: clk: microchip: core: correct return value on *_get_parent()
[Checking commit] 910546c58dc2 clk: microchip: core: correct return value
on *_get_parent()
[Checkpatch]      WARNING: Reported-by: should be immediately followed by
Closes: with a URL to the report
#17:
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>


> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202512050233.R9hAWsJN-lkp@intel.com/
> Signed-off-by: Brian Masney <bmasney@redhat.com>

Other than the above:
Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
Re: [PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent()
Posted by Brian Masney 1 week, 2 days ago
On Sat, Dec 06, 2025 at 04:31:03PM +0200, Claudiu Beznea wrote:
> 
> 
> On 12/5/25 21:46, Brian Masney wrote:
> > roclk_get_parent() and sclk_get_parent() has the possibility of
> > returning -EINVAL, however the framework expects this call to always
> > succeed since the return value is unsigned.
> > 
> > If there is no parent map defined, then the current value programmed in
> > the hardware is used. Let's use that same value in the case where
> > -EINVAL is currently returned.
> > 
> > This index is only used by clk_core_get_parent_by_index(), and it
> > validates that it doesn't overflow the number of available parents.
> > 
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> I'm getting this from checkpatch:
> 
> Applying: clk: microchip: core: correct return value on *_get_parent()
> [Checking commit] 910546c58dc2 clk: microchip: core: correct return value
> on *_get_parent()
> [Checkpatch]      WARNING: Reported-by: should be immediately followed by
> Closes: with a URL to the report
> #17:
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>

That's a false positive from checkpatch. It doesn't like the two
Reported-by lines, with a single Closes. The warning goes away if I do
this:

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/r/202512050233.R9hAWsJN-lkp@intel.com/
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202512050233.R9hAWsJN-lkp@intel.com/

> > Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> > Closes: https://lore.kernel.org/r/202512050233.R9hAWsJN-lkp@intel.com/
> > Signed-off-by: Brian Masney <bmasney@redhat.com>
> 
> Other than the above:
> Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>

Thanks!

Brian