| Message ID | 20200702084820.35942-1-jagan@amarulasolutions.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | rockchip: rk3288: Add OF board setup | expand |
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.
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[]) {
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(+)