[PATCH 6/6] rockchip: rk3328: Add support for ROC-RK3328-CC board
Chen-Yu Tsai
wens at kernel.org
Tue Mar 31 13:46:06 CEST 2020
On Tue, Mar 31, 2020 at 6:57 PM Jagan Teki <jagan at amarulasolutions.com> wrote:
>
> On Mon, Mar 30, 2020 at 11:54 PM Chen-Yu Tsai <wens at kernel.org> wrote:
> >
> > On Tue, Mar 31, 2020 at 1:44 AM Jagan Teki <jagan at amarulasolutions.com> wrote:
> > >
> > > Hi Chen-Yu,
> > >
> > > On Fri, Mar 27, 2020 at 10:12 AM Chen-Yu Tsai <wens at kernel.org> wrote:
> > > >
> > > > From: Chen-Yu Tsai <wens at csie.org>
> > > >
> > > > The ROC-RK3328-CC from Firefly and Libre Computer Project is a credit
> > > > card size development board based on the Rockchip RK3328 SoC, with:
> > > >
> > > > - 1/2/4 GB DDR4 DRAM
> > > > - eMMC connector for optional module
> > > > - micro SD card slot
> > > > - 1 x USB 3.0 host port
> > > > - 2 x USB 2.0 host port
> > > > - 1 x USB 2.0 OTG port
> > > > - HDMI video output
> > > > - TRRS connector with audio and composite video output
> > > > - gigabit Ethernet
> > > > - consumer IR receiver
> > > > - debug UART pins
> > > >
> > > > The ROC-RK3328-CC has the enable pin of the SD card power switch tied
> > > > to GPIO_0_D6. This pin also has the function SDMMC0_PWREN, which is
> > > > muxed by default. SDMMC0_PWREN is an active high signal controlled by
> > > > the MMC controller, however the switch enable is active low, and
> > > > pulled low (enabled) by default to make things work on boot.
> > > >
> > > > As such, we need to mux away from SDMMC0_PWREN and use GPIO to enable
> > > > power to the card. The default GPIO state for the pin is pull-down and
> > > > input, which doesn't require extra configuration when paired with the
> > > > external pull-down and active low switch.
> > > >
> > > > Thus we make a custom target for this board and do the muxing in its
> > > > spl_board_init() function.
> > > >
> > > > The device tree file is synced from the Linux kernel next-20200324.
> > > >
> > > > Signed-off-by: Chen-Yu Tsai <wens at csie.org>
> > > > ---
> > >
> > > [snip]
> > >
> > > > diff --git a/board/firefly/roc-cc-rk3328/MAINTAINERS b/board/firefly/roc-cc-rk3328/MAINTAINERS
> > > > new file mode 100644
> > > > index 000000000000..f2318e71ac33
> > > > --- /dev/null
> > > > +++ b/board/firefly/roc-cc-rk3328/MAINTAINERS
> > > > @@ -0,0 +1,7 @@
> > > > +ROC-RK3328-CC
> > > > +M: Loic Devulder <ldevulder at suse.com>
> > > > +M: Chen-Yu Tsai <wens at csie.org>
> > > > +S: Maintained
> > > > +F: board/firefly/roc-cc-rk3328/
> > > > +F: configs/roc-rk3328-cc_defconfig
> > > > +F: arch/arm/dts/rk3328-roc-cc-u-boot.dtsi
> > > > diff --git a/board/firefly/roc-cc-rk3328/Makefile b/board/firefly/roc-cc-rk3328/Makefile
> > > > new file mode 100644
> > > > index 000000000000..1550b5f5f16e
> > > > --- /dev/null
> > > > +++ b/board/firefly/roc-cc-rk3328/Makefile
> > > > @@ -0,0 +1 @@
> > > > +obj-y := board.o
> > > > diff --git a/board/firefly/roc-cc-rk3328/board.c b/board/firefly/roc-cc-rk3328/board.c
> > > > new file mode 100644
> > > > index 000000000000..eca58c86b53e
> > > > --- /dev/null
> > > > +++ b/board/firefly/roc-cc-rk3328/board.c
> > > > @@ -0,0 +1,38 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * (C) Copyright 2020 Chen-Yu Tsai <wens at csie.org>
> > > > + */
> > > > +#include <asm/io.h>
> > > > +
> > > > +#include <asm/arch-rockchip/grf_rk3328.h>
> > > > +#include <asm/arch-rockchip/hardware.h>
> > > > +
> > > > +#if defined(CONFIG_SPL_BUILD)
> > > > +
> > > > +#define GRF_BASE 0xFF100000
> > > > +
> > > > +enum {
> > > > + GPIO_0_D6_GPIO = 0 << 12,
> > > > + GPIO_0_D6_MASK = 0x3 << 12
> > > > +};
> > > > +
> > > > +/*
> > > > + * The ROC-RK3328-CC has the enable pin of the SD card power switch tied
> > > > + * to GPIO_0_D6. This pin also has the function SDMMC0_PWREN, which is
> > > > + * muxed by default. SDMMC0_PWREN is an active high signal controlled by
> > > > + * the MMC controller, however the switch enable is active low, and
> > > > + * pulled low (enabled) by default to make things work on boot.
> > > > + *
> > > > + * As such, we need to mux away from SDMMC0_PWREN and use GPIO to enable
> > > > + * power to the card. The default GPIO state for the pin is pull-down and
> > > > + * input, which doesn't require extra configuration when paired with the
> > > > + * external pull-down and active low switch.
> > > > + */
> > > > +void spl_board_init(void)
> > > > +{
> > > > + struct rk3328_grf_regs * const grf = (void *)GRF_BASE;
> > > > +
> > > > + rk_clrsetreg(&grf->gpio0d_iomux, GPIO_0_D6_MASK, GPIO_0_D6_GPIO);
> > > > +}
> > >
> > > So, this seem toggle the bit 12, 13 of GRF_GPIO0D_IOMUX so-that it
> > > operate like a gpio(not like default sdmmc0_pwrenm1), isn't it
> > > required for the next stages like U-Boot proper and Linux? these has
> > > vcc_sd node where it operates a fixed regulator to active low the same
> > > pin.
> > >
> > > gpio = <&gpio0 RK_PD6 GPIO_ACTIVE_LOW>;
> >
> > So U-boot proper and Linux both have pinctrl and GPIO and all the stuff
> > to deal with this. SPL doesn't have GPIO enabled, and I believe all the
> > pinctrl properties are striped out. (BTW, not sure why we're enabling
> > pinctrl driver support in SPL if nothing is triggering them.) It seems
> > a bit heavy pulling all that stuff in just for one pin mux in SPL. The
> > same is also done for the UART pins.
>
> look like it was missed missed enable pinctrl in rk3328-u-boot.dtsi
> but usually it should have. I tried of managing all stuff in SPL to
> make GPIO-enabled but existing dts never pulled low to make working
> like what the above change does.
The defconfigs all have
CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names
interrupt-parent assigned-clocks assigned-clock-rates
assigned-clock-parents"
So by that default all pinctrl stuff is stripped from the device trees
for SPL, which means pins are not muxed properly despite having pinctrl
support built into SPL.
The question is do we want to add back all the pinctrl properties back
to SPL's dtb or do we want to take the programmed way out?
Doing that adds a couple hundred bytes to the SPL dtb.
Before:
-rw-r--r-- 1 wens wens 62942 Mar 31 19:30 spl/u-boot-spl-dtb.bin
-rw-r--r-- 1 wens wens 3166 Mar 31 19:30 spl/u-boot-spl.dtb
After:
-rw-r--r-- 1 wens wens 63094 Mar 31 19:27 spl/u-boot-spl-dtb.bin
-rw-r--r-- 1 wens wens 3318 Mar 31 19:27 spl/u-boot-spl.dtb
Then you'll need to enable GPIO, power/regulator (fixed, gpio),
I2C (because rk8xx needs it) in SPL, which adds 10+ KB.
-rw-r--r-- 1 wens wens 76662 Mar 31 19:33 spl/u-boot-spl-dtb.bin
-rw-r--r-- 1 wens wens 3318 Mar 31 19:33 spl/u-boot-spl.dtb
Note I haven't tested the last one, only built it.
And no, you got it wrong. It's not that it's not pulled down, it's
that the PWREN signal from the MMC signal is likely driving the pin
high, overriding the pull-down, but the regulator is active low.
So again, the question is do we want to add 15 KB just to deal
with a signal pinmux. My personal preference is to not do that.
Regards
ChenYu
> Here is a sample log: (dts has emmc disabled and vqmmc-supply dropped)
>
> U-Boot TPL 2020.04-rc3-00198-g790eeaefc1-dirty (Mar 31 2020 - 16:19:18)
> DDR4, 333MHz
> BW=32 Col=10 Bk=4 BG=2 CS0 Row=16 CS1 Row=16 CS=2 Die BW=16 Size=4096MB
> Trying to boot from BOOTROM
> Returning to boot ROM...
>
> U-Boot SPL 2020.04-rc3-00198-g790eeaefc1-dirty (Mar 31 2020 - 16:19:18 +0530)
> Trying to boot from MMC1
> device_get_supply_regulator: supply vmmc-supply
> rockchip_gpio_direction_output: ff210000, mask 0x40000000, value 0x1
> rockchip_gpio_set_value: ff210000, mask 0x40000000, value 0x0
> rockchip_gpio_set_value: ff210000 value is 0x0
> device_get_supply_regulator: supply vqmmc-supply
> mmc at ff500000: No vqmmc supply
> rockchip_gpio_set_value: ff210000, mask 0x40000000, value 0x1
> rockchip_gpio_set_value: ff210000 value is 0x40000000
> rockchip_gpio_set_value: ff210000, mask 0x40000000, value 0x0
> rockchip_gpio_set_value: ff210000 value is 0x0
> rockchip_gpio_set_value: grf gpio0d value 0x3000
> Card did not respond to voltage select!
> spl: mmc init failed with error: -95
> SPL: failed to boot from all boot devices
> ### ERROR ### Please RESET the board ###
>
> Jagan.
More information about the U-Boot
mailing list