Hi James, On Wed, Nov 01, 2023 at 08:47:07PM +0000, James Ogletree wrote: > Hi Jeff, > > > On Oct 24, 2023, at 10:03 PM, Jeff LaBundy <jeff@xxxxxxxxxxx> wrote: > >> > >> +static const struct cs_dsp_client_ops cs40l50_cs_dsp_client_ops; > >> + > >> +static const struct cs_dsp_region cs40l50_dsp_regions[] = { > >> + { > >> + .type = WMFW_HALO_PM_PACKED, > >> + .base = CS40L50_DSP1_PMEM_0 > >> + }, > >> + { > >> + .type = WMFW_HALO_XM_PACKED, > >> + .base = CS40L50_DSP1_XMEM_PACKED_0 > >> + }, > >> + { > >> + .type = WMFW_HALO_YM_PACKED, > >> + .base = CS40L50_DSP1_YMEM_PACKED_0 > >> + }, > >> + { > >> + .type = WMFW_ADSP2_XM, > >> + .base = CS40L50_DSP1_XMEM_UNPACKED24_0 > >> + }, > >> + { > >> + .type = WMFW_ADSP2_YM, > >> + .base = CS40L50_DSP1_YMEM_UNPACKED24_0 > >> + }, > >> +}; > >> + > >> +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50) > >> +{ > >> + cs40l50->dsp.num = 1; > >> + cs40l50->dsp.type = WMFW_HALO; > >> + cs40l50->dsp.dev = cs40l50->dev; > >> + cs40l50->dsp.regmap = cs40l50->regmap; > >> + cs40l50->dsp.base = CS40L50_CORE_BASE; > >> + cs40l50->dsp.base_sysinfo = CS40L50_SYS_INFO_ID; > >> + cs40l50->dsp.mem = cs40l50_dsp_regions; > >> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions); > >> + cs40l50->dsp.no_core_startstop = true; > >> + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops; > >> + > >> + return cs_dsp_halo_init(&cs40l50->dsp); > >> +} > >> + > >> +static struct cs_hap_bank cs40l50_banks[] = { > >> + { > >> + .bank = WVFRM_BANK_RAM, > >> + .base_index = CS40L50_RAM_BANK_INDEX_START, > >> + .max_index = CS40L50_RAM_BANK_INDEX_START, > >> + }, > >> + { > >> + .bank = WVFRM_BANK_ROM, > >> + .base_index = CS40L50_ROM_BANK_INDEX_START, > >> + .max_index = CS40L50_ROM_BANK_INDEX_END, > >> + }, > >> + { > >> + .bank = WVFRM_BANK_OWT, > >> + .base_index = CS40L50_RTH_INDEX_START, > >> + .max_index = CS40L50_RTH_INDEX_END, > >> + }, > >> +}; > > > > These structs describe the DSP, and hence the silicon; they are not > > specific to the input/FF device. Presumably the DSP could run algorithms > > that support only the I2S streaming case as well (e.g. A2H); therefore, > > these seem more appropriately placed in the MFD. > > Acknowledged, but would you consider the last struct “cs40l50_banks” as > an exception? It would go unused in a codec-only setup. I agree with you; I should have inserted my comment after cs40l50_cs_dsp_init() and not cs40l50_banks[]. > > Best, > James > > Kind regards, Jeff LaBundy