Re: [PATCH v2] arm: mvebu: a37xx: Define CONFIG_SYS_REF_CLK and use it instead of get_ref_clk()

Stefan Roese
2021-08-17T12:56:46Z

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