rockchip: rk3288: Add OF board setup

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 <jagan@amarulasolutions.com>
---
 arch/arm/mach-rockchip/Kconfig         |  1 +
 arch/arm/mach-rockchip/rk3288/rk3288.c | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)
  

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 <jagan@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.

Robin.

> +		if (ret < 0) {
> +			printf("failed to set rk3288w compatible (ret=%d)\n",
> +			       ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
>   static int do_clock(struct cmd_tbl *cmdtp, int flag, int argc,
>   		    char *const argv[])
>   {
>
  

On Thu, Jul 2, 2020 at 7:26 PM Robin Murphy <robin.murphy@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 <jagan@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.

Jagan.
  

On 2020-07-03 11:10, Jagan Teki wrote:
> On Thu, Jul 2, 2020 at 7:26 PM Robin Murphy <robin.murphy@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 <jagan@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.

Robin.
  

On Fri, Jul 3, 2020 at 6:09 PM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2020-07-03 11:10, Jagan Teki wrote:
> > On Thu, Jul 2, 2020 at 7:26 PM Robin Murphy <robin.murphy@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 <jagan@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.
  

On Fri, Jul 3, 2020 at 7:34 PM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Fri, Jul 3, 2020 at 6:09 PM Robin Murphy <robin.murphy@arm.com> wrote:
> >
> > On 2020-07-03 11:10, Jagan Teki wrote:
> > > On Thu, Jul 2, 2020 at 7:26 PM Robin Murphy <robin.murphy@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 <jagan@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.

Look like we didn't have any requirement to change the root compatible
at this point since disabling ohci driver for legacy rk3288 revision
chips of no use.


Jagan.
  

On 2020/7/2 下午4: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 <jagan@amarulasolutions.com>

Reviewed-by: Kever Yang<kever.yang@rock-chips.com>

Thanks,
- Kever

> ---
>   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");
> +		if (ret < 0) {
> +			printf("failed to set rk3288w compatible (ret=%d)\n",
> +			       ret);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
>   static int do_clock(struct cmd_tbl *cmdtp, int flag, int argc,
>   		    char *const argv[])
>   {
  

On Wed, Jul 8, 2020 at 3:35 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>
>
> On 2020/7/2 下午4: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 <jagan@amarulasolutions.com>
>
> Reviewed-by: Kever Yang<kever.yang@rock-chips.com>

Added v2 for this. please have a look

Jagan.