Hi Bin, On 12 September 2017 at 09:20, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Wed, Sep 6, 2017 at 9:39 AM, Simon Glass <s...@chromium.org> wrote: >> Hi Bin, >> >> On 26 August 2017 at 18:10, Bin Meng <bmeng...@gmail.com> wrote: >>> Hi Simon, >>> >>> On Sun, Aug 27, 2017 at 6:39 AM, Simon Glass <s...@chromium.org> wrote: >>>> Hi Bin, >>>> >>>> On 26 August 2017 at 07:56, Bin Meng <bmeng...@gmail.com> wrote: >>>>> Hi Simon, >>>>> >>>>> On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass <s...@chromium.org> wrote: >>>>>> On 15 August 2017 at 23:42, Bin Meng <bmeng...@gmail.com> wrote: >>>>>>> Add FSP related configuration for Braswell. >>>>>>> >>>>>>> Signed-off-by: Bin Meng <bmeng...@gmail.com> >>>>>>> --- >>>>>>> >>>>>>> arch/x86/cpu/braswell/Makefile | 2 +- >>>>>>> arch/x86/cpu/braswell/fsp_configs.c | 158 ++++++++++++++ >>>>>>> .../include/asm/arch-braswell/fsp/fsp_configs.h | 89 ++++++++ >>>>>>> arch/x86/include/asm/arch-braswell/fsp/fsp_vpd.h | 172 >>>>>>> +++++++++++++++ >>>>>>> arch/x86/include/asm/arch-braswell/gpio.h | 234 >>>>>>> +++++++++++++++++++++ >>>>>>> 5 files changed, 654 insertions(+), 1 deletion(-) >>>>>>> create mode 100644 arch/x86/cpu/braswell/fsp_configs.c >>>>>>> create mode 100644 arch/x86/include/asm/arch-braswell/fsp/fsp_configs.h >>>>>>> create mode 100644 arch/x86/include/asm/arch-braswell/fsp/fsp_vpd.h >>>>>>> create mode 100644 arch/x86/include/asm/arch-braswell/gpio.h >>>>>>> >>>>>> >>>>>> Reviewed-by: Simon Glass <s...@chromium.org> >>>>>> >>>>>> Can this use drivers instead of manual device-tree access? >>>>> >>>>> Which part? >>>> >>>> Well you have intel,braswell-fsp for example. You could create a >>>> driver with the two compatible strings and have it read the platdata >>>> from the DT in the ofdata_to_platdata() method. >>> >>> I thought this before. We discussed the possibility of adding a new >>> FSP uclass long time ago. When I added the Braswell support, I wanted >>> to have a try since Braswell's FSP is v1.1 spec complaint and if we >>> have a uclass for FSP we can put the common stuff in the uclass >>> driver. But in the end I did not do it because: >>> >>> 1. FSP's initialization sequence is just a one time initialization and >>> we don't do anything after the initialization completes. >> >> That's not a very good reason though. There will be several drivers like >> that. >> >>> 2. Making a uclass for FSP means we have to delay fsp_init() to after >>> initf_dm().But after fsp_init(), we will return to board_init_f() >>> again and do the initialization for the second time. So all previous >>> platdata of FSP that is set up by DM gets lost during this process. >> >> Yes, although this is in the nature of the broken FSP API that we hope >> Intel will fix. As I understand it we already do the init twice, this >> is just a case of knowing what stage we are at. >> > > But DM initialization is unnecessary to get FSP run. This will lead > longer boot time.
Looking at the init sequence in board_f(), we have fsp_init() quite early: #if defined(CONFIG_HAVE_FSP) arch_fsp_init, #endif arch_cpu_init, /* basic arch cpu dependent setup */ mach_cpu_init, /* SoC/machine dependent CPU setup */ initf_dm, arch_cpu_init_dm, I still feel that ultimately FSP should be a driver (and should happen before arch_cpu_init_dm(), and that this should not affect boot time (since we need to do DM init at some point), but I think this needs more investigation. We would need to first empty out arch_cpu_init() and march_cpu_init(). So for now, let's leave it as is. > >>> 3. There are some other architecture-dependent stuff in the >>> arch_fsp_init() that is not suitable to be put in a FSP driver. >> >> But I am suggesting having a driver specific to the arch, not a >> generic one, so this should not be a problem. > > I mean there will be some non-FSP driver stuff (eg: MRC cache, ACPI > S3) in the FSP driver, which doesn't look very good IMO. Well given the nature of FSP (whole platform init) I think this is fine. The FSP driver can call out to common code or pull in other drivers as needed. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
