On Tue, Jul 25, 2023 at 04:37:55PM -0500, Nishanth Menon wrote:
On 17:25-20230725, Tom Rini wrote:
On Tue, Jul 25, 2023 at 01:52:50PM -0500, Nishanth Menon wrote:
[..]
- /* Set USB0 PHY core voltage to 0.85V */
- val = readl(CTRLMMR_USB0_PHY_CTRL);
- val &= ~(CORE_VOLTAGE);
- writel(val, CTRLMMR_USB0_PHY_CTRL);
- /* Set USB1 PHY core voltage to 0.85V */
- val = readl(CTRLMMR_USB1_PHY_CTRL);
- val &= ~(CORE_VOLTAGE);
- writel(val, CTRLMMR_USB1_PHY_CTRL);
- /* We have 32k crystal, so lets enable it */
- val = readl(MCU_CTRL_LFXOSC_CTRL);
- val &= ~(MCU_CTRL_LFXOSC_32K_DISABLE_VAL);
- writel(val, MCU_CTRL_LFXOSC_CTRL);
- /* Add any TRIM needed for the crystal here.. */
- /* Make sure to mux up to take the SoC 32k from the crystal */
- writel(MCU_CTRL_DEVICE_CLKOUT_LFOSC_SELECT_VAL,
MCU_CTRL_DEVICE_CLKOUT_32K_CTRL);- /* Setup debounce conf registers - arbitrary values. Times are approx */
- /* 1.9ms debounce @ 32k */
- writel(WKUP_CTRLMMR_DBOUNCE_CFG1, 0x1);
- /* 5ms debounce @ 32k */
- writel(WKUP_CTRLMMR_DBOUNCE_CFG2, 0x5);
- /* 20ms debounce @ 32k */
- writel(WKUP_CTRLMMR_DBOUNCE_CFG3, 0x14);
- /* 46ms debounce @ 32k */
- writel(WKUP_CTRLMMR_DBOUNCE_CFG4, 0x18);
- /* 100ms debounce @ 32k */
- writel(WKUP_CTRLMMR_DBOUNCE_CFG5, 0x1c);
- /* 156ms debounce @ 32k */
- writel(WKUP_CTRLMMR_DBOUNCE_CFG6, 0x1f);
- video_setup(); enable_caches(); if (IS_ENABLED(CONFIG_SPL_SPLASH_SCREEN) && IS_ENABLED(CONFIG_SPL_BMP))
Here's a whole lot of seemingly board specific code in a function and file that's supposed to support any am62 platform. Is this really what we need, where we need it?
- without using the correct voltage for USB, we risk damaging the IOs - board specific, sure.
So what happens when we do this on the other EVM, does it have the same values? Is there some must-always-be-safe values? Or is the answer "we must do this board specific to be safe" and so need to re-think what can and can't be shared between board builds.
- 32k - without using the external 32k, 32k rc-osc comes into play, which is accurate +-20% - board specific, sure
OK..
- Debounce configuration - I'd argue this is'nt board specific as it sets up different timing configurations that can be customized, but should be sufficient for the varied common usecases.
So this is, or this can be re-worked to be always correct (or, correct enough for the circumstances and limitations) values?
That said, i can drop this patch from the series, but I am curious how we'd handle this kind of stuff cleanly..
Well, we need to figure out how to handle this cleanly.