* [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h
@ 2021-08-11 18:53 Pali Rohár
2021-08-11 18:53 ` [PATCH 2/2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk() Pali Rohár
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Pali Rohár @ 2021-08-11 18:53 UTC (permalink / raw)
To: Stefan Roese, Marek Behún; +Cc: u-boot
File mach/soc.h is included also in 64-bit mvebu processors, so define
Armada XP related macros only when compiling for Armada XP.
Signed-off-by: Pali Rohár <pali@kernel.org>
---
arch/arm/mach-mvebu/include/mach/soc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
index 8e8a4058550e..aab61f7c15cf 100644
--- a/arch/arm/mach-mvebu/include/mach/soc.h
+++ b/arch/arm/mach-mvebu/include/mach/soc.h
@@ -189,7 +189,7 @@
#define BOOT_FROM_SPI 0x3
#define CONFIG_SYS_TCLK 200000000 /* 200MHz */
-#else
+#elif defined(CONFIG_ARMADA_XP)
/* SAR values for Armada XP */
#define CONFIG_SAR_REG (MVEBU_REGISTER(0x18230))
#define CONFIG_SAR2_REG (MVEBU_REGISTER(0x18234))
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-08-11 18:53 [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h Pali Rohár
@ 2021-08-11 18:53 ` Pali Rohár
2021-08-16 10:02 ` [PATCH v2] " Pali Rohár
2021-08-16 7:58 ` [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h Stefan Roese
2021-09-01 9:09 ` Stefan Roese
2 siblings, 1 reply; 17+ messages in thread
From: Pali Rohár @ 2021-08-11 18:53 UTC (permalink / raw)
To: Stefan Roese, Marek Behún; +Cc: u-boot
Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
read from latched reset register.
Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
macro and completely remove get_ref_clk() function.
Replace also custom open-coded implementation of determining reference
clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
clock in Hz and old function get_ref_clk() returned it in MHz.
Signed-off-by: Pali Rohár <pali@kernel.org>
---
arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
arch/arm/mach-mvebu/include/mach/soc.h | 4 ++++
drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
drivers/watchdog/armada-37xx-wdt.c | 2 +-
8 files changed, 20 insertions(+), 50 deletions(-)
diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
index 7702028ba19b..bdf8dc377528 100644
--- a/arch/arm/mach-mvebu/armada3700/cpu.c
+++ b/arch/arm/mach-mvebu/armada3700/cpu.c
@@ -23,12 +23,6 @@
/* Armada 3700 */
#define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
-#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
-#define MVEBU_XTAL_MODE_MASK BIT(9)
-#define MVEBU_XTAL_MODE_OFFS 9
-#define MVEBU_XTAL_CLOCK_25MHZ 0x0
-#define MVEBU_XTAL_CLOCK_40MHZ 0x1
-
#define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
#define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
@@ -370,21 +364,3 @@ void reset_cpu(void)
*/
writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
}
-
-/*
- * get_ref_clk
- *
- * return: reference clock in MHz (25 or 40)
- */
-u32 get_ref_clk(void)
-{
- u32 regval;
-
- regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
- MVEBU_XTAL_MODE_OFFS;
-
- if (regval == MVEBU_XTAL_CLOCK_25MHZ)
- return 25;
- else
- return 40;
-}
diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
index 79858858c259..9b8907e0fe55 100644
--- a/arch/arm/mach-mvebu/include/mach/cpu.h
+++ b/arch/arm/mach-mvebu/include/mach/cpu.h
@@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
/* A3700 PCIe regions fixer for device tree */
int a3700_fdt_fix_pcie_regions(void *blob);
-/*
- * get_ref_clk
- *
- * return: reference clock in MHz (25 or 40)
- */
-u32 get_ref_clk(void);
-
#endif /* __ASSEMBLY__ */
#endif /* _MVEBU_CPU_H */
diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
index aab61f7c15cf..551eebe7d613 100644
--- a/arch/arm/mach-mvebu/include/mach/soc.h
+++ b/arch/arm/mach-mvebu/include/mach/soc.h
@@ -210,6 +210,10 @@
#define BOOT_FROM_SPI 0x3
#define CONFIG_SYS_TCLK 250000000 /* 250MHz */
+#elif defined(CONFIG_ARMADA_3700)
+/* SAR values for Armada 3700 */
+#define CONFIG_SYS_REF_CLK ((readl(MVEBU_REGISTER(0x13808)) & BIT(9)) ? \
+ 40000000 : 25000000)
#endif
#endif /* _MVEBU_SOC_H */
diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
index 3b767d706009..f5b3f5a78414 100644
--- a/drivers/clk/mvebu/armada-37xx-periph.c
+++ b/drivers/clk/mvebu/armada-37xx-periph.c
@@ -14,7 +14,7 @@
#include <clk.h>
#include <dm.h>
#include <asm/io.h>
-#include <asm/arch/cpu.h>
+#include <mach/soc.h>
#include <dm/device_compat.h>
#include <linux/bitops.h>
@@ -501,7 +501,7 @@ int armada_37xx_tbg_clk_dump(struct udevice *);
int soc_clk_dump(void)
{
- printf(" xtal at %u000000 Hz\n\n", get_ref_clk());
+ printf(" xtal at %u Hz\n\n", CONFIG_SYS_REF_CLK);
if (clk_dump("tbg@13200", armada_37xx_tbg_clk_dump))
return 1;
@@ -581,7 +581,7 @@ static int armada_37xx_periph_clk_probe(struct udevice *dev)
struct clk clk;
if (i == XTAL) {
- priv->parents[i] = get_ref_clk() * 1000000;
+ priv->parents[i] = CONFIG_SYS_REF_CLK;
continue;
}
diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
index 054aff5e6aca..2ec60b15a6af 100644
--- a/drivers/clk/mvebu/armada-37xx-tbg.c
+++ b/drivers/clk/mvebu/armada-37xx-tbg.c
@@ -13,7 +13,7 @@
#include <clk.h>
#include <dm.h>
#include <asm/io.h>
-#include <asm/arch/cpu.h>
+#include <mach/soc.h>
#include <dm/device_compat.h>
#define NUM_TBG 4
@@ -122,7 +122,7 @@ static int armada_37xx_tbg_clk_probe(struct udevice *dev)
return -ENODEV;
}
- xtal = (ulong)get_ref_clk() * 1000000;
+ xtal = CONFIG_SYS_REF_CLK;
for (i = 0; i < NUM_TBG; ++i) {
unsigned int mult, div;
diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
index 06822d1d12e0..5eb137db4884 100644
--- a/drivers/phy/marvell/comphy_a3700.c
+++ b/drivers/phy/marvell/comphy_a3700.c
@@ -211,7 +211,7 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
* 8. Check crystal jumper setting and program the Power and PLL
* Control accordingly
*/
- if (get_ref_clk() == 40) {
+ if (CONFIG_SYS_REF_CLK == 40000000) {
/* 40 MHz */
reg_set16(phy_addr(PCIE, PWR_PLL_CTRL), 0xFC63, 0xFFFF);
} else {
@@ -300,7 +300,7 @@ static int comphy_sata_power_up(u32 invert)
/*
* 2. Select reference clock and PHY mode (SATA)
*/
- if (get_ref_clk() == 40) {
+ if (CONFIG_SYS_REF_CLK == 40000000) {
/* 40 MHz */
reg_set_indirect(vphy_power_reg0, 0x3, 0x00FF);
} else {
@@ -431,7 +431,7 @@ static int comphy_usb3_power_up(u32 lane, u32 type, u32 speed, u32 invert)
* Control accordingly
* 4. Change RX wait
*/
- if (get_ref_clk() == 40) {
+ if (CONFIG_SYS_REF_CLK == 40000000) {
/* 40 MHz */
usb3_reg_set16(PWR_PLL_CTRL, 0xFCA3, 0xFFFF, lane);
usb3_reg_set16(PWR_MGM_TIM1, 0x10C, 0xFFFF, lane);
@@ -556,7 +556,7 @@ static int comphy_usb2_power_up(u8 usb32)
* 0. Setup PLL. 40MHz clock uses defaults.
* See "PLL Settings for Typical REFCLK" table
*/
- if (get_ref_clk() == 25) {
+ if (CONFIG_SYS_REF_CLK == 25000000) {
reg_set(USB2_PHY_BASE(usb32), 5 | (96 << 16),
0x3F | (0xFF << 16) | (0x3 << 28));
}
@@ -773,7 +773,7 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
* 11. Set correct reference clock frequency in COMPHY register
* REF_FREF_SEL.
*/
- if (get_ref_clk() == 40) {
+ if (CONFIG_SYS_REF_CLK == 40000000) {
reg_set16(sgmiiphy_addr(lane, PWR_PLL_CTRL),
0x4 << rf_ref_freq_sel_shift, rf_ref_freq_sel_mask);
} else {
@@ -820,7 +820,7 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
*/
debug("Running C-DPI phy init %s mode\n",
speed == COMPHY_SPEED_3_125G ? "2G5" : "1G");
- if (get_ref_clk() == 40)
+ if (CONFIG_SYS_REF_CLK == 40000000)
comphy_sgmii_phy_init(lane, speed);
/*
diff --git a/drivers/serial/serial_mvebu_a3700.c b/drivers/serial/serial_mvebu_a3700.c
index efc898c08e83..b575b70050c4 100644
--- a/drivers/serial/serial_mvebu_a3700.c
+++ b/drivers/serial/serial_mvebu_a3700.c
@@ -8,7 +8,7 @@
#include <dm.h>
#include <serial.h>
#include <asm/io.h>
-#include <asm/arch/cpu.h>
+#include <mach/soc.h>
#include <linux/delay.h>
struct mvebu_plat {
@@ -237,7 +237,7 @@ static int mvebu_serial_remove(struct udevice *dev)
/* Calculate new divisor against XTAL clock without changing baudrate */
new_oversampling = 0;
- new_parent_rate = get_ref_clk() * 1000000;
+ new_parent_rate = CONFIG_SYS_REF_CLK;
new_divider = DIV_ROUND_CLOSEST(new_parent_rate * divider * d1 * d2 *
oversampling, parent_rate * 16);
@@ -347,12 +347,11 @@ U_BOOT_DRIVER(serial_mvebu) = {
#ifdef CONFIG_DEBUG_MVEBU_A3700_UART
#include <debug_uart.h>
-#include <mach/soc.h>
static inline void _debug_uart_init(void)
{
void __iomem *base = (void __iomem *)CONFIG_DEBUG_UART_BASE;
- u32 parent_rate, divider;
+ u32 divider;
/* reset FIFOs */
writel(UART_CTRL_RXFIFO_RESET | UART_CTRL_TXFIFO_RESET,
@@ -365,9 +364,7 @@ static inline void _debug_uart_init(void)
* Calculate divider
* baudrate = clock / 16 / divider
*/
- parent_rate = (readl(MVEBU_REGISTER(0x13808)) & BIT(9)) ?
- 40000000 : 25000000;
- divider = DIV_ROUND_CLOSEST(parent_rate, CONFIG_BAUDRATE * 16);
+ divider = DIV_ROUND_CLOSEST(CONFIG_SYS_REF_CLK, CONFIG_BAUDRATE * 16);
writel(divider, base + UART_BAUD_REG);
/*
diff --git a/drivers/watchdog/armada-37xx-wdt.c b/drivers/watchdog/armada-37xx-wdt.c
index 6b5e1ab6fc35..915f37b3d8e6 100644
--- a/drivers/watchdog/armada-37xx-wdt.c
+++ b/drivers/watchdog/armada-37xx-wdt.c
@@ -165,7 +165,7 @@ static int a37xx_wdt_probe(struct udevice *dev)
goto err;
priv->reg = (void __iomem *)addr;
- priv->clk_rate = (ulong)get_ref_clk() * 1000000;
+ priv->clk_rate = CONFIG_SYS_REF_CLK;
/*
* We use counter 1 as watchdog timer, therefore we only set bit
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h
2021-08-11 18:53 [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h Pali Rohár
2021-08-11 18:53 ` [PATCH 2/2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk() Pali Rohár
@ 2021-08-16 7:58 ` Stefan Roese
2021-09-01 9:09 ` Stefan Roese
2 siblings, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2021-08-16 7:58 UTC (permalink / raw)
To: Pali Rohár, Marek Behún; +Cc: u-boot
On 11.08.21 20:53, Pali Rohár wrote:
> File mach/soc.h is included also in 64-bit mvebu processors, so define
> Armada XP related macros only when compiling for Armada XP.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> ---
> arch/arm/mach-mvebu/include/mach/soc.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> index 8e8a4058550e..aab61f7c15cf 100644
> --- a/arch/arm/mach-mvebu/include/mach/soc.h
> +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> @@ -189,7 +189,7 @@
> #define BOOT_FROM_SPI 0x3
>
> #define CONFIG_SYS_TCLK 200000000 /* 200MHz */
> -#else
> +#elif defined(CONFIG_ARMADA_XP)
> /* SAR values for Armada XP */
> #define CONFIG_SAR_REG (MVEBU_REGISTER(0x18230))
> #define CONFIG_SAR2_REG (MVEBU_REGISTER(0x18234))
>
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-08-11 18:53 ` [PATCH 2/2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk() Pali Rohár
@ 2021-08-16 10:02 ` Pali Rohár
2021-08-17 7:47 ` Stefan Roese
2021-09-01 9:12 ` Stefan Roese
0 siblings, 2 replies; 17+ messages in thread
From: Pali Rohár @ 2021-08-16 10:02 UTC (permalink / raw)
To: Stefan Roese, Marek Behún; +Cc: u-boot
Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
read from latched reset register.
Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
macro and completely remove get_ref_clk() function.
Replace also custom open-coded implementation of determining reference
clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
clock in Hz and old function get_ref_clk() returned it in MHz.
Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v2:
* Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
---
arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
drivers/watchdog/armada-37xx-wdt.c | 2 +-
8 files changed, 22 insertions(+), 50 deletions(-)
diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
index 7702028ba19b..bdf8dc377528 100644
--- a/arch/arm/mach-mvebu/armada3700/cpu.c
+++ b/arch/arm/mach-mvebu/armada3700/cpu.c
@@ -23,12 +23,6 @@
/* Armada 3700 */
#define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
-#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
-#define MVEBU_XTAL_MODE_MASK BIT(9)
-#define MVEBU_XTAL_MODE_OFFS 9
-#define MVEBU_XTAL_CLOCK_25MHZ 0x0
-#define MVEBU_XTAL_CLOCK_40MHZ 0x1
-
#define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
#define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
@@ -370,21 +364,3 @@ void reset_cpu(void)
*/
writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
}
-
-/*
- * get_ref_clk
- *
- * return: reference clock in MHz (25 or 40)
- */
-u32 get_ref_clk(void)
-{
- u32 regval;
-
- regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
- MVEBU_XTAL_MODE_OFFS;
-
- if (regval == MVEBU_XTAL_CLOCK_25MHZ)
- return 25;
- else
- return 40;
-}
diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
index 79858858c259..9b8907e0fe55 100644
--- a/arch/arm/mach-mvebu/include/mach/cpu.h
+++ b/arch/arm/mach-mvebu/include/mach/cpu.h
@@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
/* A3700 PCIe regions fixer for device tree */
int a3700_fdt_fix_pcie_regions(void *blob);
-/*
- * get_ref_clk
- *
- * return: reference clock in MHz (25 or 40)
- */
-u32 get_ref_clk(void);
-
#endif /* __ASSEMBLY__ */
#endif /* _MVEBU_CPU_H */
diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
index aab61f7c15cf..b03b6de3c6cd 100644
--- a/arch/arm/mach-mvebu/include/mach/soc.h
+++ b/arch/arm/mach-mvebu/include/mach/soc.h
@@ -210,6 +210,12 @@
#define BOOT_FROM_SPI 0x3
#define CONFIG_SYS_TCLK 250000000 /* 250MHz */
+#elif defined(CONFIG_ARMADA_3700)
+/* SAR values for Armada 3700 */
+#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
+#define MVEBU_XTAL_MODE_MASK BIT(9)
+#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
+ 40000000 : 25000000)
#endif
#endif /* _MVEBU_SOC_H */
diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
index 3b767d706009..f5b3f5a78414 100644
--- a/drivers/clk/mvebu/armada-37xx-periph.c
+++ b/drivers/clk/mvebu/armada-37xx-periph.c
@@ -14,7 +14,7 @@
#include <clk.h>
#include <dm.h>
#include <asm/io.h>
-#include <asm/arch/cpu.h>
+#include <mach/soc.h>
#include <dm/device_compat.h>
#include <linux/bitops.h>
@@ -501,7 +501,7 @@ int armada_37xx_tbg_clk_dump(struct udevice *);
int soc_clk_dump(void)
{
- printf(" xtal at %u000000 Hz\n\n", get_ref_clk());
+ printf(" xtal at %u Hz\n\n", CONFIG_SYS_REF_CLK);
if (clk_dump("tbg@13200", armada_37xx_tbg_clk_dump))
return 1;
@@ -581,7 +581,7 @@ static int armada_37xx_periph_clk_probe(struct udevice *dev)
struct clk clk;
if (i == XTAL) {
- priv->parents[i] = get_ref_clk() * 1000000;
+ priv->parents[i] = CONFIG_SYS_REF_CLK;
continue;
}
diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
index 054aff5e6aca..2ec60b15a6af 100644
--- a/drivers/clk/mvebu/armada-37xx-tbg.c
+++ b/drivers/clk/mvebu/armada-37xx-tbg.c
@@ -13,7 +13,7 @@
#include <clk.h>
#include <dm.h>
#include <asm/io.h>
-#include <asm/arch/cpu.h>
+#include <mach/soc.h>
#include <dm/device_compat.h>
#define NUM_TBG 4
@@ -122,7 +122,7 @@ static int armada_37xx_tbg_clk_probe(struct udevice *dev)
return -ENODEV;
}
- xtal = (ulong)get_ref_clk() * 1000000;
+ xtal = CONFIG_SYS_REF_CLK;
for (i = 0; i < NUM_TBG; ++i) {
unsigned int mult, div;
diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
index 06822d1d12e0..5eb137db4884 100644
--- a/drivers/phy/marvell/comphy_a3700.c
+++ b/drivers/phy/marvell/comphy_a3700.c
@@ -211,7 +211,7 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
* 8. Check crystal jumper setting and program the Power and PLL
* Control accordingly
*/
- if (get_ref_clk() == 40) {
+ if (CONFIG_SYS_REF_CLK == 40000000) {
/* 40 MHz */
reg_set16(phy_addr(PCIE, PWR_PLL_CTRL), 0xFC63, 0xFFFF);
} else {
@@ -300,7 +300,7 @@ static int comphy_sata_power_up(u32 invert)
/*
* 2. Select reference clock and PHY mode (SATA)
*/
- if (get_ref_clk() == 40) {
+ if (CONFIG_SYS_REF_CLK == 40000000) {
/* 40 MHz */
reg_set_indirect(vphy_power_reg0, 0x3, 0x00FF);
} else {
@@ -431,7 +431,7 @@ static int comphy_usb3_power_up(u32 lane, u32 type, u32 speed, u32 invert)
* Control accordingly
* 4. Change RX wait
*/
- if (get_ref_clk() == 40) {
+ if (CONFIG_SYS_REF_CLK == 40000000) {
/* 40 MHz */
usb3_reg_set16(PWR_PLL_CTRL, 0xFCA3, 0xFFFF, lane);
usb3_reg_set16(PWR_MGM_TIM1, 0x10C, 0xFFFF, lane);
@@ -556,7 +556,7 @@ static int comphy_usb2_power_up(u8 usb32)
* 0. Setup PLL. 40MHz clock uses defaults.
* See "PLL Settings for Typical REFCLK" table
*/
- if (get_ref_clk() == 25) {
+ if (CONFIG_SYS_REF_CLK == 25000000) {
reg_set(USB2_PHY_BASE(usb32), 5 | (96 << 16),
0x3F | (0xFF << 16) | (0x3 << 28));
}
@@ -773,7 +773,7 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
* 11. Set correct reference clock frequency in COMPHY register
* REF_FREF_SEL.
*/
- if (get_ref_clk() == 40) {
+ if (CONFIG_SYS_REF_CLK == 40000000) {
reg_set16(sgmiiphy_addr(lane, PWR_PLL_CTRL),
0x4 << rf_ref_freq_sel_shift, rf_ref_freq_sel_mask);
} else {
@@ -820,7 +820,7 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
*/
debug("Running C-DPI phy init %s mode\n",
speed == COMPHY_SPEED_3_125G ? "2G5" : "1G");
- if (get_ref_clk() == 40)
+ if (CONFIG_SYS_REF_CLK == 40000000)
comphy_sgmii_phy_init(lane, speed);
/*
diff --git a/drivers/serial/serial_mvebu_a3700.c b/drivers/serial/serial_mvebu_a3700.c
index efc898c08e83..b575b70050c4 100644
--- a/drivers/serial/serial_mvebu_a3700.c
+++ b/drivers/serial/serial_mvebu_a3700.c
@@ -8,7 +8,7 @@
#include <dm.h>
#include <serial.h>
#include <asm/io.h>
-#include <asm/arch/cpu.h>
+#include <mach/soc.h>
#include <linux/delay.h>
struct mvebu_plat {
@@ -237,7 +237,7 @@ static int mvebu_serial_remove(struct udevice *dev)
/* Calculate new divisor against XTAL clock without changing baudrate */
new_oversampling = 0;
- new_parent_rate = get_ref_clk() * 1000000;
+ new_parent_rate = CONFIG_SYS_REF_CLK;
new_divider = DIV_ROUND_CLOSEST(new_parent_rate * divider * d1 * d2 *
oversampling, parent_rate * 16);
@@ -347,12 +347,11 @@ U_BOOT_DRIVER(serial_mvebu) = {
#ifdef CONFIG_DEBUG_MVEBU_A3700_UART
#include <debug_uart.h>
-#include <mach/soc.h>
static inline void _debug_uart_init(void)
{
void __iomem *base = (void __iomem *)CONFIG_DEBUG_UART_BASE;
- u32 parent_rate, divider;
+ u32 divider;
/* reset FIFOs */
writel(UART_CTRL_RXFIFO_RESET | UART_CTRL_TXFIFO_RESET,
@@ -365,9 +364,7 @@ static inline void _debug_uart_init(void)
* Calculate divider
* baudrate = clock / 16 / divider
*/
- parent_rate = (readl(MVEBU_REGISTER(0x13808)) & BIT(9)) ?
- 40000000 : 25000000;
- divider = DIV_ROUND_CLOSEST(parent_rate, CONFIG_BAUDRATE * 16);
+ divider = DIV_ROUND_CLOSEST(CONFIG_SYS_REF_CLK, CONFIG_BAUDRATE * 16);
writel(divider, base + UART_BAUD_REG);
/*
diff --git a/drivers/watchdog/armada-37xx-wdt.c b/drivers/watchdog/armada-37xx-wdt.c
index 6b5e1ab6fc35..915f37b3d8e6 100644
--- a/drivers/watchdog/armada-37xx-wdt.c
+++ b/drivers/watchdog/armada-37xx-wdt.c
@@ -165,7 +165,7 @@ static int a37xx_wdt_probe(struct udevice *dev)
goto err;
priv->reg = (void __iomem *)addr;
- priv->clk_rate = (ulong)get_ref_clk() * 1000000;
+ priv->clk_rate = CONFIG_SYS_REF_CLK;
/*
* We use counter 1 as watchdog timer, therefore we only set bit
--
2.20.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-08-16 10:02 ` [PATCH v2] " Pali Rohár
@ 2021-08-17 7:47 ` Stefan Roese
2021-08-17 12:50 ` Marek Behún
2021-09-01 9:12 ` Stefan Roese
1 sibling, 1 reply; 17+ messages in thread
From: Stefan Roese @ 2021-08-17 7:47 UTC (permalink / raw)
To: Pali Rohár, Marek Behún; +Cc: u-boot
On 16.08.21 12:02, Pali Rohár wrote:
> Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> read from latched reset register.
>
> Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> macro and completely remove get_ref_clk() function.
>
> Replace also custom open-coded implementation of determining reference
> clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
>
> The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> clock in Hz and old function get_ref_clk() returned it in MHz.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
>
> ---
> Changes in v2:
> * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
Reviewed-by: Stefan Roese <sr@denx.de>
Thanks,
Stefan
> ---
> arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> drivers/watchdog/armada-37xx-wdt.c | 2 +-
> 8 files changed, 22 insertions(+), 50 deletions(-)
>
> diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> index 7702028ba19b..bdf8dc377528 100644
> --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> @@ -23,12 +23,6 @@
> /* Armada 3700 */
> #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
>
> -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> -#define MVEBU_XTAL_MODE_MASK BIT(9)
> -#define MVEBU_XTAL_MODE_OFFS 9
> -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> -
> #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
>
> @@ -370,21 +364,3 @@ void reset_cpu(void)
> */
> writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> }
> -
> -/*
> - * get_ref_clk
> - *
> - * return: reference clock in MHz (25 or 40)
> - */
> -u32 get_ref_clk(void)
> -{
> - u32 regval;
> -
> - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> - MVEBU_XTAL_MODE_OFFS;
> -
> - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> - return 25;
> - else
> - return 40;
> -}
> diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> index 79858858c259..9b8907e0fe55 100644
> --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> /* A3700 PCIe regions fixer for device tree */
> int a3700_fdt_fix_pcie_regions(void *blob);
>
> -/*
> - * get_ref_clk
> - *
> - * return: reference clock in MHz (25 or 40)
> - */
> -u32 get_ref_clk(void);
> -
> #endif /* __ASSEMBLY__ */
> #endif /* _MVEBU_CPU_H */
> diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> index aab61f7c15cf..b03b6de3c6cd 100644
> --- a/arch/arm/mach-mvebu/include/mach/soc.h
> +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> @@ -210,6 +210,12 @@
> #define BOOT_FROM_SPI 0x3
>
> #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> +#elif defined(CONFIG_ARMADA_3700)
> +/* SAR values for Armada 3700 */
> +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> +#define MVEBU_XTAL_MODE_MASK BIT(9)
> +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> + 40000000 : 25000000)
> #endif
>
> #endif /* _MVEBU_SOC_H */
> diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
> index 3b767d706009..f5b3f5a78414 100644
> --- a/drivers/clk/mvebu/armada-37xx-periph.c
> +++ b/drivers/clk/mvebu/armada-37xx-periph.c
> @@ -14,7 +14,7 @@
> #include <clk.h>
> #include <dm.h>
> #include <asm/io.h>
> -#include <asm/arch/cpu.h>
> +#include <mach/soc.h>
> #include <dm/device_compat.h>
> #include <linux/bitops.h>
>
> @@ -501,7 +501,7 @@ int armada_37xx_tbg_clk_dump(struct udevice *);
>
> int soc_clk_dump(void)
> {
> - printf(" xtal at %u000000 Hz\n\n", get_ref_clk());
> + printf(" xtal at %u Hz\n\n", CONFIG_SYS_REF_CLK);
>
> if (clk_dump("tbg@13200", armada_37xx_tbg_clk_dump))
> return 1;
> @@ -581,7 +581,7 @@ static int armada_37xx_periph_clk_probe(struct udevice *dev)
> struct clk clk;
>
> if (i == XTAL) {
> - priv->parents[i] = get_ref_clk() * 1000000;
> + priv->parents[i] = CONFIG_SYS_REF_CLK;
> continue;
> }
>
> diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
> index 054aff5e6aca..2ec60b15a6af 100644
> --- a/drivers/clk/mvebu/armada-37xx-tbg.c
> +++ b/drivers/clk/mvebu/armada-37xx-tbg.c
> @@ -13,7 +13,7 @@
> #include <clk.h>
> #include <dm.h>
> #include <asm/io.h>
> -#include <asm/arch/cpu.h>
> +#include <mach/soc.h>
> #include <dm/device_compat.h>
>
> #define NUM_TBG 4
> @@ -122,7 +122,7 @@ static int armada_37xx_tbg_clk_probe(struct udevice *dev)
> return -ENODEV;
> }
>
> - xtal = (ulong)get_ref_clk() * 1000000;
> + xtal = CONFIG_SYS_REF_CLK;
>
> for (i = 0; i < NUM_TBG; ++i) {
> unsigned int mult, div;
> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> index 06822d1d12e0..5eb137db4884 100644
> --- a/drivers/phy/marvell/comphy_a3700.c
> +++ b/drivers/phy/marvell/comphy_a3700.c
> @@ -211,7 +211,7 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
> * 8. Check crystal jumper setting and program the Power and PLL
> * Control accordingly
> */
> - if (get_ref_clk() == 40) {
> + if (CONFIG_SYS_REF_CLK == 40000000) {
> /* 40 MHz */
> reg_set16(phy_addr(PCIE, PWR_PLL_CTRL), 0xFC63, 0xFFFF);
> } else {
> @@ -300,7 +300,7 @@ static int comphy_sata_power_up(u32 invert)
> /*
> * 2. Select reference clock and PHY mode (SATA)
> */
> - if (get_ref_clk() == 40) {
> + if (CONFIG_SYS_REF_CLK == 40000000) {
> /* 40 MHz */
> reg_set_indirect(vphy_power_reg0, 0x3, 0x00FF);
> } else {
> @@ -431,7 +431,7 @@ static int comphy_usb3_power_up(u32 lane, u32 type, u32 speed, u32 invert)
> * Control accordingly
> * 4. Change RX wait
> */
> - if (get_ref_clk() == 40) {
> + if (CONFIG_SYS_REF_CLK == 40000000) {
> /* 40 MHz */
> usb3_reg_set16(PWR_PLL_CTRL, 0xFCA3, 0xFFFF, lane);
> usb3_reg_set16(PWR_MGM_TIM1, 0x10C, 0xFFFF, lane);
> @@ -556,7 +556,7 @@ static int comphy_usb2_power_up(u8 usb32)
> * 0. Setup PLL. 40MHz clock uses defaults.
> * See "PLL Settings for Typical REFCLK" table
> */
> - if (get_ref_clk() == 25) {
> + if (CONFIG_SYS_REF_CLK == 25000000) {
> reg_set(USB2_PHY_BASE(usb32), 5 | (96 << 16),
> 0x3F | (0xFF << 16) | (0x3 << 28));
> }
> @@ -773,7 +773,7 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> * 11. Set correct reference clock frequency in COMPHY register
> * REF_FREF_SEL.
> */
> - if (get_ref_clk() == 40) {
> + if (CONFIG_SYS_REF_CLK == 40000000) {
> reg_set16(sgmiiphy_addr(lane, PWR_PLL_CTRL),
> 0x4 << rf_ref_freq_sel_shift, rf_ref_freq_sel_mask);
> } else {
> @@ -820,7 +820,7 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> */
> debug("Running C-DPI phy init %s mode\n",
> speed == COMPHY_SPEED_3_125G ? "2G5" : "1G");
> - if (get_ref_clk() == 40)
> + if (CONFIG_SYS_REF_CLK == 40000000)
> comphy_sgmii_phy_init(lane, speed);
>
> /*
> diff --git a/drivers/serial/serial_mvebu_a3700.c b/drivers/serial/serial_mvebu_a3700.c
> index efc898c08e83..b575b70050c4 100644
> --- a/drivers/serial/serial_mvebu_a3700.c
> +++ b/drivers/serial/serial_mvebu_a3700.c
> @@ -8,7 +8,7 @@
> #include <dm.h>
> #include <serial.h>
> #include <asm/io.h>
> -#include <asm/arch/cpu.h>
> +#include <mach/soc.h>
> #include <linux/delay.h>
>
> struct mvebu_plat {
> @@ -237,7 +237,7 @@ static int mvebu_serial_remove(struct udevice *dev)
>
> /* Calculate new divisor against XTAL clock without changing baudrate */
> new_oversampling = 0;
> - new_parent_rate = get_ref_clk() * 1000000;
> + new_parent_rate = CONFIG_SYS_REF_CLK;
> new_divider = DIV_ROUND_CLOSEST(new_parent_rate * divider * d1 * d2 *
> oversampling, parent_rate * 16);
>
> @@ -347,12 +347,11 @@ U_BOOT_DRIVER(serial_mvebu) = {
> #ifdef CONFIG_DEBUG_MVEBU_A3700_UART
>
> #include <debug_uart.h>
> -#include <mach/soc.h>
>
> static inline void _debug_uart_init(void)
> {
> void __iomem *base = (void __iomem *)CONFIG_DEBUG_UART_BASE;
> - u32 parent_rate, divider;
> + u32 divider;
>
> /* reset FIFOs */
> writel(UART_CTRL_RXFIFO_RESET | UART_CTRL_TXFIFO_RESET,
> @@ -365,9 +364,7 @@ static inline void _debug_uart_init(void)
> * Calculate divider
> * baudrate = clock / 16 / divider
> */
> - parent_rate = (readl(MVEBU_REGISTER(0x13808)) & BIT(9)) ?
> - 40000000 : 25000000;
> - divider = DIV_ROUND_CLOSEST(parent_rate, CONFIG_BAUDRATE * 16);
> + divider = DIV_ROUND_CLOSEST(CONFIG_SYS_REF_CLK, CONFIG_BAUDRATE * 16);
> writel(divider, base + UART_BAUD_REG);
>
> /*
> diff --git a/drivers/watchdog/armada-37xx-wdt.c b/drivers/watchdog/armada-37xx-wdt.c
> index 6b5e1ab6fc35..915f37b3d8e6 100644
> --- a/drivers/watchdog/armada-37xx-wdt.c
> +++ b/drivers/watchdog/armada-37xx-wdt.c
> @@ -165,7 +165,7 @@ static int a37xx_wdt_probe(struct udevice *dev)
> goto err;
> priv->reg = (void __iomem *)addr;
>
> - priv->clk_rate = (ulong)get_ref_clk() * 1000000;
> + priv->clk_rate = CONFIG_SYS_REF_CLK;
>
> /*
> * We use counter 1 as watchdog timer, therefore we only set bit
>
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-08-17 7:47 ` Stefan Roese
@ 2021-08-17 12:50 ` Marek Behún
2021-08-17 12:56 ` Stefan Roese
0 siblings, 1 reply; 17+ messages in thread
From: Marek Behún @ 2021-08-17 12:50 UTC (permalink / raw)
To: Stefan Roese; +Cc: Pali Rohár, u-boot
> > +#elif defined(CONFIG_ARMADA_3700)
> > +/* SAR values for Armada 3700 */
> > +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> > +#define MVEBU_XTAL_MODE_MASK BIT(9)
> > +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> > + 40000000 : 25000000)
Stefan,
we discussed this with Pali yesterday. For now, I am okay with it, but
I consider this bad design.
This either should be done via the clk API or, if we insist on keeping
it simple, via a static inline function, with caching via a static
variable, so that the register needs to be read only once (at least for
each compilation unit).
Regarding the clk API kernel has driver for armada-37xx-xtal. But I am
thinking about expanding the clk-fixed-mmio driver so that it would be
more generic and could be used for this.
What do you think?
Marek
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-08-17 12:50 ` Marek Behún
@ 2021-08-17 12:56 ` Stefan Roese
0 siblings, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2021-08-17 12:56 UTC (permalink / raw)
To: Marek Behún; +Cc: Pali Rohár, u-boot
Hi Marek,
On 17.08.21 14:50, Marek Behún wrote:
>>> +#elif defined(CONFIG_ARMADA_3700)
>>> +/* SAR values for Armada 3700 */
>>> +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
>>> +#define MVEBU_XTAL_MODE_MASK BIT(9)
>>> +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
>>> + 40000000 : 25000000)
>
> Stefan,
>
> we discussed this with Pali yesterday. For now, I am okay with it, but
> I consider this bad design.
I have to admit, that I'm also not 100% happy. But I didn't want to
burden Pali with more cleanup here.
> This either should be done via the clk API or, if we insist on keeping
> it simple, via a static inline function, with caching via a static
> variable, so that the register needs to be read only once (at least for
> each compilation unit).
>
> Regarding the clk API kernel has driver for armada-37xx-xtal. But I am
> thinking about expanding the clk-fixed-mmio driver so that it would be
> more generic and could be used for this.
>
> What do you think?
I would very much welcome such a change / enhancement. This legacy type
input clock handling in MVEBU (and some other platforms) definitely
needs the get changed to "correct CLK API" handling.
Thanks,
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h
2021-08-11 18:53 [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h Pali Rohár
2021-08-11 18:53 ` [PATCH 2/2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk() Pali Rohár
2021-08-16 7:58 ` [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h Stefan Roese
@ 2021-09-01 9:09 ` Stefan Roese
2 siblings, 0 replies; 17+ messages in thread
From: Stefan Roese @ 2021-09-01 9:09 UTC (permalink / raw)
To: Pali Rohár, Marek Behún; +Cc: u-boot
On 11.08.21 20:53, Pali Rohár wrote:
> File mach/soc.h is included also in 64-bit mvebu processors, so define
> Armada XP related macros only when compiling for Armada XP.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
Applied to u-boot-marvell/master
Thanks,
Stefan
> ---
> arch/arm/mach-mvebu/include/mach/soc.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> index 8e8a4058550e..aab61f7c15cf 100644
> --- a/arch/arm/mach-mvebu/include/mach/soc.h
> +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> @@ -189,7 +189,7 @@
> #define BOOT_FROM_SPI 0x3
>
> #define CONFIG_SYS_TCLK 200000000 /* 200MHz */
> -#else
> +#elif defined(CONFIG_ARMADA_XP)
> /* SAR values for Armada XP */
> #define CONFIG_SAR_REG (MVEBU_REGISTER(0x18230))
> #define CONFIG_SAR2_REG (MVEBU_REGISTER(0x18234))
>
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-08-16 10:02 ` [PATCH v2] " Pali Rohár
2021-08-17 7:47 ` Stefan Roese
@ 2021-09-01 9:12 ` Stefan Roese
2021-09-01 12:14 ` Tom Rini
1 sibling, 1 reply; 17+ messages in thread
From: Stefan Roese @ 2021-09-01 9:12 UTC (permalink / raw)
To: Pali Rohár, Marek Behún; +Cc: u-boot
Hi Pali,
On 16.08.21 12:02, Pali Rohár wrote:
> Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> read from latched reset register.
>
> Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> macro and completely remove get_ref_clk() function.
>
> Replace also custom open-coded implementation of determining reference
> clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
>
> The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> clock in Hz and old function get_ref_clk() returned it in MHz.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
>
> ---
> Changes in v2:
> * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
This patch does not apply any more, with all the other patches applied.
Please wait a bit until these patches are included in master and then
send a new version.
Sorry for the trouble.
Thanks,
Stefan
> ---
> arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> drivers/watchdog/armada-37xx-wdt.c | 2 +-
> 8 files changed, 22 insertions(+), 50 deletions(-)
>
> diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> index 7702028ba19b..bdf8dc377528 100644
> --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> @@ -23,12 +23,6 @@
> /* Armada 3700 */
> #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
>
> -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> -#define MVEBU_XTAL_MODE_MASK BIT(9)
> -#define MVEBU_XTAL_MODE_OFFS 9
> -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> -
> #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
>
> @@ -370,21 +364,3 @@ void reset_cpu(void)
> */
> writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> }
> -
> -/*
> - * get_ref_clk
> - *
> - * return: reference clock in MHz (25 or 40)
> - */
> -u32 get_ref_clk(void)
> -{
> - u32 regval;
> -
> - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> - MVEBU_XTAL_MODE_OFFS;
> -
> - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> - return 25;
> - else
> - return 40;
> -}
> diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> index 79858858c259..9b8907e0fe55 100644
> --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> /* A3700 PCIe regions fixer for device tree */
> int a3700_fdt_fix_pcie_regions(void *blob);
>
> -/*
> - * get_ref_clk
> - *
> - * return: reference clock in MHz (25 or 40)
> - */
> -u32 get_ref_clk(void);
> -
> #endif /* __ASSEMBLY__ */
> #endif /* _MVEBU_CPU_H */
> diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> index aab61f7c15cf..b03b6de3c6cd 100644
> --- a/arch/arm/mach-mvebu/include/mach/soc.h
> +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> @@ -210,6 +210,12 @@
> #define BOOT_FROM_SPI 0x3
>
> #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> +#elif defined(CONFIG_ARMADA_3700)
> +/* SAR values for Armada 3700 */
> +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> +#define MVEBU_XTAL_MODE_MASK BIT(9)
> +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> + 40000000 : 25000000)
> #endif
>
> #endif /* _MVEBU_SOC_H */
> diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
> index 3b767d706009..f5b3f5a78414 100644
> --- a/drivers/clk/mvebu/armada-37xx-periph.c
> +++ b/drivers/clk/mvebu/armada-37xx-periph.c
> @@ -14,7 +14,7 @@
> #include <clk.h>
> #include <dm.h>
> #include <asm/io.h>
> -#include <asm/arch/cpu.h>
> +#include <mach/soc.h>
> #include <dm/device_compat.h>
> #include <linux/bitops.h>
>
> @@ -501,7 +501,7 @@ int armada_37xx_tbg_clk_dump(struct udevice *);
>
> int soc_clk_dump(void)
> {
> - printf(" xtal at %u000000 Hz\n\n", get_ref_clk());
> + printf(" xtal at %u Hz\n\n", CONFIG_SYS_REF_CLK);
>
> if (clk_dump("tbg@13200", armada_37xx_tbg_clk_dump))
> return 1;
> @@ -581,7 +581,7 @@ static int armada_37xx_periph_clk_probe(struct udevice *dev)
> struct clk clk;
>
> if (i == XTAL) {
> - priv->parents[i] = get_ref_clk() * 1000000;
> + priv->parents[i] = CONFIG_SYS_REF_CLK;
> continue;
> }
>
> diff --git a/drivers/clk/mvebu/armada-37xx-tbg.c b/drivers/clk/mvebu/armada-37xx-tbg.c
> index 054aff5e6aca..2ec60b15a6af 100644
> --- a/drivers/clk/mvebu/armada-37xx-tbg.c
> +++ b/drivers/clk/mvebu/armada-37xx-tbg.c
> @@ -13,7 +13,7 @@
> #include <clk.h>
> #include <dm.h>
> #include <asm/io.h>
> -#include <asm/arch/cpu.h>
> +#include <mach/soc.h>
> #include <dm/device_compat.h>
>
> #define NUM_TBG 4
> @@ -122,7 +122,7 @@ static int armada_37xx_tbg_clk_probe(struct udevice *dev)
> return -ENODEV;
> }
>
> - xtal = (ulong)get_ref_clk() * 1000000;
> + xtal = CONFIG_SYS_REF_CLK;
>
> for (i = 0; i < NUM_TBG; ++i) {
> unsigned int mult, div;
> diff --git a/drivers/phy/marvell/comphy_a3700.c b/drivers/phy/marvell/comphy_a3700.c
> index 06822d1d12e0..5eb137db4884 100644
> --- a/drivers/phy/marvell/comphy_a3700.c
> +++ b/drivers/phy/marvell/comphy_a3700.c
> @@ -211,7 +211,7 @@ static int comphy_pcie_power_up(u32 speed, u32 invert)
> * 8. Check crystal jumper setting and program the Power and PLL
> * Control accordingly
> */
> - if (get_ref_clk() == 40) {
> + if (CONFIG_SYS_REF_CLK == 40000000) {
> /* 40 MHz */
> reg_set16(phy_addr(PCIE, PWR_PLL_CTRL), 0xFC63, 0xFFFF);
> } else {
> @@ -300,7 +300,7 @@ static int comphy_sata_power_up(u32 invert)
> /*
> * 2. Select reference clock and PHY mode (SATA)
> */
> - if (get_ref_clk() == 40) {
> + if (CONFIG_SYS_REF_CLK == 40000000) {
> /* 40 MHz */
> reg_set_indirect(vphy_power_reg0, 0x3, 0x00FF);
> } else {
> @@ -431,7 +431,7 @@ static int comphy_usb3_power_up(u32 lane, u32 type, u32 speed, u32 invert)
> * Control accordingly
> * 4. Change RX wait
> */
> - if (get_ref_clk() == 40) {
> + if (CONFIG_SYS_REF_CLK == 40000000) {
> /* 40 MHz */
> usb3_reg_set16(PWR_PLL_CTRL, 0xFCA3, 0xFFFF, lane);
> usb3_reg_set16(PWR_MGM_TIM1, 0x10C, 0xFFFF, lane);
> @@ -556,7 +556,7 @@ static int comphy_usb2_power_up(u8 usb32)
> * 0. Setup PLL. 40MHz clock uses defaults.
> * See "PLL Settings for Typical REFCLK" table
> */
> - if (get_ref_clk() == 25) {
> + if (CONFIG_SYS_REF_CLK == 25000000) {
> reg_set(USB2_PHY_BASE(usb32), 5 | (96 << 16),
> 0x3F | (0xFF << 16) | (0x3 << 28));
> }
> @@ -773,7 +773,7 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> * 11. Set correct reference clock frequency in COMPHY register
> * REF_FREF_SEL.
> */
> - if (get_ref_clk() == 40) {
> + if (CONFIG_SYS_REF_CLK == 40000000) {
> reg_set16(sgmiiphy_addr(lane, PWR_PLL_CTRL),
> 0x4 << rf_ref_freq_sel_shift, rf_ref_freq_sel_mask);
> } else {
> @@ -820,7 +820,7 @@ static int comphy_sgmii_power_up(u32 lane, u32 speed, u32 invert)
> */
> debug("Running C-DPI phy init %s mode\n",
> speed == COMPHY_SPEED_3_125G ? "2G5" : "1G");
> - if (get_ref_clk() == 40)
> + if (CONFIG_SYS_REF_CLK == 40000000)
> comphy_sgmii_phy_init(lane, speed);
>
> /*
> diff --git a/drivers/serial/serial_mvebu_a3700.c b/drivers/serial/serial_mvebu_a3700.c
> index efc898c08e83..b575b70050c4 100644
> --- a/drivers/serial/serial_mvebu_a3700.c
> +++ b/drivers/serial/serial_mvebu_a3700.c
> @@ -8,7 +8,7 @@
> #include <dm.h>
> #include <serial.h>
> #include <asm/io.h>
> -#include <asm/arch/cpu.h>
> +#include <mach/soc.h>
> #include <linux/delay.h>
>
> struct mvebu_plat {
> @@ -237,7 +237,7 @@ static int mvebu_serial_remove(struct udevice *dev)
>
> /* Calculate new divisor against XTAL clock without changing baudrate */
> new_oversampling = 0;
> - new_parent_rate = get_ref_clk() * 1000000;
> + new_parent_rate = CONFIG_SYS_REF_CLK;
> new_divider = DIV_ROUND_CLOSEST(new_parent_rate * divider * d1 * d2 *
> oversampling, parent_rate * 16);
>
> @@ -347,12 +347,11 @@ U_BOOT_DRIVER(serial_mvebu) = {
> #ifdef CONFIG_DEBUG_MVEBU_A3700_UART
>
> #include <debug_uart.h>
> -#include <mach/soc.h>
>
> static inline void _debug_uart_init(void)
> {
> void __iomem *base = (void __iomem *)CONFIG_DEBUG_UART_BASE;
> - u32 parent_rate, divider;
> + u32 divider;
>
> /* reset FIFOs */
> writel(UART_CTRL_RXFIFO_RESET | UART_CTRL_TXFIFO_RESET,
> @@ -365,9 +364,7 @@ static inline void _debug_uart_init(void)
> * Calculate divider
> * baudrate = clock / 16 / divider
> */
> - parent_rate = (readl(MVEBU_REGISTER(0x13808)) & BIT(9)) ?
> - 40000000 : 25000000;
> - divider = DIV_ROUND_CLOSEST(parent_rate, CONFIG_BAUDRATE * 16);
> + divider = DIV_ROUND_CLOSEST(CONFIG_SYS_REF_CLK, CONFIG_BAUDRATE * 16);
> writel(divider, base + UART_BAUD_REG);
>
> /*
> diff --git a/drivers/watchdog/armada-37xx-wdt.c b/drivers/watchdog/armada-37xx-wdt.c
> index 6b5e1ab6fc35..915f37b3d8e6 100644
> --- a/drivers/watchdog/armada-37xx-wdt.c
> +++ b/drivers/watchdog/armada-37xx-wdt.c
> @@ -165,7 +165,7 @@ static int a37xx_wdt_probe(struct udevice *dev)
> goto err;
> priv->reg = (void __iomem *)addr;
>
> - priv->clk_rate = (ulong)get_ref_clk() * 1000000;
> + priv->clk_rate = CONFIG_SYS_REF_CLK;
>
> /*
> * We use counter 1 as watchdog timer, therefore we only set bit
>
Viele Grüße,
Stefan
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-09-01 9:12 ` Stefan Roese
@ 2021-09-01 12:14 ` Tom Rini
2021-09-01 12:32 ` Pali Rohár
0 siblings, 1 reply; 17+ messages in thread
From: Tom Rini @ 2021-09-01 12:14 UTC (permalink / raw)
To: Stefan Roese; +Cc: Pali Rohár, Marek Behún, u-boot
[-- Attachment #1: Type: text/plain, Size: 4536 bytes --]
On Wed, Sep 01, 2021 at 11:12:58AM +0200, Stefan Roese wrote:
> Hi Pali,
>
> On 16.08.21 12:02, Pali Rohár wrote:
> > Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> > CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> > read from latched reset register.
> >
> > Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> > macro and completely remove get_ref_clk() function.
> >
> > Replace also custom open-coded implementation of determining reference
> > clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
> >
> > The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> > clock in Hz and old function get_ref_clk() returned it in MHz.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> >
> > ---
> > Changes in v2:
> > * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
>
> This patch does not apply any more, with all the other patches applied.
> Please wait a bit until these patches are included in master and then
> send a new version.
>
> Sorry for the trouble.
>
> Thanks,
> Stefan
>
> > ---
> > arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> > arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> > arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> > drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> > drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> > drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> > drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> > drivers/watchdog/armada-37xx-wdt.c | 2 +-
> > 8 files changed, 22 insertions(+), 50 deletions(-)
> >
> > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> > index 7702028ba19b..bdf8dc377528 100644
> > --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> > @@ -23,12 +23,6 @@
> > /* Armada 3700 */
> > #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
> > -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> > -#define MVEBU_XTAL_MODE_MASK BIT(9)
> > -#define MVEBU_XTAL_MODE_OFFS 9
> > -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> > -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> > -
> > #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> > #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
> > @@ -370,21 +364,3 @@ void reset_cpu(void)
> > */
> > writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> > }
> > -
> > -/*
> > - * get_ref_clk
> > - *
> > - * return: reference clock in MHz (25 or 40)
> > - */
> > -u32 get_ref_clk(void)
> > -{
> > - u32 regval;
> > -
> > - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> > - MVEBU_XTAL_MODE_OFFS;
> > -
> > - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> > - return 25;
> > - else
> > - return 40;
> > -}
> > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> > index 79858858c259..9b8907e0fe55 100644
> > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> > /* A3700 PCIe regions fixer for device tree */
> > int a3700_fdt_fix_pcie_regions(void *blob);
> > -/*
> > - * get_ref_clk
> > - *
> > - * return: reference clock in MHz (25 or 40)
> > - */
> > -u32 get_ref_clk(void);
> > -
> > #endif /* __ASSEMBLY__ */
> > #endif /* _MVEBU_CPU_H */
> > diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> > index aab61f7c15cf..b03b6de3c6cd 100644
> > --- a/arch/arm/mach-mvebu/include/mach/soc.h
> > +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> > @@ -210,6 +210,12 @@
> > #define BOOT_FROM_SPI 0x3
> > #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> > +#elif defined(CONFIG_ARMADA_3700)
> > +/* SAR values for Armada 3700 */
> > +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> > +#define MVEBU_XTAL_MODE_MASK BIT(9)
> > +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> > + 40000000 : 25000000)
NAK. CONFIG_xxx which evaluate out to a macro / function are the
hardest to convert to Kconfig. This patch is taking a step backwards.
In fact, wait, how does patch apply and work? There are no
CONFIG_SYS_REF_CLK instances today, so the build should blow up about
adding a new non-Kconfig symbol.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-09-01 12:14 ` Tom Rini
@ 2021-09-01 12:32 ` Pali Rohár
2021-09-01 12:35 ` Tom Rini
0 siblings, 1 reply; 17+ messages in thread
From: Pali Rohár @ 2021-09-01 12:32 UTC (permalink / raw)
To: Tom Rini; +Cc: Stefan Roese, Marek Behún, u-boot
On Wednesday 01 September 2021 08:14:10 Tom Rini wrote:
> On Wed, Sep 01, 2021 at 11:12:58AM +0200, Stefan Roese wrote:
>
> > Hi Pali,
> >
> > On 16.08.21 12:02, Pali Rohár wrote:
> > > Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> > > CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> > > read from latched reset register.
> > >
> > > Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> > > macro and completely remove get_ref_clk() function.
> > >
> > > Replace also custom open-coded implementation of determining reference
> > > clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
> > >
> > > The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> > > clock in Hz and old function get_ref_clk() returned it in MHz.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > >
> > > ---
> > > Changes in v2:
> > > * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
> >
> > This patch does not apply any more, with all the other patches applied.
> > Please wait a bit until these patches are included in master and then
> > send a new version.
> >
> > Sorry for the trouble.
> >
> > Thanks,
> > Stefan
> >
> > > ---
> > > arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> > > arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> > > arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> > > drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> > > drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> > > drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> > > drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> > > drivers/watchdog/armada-37xx-wdt.c | 2 +-
> > > 8 files changed, 22 insertions(+), 50 deletions(-)
> > >
> > > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > index 7702028ba19b..bdf8dc377528 100644
> > > --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> > > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > @@ -23,12 +23,6 @@
> > > /* Armada 3700 */
> > > #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
> > > -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> > > -#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > -#define MVEBU_XTAL_MODE_OFFS 9
> > > -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> > > -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> > > -
> > > #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> > > #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
> > > @@ -370,21 +364,3 @@ void reset_cpu(void)
> > > */
> > > writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> > > }
> > > -
> > > -/*
> > > - * get_ref_clk
> > > - *
> > > - * return: reference clock in MHz (25 or 40)
> > > - */
> > > -u32 get_ref_clk(void)
> > > -{
> > > - u32 regval;
> > > -
> > > - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> > > - MVEBU_XTAL_MODE_OFFS;
> > > -
> > > - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> > > - return 25;
> > > - else
> > > - return 40;
> > > -}
> > > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > index 79858858c259..9b8907e0fe55 100644
> > > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> > > /* A3700 PCIe regions fixer for device tree */
> > > int a3700_fdt_fix_pcie_regions(void *blob);
> > > -/*
> > > - * get_ref_clk
> > > - *
> > > - * return: reference clock in MHz (25 or 40)
> > > - */
> > > -u32 get_ref_clk(void);
> > > -
> > > #endif /* __ASSEMBLY__ */
> > > #endif /* _MVEBU_CPU_H */
> > > diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> > > index aab61f7c15cf..b03b6de3c6cd 100644
> > > --- a/arch/arm/mach-mvebu/include/mach/soc.h
> > > +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> > > @@ -210,6 +210,12 @@
> > > #define BOOT_FROM_SPI 0x3
> > > #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> > > +#elif defined(CONFIG_ARMADA_3700)
> > > +/* SAR values for Armada 3700 */
> > > +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> > > +#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> > > + 40000000 : 25000000)
>
> NAK. CONFIG_xxx which evaluate out to a macro / function are the
> hardest to convert to Kconfig. This patch is taking a step backwards.
> In fact, wait, how does patch apply and work? There are no
> CONFIG_SYS_REF_CLK instances today, so the build should blow up about
> adding a new non-Kconfig symbol.
So, could you please provide some other solution for this issue which
Marek and Stefan pointed?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-09-01 12:32 ` Pali Rohár
@ 2021-09-01 12:35 ` Tom Rini
2021-09-01 12:40 ` Pali Rohár
0 siblings, 1 reply; 17+ messages in thread
From: Tom Rini @ 2021-09-01 12:35 UTC (permalink / raw)
To: Pali Rohár; +Cc: Stefan Roese, Marek Behún, u-boot
[-- Attachment #1: Type: text/plain, Size: 5566 bytes --]
On Wed, Sep 01, 2021 at 02:32:43PM +0200, Pali Rohár wrote:
> On Wednesday 01 September 2021 08:14:10 Tom Rini wrote:
> > On Wed, Sep 01, 2021 at 11:12:58AM +0200, Stefan Roese wrote:
> >
> > > Hi Pali,
> > >
> > > On 16.08.21 12:02, Pali Rohár wrote:
> > > > Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> > > > CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> > > > read from latched reset register.
> > > >
> > > > Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> > > > macro and completely remove get_ref_clk() function.
> > > >
> > > > Replace also custom open-coded implementation of determining reference
> > > > clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
> > > >
> > > > The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> > > > clock in Hz and old function get_ref_clk() returned it in MHz.
> > > >
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > >
> > > > ---
> > > > Changes in v2:
> > > > * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
> > >
> > > This patch does not apply any more, with all the other patches applied.
> > > Please wait a bit until these patches are included in master and then
> > > send a new version.
> > >
> > > Sorry for the trouble.
> > >
> > > Thanks,
> > > Stefan
> > >
> > > > ---
> > > > arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> > > > arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> > > > arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> > > > drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> > > > drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> > > > drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> > > > drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> > > > drivers/watchdog/armada-37xx-wdt.c | 2 +-
> > > > 8 files changed, 22 insertions(+), 50 deletions(-)
> > > >
> > > > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > index 7702028ba19b..bdf8dc377528 100644
> > > > --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > @@ -23,12 +23,6 @@
> > > > /* Armada 3700 */
> > > > #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
> > > > -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> > > > -#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > -#define MVEBU_XTAL_MODE_OFFS 9
> > > > -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> > > > -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> > > > -
> > > > #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> > > > #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
> > > > @@ -370,21 +364,3 @@ void reset_cpu(void)
> > > > */
> > > > writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> > > > }
> > > > -
> > > > -/*
> > > > - * get_ref_clk
> > > > - *
> > > > - * return: reference clock in MHz (25 or 40)
> > > > - */
> > > > -u32 get_ref_clk(void)
> > > > -{
> > > > - u32 regval;
> > > > -
> > > > - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> > > > - MVEBU_XTAL_MODE_OFFS;
> > > > -
> > > > - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> > > > - return 25;
> > > > - else
> > > > - return 40;
> > > > -}
> > > > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > index 79858858c259..9b8907e0fe55 100644
> > > > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> > > > /* A3700 PCIe regions fixer for device tree */
> > > > int a3700_fdt_fix_pcie_regions(void *blob);
> > > > -/*
> > > > - * get_ref_clk
> > > > - *
> > > > - * return: reference clock in MHz (25 or 40)
> > > > - */
> > > > -u32 get_ref_clk(void);
> > > > -
> > > > #endif /* __ASSEMBLY__ */
> > > > #endif /* _MVEBU_CPU_H */
> > > > diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > index aab61f7c15cf..b03b6de3c6cd 100644
> > > > --- a/arch/arm/mach-mvebu/include/mach/soc.h
> > > > +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > @@ -210,6 +210,12 @@
> > > > #define BOOT_FROM_SPI 0x3
> > > > #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> > > > +#elif defined(CONFIG_ARMADA_3700)
> > > > +/* SAR values for Armada 3700 */
> > > > +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> > > > +#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> > > > + 40000000 : 25000000)
> >
> > NAK. CONFIG_xxx which evaluate out to a macro / function are the
> > hardest to convert to Kconfig. This patch is taking a step backwards.
> > In fact, wait, how does patch apply and work? There are no
> > CONFIG_SYS_REF_CLK instances today, so the build should blow up about
> > adding a new non-Kconfig symbol.
>
> So, could you please provide some other solution for this issue which
> Marek and Stefan pointed?
I don't know what the issue is, sorry. But you cannot do what you're
doing there with CONFIG. If for some reason you cannot use an inline
function, just don't name it CONFIG_SYS_REF_CLK. But also, really, did
your build not fail when you tried to do this? It really should have
failed and told you to not add new CONFIG symbols.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-09-01 12:35 ` Tom Rini
@ 2021-09-01 12:40 ` Pali Rohár
2021-09-01 12:41 ` Tom Rini
0 siblings, 1 reply; 17+ messages in thread
From: Pali Rohár @ 2021-09-01 12:40 UTC (permalink / raw)
To: Tom Rini; +Cc: Stefan Roese, Marek Behún, u-boot
On Wednesday 01 September 2021 08:35:33 Tom Rini wrote:
> On Wed, Sep 01, 2021 at 02:32:43PM +0200, Pali Rohár wrote:
> > On Wednesday 01 September 2021 08:14:10 Tom Rini wrote:
> > > On Wed, Sep 01, 2021 at 11:12:58AM +0200, Stefan Roese wrote:
> > >
> > > > Hi Pali,
> > > >
> > > > On 16.08.21 12:02, Pali Rohár wrote:
> > > > > Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> > > > > CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> > > > > read from latched reset register.
> > > > >
> > > > > Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> > > > > macro and completely remove get_ref_clk() function.
> > > > >
> > > > > Replace also custom open-coded implementation of determining reference
> > > > > clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
> > > > >
> > > > > The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> > > > > clock in Hz and old function get_ref_clk() returned it in MHz.
> > > > >
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > >
> > > > > ---
> > > > > Changes in v2:
> > > > > * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
> > > >
> > > > This patch does not apply any more, with all the other patches applied.
> > > > Please wait a bit until these patches are included in master and then
> > > > send a new version.
> > > >
> > > > Sorry for the trouble.
> > > >
> > > > Thanks,
> > > > Stefan
> > > >
> > > > > ---
> > > > > arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> > > > > arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> > > > > arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> > > > > drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> > > > > drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> > > > > drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> > > > > drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> > > > > drivers/watchdog/armada-37xx-wdt.c | 2 +-
> > > > > 8 files changed, 22 insertions(+), 50 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > index 7702028ba19b..bdf8dc377528 100644
> > > > > --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > @@ -23,12 +23,6 @@
> > > > > /* Armada 3700 */
> > > > > #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
> > > > > -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> > > > > -#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > -#define MVEBU_XTAL_MODE_OFFS 9
> > > > > -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> > > > > -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> > > > > -
> > > > > #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> > > > > #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
> > > > > @@ -370,21 +364,3 @@ void reset_cpu(void)
> > > > > */
> > > > > writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> > > > > }
> > > > > -
> > > > > -/*
> > > > > - * get_ref_clk
> > > > > - *
> > > > > - * return: reference clock in MHz (25 or 40)
> > > > > - */
> > > > > -u32 get_ref_clk(void)
> > > > > -{
> > > > > - u32 regval;
> > > > > -
> > > > > - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> > > > > - MVEBU_XTAL_MODE_OFFS;
> > > > > -
> > > > > - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> > > > > - return 25;
> > > > > - else
> > > > > - return 40;
> > > > > -}
> > > > > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > index 79858858c259..9b8907e0fe55 100644
> > > > > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> > > > > /* A3700 PCIe regions fixer for device tree */
> > > > > int a3700_fdt_fix_pcie_regions(void *blob);
> > > > > -/*
> > > > > - * get_ref_clk
> > > > > - *
> > > > > - * return: reference clock in MHz (25 or 40)
> > > > > - */
> > > > > -u32 get_ref_clk(void);
> > > > > -
> > > > > #endif /* __ASSEMBLY__ */
> > > > > #endif /* _MVEBU_CPU_H */
> > > > > diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > index aab61f7c15cf..b03b6de3c6cd 100644
> > > > > --- a/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > @@ -210,6 +210,12 @@
> > > > > #define BOOT_FROM_SPI 0x3
> > > > > #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> > > > > +#elif defined(CONFIG_ARMADA_3700)
> > > > > +/* SAR values for Armada 3700 */
> > > > > +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> > > > > +#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> > > > > + 40000000 : 25000000)
> > >
> > > NAK. CONFIG_xxx which evaluate out to a macro / function are the
> > > hardest to convert to Kconfig. This patch is taking a step backwards.
> > > In fact, wait, how does patch apply and work? There are no
> > > CONFIG_SYS_REF_CLK instances today, so the build should blow up about
> > > adding a new non-Kconfig symbol.
> >
> > So, could you please provide some other solution for this issue which
> > Marek and Stefan pointed?
>
> I don't know what the issue is, sorry. But you cannot do what you're
> doing there with CONFIG. If for some reason you cannot use an inline
> function, just don't name it CONFIG_SYS_REF_CLK.
Inline function is possible.
> But also, really, did
> your build not fail when you tried to do this? It really should have
> failed and told you to not add new CONFIG symbols.
There were no build issues, and built binary worked fine without any
issue.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-09-01 12:40 ` Pali Rohár
@ 2021-09-01 12:41 ` Tom Rini
2021-09-01 12:46 ` Pali Rohár
2021-09-01 12:56 ` Tom Rini
0 siblings, 2 replies; 17+ messages in thread
From: Tom Rini @ 2021-09-01 12:41 UTC (permalink / raw)
To: Pali Rohár; +Cc: Stefan Roese, Marek Behún, u-boot
[-- Attachment #1: Type: text/plain, Size: 6448 bytes --]
On Wed, Sep 01, 2021 at 02:40:21PM +0200, Pali Rohár wrote:
> On Wednesday 01 September 2021 08:35:33 Tom Rini wrote:
> > On Wed, Sep 01, 2021 at 02:32:43PM +0200, Pali Rohár wrote:
> > > On Wednesday 01 September 2021 08:14:10 Tom Rini wrote:
> > > > On Wed, Sep 01, 2021 at 11:12:58AM +0200, Stefan Roese wrote:
> > > >
> > > > > Hi Pali,
> > > > >
> > > > > On 16.08.21 12:02, Pali Rohár wrote:
> > > > > > Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> > > > > > CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> > > > > > read from latched reset register.
> > > > > >
> > > > > > Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> > > > > > macro and completely remove get_ref_clk() function.
> > > > > >
> > > > > > Replace also custom open-coded implementation of determining reference
> > > > > > clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
> > > > > >
> > > > > > The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> > > > > > clock in Hz and old function get_ref_clk() returned it in MHz.
> > > > > >
> > > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > >
> > > > > > ---
> > > > > > Changes in v2:
> > > > > > * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
> > > > >
> > > > > This patch does not apply any more, with all the other patches applied.
> > > > > Please wait a bit until these patches are included in master and then
> > > > > send a new version.
> > > > >
> > > > > Sorry for the trouble.
> > > > >
> > > > > Thanks,
> > > > > Stefan
> > > > >
> > > > > > ---
> > > > > > arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> > > > > > arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> > > > > > arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> > > > > > drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> > > > > > drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> > > > > > drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> > > > > > drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> > > > > > drivers/watchdog/armada-37xx-wdt.c | 2 +-
> > > > > > 8 files changed, 22 insertions(+), 50 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > index 7702028ba19b..bdf8dc377528 100644
> > > > > > --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > @@ -23,12 +23,6 @@
> > > > > > /* Armada 3700 */
> > > > > > #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
> > > > > > -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> > > > > > -#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > > -#define MVEBU_XTAL_MODE_OFFS 9
> > > > > > -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> > > > > > -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> > > > > > -
> > > > > > #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> > > > > > #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
> > > > > > @@ -370,21 +364,3 @@ void reset_cpu(void)
> > > > > > */
> > > > > > writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> > > > > > }
> > > > > > -
> > > > > > -/*
> > > > > > - * get_ref_clk
> > > > > > - *
> > > > > > - * return: reference clock in MHz (25 or 40)
> > > > > > - */
> > > > > > -u32 get_ref_clk(void)
> > > > > > -{
> > > > > > - u32 regval;
> > > > > > -
> > > > > > - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> > > > > > - MVEBU_XTAL_MODE_OFFS;
> > > > > > -
> > > > > > - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> > > > > > - return 25;
> > > > > > - else
> > > > > > - return 40;
> > > > > > -}
> > > > > > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > index 79858858c259..9b8907e0fe55 100644
> > > > > > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> > > > > > /* A3700 PCIe regions fixer for device tree */
> > > > > > int a3700_fdt_fix_pcie_regions(void *blob);
> > > > > > -/*
> > > > > > - * get_ref_clk
> > > > > > - *
> > > > > > - * return: reference clock in MHz (25 or 40)
> > > > > > - */
> > > > > > -u32 get_ref_clk(void);
> > > > > > -
> > > > > > #endif /* __ASSEMBLY__ */
> > > > > > #endif /* _MVEBU_CPU_H */
> > > > > > diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > index aab61f7c15cf..b03b6de3c6cd 100644
> > > > > > --- a/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > @@ -210,6 +210,12 @@
> > > > > > #define BOOT_FROM_SPI 0x3
> > > > > > #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> > > > > > +#elif defined(CONFIG_ARMADA_3700)
> > > > > > +/* SAR values for Armada 3700 */
> > > > > > +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> > > > > > +#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > > +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> > > > > > + 40000000 : 25000000)
> > > >
> > > > NAK. CONFIG_xxx which evaluate out to a macro / function are the
> > > > hardest to convert to Kconfig. This patch is taking a step backwards.
> > > > In fact, wait, how does patch apply and work? There are no
> > > > CONFIG_SYS_REF_CLK instances today, so the build should blow up about
> > > > adding a new non-Kconfig symbol.
> > >
> > > So, could you please provide some other solution for this issue which
> > > Marek and Stefan pointed?
> >
> > I don't know what the issue is, sorry. But you cannot do what you're
> > doing there with CONFIG. If for some reason you cannot use an inline
> > function, just don't name it CONFIG_SYS_REF_CLK.
>
> Inline function is possible.
Then that please.
> > But also, really, did
> > your build not fail when you tried to do this? It really should have
> > failed and told you to not add new CONFIG symbols.
>
> There were no build issues, and built binary worked fine without any
> issue.
Ugh. I can reproduce that failure of the check here as well. Time to
go see what's going on there.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-09-01 12:41 ` Tom Rini
@ 2021-09-01 12:46 ` Pali Rohár
2021-09-01 12:58 ` Tom Rini
2021-09-01 12:56 ` Tom Rini
1 sibling, 1 reply; 17+ messages in thread
From: Pali Rohár @ 2021-09-01 12:46 UTC (permalink / raw)
To: Tom Rini; +Cc: Stefan Roese, Marek Behún, u-boot
On Wednesday 01 September 2021 08:41:10 Tom Rini wrote:
> On Wed, Sep 01, 2021 at 02:40:21PM +0200, Pali Rohár wrote:
> > On Wednesday 01 September 2021 08:35:33 Tom Rini wrote:
> > > On Wed, Sep 01, 2021 at 02:32:43PM +0200, Pali Rohár wrote:
> > > > On Wednesday 01 September 2021 08:14:10 Tom Rini wrote:
> > > > > On Wed, Sep 01, 2021 at 11:12:58AM +0200, Stefan Roese wrote:
> > > > >
> > > > > > Hi Pali,
> > > > > >
> > > > > > On 16.08.21 12:02, Pali Rohár wrote:
> > > > > > > Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> > > > > > > CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> > > > > > > read from latched reset register.
> > > > > > >
> > > > > > > Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> > > > > > > macro and completely remove get_ref_clk() function.
> > > > > > >
> > > > > > > Replace also custom open-coded implementation of determining reference
> > > > > > > clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
> > > > > > >
> > > > > > > The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> > > > > > > clock in Hz and old function get_ref_clk() returned it in MHz.
> > > > > > >
> > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > > >
> > > > > > > ---
> > > > > > > Changes in v2:
> > > > > > > * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
> > > > > >
> > > > > > This patch does not apply any more, with all the other patches applied.
> > > > > > Please wait a bit until these patches are included in master and then
> > > > > > send a new version.
> > > > > >
> > > > > > Sorry for the trouble.
> > > > > >
> > > > > > Thanks,
> > > > > > Stefan
> > > > > >
> > > > > > > ---
> > > > > > > arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> > > > > > > arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> > > > > > > arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> > > > > > > drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> > > > > > > drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> > > > > > > drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> > > > > > > drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> > > > > > > drivers/watchdog/armada-37xx-wdt.c | 2 +-
> > > > > > > 8 files changed, 22 insertions(+), 50 deletions(-)
> > > > > > >
> > > > > > > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > > index 7702028ba19b..bdf8dc377528 100644
> > > > > > > --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > > @@ -23,12 +23,6 @@
> > > > > > > /* Armada 3700 */
> > > > > > > #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
> > > > > > > -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> > > > > > > -#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > > > -#define MVEBU_XTAL_MODE_OFFS 9
> > > > > > > -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> > > > > > > -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> > > > > > > -
> > > > > > > #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> > > > > > > #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
> > > > > > > @@ -370,21 +364,3 @@ void reset_cpu(void)
> > > > > > > */
> > > > > > > writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> > > > > > > }
> > > > > > > -
> > > > > > > -/*
> > > > > > > - * get_ref_clk
> > > > > > > - *
> > > > > > > - * return: reference clock in MHz (25 or 40)
> > > > > > > - */
> > > > > > > -u32 get_ref_clk(void)
> > > > > > > -{
> > > > > > > - u32 regval;
> > > > > > > -
> > > > > > > - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> > > > > > > - MVEBU_XTAL_MODE_OFFS;
> > > > > > > -
> > > > > > > - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> > > > > > > - return 25;
> > > > > > > - else
> > > > > > > - return 40;
> > > > > > > -}
> > > > > > > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > > index 79858858c259..9b8907e0fe55 100644
> > > > > > > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > > @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> > > > > > > /* A3700 PCIe regions fixer for device tree */
> > > > > > > int a3700_fdt_fix_pcie_regions(void *blob);
> > > > > > > -/*
> > > > > > > - * get_ref_clk
> > > > > > > - *
> > > > > > > - * return: reference clock in MHz (25 or 40)
> > > > > > > - */
> > > > > > > -u32 get_ref_clk(void);
> > > > > > > -
> > > > > > > #endif /* __ASSEMBLY__ */
> > > > > > > #endif /* _MVEBU_CPU_H */
> > > > > > > diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > > index aab61f7c15cf..b03b6de3c6cd 100644
> > > > > > > --- a/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > > +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > > @@ -210,6 +210,12 @@
> > > > > > > #define BOOT_FROM_SPI 0x3
> > > > > > > #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> > > > > > > +#elif defined(CONFIG_ARMADA_3700)
> > > > > > > +/* SAR values for Armada 3700 */
> > > > > > > +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> > > > > > > +#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > > > +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> > > > > > > + 40000000 : 25000000)
> > > > >
> > > > > NAK. CONFIG_xxx which evaluate out to a macro / function are the
> > > > > hardest to convert to Kconfig. This patch is taking a step backwards.
> > > > > In fact, wait, how does patch apply and work? There are no
> > > > > CONFIG_SYS_REF_CLK instances today, so the build should blow up about
> > > > > adding a new non-Kconfig symbol.
> > > >
> > > > So, could you please provide some other solution for this issue which
> > > > Marek and Stefan pointed?
> > >
> > > I don't know what the issue is, sorry. But you cannot do what you're
> > > doing there with CONFIG. If for some reason you cannot use an inline
> > > function, just don't name it CONFIG_SYS_REF_CLK.
> >
> > Inline function is possible.
>
> Then that please.
Stefan, Marek and Tom, please then discuss how do you want to see this
issue solved.
I just do not want to develop third patch with another approach which
would be rejected by somebody else.
So for now, I'm letting code in the current state (because it is
working).
> > > But also, really, did
> > > your build not fail when you tried to do this? It really should have
> > > failed and told you to not add new CONFIG symbols.
> >
> > There were no build issues, and built binary worked fine without any
> > issue.
>
> Ugh. I can reproduce that failure of the check here as well. Time to
> go see what's going on there.
>
> --
> Tom
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-09-01 12:41 ` Tom Rini
2021-09-01 12:46 ` Pali Rohár
@ 2021-09-01 12:56 ` Tom Rini
1 sibling, 0 replies; 17+ messages in thread
From: Tom Rini @ 2021-09-01 12:56 UTC (permalink / raw)
To: Pali Rohár; +Cc: Stefan Roese, Marek Behún, u-boot
[-- Attachment #1: Type: text/plain, Size: 6977 bytes --]
On Wed, Sep 01, 2021 at 08:41:10AM -0400, Tom Rini wrote:
> On Wed, Sep 01, 2021 at 02:40:21PM +0200, Pali Rohár wrote:
> > On Wednesday 01 September 2021 08:35:33 Tom Rini wrote:
> > > On Wed, Sep 01, 2021 at 02:32:43PM +0200, Pali Rohár wrote:
> > > > On Wednesday 01 September 2021 08:14:10 Tom Rini wrote:
> > > > > On Wed, Sep 01, 2021 at 11:12:58AM +0200, Stefan Roese wrote:
> > > > >
> > > > > > Hi Pali,
> > > > > >
> > > > > > On 16.08.21 12:02, Pali Rohár wrote:
> > > > > > > Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> > > > > > > CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> > > > > > > read from latched reset register.
> > > > > > >
> > > > > > > Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> > > > > > > macro and completely remove get_ref_clk() function.
> > > > > > >
> > > > > > > Replace also custom open-coded implementation of determining reference
> > > > > > > clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
> > > > > > >
> > > > > > > The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> > > > > > > clock in Hz and old function get_ref_clk() returned it in MHz.
> > > > > > >
> > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > > >
> > > > > > > ---
> > > > > > > Changes in v2:
> > > > > > > * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
> > > > > >
> > > > > > This patch does not apply any more, with all the other patches applied.
> > > > > > Please wait a bit until these patches are included in master and then
> > > > > > send a new version.
> > > > > >
> > > > > > Sorry for the trouble.
> > > > > >
> > > > > > Thanks,
> > > > > > Stefan
> > > > > >
> > > > > > > ---
> > > > > > > arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> > > > > > > arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> > > > > > > arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> > > > > > > drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> > > > > > > drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> > > > > > > drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> > > > > > > drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> > > > > > > drivers/watchdog/armada-37xx-wdt.c | 2 +-
> > > > > > > 8 files changed, 22 insertions(+), 50 deletions(-)
> > > > > > >
> > > > > > > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > > index 7702028ba19b..bdf8dc377528 100644
> > > > > > > --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > > @@ -23,12 +23,6 @@
> > > > > > > /* Armada 3700 */
> > > > > > > #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
> > > > > > > -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> > > > > > > -#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > > > -#define MVEBU_XTAL_MODE_OFFS 9
> > > > > > > -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> > > > > > > -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> > > > > > > -
> > > > > > > #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> > > > > > > #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
> > > > > > > @@ -370,21 +364,3 @@ void reset_cpu(void)
> > > > > > > */
> > > > > > > writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> > > > > > > }
> > > > > > > -
> > > > > > > -/*
> > > > > > > - * get_ref_clk
> > > > > > > - *
> > > > > > > - * return: reference clock in MHz (25 or 40)
> > > > > > > - */
> > > > > > > -u32 get_ref_clk(void)
> > > > > > > -{
> > > > > > > - u32 regval;
> > > > > > > -
> > > > > > > - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> > > > > > > - MVEBU_XTAL_MODE_OFFS;
> > > > > > > -
> > > > > > > - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> > > > > > > - return 25;
> > > > > > > - else
> > > > > > > - return 40;
> > > > > > > -}
> > > > > > > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > > index 79858858c259..9b8907e0fe55 100644
> > > > > > > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > > @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> > > > > > > /* A3700 PCIe regions fixer for device tree */
> > > > > > > int a3700_fdt_fix_pcie_regions(void *blob);
> > > > > > > -/*
> > > > > > > - * get_ref_clk
> > > > > > > - *
> > > > > > > - * return: reference clock in MHz (25 or 40)
> > > > > > > - */
> > > > > > > -u32 get_ref_clk(void);
> > > > > > > -
> > > > > > > #endif /* __ASSEMBLY__ */
> > > > > > > #endif /* _MVEBU_CPU_H */
> > > > > > > diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > > index aab61f7c15cf..b03b6de3c6cd 100644
> > > > > > > --- a/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > > +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > > @@ -210,6 +210,12 @@
> > > > > > > #define BOOT_FROM_SPI 0x3
> > > > > > > #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> > > > > > > +#elif defined(CONFIG_ARMADA_3700)
> > > > > > > +/* SAR values for Armada 3700 */
> > > > > > > +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> > > > > > > +#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > > > +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> > > > > > > + 40000000 : 25000000)
> > > > >
> > > > > NAK. CONFIG_xxx which evaluate out to a macro / function are the
> > > > > hardest to convert to Kconfig. This patch is taking a step backwards.
> > > > > In fact, wait, how does patch apply and work? There are no
> > > > > CONFIG_SYS_REF_CLK instances today, so the build should blow up about
> > > > > adding a new non-Kconfig symbol.
> > > >
> > > > So, could you please provide some other solution for this issue which
> > > > Marek and Stefan pointed?
> > >
> > > I don't know what the issue is, sorry. But you cannot do what you're
> > > doing there with CONFIG. If for some reason you cannot use an inline
> > > function, just don't name it CONFIG_SYS_REF_CLK.
> >
> > Inline function is possible.
>
> Then that please.
>
> > > But also, really, did
> > > your build not fail when you tried to do this? It really should have
> > > failed and told you to not add new CONFIG symbols.
> >
> > There were no build issues, and built binary worked fine without any
> > issue.
>
> Ugh. I can reproduce that failure of the check here as well. Time to
> go see what's going on there.
Ah, common.h including a whole lot less means that some uncommon
headers, such as <mach/soc.h> might slip through the checks. Sigh, one
more reason everything needs to get migrated.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
2021-09-01 12:46 ` Pali Rohár
@ 2021-09-01 12:58 ` Tom Rini
0 siblings, 0 replies; 17+ messages in thread
From: Tom Rini @ 2021-09-01 12:58 UTC (permalink / raw)
To: Pali Rohár; +Cc: Stefan Roese, Marek Behún, u-boot
[-- Attachment #1: Type: text/plain, Size: 7134 bytes --]
On Wed, Sep 01, 2021 at 02:46:09PM +0200, Pali Rohár wrote:
> On Wednesday 01 September 2021 08:41:10 Tom Rini wrote:
> > On Wed, Sep 01, 2021 at 02:40:21PM +0200, Pali Rohár wrote:
> > > On Wednesday 01 September 2021 08:35:33 Tom Rini wrote:
> > > > On Wed, Sep 01, 2021 at 02:32:43PM +0200, Pali Rohár wrote:
> > > > > On Wednesday 01 September 2021 08:14:10 Tom Rini wrote:
> > > > > > On Wed, Sep 01, 2021 at 11:12:58AM +0200, Stefan Roese wrote:
> > > > > >
> > > > > > > Hi Pali,
> > > > > > >
> > > > > > > On 16.08.21 12:02, Pali Rohár wrote:
> > > > > > > > Like for all other mvebu platforms with CONFIG_SYS_TCLK macro, define
> > > > > > > > CONFIG_SYS_REF_CLK macro for a37xx with base reference clock value which is
> > > > > > > > read from latched reset register.
> > > > > > > >
> > > > > > > > Replace all usages of get_ref_clk() function by this CONFIG_SYS_REF_CLK
> > > > > > > > macro and completely remove get_ref_clk() function.
> > > > > > > >
> > > > > > > > Replace also custom open-coded implementation of determining reference
> > > > > > > > clock by CONFIG_SYS_REF_CLK in a37xx serial driver.
> > > > > > > >
> > > > > > > > The only difference is that macro CONFIG_SYS_REF_CLK returns base reference
> > > > > > > > clock in Hz and old function get_ref_clk() returned it in MHz.
> > > > > > > >
> > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > > > >
> > > > > > > > ---
> > > > > > > > Changes in v2:
> > > > > > > > * Do not remove MVEBU_TEST_PIN_LATCH_N and MVEBU_XTAL_MODE_MASK macros
> > > > > > >
> > > > > > > This patch does not apply any more, with all the other patches applied.
> > > > > > > Please wait a bit until these patches are included in master and then
> > > > > > > send a new version.
> > > > > > >
> > > > > > > Sorry for the trouble.
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Stefan
> > > > > > >
> > > > > > > > ---
> > > > > > > > arch/arm/mach-mvebu/armada3700/cpu.c | 24 ------------------------
> > > > > > > > arch/arm/mach-mvebu/include/mach/cpu.h | 7 -------
> > > > > > > > arch/arm/mach-mvebu/include/mach/soc.h | 6 ++++++
> > > > > > > > drivers/clk/mvebu/armada-37xx-periph.c | 6 +++---
> > > > > > > > drivers/clk/mvebu/armada-37xx-tbg.c | 4 ++--
> > > > > > > > drivers/phy/marvell/comphy_a3700.c | 12 ++++++------
> > > > > > > > drivers/serial/serial_mvebu_a3700.c | 11 ++++-------
> > > > > > > > drivers/watchdog/armada-37xx-wdt.c | 2 +-
> > > > > > > > 8 files changed, 22 insertions(+), 50 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/arch/arm/mach-mvebu/armada3700/cpu.c b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > > > index 7702028ba19b..bdf8dc377528 100644
> > > > > > > > --- a/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > > > +++ b/arch/arm/mach-mvebu/armada3700/cpu.c
> > > > > > > > @@ -23,12 +23,6 @@
> > > > > > > > /* Armada 3700 */
> > > > > > > > #define MVEBU_GPIO_NB_REG_BASE (MVEBU_REGISTER(0x13800))
> > > > > > > > -#define MVEBU_TEST_PIN_LATCH_N (MVEBU_GPIO_NB_REG_BASE + 0x8)
> > > > > > > > -#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > > > > -#define MVEBU_XTAL_MODE_OFFS 9
> > > > > > > > -#define MVEBU_XTAL_CLOCK_25MHZ 0x0
> > > > > > > > -#define MVEBU_XTAL_CLOCK_40MHZ 0x1
> > > > > > > > -
> > > > > > > > #define MVEBU_NB_WARM_RST_REG (MVEBU_GPIO_NB_REG_BASE + 0x40)
> > > > > > > > #define MVEBU_NB_WARM_RST_MAGIC_NUM 0x1d1e
> > > > > > > > @@ -370,21 +364,3 @@ void reset_cpu(void)
> > > > > > > > */
> > > > > > > > writel(MVEBU_NB_WARM_RST_MAGIC_NUM, MVEBU_NB_WARM_RST_REG);
> > > > > > > > }
> > > > > > > > -
> > > > > > > > -/*
> > > > > > > > - * get_ref_clk
> > > > > > > > - *
> > > > > > > > - * return: reference clock in MHz (25 or 40)
> > > > > > > > - */
> > > > > > > > -u32 get_ref_clk(void)
> > > > > > > > -{
> > > > > > > > - u32 regval;
> > > > > > > > -
> > > > > > > > - regval = (readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) >>
> > > > > > > > - MVEBU_XTAL_MODE_OFFS;
> > > > > > > > -
> > > > > > > > - if (regval == MVEBU_XTAL_CLOCK_25MHZ)
> > > > > > > > - return 25;
> > > > > > > > - else
> > > > > > > > - return 40;
> > > > > > > > -}
> > > > > > > > diff --git a/arch/arm/mach-mvebu/include/mach/cpu.h b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > > > index 79858858c259..9b8907e0fe55 100644
> > > > > > > > --- a/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > > > +++ b/arch/arm/mach-mvebu/include/mach/cpu.h
> > > > > > > > @@ -183,12 +183,5 @@ int a3700_dram_init_banksize(void);
> > > > > > > > /* A3700 PCIe regions fixer for device tree */
> > > > > > > > int a3700_fdt_fix_pcie_regions(void *blob);
> > > > > > > > -/*
> > > > > > > > - * get_ref_clk
> > > > > > > > - *
> > > > > > > > - * return: reference clock in MHz (25 or 40)
> > > > > > > > - */
> > > > > > > > -u32 get_ref_clk(void);
> > > > > > > > -
> > > > > > > > #endif /* __ASSEMBLY__ */
> > > > > > > > #endif /* _MVEBU_CPU_H */
> > > > > > > > diff --git a/arch/arm/mach-mvebu/include/mach/soc.h b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > > > index aab61f7c15cf..b03b6de3c6cd 100644
> > > > > > > > --- a/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > > > +++ b/arch/arm/mach-mvebu/include/mach/soc.h
> > > > > > > > @@ -210,6 +210,12 @@
> > > > > > > > #define BOOT_FROM_SPI 0x3
> > > > > > > > #define CONFIG_SYS_TCLK 250000000 /* 250MHz */
> > > > > > > > +#elif defined(CONFIG_ARMADA_3700)
> > > > > > > > +/* SAR values for Armada 3700 */
> > > > > > > > +#define MVEBU_TEST_PIN_LATCH_N MVEBU_REGISTER(0x13808)
> > > > > > > > +#define MVEBU_XTAL_MODE_MASK BIT(9)
> > > > > > > > +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_TEST_PIN_LATCH_N) & MVEBU_XTAL_MODE_MASK) ? \
> > > > > > > > + 40000000 : 25000000)
> > > > > >
> > > > > > NAK. CONFIG_xxx which evaluate out to a macro / function are the
> > > > > > hardest to convert to Kconfig. This patch is taking a step backwards.
> > > > > > In fact, wait, how does patch apply and work? There are no
> > > > > > CONFIG_SYS_REF_CLK instances today, so the build should blow up about
> > > > > > adding a new non-Kconfig symbol.
> > > > >
> > > > > So, could you please provide some other solution for this issue which
> > > > > Marek and Stefan pointed?
> > > >
> > > > I don't know what the issue is, sorry. But you cannot do what you're
> > > > doing there with CONFIG. If for some reason you cannot use an inline
> > > > function, just don't name it CONFIG_SYS_REF_CLK.
> > >
> > > Inline function is possible.
> >
> > Then that please.
>
> Stefan, Marek and Tom, please then discuss how do you want to see this
> issue solved.
>
> I just do not want to develop third patch with another approach which
> would be rejected by somebody else.
>
> So for now, I'm letting code in the current state (because it is
> working).
And whatever it is should also fix CONFIG_SYS_TCLK to not be CONFIG_xxx
at minimum. Thanks.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-09-01 12:59 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-11 18:53 [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h Pali Rohár
2021-08-11 18:53 ` [PATCH 2/2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk() Pali Rohár
2021-08-16 10:02 ` [PATCH v2] " Pali Rohár
2021-08-17 7:47 ` Stefan Roese
2021-08-17 12:50 ` Marek Behún
2021-08-17 12:56 ` Stefan Roese
2021-09-01 9:12 ` Stefan Roese
2021-09-01 12:14 ` Tom Rini
2021-09-01 12:32 ` Pali Rohár
2021-09-01 12:35 ` Tom Rini
2021-09-01 12:40 ` Pali Rohár
2021-09-01 12:41 ` Tom Rini
2021-09-01 12:46 ` Pali Rohár
2021-09-01 12:58 ` Tom Rini
2021-09-01 12:56 ` Tom Rini
2021-08-16 7:58 ` [PATCH 1/2] arm: mvebu: axp: Properly check for Armada XP in mach/soc.h Stefan Roese
2021-09-01 9:09 ` Stefan Roese
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.