On Fri, Jul 3, 2020 at 6:09 PM Robin Murphy <robin.mur...@arm.com> wrote:
>
> On 2020-07-03 11:10, Jagan Teki wrote:
> > On Thu, Jul 2, 2020 at 7:26 PM Robin Murphy <robin.mur...@arm.com> wrote:
> >>
> >> On 2020-07-02 09:48, Jagan Teki wrote:
> >>> The new rk3288 revision rk3288w has some changes with respect
> >>> to legacy rk3288 like hclk_vio and usb host0 ohci.
> >>>
> >>> In order to work these on the same in Linux kernel update the
> >>> compatible the root compatible with rockchip,rk3288w before
> >>> booting.
> >>>
> >>> So, this support during of board setup code of rk3288.
> >>>
> >>> Signed-off-by: Jagan Teki <ja...@amarulasolutions.com>
> >>> ---
> >>> arch/arm/mach-rockchip/Kconfig | 1 +
> >>> arch/arm/mach-rockchip/rk3288/rk3288.c | 26 ++++++++++++++++++++++++++
> >>> 2 files changed, 27 insertions(+)
> >>>
> >>> diff --git a/arch/arm/mach-rockchip/Kconfig
> >>> b/arch/arm/mach-rockchip/Kconfig
> >>> index b1008a5058..822d8d4e9c 100644
> >>> --- a/arch/arm/mach-rockchip/Kconfig
> >>> +++ b/arch/arm/mach-rockchip/Kconfig
> >>> @@ -98,6 +98,7 @@ config ROCKCHIP_RK322X
> >>> config ROCKCHIP_RK3288
> >>> bool "Support Rockchip RK3288"
> >>> select CPU_V7A
> >>> + select OF_BOARD_SETUP
> >>> select SUPPORT_SPL
> >>> select SPL
> >>> select SUPPORT_TPL
> >>> diff --git a/arch/arm/mach-rockchip/rk3288/rk3288.c
> >>> b/arch/arm/mach-rockchip/rk3288/rk3288.c
> >>> index 804abe8a1b..8a682675e6 100644
> >>> --- a/arch/arm/mach-rockchip/rk3288/rk3288.c
> >>> +++ b/arch/arm/mach-rockchip/rk3288/rk3288.c
> >>> @@ -115,6 +115,32 @@ int rk_board_late_init(void)
> >>> return rk3288_board_late_init();
> >>> }
> >>>
> >>> +#ifdef CONFIG_OF_BOARD_SETUP
> >>> +
> >>> +#define RK3288_HDMI_PHYS 0xff980000
> >>> +#define RK3288W_HDMI_REV 0x1A
> >>> +#define HDMI_CONFIG0_ID 0x04
> >>> +
> >>> +int ft_board_setup(void *blob, bd_t *bd)
> >>> +{
> >>> + u8 config0;
> >>> + int ret;
> >>> +
> >>> + config0 = readb(RK3288_HDMI_PHYS + HDMI_CONFIG0_ID);
> >>> + if (config0 == RK3288W_HDMI_REV) {
> >>> + ret = fdt_setprop_string(blob, 0,
> >>> + "compatible", "rockchip,rk3288w");
> >>
> >> Does this end up replacing the entire top-level compatible property?
> >> i.e. from:
> >>
> >> compatible = "vendor,board\0rockchip,rk3288";
> >>
> >> to just:
> >>
> >> compatible = "rockchip,rk3288w";
> >>
> >> If so, that's a bit of a problem for various drivers that care about the
> >> actual board compatible rather than the SoC.
> >
> > Yes, It looks replacing the entire compatible. I think the root
> > compatible is mostly untouchable because of this reason.
> >
> > But, if we skip the root compatible and trying to replace individual
> > nodes for W revision then it requires extra registration code on in
> > the Linux drivers like Linux clock driver is registering the clock
> > with rockchip,rk3288-cru but updating this compatible with
> > rockchip,rk3288w-cru will require another registration code. Having
> > common rockchip,rk3288w can be possible to check any code in the tree.
>
> Right, it's definitely reasonable to update the top-level SoC
> compatible, we just need to be prepared to deal with a whole string list
> here. It looks like libfdt doesn't offer any nice helpers for
> inserting/removing/updating in string lists, but given that the SoC
> should be the last entry here we might be able to cheat a bit - just get
> the whole raw property, check that the final characters are exactly
> "rockchip,rk3288", and if so append a "w" to the end and write it back.
Look like exiting fdt helper does print the compatible up to \0
instead of whole root compatible, at least I have checked with
fdt_getpro and fdt_get_property.
