Hi Bernhard, On Tue, 14 Apr 2020 at 03:26, Bernhard Messerklinger <bernhard.messerklin...@br-automation.com> wrote: > > Move FSP-S configuration to the device-tree like it's already done for > other SoCs (Baytrail). > > Signed-off-by: Bernhard Messerklinger > <bernhard.messerklin...@br-automation.com> > --- > > Changes in v2: > Remove FSP-M binding file > > arch/x86/cpu/apollolake/fsp_s.c | 1070 +++++++++++------ > arch/x86/dts/chromebook_coral.dts | 35 +- > .../asm/arch-apollolake/fsp/fsp_s_upd.h | 268 +++++ > .../fsp/fsp2/apollolake/fsp-s.txt | 485 ++++++++ > 4 files changed, 1489 insertions(+), 369 deletions(-) > create mode 100644 doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
Tested on chromebook-coral: Tested-by: Simon Glass <s...@chromium.org> > > diff --git a/arch/x86/cpu/apollolake/fsp_s.c b/arch/x86/cpu/apollolake/fsp_s.c > index 458825bc49..7d516adc92 100644 > --- a/arch/x86/cpu/apollolake/fsp_s.c > +++ b/arch/x86/cpu/apollolake/fsp_s.c [..] > + > +const u8 pcie_rp_clk_req_number_def[6] = { 0x4, 0x5, 0x0, 0x1, 0x2, 0x3 }; > +const u8 physical_slot_number_def[6] = { 0x0, 0x1, 0x2, 0x3, 0x4, 0x5 }; > +const u8 ipc_def[16] = { 0xf8, 0xef, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, > 0xff, > + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; > +const u8 port_usb20_per_port_pe_txi_set_def[8] = { 0x07, 0x06, 0x06, 0x06, > 0x07, > + 0x07, 0x07, 0x01 }; > +const u8 port_usb20_per_port_txi_set_def[8] = { 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x03}; > +const u8 port_usb20_hs_skew_sel_def[8] = { 0x00, 0x00, 0x00, 0x00, 0x00, > 0x00, > + 0x00, 0x01 }; > +const u8 port_usb20_i_usb_tx_emphasis_en_def[8] = { 0x03, 0x03, 0x03, 0x03, > + 0x03, 0x03, 0x03, 0x01 }; > +const u8 port_usb20_hs_npre_drv_sel_def[8] = { 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x03 }; Do we actually need these, or does the FSP have these defaults in the right place anyway? > + > +static int read_u8_array(u8 prop[], ofnode node, const char *propname, int > sz, > + u8 def) Please add function comment > { > const u8 *ptr; [..] > + ptr = ofnode_read_u8_array_ptr(node, propname, sz); > + if (ptr) { > + memcpy(prop, ptr, sz); > + } else { > + memset(prop, def, sz); > + return -EINVAL; > + } > > return 0; > } > > -static void apl_fsp_silicon_init_params_cb(struct apl_config *apl, > - struct fsp_s_config *cfg) > +static int read_u16_array(u16 prop[], ofnode node, const char *propname, int > sz, > + u16 def) > { same here > - u8 port; > - > - for (port = 0; port < MAX_USB2_PORTS; port++) { > - if (apl->usb2eye[port].per_port_tx_pe_half) > - cfg->port_usb20_per_port_tx_pe_half[port] = > - apl->usb2eye[port].per_port_tx_pe_half; > - > - if (apl->usb2eye[port].per_port_pe_txi_set) > - cfg->port_usb20_per_port_pe_txi_set[port] = > - apl->usb2eye[port].per_port_pe_txi_set; > - > - if (apl->usb2eye[port].per_port_txi_set) > - cfg->port_usb20_per_port_txi_set[port] = > - apl->usb2eye[port].per_port_txi_set; > + u32 *array_buf; > + int ret; > > - if (apl->usb2eye[port].hs_skew_sel) > - cfg->port_usb20_hs_skew_sel[port] = > - apl->usb2eye[port].hs_skew_sel; > + array_buf = malloc(sz * sizeof(u32)); Is it possible to use a dynamic array, like u32 array_buf[siz]; since these arrays are small and free() does not actually do anything in SPL by default. > + if (!array_buf) > + return -ENOMEM; > + > + ret = ofnode_read_u32_array(node, propname, array_buf, sz); > + if (!ret) { > + for (int i = 0; i < sz; i++) > + prop[i] = array_buf[i]; > + } else if (ret == -EINVAL) { > + for (int i = 0; i < sz; i++) > + prop[i] = def; > + ret = 0; > + } > + free(array_buf); > > - if (apl->usb2eye[port].usb_tx_emphasis_en) > - cfg->port_usb20_i_usb_tx_emphasis_en[port] = > - apl->usb2eye[port].usb_tx_emphasis_en; > + return ret; > +} > > - if (apl->usb2eye[port].per_port_rxi_set) > - cfg->port_usb20_per_port_rxi_set[port] = > - apl->usb2eye[port].per_port_rxi_set; > +static int read_u32_array(u32 prop[], ofnode node, const char *propname, int > sz, > + u32 def) > +{ > + int ret; > > - if (apl->usb2eye[port].hs_npre_drv_sel) > - cfg->port_usb20_hs_npre_drv_sel[port] = > - apl->usb2eye[port].hs_npre_drv_sel; > + ret = ofnode_read_u32_array(node, propname, prop, sz); > + if (ret == -EINVAL) { > + for (int i = 0; i < sz; i++) > + prop[i] = def; > + ret = 0; > } > + > + return ret; > } > [...] > +#ifdef CONFIG_HAVE_VBT if (IS_ENABLED(CONFIG_HAVE_VBT) > + cfg->graphics_config_ptr = (ulong)vbt_buf; > +#else > + cfg->graphics_config_ptr = 0; > +#endif [..] > + for (int i = 0; i < FSP_I2C_COUNT; i++) { > + snprintf(buf, sizeof(buf), "fsps,enable-i2c%d", i); > + *(&cfg->i2c0_enable + i) = ofnode_read_u32_default(node, buf, > + I2CX_ENABLE_PCI_MODE); cfg->i2c0_enable[i] same below > + } > + for (int i = 0; i < FSP_HSUART_COUNT; i++) { > + snprintf(buf, sizeof(buf), "fsps,enable-hsuart%d", i); > + *(&cfg->hsuart0_enable + i) = ofnode_read_u32_default(node, > buf, > + HSUARTX_ENABLE_PCI_MODE); > + } > + for (int i = 0; i < FSP_SPI_COUNT; i++) { > + snprintf(buf, sizeof(buf), "fsps,enable-spi%d", i); > + *(&cfg->spi0_enable + i) = ofnode_read_u32_default(node, buf, > + SPIX_ENABLE_PCI_MODE); > + } > + cfg->os_dbg_enable [..] Regards, Simon