| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZNWz7F7qLeNKDlG/@nixie71>
Date: Thu, 10 Aug 2023 23:07:08 -0500
From: Jeff LaBundy <jeff@...undy.com>
To: James Ogletree <james.ogletree@...rus.com>
Cc: dmitry.torokhov@...il.com, Fred Treven <fred.treven@...rus.com>,
Ben Bright <ben.bright@...rus.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Lee Jones <lee@...nel.org>,
Joel Stanley <joel@....id.au>, Arnd Bergmann <arnd@...db.de>,
Jacky Bai <ping.bai@....com>, Jean Delvare <jdelvare@...e.de>,
Eddie James <eajames@...ux.ibm.com>,
Markus Schneider-Pargmann <msp@...libre.com>,
ChiYuan Huang <cy_huang@...htek.com>,
Randy Dunlap <rdunlap@...radead.org>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
patches@...rus.com, linux-input@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/2] Input: cs40l50 - Initial support for Cirrus Logic
CS40L50
Hi James,
Great work as always; just some minor comments throughout.
On Wed, Aug 09, 2023 at 07:10:28PM +0000, James Ogletree wrote:
> Introduce support for Cirrus Logic Device CS40L50: a
> haptics driver with waveform memory DSP and closed-loop
> algorithms.
>From my extremely naive point of view, some of the code that follows
bares resemblance to the recently reviewed L26. My assumption is that
these devices are different enough in nature to warrant completely
different drivers; is that accurate?
One reason for asking is because the L26 driver included a cornucopia
of power-management overhead, yet we see none of that here. Assuming
both L50 and L26 are built around the same Halo DSP, why is there such
a fundamental difference between the two in terms of power management?
To that end, how does this driver handle hibernation? Is hibernation
not supported, or do we simply defer to the DSP? In the case of the
latter, why is L50 given this freedom but not L26?
This is not a suggestion to change anything; it's just to help set the
context and provide more background information. However, I'm curious
whether you've considered consolidating both L26 and L50 into a single
driver, especially given recent discussion around promoting L26 into an
MFD with both input and codec children. Would L50 ever support audio-to
-haptics over I2S?
>
> Signed-off-by: James Ogletree <james.ogletree@...rus.com>
> ---
> MAINTAINERS | 2 +
> drivers/input/misc/Kconfig | 33 +
> drivers/input/misc/Makefile | 3 +
> drivers/input/misc/cs40l50-i2c.c | 67 ++
> drivers/input/misc/cs40l50-spi.c | 67 ++
> drivers/input/misc/cs40l50.c | 1008 ++++++++++++++++++++++++++++++
> include/linux/input/cs40l50.h | 321 ++++++++++
> 7 files changed, 1501 insertions(+)
> create mode 100644 drivers/input/misc/cs40l50-i2c.c
> create mode 100644 drivers/input/misc/cs40l50-spi.c
> create mode 100644 drivers/input/misc/cs40l50.c
> create mode 100644 include/linux/input/cs40l50.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2869dd2dbbcf..e82d03293383 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2088,6 +2088,8 @@ M: Ben Bright <ben.bright@...rus.com>
> L: patches@...nsource.cirrus.com
> S: Supported
> F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> +F: drivers/input/misc/cs40l*
> +F: include/linux/input/cs40l*
>
> ARM/CLKDEV SUPPORT
> M: Russell King <linux@...linux.org.uk>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index c47eecc6f83b..c1c1572ec39c 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -140,6 +140,39 @@ config INPUT_BMA150
> To compile this driver as a module, choose M here: the
> module will be called bma150.
>
> +config INPUT_CS40L50
> + tristate "Cirrus Logic CS40L50 Haptic Driver support"
> + select CS_DSP
> + select REGMAP_IRQ
> + help
> + Say Y here to enable support for CS40L50 boosted haptic
> + driver.
> +
> + To compile the driver as a module choose M here: the module
> + will be called cs40l50.
> +
> +config INPUT_CS40L50_I2C
> + tristate "Support I2C bus connection"
> + depends on INPUT_CS40L50 && I2C
> + select REGMAP_I2C
> + help
> + Say Y here if you have Cirrus Logic's CS40L50 connected
> + to an I2C bus.
> +
> + To compile the driver as a module choose M here: the
> + module will be called cs40l50_i2c.
> +
> +config INPUT_CS40L50_SPI
> + tristate "Support SPI bus connection"
> + depends on INPUT_CS40L50 && SPI
> + select REGMAP_SPI
> + help
> + Say Y here if you have Cirrus Logic's CS40L50 connected
> + to a SPI bus.
> +
> + To compile the driver as a module choose M here: the
> + module will be called cs40l50_spi.
> +
> config INPUT_E3X0_BUTTON
> tristate "NI Ettus Research USRP E3xx Button support."
> default n
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 04296a4abe8e..77f34a33c364 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -28,6 +28,9 @@ obj-$(CONFIG_INPUT_CMA3000) += cma3000_d0x.o
> obj-$(CONFIG_INPUT_CMA3000_I2C) += cma3000_d0x_i2c.o
> obj-$(CONFIG_INPUT_COBALT_BTNS) += cobalt_btns.o
> obj-$(CONFIG_INPUT_CPCAP_PWRBUTTON) += cpcap-pwrbutton.o
> +obj-$(CONFIG_INPUT_CS40L50) += cs40l50.o
> +obj-$(CONFIG_INPUT_CS40L50_I2C) += cs40l50-i2c.o
> +obj-$(CONFIG_INPUT_CS40L50_SPI) += cs40l50-spi.o
> obj-$(CONFIG_INPUT_DA7280_HAPTICS) += da7280.o
> obj-$(CONFIG_INPUT_DA9052_ONKEY) += da9052_onkey.o
> obj-$(CONFIG_INPUT_DA9055_ONKEY) += da9055_onkey.o
> diff --git a/drivers/input/misc/cs40l50-i2c.c b/drivers/input/misc/cs40l50-i2c.c
> new file mode 100644
> index 000000000000..6ebe1b51990b
> --- /dev/null
> +++ b/drivers/input/misc/cs40l50-i2c.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP
> + * and Closed-Loop Algorithms
Nit: the capitalization and general sentence structure here is hard to
follow; maybe something like "... with DSP-based waveform memory and
closed-loop algorithms" would make more sense?
> + *
> + * Copyright 2023 Cirrus Logic, Inc.
> + *
> + */
> +#include <linux/i2c.h>
> +#include <linux/input/cs40l50.h>
> +
> +static const struct i2c_device_id cs40l50_id_i2c[] = {
> + {"cs40l50", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, cs40l50_id_i2c);
> +
> +static const struct of_device_id cs40l50_of_match[] = {
> + { .compatible = "cirrus,cs40l50" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, cs40l50_of_match);
> +
> +static int cs40l50_i2c_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct cs40l50_private *cs40l50;
> +
> + cs40l50 = devm_kzalloc(dev, sizeof(*cs40l50), GFP_KERNEL);
> + if (!cs40l50)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, cs40l50);
> +
> + cs40l50->regmap = devm_regmap_init_i2c(client, &cs40l50_regmap);
> + if (IS_ERR(cs40l50->regmap))
> + return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap),
> + "Failed to initialize register map\n");
Nit: line breaks tend to be more readable when aligned against the
parenthesis as follows:
if (...)
return dev_err_probe(...,
...);
At least, it's much more common.
> +
> + cs40l50->dev = dev;
> + cs40l50->irq = client->irq;
> +
> + return cs40l50_probe(cs40l50);
> +}
> +
> +static void cs40l50_i2c_remove(struct i2c_client *client)
> +{
> + struct cs40l50_private *cs40l50 = i2c_get_clientdata(client);
> +
> + cs40l50_remove(cs40l50);
> +}
> +
> +static struct i2c_driver cs40l50_i2c_driver = {
> + .driver = {
> + .name = "cs40l50",
> + .of_match_table = cs40l50_of_match,
> + },
> + .id_table = cs40l50_id_i2c,
> + .probe_new = cs40l50_i2c_probe,
Mainline has moved back to .probe now; .probe_new is becoming depracated.
> + .remove = cs40l50_i2c_remove,
> +};
> +
> +module_i2c_driver(cs40l50_i2c_driver);
> +
> +MODULE_DESCRIPTION("CS40L50 I2C Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@...rus.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/input/misc/cs40l50-spi.c b/drivers/input/misc/cs40l50-spi.c
> new file mode 100644
> index 000000000000..5fcf3ba137f0
> --- /dev/null
> +++ b/drivers/input/misc/cs40l50-spi.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP
> + * and Closed-Loop Algorithms
> + *
> + * Copyright 2023 Cirrus Logic, Inc.
> + *
> + */
> +#include <linux/spi/spi.h>
> +#include <linux/input/cs40l50.h>
> +
> +static const struct spi_device_id cs40l50_id_spi[] = {
> + {"cs40l50", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(spi, cs40l50_id_spi);
> +
> +static const struct of_device_id cs40l50_of_match[] = {
> + { .compatible = "cirrus,cs40l50" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, cs40l50_of_match);
> +
> +static int cs40l50_spi_probe(struct spi_device *spi)
> +{
> + struct cs40l50_private *cs40l50;
> + struct device *dev = &spi->dev;
> +
> + cs40l50 = devm_kzalloc(dev, sizeof(*cs40l50), GFP_KERNEL);
> + if (!cs40l50)
> + return -ENOMEM;
> +
> + spi_set_drvdata(spi, cs40l50);
> +
> + cs40l50->regmap = devm_regmap_init_spi(spi, &cs40l50_regmap);
> + if (IS_ERR(cs40l50->regmap))
> + return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap),
> + "Failed to initialize register map\n");
> +
> + cs40l50->dev = dev;
> + cs40l50->irq = spi->irq;
> +
> + return cs40l50_probe(cs40l50);
> +}
> +
> +static void cs40l50_spi_remove(struct spi_device *spi)
> +{
> + struct cs40l50_private *cs40l50 = spi_get_drvdata(spi);
> +
> + cs40l50_remove(cs40l50);
> +}
> +
> +static struct spi_driver cs40l50_spi_driver = {
> + .driver = {
> + .name = "cs40l50",
> + .of_match_table = cs40l50_of_match,
> + },
> + .id_table = cs40l50_id_spi,
> + .probe = cs40l50_spi_probe,
> + .remove = cs40l50_spi_remove,
> +};
> +
> +module_spi_driver(cs40l50_spi_driver);
> +
> +MODULE_DESCRIPTION("CS40L50 SPI Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@...rus.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/input/misc/cs40l50.c b/drivers/input/misc/cs40l50.c
> new file mode 100644
> index 000000000000..9bb2881683c0
> --- /dev/null
> +++ b/drivers/input/misc/cs40l50.c
> @@ -0,0 +1,1008 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP
> + * and Closed-Loop Algorithms
> + *
> + * Copyright 2023 Cirrus Logic, Inc.
> + *
> + */
> +#include <linux/firmware.h>
> +#include <linux/i2c.h>
Why to #include i2c.h here? This driver should be agnostic to the transport.
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +#include <linux/bitfield.h>
> +#include <linux/input/cs40l50.h>
Please sort these alphabetically. Are you sure you're using all of them?
> +
> +const struct regmap_config cs40l50_regmap = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .reg_format_endian = REGMAP_ENDIAN_BIG,
> + .val_format_endian = REGMAP_ENDIAN_BIG,
> +};
> +EXPORT_SYMBOL_GPL(cs40l50_regmap);
> +
> +static struct regulator_bulk_data cs40l50_supplies[] = {
> + { .supply = "vp" },
> + { .supply = "vio" },
> + { .supply = "va" },
> +};
> +
> +static int cs40l50_dsp_write(struct cs40l50_private *cs40l50, u32 write_reg, u32 write_val)
> +{
> + int error = 0, i;
No need to initialize 'error' here.
> +
> + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) {
> + error = regmap_write(cs40l50->regmap, write_reg, write_val);
> + if (!error)
> + break;
This seems more efficient:
if (!error)
return 0;
...then you can avoid the check for i == CS40L50_DSP_TIMEOUT_COUNT below.
> +
> + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100);
> + }
> +
> + if (i == CS40L50_DSP_TIMEOUT_COUNT) {
> + dev_err(cs40l50->dev, "Error writing %#X to %#X: %d", write_val, write_reg, error);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int cs40l50_dsp_read(struct cs40l50_private *cs40l50, u32 read_reg, u32 *read_val)
> +{
> + int error = 0, i;
...and here.
> +
> + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) {
> + error = regmap_read(cs40l50->regmap, read_reg, read_val);
> + if (!error)
> + break;
...and here.
> +
> + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100);
> + }
> +
> + if (i == CS40L50_DSP_TIMEOUT_COUNT) {
> + dev_err(cs40l50->dev, "Error reading %#X: %d", read_reg, error);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int cs40l50_pseq_write(struct cs40l50_private *cs40l50, u32 addr, u32 data)
> +{
> + u8 operation = 0;
> + struct cs40l50_pseq_op *op_new, *op_end;
> + int error;
Nit: it is kind of personal preference, but "reverse Christmas tree" tends
to be more common, i.e. move the struct definitions (longest) to the top.
> +
> + op_new = devm_kzalloc(cs40l50->dev, sizeof(*op_new), GFP_KERNEL);
> + if (!op_new)
> + return -ENOMEM;
> +
> + op_new->words[0] = (addr >> CS40L50_PSEQ_WRITE_FULL_UPPER_ADDR_SHIFT);
> + op_new->words[1] = ((addr & CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_MASK) <<
> + CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_SHIFT) |
> + (data >> CS40L50_PSEQ_WRITE_FULL_UPPER_DATA_SHIFT);
Nit: consider aligning the line breaks past the equal sign for readability.
> + op_new->words[2] = data & CS40L50_PSEQ_WRITE_FULL_LOWER_DATA_MASK;
> +
> + list_for_each_entry(op_end, &cs40l50->pseq_op_head, list) {
> + operation = op_end->operation;
> + if (operation == CS40L50_PSEQ_OP_END)
> + break;
> + }
> +
> + if (operation != CS40L50_PSEQ_OP_END) {
> + error = -ENOENT;
> + goto op_new_free;
> + }
> +
> + if ((CS40L50_PSEQ_MAX_WORDS - op_end->offset) < ARRAY_SIZE(op_new->words)) {
> + error = -ENOSPC;
> + goto op_new_free;
> + }
> +
> + op_new->offset = op_end->offset;
> + op_end->offset += (ARRAY_SIZE(op_new->words) * sizeof(u32));
> +
> + error = regmap_bulk_write(cs40l50->regmap, CS40L50_DSP1_POWER_ON_SEQ + op_new->offset,
> + op_new->words, ARRAY_SIZE(op_new->words));
Nit: alignment
> + if (error)
> + goto op_new_free;
> +
> + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP1_POWER_ON_SEQ + op_end->offset,
> + op_end->words[0]);
...and here.
> + if (error)
> + goto op_new_free;
> +
> + list_add(&op_new->list, &cs40l50->pseq_op_head);
> +
> + return 0;
> +
> +op_new_free:
> + devm_kfree(cs40l50->dev, op_new);
> +
> + return error;
> +}
> +
> +static struct cs40l50_effect *cs40l50_find_effect(struct cs40l50_private *cs40l50, int id)
> +{
> + struct cs40l50_effect *effect;
> +
> + if (!list_empty(&cs40l50->effect_head)) {
> + list_for_each_entry(effect, &cs40l50->effect_head, list) {
> + if (effect->id == id)
> + return effect;
> + }
> + }
Are you sure the check against list_empty is necessary? Won't list_for_each_entry()
simply be a NOP if the list is empty? (genuine question)
> +
> + return NULL;
> +}
> +
> +static int cs40l50_owt_upload(struct cs40l50_private *cs40l50, s16 *in_data, u32 in_data_nibbles)
> +{
> + size_t in_data_bytes = 2 * in_data_nibbles;
> + struct cs40l50_owt_header header;
> + u32 wt_offset, wt_size_words;
> + int error;
> + u8 *data;
> +
> + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_OWT_NEXT, &wt_offset);
> + if (error)
> + return error;
> +
> + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_OWT_SIZE, &wt_size_words);
> + if (error)
> + return error;
> +
> + if ((wt_size_words * sizeof(u32)) < CS40L50_WT_HEADER_PWLE_SIZE + in_data_bytes)
> + return -ENOSPC;
> +
> + data = kcalloc(CS40L50_WT_HEADER_PWLE_SIZE + in_data_bytes, sizeof(u8), GFP_KERNEL);
> + if (IS_ERR_OR_NULL(data))
> + return -ENOMEM;
> +
> + header.wvfrm_type = in_data[0] == CS40L50_PCM_ID ? CS40L50_WT_TYPE_PCM :
> + CS40L50_WT_TYPE_PWLE;
Nit: this is a bit more readable:
header.wvfrm_type = in_data[0] == CS40L50_PCM_ID ? CS40L50_WT_TYPE_PCM
: CS40L50_WT_TYPE_PWLE;
> + header.offset = CS40L50_WT_HEADER_OFFSET;
> + header.data_size = in_data_bytes / sizeof(u32);
> +
> + memcpy(data, &header, sizeof(header));
> + memcpy(data + (CS40L50_WT_HEADER_OFFSET * sizeof(u32)), in_data, in_data_bytes);
> +
> + error = regmap_bulk_write(cs40l50->regmap, CS40L50_DSP1_OWT_BASE +
> + (wt_offset * sizeof(u32)), data,
> + CS40L50_WT_HEADER_PWLE_SIZE + in_data_bytes);
> + if (error)
> + goto err_free;
> +
> + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, CS40L50_MBOX_CMD_OWT_PUSH);
> +err_free:
> + kfree(data);
We can avoid a goto and label for this simple case as follows:
error = ...
if (!error)
error = ...
kfree(...);
return error;
> +
> + return error;
> +}
> +
> +static int cs40l50_add_effect(struct input_dev *dev, struct ff_effect *effect,
> + struct ff_effect *old)
Nit: alignment
> +{
> + struct cs40l50_private *cs40l50 = input_get_drvdata(dev);
> + bool is_new = false;
> + struct cs40l50_effect *new_effect;
> + u32 nwaves, base_index, max_index;
> + s16 *raw_custom_data;
> + int error;
> +
> + if (effect->type != FF_PERIODIC || effect->u.periodic.waveform != FF_CUSTOM) {
> + dev_err(cs40l50->dev, "Type (%#X) or waveform (%#X) unsupported\n",
> + effect->type, effect->u.periodic.waveform);
> + return -EINVAL;
> + }
> +
> + raw_custom_data = kcalloc(effect->u.periodic.custom_len, sizeof(s16), GFP_KERNEL);
> + if (IS_ERR_OR_NULL(raw_custom_data))
> + return -ENOMEM;
> +
> + error = copy_from_user(raw_custom_data, effect->u.periodic.custom_data,
> + sizeof(s16) * effect->u.periodic.custom_len);
> + if (error)
> + goto out_free;
> +
> + mutex_lock(&cs40l50->lock);
> +
> + new_effect = cs40l50_find_effect(cs40l50, effect->id);
> + if (!new_effect) {
> + is_new = true;
> + new_effect = kzalloc(sizeof(*new_effect), GFP_KERNEL);
> + if (IS_ERR_OR_NULL(new_effect)) {
> + error = -ENOMEM;
> + goto err_mutex;
> + }
> + }
> +
> + if (effect->u.periodic.custom_len > CS40L50_CUSTOM_DATA_SIZE) {
> + error = cs40l50_dsp_read(cs40l50, CS40L50_NUM_OF_OWT_WAVES,
> + &new_effect->trigger_index);
> + if (error)
> + goto err_free;
> +
> + new_effect->wvfrm_bank = CS40L50_WVFRM_BANK_OWT;
> + base_index = CS40L50_OWT_INDEX_START;
> + max_index = CS40L50_OWT_INDEX_END;
> + } else {
> + new_effect->trigger_index = (raw_custom_data[1] & 0xffffu);
> + new_effect->wvfrm_bank = (raw_custom_data[0] & 0xffffu);
> +
> + if (new_effect->wvfrm_bank == CS40L50_WVFRM_BANK_ROM) {
> + base_index = CS40L50_ROM_BANK_INDEX_START;
> + max_index = CS40L50_ROM_BANK_INDEX_END;
> + } else if (new_effect->wvfrm_bank == CS40L50_WVFRM_BANK_RAM) {
> + error = cs40l50_dsp_read(cs40l50, CS40L50_NUM_OF_WAVES, &nwaves);
> + if (error)
> + goto err_free;
> +
> + base_index = CS40L50_RAM_BANK_INDEX_START;
> + max_index = base_index + nwaves - 1;
> + } else {
> + dev_err(cs40l50->dev, "Invalid waveform bank\n");
> + error = -EINVAL;
> + goto err_free;
> + }
> + }
> +
> + new_effect->trigger_index += base_index;
> +
> + if (new_effect->trigger_index > max_index) {
> + dev_err(cs40l50->dev, "Index %#X out of bounds\n", new_effect->trigger_index);
> + error = -EINVAL;
> + goto err_free;
> + }
> +
> + if (effect->trigger.button) {
> + new_effect->mapping = CS40L50_DSP1_GPIO_BASE + sizeof(u32) *
> + FIELD_GET(CS40L50_BTN_NUM_MASK, effect->trigger.button) * 2 -
> + FIELD_GET(CS40L50_BTN_EDGE_MASK, effect->trigger.button);
> +
> + error = cs40l50_dsp_write(cs40l50, new_effect->mapping, effect->trigger.button);
> + if (error)
> + goto err_free;
> + }
> +
> + if (new_effect->wvfrm_bank == CS40L50_WVFRM_BANK_OWT) {
> + error = cs40l50_owt_upload(cs40l50, raw_custom_data,
> + effect->u.periodic.custom_len);
> + if (error)
> + goto err_free;
> + }
> +
> +err_free:
> + if (is_new)
> + error ? kfree(new_effect) : list_add(&new_effect->list, &cs40l50->effect_head);
Just wanted to echo Charles' point that this made my head spin a bit. Good
to know it's not just me ;)
> +err_mutex:
> + mutex_unlock(&cs40l50->lock);
> +out_free:
> + kfree(raw_custom_data);
> +
> + return error ? -EFAULT : 0;
> +}
> +
> +
> +static void cs40l50_vibe_start_worker(struct work_struct *work)
> +{
> + struct cs40l50_private *cs40l50 =
> + container_of(work, struct cs40l50_private, vibe_start_work);
> + struct cs40l50_effect *effect;
> +
> + mutex_lock(&cs40l50->lock);
> +
> + effect = cs40l50_find_effect(cs40l50, cs40l50->trigger_effect->id);
> + if (effect)
> + cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, effect->trigger_index);
> +
> + mutex_unlock(&cs40l50->lock);
> +}
> +
> +static void cs40l50_vibe_stop_worker(struct work_struct *work)
> +{
> + struct cs40l50_private *cs40l50 =
> + container_of(work, struct cs40l50_private, vibe_stop_work);
> +
> + mutex_lock(&cs40l50->lock);
> +
> + cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1, CS40L50_MBOX_CMD_STOP_PLAYBACK);
> +
> + mutex_unlock(&cs40l50->lock);
> +}
> +
> +static int cs40l50_playback_effect(struct input_dev *dev, int effect_id, int val)
> +{
> + struct cs40l50_private *cs40l50 = input_get_drvdata(dev);
> +
> + cs40l50->trigger_effect = &dev->ff->effects[effect_id];
> +
> + if (val > 0)
> + queue_work(cs40l50->vibe_workqueue, &cs40l50->vibe_start_work);
> + else
> + queue_work(cs40l50->vibe_workqueue, &cs40l50->vibe_stop_work);
> +
> + return 0;
> +}
> +
> +static int cs40l50_erase_effect(struct input_dev *dev, int effect_id)
> +{
> + int error = 0;
> + struct cs40l50_private *cs40l50 = input_get_drvdata(dev);
> + struct cs40l50_effect *effect, *effect_owt;
> +
> + mutex_lock(&cs40l50->lock);
> +
> + effect = cs40l50_find_effect(cs40l50, dev->ff->effects[effect_id].id);
> + if (!effect) {
> + error = -EINVAL;
> + goto err_mutex;
> + }
> +
> + if (effect->mapping != CS40L50_GPIO_MAPPING_INVALID) {
> + error = cs40l50_dsp_write(cs40l50, effect->mapping, CS40L50_GPI_DISABLE);
> + if (error)
> + goto err_mutex;
> + }
> +
> + if (effect->wvfrm_bank == CS40L50_WVFRM_BANK_OWT) {
> + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP_VIRTUAL1_MBOX_1,
> + CS40L50_MBOX_CMD_OWT_DELETE | effect->trigger_index);
Nit: alignment here and elsewhere.
> + if (error)
> + goto err_mutex;
> +
> + list_for_each_entry(effect_owt, &cs40l50->effect_head, list) {
> + if (effect_owt->wvfrm_bank == CS40L50_WVFRM_BANK_OWT &&
> + effect_owt->trigger_index > effect->trigger_index)
> + effect_owt->trigger_index--;
> + }
> + }
> +
> + list_del(&effect->list);
> + kfree(effect);
> +err_mutex:
> + mutex_unlock(&cs40l50->lock);
> +
> + return error;
> +}
> +
> +static int cs40l50_mailbox_read_next(struct cs40l50_private *cs40l50, u32 *val)
> +{
> + u32 wt_ptr, rd_ptr;
> + int error = 0;
No need to initialize 'error' here.
> +
> + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_MBOX_QUEUE_WT, &wt_ptr);
> + if (error)
> + return error;
> +
> + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_MBOX_QUEUE_RD, &rd_ptr);
> + if (error)
> + return error;
> +
> + if (wt_ptr == rd_ptr) {
> + *val = 0;
> + return 0;
> + }
> +
> + error = cs40l50_dsp_read(cs40l50, rd_ptr, val);
> + if (error)
> + return error;
> +
> + rd_ptr += sizeof(u32);
> + if (rd_ptr > CS40L50_MBOX_QUEUE_END)
> + rd_ptr = CS40L50_MBOX_QUEUE_BASE;
> +
> + return cs40l50_dsp_write(cs40l50, CS40L50_DSP1_MBOX_QUEUE_RD, rd_ptr);
> +}
> +
> +static int cs40l50_handle_f0_est_done(struct cs40l50_private *cs40l50)
> +{
> + int error;
> + u32 f_zero;
> +
> + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_F0_ESTIMATION, &f_zero);
> + if (error)
> + return error;
> +
> + return cs40l50_dsp_write(cs40l50, CS40L50_DSP1_F0_STORED, f_zero);
> +}
> +
> +static int cs40l50_handle_redc_est_done(struct cs40l50_private *cs40l50)
> +{
> + int integer, fractional, error;
> + u32 redc;
> +
> + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_RE_EST_STATUS, &redc);
> + if (error)
> + return error;
> +
> + error = cs40l50_dsp_write(cs40l50, CS40L50_DSP1_REDC_ESTIMATION, redc);
> + if (error)
> + return error;
> +
> + /* Convert from Q8.15 to (Q7.17 * 29/240) */
> + integer = (redc >> 15) & 0xFF;
> + fractional = (redc & 0x7FFF) * 4;
> +
> + return cs40l50_dsp_write(cs40l50, CS40L50_DSP1_REDC_STORED,
> + (((integer << 17) | fractional) * 29 / 240) & GENMASK(23, 0));
> +}
> +
> +static int cs40l50_error_release(struct cs40l50_private *cs40l50)
> +{
> + int error;
> +
> + error = cs40l50_dsp_write(cs40l50, CS40L50_ERROR_RELEASE, CS40L50_ERROR_RELEASE_MASK);
> + if (error)
> + return error;
> +
> + return cs40l50_dsp_write(cs40l50, CS40L50_ERROR_RELEASE, 0);
> +}
> +
> +static irqreturn_t cs40l50_handle_mbox_buffer(int irq, void *data)
> +{
> + struct cs40l50_private *cs40l50 = data;
> + int error = 0;
> + u32 val;
> +
> + mutex_lock(&cs40l50->lock);
> +
> + while (!cs40l50_mailbox_read_next(cs40l50, &val)) {
> + switch (val) {
> + case 0:
> + goto out_irq;
> + case CS40L50_DSP_MBOX_HAPTIC_TRIGGER_GPIO:
> + dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_GPIO\n");
> + break;
> + case CS40L50_DSP_MBOX_HAPTIC_TRIGGER_MBOX:
> + dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_MBOX\n");
> + break;
> + case CS40L50_DSP_MBOX_HAPTIC_TRIGGER_I2S:
> + dev_dbg(cs40l50->dev, "Mailbox: TRIGGER_I2S\n");
> + break;
> + case CS40L50_DSP_MBOX_HAPTIC_COMPLETE_MBOX:
> + dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_MBOX\n");
> + break;
> + case CS40L50_DSP_MBOX_HAPTIC_COMPLETE_GPIO:
> + dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_GPIO\n");
> + break;
> + case CS40L50_DSP_MBOX_HAPTIC_COMPLETE_I2S:
> + dev_dbg(cs40l50->dev, "Mailbox: COMPLETE_I2S\n");
> + break;
> + case CS40L50_DSP_MBOX_F0_EST_START:
> + dev_dbg(cs40l50->dev, "Mailbox: F0_EST_START\n");
> + break;
> + case CS40L50_DSP_MBOX_F0_EST_DONE:
> + dev_dbg(cs40l50->dev, "Mailbox: F0_EST_DONE\n");
> + error = cs40l50_handle_f0_est_done(cs40l50);
> + if (error)
> + goto out_irq;
> + break;
> + case CS40L50_DSP_MBOX_REDC_EST_START:
> + dev_dbg(cs40l50->dev, "Mailbox: REDC_EST_START\n");
> + break;
> + case CS40L50_DSP_MBOX_REDC_EST_DONE:
> + dev_dbg(cs40l50->dev, "Mailbox: REDC_EST_DONE\n");
> + error = cs40l50_handle_redc_est_done(cs40l50);
> + if (error)
> + goto out_irq;
> + break;
> + case CS40L50_DSP_MBOX_LE_EST_START:
> + dev_dbg(cs40l50->dev, "Mailbox: LE_EST_START\n");
> + break;
> + case CS40L50_DSP_MBOX_LE_EST_DONE:
> + dev_dbg(cs40l50->dev, "Mailbox: LE_EST_DONE\n");
> + break;
> + case CS40L50_DSP_MBOX_AWAKE:
> + dev_dbg(cs40l50->dev, "Mailbox: AWAKE\n");
> + break;
> + case CS40L50_DSP_MBOX_INIT:
> + dev_dbg(cs40l50->dev, "Mailbox: INIT\n");
> + break;
> + case CS40L50_DSP_MBOX_ACK:
> + dev_dbg(cs40l50->dev, "Mailbox: ACK\n");
> + break;
> + case CS40L50_DSP_MBOX_ERR_EVENT_UNMAPPED:
> + dev_err(cs40l50->dev, "Unmapped event\n");
> + break;
> + case CS40L50_DSP_MBOX_ERR_EVENT_MODIFY:
> + dev_err(cs40l50->dev, "Attempt to modify event index failed\n");
> + break;
> + case CS40L50_DSP_MBOX_ERR_NULLPTR:
> + dev_err(cs40l50->dev, "Null pointer\n");
> + break;
> + case CS40L50_DSP_MBOX_ERR_BRAKING:
> + dev_err(cs40l50->dev, "Braking not in progress\n");
> + break;
> + case CS40L50_DSP_MBOX_ERR_INVAL_SRC:
> + dev_err(cs40l50->dev, "Suspend/resume invalid source\n");
> + break;
> + case CS40L50_DSP_MBOX_ERR_ENABLE_RANGE:
> + dev_err(cs40l50->dev, "GPIO enable out of range\n");
> + break;
> + case CS40L50_DSP_MBOX_ERR_GPIO_UNMAPPED:
> + dev_err(cs40l50->dev, "GPIO not mapped\n");
> + break;
> + case CS40L50_DSP_MBOX_ERR_ISR_RANGE:
> + dev_err(cs40l50->dev, "GPIO ISR out of range\n");
> + break;
> + case CS40L50_DSP_MBOX_PERMANENT_SHORT:
> + dev_crit(cs40l50->dev, "Permanent short detected\n");
> + break;
> + case CS40L50_DSP_MBOX_RUNTIME_SHORT:
> + dev_err(cs40l50->dev, "Runtime short detected\n");
> + error = cs40l50_error_release(cs40l50);
> + if (error)
> + goto out_irq;
> + break;
> + default:
> + dev_err(cs40l50->dev, "Mailbox value %#X not recognized\n", val);
> + error = -EINVAL;
> + goto out_irq;
> + }
> + }
> +
> + error = -EIO;
> +
> +out_irq:
> + mutex_unlock(&cs40l50->lock);
> +
> + return IRQ_RETVAL(!error);
> +}
> +
> +
> +static irqreturn_t cs40l50_vddb_uv(int irq, void *data)
> +{
> + struct cs40l50_private *cs40l50 = data;
> +
> + dev_err(cs40l50->dev, "VDD_B undervoltage error\n");
> +
> + return IRQ_RETVAL(!cs40l50_error_release(cs40l50));
> +}
> +
> +static irqreturn_t cs40l50_boost_current_limit(int irq, void *data)
> +{
> + struct cs40l50_private *cs40l50 = data;
> +
> + dev_err(cs40l50->dev, "Boost current limit engagement\n");
> +
> + return IRQ_RETVAL(!cs40l50_error_release(cs40l50));
> +}
> +
> +static irqreturn_t cs40l50_boost_short(int irq, void *data)
> +{
> + struct cs40l50_private *cs40l50 = data;
> +
> + dev_err(cs40l50->dev, "Boost inductor short error\n");
> +
> + return IRQ_RETVAL(!cs40l50_error_release(cs40l50));
> +}
> +
> +static irqreturn_t cs40l50_boost_uv(int irq, void *data)
> +{
> + struct cs40l50_private *cs40l50 = data;
> +
> + dev_err(cs40l50->dev, "Boost undervoltage error\n");
> +
> + return IRQ_RETVAL(!cs40l50_error_release(cs40l50));
> +}
> +
> +static irqreturn_t cs40l50_overtemperature(int irq, void *data)
> +{
> + struct cs40l50_private *cs40l50 = data;
> +
> + dev_err(cs40l50->dev, "Overtemperature error\n");
> +
> + return IRQ_RETVAL(!cs40l50_error_release(cs40l50));
> +}
> +
> +static irqreturn_t cs40l50_amp_short(int irq, void *data)
> +{
> + struct cs40l50_private *cs40l50 = data;
> +
> + dev_err(cs40l50->dev, "Amp short error\n");
> +
> + return IRQ_RETVAL(!cs40l50_error_release(cs40l50));
> +}
> +
> +static irqreturn_t cs40l50_global_error(int irq, void *data)
> +{
> + struct cs40l50_private *cs40l50 = data;
> +
> + dev_err(cs40l50->dev, "MSM chip error\n");
> +
> + return IRQ_RETVAL(!cs40l50_error_release(cs40l50));
> +}
> +
> +static const struct cs40l50_irq cs40l50_irqs[] = {
> + CS40L50_IRQ(CS40L50_AMP_SHORT_IRQ, "Amp short", cs40l50_amp_short),
> + CS40L50_IRQ(CS40L50_VIRT2_MBOX_IRQ, "Firmware interrupt", cs40l50_handle_mbox_buffer),
> + CS40L50_IRQ(CS40L50_TEMP_ERR_IRQ, "Overtemperature", cs40l50_overtemperature),
> + CS40L50_IRQ(CS40L50_BST_UVP_IRQ, "Boost undervoltage", cs40l50_boost_uv),
> + CS40L50_IRQ(CS40L50_BST_SHORT_IRQ, "Boost short", cs40l50_boost_short),
> + CS40L50_IRQ(CS40L50_BST_ILIMIT_IRQ, "Boost current limit", cs40l50_boost_current_limit),
> + CS40L50_IRQ(CS40L50_UVLO_VDDBATT_IRQ, "VDD_B undervoltage", cs40l50_vddb_uv),
> + CS40L50_IRQ(CS40L50_GLOBAL_ERROR_IRQ, "Global error", cs40l50_global_error),
> +};
> +
> +static const struct regmap_irq cs40l50_reg_irqs[] = {
> + CS40L50_REG_IRQ(IRQ1_INT_1, AMP_SHORT),
> + CS40L50_REG_IRQ(IRQ1_INT_2, VIRT2_MBOX),
> + CS40L50_REG_IRQ(IRQ1_INT_8, TEMP_ERR),
> + CS40L50_REG_IRQ(IRQ1_INT_9, BST_UVP),
> + CS40L50_REG_IRQ(IRQ1_INT_9, BST_SHORT),
> + CS40L50_REG_IRQ(IRQ1_INT_9, BST_ILIMIT),
> + CS40L50_REG_IRQ(IRQ1_INT_10, UVLO_VDDBATT),
> + CS40L50_REG_IRQ(IRQ1_INT_18, GLOBAL_ERROR),
> +};
> +
> +static const struct regmap_irq_chip cs40l50_regmap_irq_chip = {
> + .name = "cs40l50 IRQ1 Controller",
> + .status_base = CS40L50_IRQ1_INT_1,
> + .mask_base = CS40L50_IRQ1_MASK_1,
> + .ack_base = CS40L50_IRQ1_INT_1,
> + .num_regs = 19,
> + .irqs = cs40l50_reg_irqs,
> + .num_irqs = ARRAY_SIZE(cs40l50_reg_irqs),
> + .runtime_pm = true,
> +};
> +
> +static const struct reg_sequence cs40l50_ext_bst_seq[] = {
> + { CS40L50_BLOCK_ENABLES, CS40L50_MON_ENABLE },
> + { CS40L50_MON_VALUE_CTRL, CS40L50_VBST_VDD_AMP },
> + { CS40L50_TEST_KEY, CS40L50_KEY_SET_1 },
> + { CS40L50_TEST_KEY, CS40L50_KEY_SET_2 },
> + { CS40L50_TEST_VIS_SPARE, CS40L50_TEMP_SENSOR_3 },
> + { CS40L50_MON_ERROR_OVERIDE, CS40L50_TEMP_ERR_THLD },
> + { CS40L50_STATUS, CS40L50_TEMP_WARN_THLD },
> + { CS40L50_TEST_CONFIG, CS40L50_TEMP_FILT_DEBOUNCE_OFF },
> + { CS40L50_TEMP_WARN_CONFIG, CS40L50_TEMP_WARN_ATK_VOL },
> +};
> +
> +static int cs40l50_config_bst(struct cs40l50_private *cs40l50)
> +{
> + int error, i;
> +
> + if (device_property_present(cs40l50->dev, "cirrus,external-boost")) {
> + error = regmap_multi_reg_write(cs40l50->regmap, cs40l50_ext_bst_seq,
> + ARRAY_SIZE(cs40l50_ext_bst_seq));
> + if (error)
> + return error;
> +
> + for (i = 0; i < ARRAY_SIZE(cs40l50_ext_bst_seq); i++) {
> + error = cs40l50_pseq_write(cs40l50, cs40l50_ext_bst_seq[i].reg,
> + cs40l50_ext_bst_seq[i].def);
> + if (error)
> + return error;
> + }
> +
> + return 0;
> + }
> +
> + /* Internal boost config */
This driver is conspicuously void of comments and documentation; why is
this line mysterious enough to warrant one?
> + return cs40l50_dsp_write(cs40l50, CS40L50_BST_LPMODE_SEL, CS40L50_DCM_LOW_POWER);
> +}
> +
> +static int cs40l50_patch_firmware(struct cs40l50_private *cs40l50)
> +{
> + const struct firmware *bin = NULL, *wmfw = NULL;
> + int error = 0;
> +
> + if (request_firmware(&bin, "cs40l50.bin", cs40l50->dev))
> + dev_info(cs40l50->dev, "Could not request wavetable file: %d\n", error);
> +
> + if (request_firmware(&wmfw, "cs40l50.wmfw", cs40l50->dev))
> + dev_info(cs40l50->dev, "Could not request firmware patch file: %d\n", error);
> +
> + if (wmfw) {
It is a much more common design pattern to bail if request_firmware() returns
an error, than to proceed and check against the FW pointer remaining NULL.
Is this done because cs_dsp_power_up() must be called, with or without a wmfw
or coefficient file being available?
> + error = cs40l50_dsp_write(cs40l50, CS40L50_CCM_CORE_CONTROL,
> + CS40L50_DSP1_CLOCK_DISABLE);
> + if (error)
> + goto err_fw;
> +
> + error = cs40l50_dsp_write(cs40l50, CS40L50_RAM_INIT,
> + CS40L50_DSP1_RAM_INIT_FLAG);
> + if (error)
> + goto err_fw;
> +
> + error = cs40l50_dsp_write(cs40l50, CS40L50_PWRMGT_CTL,
> + CS40L50_DSP1_MEM_RDY_HW);
> + if (error)
> + goto err_fw;
> + }
> +
> + mutex_lock(&cs40l50->lock);
> +
> + cs_dsp_power_up(&cs40l50->dsp, wmfw, "cs40l50.wmfw", bin, "cs40l50.bin", "cs40l50");
> +
> + mutex_unlock(&cs40l50->lock);
> +
> + if (wmfw)
> + error = cs40l50_dsp_write(cs40l50, CS40L50_CCM_CORE_CONTROL,
> + CS40L50_DSP1_CLOCK_ENABLE);
> +
> +err_fw:
> + release_firmware(wmfw);
> + release_firmware(bin);
> +
> + return error;
> +}
> +
> +static int cs40l50_pseq_init(struct cs40l50_private *cs40l50)
> +{
> + struct cs40l50_pseq_op *pseq_op;
> + int error, i, num_words;
> + u8 operation;
> + u32 *words;
> +
> + INIT_LIST_HEAD(&cs40l50->pseq_op_head);
> +
> + words = kcalloc(CS40L50_PSEQ_MAX_WORDS, sizeof(u32), GFP_KERNEL);
> + if (IS_ERR_OR_NULL(words))
> + return -ENOMEM;
Just curious what is the need to call IS_ERR_OR_NULL() here and throughout
after allocation rather than just !foo ?
> +
> + error = regmap_bulk_read(cs40l50->regmap, CS40L50_DSP1_POWER_ON_SEQ, words,
> + CS40L50_PSEQ_MAX_WORDS);
> + if (error)
> + goto err_free;
> +
> + for (i = 0; i < CS40L50_PSEQ_MAX_WORDS; i++)
> + words[i] = be32_to_cpu(words[i]);
> +
> + for (i = 0; i < CS40L50_PSEQ_MAX_WORDS; i += num_words) {
> + operation = (words[i] & CS40L50_PSEQ_OP_MASK) >> CS40L50_PSEQ_OP_SHIFT;
> +
> + switch (operation) {
> + case CS40L50_PSEQ_OP_END:
> + num_words = CS40L50_PSEQ_OP_END_WORDS;
> + break;
> + case CS40L50_PSEQ_OP_WRITE_ADDR8:
> + case CS40L50_PSEQ_OP_WRITE_H16:
> + case CS40L50_PSEQ_OP_WRITE_L16:
> + num_words = CS40L50_PSEQ_OP_WRITE_X16_WORDS;
> + break;
> + case CS40L50_PSEQ_OP_WRITE_FULL:
> + num_words = CS40L50_PSEQ_OP_WRITE_FULL_WORDS;
> + break;
> + default:
> + dev_err(cs40l50->dev, "Invalid OP code 0x%02X\n", operation);
> + error = -EINVAL;
> + goto err_free;
> + }
> +
> + pseq_op = devm_kzalloc(cs40l50->dev, sizeof(*pseq_op), GFP_KERNEL);
> + if (!pseq_op) {
> + error = -ENOMEM;
> + goto err_free;
> + }
> +
> + memcpy(pseq_op->words, &words[i], num_words * sizeof(u32));
> + pseq_op->size = num_words;
> + pseq_op->offset = i * sizeof(u32);
> + pseq_op->operation = operation;
> + list_add(&pseq_op->list, &cs40l50->pseq_op_head);
> +
> + if (operation == CS40L50_PSEQ_OP_END)
> + break;
> + }
> +
> + if (operation != CS40L50_PSEQ_OP_END) {
> + dev_err(cs40l50->dev, "PSEQ_END_OF_SCRIPT not found\n");
> + error = -ENOENT;
> + }
> +
> +err_free:
> + kfree(words);
> +
> + return error;
> +}
> +
> +static int cs40l50_input_init(struct cs40l50_private *cs40l50)
> +{
> + int error;
> + u32 val;
> +
> + error = cs40l50_dsp_read(cs40l50, CS40L50_DEVID, &val);
> + if (error)
> + return error;
> +
> + if (val != CS40L50_DEVID_A) {
> + dev_err(cs40l50->dev, "Invalid device ID: %#010X\n", val);
> + return -EINVAL;
> + }
> +
> + cs40l50->input->id.product = val & 0xFFFF;
> +
> + error = cs40l50_dsp_read(cs40l50, CS40L50_REVID, &val);
> + if (error)
> + return error;
> +
> + if (val != CS40L50_REVID_B0) {
> + dev_err(cs40l50->dev, "Invalid device revision: %#04X\n", val);
> + return -EINVAL;
> + }
> +
> + cs40l50->input->id.version = val;
> +
> + cs40l50->input = devm_input_allocate_device(cs40l50->dev);
> + if (IS_ERR_OR_NULL(cs40l50->input))
> + return cs40l50->input ? PTR_ERR(cs40l50->input) : -ENOMEM;
> +
> + cs40l50->input->name = "cs40l50_input";
> + input_set_drvdata(cs40l50->input, cs40l50);
> + input_set_capability(cs40l50->input, EV_FF, FF_PERIODIC);
> + input_set_capability(cs40l50->input, EV_FF, FF_CUSTOM);
> +
> + error = input_ff_create(cs40l50->input, FF_MAX_EFFECTS);
> + if (error)
> + return error;
> +
> + clear_bit(FF_RUMBLE, cs40l50->input->ffbit);
If there is a HW limitation that prevents this capability, it would be
handy to explain as much with a comment.
> +
> + cs40l50->input->ff->upload = cs40l50_add_effect;
> + cs40l50->input->ff->playback = cs40l50_playback_effect;
> + cs40l50->input->ff->erase = cs40l50_erase_effect;
> +
> + INIT_LIST_HEAD(&cs40l50->effect_head);
> +
> + return input_register_device(cs40l50->input);
> +}
> +
> +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 },
> +};
This is more readable as:
[...],
{
.type = ...,
.base = ...,
},
[...],
> +
> +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_DSP1_CORE_BASE;
> + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID;
> + cs40l50->dsp.mem = cs40l50_dsp_regions;
> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
> + cs40l50->dsp.lock_regions = 0xFFFFFFFF;
> + cs40l50->dsp.no_core_startstop = true;
> + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops;
> +
> + return cs_dsp_halo_init(&cs40l50->dsp);
> +}
> +
> +int cs40l50_probe(struct cs40l50_private *cs40l50)
> +{
> + int error, i, irq;
> + u32 val;
> +
> + mutex_init(&cs40l50->lock);
> +
> + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies),
> + cs40l50_supplies);
> + if (error)
> + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n");
> +
> + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies);
> + if (error)
> + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n");
> +
> + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(cs40l50->reset_gpio)) {
> + error = PTR_ERR(cs40l50->reset_gpio);
> + goto err;
> + }
Krzysztof covered this already, but I just want to re-iterate that you are
kind of swallowing your own head by calling cs40l50_remove() in your error
path. This is a confusing and uncommon design pattern.
You cannot have possibly booted the DSP or allocated work here, so why not
just return?
> +
> + usleep_range(CS40L50_MIN_RESET_PULSE_WIDTH_US, CS40L50_MIN_RESET_PULSE_WIDTH_US + 100);
> +
> + gpiod_set_value_cansleep(cs40l50->reset_gpio, 0);
> +
> + usleep_range(CS40L50_CP_READY_DELAY_US, CS40L50_CP_READY_DELAY_US + 1000);
> +
> + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) {
> + error = cs40l50_dsp_read(cs40l50, CS40L50_DSP1_HALO_STATE, &val);
> + if (!error && val < 0xFFFF && val >= CS40L50_HALO_STATE_BOOT_DONE)
> + break;
> +
> + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100);
> + }
> +
> + if (i == CS40L50_DSP_TIMEOUT_COUNT) {
> + dev_err(cs40l50->dev, "Firmware boot failed: %d, halo state = %#x\n", error, val);
> + goto err;
> + }
> +
> + cs40l50->vibe_workqueue = alloc_ordered_workqueue("cs40l50_workqueue", WQ_HIGHPRI);
> + if (!cs40l50->vibe_workqueue) {
> + error = -ENOMEM;
> + goto err;
> + }
> +
> + INIT_WORK(&cs40l50->vibe_start_work, cs40l50_vibe_start_worker);
> + INIT_WORK(&cs40l50->vibe_stop_work, cs40l50_vibe_stop_worker);
> +
> + error = cs40l50_cs_dsp_init(cs40l50);
> + if (error)
> + goto err;
It seems your error path is complex because you intend to unwind the DSP
in case of an error, in which case any teardown should be explicitly
called from the bottom of probe() rather than calling remove() on your
own.
Opinions may differ here, but mine is that you need not go out of your
way to power down the DSP if glaring errors are encountered during probe.
If you cannot read HALO_STATE, do you really expect to be able to do any
additional work anyway?
Understanding that haptics bears the risk of a motor stuck on, perhaps it
is sufficient to simply throw the device into reset if things go afoul?
> +
> + error = cs40l50_input_init(cs40l50);
> + if (error)
> + goto err;
> +
> + error = cs40l50_patch_firmware(cs40l50);
> + if (error)
> + goto err;
I just want to re-iterate Charles' comment; this _will_not_ work across
all customer platforms because you cannot guarantee the filesystem is
ready here if the driver is built-in.
In the best case you will get -ENOENT right away; in the worst case you
will get a deadlock. If this device does not have nonvolatile memory and
you must load FW from probe, use request_firmware_nowait() instead.
> +
> + error = cs40l50_pseq_init(cs40l50);
> + if (error)
> + goto err;
> +
> + error = cs40l50_config_bst(cs40l50);
> + if (error)
> + goto err;
> +
> + error = devm_regmap_add_irq_chip(cs40l50->dev, cs40l50->regmap, cs40l50->irq,
> + IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW, 0, &cs40l50_regmap_irq_chip,
> + &cs40l50->irq_data);
We should not be mandating IRQ polarity in the driver. Sometimes the customer
may use a GPIO expander with inverted polarity; we need to let dts govern the
polarity.
> + if (error) {
> + dev_err(cs40l50->dev, "Failed to register IRQ chip: %d\n", error);
> + goto err;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(cs40l50_irqs); i++) {
> + irq = regmap_irq_get_virq(cs40l50->irq_data, cs40l50_irqs[i].irq);
> + if (irq < 0) {
> + dev_err(cs40l50->dev, "Failed to get %s\n", cs40l50_irqs[i].name);
> + error = irq;
> + goto err;
> + }
> +
> + error = devm_request_threaded_irq(cs40l50->dev, irq, NULL, cs40l50_irqs[i].handler,
> + IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW,
> + cs40l50_irqs[i].name, cs40l50);
> + if (error) {
> + dev_err(cs40l50->dev, "Failed to request IRQ %s: %d\n",
> + cs40l50_irqs[i].name, error);
> + goto err;
> + }
> + }
> +
> + return 0;
> +
> +err:
> + cs40l50_remove(cs40l50);
> +
> + return error;
> +}
> +EXPORT_SYMBOL_GPL(cs40l50_probe);
> +
> +int cs40l50_remove(struct cs40l50_private *cs40l50)
> +{
> + disable_irq(cs40l50->irq);
> +
> + if (cs40l50->dsp.booted)
> + cs_dsp_power_down(&cs40l50->dsp);
> + if (&cs40l50->dsp)
> + cs_dsp_remove(&cs40l50->dsp);
> +
> + if (cs40l50->vibe_workqueue) {
> + flush_workqueue(cs40l50->vibe_workqueue);
> + destroy_workqueue(cs40l50->vibe_workqueue);
> + }
> +
> + gpiod_set_value_cansleep(cs40l50->reset_gpio, 1);
> + regulator_bulk_disable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(cs40l50_remove);
> +
> +MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@...rus.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/input/cs40l50.h b/include/linux/input/cs40l50.h
> new file mode 100644
> index 000000000000..f2c8a101fa00
> --- /dev/null
> +++ b/include/linux/input/cs40l50.h
> @@ -0,0 +1,321 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * CS40L50 Advanced Haptic Driver with Waveform Memory DSP
> + * and Closed-Loop Algorithms
> + *
> + * Copyright 2023 Cirrus Logic, Inc.
> + *
> + */
> +#ifndef __CS40L50_H__
> +#define __CS40L50_H__
> +
> +#include <linux/bitops.h>
bitops.h is quite outdated; did you mean bits.h?
> +#include <linux/gpio/consumer.h>
> +#include <linux/input.h>
> +#include <linux/regmap.h>
> +#include <linux/interrupt.h>
> +#include <linux/firmware/cirrus/cs_dsp.h>
> +#include <linux/firmware/cirrus/wmfw.h>
> +
> +/* Boost */
> +#define CS40L50_TEST_KEY 0x40
> +#define CS40L50_BLOCK_ENABLES 0x2018
> +#define CS40L50_ERROR_RELEASE 0x2034
> +#define CS40L50_PWRMGT_CTL 0x2900
> +#define CS40L50_BST_LPMODE_SEL 0x3810
> +#define CS40L50_STATUS 0x4200
> +#define CS40L50_MON_ERROR_OVERIDE 0x4220
> +#define CS40L50_TEST_CONFIG 0x4240
> +#define CS40L50_MON_VALUE_CTRL 0x4404
> +#define CS40L50_TEST_VIS_SPARE 0x5C00
> +#define CS40L50_TEMP_WARN_CONFIG 0x6024
> +#define CS40L50_KEY_SET_1 0x55
> +#define CS40L50_KEY_SET_2 0xAA
> +#define CS40L50_TEMP_SENSOR_3 0x400
> +#define CS40L50_DCM_LOW_POWER 0x1
> +#define CS40L50_MON_ENABLE 0x3201
> +#define CS40L50_VBST_VDD_AMP 0x1000000
> +#define CS40L50_TEMP_ERR_THLD 0x8000007D
> +#define CS40L50_TEMP_WARN_THLD 0x8
> +#define CS40L50_TEMP_FILT_DEBOUNCE_OFF 0x510002B5
> +#define CS40L50_TEMP_WARN_ATK_VOL 0x522303
> +
> +/* Interrupts */
> +#define CS40L50_IRQ1_INT_1 0xE010
> +#define CS40L50_IRQ1_INT_2 0xE014
> +#define CS40L50_IRQ1_INT_3 0xE018
> +#define CS40L50_IRQ1_INT_4 0xE01C
> +#define CS40L50_IRQ1_INT_8 0xE02C
> +#define CS40L50_IRQ1_INT_9 0xE030
> +#define CS40L50_IRQ1_INT_10 0xE034
> +#define CS40L50_IRQ1_INT_18 0xE054
> +#define CS40L50_IRQ1_INT_19 0xE058
> +#define CS40L50_IRQ1_MASK_1 0xE090
> +#define CS40L50_IRQ1_MASK_2 0xE094
> +#define CS40L50_IRQ1_MASK_3 0xE098
> +#define CS40L50_IRQ1_MASK_8 0xE0AC
> +#define CS40L50_IRQ1_MASK_9 0xE0B0
> +#define CS40L50_IRQ1_MASK_10 0xE0B4
> +#define CS40L50_IRQ1_MASK_18 0xE0D4
> +#define CS40L50_IRQ1_MASK_19 0xE0D8
> +#define CS40L50_IRQ1_STS_1 0xE210
> +#define CS40L50_IRQ1_STS_2 0xE214
> +#define CS40L50_IRQ1_STS_3 0xE218
> +#define CS40L50_IRQ1_STS_8 0xE22C
> +#define CS40L50_IRQ1_STS_9 0xE230
> +#define CS40L50_IRQ1_STS_10 0xE234
> +#define CS40L50_IRQ1_STS_18 0xE254
> +#define CS40L50_IRQ1_STS_19 0xE258
> +#define CS40L50_AMP_SHORT_MASK BIT(31)
> +#define CS40L50_VIRT2_MBOX_MASK BIT(21)
> +#define CS40L50_TEMP_ERR_MASK BIT(31)
> +#define CS40L50_BST_UVP_MASK BIT(6)
> +#define CS40L50_BST_SHORT_MASK BIT(7)
> +#define CS40L50_BST_ILIMIT_MASK BIT(8)
> +#define CS40L50_UVLO_VDDBATT_MASK BIT(16)
> +#define CS40L50_GLOBAL_ERROR_MASK BIT(15)
> +#define CS40L50_ERROR_RELEASE_MASK BIT(11)
> +#define CS40L50_BOOT_DONE_MASK BIT(1)
> +#define CS40L50_IRQ(_irq, _name, _hand) \
> + { \
> + .irq = _irq, \
> + .name = _name, \
> + .handler = _hand, \
> + }
> +#define CS40L50_REG_IRQ(_reg, _irq) \
> + [CS40L50_ ## _irq ## _IRQ] = { \
> + .reg_offset = (CS40L50_ ## _reg) - CS40L50_IRQ1_INT_1, \
> + .mask = CS40L50_ ## _irq ## _MASK \
> + }
> +
> +/* Mailbox Inbound Commands */
> +#define CS40L50_RAM_BANK_INDEX_START 0x1000000
> +#define CS40L50_RAM_BANK_INDEX_END 0x100007F
> +#define CS40L50_OWT_INDEX_START 0x1400000
> +#define CS40L50_OWT_INDEX_END 0x140007F
> +#define CS40L50_ROM_BANK_INDEX_START 0x1800000
> +#define CS40L50_ROM_BANK_INDEX_END 0x180001A
> +#define CS40L50_OTP_BUZZ 0x1800080
> +#define CS40L50_RAM_BUZZ_INDEX_START 0x1800081
> +#define CS40L50_RAM_BUZZ_INDEX_END 0x1800085
> +#define CS40L50_MBOX_CMD_HIBERNATE 0x2000001
> +#define CS40L50_MBOX_CMD_WAKEUP 0x2000002
> +#define CS40L50_MBOX_CMD_PREVENT_HIBERNATE 0x2000003
> +#define CS40L50_MBOX_CMD_ALLOW_HIBERNATION 0x2000004
> +#define CS40L50_MBOX_CMD_SHUTDOWN 0x2000005
> +#define CS40L50_MBOX_CMD_DURATION_REPORT 0x3000001
> +#define CS40L50_MBOX_CMD_START_I2S 0x3000002
> +#define CS40L50_MBOX_CMD_STOP_I2S 0x3000003
> +#define CS40L50_MBOX_CMD_HAPTICS_LOGGER_RESET 0x3000004
> +#define CS40L50_MBOX_CMD_OWT_PUSH 0x3000008
> +#define CS40L50_MBOX_CMD_OWT_RESET 0x3000009
> +#define CS40L50_MBOX_CMD_DVL_REINIT 0x300000A
> +#define CS40L50_MBOX_CMD_STOP_PLAYBACK 0x5000000
> +#define CS40L50_MBOX_CMD_F0_EST 0x7000001
> +#define CS40L50_MBOX_CMD_REDC_EST 0x7000002
> +#define CS40L50_MBOX_CMD_LE_EST 0x7000004
> +#define CS40L50_MBOX_CMD_OWT_DELETE 0xD000000
> +
> +/* Mailbox Outbound Commands */
> +#define CS40L50_MBOX_QUEUE_BASE 0x11004
> +#define CS40L50_MBOX_QUEUE_END 0x1101C
> +#define CS40L50_DSP_VIRTUAL1_MBOX_1 0x11020
> +#define CS40L50_DSP1_MBOX_QUEUE_WT 0x28042C8
> +#define CS40L50_DSP1_MBOX_QUEUE_RD 0x28042CC
> +#define CS40L50_DSP_MBOX_HAPTIC_COMPLETE_MBOX 0x1000000
> +#define CS40L50_DSP_MBOX_HAPTIC_COMPLETE_GPIO 0x1000001
> +#define CS40L50_DSP_MBOX_HAPTIC_COMPLETE_I2S 0x1000002
> +#define CS40L50_DSP_MBOX_HAPTIC_TRIGGER_MBOX 0x1000010
> +#define CS40L50_DSP_MBOX_HAPTIC_TRIGGER_GPIO 0x1000011
> +#define CS40L50_DSP_MBOX_HAPTIC_TRIGGER_I2S 0x1000012
> +#define CS40L50_DSP_MBOX_INIT 0x2000000
> +#define CS40L50_DSP_MBOX_AWAKE 0x2000002
> +#define CS40L50_DSP_MBOX_F0_EST_START 0x7000011
> +#define CS40L50_DSP_MBOX_F0_EST_DONE 0x7000021
> +#define CS40L50_DSP_MBOX_REDC_EST_START 0x7000012
> +#define CS40L50_DSP_MBOX_REDC_EST_DONE 0x7000022
> +#define CS40L50_DSP_MBOX_LE_EST_START 0x7000014
> +#define CS40L50_DSP_MBOX_LE_EST_DONE 0x7000024
> +#define CS40L50_DSP_MBOX_ACK 0xA000000
> +#define CS40L50_DSP_MBOX_PANIC 0xC000000
> +#define CS40L50_DSP_MBOX_WATERMARK 0xD000000
> +#define CS40L50_DSP_MBOX_ERR_EVENT_UNMAPPED 0xC0004B3
> +#define CS40L50_DSP_MBOX_ERR_EVENT_MODIFY 0xC0004B4
> +#define CS40L50_DSP_MBOX_ERR_NULLPTR 0xC0006A5
> +#define CS40L50_DSP_MBOX_ERR_BRAKING 0xC0006A8
> +#define CS40L50_DSP_MBOX_ERR_INVAL_SRC 0xC0006AC
> +#define CS40L50_DSP_MBOX_ERR_ENABLE_RANGE 0xC000836
> +#define CS40L50_DSP_MBOX_ERR_GPIO_UNMAPPED 0xC000837
> +#define CS40L50_DSP_MBOX_ERR_ISR_RANGE 0xC000838
> +#define CS40L50_DSP_MBOX_PERMANENT_SHORT 0xC000C1C
> +#define CS40L50_DSP_MBOX_RUNTIME_SHORT 0xC000C1D
> +
> +/* DSP */
> +#define CS40L50_DSP1_XMEM_PACKED_0 0x2000000
> +#define CS40L50_DSP1_XMEM_UNPACKED32_0 0x2400000
> +#define CS40L50_DSP1_SYS_INFO_ID 0x25E0000
> +#define CS40L50_DSP1_XMEM_UNPACKED24_0 0x2800000
> +#define CS40L50_NUM_OF_WAVES 0x280CB4C
> +#define CS40L50_DSP1_CORE_BASE 0x2B80000
> +#define CS40L50_DSP1_SCRATCH1 0x2B805C0
> +#define CS40L50_DSP1_SCRATCH2 0x2B805C8
> +#define CS40L50_DSP1_SCRATCH3 0x2B805D0
> +#define CS40L50_DSP1_SCRATCH4 0x2B805D8
I didn't look too closely, but some of these #defines are unused.
> +#define CS40L50_CCM_CORE_CONTROL 0x2BC1000
> +#define CS40L50_RAM_INIT 0x28021DC
> +#define CS40L50_DSP1_YMEM_PACKED_0 0x2C00000
> +#define CS40L50_DSP1_YMEM_UNPACKED32_0 0x3000000
> +#define CS40L50_DSP1_YMEM_UNPACKED24_0 0x3400000
> +#define CS40L50_DSP1_PMEM_0 0x3800000
> +#define CS40L50_DSP1_PMEM_5114 0x3804FE8
> +#define CS40L50_DSP1_MEM_RDY_HW 0x2
> +#define CS40L50_DSP1_RAM_INIT_FLAG 0x1
> +#define CS40L50_DSP1_CLOCK_DISABLE 0x80
> +#define CS40L50_DSP1_CLOCK_ENABLE 0x281
> +#define CS40L50_DSP_POLL_US 1000
> +#define CS40L50_DSP_TIMEOUT_COUNT 100
> +#define CS40L50_MBOX_POLL_US 5000
> +
> +/* Calibration */
> +#define CS40L50_DSP1_REDC_ESTIMATION 0x2802F7C
> +#define CS40L50_DSP1_F0_ESTIMATION 0x2802F84
> +#define CS40L50_DSP1_DYNAMIC_F0_ENABLE 0x2802F8C
> +#define CS40L50_DSP1_DYNAMIC_F0_THRESHOLD 0x2802F90
> +#define CS40L50_DSP1_F0_STORED 0x2805C00
> +#define CS40L50_DSP1_REDC_STORED 0x2805C04
> +#define CS40L50_DSP1_RE_EST_STATUS 0x3401B40
> +#define CS40L50_REDC_EST_DELAY_US 30000
> +#define CS40L50_F0_EST_DELAY_US 10000
> +
> +/* OWT */
> +#define CS40L50_DSP1_OWT_SIZE 0x2805C38
> +#define CS40L50_DSP1_OWT_NEXT 0x2805C3C
> +#define CS40L50_NUM_OF_OWT_WAVES 0x2805C40
> +#define CS40L50_DSP1_OWT_BASE 0x2805C34
> +#define CS40L50_MAX_PWLE_SECTIONS 126
> +#define CS40L50_CUSTOM_DATA_SIZE 2
> +#define CS40L50_WT_HEADER_PWLE_SIZE 12
> +#define CS40L50_WT_HEADER_DEFAULT_FLAGS 0x0000
> +#define CS40L50_WT_HEADER_OFFSET 3
> +#define CS40L50_WT_TYPE_PCM 8
> +#define CS40L50_WT_TYPE_PWLE 12
> +#define CS40L50_PCM_ID 0x0000
> +
> +/* GPIO */
> +#define CS40L50_DSP1_GPIO_BASE 0x2804140
> +#define CS40L50_GPIO_MAPPING_INVALID 0
> +#define CS40L50_GPI_DISABLE 0x1FF
> +#define CS40L50_BTN_NUM_MASK GENMASK(14, 12)
> +#define CS40L50_BTN_EDGE_MASK BIT(15)
> +
> +/* State */
> +#define CS40L50_DSP1_HALO_STATE 0x28021E0
> +#define CS40L50_HALO_STATE_BOOT_DONE 2
> +#define CS40L50_MIN_RESET_PULSE_WIDTH_US 1100
> +#define CS40L50_CP_READY_DELAY_US 2200
> +
> +/* Device */
> +#define CS40L50_DEVID 0x0
> +#define CS40L50_REVID 0x4
> +#define CS40L50_DEVID_A 0x40A50
> +#define CS40L50_REVID_B0 0xB0
> +
> +/* Write sequencer */
> +#define CS40L50_DSP1_POWER_ON_SEQ 0x280C320
> +#define CS40L50_PSEQ_MAX_WORDS 200
> +#define CS40L50_PSEQ_OP_MASK GENMASK(23, 16)
> +#define CS40L50_PSEQ_OP_SHIFT 16
> +#define CS40L50_PSEQ_OP_WRITE_ADDR8 0x02
> +#define CS40L50_PSEQ_OP_WRITE_L16 0x04
> +#define CS40L50_PSEQ_OP_WRITE_H16 0x05
> +#define CS40L50_PSEQ_OP_WRITE_FULL 0x00
> +#define CS40L50_PSEQ_OP_WRITE_FULL_WORDS 3
> +#define CS40L50_PSEQ_OP_WRITE_X16_WORDS 2
> +#define CS40L50_PSEQ_OP_END 0xFF
> +#define CS40L50_PSEQ_OP_END_WORDS 1
> +#define CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_SHIFT 8
> +#define CS40L50_PSEQ_WRITE_FULL_UPPER_ADDR_SHIFT 16
> +#define CS40L50_PSEQ_WRITE_FULL_LOWER_ADDR_MASK GENMASK(15, 0)
> +#define CS40L50_PSEQ_WRITE_FULL_UPPER_DATA_SHIFT 24
> +#define CS40L50_PSEQ_WRITE_FULL_LOWER_DATA_MASK GENMASK(23, 0)
> +
> +enum cs40l50_wvfrm_bank {
> + CS40L50_WVFRM_BANK_RAM,
> + CS40L50_WVFRM_BANK_ROM,
> + CS40L50_WVFRM_BANK_OWT,
> + CS40L50_WVFRM_BANK_BUZ,
> + CS40L50_WVFRM_BANK_NUM,
> +};
> +
> +enum cs40l50_irq_list {
> + CS40L50_GLOBAL_ERROR_IRQ,
> + CS40L50_UVLO_VDDBATT_IRQ,
> + CS40L50_BST_ILIMIT_IRQ,
> + CS40L50_BST_SHORT_IRQ,
> + CS40L50_BST_UVP_IRQ,
> + CS40L50_TEMP_ERR_IRQ,
> + CS40L50_VIRT2_MBOX_IRQ,
> + CS40L50_AMP_SHORT_IRQ,
> + CS40L50_NUM_IRQ
> +};
> +
> +union cs40l50_buzzgen {
> + u32 words[3];
> + struct {
> + u32 freq;
> + u32 level;
> + u32 duration;
> + } params;
> +};
> +
> +struct cs40l50_owt_header {
> + u32 wvfrm_type;
> + u32 offset;
> + u32 data_size;
> +};
> +
> +struct cs40l50_irq {
> + int irq;
> + const char *name;
> + irqreturn_t (*handler)(int irq, void *data);
> +};
> +
> +struct cs40l50_effect {
> + int id;
> + u32 trigger_index;
> + enum cs40l50_wvfrm_bank wvfrm_bank;
> + u32 mapping;
> + struct list_head list;
> +};
> +
> +struct cs40l50_pseq_op {
> + u16 offset;
> + u8 operation;
> + u32 words[3];
> + u32 size;
> + struct list_head list;
> +};
> +
> +struct cs40l50_private {
> + struct device *dev;
> + struct regmap *regmap;
> + struct cs_dsp dsp;
> + struct mutex lock;
> + struct gpio_desc *reset_gpio;
> + struct input_dev *input;
> + struct list_head effect_head;
> + struct ff_effect *trigger_effect;
> + struct workqueue_struct *vibe_workqueue;
> + struct work_struct vibe_start_work;
> + struct work_struct vibe_stop_work;
> + struct regmap_irq_chip_data *irq_data;
> + struct list_head pseq_op_head;
> + int irq;
> +};
> +
> +int cs40l50_probe(struct cs40l50_private *cs40l50);
> +int cs40l50_remove(struct cs40l50_private *cs40l50);
> +
> +extern const struct regmap_config cs40l50_regmap;
> +
> +#endif /* __CS40L50_H__ */
> --
> 2.25.1
>
Kind regards,
Jeff LaBundy
Powered by blists - more mailing lists