* [PATCH v3 1/4] clk: microchip: core: remove duplicate determine_rate on pic32_sclk_ops
2025-12-05 19:46 [PATCH v3 0/4] clk: microchip: core: fix issue with round_rate conversion and allow compile test Brian Masney
@ 2025-12-05 19:46 ` Brian Masney
2025-12-06 14:30 ` Claudiu Beznea
2025-12-05 19:46 ` [PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent() Brian Masney
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Brian Masney @ 2025-12-05 19:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Maxime Ripard, Claudiu Beznea,
Conor Dooley, Dan Carpenter
Cc: linux-clk, linux-kernel, Brian Masney, kernel test robot
pic32_sclk_ops previously had a sclk_round_rate() member, and this was
recently converted over to sclk_determine_rate() with the help of a
Coccinelle semantic patch. pic32_sclk_ops now has two conflicting
determine_rate ops members.
Prior to the conversion, pic32_sclk_ops already had a determine_rate
member that points to __clk_mux_determine_rate(). When both the
round_rate() and determine_rate() ops are defined, the clk core only
uses the determine_rate() op. Let's go ahead and drop the recently
converted sclk_determine_rate() to match the previous functionality
prior to the conversion.
Fixes: e9f039c08cdc ("clk: microchip: core: convert from round_rate() to determine_rate()")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202511222115.uvHrP95A-lkp@intel.com/
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
drivers/clk/microchip/clk-core.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c
index b34348d491f3e1b576b2b9a8a66bfddd8c2296ea..a0163441dfe5c1dfc27dae48e64cf3cb3d6b764f 100644
--- a/drivers/clk/microchip/clk-core.c
+++ b/drivers/clk/microchip/clk-core.c
@@ -780,15 +780,6 @@ static unsigned long sclk_get_rate(struct clk_hw *hw, unsigned long parent_rate)
return parent_rate / div;
}
-static int sclk_determine_rate(struct clk_hw *hw,
- struct clk_rate_request *req)
-{
- req->rate = calc_best_divided_rate(req->rate, req->best_parent_rate,
- SLEW_SYSDIV, 1);
-
- return 0;
-}
-
static int sclk_set_rate(struct clk_hw *hw,
unsigned long rate, unsigned long parent_rate)
{
@@ -912,7 +903,6 @@ static int sclk_init(struct clk_hw *hw)
const struct clk_ops pic32_sclk_ops = {
.get_parent = sclk_get_parent,
.set_parent = sclk_set_parent,
- .determine_rate = sclk_determine_rate,
.set_rate = sclk_set_rate,
.recalc_rate = sclk_get_rate,
.init = sclk_init,
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 1/4] clk: microchip: core: remove duplicate determine_rate on pic32_sclk_ops
2025-12-05 19:46 ` [PATCH v3 1/4] clk: microchip: core: remove duplicate determine_rate on pic32_sclk_ops Brian Masney
@ 2025-12-06 14:30 ` Claudiu Beznea
0 siblings, 0 replies; 10+ messages in thread
From: Claudiu Beznea @ 2025-12-06 14:30 UTC (permalink / raw)
To: Brian Masney, Michael Turquette, Stephen Boyd, Maxime Ripard,
Conor Dooley, Dan Carpenter
Cc: linux-clk, linux-kernel, kernel test robot
On 12/5/25 21:46, Brian Masney wrote:
> pic32_sclk_ops previously had a sclk_round_rate() member, and this was
> recently converted over to sclk_determine_rate() with the help of a
> Coccinelle semantic patch. pic32_sclk_ops now has two conflicting
> determine_rate ops members.
>
> Prior to the conversion, pic32_sclk_ops already had a determine_rate
> member that points to __clk_mux_determine_rate(). When both the
> round_rate() and determine_rate() ops are defined, the clk core only
> uses the determine_rate() op. Let's go ahead and drop the recently
> converted sclk_determine_rate() to match the previous functionality
> prior to the conversion.
>
> Fixes: e9f039c08cdc ("clk: microchip: core: convert from round_rate() to determine_rate()")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202511222115.uvHrP95A-lkp@intel.com/
> Signed-off-by: Brian Masney <bmasney@redhat.com>
Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent()
2025-12-05 19:46 [PATCH v3 0/4] clk: microchip: core: fix issue with round_rate conversion and allow compile test Brian Masney
2025-12-05 19:46 ` [PATCH v3 1/4] clk: microchip: core: remove duplicate determine_rate on pic32_sclk_ops Brian Masney
@ 2025-12-05 19:46 ` Brian Masney
2025-12-06 14:31 ` Claudiu Beznea
2025-12-05 19:46 ` [PATCH v3 3/4] clk: microchip: core: remove unused include asm/traps.h Brian Masney
2025-12-05 19:46 ` [PATCH v3 4/4] clk: microchip: core: allow driver to be compiled with COMPILE_TEST Brian Masney
3 siblings, 1 reply; 10+ messages in thread
From: Brian Masney @ 2025-12-05 19:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Maxime Ripard, Claudiu Beznea,
Conor Dooley, Dan Carpenter
Cc: linux-clk, linux-kernel, Brian Masney, kernel test robot
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent()
2025-12-05 19:46 ` [PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent() Brian Masney
@ 2025-12-06 14:31 ` Claudiu Beznea
2025-12-06 22:38 ` Brian Masney
0 siblings, 1 reply; 10+ messages in thread
From: Claudiu Beznea @ 2025-12-06 14:31 UTC (permalink / raw)
To: Brian Masney, Michael Turquette, Stephen Boyd, Maxime Ripard,
Conor Dooley, Dan Carpenter
Cc: linux-clk, linux-kernel, kernel test robot
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent()
2025-12-06 14:31 ` Claudiu Beznea
@ 2025-12-06 22:38 ` Brian Masney
0 siblings, 0 replies; 10+ messages in thread
From: Brian Masney @ 2025-12-06 22:38 UTC (permalink / raw)
To: Claudiu Beznea
Cc: Michael Turquette, Stephen Boyd, Maxime Ripard, Conor Dooley,
Dan Carpenter, linux-clk, linux-kernel, kernel test robot
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/4] clk: microchip: core: remove unused include asm/traps.h
2025-12-05 19:46 [PATCH v3 0/4] clk: microchip: core: fix issue with round_rate conversion and allow compile test Brian Masney
2025-12-05 19:46 ` [PATCH v3 1/4] clk: microchip: core: remove duplicate determine_rate on pic32_sclk_ops Brian Masney
2025-12-05 19:46 ` [PATCH v3 2/4] clk: microchip: core: correct return value on *_get_parent() Brian Masney
@ 2025-12-05 19:46 ` Brian Masney
2025-12-05 19:46 ` [PATCH v3 4/4] clk: microchip: core: allow driver to be compiled with COMPILE_TEST Brian Masney
3 siblings, 0 replies; 10+ messages in thread
From: Brian Masney @ 2025-12-05 19:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Maxime Ripard, Claudiu Beznea,
Conor Dooley, Dan Carpenter
Cc: linux-clk, linux-kernel, Brian Masney
The asm/traps.h include file is not actually used, so let's go ahead and
remove it.
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
drivers/clk/microchip/clk-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c
index 82f62731fc0ed566b0c6b007381c4f10a2a8a7e7..f467d7bc28c87a50fb18dc527574f973c4b7e615 100644
--- a/drivers/clk/microchip/clk-core.c
+++ b/drivers/clk/microchip/clk-core.c
@@ -10,7 +10,6 @@
#include <linux/io.h>
#include <linux/iopoll.h>
#include <asm/mach-pic32/pic32.h>
-#include <asm/traps.h>
#include "clk-core.h"
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 4/4] clk: microchip: core: allow driver to be compiled with COMPILE_TEST
2025-12-05 19:46 [PATCH v3 0/4] clk: microchip: core: fix issue with round_rate conversion and allow compile test Brian Masney
` (2 preceding siblings ...)
2025-12-05 19:46 ` [PATCH v3 3/4] clk: microchip: core: remove unused include asm/traps.h Brian Masney
@ 2025-12-05 19:46 ` Brian Masney
2025-12-06 14:28 ` Claudiu Beznea
3 siblings, 1 reply; 10+ messages in thread
From: Brian Masney @ 2025-12-05 19:46 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Maxime Ripard, Claudiu Beznea,
Conor Dooley, Dan Carpenter
Cc: linux-clk, linux-kernel, Brian Masney
This driver currently only supports builds against a PIC32 target. To
avoid future breakage in the future, let's update the Kconfig and the
driver so that it can be built with CONFIG_COMPILE_TEST enabled.
Note that with the existing asm calls is not how I'd want to do this
today if this was a new driver, however I don't have access to this
hardware. To avoid any breakage, let's keep the existing behavior.
Signed-off-by: Brian Masney <bmasney@redhat.com>
---
drivers/clk/microchip/Kconfig | 2 +-
drivers/clk/microchip/clk-core.c | 32 +++++++++++++++++++++++---------
2 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/microchip/Kconfig b/drivers/clk/microchip/Kconfig
index 1b9e43eb54976b219a0277cc971f353fd6af226a..1e56a057319d97e20440fe4e107d26fa85c95ab1 100644
--- a/drivers/clk/microchip/Kconfig
+++ b/drivers/clk/microchip/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
config COMMON_CLK_PIC32
- def_bool COMMON_CLK && MACH_PIC32
+ def_bool (COMMON_CLK && MACH_PIC32) || COMPILE_TEST
config MCHP_CLK_MPFS
bool "Clk driver for PolarFire SoC"
diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c
index f467d7bc28c87a50fb18dc527574f973c4b7e615..fad4b45d908310ffb59e4ed57c55ae4266253444 100644
--- a/drivers/clk/microchip/clk-core.c
+++ b/drivers/clk/microchip/clk-core.c
@@ -9,7 +9,15 @@
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/iopoll.h>
+
+#if !defined(CONFIG_MACH_PIC32) && defined(CONFIG_COMPILE_TEST)
+#define PIC32_CLR(_reg) ((_reg) + 0x04)
+#define PIC32_SET(_reg) ((_reg) + 0x08)
+#define PIC32_INV(_reg) ((_reg) + 0x0C)
+#define pic32_syskey_unlock()
+#else
#include <asm/mach-pic32/pic32.h>
+#endif
#include "clk-core.h"
@@ -74,15 +82,21 @@
/* SoC specific clock needed during SPLL clock rate switch */
static struct clk_hw *pic32_sclk_hw;
-/* add instruction pipeline delay while CPU clock is in-transition. */
-#define cpu_nop5() \
-do { \
- __asm__ __volatile__("nop"); \
- __asm__ __volatile__("nop"); \
- __asm__ __volatile__("nop"); \
- __asm__ __volatile__("nop"); \
- __asm__ __volatile__("nop"); \
-} while (0)
+#if !defined(CONFIG_MACH_PIC32) && defined(CONFIG_COMPILE_TEST)
+#define cpu_nop5()
+#else
+{
+ /* add instruction pipeline delay while CPU clock is in-transition. */
+ #define cpu_nop5() \
+ do { \
+ __asm__ __volatile__("nop"); \
+ __asm__ __volatile__("nop"); \
+ __asm__ __volatile__("nop"); \
+ __asm__ __volatile__("nop"); \
+ __asm__ __volatile__("nop"); \
+ } while (0)
+}
+#endif
/* Perpheral bus clocks */
struct pic32_periph_clk {
--
2.52.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 4/4] clk: microchip: core: allow driver to be compiled with COMPILE_TEST
2025-12-05 19:46 ` [PATCH v3 4/4] clk: microchip: core: allow driver to be compiled with COMPILE_TEST Brian Masney
@ 2025-12-06 14:28 ` Claudiu Beznea
2025-12-19 19:32 ` Brian Masney
0 siblings, 1 reply; 10+ messages in thread
From: Claudiu Beznea @ 2025-12-06 14:28 UTC (permalink / raw)
To: Brian Masney, Michael Turquette, Stephen Boyd, Maxime Ripard,
Conor Dooley, Dan Carpenter
Cc: linux-clk, linux-kernel
Hi, Brian,
On 12/5/25 21:46, Brian Masney wrote:
> This driver currently only supports builds against a PIC32 target. To
> avoid future breakage in the future, let's update the Kconfig and the
> driver so that it can be built with CONFIG_COMPILE_TEST enabled.
>
> Note that with the existing asm calls is not how I'd want to do this
> today if this was a new driver, however I don't have access to this
> hardware. To avoid any breakage, let's keep the existing behavior.
>
> Signed-off-by: Brian Masney <bmasney@redhat.com>
> ---
> drivers/clk/microchip/Kconfig | 2 +-
> drivers/clk/microchip/clk-core.c | 32 +++++++++++++++++++++++---------
> 2 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/microchip/Kconfig b/drivers/clk/microchip/Kconfig
> index 1b9e43eb54976b219a0277cc971f353fd6af226a..1e56a057319d97e20440fe4e107d26fa85c95ab1 100644
> --- a/drivers/clk/microchip/Kconfig
> +++ b/drivers/clk/microchip/Kconfig
> @@ -1,7 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
>
> config COMMON_CLK_PIC32
> - def_bool COMMON_CLK && MACH_PIC32
> + def_bool (COMMON_CLK && MACH_PIC32) || COMPILE_TEST
>
> config MCHP_CLK_MPFS
> bool "Clk driver for PolarFire SoC"
> diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c
> index f467d7bc28c87a50fb18dc527574f973c4b7e615..fad4b45d908310ffb59e4ed57c55ae4266253444 100644
> --- a/drivers/clk/microchip/clk-core.c
> +++ b/drivers/clk/microchip/clk-core.c
> @@ -9,7 +9,15 @@
> #include <linux/interrupt.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> +
> +#if !defined(CONFIG_MACH_PIC32) && defined(CONFIG_COMPILE_TEST)
Can't we have something like:
#if defined(CONFIG_MATCH_PIC32)
#include <asm/match-pic32/pic32.h>
#else
#define PIC32_CLR(_reg) ((_reg) + 0x04)
#define PIC32_SET(_reg) ((_reg) + 0x08)
#define PIC32_INV(_reg) ((_reg) + 0x0C)
#define pic32_syskey_unlock()
#endif
> +#define PIC32_CLR(_reg) ((_reg) + 0x04)
> +#define PIC32_SET(_reg) ((_reg) + 0x08)
> +#define PIC32_INV(_reg) ((_reg) + 0x0C)
> +#define pic32_syskey_unlock()
On the other side, there are other drivers using these defines, maybe a
unified approach would fit better? Maybe moving these to
include/linux/platform_data ?
> +#else
> #include <asm/mach-pic32/pic32.h>
> +#endif
>
> #include "clk-core.h"
>
> @@ -74,15 +82,21 @@
> /* SoC specific clock needed during SPLL clock rate switch */
> static struct clk_hw *pic32_sclk_hw;
>
> -/* add instruction pipeline delay while CPU clock is in-transition. */
> -#define cpu_nop5() \
> -do { \
> - __asm__ __volatile__("nop"); \
> - __asm__ __volatile__("nop"); \
> - __asm__ __volatile__("nop"); \
> - __asm__ __volatile__("nop"); \
> - __asm__ __volatile__("nop"); \
> -} while (0)
> +#if !defined(CONFIG_MACH_PIC32) && defined(CONFIG_COMPILE_TEST)
Same here, can't we have:
#ifdef CONFIG_MATCH_PIC32
#define cpu_nop5() \
do { \
__asm__ __volatile__("nop"); \
__asm__ __volatile__("nop"); \
__asm__ __volatile__("nop"); \
__asm__ __volatile__("nop"); \
__asm__ __volatile__("nop"); \
} while (0)
#else
#define cpu_nop5()
#endif
?
> +#define cpu_nop5()
> +#else
> +{
This leads to the following error when compiling:
../drivers/clk/microchip/clk-core.c:88:1: error: expected identifier or ‘(’
before ‘{’ token
88 | {
| ^
Thank you,
Claudiu
> + /* add instruction pipeline delay while CPU clock is in-transition. */
> + #define cpu_nop5() \
> + do { \
> + __asm__ __volatile__("nop"); \
> + __asm__ __volatile__("nop"); \
> + __asm__ __volatile__("nop"); \
> + __asm__ __volatile__("nop"); \
> + __asm__ __volatile__("nop"); \
> + } while (0)
> +}
> +#endif
>
> /* Perpheral bus clocks */
> struct pic32_periph_clk {
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 4/4] clk: microchip: core: allow driver to be compiled with COMPILE_TEST
2025-12-06 14:28 ` Claudiu Beznea
@ 2025-12-19 19:32 ` Brian Masney
0 siblings, 0 replies; 10+ messages in thread
From: Brian Masney @ 2025-12-19 19:32 UTC (permalink / raw)
To: Claudiu Beznea
Cc: Michael Turquette, Stephen Boyd, Maxime Ripard, Conor Dooley,
Dan Carpenter, linux-clk, linux-kernel
Hi Claudiu,
Sorry about the delay writing back. I was at Linux Plumbers in Tokyo.
On Sat, Dec 06, 2025 at 04:28:31PM +0200, Claudiu Beznea wrote:
> On 12/5/25 21:46, Brian Masney wrote:
> > This driver currently only supports builds against a PIC32 target. To
> > avoid future breakage in the future, let's update the Kconfig and the
> > driver so that it can be built with CONFIG_COMPILE_TEST enabled.
> >
> > Note that with the existing asm calls is not how I'd want to do this
> > today if this was a new driver, however I don't have access to this
> > hardware. To avoid any breakage, let's keep the existing behavior.
> >
> > Signed-off-by: Brian Masney <bmasney@redhat.com>
> > ---
> > drivers/clk/microchip/Kconfig | 2 +-
> > drivers/clk/microchip/clk-core.c | 32 +++++++++++++++++++++++---------
> > 2 files changed, 24 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/clk/microchip/Kconfig b/drivers/clk/microchip/Kconfig
> > index 1b9e43eb54976b219a0277cc971f353fd6af226a..1e56a057319d97e20440fe4e107d26fa85c95ab1 100644
> > --- a/drivers/clk/microchip/Kconfig
> > +++ b/drivers/clk/microchip/Kconfig
> > @@ -1,7 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0
> >
> > config COMMON_CLK_PIC32
> > - def_bool COMMON_CLK && MACH_PIC32
> > + def_bool (COMMON_CLK && MACH_PIC32) || COMPILE_TEST
> >
> > config MCHP_CLK_MPFS
> > bool "Clk driver for PolarFire SoC"
> > diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c
> > index f467d7bc28c87a50fb18dc527574f973c4b7e615..fad4b45d908310ffb59e4ed57c55ae4266253444 100644
> > --- a/drivers/clk/microchip/clk-core.c
> > +++ b/drivers/clk/microchip/clk-core.c
> > @@ -9,7 +9,15 @@
> > #include <linux/interrupt.h>
> > #include <linux/io.h>
> > #include <linux/iopoll.h>
> > +
> > +#if !defined(CONFIG_MACH_PIC32) && defined(CONFIG_COMPILE_TEST)
>
> Can't we have something like:
>
> #if defined(CONFIG_MATCH_PIC32)
> #include <asm/match-pic32/pic32.h>
> #else
> #define PIC32_CLR(_reg) ((_reg) + 0x04)
> #define PIC32_SET(_reg) ((_reg) + 0x08)
> #define PIC32_INV(_reg) ((_reg) + 0x0C)
> #define pic32_syskey_unlock()
> #endif
Yes, I agree that would be simpler.
> > +#define PIC32_CLR(_reg) ((_reg) + 0x04)
> > +#define PIC32_SET(_reg) ((_reg) + 0x08)
> > +#define PIC32_INV(_reg) ((_reg) + 0x0C)
> > +#define pic32_syskey_unlock()
>
> On the other side, there are other drivers using these defines, maybe a
> unified approach would fit better? Maybe moving these to
> include/linux/platform_data ?
I agree that would be the better approach. Specifically:
- Move arch/mips/include/asm/mach-pic32/pic32.h to
include/linux/platform_data
- Drop the unused include linux/io.h in pic32.h
- Check for CONFIG_MATCH_PIC32 for the pic32_syskey_unlock define. Make
it a noop for all other architectures.
That would allow us to have the following in the drivers, with no #if's:
#include <linux/platform_data/pic32.h>
I initially wanted to go this route, and I feel that's the best
technical decision, however my only hesitation is that this is going to
touch at least 8 different subsystems. I could probably get the MIPS
folks to take all of this, however it's going to be a pain to collect
all of the ACKs from the different subsystems.
x1:~/src/linux/linus (master %)$ git grep pic32.h
arch/mips/pic32/common/reset.c:#include <asm/mach-pic32/pic32.h>
arch/mips/pic32/common/reset.c:static void pic32_halt(void)
arch/mips/pic32/common/reset.c: pic32_halt();
arch/mips/pic32/common/reset.c: pic32_halt();
arch/mips/pic32/pic32mzda/config.c:#include <asm/mach-pic32/pic32.h>
arch/mips/pic32/pic32mzda/early_clk.c:#include <asm/mach-pic32/pic32.h>
arch/mips/pic32/pic32mzda/early_console.c:#include <asm/mach-pic32/pic32.h>
arch/mips/pic32/pic32mzda/init.c:#include <linux/platform_data/sdhci-pic32.h>
drivers/clk/microchip/clk-core.c:#include <asm/mach-pic32/pic32.h>
drivers/irqchip/irq-pic32-evic.c:#include <asm/mach-pic32/pic32.h>
drivers/mmc/host/sdhci-pic32.c:#include <linux/platform_data/sdhci-pic32.h>
drivers/pinctrl/pinctrl-pic32.c:#include <asm/mach-pic32/pic32.h>
drivers/pinctrl/pinctrl-pic32.c:#include "pinctrl-pic32.h"
drivers/rtc/rtc-pic32.c:#include <asm/mach-pic32/pic32.h>
drivers/tty/serial/pic32_uart.c:#include <asm/mach-pic32/pic32.h>
drivers/watchdog/pic32-dmt.c:#include <asm/mach-pic32/pic32.h>
drivers/watchdog/pic32-wdt.c:#include <asm/mach-pic32/pic32.h>
Thoughts?
> > +#if !defined(CONFIG_MACH_PIC32) && defined(CONFIG_COMPILE_TEST)
>
> Same here, can't we have:
>
> #ifdef CONFIG_MATCH_PIC32
> #define cpu_nop5() \
> do { \
> __asm__ __volatile__("nop"); \
> __asm__ __volatile__("nop"); \
> __asm__ __volatile__("nop"); \
> __asm__ __volatile__("nop"); \
> __asm__ __volatile__("nop"); \
> } while (0)
> #else
> #define cpu_nop5()
> #endif
Yes that sounds good.
Brian
^ permalink raw reply [flat|nested] 10+ messages in thread