Re: [PATCH v3 12/12] rockchip: fix boot_devices constants

Simon Glass
2022-03-28T06:38:41Z

Hi Johan,

On Fri, 18 Mar 2022 at 09:17, Johan Jonker <jbx6244@gmail.com> wrote:
>
> Hi Simon,
>
> On 3/12/22 03:24, Simon Glass wrote:
> > Hi Johan,
> >
> > On Thu, 3 Mar 2022 at 16:53, Johan Jonker <jbx6244@gmail.com> wrote:
> >>
> >> The DT node name pattern in mmc-controller.yaml for mmc
> >> is "^mmc(@.*)?$". The Rockchip mmc nodes have been synced
> >> with Linux, so update the boot_devices constants as well.
> >>
> >> Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> >> Reviewed-by: Simon Glass <sjg@chromium.org>
> >> ---
> >> arch/arm/mach-rockchip/rk3188/rk3188.c | 4 ++--
> >> arch/arm/mach-rockchip/rk322x/rk322x.c | 4 ++--
> >> arch/arm/mach-rockchip/rk3288/rk3288.c | 4 ++--
> >> arch/arm/mach-rockchip/rk3328/rk3328.c | 4 ++--
> >> arch/arm/mach-rockchip/rk3368/rk3368.c | 4 ++--
> >> 5 files changed, 10 insertions(+), 10 deletions(-)
> >
> > This series seems to work OK on snow. But on jerry I get:
> >
> > U-Boot 2022.04-rc3-00077-g088bbcb3fad (Mar 11 2022 - 15:59:54 -0700)
> >
> > Model: Google Jerry
> > DRAM: 2 GiB
> > PMIC: RK808
> > Core: 256 devices, 25 uclasses, devicetree: separate
> > MMC: mmc@ff0c0000: 1, mmc@ff0d0000: 2, mmc@ff0f0000: 0
> > Loading Environment from nowhere... OK
>
> > cros_ec_spi_command: Returned status 6
> > cros_ec_check_version: ERROR: old EC interface not supported
> > Failed to probe keyboard 'keyboard-controller'
>
> When I compare the ec node in the dtb before and after it looks like
> "ec-interrupt" is missing.
>
> Could you test/confirm by adding it to rk3288-veyron-jerry-u-boot.dtsi?

Yes that looks like it.

>
> It originates from rk3288-veyron-chromebook.dtsi, but what should be the
> u-boot specific correct location now for the other rk3288 veyron boards?
>
> &cros_ec {
> ec-interrupt = <&gpio7 7 GPIO_ACTIVE_LOW>;
> };
>
> ===
>
> This property is not in the binding:
> Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
>
> What to do with it? (no u-boot prefix)

There is a standard way of doing interrupts, so could the code be
updated to use that? For now a u-boot, prefix would be OK, I suppose,
to unblock your work.

>
> ===
>
> dtc -s -I dtb -O dts rk3288-veyron-jerry-before.dtb >
> rk3288-veyron-jerry-before-sort.dts
>
> dtc -s -I dtb -O dts rk3288-veyron-jerry-after.dtb >
> rk3288-veyron-jerry-after-sort.dts
>
> ec@0 {
> compatible = "google,cros-ec-spi";
> ===
> ec-interrupt = < 0x07 0x07 0x01 >;
> ===
> google,cros-ec-spi-pre-delay = < 0x1e >;
> interrupt-parent = < 0x07 >;
> interrupts = < 0x07 0x08 >;
> pinctrl-0 = < 0x20 >;
> pinctrl-names = "default";
> reg = < 0x00 >;
> spi-max-frequency = < 0x2dc6c0 >;
>
> i2c-tunnel {
>
> };
>
> keyboard-controller {
>
> };
> };
>
>
> > rockchip_rk3288_vop vop@ff930000: failed to get ahb reset (ret=-524)
> > rockchip_rk3288_vop vop@ff930000: failed to get ahb reset (ret=-524)
>
> Could you test before and after Linux sync and with reg size?
>
>
> vopb: vop@ff930000 {
> compatible = "rockchip,rk3288-vop";
>
> reg = <0xff930000 0x19c>, <0xff931000 0x1000>;
>
> vs.
> reg = <0xff930000 0x19c>;
>
> }
>
> ===
>
> From rk_vop.c:
>
> priv->regs = (struct rk3288_vop *)dev_read_addr(dev);
>
> ===
>
> From rockchip-vop.yaml:
>
> reg:
> minItems: 1
> items:
> - description:
> Must contain one entry corresponding to the base address and
> length
> of the register space.
> - description:
> Can optionally contain a second entry corresponding to
> the CRTC gamma LUT address.
>
> ===
>
> Does vop driver need a fix to comply with binding?

Quite possibly.

>
>
> > In: serial
> >
> > So it seems the EC and video devicetree are broken in some way, or
> > drivers need updating for new bindings?
>
> Could the veyron board maintainer fix this?
> What to do with the things below to move forward with this serie?
>
> Before sync:
>
> panel: panel {
> compatible ="cnm,n116bgeea2","simple-panel";
> status = "okay";
> power-supply = <&vcc33_lcd>;
> backlight = <&backlight>;
> };
>
> &edp {
> status = "okay";
> rockchip,panel = <&panel>;
> };
>
> ===
>
> After sync:
>
> panel: panel {
> compatible = "innolux,n116bge";
> status = "okay";
> power-supply = <&panel_regulator>;
> backlight = <&backlight>;
>
> panel-timing {
> clock-frequency = <74250000>;
> hactive = <1366>;
> hfront-porch = <136>;
> hback-porch = <60>;
> hsync-len = <30>;
> hsync-active = <0>;
> vactive = <768>;
> vfront-porch = <8>;
> vback-porch = <12>;
> vsync-len = <12>;
> vsync-active = <0>;
> };
>
> ports {
> panel_in: port {
> panel_in_edp: endpoint {
> remote-endpoint = <&edp_out_panel>;
> };
> };
> };
> };
>
>
> &edp {
> status = "okay";
>
> pinctrl-names = "default";
> pinctrl-0 = <&edp_hpd>;
>
> ports {
> edp_out: port@1 {
> reg = <1>;
> #address-cells = <1>;
> #size-cells = <0>;
> edp_out_panel: endpoint@0 {
> reg = <0>;
> remote-endpoint = <&panel_in_edp>;
> };
> };
> };
> };
>
> &edp_phy {
> status = "okay";
> };
>
> ===
>
> The edp node has different compatible strings and phys reset-names??
> Please advise ??
>
>
>
> edp: edp@ff970000 {
>
> compatible = "rockchip,rk3288-edp";
>
> reg = <0xff970000 0x4000>;
> interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&cru SCLK_EDP>, <&cru SCLK_EDP_24M>, <&cru PCLK_EDP_CTRL>;
> rockchip,grf = <&grf>;
> clock-names = "clk_edp", "clk_edp_24m", "pclk_edp";
> resets = <&cru 111>;
>
> reset-names = "edp";
>
> power-domains = <&power RK3288_PD_VIO>;
> status = "disabled";
>
>
> edp: dp@ff970000 {
>
> compatible = "rockchip,rk3288-dp";
>
> reg = <0xff970000 0x4000>;
> interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
> clocks = <&cru SCLK_EDP>, <&cru PCLK_EDP_CTRL>;
> clock-names = "dp", "pclk";
>
> phys = <&edp_phy>;
> phy-names = "dp";
>
> resets = <&cru SRST_EDP>;
>
> reset-names = "dp";
>
> rockchip,grf = <&grf>;
> status = "disabled";
>
> >>> It also seemed fine on bob although my display is not coming up, for
> >>> different reasons.
> >>
> >> In SPL or full U-boot?
> >
> > There is no display in SPL.
> ===
>
> Before and after only vopb and vopl node available in jerry.
> No other hdmi mipi edp.
> There's no "u-boot,dm-pre-reloc" for them.
> What do you expect to work in this stage?

It should work OK with the internal display. If an HDMI display is
connected, that should work too, if selected.

Regards,
Simon