Re: [PATCH 2/2] serial: a37xx: Do not call get_ref_clk() in _debug_uart_init()

Pali Rohár
2021-08-16T08:33:40Z

On Monday 16 August 2021 10:04:39 Stefan Roese wrote:
> Hi Pali,
>
> On 11.08.21 20:57, Pali Rohár wrote:
> > On Monday 26 July 2021 17:24:26 Marek Behun wrote:
> > > On Mon, 26 Jul 2021 16:58:04 +0200
> > > Pali Rohár <pali@kernel.org> wrote:
> > >
> > > > On Monday 26 July 2021 16:55:22 Marek Behun wrote:
> > > > > On Mon, 26 Jul 2021 14:58:59 +0200
> > > > > Pali Rohár <pali@kernel.org> wrote:
> > > > > > Static inline function _debug_uart_init() should avoid calling external
> > > > > > (non-inline) functions.
> > > > >
> > > > > Why?
> > > >
> > > > Function is called in stage when stack is not fully initialized and
> > > > documentation suggest to avoid stack usage and other functions.
> > >
> > > OK, but maybe we should use the macro names for register constants.
> > >
> > > Could you move (in a separate patch) the corresponding macros from
> > > arch/arm/mach-mvebu/armada3700/cpu.c to
> > > arch/arm/mach-mvebu/include/mach/soc.h and then in this patch
> > > include <asm/arch/soc.h> in the serial driver and use the constant
> > > names?
> >
> > Implemented in separate patch as Stefan wanted:
> > http://patchwork.ozlabs.org/project/uboot/patch/20210811185330.15414-2-pali@kernel.org/
>
> Did I really want this? You're removing the macro names with this patch,
> and I would prefer using the defines/names instead of register numbers
> like here:
>
> +#elif defined(CONFIG_ARMADA_3700)
> +/* SAR values for Armada 3700 */
> +#define CONFIG_SYS_REF_CLK ((readl(MVEBU_REGISTER(0x13808)) & BIT(9)) ? \
> + 40000000 : 25000000)
> #endif
>
> Or did I miss something?
>
> Thanks,
> Stefan

It is global include header file, so I though it would be better to not
export lot of other macros which are relevant only for CONFIG_SYS_REF_CLK

But if you want to see names, I can change it e.g. to:

+#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)