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 <p...@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
signature.asc
Description: PGP signature