Re: [PATCH v4 4/4] Input: cs40l50 - Add support for the CS40L50 haptic driver

From: James Ogletree <James.Ogletree@cirrus.com>
To: Jeff LaBundy <jeff@labundy.com>
Cc: James Ogletree <james.ogletree@opensource.cirrus.com>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Lee Jones <lee@kernel.org>, Fred Treven <Fred.Treven@cirrus.com>,
	Ben Bright <Ben.Bright@cirrus.com>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 4/4] Input: cs40l50 - Add support for the CS40L50 haptic driver
Date: Wed, 1 Nov 2023 20:47:07 +0000	[thread overview]
Message-ID: <120DA9BB-2607-4A85-B96C-5A2490E50876@cirrus.com> (raw)
In-Reply-To: <ZTiFbmutojF0LRZU@nixie71>

Hi Jeff,

> On Oct 24, 2023, at 10:03 PM, Jeff LaBundy <jeff@labundy.com> 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.

Best,
James