All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali@kernel.org>
To: Tom Rini <trini@konsulko.com>
Cc: "Stefan Roese" <sr@denx.de>, "Marek Behún" <marek.behun@nic.cz>,
	u-boot@lists.denx.de
Subject: Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()
Date: Wed, 1 Sep 2021 14:32:43 +0200	[thread overview]
Message-ID: <20210901123243.qs5ugus36qkbpd4e@pali> (raw)
In-Reply-To: <20210901121410.GZ858@bill-the-cat>

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?

  reply	other threads:[~2021-09-01 12:33 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210901123243.qs5ugus36qkbpd4e@pali \
    --to=pali@kernel.org \
    --cc=marek.behun@nic.cz \
    --cc=sr@denx.de \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.