| Message ID | 20211125173941.3038-2-alpernebiyasak@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | rockchip: Improve support for Bob chromebook and add support for Kevin | expand |
Hi Alper, On Thu, 25 Nov 2021 at 10:40, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote: > > The RK3399 SoC needs to know the voltage value provided by some > regulators, which is done by setting relevant register bits. Configure > these the way other RK3399 boards do, but with values set in coreboot. What do you mean by values set in coreboot? We don't need that to run here, do we? > > Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > --- > There is a driver for this on Rockchip's repo, I managed to forward-port > it and get it working. If that's more desirable than doing it per-board > like this I can send that upstream (but I'd prefer to do it after this). > > board/google/gru/gru.c | 54 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 54 insertions(+) > > diff --git a/board/google/gru/gru.c b/board/google/gru/gru.c > index 23080c1798b7..cddcb286a380 100644 > --- a/board/google/gru/gru.c > +++ b/board/google/gru/gru.c > @@ -6,6 +6,17 @@ > #include <common.h> > #include <dm.h> > #include <init.h> > +#include <syscon.h> > +#include <asm/io.h> > +#include <asm/arch-rockchip/clock.h> > +#include <asm/arch-rockchip/grf_rk3399.h> > +#include <asm/arch-rockchip/hardware.h> > +#include <asm/arch-rockchip/misc.h> > + > +#define GRF_IO_VSEL_BT656_SHIFT 0 > +#define GRF_IO_VSEL_AUDIO_SHIFT 1 > +#define PMUGRF_CON0_VSEL_SHIFT 8 > +#define PMUGRF_CON0_VOL_SHIFT 9 > > #ifdef CONFIG_SPL_BUILD > /* provided to defeat compiler optimisation in board_init_f() */ > @@ -54,3 +65,46 @@ int board_early_init_r(void) > return 0; > } > #endif > + > +#ifdef CONFIG_MISC_INIT_R Can you just drop this #ifdef? It doesn't seem necessasry. > +static void setup_iodomain(void) > +{ > + struct rk3399_grf_regs *grf = > + syscon_get_first_range(ROCKCHIP_SYSCON_GRF); > + struct rk3399_pmugrf_regs *pmugrf = > + syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF); > + > + /* BT656 and audio is in 1.8v domain */ > + rk_setreg(&grf->io_vsel, (1 << GRF_IO_VSEL_BT656_SHIFT | > + 1 << GRF_IO_VSEL_AUDIO_SHIFT)); > + > + /* > + * Set GPIO1 1.8v/3.0v source select to PMU1830_VOL > + * and explicitly configure that PMU1830_VOL to be 1.8V > + */ > + rk_setreg(&pmugrf->soc_con0, (1 << PMUGRF_CON0_VSEL_SHIFT | > + 1 << PMUGRF_CON0_VOL_SHIFT)); > +} > + > +int misc_init_r(void) > +{ > + const u32 cpuid_offset = 0x7; > + const u32 cpuid_length = 0x10; > + u8 cpuid[cpuid_length]; > + int ret; > + > + setup_iodomain(); > + > + ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid); > + if (ret) > + return ret; > + > + ret = rockchip_cpuid_set(cpuid, cpuid_length); > + if (ret) > + return ret; > + > + ret = rockchip_setup_macaddr(); > + > + return ret; > +} > +#endif > -- > 2.34.0 > Regards, Simon
On 03/12/2021 06:31, Simon Glass wrote: > On Thu, 25 Nov 2021 at 10:40, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote: >> The RK3399 SoC needs to know the voltage value provided by some >> regulators, which is done by setting relevant register bits. Configure >> these the way other RK3399 boards do, but with values set in coreboot. > > What do you mean by values set in coreboot? We don't need that to run > here, do we? I meant that I wasn't blindly copying from other boards which have the same block (e.g. Pinebook Pro), but was using known-good values for Gru boards that coreboot also uses [1]. I tested again and it looks like my Kevin works just as good without this patch, so I'll drop it. [1] https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src/mainboard/google/gru/bootblock.c#19 >> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> >> --- >> There is a driver for this on Rockchip's repo, I managed to forward-port >> it and get it working. If that's more desirable than doing it per-board >> like this I can send that upstream (but I'd prefer to do it after this). >> >> board/google/gru/gru.c | 54 ++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 54 insertions(+)
Hi Alper, On Tue, 7 Dec 2021 at 13:31, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote: > > On 03/12/2021 06:31, Simon Glass wrote: > > On Thu, 25 Nov 2021 at 10:40, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote: > >> The RK3399 SoC needs to know the voltage value provided by some > >> regulators, which is done by setting relevant register bits. Configure > >> these the way other RK3399 boards do, but with values set in coreboot. > > > > What do you mean by values set in coreboot? We don't need that to run > > here, do we? > > I meant that I wasn't blindly copying from other boards which have the > same block (e.g. Pinebook Pro), but was using known-good values for Gru > boards that coreboot also uses [1]. > > I tested again and it looks like my Kevin works just as good without > this patch, so I'll drop it. Well I have no objection to the patch. I'd suggest saying 'but with the same values as asre set in the equivalent code in coreboot'. Regards, Simon > [1] > https://review.coreboot.org/plugins/gitiles/coreboot/+/refs/heads/master/src/mainboard/google/gru/bootblock.c#19 > > >> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> > >> --- > >> There is a driver for this on Rockchip's repo, I managed to forward-port > >> it and get it working. If that's more desirable than doing it per-board > >> like this I can send that upstream (but I'd prefer to do it after this). > >> > >> board/google/gru/gru.c | 54 ++++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 54 insertions(+)
On 09/12/2021 05:32, Simon Glass wrote: > On Tue, 7 Dec 2021 at 13:31, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote: >> On 03/12/2021 06:31, Simon Glass wrote: >>> On Thu, 25 Nov 2021 at 10:40, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote: >>>> The RK3399 SoC needs to know the voltage value provided by some >>>> regulators, which is done by setting relevant register bits. Configure >>>> these the way other RK3399 boards do, but with values set in coreboot. >>> >>> What do you mean by values set in coreboot? We don't need that to run >>> here, do we? >> >> I meant that I wasn't blindly copying from other boards which have the >> same block (e.g. Pinebook Pro), but was using known-good values for Gru >> boards that coreboot also uses [1]. >> >> I tested again and it looks like my Kevin works just as good without >> this patch, so I'll drop it. > > Well I have no objection to the patch. I'd suggest saying 'but with > the same values as asre set in the equivalent code in coreboot'. Ah, OK. I'll keep it with that change, then. (I'll also remove the unnecessary ifdef.)
diff --git a/board/google/gru/gru.c b/board/google/gru/gru.c index 23080c1798b7..cddcb286a380 100644 --- a/board/google/gru/gru.c +++ b/board/google/gru/gru.c @@ -6,6 +6,17 @@ #include <common.h> #include <dm.h> #include <init.h> +#include <syscon.h> +#include <asm/io.h> +#include <asm/arch-rockchip/clock.h> +#include <asm/arch-rockchip/grf_rk3399.h> +#include <asm/arch-rockchip/hardware.h> +#include <asm/arch-rockchip/misc.h> + +#define GRF_IO_VSEL_BT656_SHIFT 0 +#define GRF_IO_VSEL_AUDIO_SHIFT 1 +#define PMUGRF_CON0_VSEL_SHIFT 8 +#define PMUGRF_CON0_VOL_SHIFT 9 #ifdef CONFIG_SPL_BUILD /* provided to defeat compiler optimisation in board_init_f() */ @@ -54,3 +65,46 @@ int board_early_init_r(void) return 0; } #endif + +#ifdef CONFIG_MISC_INIT_R +static void setup_iodomain(void) +{ + struct rk3399_grf_regs *grf = + syscon_get_first_range(ROCKCHIP_SYSCON_GRF); + struct rk3399_pmugrf_regs *pmugrf = + syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF); + + /* BT656 and audio is in 1.8v domain */ + rk_setreg(&grf->io_vsel, (1 << GRF_IO_VSEL_BT656_SHIFT | + 1 << GRF_IO_VSEL_AUDIO_SHIFT)); + + /* + * Set GPIO1 1.8v/3.0v source select to PMU1830_VOL + * and explicitly configure that PMU1830_VOL to be 1.8V + */ + rk_setreg(&pmugrf->soc_con0, (1 << PMUGRF_CON0_VSEL_SHIFT | + 1 << PMUGRF_CON0_VOL_SHIFT)); +} + +int misc_init_r(void) +{ + const u32 cpuid_offset = 0x7; + const u32 cpuid_length = 0x10; + u8 cpuid[cpuid_length]; + int ret; + + setup_iodomain(); + + ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid); + if (ret) + return ret; + + ret = rockchip_cpuid_set(cpuid, cpuid_length); + if (ret) + return ret; + + ret = rockchip_setup_macaddr(); + + return ret; +} +#endif
The RK3399 SoC needs to know the voltage value provided by some regulators, which is done by setting relevant register bits. Configure these the way other RK3399 boards do, but with values set in coreboot. Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> --- There is a driver for this on Rockchip's repo, I managed to forward-port it and get it working. If that's more desirable than doing it per-board like this I can send that upstream (but I'd prefer to do it after this). board/google/gru/gru.c | 54 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)