| Message ID | 1443490081-19526-1-git-send-email-thomas@wytron.com.tw |
|---|---|
| State | Superseded |
| Delegated to: | Thomas Chou |
| Headers | show |
On Tuesday, September 29, 2015 at 03:28:01 AM, Thomas Chou wrote: > Convert nios2 cpu to driver model. The cpu parameters are > extracted from device tree and saved to global data structure. > We will use them to replace the custom_fpga.h . > > Signed-off-by: Thomas Chou <thomas@wytron.com.tw> Hi! Minor nitpicks below. > --- > v2 > move cpu param setup to arch_cpu_init_dm, remove probe. > > arch/Kconfig | 3 ++ > arch/nios2/cpu/cpu.c | 76 > ++++++++++++++++++++++++++++++++++-- arch/nios2/dts/3c120_devboard.dts > | 1 + > arch/nios2/include/asm/global_data.h | 8 ++++ > configs/nios2-generic_defconfig | 2 - > 5 files changed, 85 insertions(+), 5 deletions(-) > > diff --git a/arch/Kconfig b/arch/Kconfig > index 207c778..9be1538 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -64,6 +64,9 @@ config NIOS2 > select HAVE_GENERIC_BOARD > select SYS_GENERIC_BOARD > select SUPPORT_OF_CONTROL > + select OF_CONTROL > + select DM > + select CPU What's this CONFIG_CPU for please ? > config OPENRISC > bool "OpenRISC architecture" > diff --git a/arch/nios2/cpu/cpu.c b/arch/nios2/cpu/cpu.c > index 39ae972..88984e2 100644 > --- a/arch/nios2/cpu/cpu.c > +++ b/arch/nios2/cpu/cpu.c > @@ -6,7 +6,9 @@ > */ > > #include <common.h> > -#include <asm/nios2.h> > +#include <cpu.h> > +#include <dm.h> > +#include <errno.h> > #include <asm/cache.h> > > DECLARE_GLOBAL_DATA_PTR; > @@ -51,10 +53,78 @@ void dcache_disable(void) > flush_dcache(CONFIG_SYS_DCACHE_SIZE, CONFIG_SYS_DCACHELINE_SIZE); > } > > -int arch_cpu_init(void) > +int arch_cpu_init_dm(void) > { > - gd->cpu_clk = CONFIG_SYS_CLK_FREQ; > + struct udevice *dev; > + > + uclass_first_device(UCLASS_CPU, &dev); > + if (!dev) > + return -ENODEV; > + > + gd->cpu_clk = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > + "clock-frequency", 0); > + gd->arch.dcache_line_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > + "dcache-line-size", 0); > + gd->arch.icache_line_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > + "icache-line-size", 0); > + gd->arch.dcache_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > + "dcache-size", 0); > + gd->arch.icache_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > + "icache-size", 0); > + gd->arch.reset_addr = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > + "altr,reset-addr", 0); > + gd->arch.exception_addr = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > + "altr,exception-addr", 0); > + gd->arch.has_mmu = fdtdec_get_int(gd->fdt_blob, dev->of_offset, > + "altr,has-mmu", 0); Shouldn't there be some sort of return value checking here ? > + gd->arch.io_region_base = gd->arch.has_mmu ? 0xe0000000 : 0x8000000; > gd->ram_size = CONFIG_SYS_SDRAM_SIZE; > > return 0; > } > + > +static int altera_nios2_get_desc(struct udevice *dev, char *buf, int size) > +{ > + const char *cpu_name = "Nios-II"; > + > + if (size < strlen(cpu_name)) > + return -ENOSPC; > + strcpy(buf, cpu_name); > + > + return 0; > +} > + > +static int altera_nios2_get_info(struct udevice *dev, struct cpu_info > *info) +{ > + > + info->cpu_freq = gd->cpu_clk; > + info->features = 1 << CPU_FEAT_L1_CACHE > + | (gd->arch.has_mmu ? 1 << CPU_FEAT_MMU : 0); I'd add parenthesis around the bitshifts, for the sake of clarity. Also, please put the ORR operator at the end of the line. > + return 0; > +} Thanks a lot :)
Hi Marek, On 09/29/2015 10:45 AM, Marek Vasut wrote: >> --- a/arch/Kconfig >> +++ b/arch/Kconfig >> @@ -64,6 +64,9 @@ config NIOS2 >> select HAVE_GENERIC_BOARD >> select SYS_GENERIC_BOARD >> select SUPPORT_OF_CONTROL >> + select OF_CONTROL >> + select DM >> + select CPU > > What's this CONFIG_CPU for please ? drivers/cpu/Kconfig It is "Enable CPU drivers using Driver Model". I think it should have been named as CONFIG_DM_CPU to avoid confusion. >> -int arch_cpu_init(void) >> +int arch_cpu_init_dm(void) >> { >> - gd->cpu_clk = CONFIG_SYS_CLK_FREQ; >> + struct udevice *dev; >> + >> + uclass_first_device(UCLASS_CPU, &dev); >> + if (!dev) >> + return -ENODEV; >> + >> + gd->cpu_clk = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> + "clock-frequency", 0); >> + gd->arch.dcache_line_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> + "dcache-line-size", 0); >> + gd->arch.icache_line_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> + "icache-line-size", 0); >> + gd->arch.dcache_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> + "dcache-size", 0); >> + gd->arch.icache_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> + "icache-size", 0); >> + gd->arch.reset_addr = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> + "altr,reset-addr", 0); >> + gd->arch.exception_addr = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> + "altr,exception-addr", 0); >> + gd->arch.has_mmu = fdtdec_get_int(gd->fdt_blob, dev->of_offset, >> + "altr,has-mmu", 0); > > Shouldn't there be some sort of return value checking here ? There might be some. Though I would like to depend upon the sopc2dts which generates the dts. I don't think these values should be tweaked by a human. :) > >> + info->features = 1 << CPU_FEAT_L1_CACHE >> + | (gd->arch.has_mmu ? 1 << CPU_FEAT_MMU : 0); > > I'd add parenthesis around the bitshifts, for the sake of clarity. > Also, please put the ORR operator at the end of the line. Will do as suggested. Thanks a lot for your review. Best regards, Thomas Chou
+Simon On Tue, Sep 29, 2015 at 3:18 PM, Thomas Chou <thomas@wytron.com.tw> wrote: > Hi Marek, > > On 09/29/2015 10:45 AM, Marek Vasut wrote: >>> >>> --- a/arch/Kconfig >>> +++ b/arch/Kconfig >>> @@ -64,6 +64,9 @@ config NIOS2 >>> select HAVE_GENERIC_BOARD >>> select SYS_GENERIC_BOARD >>> select SUPPORT_OF_CONTROL >>> + select OF_CONTROL >>> + select DM >>> + select CPU >> >> >> What's this CONFIG_CPU for please ? > > > drivers/cpu/Kconfig > > It is "Enable CPU drivers using Driver Model". I think it should have been > named as CONFIG_DM_CPU to avoid confusion. I remember Simon intended to name it as CONFIG_DM_CPU as I raised similar comments before. [snip] Regards, Bin
On Tue, Sep 29, 2015 at 10:28 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > +Simon > > On Tue, Sep 29, 2015 at 3:18 PM, Thomas Chou <thomas@wytron.com.tw> wrote: >> Hi Marek, >> >> On 09/29/2015 10:45 AM, Marek Vasut wrote: >>>> >>>> --- a/arch/Kconfig >>>> +++ b/arch/Kconfig >>>> @@ -64,6 +64,9 @@ config NIOS2 >>>> select HAVE_GENERIC_BOARD >>>> select SYS_GENERIC_BOARD >>>> select SUPPORT_OF_CONTROL >>>> + select OF_CONTROL >>>> + select DM >>>> + select CPU >>> >>> >>> What's this CONFIG_CPU for please ? >> >> >> drivers/cpu/Kconfig >> >> It is "Enable CPU drivers using Driver Model". I think it should have been >> named as CONFIG_DM_CPU to avoid confusion. > > I remember Simon intended to name it as CONFIG_DM_CPU as I raised > similar comments before. Oops, I must be drunk! I mean CONFIG_CPU ... > > [snip] > > Regards, > Bin
Hi, On 29 September 2015 at 08:29, Bin Meng <bmeng.cn@gmail.com> wrote: > On Tue, Sep 29, 2015 at 10:28 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >> +Simon >> >> On Tue, Sep 29, 2015 at 3:18 PM, Thomas Chou <thomas@wytron.com.tw> wrote: >>> Hi Marek, >>> >>> On 09/29/2015 10:45 AM, Marek Vasut wrote: >>>>> >>>>> --- a/arch/Kconfig >>>>> +++ b/arch/Kconfig >>>>> @@ -64,6 +64,9 @@ config NIOS2 >>>>> select HAVE_GENERIC_BOARD >>>>> select SYS_GENERIC_BOARD >>>>> select SUPPORT_OF_CONTROL >>>>> + select OF_CONTROL >>>>> + select DM >>>>> + select CPU >>>> >>>> >>>> What's this CONFIG_CPU for please ? >>> >>> >>> drivers/cpu/Kconfig >>> >>> It is "Enable CPU drivers using Driver Model". I think it should have been >>> named as CONFIG_DM_CPU to avoid confusion. >> >> I remember Simon intended to name it as CONFIG_DM_CPU as I raised >> similar comments before. > > Oops, I must be drunk! I mean CONFIG_CPU ... Yes. The CONFIG_DM_xxx options will go away as each subsystem is converted to driver model. Regards, Simon
diff --git a/arch/Kconfig b/arch/Kconfig index 207c778..9be1538 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -64,6 +64,9 @@ config NIOS2 select HAVE_GENERIC_BOARD select SYS_GENERIC_BOARD select SUPPORT_OF_CONTROL + select OF_CONTROL + select DM + select CPU config OPENRISC bool "OpenRISC architecture" diff --git a/arch/nios2/cpu/cpu.c b/arch/nios2/cpu/cpu.c index 39ae972..88984e2 100644 --- a/arch/nios2/cpu/cpu.c +++ b/arch/nios2/cpu/cpu.c @@ -6,7 +6,9 @@ */ #include <common.h> -#include <asm/nios2.h> +#include <cpu.h> +#include <dm.h> +#include <errno.h> #include <asm/cache.h> DECLARE_GLOBAL_DATA_PTR; @@ -51,10 +53,78 @@ void dcache_disable(void) flush_dcache(CONFIG_SYS_DCACHE_SIZE, CONFIG_SYS_DCACHELINE_SIZE); } -int arch_cpu_init(void) +int arch_cpu_init_dm(void) { - gd->cpu_clk = CONFIG_SYS_CLK_FREQ; + struct udevice *dev; + + uclass_first_device(UCLASS_CPU, &dev); + if (!dev) + return -ENODEV; + + gd->cpu_clk = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "clock-frequency", 0); + gd->arch.dcache_line_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "dcache-line-size", 0); + gd->arch.icache_line_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "icache-line-size", 0); + gd->arch.dcache_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "dcache-size", 0); + gd->arch.icache_size = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "icache-size", 0); + gd->arch.reset_addr = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "altr,reset-addr", 0); + gd->arch.exception_addr = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "altr,exception-addr", 0); + gd->arch.has_mmu = fdtdec_get_int(gd->fdt_blob, dev->of_offset, + "altr,has-mmu", 0); + gd->arch.io_region_base = gd->arch.has_mmu ? 0xe0000000 : 0x8000000; gd->ram_size = CONFIG_SYS_SDRAM_SIZE; return 0; } + +static int altera_nios2_get_desc(struct udevice *dev, char *buf, int size) +{ + const char *cpu_name = "Nios-II"; + + if (size < strlen(cpu_name)) + return -ENOSPC; + strcpy(buf, cpu_name); + + return 0; +} + +static int altera_nios2_get_info(struct udevice *dev, struct cpu_info *info) +{ + + info->cpu_freq = gd->cpu_clk; + info->features = 1 << CPU_FEAT_L1_CACHE + | (gd->arch.has_mmu ? 1 << CPU_FEAT_MMU : 0); + + return 0; +} + +static int altera_nios2_get_count(struct udevice *dev) +{ + return 1; +} + +static const struct cpu_ops altera_nios2_ops = { + .get_desc = altera_nios2_get_desc, + .get_info = altera_nios2_get_info, + .get_count = altera_nios2_get_count, +}; + +static const struct udevice_id altera_nios2_ids[] = { + { .compatible = "altr,nios2-1.0" }, + { .compatible = "altr,nios2-1.1" }, + { } +}; + +U_BOOT_DRIVER(altera_nios2) = { + .name = "altera_nios2", + .id = UCLASS_CPU, + .of_match = altera_nios2_ids, + .ops = &altera_nios2_ops, + .flags = DM_FLAG_PRE_RELOC, +}; diff --git a/arch/nios2/dts/3c120_devboard.dts b/arch/nios2/dts/3c120_devboard.dts index 781a652..2e2956f 100644 --- a/arch/nios2/dts/3c120_devboard.dts +++ b/arch/nios2/dts/3c120_devboard.dts @@ -41,6 +41,7 @@ altr,exception-addr = <0xd0000020>; altr,has-initda = <1>; altr,has-mmu = <1>; + u-boot,dm-pre-reloc; }; }; diff --git a/arch/nios2/include/asm/global_data.h b/arch/nios2/include/asm/global_data.h index 580b019..ee5180b 100644 --- a/arch/nios2/include/asm/global_data.h +++ b/arch/nios2/include/asm/global_data.h @@ -9,6 +9,14 @@ /* Architecture-specific global data */ struct arch_global_data { + u32 dcache_line_size; + u32 icache_line_size; + u32 dcache_size; + u32 icache_size; + u32 reset_addr; + u32 exception_addr; + int has_mmu; + u32 io_region_base; }; #include <asm-generic/global_data.h> diff --git a/configs/nios2-generic_defconfig b/configs/nios2-generic_defconfig index ad72272..63d99d8 100644 --- a/configs/nios2-generic_defconfig +++ b/configs/nios2-generic_defconfig @@ -13,9 +13,7 @@ CONFIG_HUSH_PARSER=y CONFIG_CMD_DHCP=y # CONFIG_CMD_NFS is not set CONFIG_CMD_PING=y -CONFIG_OF_CONTROL=y CONFIG_NET_RANDOM_ETHADDR=y -CONFIG_DM=y CONFIG_ALTERA_PIO=y CONFIG_ALTERA_JTAG_UART=y CONFIG_ALTERA_JTAG_UART_BYPASS=y
Convert nios2 cpu to driver model. The cpu parameters are extracted from device tree and saved to global data structure. We will use them to replace the custom_fpga.h . Signed-off-by: Thomas Chou <thomas@wytron.com.tw> --- v2 move cpu param setup to arch_cpu_init_dm, remove probe. arch/Kconfig | 3 ++ arch/nios2/cpu/cpu.c | 76 ++++++++++++++++++++++++++++++++++-- arch/nios2/dts/3c120_devboard.dts | 1 + arch/nios2/include/asm/global_data.h | 8 ++++ configs/nios2-generic_defconfig | 2 - 5 files changed, 85 insertions(+), 5 deletions(-)