* [PATCH v9 0/5] Add support for CS40L50
@ 2024-03-08 22:24 James Ogletree
2024-03-08 22:24 ` [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface James Ogletree
` (4 more replies)
0 siblings, 5 replies; 22+ messages in thread
From: James Ogletree @ 2024-03-08 22:24 UTC (permalink / raw
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, jeff
Cc: patches, linux-sound, linux-input, devicetree, James Ogletree
Changes in v9:
- Fixed empty struct by utilizing cs_dsp's post_run callback
- Style fixes in MFD driver
Changes in v8:
- set_sysclk() -> set_bclk_ratio()
- Added ID table to codec driver
- Style improvements
- Fixed ordering of new write sequence operations
Changes in v7:
- Fixed sparse warning
- Moved write sequences to private data structure
- Logical and style improvements in write sequence interface
Changes in v6:
- Updated write sequencer interface to be control-name based
- Fixed a race condition and non-handling of repeats in playback callback
- Stylistic and logical improvements all around
Changes in v5:
- Added a codec sub-device to support I2S streaming
- Moved write sequencer code from cirrus_haptics to cs_dsp
- Reverted cirrus_haptics library; future Cirrus input
drivers will export and utilize cs40l50_vibra functions
- Added more comments
- Many small stylistic and logical improvements
Changes in v4:
- Moved from Input to MFD
- Moved common Cirrus haptic functions to a library
- Incorporated runtime PM framework
- Many style improvements
Changes in v3:
- YAML formatting corrections
- Fixed typo in MAINTAINERS
- Used generic node name "haptic-driver"
- Fixed probe error code paths
- Switched to "sizeof(*)"
- Removed tree reference in MAINTAINERS
Changes in v2:
- Fixed checkpatch warnings
James Ogletree (5):
firmware: cs_dsp: Add write sequencer interface
dt-bindings: input: cirrus,cs40l50: Add initial DT binding
mfd: cs40l50: Add support for CS40L50 core driver
Input: cs40l50 - Add support for the CS40L50 haptic driver
ASoC: cs40l50: Support I2S streaming to CS40L50
.../bindings/input/cirrus,cs40l50.yaml | 70 +++
MAINTAINERS | 12 +
drivers/firmware/cirrus/cs_dsp.c | 263 ++++++++
drivers/input/misc/Kconfig | 10 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/cs40l50-vibra.c | 578 ++++++++++++++++++
drivers/mfd/Kconfig | 30 +
drivers/mfd/Makefile | 4 +
drivers/mfd/cs40l50-core.c | 528 ++++++++++++++++
drivers/mfd/cs40l50-i2c.c | 68 +++
drivers/mfd/cs40l50-spi.c | 68 +++
include/linux/firmware/cirrus/cs_dsp.h | 28 +
include/linux/mfd/cs40l50.h | 144 +++++
sound/soc/codecs/Kconfig | 11 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/cs40l50-codec.c | 307 ++++++++++
16 files changed, 2124 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
create mode 100644 drivers/input/misc/cs40l50-vibra.c
create mode 100644 drivers/mfd/cs40l50-core.c
create mode 100644 drivers/mfd/cs40l50-i2c.c
create mode 100644 drivers/mfd/cs40l50-spi.c
create mode 100644 include/linux/mfd/cs40l50.h
create mode 100644 sound/soc/codecs/cs40l50-codec.c
--
2.25.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface
2024-03-08 22:24 [PATCH v9 0/5] Add support for CS40L50 James Ogletree
@ 2024-03-08 22:24 ` James Ogletree
2024-03-10 19:12 ` Jeff LaBundy
2024-03-08 22:24 ` [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding James Ogletree
` (3 subsequent siblings)
4 siblings, 1 reply; 22+ messages in thread
From: James Ogletree @ 2024-03-08 22:24 UTC (permalink / raw
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, jeff
Cc: patches, linux-sound, linux-input, devicetree, James Ogletree,
Charles Keepax
A write sequencer is a sequence of register addresses
and values executed by some Cirrus DSPs following
power state transitions.
Add support for Cirrus drivers to update or add to a
write sequencer present in firmware.
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
drivers/firmware/cirrus/cs_dsp.c | 263 +++++++++++++++++++++++++
include/linux/firmware/cirrus/cs_dsp.h | 28 +++
2 files changed, 291 insertions(+)
diff --git a/drivers/firmware/cirrus/cs_dsp.c b/drivers/firmware/cirrus/cs_dsp.c
index 79d4254d1f9b..90cd56d70c49 100644
--- a/drivers/firmware/cirrus/cs_dsp.c
+++ b/drivers/firmware/cirrus/cs_dsp.c
@@ -275,6 +275,12 @@
#define HALO_MPU_VIO_ERR_SRC_MASK 0x00007fff
#define HALO_MPU_VIO_ERR_SRC_SHIFT 0
+/*
+ * Write Sequence
+ */
+#define WSEQ_OP_MAX_WORDS 3
+#define WSEQ_END_OF_SCRIPT 0xFFFFFF
+
struct cs_dsp_ops {
bool (*validate_version)(struct cs_dsp *dsp, unsigned int version);
unsigned int (*parse_sizes)(struct cs_dsp *dsp,
@@ -3339,6 +3345,263 @@ int cs_dsp_chunk_read(struct cs_dsp_chunk *ch, int nbits)
}
EXPORT_SYMBOL_NS_GPL(cs_dsp_chunk_read, FW_CS_DSP);
+
+struct cs_dsp_wseq_op {
+ struct list_head list;
+ u32 address;
+ u32 data;
+ u16 offset;
+ u8 operation;
+};
+
+static int cs_dsp_populate_wseq(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq)
+{
+ struct cs_dsp_wseq_op *op = NULL;
+ struct cs_dsp_chunk ch;
+ u8 *words;
+ int ret;
+
+ if (!wseq->ctl) {
+ cs_dsp_err(dsp, "No control for write sequence\n");
+ return -EINVAL;
+ }
+
+ words = kzalloc(wseq->ctl->len, GFP_KERNEL);
+ if (!words)
+ return -ENOMEM;
+
+ ret = cs_dsp_coeff_read_ctrl(wseq->ctl, 0, words, wseq->ctl->len);
+ if (ret) {
+ cs_dsp_err(dsp, "Failed to read %s: %d\n", wseq->ctl->subname, ret);
+ goto err_free;
+ }
+
+ INIT_LIST_HEAD(&wseq->ops);
+
+ ch = cs_dsp_chunk(words, wseq->ctl->len);
+
+ while (!cs_dsp_chunk_end(&ch)) {
+ op = devm_kzalloc(dsp->dev, sizeof(*op), GFP_KERNEL);
+ if (!op) {
+ ret = -ENOMEM;
+ goto err_free;
+ }
+
+ op->offset = cs_dsp_chunk_bytes(&ch);
+ op->operation = cs_dsp_chunk_read(&ch, 8);
+
+ switch (op->operation) {
+ case CS_DSP_WSEQ_END:
+ op->data = WSEQ_END_OF_SCRIPT;
+ break;
+ case CS_DSP_WSEQ_UNLOCK:
+ op->data = cs_dsp_chunk_read(&ch, 16);
+ break;
+ case CS_DSP_WSEQ_ADDR8:
+ op->address = cs_dsp_chunk_read(&ch, 8);
+ op->data = cs_dsp_chunk_read(&ch, 32);
+ break;
+ case CS_DSP_WSEQ_H16:
+ case CS_DSP_WSEQ_L16:
+ op->address = cs_dsp_chunk_read(&ch, 24);
+ op->data = cs_dsp_chunk_read(&ch, 16);
+ break;
+ case CS_DSP_WSEQ_FULL:
+ op->address = cs_dsp_chunk_read(&ch, 32);
+ op->data = cs_dsp_chunk_read(&ch, 32);
+ break;
+ default:
+ ret = -EINVAL;
+ cs_dsp_err(dsp, "Unsupported op: %X\n", op->operation);
+ goto err_free;
+ }
+
+ list_add_tail(&op->list, &wseq->ops);
+
+ if (op->operation == CS_DSP_WSEQ_END)
+ break;
+ }
+
+ if (op && op->operation != CS_DSP_WSEQ_END) {
+ cs_dsp_err(dsp, "Write sequence missing end terminator\n");
+ ret = -ENOENT;
+ }
+
+err_free:
+ kfree(words);
+
+ return ret;
+}
+
+/**
+ * cs_dsp_wseq_init() - Initialize write sequences contained within the loaded DSP firmware
+ * @dsp: Pointer to DSP structure
+ * @wseqs: List of write sequences to initialize
+ * @num_wseqs: Number of write sequences to initialize
+ *
+ * Return: Zero for success, a negative number on error.
+ */
+int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs)
+{
+ int i, ret = 0;
+
+ lockdep_assert_held(&dsp->pwr_lock);
+
+ for (i = 0; i < num_wseqs; i++) {
+ ret = cs_dsp_populate_wseq(dsp, &wseqs[i]);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_init, FW_CS_DSP);
+
+static struct cs_dsp_wseq_op *cs_dsp_wseq_find_op(u32 addr, u8 op_code,
+ struct list_head *wseq_ops)
+{
+ struct cs_dsp_wseq_op *op;
+
+ list_for_each_entry(op, wseq_ops, list) {
+ if (op->operation == op_code && op->address == addr)
+ return op;
+ }
+
+ return NULL;
+}
+
+/**
+ * cs_dsp_wseq_write() - Add or update an entry in a write sequence
+ * @dsp: Pointer to a DSP structure
+ * @wseq: Write sequence to write to
+ * @addr: Address of the register to be written to
+ * @data: Data to be written
+ * @op_code: The type of operation of the new entry
+ * @update: If true, searches for the first entry in the write sequence with
+ * the same address and op_code, and replaces it. If false, creates a new entry
+ * at the tail.
+ *
+ * This function formats register address and value pairs into the format
+ * required for write sequence entries, and either updates or adds the
+ * new entry into the write sequence.
+ *
+ * If update is set to true and no matching entry is found, it will add a new entry.
+ *
+ * Return: Zero for success, a negative number on error.
+ */
+int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
+ u32 addr, u32 data, u8 op_code, bool update)
+{
+ struct cs_dsp_wseq_op *op_end, *op_new = NULL;
+ u32 words[WSEQ_OP_MAX_WORDS];
+ struct cs_dsp_chunk ch;
+ int new_op_size, ret;
+
+ if (update)
+ op_new = cs_dsp_wseq_find_op(addr, op_code, &wseq->ops);
+
+ /* If entry to update is not found, treat it as a new operation */
+ if (!op_new) {
+ op_end = cs_dsp_wseq_find_op(0, CS_DSP_WSEQ_END, &wseq->ops);
+ if (!op_end) {
+ cs_dsp_err(dsp, "Missing write sequence list terminator\n");
+ return -EINVAL;
+ }
+
+ op_new = devm_kzalloc(dsp->dev, sizeof(*op_new), GFP_KERNEL);
+ if (!op_new)
+ return -ENOMEM;
+
+ op_new->operation = op_code;
+ op_new->address = addr;
+ op_new->offset = op_end->offset;
+ update = false;
+ }
+
+ op_new->data = data;
+
+ ch = cs_dsp_chunk(words, sizeof(words));
+ cs_dsp_chunk_write(&ch, 8, op_new->operation);
+ switch (op_code) {
+ case CS_DSP_WSEQ_FULL:
+ cs_dsp_chunk_write(&ch, 32, op_new->address);
+ cs_dsp_chunk_write(&ch, 32, op_new->data);
+ break;
+ case CS_DSP_WSEQ_L16:
+ case CS_DSP_WSEQ_H16:
+ cs_dsp_chunk_write(&ch, 24, op_new->address);
+ cs_dsp_chunk_write(&ch, 16, op_new->data);
+ break;
+ default:
+ ret = -EINVAL;
+ cs_dsp_err(dsp, "Op code not supported: %X\n", op_code);
+ goto op_new_free;
+ }
+
+ new_op_size = cs_dsp_chunk_bytes(&ch);
+
+ if (!update) {
+ if (wseq->ctl->len - op_end->offset < new_op_size) {
+ cs_dsp_err(dsp, "Not enough memory in write sequence for entry\n");
+ ret = -ENOMEM;
+ goto op_new_free;
+ }
+
+ op_end->offset += new_op_size;
+
+ ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_end->offset / sizeof(u32),
+ &op_end->data, sizeof(u32));
+ if (ret)
+ goto op_new_free;
+
+ list_add_tail(&op_new->list, &op_end->list);
+ }
+
+ ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_new->offset / sizeof(u32),
+ words, new_op_size);
+ if (ret)
+ goto op_new_free;
+
+ return 0;
+
+op_new_free:
+ devm_kfree(dsp->dev, op_new);
+
+ return ret;
+}
+EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_write, FW_CS_DSP);
+
+/**
+ * cs_dsp_wseq_multi_write() - Add or update multiple entries in the write sequence
+ * @dsp: Pointer to a DSP structure
+ * @wseq: Write sequence to write to
+ * @reg_seq: List of address-data pairs
+ * @num_regs: Number of address-data pairs
+ * @op_code: The types of operations of the new entries
+ * @update: If true, searches for the first entry in the write sequence with the same
+ * address and op code, and replaces it. If false, creates a new entry at the tail.
+ *
+ * This function calls cs_dsp_wseq_write() for multiple address-data pairs.
+ *
+ * Return: Zero for success, a negative number on error.
+ */
+int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
+ const struct reg_sequence *reg_seq, int num_regs,
+ u8 op_code, bool update)
+{
+ int ret, i;
+
+ for (i = 0; i < num_regs; i++) {
+ ret = cs_dsp_wseq_write(dsp, wseq, reg_seq[i].reg,
+ reg_seq[i].def, update, op_code);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_multi_write, FW_CS_DSP);
+
MODULE_DESCRIPTION("Cirrus Logic DSP Support");
MODULE_AUTHOR("Simon Trimmer <simont@opensource.cirrus.com>");
MODULE_LICENSE("GPL v2");
diff --git a/include/linux/firmware/cirrus/cs_dsp.h b/include/linux/firmware/cirrus/cs_dsp.h
index 29cd11d5a3cf..cfeab75772f6 100644
--- a/include/linux/firmware/cirrus/cs_dsp.h
+++ b/include/linux/firmware/cirrus/cs_dsp.h
@@ -42,6 +42,16 @@
#define CS_DSP_ACKED_CTL_MIN_VALUE 0
#define CS_DSP_ACKED_CTL_MAX_VALUE 0xFFFFFF
+/*
+ * Write sequence operation codes
+ */
+#define CS_DSP_WSEQ_FULL 0x00
+#define CS_DSP_WSEQ_ADDR8 0x02
+#define CS_DSP_WSEQ_L16 0x04
+#define CS_DSP_WSEQ_H16 0x05
+#define CS_DSP_WSEQ_UNLOCK 0xFD
+#define CS_DSP_WSEQ_END 0xFF
+
/**
* struct cs_dsp_region - Describes a logical memory region in DSP address space
* @type: Memory region type
@@ -255,6 +265,24 @@ struct cs_dsp_alg_region *cs_dsp_find_alg_region(struct cs_dsp *dsp,
const char *cs_dsp_mem_region_name(unsigned int type);
+/**
+ * struct cs_dsp_wseq - Describes a write sequence
+ * @name: Name of cs_dsp control
+ * @ctl: Write sequence cs_dsp control
+ * @ops: Operations contained within this write sequence
+ */
+struct cs_dsp_wseq {
+ struct cs_dsp_coeff_ctl *ctl;
+ struct list_head ops;
+};
+
+int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs);
+int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq, u32 addr, u32 data,
+ u8 op_code, bool update);
+int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
+ const struct reg_sequence *reg_seq, int num_regs,
+ u8 op_code, bool update);
+
/**
* struct cs_dsp_chunk - Describes a buffer holding data formatted for the DSP
* @data: Pointer to underlying buffer memory
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
2024-03-08 22:24 [PATCH v9 0/5] Add support for CS40L50 James Ogletree
2024-03-08 22:24 ` [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface James Ogletree
@ 2024-03-08 22:24 ` James Ogletree
2024-03-10 19:29 ` Jeff LaBundy
2024-03-08 22:24 ` [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver James Ogletree
` (2 subsequent siblings)
4 siblings, 1 reply; 22+ messages in thread
From: James Ogletree @ 2024-03-08 22:24 UTC (permalink / raw
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, jeff
Cc: patches, linux-sound, linux-input, devicetree, James Ogletree,
Krzysztof Kozlowski
The CS40L50 is a haptic driver with waveform memory,
integrated DSP, and closed-loop algorithms.
Add a YAML DT binding document for this device.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
.../bindings/input/cirrus,cs40l50.yaml | 70 +++++++++++++++++++
MAINTAINERS | 8 +++
2 files changed, 78 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
diff --git a/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
new file mode 100644
index 000000000000..6a5bdafed56b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
@@ -0,0 +1,70 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cirrus,cs40l50.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic CS40L50 Advanced Haptic Driver
+
+maintainers:
+ - James Ogletree <james.ogletree@cirrus.com>
+
+description:
+ CS40L50 is a haptic driver with waveform memory,
+ integrated DSP, and closed-loop algorithms.
+
+properties:
+ compatible:
+ enum:
+ - cirrus,cs40l50
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ va-supply:
+ description: Power supply for internal analog circuits.
+
+ vp-supply:
+ description: Power supply for always-on circuits.
+
+ vio-supply:
+ description: Power supply for digital input/output.
+
+ vamp-supply:
+ description: Power supply for the Class D amplifier.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - reset-gpios
+ - vp-supply
+ - vio-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ haptic-driver@34 {
+ compatible = "cirrus,cs40l50";
+ reg = <0x34>;
+ interrupt-parent = <&gpio>;
+ interrupts = <113 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio 112 GPIO_ACTIVE_LOW>;
+ vp-supply = <&vreg>;
+ vio-supply = <&vreg>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index dd5de540ec0b..b71017a187f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4933,6 +4933,14 @@ F: sound/pci/hda/cs*
F: sound/pci/hda/hda_cs_dsp_ctl.*
F: sound/soc/codecs/cs*
+CIRRUS LOGIC HAPTIC DRIVERS
+M: James Ogletree <james.ogletree@cirrus.com>
+M: Fred Treven <fred.treven@cirrus.com>
+M: Ben Bright <ben.bright@cirrus.com>
+L: patches@opensource.cirrus.com
+S: Supported
+F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
+
CIRRUS LOGIC DSP FIRMWARE DRIVER
M: Simon Trimmer <simont@opensource.cirrus.com>
M: Charles Keepax <ckeepax@opensource.cirrus.com>
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver
2024-03-08 22:24 [PATCH v9 0/5] Add support for CS40L50 James Ogletree
2024-03-08 22:24 ` [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface James Ogletree
2024-03-08 22:24 ` [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding James Ogletree
@ 2024-03-08 22:24 ` James Ogletree
2024-03-10 23:40 ` Jeff LaBundy
2024-03-08 22:24 ` [PATCH v9 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver James Ogletree
2024-03-08 22:24 ` [PATCH v9 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50 James Ogletree
4 siblings, 1 reply; 22+ messages in thread
From: James Ogletree @ 2024-03-08 22:24 UTC (permalink / raw
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, jeff
Cc: patches, linux-sound, linux-input, devicetree, James Ogletree
Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.
The MFD component registers and initializes the device.
Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
MAINTAINERS | 2 +
drivers/mfd/Kconfig | 30 ++
drivers/mfd/Makefile | 4 +
drivers/mfd/cs40l50-core.c | 528 ++++++++++++++++++++++++++++++++++++
drivers/mfd/cs40l50-i2c.c | 68 +++++
drivers/mfd/cs40l50-spi.c | 68 +++++
include/linux/mfd/cs40l50.h | 144 ++++++++++
7 files changed, 844 insertions(+)
create mode 100644 drivers/mfd/cs40l50-core.c
create mode 100644 drivers/mfd/cs40l50-i2c.c
create mode 100644 drivers/mfd/cs40l50-spi.c
create mode 100644 include/linux/mfd/cs40l50.h
diff --git a/MAINTAINERS b/MAINTAINERS
index b71017a187f8..69a9e0a3b968 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4940,6 +4940,8 @@ M: Ben Bright <ben.bright@cirrus.com>
L: patches@opensource.cirrus.com
S: Supported
F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
+F: drivers/mfd/cs40l*
+F: include/linux/mfd/cs40l*
CIRRUS LOGIC DSP FIRMWARE DRIVER
M: Simon Trimmer <simont@opensource.cirrus.com>
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 90ce58fd629e..6273c255f107 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -2241,6 +2241,36 @@ config MCP_UCB1200_TS
endmenu
+config MFD_CS40L50_CORE
+ tristate
+ select MFD_CORE
+ select FW_CS_DSP
+ select REGMAP_IRQ
+
+config MFD_CS40L50_I2C
+ tristate "Cirrus Logic CS40L50 (I2C)"
+ select REGMAP_I2C
+ select MFD_CS40L50_CORE
+ depends on I2C
+ help
+ Select this to support the Cirrus Logic CS40L50 Haptic
+ Driver over I2C.
+
+ This driver can be built as a module. If built as a module it will be
+ called "cs40l50-i2c".
+
+config MFD_CS40L50_SPI
+ tristate "Cirrus Logic CS40L50 (SPI)"
+ select REGMAP_SPI
+ select MFD_CS40L50_CORE
+ depends on SPI
+ help
+ Select this to support the Cirrus Logic CS40L50 Haptic
+ Driver over SPI.
+
+ This driver can be built as a module. If built as a module it will be
+ called "cs40l50-spi".
+
config MFD_VEXPRESS_SYSREG
tristate "Versatile Express System Registers"
depends on VEXPRESS_CONFIG && GPIOLIB
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c66f07edcd0e..a8d18ba155d0 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -88,6 +88,10 @@ obj-$(CONFIG_MFD_MADERA) += madera.o
obj-$(CONFIG_MFD_MADERA_I2C) += madera-i2c.o
obj-$(CONFIG_MFD_MADERA_SPI) += madera-spi.o
+obj-$(CONFIG_MFD_CS40L50_CORE) += cs40l50-core.o
+obj-$(CONFIG_MFD_CS40L50_I2C) += cs40l50-i2c.o
+obj-$(CONFIG_MFD_CS40L50_SPI) += cs40l50-spi.o
+
obj-$(CONFIG_TPS6105X) += tps6105x.o
obj-$(CONFIG_TPS65010) += tps65010.o
obj-$(CONFIG_TPS6507X) += tps6507x.o
diff --git a/drivers/mfd/cs40l50-core.c b/drivers/mfd/cs40l50-core.c
new file mode 100644
index 000000000000..92e67f80f36a
--- /dev/null
+++ b/drivers/mfd/cs40l50-core.c
@@ -0,0 +1,528 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#include <linux/firmware/cirrus/cs_dsp.h>
+#include <linux/firmware/cirrus/wmfw.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+
+static const struct mfd_cell cs40l50_devs[] = {
+ { .name = "cs40l50-codec", },
+ { .name = "cs40l50-vibra", },
+};
+
+const struct regmap_config cs40l50_regmap = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .reg_format_endian = REGMAP_ENDIAN_BIG,
+ .val_format_endian = REGMAP_ENDIAN_BIG,
+};
+EXPORT_SYMBOL_GPL(cs40l50_regmap);
+
+static const char * const cs40l50_supplies[] = {
+ "vp", "vio",
+};
+
+static const struct regmap_irq cs40l50_reg_irqs[] = {
+ REGMAP_IRQ_REG(CS40L50_DSP_QUEUE_IRQ, CS40L50_IRQ1_INT_2_OFFSET,
+ CS40L50_DSP_QUEUE_MASK),
+ REGMAP_IRQ_REG(CS40L50_AMP_SHORT_IRQ, CS40L50_IRQ1_INT_1_OFFSET,
+ CS40L50_AMP_SHORT_MASK),
+ REGMAP_IRQ_REG(CS40L50_TEMP_ERR_IRQ, CS40L50_IRQ1_INT_8_OFFSET,
+ CS40L50_TEMP_ERR_MASK),
+ REGMAP_IRQ_REG(CS40L50_BST_UVP_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
+ CS40L50_BST_UVP_MASK),
+ REGMAP_IRQ_REG(CS40L50_BST_SHORT_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
+ CS40L50_BST_SHORT_MASK),
+ REGMAP_IRQ_REG(CS40L50_BST_ILIMIT_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
+ CS40L50_BST_ILIMIT_MASK),
+ REGMAP_IRQ_REG(CS40L50_UVLO_VDDBATT_IRQ, CS40L50_IRQ1_INT_10_OFFSET,
+ CS40L50_UVLO_VDDBATT_MASK),
+ REGMAP_IRQ_REG(CS40L50_GLOBAL_ERROR_IRQ, CS40L50_IRQ1_INT_18_OFFSET,
+ CS40L50_GLOBAL_ERROR_MASK),
+};
+
+static struct regmap_irq_chip cs40l50_irq_chip = {
+ .name = "cs40l50",
+
+ .status_base = CS40L50_IRQ1_INT_1,
+ .mask_base = CS40L50_IRQ1_MASK_1,
+ .ack_base = CS40L50_IRQ1_INT_1,
+ .num_regs = 22,
+
+ .irqs = cs40l50_reg_irqs,
+ .num_irqs = ARRAY_SIZE(cs40l50_reg_irqs),
+
+ .runtime_pm = true,
+};
+
+int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val)
+{
+ int err, i;
+ u32 ack;
+
+ /* Device NAKs if hibernating, so optionally retry */
+ for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) {
+ err = regmap_write(regmap, CS40L50_DSP_QUEUE, val);
+ if (!err)
+ break;
+
+ usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100);
+ }
+
+ /* If write never occurred, don't bother polling for ACK */
+ if (i == CS40L50_DSP_TIMEOUT_COUNT) {
+ dev_err(dev, "Timed out writing %#X to DSP: %d\n", val, err);
+ return err;
+ }
+
+ err = regmap_read_poll_timeout(regmap, CS40L50_DSP_QUEUE, ack, !ack,
+ CS40L50_DSP_POLL_US,
+ CS40L50_DSP_POLL_US * CS40L50_DSP_TIMEOUT_COUNT);
+ if (err)
+ dev_err(dev, "DSP did not ACK %#X: %d\n", val, err);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(cs40l50_dsp_write);
+
+static const struct cs_dsp_region cs40l50_dsp_regions[] = {
+ { .type = WMFW_HALO_PM_PACKED, .base = CS40L50_PMEM_0 },
+ { .type = WMFW_HALO_XM_PACKED, .base = CS40L50_XMEM_PACKED_0 },
+ { .type = WMFW_HALO_YM_PACKED, .base = CS40L50_YMEM_PACKED_0 },
+ { .type = WMFW_ADSP2_XM, .base = CS40L50_XMEM_UNPACKED24_0 },
+ { .type = WMFW_ADSP2_YM, .base = CS40L50_YMEM_UNPACKED24_0 },
+};
+
+static const struct reg_sequence cs40l50_internal_vamp_config[] = {
+ { CS40L50_BST_LPMODE_SEL, CS40L50_DCM_LOW_POWER },
+ { CS40L50_BLOCK_ENABLES2, CS40L50_OVERTEMP_WARN },
+};
+
+static const struct reg_sequence cs40l50_irq_mask_override[] = {
+ { CS40L50_IRQ1_MASK_2, CS40L50_IRQ_MASK_2_OVERRIDE },
+ { CS40L50_IRQ1_MASK_20, CS40L50_IRQ_MASK_20_OVERRIDE },
+};
+
+static int cs40l50_configure_dsp(struct cs_dsp *dsp)
+{
+ struct cs40l50 *cs40l50 = container_of(dsp, struct cs40l50, dsp);
+ u32 nwaves;
+ int err;
+
+ /* Log number of effects if wavetable was loaded */
+ if (cs40l50->bin) {
+ err = regmap_read(dsp->regmap, CS40L50_NUM_WAVES, &nwaves);
+ if (err)
+ return err;
+
+ dev_info(dsp->dev, "%u effects loaded\n", nwaves);
+ }
+
+ cs40l50->wseqs[CS40L50_PWR_ON].ctl = cs_dsp_get_ctl(dsp, "PM_PWR_ON_SEQ",
+ WMFW_ADSP2_XM,
+ CS40L50_PM_ALGO);
+ if (!cs40l50->wseqs[CS40L50_PWR_ON].ctl) {
+ dev_err(dsp->dev, "No control for power-on write sequence\n");
+ return -ENOENT;
+ }
+
+ /* Initialize the power-on write sequencer */
+ err = cs_dsp_wseq_init(dsp, cs40l50->wseqs, 1);
+ if (err) {
+ dev_err(dsp->dev, "Failed to initialize power-on write sequence\n");
+ return err;
+ }
+
+ /* Configure internal V_AMP supply */
+ err = regmap_multi_reg_write(dsp->regmap, cs40l50_internal_vamp_config,
+ ARRAY_SIZE(cs40l50_internal_vamp_config));
+ if (err)
+ return err;
+
+ err = cs_dsp_wseq_multi_write(dsp, &cs40l50->wseqs[CS40L50_PWR_ON],
+ cs40l50_internal_vamp_config, CS_DSP_WSEQ_FULL,
+ ARRAY_SIZE(cs40l50_internal_vamp_config), false);
+ if (err)
+ return err;
+
+ /* Override firmware defaults for IRQ masks */
+ err = regmap_multi_reg_write(dsp->regmap, cs40l50_irq_mask_override,
+ ARRAY_SIZE(cs40l50_irq_mask_override));
+ if (err)
+ return err;
+
+ err = cs_dsp_wseq_multi_write(dsp, &cs40l50->wseqs[CS40L50_PWR_ON],
+ cs40l50_irq_mask_override, CS_DSP_WSEQ_FULL,
+ ARRAY_SIZE(cs40l50_irq_mask_override), false);
+ if (err)
+ return err;
+
+ /* Add child devices now that DSP is running */
+ err = devm_mfd_add_devices(dsp->dev, PLATFORM_DEVID_NONE, cs40l50_devs,
+ ARRAY_SIZE(cs40l50_devs), NULL, 0, NULL);
+ if (err)
+ dev_err(dsp->dev, "Failed to add child devices: %d\n", err);
+
+ return err;
+}
+
+static const struct cs_dsp_client_ops client_ops = {
+ .post_run = cs40l50_configure_dsp,
+};
+
+static void cs40l50_dsp_remove(void *data)
+{
+ cs_dsp_remove(data);
+}
+
+static int cs40l50_dsp_init(struct cs40l50 *cs40l50)
+{
+ int err;
+
+ 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 = &client_ops;
+
+ err = cs_dsp_halo_init(&cs40l50->dsp);
+ if (err)
+ return err;
+
+ return devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_remove,
+ &cs40l50->dsp);
+}
+
+static void cs40l50_dsp_power_down(void *data)
+{
+ cs_dsp_power_down(data);
+}
+
+static void cs40l50_dsp_stop(void *data)
+{
+ cs_dsp_stop(data);
+}
+
+static void cs40l50_start_dsp(const struct firmware *bin, void *context)
+{
+ struct cs40l50 *cs40l50 = context;
+ int err;
+
+ /* Wavetable is optional; start DSP regardless */
+ cs40l50->bin = bin;
+
+ mutex_lock(&cs40l50->lock);
+
+ err = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_SHUTDOWN);
+ if (err)
+ goto err_mutex;
+
+ err = cs_dsp_power_up(&cs40l50->dsp, cs40l50->fw, "cs40l50.wmfw",
+ cs40l50->bin, "cs40l50.bin", "cs40l50");
+ if (err)
+ goto err_mutex;
+
+ err = devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_power_down,
+ &cs40l50->dsp);
+ if (err)
+ goto err_mutex;
+
+ err = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_SYSTEM_RESET);
+ if (err)
+ goto err_mutex;
+
+ err = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_PREVENT_HIBER);
+ if (err)
+ goto err_mutex;
+
+ err = cs_dsp_run(&cs40l50->dsp);
+ if (err)
+ goto err_mutex;
+
+ err = devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_stop, &cs40l50->dsp);
+err_mutex:
+ mutex_unlock(&cs40l50->lock);
+ release_firmware(cs40l50->bin);
+ release_firmware(cs40l50->fw);
+
+ if (err)
+ dev_err(cs40l50->dev, "Failed to start DSP: %d", err);
+}
+
+static void cs40l50_request_firmware(const struct firmware *fw, void *context)
+{
+ struct cs40l50 *cs40l50 = context;
+
+ if (!fw) {
+ dev_err(cs40l50->dev, "No firmware file found\n");
+ return;
+ }
+
+ cs40l50->fw = fw;
+
+ if (request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, CS40L50_WT,
+ cs40l50->dev, GFP_KERNEL, cs40l50,
+ cs40l50_start_dsp)) {
+ dev_err(cs40l50->dev, "Failed to request %s\n", CS40L50_WT);
+ release_firmware(cs40l50->fw);
+ }
+}
+
+struct cs40l50_irq {
+ const char *name;
+ int virq;
+};
+
+static struct cs40l50_irq cs40l50_irqs[] = {
+ { "DSP", },
+ { "Global", },
+ { "Boost UVLO", },
+ { "Boost current limit", },
+ { "Boost short", },
+ { "Boost undervolt", },
+ { "Overtemp", },
+ { "Amp short", },
+};
+
+static const struct reg_sequence cs40l50_err_rls[] = {
+ { CS40L50_ERR_RLS, CS40L50_GLOBAL_ERR_RLS_SET },
+ { CS40L50_ERR_RLS, CS40L50_GLOBAL_ERR_RLS_CLEAR },
+};
+
+static irqreturn_t cs40l50_hw_err(int irq, void *data)
+{
+ struct cs40l50 *cs40l50 = data;
+ int err = 0, i;
+
+ mutex_lock(&cs40l50->lock);
+
+ /* Log hardware interrupt and execute error release sequence */
+ for (i = 1; i < ARRAY_SIZE(cs40l50_irqs); i++) {
+ if (cs40l50_irqs[i].virq == irq) {
+ dev_err(cs40l50->dev, "%s error\n", cs40l50_irqs[i].name);
+ err = regmap_multi_reg_write(cs40l50->regmap, cs40l50_err_rls,
+ ARRAY_SIZE(cs40l50_err_rls));
+ goto exit;
+ }
+ }
+exit:
+ mutex_unlock(&cs40l50->lock);
+ return IRQ_RETVAL(!err);
+}
+
+static irqreturn_t cs40l50_dsp_queue(int irq, void *data)
+{
+ struct cs40l50 *cs40l50 = data;
+ u32 rd_ptr, val, wt_ptr;
+ int err = 0;
+
+ mutex_lock(&cs40l50->lock);
+
+ /* Read from DSP queue, log, and update read pointer */
+ while (!err) {
+ err = regmap_read(cs40l50->regmap, CS40L50_DSP_QUEUE_WT, &wt_ptr);
+ if (err)
+ goto exit;
+
+ err = regmap_read(cs40l50->regmap, CS40L50_DSP_QUEUE_RD, &rd_ptr);
+ if (err)
+ goto exit;
+
+ /* Check if queue is empty */
+ if (wt_ptr == rd_ptr)
+ goto exit;
+
+ err = regmap_read(cs40l50->regmap, rd_ptr, &val);
+ if (err)
+ goto exit;
+
+ dev_dbg(cs40l50->dev, "DSP payload: %#X", val);
+
+ rd_ptr += sizeof(u32);
+
+ if (rd_ptr > CS40L50_DSP_QUEUE_END)
+ rd_ptr = CS40L50_DSP_QUEUE_BASE;
+
+ err = regmap_write(cs40l50->regmap, CS40L50_DSP_QUEUE_RD, rd_ptr);
+ if (err)
+ goto exit;
+ }
+exit:
+ mutex_unlock(&cs40l50->lock);
+
+ return IRQ_RETVAL(!err);
+}
+
+static int cs40l50_irq_init(struct cs40l50 *cs40l50)
+{
+ struct device *dev = cs40l50->dev;
+ int err, i, virq;
+
+ err = devm_regmap_add_irq_chip(dev, cs40l50->regmap, cs40l50->irq,
+ IRQF_ONESHOT | IRQF_SHARED, 0,
+ &cs40l50_irq_chip, &cs40l50->irq_data);
+ if (err) {
+ dev_err(dev, "Failed adding IRQ chip\n");
+ return err;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(cs40l50_irqs); i++) {
+ virq = regmap_irq_get_virq(cs40l50->irq_data, i);
+ if (virq < 0) {
+ dev_err(dev, "Failed getting %s\n", cs40l50_irqs[i].name);
+ return virq;
+ }
+
+ cs40l50_irqs[i].virq = virq;
+
+ /* Handle DSP and hardware interrupts separately */
+ err = devm_request_threaded_irq(dev, virq, NULL,
+ i ? cs40l50_hw_err : cs40l50_dsp_queue,
+ IRQF_ONESHOT | IRQF_SHARED,
+ cs40l50_irqs[i].name, cs40l50);
+ if (err) {
+ dev_err(dev, "Failed requesting %s IRQ\n",
+ cs40l50_irqs[i].name);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+static int cs40l50_get_model(struct cs40l50 *cs40l50)
+{
+ int err;
+
+ err = regmap_read(cs40l50->regmap, CS40L50_DEVID, &cs40l50->devid);
+ if (err)
+ return err;
+
+ if (cs40l50->devid != CS40L50_DEVID_A)
+ return -EINVAL;
+
+ err = regmap_read(cs40l50->regmap, CS40L50_REVID, &cs40l50->revid);
+ if (err)
+ return err;
+
+ if (cs40l50->revid < CS40L50_REVID_B0)
+ return -EINVAL;
+
+ dev_info(cs40l50->dev, "Cirrus Logic CS40L50 rev. %02X\n", cs40l50->revid);
+
+ return 0;
+}
+
+static int cs40l50_pm_runtime_setup(struct device *dev)
+{
+ int err;
+
+ pm_runtime_set_autosuspend_delay(dev, CS40L50_AUTOSUSPEND_MS);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_get_noresume(dev);
+ err = pm_runtime_set_active(dev);
+ if (err)
+ return err;
+
+ return devm_pm_runtime_enable(dev);
+}
+
+int cs40l50_probe(struct cs40l50 *cs40l50)
+{
+ struct device *dev = cs40l50->dev;
+ int err;
+
+ mutex_init(&cs40l50->lock);
+
+ cs40l50->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(cs40l50->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(cs40l50->reset_gpio),
+ "Failed getting reset GPIO\n");
+
+ err = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(cs40l50_supplies),
+ cs40l50_supplies);
+ if (err)
+ return dev_err_probe(dev, err, "Failed getting supplies\n");
+
+ /* Ensure minimum reset pulse width */
+ usleep_range(CS40L50_RESET_PULSE_US, CS40L50_RESET_PULSE_US + 100);
+
+ gpiod_set_value_cansleep(cs40l50->reset_gpio, 0);
+
+ /* Wait for control port to be ready */
+ usleep_range(CS40L50_CP_READY_US, CS40L50_CP_READY_US + 100);
+
+ err = cs40l50_dsp_init(cs40l50);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to initialize DSP\n");
+
+ err = cs40l50_pm_runtime_setup(dev);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to initialize runtime PM\n");
+
+ err = cs40l50_get_model(cs40l50);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to get part number\n");
+
+ err = cs40l50_irq_init(cs40l50);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to request IRQs\n");
+
+ err = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, CS40L50_FW,
+ dev, GFP_KERNEL, cs40l50, cs40l50_request_firmware);
+ if (err)
+ return dev_err_probe(dev, err, "Failed to request %s\n", CS40L50_FW);
+
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cs40l50_probe);
+
+int cs40l50_remove(struct cs40l50 *cs40l50)
+{
+ gpiod_set_value_cansleep(cs40l50->reset_gpio, 1);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cs40l50_remove);
+
+static int cs40l50_runtime_suspend(struct device *dev)
+{
+ struct cs40l50 *cs40l50 = dev_get_drvdata(dev);
+
+ return regmap_write(cs40l50->regmap, CS40L50_DSP_QUEUE, CS40L50_ALLOW_HIBER);
+}
+
+static int cs40l50_runtime_resume(struct device *dev)
+{
+ struct cs40l50 *cs40l50 = dev_get_drvdata(dev);
+
+ return cs40l50_dsp_write(dev, cs40l50->regmap, CS40L50_PREVENT_HIBER);
+}
+
+EXPORT_GPL_DEV_PM_OPS(cs40l50_pm_ops) = {
+ RUNTIME_PM_OPS(cs40l50_runtime_suspend, cs40l50_runtime_resume, NULL)
+};
+
+MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(FW_CS_DSP);
diff --git a/drivers/mfd/cs40l50-i2c.c b/drivers/mfd/cs40l50-i2c.c
new file mode 100644
index 000000000000..639be743d956
--- /dev/null
+++ b/drivers/mfd/cs40l50-i2c.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#include <linux/i2c.h>
+#include <linux/mfd/cs40l50.h>
+
+static int cs40l50_i2c_probe(struct i2c_client *i2c)
+{
+ struct cs40l50 *cs40l50;
+
+ cs40l50 = devm_kzalloc(&i2c->dev, sizeof(*cs40l50), GFP_KERNEL);
+ if (!cs40l50)
+ return -ENOMEM;
+
+ i2c_set_clientdata(i2c, cs40l50);
+
+ cs40l50->dev = &i2c->dev;
+ cs40l50->irq = i2c->irq;
+
+ cs40l50->regmap = devm_regmap_init_i2c(i2c, &cs40l50_regmap);
+ if (IS_ERR(cs40l50->regmap))
+ return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap),
+ "Failed to initialize register map\n");
+
+ return cs40l50_probe(cs40l50);
+}
+
+static void cs40l50_i2c_remove(struct i2c_client *i2c)
+{
+ struct cs40l50 *cs40l50 = i2c_get_clientdata(i2c);
+
+ cs40l50_remove(cs40l50);
+}
+
+static const struct i2c_device_id cs40l50_id_i2c[] = {
+ { "cs40l50" },
+ {}
+};
+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 struct i2c_driver cs40l50_i2c_driver = {
+ .driver = {
+ .name = "cs40l50",
+ .of_match_table = cs40l50_of_match,
+ .pm = pm_ptr(&cs40l50_pm_ops),
+ },
+ .id_table = cs40l50_id_i2c,
+ .probe = cs40l50_i2c_probe,
+ .remove = cs40l50_i2c_remove,
+};
+module_i2c_driver(cs40l50_i2c_driver);
+
+MODULE_DESCRIPTION("CS40L50 I2C Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/mfd/cs40l50-spi.c b/drivers/mfd/cs40l50-spi.c
new file mode 100644
index 000000000000..53526b595a0d
--- /dev/null
+++ b/drivers/mfd/cs40l50-spi.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#include <linux/mfd/cs40l50.h>
+#include <linux/spi/spi.h>
+
+static int cs40l50_spi_probe(struct spi_device *spi)
+{
+ struct cs40l50 *cs40l50;
+
+ cs40l50 = devm_kzalloc(&spi->dev, sizeof(*cs40l50), GFP_KERNEL);
+ if (!cs40l50)
+ return -ENOMEM;
+
+ spi_set_drvdata(spi, cs40l50);
+
+ cs40l50->dev = &spi->dev;
+ cs40l50->irq = spi->irq;
+
+ 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");
+
+ return cs40l50_probe(cs40l50);
+}
+
+static void cs40l50_spi_remove(struct spi_device *spi)
+{
+ struct cs40l50 *cs40l50 = spi_get_drvdata(spi);
+
+ cs40l50_remove(cs40l50);
+}
+
+static const struct spi_device_id cs40l50_id_spi[] = {
+ { "cs40l50" },
+ {}
+};
+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 struct spi_driver cs40l50_spi_driver = {
+ .driver = {
+ .name = "cs40l50",
+ .of_match_table = cs40l50_of_match,
+ .pm = pm_ptr(&cs40l50_pm_ops),
+ },
+ .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@cirrus.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/cs40l50.h b/include/linux/mfd/cs40l50.h
new file mode 100644
index 000000000000..cc0661926918
--- /dev/null
+++ b/include/linux/mfd/cs40l50.h
@@ -0,0 +1,144 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#ifndef __MFD_CS40L50_H__
+#define __MFD_CS40L50_H__
+
+#include <linux/firmware/cirrus/cs_dsp.h>
+#include <linux/gpio/consumer.h>
+#include <linux/pm.h>
+#include <linux/regmap.h>
+
+/* Power Supply Configuration */
+#define CS40L50_BLOCK_ENABLES2 0x201C
+#define CS40L50_ERR_RLS 0x2034
+#define CS40L50_PWRMGT_CTL 0x2900
+#define CS40L50_BST_LPMODE_SEL 0x3810
+#define CS40L50_DCM_LOW_POWER 0x1
+#define CS40L50_OVERTEMP_WARN 0x4000010
+
+/* Interrupts */
+#define CS40L50_IRQ1_INT_1 0xE010
+#define CS40L50_IRQ1_BASE CS40L50_IRQ1_INT_1
+#define CS40L50_IRQ1_INT_2 0xE014
+#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_MASK_1 0xE090
+#define CS40L50_IRQ1_MASK_2 0xE094
+#define CS40L50_IRQ1_MASK_20 0xE0DC
+#define CS40L50_IRQ1_INT_1_OFFSET (CS40L50_IRQ1_INT_1 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_2_OFFSET (CS40L50_IRQ1_INT_2 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_8_OFFSET (CS40L50_IRQ1_INT_8 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_9_OFFSET (CS40L50_IRQ1_INT_9 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_10_OFFSET (CS40L50_IRQ1_INT_10 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ1_INT_18_OFFSET (CS40L50_IRQ1_INT_18 - CS40L50_IRQ1_BASE)
+#define CS40L50_IRQ_MASK_2_OVERRIDE 0xFFDF7FFF
+#define CS40L50_IRQ_MASK_20_OVERRIDE 0x15C01000
+#define CS40L50_AMP_SHORT_MASK BIT(31)
+#define CS40L50_DSP_QUEUE_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(18)
+#define CS40L50_UVLO_VDDBATT_MASK BIT(16)
+#define CS40L50_GLOBAL_ERROR_MASK BIT(15)
+
+enum cs40l50_irq_list {
+ CS40L50_DSP_QUEUE_IRQ,
+ 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_AMP_SHORT_IRQ,
+};
+
+/* DSP */
+#define CS40L50_XMEM_PACKED_0 0x2000000
+#define CS40L50_XMEM_UNPACKED24_0 0x2800000
+#define CS40L50_SYS_INFO_ID 0x25E0000
+#define CS40L50_RAM_INIT 0x28021DC
+#define CS40L50_DSP_QUEUE_WT 0x28042C8
+#define CS40L50_DSP_QUEUE_RD 0x28042CC
+#define CS40L50_NUM_WAVES 0x2805C18
+#define CS40L50_CORE_BASE 0x2B80000
+#define CS40L50_CCM_CORE_CONTROL 0x2BC1000
+#define CS40L50_YMEM_PACKED_0 0x2C00000
+#define CS40L50_YMEM_UNPACKED24_0 0x3400000
+#define CS40L50_PMEM_0 0x3800000
+#define CS40L50_MEM_RDY_HW 0x2
+#define CS40L50_RAM_INIT_FLAG 0x1
+#define CS40L50_CLOCK_DISABLE 0x80
+#define CS40L50_CLOCK_ENABLE 0x281
+#define CS40L50_DSP_POLL_US 1000
+#define CS40L50_DSP_TIMEOUT_COUNT 100
+#define CS40L50_RESET_PULSE_US 2200
+#define CS40L50_CP_READY_US 3100
+#define CS40L50_AUTOSUSPEND_MS 2000
+#define CS40L50_PM_ALGO 0x9F206
+#define CS40L50_GLOBAL_ERR_RLS_SET BIT(11)
+#define CS40L50_GLOBAL_ERR_RLS_CLEAR 0
+
+enum cs40l50_wseqs {
+ CS40L50_PWR_ON,
+ CS40L50_STANDBY,
+ CS40L50_ACTIVE,
+ CS40L50_NUM_WSEQS,
+};
+
+/* DSP Queue */
+#define CS40L50_DSP_QUEUE_BASE 0x11004
+#define CS40L50_DSP_QUEUE_END 0x1101C
+#define CS40L50_DSP_QUEUE 0x11020
+#define CS40L50_PREVENT_HIBER 0x2000003
+#define CS40L50_ALLOW_HIBER 0x2000004
+#define CS40L50_SHUTDOWN 0x2000005
+#define CS40L50_SYSTEM_RESET 0x02000007
+#define CS40L50_START_I2S 0x3000002
+#define CS40L50_OWT_PUSH 0x3000008
+#define CS40L50_STOP_PLAYBACK 0x5000000
+#define CS40L50_OWT_DELETE 0xD000000
+
+/* Firmware files */
+#define CS40L50_FW "cs40l50.wmfw"
+#define CS40L50_WT "cs40l50.bin"
+
+/* Device */
+#define CS40L50_DEVID 0x0
+#define CS40L50_REVID 0x4
+#define CS40L50_DEVID_A 0x40A50
+#define CS40L50_REVID_B0 0xB0
+
+struct cs40l50 {
+ struct device *dev;
+ struct regmap *regmap;
+ struct mutex lock;
+ struct cs_dsp dsp;
+ struct gpio_desc *reset_gpio;
+ struct regmap_irq_chip_data *irq_data;
+ const struct firmware *fw;
+ const struct firmware *bin;
+ struct cs_dsp_wseq wseqs[CS40L50_NUM_WSEQS];
+ int irq;
+ u32 devid;
+ u32 revid;
+};
+
+int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val);
+int cs40l50_probe(struct cs40l50 *cs40l50);
+int cs40l50_remove(struct cs40l50 *cs40l50);
+
+extern const struct regmap_config cs40l50_regmap;
+extern const struct dev_pm_ops cs40l50_pm_ops;
+
+#endif /* __MFD_CS40L50_H__ */
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
2024-03-08 22:24 [PATCH v9 0/5] Add support for CS40L50 James Ogletree
` (2 preceding siblings ...)
2024-03-08 22:24 ` [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver James Ogletree
@ 2024-03-08 22:24 ` James Ogletree
2024-03-14 15:54 ` Jeff LaBundy
2024-03-08 22:24 ` [PATCH v9 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50 James Ogletree
4 siblings, 1 reply; 22+ messages in thread
From: James Ogletree @ 2024-03-08 22:24 UTC (permalink / raw
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, jeff
Cc: patches, linux-sound, linux-input, devicetree, James Ogletree
Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.
The input driver provides the interface for control of
haptic effects through the device.
Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
v7: v8: v9:
Please advise if playback stop is still misused with respect to not
specifying an effect ID. The device can only play one effect at a
time, but setting max effects for the EVIOCGEFFECTS ioctl to 1 would
restrict the number of uploads to 1 as well. So I'm unsure of the correct
approach here.
MAINTAINERS | 1 +
drivers/input/misc/Kconfig | 10 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/cs40l50-vibra.c | 578 +++++++++++++++++++++++++++++
4 files changed, 590 insertions(+)
create mode 100644 drivers/input/misc/cs40l50-vibra.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 69a9e0a3b968..24cfb4f017bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4940,6 +4940,7 @@ M: Ben Bright <ben.bright@cirrus.com>
L: patches@opensource.cirrus.com
S: Supported
F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
+F: drivers/input/misc/cs40l*
F: drivers/mfd/cs40l*
F: include/linux/mfd/cs40l*
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 6ba984d7f0b1..ee45dbb0636e 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -140,6 +140,16 @@ config INPUT_BMA150
To compile this driver as a module, choose M here: the
module will be called bma150.
+config INPUT_CS40L50_VIBRA
+ tristate "CS40L50 Haptic Driver support"
+ depends on MFD_CS40L50_CORE
+ help
+ Say Y here to enable support for Cirrus Logic's CS40L50
+ haptic driver.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cs40l50-vibra.
+
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..88279de6d3d5 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -28,6 +28,7 @@ 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_VIBRA) += cs40l50-vibra.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-vibra.c b/drivers/input/misc/cs40l50-vibra.c
new file mode 100644
index 000000000000..c7e68862bbfe
--- /dev/null
+++ b/drivers/input/misc/cs40l50-vibra.c
@@ -0,0 +1,578 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * CS40L50 Advanced Haptic Driver with waveform memory,
+ * integrated DSP, and closed-loop algorithms
+ *
+ * Copyright 2024 Cirrus Logic, Inc.
+ *
+ * Author: James Ogletree <james.ogletree@cirrus.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/input.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+/* Wavetables */
+#define CS40L50_RAM_INDEX_START 0x1000000
+#define CS40L50_RAM_INDEX_END 0x100007F
+#define CS40L50_RTH_INDEX_START 0x1400000
+#define CS40L50_RTH_INDEX_END 0x1400001
+#define CS40L50_ROM_INDEX_START 0x1800000
+#define CS40L50_ROM_INDEX_END 0x180001A
+#define CS40L50_TYPE_PCM 8
+#define CS40L50_TYPE_PWLE 12
+#define CS40L50_PCM_ID 0x0
+#define CS40L50_OWT_CUSTOM_DATA_SIZE 2
+
+/* DSP */
+#define CS40L50_GPIO_BASE 0x2804140
+#define CS40L50_OWT_BASE 0x2805C34
+#define CS40L50_OWT_SIZE 0x2805C38
+#define CS40L50_OWT_NEXT 0x2805C3C
+
+/* GPIO */
+#define CS40L50_GPIO_NUM_MASK GENMASK(14, 12)
+#define CS40L50_GPIO_EDGE_MASK BIT(15)
+#define CS40L50_GPIO_MAPPING_NONE 0
+#define CS40L50_GPIO_DISABLE 0x1FF
+
+enum vibra_bank_type {
+ WVFRM_BANK_RAM,
+ WVFRM_BANK_ROM,
+ WVFRM_BANK_OWT,
+ WVFRM_BANK_NUM,
+};
+
+/* Describes an area in DSP memory populated by effects */
+struct vibra_bank {
+ enum vibra_bank_type bank;
+ u32 base_index;
+ u32 max_index;
+};
+
+struct vibra_effect {
+ enum vibra_bank_type bank;
+ struct list_head list;
+ u32 gpio_reg;
+ u32 index;
+ int id;
+};
+
+/* Describes haptic interface of loaded DSP firmware */
+struct vibra_dsp {
+ struct vibra_bank *banks;
+ u32 gpio_base_reg;
+ u32 owt_offset_reg;
+ u32 owt_size_reg;
+ u32 owt_base_reg;
+ int (*write)(struct device *dev, struct regmap *regmap, u32 val);
+ u32 push_owt_cmd;
+ u32 delete_owt_cmd;
+ u32 stop_cmd;
+};
+
+/* Describes configuration and state of haptic operations */
+struct vibra_info {
+ struct device *dev;
+ struct regmap *regmap;
+ struct input_dev *input;
+ struct mutex lock;
+ struct workqueue_struct *vibe_wq;
+ struct list_head effect_head;
+ struct vibra_dsp dsp;
+};
+
+struct vibra_work {
+ struct vibra_info *info;
+ struct ff_effect *effect;
+ struct work_struct work;
+ s16 *custom_data;
+ int custom_len;
+ int count;
+ int error;
+};
+
+static struct vibra_bank cs40l50_banks[] = {
+ {
+ .bank = WVFRM_BANK_RAM,
+ .base_index = CS40L50_RAM_INDEX_START,
+ .max_index = CS40L50_RAM_INDEX_END,
+ },
+ {
+ .bank = WVFRM_BANK_ROM,
+ .base_index = CS40L50_ROM_INDEX_START,
+ .max_index = CS40L50_ROM_INDEX_END,
+ },
+ {
+ .bank = WVFRM_BANK_OWT,
+ .base_index = CS40L50_RTH_INDEX_START,
+ .max_index = CS40L50_RTH_INDEX_END,
+ },
+};
+
+static struct vibra_dsp cs40l50_dsp = {
+ .banks = cs40l50_banks,
+ .gpio_base_reg = CS40L50_GPIO_BASE,
+ .owt_base_reg = CS40L50_OWT_BASE,
+ .owt_offset_reg = CS40L50_OWT_NEXT,
+ .owt_size_reg = CS40L50_OWT_SIZE,
+
+ .push_owt_cmd = CS40L50_OWT_PUSH,
+ .delete_owt_cmd = CS40L50_OWT_DELETE,
+ .stop_cmd = CS40L50_STOP_PLAYBACK,
+
+ .write = cs40l50_dsp_write,
+};
+
+static struct vibra_effect *vibra_find_effect(int id, struct list_head *effect_head)
+{
+ struct vibra_effect *effect;
+
+ list_for_each_entry(effect, effect_head, list)
+ if (effect->id == id)
+ return effect;
+
+ return NULL;
+}
+
+static int vibra_effect_bank_set(struct vibra_work *work_data,
+ struct vibra_effect *effect)
+{
+ s16 bank = work_data->custom_data[0] & 0xffffu;
+
+ if (bank >= WVFRM_BANK_NUM) {
+ dev_err(work_data->info->dev, "Invalid waveform bank: %d\n", bank);
+ return -EINVAL;
+ }
+
+ if (work_data->custom_len > CS40L50_OWT_CUSTOM_DATA_SIZE)
+ effect->bank = WVFRM_BANK_OWT;
+ else
+ effect->bank = bank;
+
+ return 0;
+}
+
+static int vibra_effect_gpio_mapping_set(struct vibra_work *work_data,
+ struct vibra_effect *effect)
+{
+ u16 button = work_data->effect->trigger.button;
+ struct vibra_info *info = work_data->info;
+ u32 gpio_num, gpio_edge;
+
+ if (button) {
+ gpio_num = FIELD_GET(CS40L50_GPIO_NUM_MASK, button);
+ gpio_edge = FIELD_GET(CS40L50_GPIO_EDGE_MASK, button);
+ effect->gpio_reg = info->dsp.gpio_base_reg + (gpio_num * 8) - gpio_edge;
+
+ return regmap_write(info->regmap, effect->gpio_reg, button);
+ }
+
+ effect->gpio_reg = CS40L50_GPIO_MAPPING_NONE;
+
+ return 0;
+}
+
+static int vibra_effect_index_set(struct vibra_work *work_data,
+ struct vibra_effect *effect)
+{
+ struct vibra_info *info = work_data->info;
+ struct vibra_effect *owt_effect;
+ u32 base_index, max_index;
+
+ base_index = info->dsp.banks[effect->bank].base_index;
+ max_index = info->dsp.banks[effect->bank].max_index;
+
+ effect->index = base_index;
+
+ switch (effect->bank) {
+ case WVFRM_BANK_OWT:
+ list_for_each_entry(owt_effect, &info->effect_head, list)
+ if (owt_effect->bank == WVFRM_BANK_OWT)
+ effect->index++;
+ break;
+ case WVFRM_BANK_ROM:
+ case WVFRM_BANK_RAM:
+ effect->index += work_data->custom_data[1] & 0xffffu;
+ break;
+ default:
+ dev_err(info->dev, "Bank not supported: %d\n", effect->bank);
+ return -EINVAL;
+ }
+
+ if (effect->index > max_index || effect->index < base_index) {
+ dev_err(info->dev, "Index out of bounds: %u\n", effect->index);
+ return -ENOSPC;
+ }
+
+ return 0;
+}
+
+/* Describes a header for an OWT effect */
+struct owt_header {
+ u32 type;
+ u32 data_words;
+ u32 offset;
+} __packed;
+
+static int vibra_upload_owt(struct vibra_work *work_data)
+{
+ u32 len = 2 * work_data->custom_len, wt_offset, wt_size;
+ struct vibra_info *info = work_data->info;
+ struct owt_header header;
+ u8 *out_data;
+ int error;
+
+ error = regmap_read(info->regmap, info->dsp.owt_size_reg, &wt_size);
+ if (error)
+ return error;
+
+ if ((wt_size * sizeof(u32)) < sizeof(header) + len) {
+ dev_err(info->dev, "No space in OWT bank for effect\n");
+ return -ENOSPC;
+ }
+
+ out_data = kzalloc(sizeof(header) + len, GFP_KERNEL);
+ if (!out_data)
+ return -ENOMEM;
+
+ header.type = work_data->custom_data[0] == CS40L50_PCM_ID ? CS40L50_TYPE_PCM :
+ CS40L50_TYPE_PWLE;
+ header.offset = sizeof(header) / sizeof(u32);
+ header.data_words = len / sizeof(u32);
+
+ memcpy(out_data, &header, sizeof(header));
+ memcpy(out_data + sizeof(header), work_data->custom_data, len);
+
+ error = regmap_read(info->regmap, info->dsp.owt_offset_reg, &wt_offset);
+ if (error)
+ goto err_free;
+
+ error = regmap_bulk_write(info->regmap, info->dsp.owt_base_reg +
+ (wt_offset * sizeof(u32)), out_data,
+ sizeof(header) + len);
+ if (error)
+ goto err_free;
+
+ error = info->dsp.write(info->dev, info->regmap, info->dsp.push_owt_cmd);
+err_free:
+ kfree(out_data);
+
+ return error;
+}
+
+static void vibra_add_worker(struct work_struct *work)
+{
+ struct vibra_work *work_data = container_of(work, struct vibra_work, work);
+ struct vibra_info *info = work_data->info;
+ struct vibra_effect *effect;
+ bool is_new = false;
+ int error;
+
+ error = pm_runtime_resume_and_get(info->dev);
+ if (error < 0)
+ goto err;
+
+ mutex_lock(&info->lock);
+
+ /* Update effect if already present, otherwise create new effect */
+ effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
+ if (!effect) {
+ effect = kzalloc(sizeof(*effect), GFP_KERNEL);
+ if (!effect) {
+ error = -ENOMEM;
+ goto err_mutex;
+ }
+
+ effect->id = work_data->effect->id;
+ is_new = true;
+ }
+
+ error = vibra_effect_bank_set(work_data, effect);
+ if (error)
+ goto err_free;
+
+ error = vibra_effect_index_set(work_data, effect);
+ if (error)
+ goto err_free;
+
+ error = vibra_effect_gpio_mapping_set(work_data, effect);
+ if (error)
+ goto err_free;
+
+ if (effect->bank == WVFRM_BANK_OWT)
+ error = vibra_upload_owt(work_data);
+err_free:
+ if (is_new) {
+ if (error)
+ kfree(effect);
+ else
+ list_add(&effect->list, &info->effect_head);
+ }
+err_mutex:
+ mutex_unlock(&info->lock);
+
+ pm_runtime_mark_last_busy(info->dev);
+ pm_runtime_put_autosuspend(info->dev);
+err:
+ work_data->error = error;
+}
+
+static int vibra_add(struct input_dev *dev, struct ff_effect *effect,
+ struct ff_effect *old)
+{
+ struct ff_periodic_effect *periodic = &effect->u.periodic;
+ struct vibra_info *info = input_get_drvdata(dev);
+ u32 len = effect->u.periodic.custom_len;
+ struct vibra_work work_data;
+
+ if (effect->type != FF_PERIODIC || periodic->waveform != FF_CUSTOM) {
+ dev_err(info->dev, "Type (%#X) or waveform (%#X) unsupported\n",
+ effect->type, periodic->waveform);
+ return -EINVAL;
+ }
+
+ work_data.custom_data = kcalloc(len, sizeof(s16), GFP_KERNEL);
+ if (!work_data.custom_data)
+ return -ENOMEM;
+
+ if (copy_from_user(work_data.custom_data, effect->u.periodic.custom_data,
+ sizeof(s16) * len)) {
+ work_data.error = -EFAULT;
+ goto err_free;
+ }
+
+ work_data.custom_len = len;
+ work_data.info = info;
+ work_data.effect = effect;
+ INIT_WORK(&work_data.work, vibra_add_worker);
+
+ /* Push to the workqueue to serialize with playbacks */
+ queue_work(info->vibe_wq, &work_data.work);
+ flush_work(&work_data.work);
+err_free:
+ kfree(work_data.custom_data);
+
+ return work_data.error;
+}
+
+static void vibra_start_worker(struct work_struct *work)
+{
+ struct vibra_work *work_data = container_of(work, struct vibra_work, work);
+ struct vibra_info *info = work_data->info;
+ struct vibra_effect *start_effect;
+
+ if (pm_runtime_resume_and_get(info->dev) < 0)
+ goto err_free;
+
+ mutex_lock(&info->lock);
+
+ start_effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
+ if (start_effect) {
+ while (--work_data->count >= 0) {
+ info->dsp.write(info->dev, info->regmap, start_effect->index);
+ usleep_range(work_data->effect->replay.length,
+ work_data->effect->replay.length + 100);
+ }
+ }
+
+ mutex_unlock(&info->lock);
+
+ if (!start_effect)
+ dev_err(info->dev, "Effect to play not found\n");
+
+ pm_runtime_mark_last_busy(info->dev);
+ pm_runtime_put_autosuspend(info->dev);
+err_free:
+ kfree(work_data);
+}
+
+static void vibra_stop_worker(struct work_struct *work)
+{
+ struct vibra_work *work_data = container_of(work, struct vibra_work, work);
+ struct vibra_info *info = work_data->info;
+
+ if (pm_runtime_resume_and_get(info->dev) < 0)
+ return;
+
+ mutex_lock(&info->lock);
+
+ info->dsp.write(info->dev, info->regmap, info->dsp.stop_cmd);
+
+ mutex_unlock(&info->lock);
+
+ pm_runtime_mark_last_busy(info->dev);
+ pm_runtime_put_autosuspend(info->dev);
+
+ kfree(work_data);
+}
+
+static int vibra_playback(struct input_dev *dev, int effect_id, int val)
+{
+ struct vibra_info *info = input_get_drvdata(dev);
+ struct vibra_work *work_data;
+
+ work_data = kzalloc(sizeof(*work_data), GFP_ATOMIC);
+ if (!work_data)
+ return -ENOMEM;
+
+ work_data->info = info;
+
+ if (val > 0) {
+ work_data->effect = &dev->ff->effects[effect_id];
+ work_data->count = val;
+ INIT_WORK(&work_data->work, vibra_start_worker);
+ } else {
+ /* Stop the amplifier as device drives only one effect */
+ INIT_WORK(&work_data->work, vibra_stop_worker);
+ }
+
+ queue_work(info->vibe_wq, &work_data->work);
+
+ return 0;
+}
+
+static void vibra_erase_worker(struct work_struct *work)
+{
+ struct vibra_work *work_data = container_of(work, struct vibra_work, work);
+ struct vibra_effect *owt_effect, *erase_effect;
+ struct vibra_info *info = work_data->info;
+ int error;
+
+ error = pm_runtime_resume_and_get(info->dev);
+ if (error < 0)
+ goto err;
+
+ mutex_lock(&info->lock);
+
+ erase_effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
+ if (!erase_effect) {
+ dev_err(info->dev, "Effect to erase not found\n");
+ error = -EINVAL;
+ goto err_mutex;
+ }
+
+ if (erase_effect->gpio_reg != CS40L50_GPIO_MAPPING_NONE) {
+ error = regmap_write(info->regmap, erase_effect->gpio_reg,
+ CS40L50_GPIO_DISABLE);
+ if (error)
+ goto err_mutex;
+ }
+
+ if (erase_effect->bank == WVFRM_BANK_OWT) {
+ error = info->dsp.write(info->dev, info->regmap,
+ info->dsp.delete_owt_cmd |
+ erase_effect->index);
+ if (error)
+ goto err_mutex;
+
+ list_for_each_entry(owt_effect, &info->effect_head, list)
+ if (owt_effect->bank == WVFRM_BANK_OWT &&
+ owt_effect->index > erase_effect->index)
+ owt_effect->index--;
+ }
+
+ list_del(&erase_effect->list);
+ kfree(erase_effect);
+err_mutex:
+ mutex_unlock(&info->lock);
+
+ pm_runtime_mark_last_busy(info->dev);
+ pm_runtime_put_autosuspend(info->dev);
+err:
+ work_data->error = error;
+}
+
+static int vibra_erase(struct input_dev *dev, int effect_id)
+{
+ struct vibra_info *info = input_get_drvdata(dev);
+ struct vibra_work work_data;
+
+ work_data.info = info;
+ work_data.effect = &dev->ff->effects[effect_id];
+
+ INIT_WORK(&work_data.work, vibra_erase_worker);
+
+ /* Push to workqueue to serialize with playbacks */
+ queue_work(info->vibe_wq, &work_data.work);
+ flush_work(&work_data.work);
+
+ return work_data.error;
+}
+
+static void vibra_remove_wq(void *data)
+{
+ flush_workqueue(data);
+ destroy_workqueue(data);
+}
+
+static int cs40l50_vibra_probe(struct platform_device *pdev)
+{
+ struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
+ struct vibra_info *info;
+ int error;
+
+ info = devm_kzalloc(pdev->dev.parent, sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return -ENOMEM;
+
+ mutex_init(&info->lock);
+
+ info->dev = cs40l50->dev;
+ info->regmap = cs40l50->regmap;
+ info->dsp = cs40l50_dsp;
+
+ info->input = devm_input_allocate_device(info->dev);
+ if (!info->input)
+ return -ENOMEM;
+ info->input->id.product = cs40l50->devid & 0xFFFF;
+ info->input->id.version = cs40l50->revid;
+ info->input->name = "cs40l50_vibra";
+
+ input_set_drvdata(info->input, info);
+ input_set_capability(info->input, EV_FF, FF_PERIODIC);
+ input_set_capability(info->input, EV_FF, FF_CUSTOM);
+
+ error = input_ff_create(info->input, FF_MAX_EFFECTS);
+ if (error) {
+ dev_err(info->dev, "Failed to create input device\n");
+ return error;
+ }
+
+ info->input->ff->upload = vibra_add;
+ info->input->ff->playback = vibra_playback;
+ info->input->ff->erase = vibra_erase;
+
+ INIT_LIST_HEAD(&info->effect_head);
+
+ info->vibe_wq = alloc_ordered_workqueue("vibe_wq", WQ_HIGHPRI);
+ if (!info->vibe_wq)
+ return -ENOMEM;
+
+ error = devm_add_action_or_reset(info->dev, vibra_remove_wq, info->vibe_wq);
+ if (error)
+ return error;
+
+ return input_register_device(info->input);
+}
+
+static const struct platform_device_id vibra_id_match[] = {
+ { "cs40l50-vibra", },
+ {}
+};
+MODULE_DEVICE_TABLE(platform, vibra_id_match);
+
+static struct platform_driver cs40l50_vibra_driver = {
+ .probe = cs40l50_vibra_probe,
+ .id_table = vibra_id_match,
+ .driver = {
+ .name = "cs40l50-vibra",
+ },
+};
+module_platform_driver(cs40l50_vibra_driver);
+
+MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
+MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
2024-03-08 22:24 [PATCH v9 0/5] Add support for CS40L50 James Ogletree
` (3 preceding siblings ...)
2024-03-08 22:24 ` [PATCH v9 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver James Ogletree
@ 2024-03-08 22:24 ` James Ogletree
2024-03-14 16:05 ` Jeff LaBundy
4 siblings, 1 reply; 22+ messages in thread
From: James Ogletree @ 2024-03-08 22:24 UTC (permalink / raw
To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, jeff
Cc: patches, linux-sound, linux-input, devicetree, James Ogletree
Introduce support for Cirrus Logic Device CS40L50: a
haptic driver with waveform memory, integrated DSP,
and closed-loop algorithms.
The ASoC driver enables I2S streaming to the device.
Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
---
MAINTAINERS | 1 +
sound/soc/codecs/Kconfig | 11 ++
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/cs40l50-codec.c | 307 +++++++++++++++++++++++++++++++
4 files changed, 321 insertions(+)
create mode 100644 sound/soc/codecs/cs40l50-codec.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 24cfb4f017bb..fca2454a7a38 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4943,6 +4943,7 @@ F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
F: drivers/input/misc/cs40l*
F: drivers/mfd/cs40l*
F: include/linux/mfd/cs40l*
+F: sound/soc/codecs/cs40l*
CIRRUS LOGIC DSP FIRMWARE DRIVER
M: Simon Trimmer <simont@opensource.cirrus.com>
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index f1e1dbc509f6..1a81bedfdbe3 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -73,6 +73,7 @@ config SND_SOC_ALL_CODECS
imply SND_SOC_CS35L56_I2C
imply SND_SOC_CS35L56_SPI
imply SND_SOC_CS35L56_SDW
+ imply SND_SOC_CS40L50
imply SND_SOC_CS42L42
imply SND_SOC_CS42L42_SDW
imply SND_SOC_CS42L43
@@ -800,6 +801,16 @@ config SND_SOC_CS35L56_SDW
help
Enable support for Cirrus Logic CS35L56 boosted amplifier with SoundWire control
+config SND_SOC_CS40L50
+ tristate "Cirrus Logic CS40L50 CODEC"
+ depends on MFD_CS40L50_CORE
+ help
+ This option enables support for I2S streaming to Cirrus Logic CS40L50.
+
+ CS40L50 is a haptic driver with waveform memory, an integrated
+ DSP, and closed-loop algorithms. If built as a module, it will be
+ called snd-soc-cs40l50.
+
config SND_SOC_CS42L42_CORE
tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index a87e56938ce5..7e31f000774a 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -74,6 +74,7 @@ snd-soc-cs35l56-shared-objs := cs35l56-shared.o
snd-soc-cs35l56-i2c-objs := cs35l56-i2c.o
snd-soc-cs35l56-spi-objs := cs35l56-spi.o
snd-soc-cs35l56-sdw-objs := cs35l56-sdw.o
+snd-soc-cs40l50-objs := cs40l50-codec.o
snd-soc-cs42l42-objs := cs42l42.o
snd-soc-cs42l42-i2c-objs := cs42l42-i2c.o
snd-soc-cs42l42-sdw-objs := cs42l42-sdw.o
@@ -460,6 +461,7 @@ obj-$(CONFIG_SND_SOC_CS35L56_SHARED) += snd-soc-cs35l56-shared.o
obj-$(CONFIG_SND_SOC_CS35L56_I2C) += snd-soc-cs35l56-i2c.o
obj-$(CONFIG_SND_SOC_CS35L56_SPI) += snd-soc-cs35l56-spi.o
obj-$(CONFIG_SND_SOC_CS35L56_SDW) += snd-soc-cs35l56-sdw.o
+obj-$(CONFIG_SND_SOC_CS40L50) += snd-soc-cs40l50.o
obj-$(CONFIG_SND_SOC_CS42L42_CORE) += snd-soc-cs42l42.o
obj-$(CONFIG_SND_SOC_CS42L42) += snd-soc-cs42l42-i2c.o
obj-$(CONFIG_SND_SOC_CS42L42_SDW) += snd-soc-cs42l42-sdw.o
diff --git a/sound/soc/codecs/cs40l50-codec.c b/sound/soc/codecs/cs40l50-codec.c
new file mode 100644
index 000000000000..23299b8dacfb
--- /dev/null
+++ b/sound/soc/codecs/cs40l50-codec.c
@@ -0,0 +1,307 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// CS40L50 Advanced Haptic Driver with waveform memory,
+// integrated DSP, and closed-loop algorithms
+//
+// Copyright 2024 Cirrus Logic, Inc.
+//
+// Author: James Ogletree <james.ogletree@cirrus.com>
+
+#include <linux/bitfield.h>
+#include <linux/mfd/cs40l50.h>
+#include <linux/pm_runtime.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+#define CS40L50_REFCLK_INPUT 0x2C04
+#define CS40L50_ASP_CONTROL2 0x4808
+#define CS40L50_ASP_DATA_CONTROL5 0x4840
+
+/* PLL Config */
+#define CS40L50_PLL_REFCLK_BCLK 0x0
+#define CS40L50_PLL_REFCLK_MCLK 0x5
+#define CS40L50_PLL_REFCLK_LOOP_MASK BIT(11)
+#define CS40L50_PLL_REFCLK_OPEN_LOOP 1
+#define CS40L50_PLL_REFCLK_CLOSED_LOOP 0
+#define CS40L50_PLL_REFCLK_LOOP_SHIFT 11
+#define CS40L50_PLL_REFCLK_FREQ_MASK GENMASK(10, 5)
+#define CS40L50_PLL_REFCLK_FREQ_SHIFT 5
+#define CS40L50_PLL_REFCLK_SEL_MASK GENMASK(2, 0)
+#define CS40L50_BCLK_RATIO_DEFAULT 32
+
+/* ASP Config */
+#define CS40L50_ASP_RX_WIDTH_SHIFT 24
+#define CS40L50_ASP_RX_WIDTH_MASK GENMASK(31, 24)
+#define CS40L50_ASP_RX_WL_MASK GENMASK(5, 0)
+#define CS40L50_ASP_FSYNC_INV_MASK BIT(2)
+#define CS40L50_ASP_BCLK_INV_MASK BIT(6)
+#define CS40L50_ASP_FMT_MASK GENMASK(10, 8)
+#define CS40L50_ASP_FMT_I2S 0x2
+
+struct cs40l50_pll_config {
+ unsigned int freq;
+ unsigned int cfg;
+};
+
+struct cs40l50_codec {
+ struct device *dev;
+ struct regmap *regmap;
+ unsigned int daifmt;
+ unsigned int bclk_ratio;
+ unsigned int rate;
+};
+
+static const struct cs40l50_pll_config cs40l50_pll_cfg[] = {
+ {32768, 0x00},
+ {1536000, 0x1B},
+ {3072000, 0x21},
+ {6144000, 0x28},
+ {9600000, 0x30},
+ {12288000, 0x33},
+};
+
+static int cs40l50_get_clk_config(unsigned int freq, unsigned int *cfg)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(cs40l50_pll_cfg); i++) {
+ if (cs40l50_pll_cfg[i].freq == freq) {
+ *cfg = cs40l50_pll_cfg[i].cfg;
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+
+static int cs40l50_swap_ext_clk(struct cs40l50_codec *codec, unsigned int clk_src)
+{
+ unsigned int cfg;
+ int ret;
+
+ switch (clk_src) {
+ case CS40L50_PLL_REFCLK_BCLK:
+ ret = cs40l50_get_clk_config(codec->bclk_ratio * codec->rate, &cfg);
+ if (ret)
+ return ret;
+ break;
+ case CS40L50_PLL_REFCLK_MCLK:
+ cfg = 0x00;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
+ CS40L50_PLL_REFCLK_LOOP_MASK,
+ CS40L50_PLL_REFCLK_OPEN_LOOP <<
+ CS40L50_PLL_REFCLK_LOOP_SHIFT);
+ if (ret)
+ return ret;
+
+ ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
+ CS40L50_PLL_REFCLK_FREQ_MASK |
+ CS40L50_PLL_REFCLK_SEL_MASK,
+ (cfg << CS40L50_PLL_REFCLK_FREQ_SHIFT) | clk_src);
+ if (ret)
+ return ret;
+
+ return regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
+ CS40L50_PLL_REFCLK_LOOP_MASK,
+ CS40L50_PLL_REFCLK_CLOSED_LOOP <<
+ CS40L50_PLL_REFCLK_LOOP_SHIFT);
+}
+
+static int cs40l50_clk_en(struct snd_soc_dapm_widget *w,
+ struct snd_kcontrol *kcontrol,
+ int event)
+{
+ struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
+ struct cs40l50_codec *codec = snd_soc_component_get_drvdata(comp);
+ int ret;
+
+ switch (event) {
+ case SND_SOC_DAPM_POST_PMU:
+ ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_STOP_PLAYBACK);
+ if (ret)
+ return ret;
+
+ ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_START_I2S);
+ if (ret)
+ return ret;
+
+ ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_BCLK);
+ if (ret)
+ return ret;
+ break;
+ case SND_SOC_DAPM_PRE_PMD:
+ ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_MCLK);
+ if (ret)
+ return ret;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static const struct snd_soc_dapm_widget cs40l50_dapm_widgets[] = {
+ SND_SOC_DAPM_SUPPLY_S("ASP PLL", 0, SND_SOC_NOPM, 0, 0, cs40l50_clk_en,
+ SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
+ SND_SOC_DAPM_AIF_IN("ASPRX1", NULL, 0, SND_SOC_NOPM, 0, 0),
+ SND_SOC_DAPM_AIF_IN("ASPRX2", NULL, 0, SND_SOC_NOPM, 0, 0),
+ SND_SOC_DAPM_OUTPUT("OUT"),
+};
+
+static const struct snd_soc_dapm_route cs40l50_dapm_routes[] = {
+ { "ASP Playback", NULL, "ASP PLL" },
+ { "ASPRX1", NULL, "ASP Playback" },
+ { "ASPRX2", NULL, "ASP Playback" },
+
+ { "OUT", NULL, "ASPRX1" },
+ { "OUT", NULL, "ASPRX2" },
+};
+
+static int cs40l50_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
+{
+ struct cs40l50_codec *codec = snd_soc_component_get_drvdata(codec_dai->component);
+
+ if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBC_CFC)
+ return -EINVAL;
+
+ switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
+ case SND_SOC_DAIFMT_NB_NF:
+ codec->daifmt = 0;
+ break;
+ case SND_SOC_DAIFMT_NB_IF:
+ codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK;
+ break;
+ case SND_SOC_DAIFMT_IB_NF:
+ codec->daifmt = CS40L50_ASP_BCLK_INV_MASK;
+ break;
+ case SND_SOC_DAIFMT_IB_IF:
+ codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK | CS40L50_ASP_BCLK_INV_MASK;
+ break;
+ default:
+ dev_err(codec->dev, "Invalid clock invert\n");
+ return -EINVAL;
+ }
+
+ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+ case SND_SOC_DAIFMT_I2S:
+ codec->daifmt |= FIELD_PREP(CS40L50_ASP_FMT_MASK, CS40L50_ASP_FMT_I2S);
+ break;
+ default:
+ dev_err(codec->dev, "Unsupported DAI format\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int cs40l50_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params,
+ struct snd_soc_dai *dai)
+{
+ struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
+ unsigned int asp_rx_wl = params_width(params);
+ int ret;
+
+ codec->rate = params_rate(params);
+
+ ret = regmap_update_bits(codec->regmap, CS40L50_ASP_DATA_CONTROL5,
+ CS40L50_ASP_RX_WL_MASK, asp_rx_wl);
+ if (ret)
+ return ret;
+
+ codec->daifmt |= (asp_rx_wl << CS40L50_ASP_RX_WIDTH_SHIFT);
+
+ return regmap_update_bits(codec->regmap, CS40L50_ASP_CONTROL2,
+ CS40L50_ASP_FSYNC_INV_MASK |
+ CS40L50_ASP_BCLK_INV_MASK |
+ CS40L50_ASP_FMT_MASK |
+ CS40L50_ASP_RX_WIDTH_MASK, codec->daifmt);
+}
+
+static int cs40l50_set_dai_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio)
+{
+ struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
+
+ codec->bclk_ratio = ratio;
+
+ return 0;
+}
+
+static const struct snd_soc_dai_ops cs40l50_dai_ops = {
+ .set_fmt = cs40l50_set_dai_fmt,
+ .set_bclk_ratio = cs40l50_set_dai_bclk_ratio,
+ .hw_params = cs40l50_hw_params,
+};
+
+static struct snd_soc_dai_driver cs40l50_dai[] = {
+ {
+ .name = "cs40l50-pcm",
+ .id = 0,
+ .playback = {
+ .stream_name = "ASP Playback",
+ .channels_min = 1,
+ .channels_max = 2,
+ .rates = SNDRV_PCM_RATE_48000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+ },
+ .ops = &cs40l50_dai_ops,
+ },
+};
+
+static int cs40l50_codec_probe(struct snd_soc_component *component)
+{
+ struct cs40l50_codec *codec = snd_soc_component_get_drvdata(component);
+
+ codec->bclk_ratio = CS40L50_BCLK_RATIO_DEFAULT;
+
+ return 0;
+}
+
+static const struct snd_soc_component_driver soc_codec_dev_cs40l50 = {
+ .probe = cs40l50_codec_probe,
+ .dapm_widgets = cs40l50_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(cs40l50_dapm_widgets),
+ .dapm_routes = cs40l50_dapm_routes,
+ .num_dapm_routes = ARRAY_SIZE(cs40l50_dapm_routes),
+};
+
+static int cs40l50_codec_driver_probe(struct platform_device *pdev)
+{
+ struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
+ struct cs40l50_codec *codec;
+
+ codec = devm_kzalloc(&pdev->dev, sizeof(*codec), GFP_KERNEL);
+ if (!codec)
+ return -ENOMEM;
+
+ codec->regmap = cs40l50->regmap;
+ codec->dev = &pdev->dev;
+
+ return devm_snd_soc_register_component(&pdev->dev, &soc_codec_dev_cs40l50,
+ cs40l50_dai, ARRAY_SIZE(cs40l50_dai));
+}
+
+static const struct platform_device_id cs40l50_id[] = {
+ { "cs40l50-codec", },
+ {}
+};
+MODULE_DEVICE_TABLE(platform, cs40l50_id);
+
+static struct platform_driver cs40l50_codec_driver = {
+ .probe = cs40l50_codec_driver_probe,
+ .id_table = cs40l50_id,
+ .driver = {
+ .name = "cs40l50-codec",
+ },
+};
+module_platform_driver(cs40l50_codec_driver);
+
+MODULE_DESCRIPTION("ASoC CS40L50 driver");
+MODULE_AUTHOR("James Ogletree <james.ogletree@cirrus.com>");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface
2024-03-08 22:24 ` [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface James Ogletree
@ 2024-03-10 19:12 ` Jeff LaBundy
2024-03-11 10:14 ` Charles Keepax
0 siblings, 1 reply; 22+ messages in thread
From: Jeff LaBundy @ 2024-03-10 19:12 UTC (permalink / raw
To: James Ogletree
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, patches, linux-sound, linux-input, devicetree,
Charles Keepax
Hi James,
On Fri, Mar 08, 2024 at 10:24:17PM +0000, James Ogletree wrote:
> A write sequencer is a sequence of register addresses
> and values executed by some Cirrus DSPs following
> power state transitions.
>
> Add support for Cirrus drivers to update or add to a
> write sequencer present in firmware.
>
> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
Truly excellent work here; thanks again for continuing to lead a
productive review. I think the overall design of the driver and
the way it is organized are solid now.
Apologies for not getting you feedback sooner; I have some minor
comments below and in a few other spots throughout the series. I
think it is ready after these last few points are addressed.
> ---
> drivers/firmware/cirrus/cs_dsp.c | 263 +++++++++++++++++++++++++
> include/linux/firmware/cirrus/cs_dsp.h | 28 +++
> 2 files changed, 291 insertions(+)
>
> diff --git a/drivers/firmware/cirrus/cs_dsp.c b/drivers/firmware/cirrus/cs_dsp.c
> index 79d4254d1f9b..90cd56d70c49 100644
> --- a/drivers/firmware/cirrus/cs_dsp.c
> +++ b/drivers/firmware/cirrus/cs_dsp.c
> @@ -275,6 +275,12 @@
> #define HALO_MPU_VIO_ERR_SRC_MASK 0x00007fff
> #define HALO_MPU_VIO_ERR_SRC_SHIFT 0
>
> +/*
> + * Write Sequence
> + */
> +#define WSEQ_OP_MAX_WORDS 3
> +#define WSEQ_END_OF_SCRIPT 0xFFFFFF
> +
> struct cs_dsp_ops {
> bool (*validate_version)(struct cs_dsp *dsp, unsigned int version);
> unsigned int (*parse_sizes)(struct cs_dsp *dsp,
> @@ -3339,6 +3345,263 @@ int cs_dsp_chunk_read(struct cs_dsp_chunk *ch, int nbits)
> }
> EXPORT_SYMBOL_NS_GPL(cs_dsp_chunk_read, FW_CS_DSP);
>
> +
> +struct cs_dsp_wseq_op {
> + struct list_head list;
> + u32 address;
> + u32 data;
> + u16 offset;
> + u8 operation;
> +};
> +
> +static int cs_dsp_populate_wseq(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq)
> +{
> + struct cs_dsp_wseq_op *op = NULL;
> + struct cs_dsp_chunk ch;
> + u8 *words;
> + int ret;
> +
> + if (!wseq->ctl) {
> + cs_dsp_err(dsp, "No control for write sequence\n");
> + return -EINVAL;
> + }
> +
> + words = kzalloc(wseq->ctl->len, GFP_KERNEL);
> + if (!words)
> + return -ENOMEM;
> +
> + ret = cs_dsp_coeff_read_ctrl(wseq->ctl, 0, words, wseq->ctl->len);
> + if (ret) {
> + cs_dsp_err(dsp, "Failed to read %s: %d\n", wseq->ctl->subname, ret);
> + goto err_free;
> + }
> +
> + INIT_LIST_HEAD(&wseq->ops);
> +
> + ch = cs_dsp_chunk(words, wseq->ctl->len);
> +
> + while (!cs_dsp_chunk_end(&ch)) {
> + op = devm_kzalloc(dsp->dev, sizeof(*op), GFP_KERNEL);
> + if (!op) {
> + ret = -ENOMEM;
> + goto err_free;
> + }
> +
> + op->offset = cs_dsp_chunk_bytes(&ch);
> + op->operation = cs_dsp_chunk_read(&ch, 8);
> +
> + switch (op->operation) {
> + case CS_DSP_WSEQ_END:
> + op->data = WSEQ_END_OF_SCRIPT;
> + break;
> + case CS_DSP_WSEQ_UNLOCK:
> + op->data = cs_dsp_chunk_read(&ch, 16);
> + break;
> + case CS_DSP_WSEQ_ADDR8:
> + op->address = cs_dsp_chunk_read(&ch, 8);
> + op->data = cs_dsp_chunk_read(&ch, 32);
> + break;
> + case CS_DSP_WSEQ_H16:
> + case CS_DSP_WSEQ_L16:
> + op->address = cs_dsp_chunk_read(&ch, 24);
> + op->data = cs_dsp_chunk_read(&ch, 16);
> + break;
> + case CS_DSP_WSEQ_FULL:
> + op->address = cs_dsp_chunk_read(&ch, 32);
> + op->data = cs_dsp_chunk_read(&ch, 32);
> + break;
> + default:
> + ret = -EINVAL;
> + cs_dsp_err(dsp, "Unsupported op: %X\n", op->operation);
> + goto err_free;
> + }
> +
> + list_add_tail(&op->list, &wseq->ops);
> +
> + if (op->operation == CS_DSP_WSEQ_END)
> + break;
> + }
> +
> + if (op && op->operation != CS_DSP_WSEQ_END) {
> + cs_dsp_err(dsp, "Write sequence missing end terminator\n");
> + ret = -ENOENT;
> + }
> +
> +err_free:
> + kfree(words);
> +
> + return ret;
> +}
> +
> +/**
> + * cs_dsp_wseq_init() - Initialize write sequences contained within the loaded DSP firmware
> + * @dsp: Pointer to DSP structure
> + * @wseqs: List of write sequences to initialize
> + * @num_wseqs: Number of write sequences to initialize
> + *
> + * Return: Zero for success, a negative number on error.
> + */
> +int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs)
> +{
> + int i, ret = 0;
> +
> + lockdep_assert_held(&dsp->pwr_lock);
> +
> + for (i = 0; i < num_wseqs; i++) {
> + ret = cs_dsp_populate_wseq(dsp, &wseqs[i]);
> + if (ret)
> + break;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_init, FW_CS_DSP);
> +
> +static struct cs_dsp_wseq_op *cs_dsp_wseq_find_op(u32 addr, u8 op_code,
> + struct list_head *wseq_ops)
> +{
> + struct cs_dsp_wseq_op *op;
> +
> + list_for_each_entry(op, wseq_ops, list) {
> + if (op->operation == op_code && op->address == addr)
> + return op;
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + * cs_dsp_wseq_write() - Add or update an entry in a write sequence
> + * @dsp: Pointer to a DSP structure
> + * @wseq: Write sequence to write to
> + * @addr: Address of the register to be written to
> + * @data: Data to be written
> + * @op_code: The type of operation of the new entry
> + * @update: If true, searches for the first entry in the write sequence with
> + * the same address and op_code, and replaces it. If false, creates a new entry
> + * at the tail.
> + *
> + * This function formats register address and value pairs into the format
> + * required for write sequence entries, and either updates or adds the
> + * new entry into the write sequence.
> + *
> + * If update is set to true and no matching entry is found, it will add a new entry.
> + *
> + * Return: Zero for success, a negative number on error.
> + */
> +int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
> + u32 addr, u32 data, u8 op_code, bool update)
> +{
> + struct cs_dsp_wseq_op *op_end, *op_new = NULL;
> + u32 words[WSEQ_OP_MAX_WORDS];
> + struct cs_dsp_chunk ch;
Nit: 'chunk' is probably a more descriptive variable name; 'ch' is ambiguous
and too easily confused with other common things (e.g. channel).
> + int new_op_size, ret;
> +
> + if (update)
> + op_new = cs_dsp_wseq_find_op(addr, op_code, &wseq->ops);
> +
> + /* If entry to update is not found, treat it as a new operation */
> + if (!op_new) {
> + op_end = cs_dsp_wseq_find_op(0, CS_DSP_WSEQ_END, &wseq->ops);
> + if (!op_end) {
> + cs_dsp_err(dsp, "Missing write sequence list terminator\n");
> + return -EINVAL;
> + }
> +
> + op_new = devm_kzalloc(dsp->dev, sizeof(*op_new), GFP_KERNEL);
> + if (!op_new)
> + return -ENOMEM;
> +
> + op_new->operation = op_code;
> + op_new->address = addr;
> + op_new->offset = op_end->offset;
> + update = false;
> + }
> +
> + op_new->data = data;
> +
> + ch = cs_dsp_chunk(words, sizeof(words));
> + cs_dsp_chunk_write(&ch, 8, op_new->operation);
Nit: maybe a newline here?
> + switch (op_code) {
> + case CS_DSP_WSEQ_FULL:
> + cs_dsp_chunk_write(&ch, 32, op_new->address);
> + cs_dsp_chunk_write(&ch, 32, op_new->data);
> + break;
> + case CS_DSP_WSEQ_L16:
> + case CS_DSP_WSEQ_H16:
> + cs_dsp_chunk_write(&ch, 24, op_new->address);
> + cs_dsp_chunk_write(&ch, 16, op_new->data);
> + break;
> + default:
> + ret = -EINVAL;
> + cs_dsp_err(dsp, "Op code not supported: %X\n", op_code);
> + goto op_new_free;
There is no need to drop down and call devm_kfree() here; it's sufficient
to simply return -EINVAL. The devres core will free any instances of
cs_dsp_wseq_op when the driver is unbound.
I imagine you're calling devm_kfree() to protect against the case where
the driver successfully probes, but the contents of the firmware are found
to be invalid later. In that case, yes, this newly allocated memory will
persist throughout the length of the driver's life.
That's an error condition though; if it happens, the customer will surely
remove the module, correct the .wmfw or .bin file, then insert the module
again. All we need to do here is make sure that memory does not leak after
the module is removed, and device-managed allocation already handles this.
> + }
> +
> + new_op_size = cs_dsp_chunk_bytes(&ch);
> +
> + if (!update) {
> + if (wseq->ctl->len - op_end->offset < new_op_size) {
> + cs_dsp_err(dsp, "Not enough memory in write sequence for entry\n");
> + ret = -ENOMEM;
> + goto op_new_free;
Same here. Maybe you want to return -E2BIG here so the caller can tell the
difference between there not being enough memory available to allocate a
new instance of cs_dsp_wseq_op, vs. one having already been allocated, but
not big enough.
> + }
> +
> + op_end->offset += new_op_size;
> +
> + ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_end->offset / sizeof(u32),
> + &op_end->data, sizeof(u32));
> + if (ret)
> + goto op_new_free;
And here; just return ret.
> +
> + list_add_tail(&op_new->list, &op_end->list);
> + }
> +
> + ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_new->offset / sizeof(u32),
> + words, new_op_size);
> + if (ret)
> + goto op_new_free;
> +
> + return 0;
Here too. This clean-up avoids the rather unsightly design pattern of placing
a tear-down path after the function's primary return path.
> +
> +op_new_free:
> + devm_kfree(dsp->dev, op_new);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_write, FW_CS_DSP);
> +
> +/**
> + * cs_dsp_wseq_multi_write() - Add or update multiple entries in the write sequence
> + * @dsp: Pointer to a DSP structure
> + * @wseq: Write sequence to write to
> + * @reg_seq: List of address-data pairs
> + * @num_regs: Number of address-data pairs
> + * @op_code: The types of operations of the new entries
> + * @update: If true, searches for the first entry in the write sequence with the same
> + * address and op code, and replaces it. If false, creates a new entry at the tail.
> + *
> + * This function calls cs_dsp_wseq_write() for multiple address-data pairs.
> + *
> + * Return: Zero for success, a negative number on error.
> + */
> +int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
> + const struct reg_sequence *reg_seq, int num_regs,
> + u8 op_code, bool update)
> +{
> + int ret, i;
> +
> + for (i = 0; i < num_regs; i++) {
> + ret = cs_dsp_wseq_write(dsp, wseq, reg_seq[i].reg,
> + reg_seq[i].def, update, op_code);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
This is absolutely a nit-pick, but in cs_dsp_wseq_init(), you use the pattern
of breaking on error and always returning ret; here you return on error. Both
are functionally equivalent but it would be nice to be consistent in terms of
style.
If you do opt to match cs_dsp_wseq_multi_write() to cs_dsp_wseq_init(), I do
think you'll need to initialize ret to zero here as well to avoid a compiler
warning for the num_regs = 0 case.
> +}
> +EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_multi_write, FW_CS_DSP);
> +
> MODULE_DESCRIPTION("Cirrus Logic DSP Support");
> MODULE_AUTHOR("Simon Trimmer <simont@opensource.cirrus.com>");
> MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/firmware/cirrus/cs_dsp.h b/include/linux/firmware/cirrus/cs_dsp.h
> index 29cd11d5a3cf..cfeab75772f6 100644
> --- a/include/linux/firmware/cirrus/cs_dsp.h
> +++ b/include/linux/firmware/cirrus/cs_dsp.h
> @@ -42,6 +42,16 @@
> #define CS_DSP_ACKED_CTL_MIN_VALUE 0
> #define CS_DSP_ACKED_CTL_MAX_VALUE 0xFFFFFF
>
> +/*
> + * Write sequence operation codes
> + */
> +#define CS_DSP_WSEQ_FULL 0x00
> +#define CS_DSP_WSEQ_ADDR8 0x02
> +#define CS_DSP_WSEQ_L16 0x04
> +#define CS_DSP_WSEQ_H16 0x05
> +#define CS_DSP_WSEQ_UNLOCK 0xFD
> +#define CS_DSP_WSEQ_END 0xFF
> +
> /**
> * struct cs_dsp_region - Describes a logical memory region in DSP address space
> * @type: Memory region type
> @@ -255,6 +265,24 @@ struct cs_dsp_alg_region *cs_dsp_find_alg_region(struct cs_dsp *dsp,
>
> const char *cs_dsp_mem_region_name(unsigned int type);
>
> +/**
> + * struct cs_dsp_wseq - Describes a write sequence
> + * @name: Name of cs_dsp control
> + * @ctl: Write sequence cs_dsp control
> + * @ops: Operations contained within this write sequence
> + */
> +struct cs_dsp_wseq {
> + struct cs_dsp_coeff_ctl *ctl;
> + struct list_head ops;
> +};
> +
> +int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs);
> +int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq, u32 addr, u32 data,
> + u8 op_code, bool update);
> +int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
> + const struct reg_sequence *reg_seq, int num_regs,
> + u8 op_code, bool update);
> +
> /**
> * struct cs_dsp_chunk - Describes a buffer holding data formatted for the DSP
> * @data: Pointer to underlying buffer memory
> --
> 2.25.1
>
Kind regards,
Jeff LaBundy
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
2024-03-08 22:24 ` [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding James Ogletree
@ 2024-03-10 19:29 ` Jeff LaBundy
2024-03-11 6:31 ` Krzysztof Kozlowski
0 siblings, 1 reply; 22+ messages in thread
From: Jeff LaBundy @ 2024-03-10 19:29 UTC (permalink / raw
To: James Ogletree
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, patches, linux-sound, linux-input, devicetree,
Krzysztof Kozlowski
Hi James,
On Fri, Mar 08, 2024 at 10:24:18PM +0000, James Ogletree wrote:
> The CS40L50 is a haptic driver with waveform memory,
> integrated DSP, and closed-loop algorithms.
>
> Add a YAML DT binding document for this device.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
> .../bindings/input/cirrus,cs40l50.yaml | 70 +++++++++++++++++++
> MAINTAINERS | 8 +++
> 2 files changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> new file mode 100644
> index 000000000000..6a5bdafed56b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/cirrus,cs40l50.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus Logic CS40L50 Advanced Haptic Driver
> +
> +maintainers:
> + - James Ogletree <james.ogletree@cirrus.com>
Nit: this email address and the one in MAINTAINERS don't match the one
you're using to sign off. There is no requirement that they do; I just
wanted to check whether this was intentional.
> +
> +description:
> + CS40L50 is a haptic driver with waveform memory,
> + integrated DSP, and closed-loop algorithms.
> +
> +properties:
> + compatible:
> + enum:
> + - cirrus,cs40l50
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> +
> + va-supply:
> + description: Power supply for internal analog circuits.
> +
> + vp-supply:
> + description: Power supply for always-on circuits.
> +
> + vio-supply:
> + description: Power supply for digital input/output.
> +
> + vamp-supply:
> + description: Power supply for the Class D amplifier.
Does L50 support external boost mode? If not, it will always be shorted
directly to VBST on the board, and there is no reason to describe it in
the binding.
If external boost mode is supported, then I recommend extending support
for it in the driver. Perhaps some additional registers must be set if
this supply is present.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - reset-gpios
> + - vp-supply
Making VP a required supply is likely inconvenient for customers; 99% of
them connect it to a battery, and end up tying this property to a dummy
regulator to keep the driver from bleating.
Only for a wall-powered case would VP be tied to something like a 3.3-V
switching supply, and I imagine those cases are rare. It seems that VP
should be optional.
> + - vio-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
Nit: most device trees tend to use 8-column indentation as with kernel code.
> +
> + haptic-driver@34 {
> + compatible = "cirrus,cs40l50";
> + reg = <0x34>;
> + interrupt-parent = <&gpio>;
> + interrupts = <113 IRQ_TYPE_LEVEL_LOW>;
> + reset-gpios = <&gpio 112 GPIO_ACTIVE_LOW>;
> + vp-supply = <&vreg>;
> + vio-supply = <&vreg>;
Showing VP and VIO tied to the same supply is not a valid example; VP
typically connects to a battery, and VIO is likely a 1.8-V supply. Their
voltage ranges do not overlap, and hence they cannot be shared. I also
suspect there are sequencing restrictions between them as well.
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dd5de540ec0b..b71017a187f8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4933,6 +4933,14 @@ F: sound/pci/hda/cs*
> F: sound/pci/hda/hda_cs_dsp_ctl.*
> F: sound/soc/codecs/cs*
>
> +CIRRUS LOGIC HAPTIC DRIVERS
> +M: James Ogletree <james.ogletree@cirrus.com>
> +M: Fred Treven <fred.treven@cirrus.com>
> +M: Ben Bright <ben.bright@cirrus.com>
> +L: patches@opensource.cirrus.com
> +S: Supported
> +F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> +
> CIRRUS LOGIC DSP FIRMWARE DRIVER
> M: Simon Trimmer <simont@opensource.cirrus.com>
> M: Charles Keepax <ckeepax@opensource.cirrus.com>
> --
> 2.25.1
>
Kind regards,
Jeff LaBundy
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver
2024-03-08 22:24 ` [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver James Ogletree
@ 2024-03-10 23:40 ` Jeff LaBundy
2024-03-11 10:30 ` Charles Keepax
2024-03-14 21:03 ` James Ogletree
0 siblings, 2 replies; 22+ messages in thread
From: Jeff LaBundy @ 2024-03-10 23:40 UTC (permalink / raw
To: James Ogletree
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, patches, linux-sound, linux-input, devicetree
Hi James,
Again, great work! I think we're almost there; I have just a few
trailing comments related to maintainability.
On Fri, Mar 08, 2024 at 10:24:19PM +0000, James Ogletree wrote:
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
>
> The MFD component registers and initializes the device.
>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
> MAINTAINERS | 2 +
> drivers/mfd/Kconfig | 30 ++
> drivers/mfd/Makefile | 4 +
> drivers/mfd/cs40l50-core.c | 528 ++++++++++++++++++++++++++++++++++++
> drivers/mfd/cs40l50-i2c.c | 68 +++++
> drivers/mfd/cs40l50-spi.c | 68 +++++
> include/linux/mfd/cs40l50.h | 144 ++++++++++
> 7 files changed, 844 insertions(+)
> create mode 100644 drivers/mfd/cs40l50-core.c
> create mode 100644 drivers/mfd/cs40l50-i2c.c
> create mode 100644 drivers/mfd/cs40l50-spi.c
> create mode 100644 include/linux/mfd/cs40l50.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b71017a187f8..69a9e0a3b968 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4940,6 +4940,8 @@ M: Ben Bright <ben.bright@cirrus.com>
> L: patches@opensource.cirrus.com
> S: Supported
> F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> +F: drivers/mfd/cs40l*
> +F: include/linux/mfd/cs40l*
>
> CIRRUS LOGIC DSP FIRMWARE DRIVER
> M: Simon Trimmer <simont@opensource.cirrus.com>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 90ce58fd629e..6273c255f107 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2241,6 +2241,36 @@ config MCP_UCB1200_TS
>
> endmenu
>
> +config MFD_CS40L50_CORE
> + tristate
> + select MFD_CORE
> + select FW_CS_DSP
> + select REGMAP_IRQ
> +
> +config MFD_CS40L50_I2C
> + tristate "Cirrus Logic CS40L50 (I2C)"
> + select REGMAP_I2C
> + select MFD_CS40L50_CORE
> + depends on I2C
> + help
> + Select this to support the Cirrus Logic CS40L50 Haptic
> + Driver over I2C.
> +
> + This driver can be built as a module. If built as a module it will be
> + called "cs40l50-i2c".
> +
> +config MFD_CS40L50_SPI
> + tristate "Cirrus Logic CS40L50 (SPI)"
> + select REGMAP_SPI
> + select MFD_CS40L50_CORE
> + depends on SPI
> + help
> + Select this to support the Cirrus Logic CS40L50 Haptic
> + Driver over SPI.
> +
> + This driver can be built as a module. If built as a module it will be
> + called "cs40l50-spi".
> +
> config MFD_VEXPRESS_SYSREG
> tristate "Versatile Express System Registers"
> depends on VEXPRESS_CONFIG && GPIOLIB
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c66f07edcd0e..a8d18ba155d0 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -88,6 +88,10 @@ obj-$(CONFIG_MFD_MADERA) += madera.o
> obj-$(CONFIG_MFD_MADERA_I2C) += madera-i2c.o
> obj-$(CONFIG_MFD_MADERA_SPI) += madera-spi.o
>
> +obj-$(CONFIG_MFD_CS40L50_CORE) += cs40l50-core.o
> +obj-$(CONFIG_MFD_CS40L50_I2C) += cs40l50-i2c.o
> +obj-$(CONFIG_MFD_CS40L50_SPI) += cs40l50-spi.o
> +
> obj-$(CONFIG_TPS6105X) += tps6105x.o
> obj-$(CONFIG_TPS65010) += tps65010.o
> obj-$(CONFIG_TPS6507X) += tps6507x.o
> diff --git a/drivers/mfd/cs40l50-core.c b/drivers/mfd/cs40l50-core.c
> new file mode 100644
> index 000000000000..92e67f80f36a
> --- /dev/null
> +++ b/drivers/mfd/cs40l50-core.c
> @@ -0,0 +1,528 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#include <linux/firmware/cirrus/cs_dsp.h>
> +#include <linux/firmware/cirrus/wmfw.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regulator/consumer.h>
> +
> +static const struct mfd_cell cs40l50_devs[] = {
> + { .name = "cs40l50-codec", },
> + { .name = "cs40l50-vibra", },
> +};
> +
> +const struct regmap_config cs40l50_regmap = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .reg_format_endian = REGMAP_ENDIAN_BIG,
> + .val_format_endian = REGMAP_ENDIAN_BIG,
> +};
> +EXPORT_SYMBOL_GPL(cs40l50_regmap);
> +
> +static const char * const cs40l50_supplies[] = {
> + "vp", "vio",
> +};
Strictly speaking, the VA supply must be called out in the binding as
a required supply; the device cannot function without it. I think you
have gotten away without handling VA because VA and VIO are likely tied
together on your board.
Are there any practical applications where VA and VIO would be split
apart on the board? For example, can VIO be tied to a 3.3-V supply in
support of 3.3-V logic, while VA remains powered by 1.8 V? If so, the
driver needs to provide a means to supply a separate VA regulator, and
the binding needs to specify that oneOf them is required.
If VA and VIO are recommended to be tied together, then I don't think
they need to be defined separately in the binding. We should only ask
customers to supply one 1.8-V regulator; perhaps the description can
mention how this regulator maps to the VA and VIO pins of the device.
For older devices like L25, VA and VIO were shorted inside the package.
The same goes for VAMP; it seems the intent here is to support internal
boost mode only, with VAMP and VBST shorted on the board. Therefore, I
don't see a need to define VAMP in the binding either. If you intend to
define VAMP in the binding, then the driver should support external
boost mode, otherwise the driver is incomplete. It seems unlikely that
customers would use external boost mode for a haptic application; this
is probably more common for audio applications that borrow the same die.
As I mention in the binding, VP should really be optional in terms of
regulator support; it's almost always connected to a battery, and not
a regulator.
> +
> +static const struct regmap_irq cs40l50_reg_irqs[] = {
> + REGMAP_IRQ_REG(CS40L50_DSP_QUEUE_IRQ, CS40L50_IRQ1_INT_2_OFFSET,
> + CS40L50_DSP_QUEUE_MASK),
> + REGMAP_IRQ_REG(CS40L50_AMP_SHORT_IRQ, CS40L50_IRQ1_INT_1_OFFSET,
> + CS40L50_AMP_SHORT_MASK),
> + REGMAP_IRQ_REG(CS40L50_TEMP_ERR_IRQ, CS40L50_IRQ1_INT_8_OFFSET,
> + CS40L50_TEMP_ERR_MASK),
> + REGMAP_IRQ_REG(CS40L50_BST_UVP_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
> + CS40L50_BST_UVP_MASK),
> + REGMAP_IRQ_REG(CS40L50_BST_SHORT_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
> + CS40L50_BST_SHORT_MASK),
> + REGMAP_IRQ_REG(CS40L50_BST_ILIMIT_IRQ, CS40L50_IRQ1_INT_9_OFFSET,
> + CS40L50_BST_ILIMIT_MASK),
> + REGMAP_IRQ_REG(CS40L50_UVLO_VDDBATT_IRQ, CS40L50_IRQ1_INT_10_OFFSET,
> + CS40L50_UVLO_VDDBATT_MASK),
> + REGMAP_IRQ_REG(CS40L50_GLOBAL_ERROR_IRQ, CS40L50_IRQ1_INT_18_OFFSET,
> + CS40L50_GLOBAL_ERROR_MASK),
> +};
> +
> +static struct regmap_irq_chip cs40l50_irq_chip = {
> + .name = "cs40l50",
> +
> + .status_base = CS40L50_IRQ1_INT_1,
> + .mask_base = CS40L50_IRQ1_MASK_1,
> + .ack_base = CS40L50_IRQ1_INT_1,
> + .num_regs = 22,
> +
> + .irqs = cs40l50_reg_irqs,
> + .num_irqs = ARRAY_SIZE(cs40l50_reg_irqs),
> +
> + .runtime_pm = true,
> +};
No need for newlines in here.
> +
> +int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val)
> +{
> + int err, i;
It seems this series uses 'err', 'error' and 'ret' throughout. I know that
'error' is typically requested in the input subsystem, but I see no reason
not to stick with 'ret' for the rest of the series.
> + u32 ack;
> +
> + /* Device NAKs if hibernating, so optionally retry */
> + for (i = 0; i < CS40L50_DSP_TIMEOUT_COUNT; i++) {
> + err = regmap_write(regmap, CS40L50_DSP_QUEUE, val);
> + if (!err)
> + break;
> +
> + usleep_range(CS40L50_DSP_POLL_US, CS40L50_DSP_POLL_US + 100);
> + }
> +
> + /* If write never occurred, don't bother polling for ACK */
> + if (i == CS40L50_DSP_TIMEOUT_COUNT) {
> + dev_err(dev, "Timed out writing %#X to DSP: %d\n", val, err);
> + return err;
> + }
> +
> + err = regmap_read_poll_timeout(regmap, CS40L50_DSP_QUEUE, ack, !ack,
> + CS40L50_DSP_POLL_US,
> + CS40L50_DSP_POLL_US * CS40L50_DSP_TIMEOUT_COUNT);
> + if (err)
> + dev_err(dev, "DSP did not ACK %#X: %d\n", val, err);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(cs40l50_dsp_write);
> +
> +static const struct cs_dsp_region cs40l50_dsp_regions[] = {
> + { .type = WMFW_HALO_PM_PACKED, .base = CS40L50_PMEM_0 },
> + { .type = WMFW_HALO_XM_PACKED, .base = CS40L50_XMEM_PACKED_0 },
> + { .type = WMFW_HALO_YM_PACKED, .base = CS40L50_YMEM_PACKED_0 },
> + { .type = WMFW_ADSP2_XM, .base = CS40L50_XMEM_UNPACKED24_0 },
> + { .type = WMFW_ADSP2_YM, .base = CS40L50_YMEM_UNPACKED24_0 },
> +};
> +
> +static const struct reg_sequence cs40l50_internal_vamp_config[] = {
> + { CS40L50_BST_LPMODE_SEL, CS40L50_DCM_LOW_POWER },
> + { CS40L50_BLOCK_ENABLES2, CS40L50_OVERTEMP_WARN },
> +};
I recommend against aligning the second values here and below. If another
register with a long name is added later, the patch would touch more lines
than needed in order to restore the alignment. What you've done for the
cs40l50_dsp_regions[] array is safer.
> +
> +static const struct reg_sequence cs40l50_irq_mask_override[] = {
> + { CS40L50_IRQ1_MASK_2, CS40L50_IRQ_MASK_2_OVERRIDE },
> + { CS40L50_IRQ1_MASK_20, CS40L50_IRQ_MASK_20_OVERRIDE },
> +};
> +
> +static int cs40l50_configure_dsp(struct cs_dsp *dsp)
> +{
> + struct cs40l50 *cs40l50 = container_of(dsp, struct cs40l50, dsp);
> + u32 nwaves;
> + int err;
> +
> + /* Log number of effects if wavetable was loaded */
> + if (cs40l50->bin) {
Is there any other clue you can use to discern whether a wavetable was
loaded? The memory at cs40l50->bin is gone now, and although you're not
dereferencing it anymore, another contributor might be fooled into doing
so down the road.
Maybe a boolean would be more maintainable; I don't feel strongly about
it though.
> + err = regmap_read(dsp->regmap, CS40L50_NUM_WAVES, &nwaves);
> + if (err)
> + return err;
> +
> + dev_info(dsp->dev, "%u effects loaded\n", nwaves);
> + }
> +
> + cs40l50->wseqs[CS40L50_PWR_ON].ctl = cs_dsp_get_ctl(dsp, "PM_PWR_ON_SEQ",
> + WMFW_ADSP2_XM,
> + CS40L50_PM_ALGO);
> + if (!cs40l50->wseqs[CS40L50_PWR_ON].ctl) {
> + dev_err(dsp->dev, "No control for power-on write sequence\n");
> + return -ENOENT;
> + }
> +
> + /* Initialize the power-on write sequencer */
> + err = cs_dsp_wseq_init(dsp, cs40l50->wseqs, 1);
> + if (err) {
> + dev_err(dsp->dev, "Failed to initialize power-on write sequence\n");
> + return err;
> + }
> +
> + /* Configure internal V_AMP supply */
> + err = regmap_multi_reg_write(dsp->regmap, cs40l50_internal_vamp_config,
> + ARRAY_SIZE(cs40l50_internal_vamp_config));
> + if (err)
> + return err;
> +
> + err = cs_dsp_wseq_multi_write(dsp, &cs40l50->wseqs[CS40L50_PWR_ON],
> + cs40l50_internal_vamp_config, CS_DSP_WSEQ_FULL,
> + ARRAY_SIZE(cs40l50_internal_vamp_config), false);
> + if (err)
> + return err;
> +
> + /* Override firmware defaults for IRQ masks */
> + err = regmap_multi_reg_write(dsp->regmap, cs40l50_irq_mask_override,
> + ARRAY_SIZE(cs40l50_irq_mask_override));
> + if (err)
> + return err;
> +
> + err = cs_dsp_wseq_multi_write(dsp, &cs40l50->wseqs[CS40L50_PWR_ON],
> + cs40l50_irq_mask_override, CS_DSP_WSEQ_FULL,
> + ARRAY_SIZE(cs40l50_irq_mask_override), false);
> + if (err)
> + return err;
> +
> + /* Add child devices now that DSP is running */
> + err = devm_mfd_add_devices(dsp->dev, PLATFORM_DEVID_NONE, cs40l50_devs,
> + ARRAY_SIZE(cs40l50_devs), NULL, 0, NULL);
> + if (err)
> + dev_err(dsp->dev, "Failed to add child devices: %d\n", err);
> +
> + return err;
> +}
> +
> +static const struct cs_dsp_client_ops client_ops = {
> + .post_run = cs40l50_configure_dsp,
> +};
> +
> +static void cs40l50_dsp_remove(void *data)
> +{
> + cs_dsp_remove(data);
> +}
> +
> +static int cs40l50_dsp_init(struct cs40l50 *cs40l50)
> +{
> + int err;
> +
> + 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 = &client_ops;
> +
> + err = cs_dsp_halo_init(&cs40l50->dsp);
> + if (err)
> + return err;
> +
> + return devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_remove,
> + &cs40l50->dsp);
> +}
> +
> +static void cs40l50_dsp_power_down(void *data)
> +{
> + cs_dsp_power_down(data);
> +}
> +
> +static void cs40l50_dsp_stop(void *data)
> +{
> + cs_dsp_stop(data);
> +}
> +
> +static void cs40l50_start_dsp(const struct firmware *bin, void *context)
> +{
> + struct cs40l50 *cs40l50 = context;
> + int err;
> +
> + /* Wavetable is optional; start DSP regardless */
> + cs40l50->bin = bin;
> +
> + mutex_lock(&cs40l50->lock);
It seems the mutex is used only to prevent interrupt handling while the DSP
is being configured; can't we just call disable_irq() and enable_irq() here?
> +
> + err = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_SHUTDOWN);
> + if (err)
> + goto err_mutex;
> +
> + err = cs_dsp_power_up(&cs40l50->dsp, cs40l50->fw, "cs40l50.wmfw",
> + cs40l50->bin, "cs40l50.bin", "cs40l50");
> + if (err)
> + goto err_mutex;
> +
> + err = devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_power_down,
> + &cs40l50->dsp);
> + if (err)
> + goto err_mutex;
> +
> + err = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_SYSTEM_RESET);
> + if (err)
> + goto err_mutex;
> +
> + err = cs40l50_dsp_write(cs40l50->dev, cs40l50->regmap, CS40L50_PREVENT_HIBER);
> + if (err)
> + goto err_mutex;
> +
> + err = cs_dsp_run(&cs40l50->dsp);
> + if (err)
> + goto err_mutex;
> +
> + err = devm_add_action_or_reset(cs40l50->dev, cs40l50_dsp_stop, &cs40l50->dsp);
In the case of L25, it was common to stop and re-start the DSP for either
of the following reasons:
1. Swap out .wmfw files. Eventually, different algorithms got so large that
different .wmfw files were needed for different use-cases (e.g. A2H vs.
f0 tracking). No matter how much RAM gets added to the HALO core, there
is never enough! :)
2. Respond to spurious reset. With one customer in particular, the reset
GPIO traveled over a flex cable, and the connector suffered some contact
bounce during drop testing.
In general, things that should be able to be executed an infinite number of
times (e.g. register writes) should be logically separate from things that
should only be executed once, like a call to devm_add_action_or_reset().
The same is true for cs40l50_configure_dsp(); it contains a mix of register
writes, but then a call to devm_mfd_add_devices(). Please consider separating
some of these tasks so that rip-up is minimal in case you need to restore or
reconfigure the device on-the-fly at a later time.
> +err_mutex:
> + mutex_unlock(&cs40l50->lock);
> + release_firmware(cs40l50->bin);
> + release_firmware(cs40l50->fw);
> +
> + if (err)
> + dev_err(cs40l50->dev, "Failed to start DSP: %d", err);
The fact that we have so much clean-up in the error path may suggest that
the bulk of this function belongs in a helper, e.g. __cs40l50_start_dsp(),
with return type of int.
> +}
> +
> +static void cs40l50_request_firmware(const struct firmware *fw, void *context)
> +{
> + struct cs40l50 *cs40l50 = context;
> +
> + if (!fw) {
> + dev_err(cs40l50->dev, "No firmware file found\n");
> + return;
> + }
> +
> + cs40l50->fw = fw;
> +
> + if (request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, CS40L50_WT,
> + cs40l50->dev, GFP_KERNEL, cs40l50,
> + cs40l50_start_dsp)) {
> + dev_err(cs40l50->dev, "Failed to request %s\n", CS40L50_WT);
Please record the return value of request_firmware_nowait() and print it
in the error message. It makes it much easier for Luddites such as myself
to debug ;)
> + release_firmware(cs40l50->fw);
> + }
> +}
> +
> +struct cs40l50_irq {
> + const char *name;
> + int virq;
> +};
> +
> +static struct cs40l50_irq cs40l50_irqs[] = {
> + { "DSP", },
> + { "Global", },
> + { "Boost UVLO", },
> + { "Boost current limit", },
> + { "Boost short", },
> + { "Boost undervolt", },
> + { "Overtemp", },
> + { "Amp short", },
> +};
> +
> +static const struct reg_sequence cs40l50_err_rls[] = {
> + { CS40L50_ERR_RLS, CS40L50_GLOBAL_ERR_RLS_SET },
> + { CS40L50_ERR_RLS, CS40L50_GLOBAL_ERR_RLS_CLEAR },
> +};
> +
> +static irqreturn_t cs40l50_hw_err(int irq, void *data)
> +{
> + struct cs40l50 *cs40l50 = data;
> + int err = 0, i;
> +
> + mutex_lock(&cs40l50->lock);
> +
> + /* Log hardware interrupt and execute error release sequence */
> + for (i = 1; i < ARRAY_SIZE(cs40l50_irqs); i++) {
> + if (cs40l50_irqs[i].virq == irq) {
> + dev_err(cs40l50->dev, "%s error\n", cs40l50_irqs[i].name);
> + err = regmap_multi_reg_write(cs40l50->regmap, cs40l50_err_rls,
> + ARRAY_SIZE(cs40l50_err_rls));
> + goto exit;
There is no need for a goto label here; just break.
> + }
> + }
> +exit:
> + mutex_unlock(&cs40l50->lock);
> + return IRQ_RETVAL(!err);
> +}
> +
> +static irqreturn_t cs40l50_dsp_queue(int irq, void *data)
> +{
> + struct cs40l50 *cs40l50 = data;
> + u32 rd_ptr, val, wt_ptr;
> + int err = 0;
> +
> + mutex_lock(&cs40l50->lock);
> +
> + /* Read from DSP queue, log, and update read pointer */
> + while (!err) {
> + err = regmap_read(cs40l50->regmap, CS40L50_DSP_QUEUE_WT, &wt_ptr);
> + if (err)
> + goto exit;
Same here, and throughout; just break out of the loop.
> +
> + err = regmap_read(cs40l50->regmap, CS40L50_DSP_QUEUE_RD, &rd_ptr);
> + if (err)
> + goto exit;
> +
> + /* Check if queue is empty */
> + if (wt_ptr == rd_ptr)
> + goto exit;
> +
> + err = regmap_read(cs40l50->regmap, rd_ptr, &val);
> + if (err)
> + goto exit;
> +
> + dev_dbg(cs40l50->dev, "DSP payload: %#X", val);
> +
> + rd_ptr += sizeof(u32);
> +
> + if (rd_ptr > CS40L50_DSP_QUEUE_END)
> + rd_ptr = CS40L50_DSP_QUEUE_BASE;
> +
> + err = regmap_write(cs40l50->regmap, CS40L50_DSP_QUEUE_RD, rd_ptr);
> + if (err)
> + goto exit;
> + }
> +exit:
> + mutex_unlock(&cs40l50->lock);
> +
> + return IRQ_RETVAL(!err);
> +}
> +
> +static int cs40l50_irq_init(struct cs40l50 *cs40l50)
> +{
> + struct device *dev = cs40l50->dev;
> + int err, i, virq;
> +
> + err = devm_regmap_add_irq_chip(dev, cs40l50->regmap, cs40l50->irq,
> + IRQF_ONESHOT | IRQF_SHARED, 0,
> + &cs40l50_irq_chip, &cs40l50->irq_data);
> + if (err) {
> + dev_err(dev, "Failed adding IRQ chip\n");
I don't see any need for individual prints in this function, since the call
to cs40l50_irq_init() is already followed by a call to dev_err_probe().
> + return err;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(cs40l50_irqs); i++) {
> + virq = regmap_irq_get_virq(cs40l50->irq_data, i);
> + if (virq < 0) {
> + dev_err(dev, "Failed getting %s\n", cs40l50_irqs[i].name);
> + return virq;
> + }
> +
> + cs40l50_irqs[i].virq = virq;
> +
> + /* Handle DSP and hardware interrupts separately */
> + err = devm_request_threaded_irq(dev, virq, NULL,
> + i ? cs40l50_hw_err : cs40l50_dsp_queue,
> + IRQF_ONESHOT | IRQF_SHARED,
> + cs40l50_irqs[i].name, cs40l50);
> + if (err) {
> + dev_err(dev, "Failed requesting %s IRQ\n",
> + cs40l50_irqs[i].name);
> + return err;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int cs40l50_get_model(struct cs40l50 *cs40l50)
> +{
> + int err;
> +
> + err = regmap_read(cs40l50->regmap, CS40L50_DEVID, &cs40l50->devid);
> + if (err)
> + return err;
> +
> + if (cs40l50->devid != CS40L50_DEVID_A)
> + return -EINVAL;
> +
> + err = regmap_read(cs40l50->regmap, CS40L50_REVID, &cs40l50->revid);
> + if (err)
> + return err;
> +
> + if (cs40l50->revid < CS40L50_REVID_B0)
> + return -EINVAL;
> +
> + dev_info(cs40l50->dev, "Cirrus Logic CS40L50 rev. %02X\n", cs40l50->revid);
This should be dev_dbg().
> +
> + return 0;
> +}
> +
> +static int cs40l50_pm_runtime_setup(struct device *dev)
> +{
> + int err;
> +
> + pm_runtime_set_autosuspend_delay(dev, CS40L50_AUTOSUSPEND_MS);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_get_noresume(dev);
> + err = pm_runtime_set_active(dev);
> + if (err)
> + return err;
> +
> + return devm_pm_runtime_enable(dev);
> +}
> +
> +int cs40l50_probe(struct cs40l50 *cs40l50)
> +{
> + struct device *dev = cs40l50->dev;
> + int err;
> +
> + mutex_init(&cs40l50->lock);
> +
> + cs40l50->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
> + if (IS_ERR(cs40l50->reset_gpio))
> + return dev_err_probe(dev, PTR_ERR(cs40l50->reset_gpio),
> + "Failed getting reset GPIO\n");
> +
> + err = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(cs40l50_supplies),
> + cs40l50_supplies);
> + if (err)
> + return dev_err_probe(dev, err, "Failed getting supplies\n");
> +
> + /* Ensure minimum reset pulse width */
> + usleep_range(CS40L50_RESET_PULSE_US, CS40L50_RESET_PULSE_US + 100);
> +
> + gpiod_set_value_cansleep(cs40l50->reset_gpio, 0);
> +
> + /* Wait for control port to be ready */
> + usleep_range(CS40L50_CP_READY_US, CS40L50_CP_READY_US + 100);
> +
> + err = cs40l50_dsp_init(cs40l50);
> + if (err)
> + return dev_err_probe(dev, err, "Failed to initialize DSP\n");
> +
> + err = cs40l50_pm_runtime_setup(dev);
> + if (err)
> + return dev_err_probe(dev, err, "Failed to initialize runtime PM\n");
> +
> + err = cs40l50_get_model(cs40l50);
> + if (err)
> + return dev_err_probe(dev, err, "Failed to get part number\n");
I think cs40l50_get_model() should be called directly after usleep_range() is
called; there is no use in doing any other initialization if we can't talk I2C
or SPI, or the model is invalid.
> +
> + err = cs40l50_irq_init(cs40l50);
> + if (err)
> + return dev_err_probe(dev, err, "Failed to request IRQs\n");
> +
> + err = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, CS40L50_FW,
> + dev, GFP_KERNEL, cs40l50, cs40l50_request_firmware);
> + if (err)
> + return dev_err_probe(dev, err, "Failed to request %s\n", CS40L50_FW);
> +
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(cs40l50_probe);
> +
> +int cs40l50_remove(struct cs40l50 *cs40l50)
> +{
> + gpiod_set_value_cansleep(cs40l50->reset_gpio, 1);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(cs40l50_remove);
> +
> +static int cs40l50_runtime_suspend(struct device *dev)
> +{
> + struct cs40l50 *cs40l50 = dev_get_drvdata(dev);
> +
> + return regmap_write(cs40l50->regmap, CS40L50_DSP_QUEUE, CS40L50_ALLOW_HIBER);
> +}
> +
> +static int cs40l50_runtime_resume(struct device *dev)
> +{
> + struct cs40l50 *cs40l50 = dev_get_drvdata(dev);
> +
> + return cs40l50_dsp_write(dev, cs40l50->regmap, CS40L50_PREVENT_HIBER);
> +}
> +
> +EXPORT_GPL_DEV_PM_OPS(cs40l50_pm_ops) = {
> + RUNTIME_PM_OPS(cs40l50_runtime_suspend, cs40l50_runtime_resume, NULL)
> +};
> +
> +MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(FW_CS_DSP);
> diff --git a/drivers/mfd/cs40l50-i2c.c b/drivers/mfd/cs40l50-i2c.c
> new file mode 100644
> index 000000000000..639be743d956
> --- /dev/null
> +++ b/drivers/mfd/cs40l50-i2c.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/mfd/cs40l50.h>
> +
> +static int cs40l50_i2c_probe(struct i2c_client *i2c)
> +{
> + struct cs40l50 *cs40l50;
> +
> + cs40l50 = devm_kzalloc(&i2c->dev, sizeof(*cs40l50), GFP_KERNEL);
> + if (!cs40l50)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(i2c, cs40l50);
> +
> + cs40l50->dev = &i2c->dev;
> + cs40l50->irq = i2c->irq;
> +
> + cs40l50->regmap = devm_regmap_init_i2c(i2c, &cs40l50_regmap);
> + if (IS_ERR(cs40l50->regmap))
> + return dev_err_probe(cs40l50->dev, PTR_ERR(cs40l50->regmap),
> + "Failed to initialize register map\n");
> +
> + return cs40l50_probe(cs40l50);
> +}
> +
> +static void cs40l50_i2c_remove(struct i2c_client *i2c)
> +{
> + struct cs40l50 *cs40l50 = i2c_get_clientdata(i2c);
> +
> + cs40l50_remove(cs40l50);
> +}
> +
> +static const struct i2c_device_id cs40l50_id_i2c[] = {
> + { "cs40l50" },
> + {}
> +};
> +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 struct i2c_driver cs40l50_i2c_driver = {
> + .driver = {
> + .name = "cs40l50",
> + .of_match_table = cs40l50_of_match,
> + .pm = pm_ptr(&cs40l50_pm_ops),
> + },
> + .id_table = cs40l50_id_i2c,
> + .probe = cs40l50_i2c_probe,
> + .remove = cs40l50_i2c_remove,
> +};
> +module_i2c_driver(cs40l50_i2c_driver);
> +
> +MODULE_DESCRIPTION("CS40L50 I2C Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/mfd/cs40l50-spi.c b/drivers/mfd/cs40l50-spi.c
> new file mode 100644
> index 000000000000..53526b595a0d
> --- /dev/null
> +++ b/drivers/mfd/cs40l50-spi.c
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/spi/spi.h>
> +
> +static int cs40l50_spi_probe(struct spi_device *spi)
> +{
> + struct cs40l50 *cs40l50;
> +
> + cs40l50 = devm_kzalloc(&spi->dev, sizeof(*cs40l50), GFP_KERNEL);
> + if (!cs40l50)
> + return -ENOMEM;
> +
> + spi_set_drvdata(spi, cs40l50);
> +
> + cs40l50->dev = &spi->dev;
> + cs40l50->irq = spi->irq;
> +
> + 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");
> +
> + return cs40l50_probe(cs40l50);
> +}
> +
> +static void cs40l50_spi_remove(struct spi_device *spi)
> +{
> + struct cs40l50 *cs40l50 = spi_get_drvdata(spi);
> +
> + cs40l50_remove(cs40l50);
> +}
> +
> +static const struct spi_device_id cs40l50_id_spi[] = {
> + { "cs40l50" },
> + {}
> +};
> +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 struct spi_driver cs40l50_spi_driver = {
> + .driver = {
> + .name = "cs40l50",
> + .of_match_table = cs40l50_of_match,
> + .pm = pm_ptr(&cs40l50_pm_ops),
> + },
> + .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@cirrus.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/mfd/cs40l50.h b/include/linux/mfd/cs40l50.h
> new file mode 100644
> index 000000000000..cc0661926918
> --- /dev/null
> +++ b/include/linux/mfd/cs40l50.h
> @@ -0,0 +1,144 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#ifndef __MFD_CS40L50_H__
> +#define __MFD_CS40L50_H__
> +
> +#include <linux/firmware/cirrus/cs_dsp.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/pm.h>
> +#include <linux/regmap.h>
> +
> +/* Power Supply Configuration */
> +#define CS40L50_BLOCK_ENABLES2 0x201C
> +#define CS40L50_ERR_RLS 0x2034
> +#define CS40L50_PWRMGT_CTL 0x2900
> +#define CS40L50_BST_LPMODE_SEL 0x3810
> +#define CS40L50_DCM_LOW_POWER 0x1
> +#define CS40L50_OVERTEMP_WARN 0x4000010
> +
> +/* Interrupts */
> +#define CS40L50_IRQ1_INT_1 0xE010
> +#define CS40L50_IRQ1_BASE CS40L50_IRQ1_INT_1
> +#define CS40L50_IRQ1_INT_2 0xE014
> +#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_MASK_1 0xE090
> +#define CS40L50_IRQ1_MASK_2 0xE094
> +#define CS40L50_IRQ1_MASK_20 0xE0DC
> +#define CS40L50_IRQ1_INT_1_OFFSET (CS40L50_IRQ1_INT_1 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_2_OFFSET (CS40L50_IRQ1_INT_2 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_8_OFFSET (CS40L50_IRQ1_INT_8 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_9_OFFSET (CS40L50_IRQ1_INT_9 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_10_OFFSET (CS40L50_IRQ1_INT_10 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ1_INT_18_OFFSET (CS40L50_IRQ1_INT_18 - CS40L50_IRQ1_BASE)
> +#define CS40L50_IRQ_MASK_2_OVERRIDE 0xFFDF7FFF
> +#define CS40L50_IRQ_MASK_20_OVERRIDE 0x15C01000
> +#define CS40L50_AMP_SHORT_MASK BIT(31)
> +#define CS40L50_DSP_QUEUE_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(18)
> +#define CS40L50_UVLO_VDDBATT_MASK BIT(16)
> +#define CS40L50_GLOBAL_ERROR_MASK BIT(15)
> +
> +enum cs40l50_irq_list {
> + CS40L50_DSP_QUEUE_IRQ,
> + 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_AMP_SHORT_IRQ,
> +};
> +
> +/* DSP */
> +#define CS40L50_XMEM_PACKED_0 0x2000000
> +#define CS40L50_XMEM_UNPACKED24_0 0x2800000
> +#define CS40L50_SYS_INFO_ID 0x25E0000
> +#define CS40L50_RAM_INIT 0x28021DC
> +#define CS40L50_DSP_QUEUE_WT 0x28042C8
> +#define CS40L50_DSP_QUEUE_RD 0x28042CC
> +#define CS40L50_NUM_WAVES 0x2805C18
> +#define CS40L50_CORE_BASE 0x2B80000
> +#define CS40L50_CCM_CORE_CONTROL 0x2BC1000
> +#define CS40L50_YMEM_PACKED_0 0x2C00000
> +#define CS40L50_YMEM_UNPACKED24_0 0x3400000
> +#define CS40L50_PMEM_0 0x3800000
> +#define CS40L50_MEM_RDY_HW 0x2
> +#define CS40L50_RAM_INIT_FLAG 0x1
I didn't see this #define used anywhere; please double check this one
and others.
> +#define CS40L50_CLOCK_DISABLE 0x80
> +#define CS40L50_CLOCK_ENABLE 0x281
> +#define CS40L50_DSP_POLL_US 1000
> +#define CS40L50_DSP_TIMEOUT_COUNT 100
> +#define CS40L50_RESET_PULSE_US 2200
> +#define CS40L50_CP_READY_US 3100
> +#define CS40L50_AUTOSUSPEND_MS 2000
> +#define CS40L50_PM_ALGO 0x9F206
> +#define CS40L50_GLOBAL_ERR_RLS_SET BIT(11)
> +#define CS40L50_GLOBAL_ERR_RLS_CLEAR 0
> +
> +enum cs40l50_wseqs {
> + CS40L50_PWR_ON,
> + CS40L50_STANDBY,
> + CS40L50_ACTIVE,
> + CS40L50_NUM_WSEQS,
> +};
> +
> +/* DSP Queue */
> +#define CS40L50_DSP_QUEUE_BASE 0x11004
> +#define CS40L50_DSP_QUEUE_END 0x1101C
> +#define CS40L50_DSP_QUEUE 0x11020
> +#define CS40L50_PREVENT_HIBER 0x2000003
> +#define CS40L50_ALLOW_HIBER 0x2000004
> +#define CS40L50_SHUTDOWN 0x2000005
> +#define CS40L50_SYSTEM_RESET 0x02000007
> +#define CS40L50_START_I2S 0x3000002
> +#define CS40L50_OWT_PUSH 0x3000008
> +#define CS40L50_STOP_PLAYBACK 0x5000000
> +#define CS40L50_OWT_DELETE 0xD000000
> +
> +/* Firmware files */
> +#define CS40L50_FW "cs40l50.wmfw"
> +#define CS40L50_WT "cs40l50.bin"
> +
> +/* Device */
> +#define CS40L50_DEVID 0x0
> +#define CS40L50_REVID 0x4
> +#define CS40L50_DEVID_A 0x40A50
> +#define CS40L50_REVID_B0 0xB0
> +
> +struct cs40l50 {
> + struct device *dev;
> + struct regmap *regmap;
> + struct mutex lock;
> + struct cs_dsp dsp;
> + struct gpio_desc *reset_gpio;
> + struct regmap_irq_chip_data *irq_data;
> + const struct firmware *fw;
> + const struct firmware *bin;
> + struct cs_dsp_wseq wseqs[CS40L50_NUM_WSEQS];
> + int irq;
> + u32 devid;
> + u32 revid;
> +};
> +
> +int cs40l50_dsp_write(struct device *dev, struct regmap *regmap, u32 val);
> +int cs40l50_probe(struct cs40l50 *cs40l50);
> +int cs40l50_remove(struct cs40l50 *cs40l50);
> +
> +extern const struct regmap_config cs40l50_regmap;
> +extern const struct dev_pm_ops cs40l50_pm_ops;
> +
> +#endif /* __MFD_CS40L50_H__ */
> --
> 2.25.1
>
Kind regards,
Jeff LaBundy
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
2024-03-10 19:29 ` Jeff LaBundy
@ 2024-03-11 6:31 ` Krzysztof Kozlowski
2024-03-14 14:42 ` Jeff LaBundy
0 siblings, 1 reply; 22+ messages in thread
From: Krzysztof Kozlowski @ 2024-03-11 6:31 UTC (permalink / raw
To: Jeff LaBundy, James Ogletree
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, patches, linux-sound, linux-input, devicetree
On 10/03/2024 20:29, Jeff LaBundy wrote:
>
>> + - vio-supply
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/gpio/gpio.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>
> Nit: most device trees tend to use 8-column indentation as with kernel code.
If you meant DTS, then kernel coding style applies, which does not use
spaces. But this is a binding, so please use indentation as defined by
writing-schema: 2 or 4 spaces.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface
2024-03-10 19:12 ` Jeff LaBundy
@ 2024-03-11 10:14 ` Charles Keepax
2024-03-14 14:40 ` Jeff LaBundy
0 siblings, 1 reply; 22+ messages in thread
From: Charles Keepax @ 2024-03-11 10:14 UTC (permalink / raw
To: Jeff LaBundy
Cc: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, lee, broonie, patches, linux-sound, linux-input,
devicetree
On Sun, Mar 10, 2024 at 02:12:16PM -0500, Jeff LaBundy wrote:
> On Fri, Mar 08, 2024 at 10:24:17PM +0000, James Ogletree wrote:
> > + switch (op_code) {
> > + case CS_DSP_WSEQ_FULL:
> > + cs_dsp_chunk_write(&ch, 32, op_new->address);
> > + cs_dsp_chunk_write(&ch, 32, op_new->data);
> > + break;
> > + case CS_DSP_WSEQ_L16:
> > + case CS_DSP_WSEQ_H16:
> > + cs_dsp_chunk_write(&ch, 24, op_new->address);
> > + cs_dsp_chunk_write(&ch, 16, op_new->data);
> > + break;
> > + default:
> > + ret = -EINVAL;
> > + cs_dsp_err(dsp, "Op code not supported: %X\n", op_code);
> > + goto op_new_free;
>
> There is no need to drop down and call devm_kfree() here; it's sufficient
> to simply return -EINVAL. The devres core will free any instances of
> cs_dsp_wseq_op when the driver is unbound.
>
> I imagine you're calling devm_kfree() to protect against the case where
> the driver successfully probes, but the contents of the firmware are found
> to be invalid later. In that case, yes, this newly allocated memory will
> persist throughout the length of the driver's life.
>
> That's an error condition though; if it happens, the customer will surely
> remove the module, correct the .wmfw or .bin file, then insert the module
> again. All we need to do here is make sure that memory does not leak after
> the module is removed, and device-managed allocation already handles this.
>
I disagree here. This is the write function, there is no guarantee
this is even called at probe time. This is generic code going
into the DSP library and will presumably get re-used for different
purposes and on different parts in the future. Freeing on the error
path is much safer here.
> > +int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
> > + const struct reg_sequence *reg_seq, int num_regs,
> > + u8 op_code, bool update)
> > +{
> > + int ret, i;
> > +
> > + for (i = 0; i < num_regs; i++) {
> > + ret = cs_dsp_wseq_write(dsp, wseq, reg_seq[i].reg,
> > + reg_seq[i].def, update, op_code);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
>
> This is absolutely a nit-pick, but in cs_dsp_wseq_init(), you use the pattern
> of breaking on error and always returning ret; here you return on error. Both
> are functionally equivalent but it would be nice to be consistent in terms of
> style.
>
> If you do opt to match cs_dsp_wseq_multi_write() to cs_dsp_wseq_init(), I do
> think you'll need to initialize ret to zero here as well to avoid a compiler
> warning for the num_regs = 0 case.
I would be inclined to make cs_dsp_wseq_init function like this
one personally, rather than the other way around. Generally best
to return if there is no clean up to do.
Thanks,
Charles
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver
2024-03-10 23:40 ` Jeff LaBundy
@ 2024-03-11 10:30 ` Charles Keepax
2024-03-14 15:15 ` Jeff LaBundy
2024-03-14 21:03 ` James Ogletree
1 sibling, 1 reply; 22+ messages in thread
From: Charles Keepax @ 2024-03-11 10:30 UTC (permalink / raw
To: Jeff LaBundy
Cc: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, lee, broonie, patches, linux-sound, linux-input,
devicetree
On Sun, Mar 10, 2024 at 06:40:04PM -0500, Jeff LaBundy wrote:
> On Fri, Mar 08, 2024 at 10:24:19PM +0000, James Ogletree wrote:
> > +static void cs40l50_start_dsp(const struct firmware *bin, void *context)
> > +{
> > + struct cs40l50 *cs40l50 = context;
> > + int err;
> > +
> > + /* Wavetable is optional; start DSP regardless */
> > + cs40l50->bin = bin;
> > +
> > + mutex_lock(&cs40l50->lock);
>
> It seems the mutex is used only to prevent interrupt handling while the DSP
> is being configured; can't we just call disable_irq() and enable_irq() here?
>
The trouble with diabling the IRQ is it masks the IRQ line. That
means if the IRQ is shared with other devices you will be
silencing their IRQs for the duration as well, which is slightly
rude. Especially given the driver requests the IRQ with the
SHARED flag I would be inclinded to stick with the mutex unless
there are other reasons not to.
> > +static int cs40l50_irq_init(struct cs40l50 *cs40l50)
> > +{
> > + struct device *dev = cs40l50->dev;
> > + int err, i, virq;
> > +
> > + err = devm_regmap_add_irq_chip(dev, cs40l50->regmap, cs40l50->irq,
> > + IRQF_ONESHOT | IRQF_SHARED, 0,
> > + &cs40l50_irq_chip, &cs40l50->irq_data);
> > + if (err) {
> > + dev_err(dev, "Failed adding IRQ chip\n");
>
> I don't see any need for individual prints in this function, since the call
> to cs40l50_irq_init() is already followed by a call to dev_err_probe().
I would probably suggest keeping these individual prints here and
removing the one in cs40l50_probe, it is nicer to know exactly
what failed when something goes wrong. That said at least the
devm_request_threaded_irq can probe defer so that print should be
a dev_err_probe since this function is only called on the probe
path.
> > + dev_info(cs40l50->dev, "Cirrus Logic CS40L50 rev. %02X\n", cs40l50->revid);
>
> This should be dev_dbg().
>
Not sure what the concenus is on this, but personally I greatly
prefer these device ID prints being infos. Nice to always have
some indication of the device and its version.
Thanks,
Charles
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface
2024-03-11 10:14 ` Charles Keepax
@ 2024-03-14 14:40 ` Jeff LaBundy
2024-03-15 9:29 ` Charles Keepax
0 siblings, 1 reply; 22+ messages in thread
From: Jeff LaBundy @ 2024-03-14 14:40 UTC (permalink / raw
To: Charles Keepax
Cc: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, lee, broonie, patches, linux-sound, linux-input,
devicetree
Hi Charles,
On Mon, Mar 11, 2024 at 10:14:17AM +0000, Charles Keepax wrote:
> On Sun, Mar 10, 2024 at 02:12:16PM -0500, Jeff LaBundy wrote:
> > On Fri, Mar 08, 2024 at 10:24:17PM +0000, James Ogletree wrote:
> > > + switch (op_code) {
> > > + case CS_DSP_WSEQ_FULL:
> > > + cs_dsp_chunk_write(&ch, 32, op_new->address);
> > > + cs_dsp_chunk_write(&ch, 32, op_new->data);
> > > + break;
> > > + case CS_DSP_WSEQ_L16:
> > > + case CS_DSP_WSEQ_H16:
> > > + cs_dsp_chunk_write(&ch, 24, op_new->address);
> > > + cs_dsp_chunk_write(&ch, 16, op_new->data);
> > > + break;
> > > + default:
> > > + ret = -EINVAL;
> > > + cs_dsp_err(dsp, "Op code not supported: %X\n", op_code);
> > > + goto op_new_free;
> >
> > There is no need to drop down and call devm_kfree() here; it's sufficient
> > to simply return -EINVAL. The devres core will free any instances of
> > cs_dsp_wseq_op when the driver is unbound.
> >
> > I imagine you're calling devm_kfree() to protect against the case where
> > the driver successfully probes, but the contents of the firmware are found
> > to be invalid later. In that case, yes, this newly allocated memory will
> > persist throughout the length of the driver's life.
> >
> > That's an error condition though; if it happens, the customer will surely
> > remove the module, correct the .wmfw or .bin file, then insert the module
> > again. All we need to do here is make sure that memory does not leak after
> > the module is removed, and device-managed allocation already handles this.
> >
>
> I disagree here. This is the write function, there is no guarantee
> this is even called at probe time. This is generic code going
> into the DSP library and will presumably get re-used for different
> purposes and on different parts in the future. Freeing on the error
> path is much safer here.
Agreed that this won't be called during probe; all I mean to say is
that I don't see any issue in hanging on to a bit of memory while the
device is in an error state, before the module is unloaded and devres
unrolls all of the device-managed resources.
It's also perfectly fine to leave this as-is; the existing method is
functionally correct, and I understand the perspective of having the
generic library clean up immediately. If that's the intent however,
the same error handling needs to be applied in cs_dsp_populate_wseq();
currently only cs_dsp_wseq_write() calls devm_kfree() on error. Since
L50 uses asynchronous FW loading, neither are called until after the
device probes.
>
> > > +int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
> > > + const struct reg_sequence *reg_seq, int num_regs,
> > > + u8 op_code, bool update)
> > > +{
> > > + int ret, i;
> > > +
> > > + for (i = 0; i < num_regs; i++) {
> > > + ret = cs_dsp_wseq_write(dsp, wseq, reg_seq[i].reg,
> > > + reg_seq[i].def, update, op_code);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + return 0;
> >
> > This is absolutely a nit-pick, but in cs_dsp_wseq_init(), you use the pattern
> > of breaking on error and always returning ret; here you return on error. Both
> > are functionally equivalent but it would be nice to be consistent in terms of
> > style.
> >
> > If you do opt to match cs_dsp_wseq_multi_write() to cs_dsp_wseq_init(), I do
> > think you'll need to initialize ret to zero here as well to avoid a compiler
> > warning for the num_regs = 0 case.
>
> I would be inclined to make cs_dsp_wseq_init function like this
> one personally, rather than the other way around. Generally best
> to return if there is no clean up to do.
Makes sense to me.
>
> Thanks,
> Charles
Kind regards,
Jeff LaBundy
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
2024-03-11 6:31 ` Krzysztof Kozlowski
@ 2024-03-14 14:42 ` Jeff LaBundy
0 siblings, 0 replies; 22+ messages in thread
From: Jeff LaBundy @ 2024-03-14 14:42 UTC (permalink / raw
To: Krzysztof Kozlowski
Cc: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, lee, broonie, patches, linux-sound, linux-input,
devicetree
Hi Krzysztof,
On Mon, Mar 11, 2024 at 07:31:36AM +0100, Krzysztof Kozlowski wrote:
> On 10/03/2024 20:29, Jeff LaBundy wrote:
> >
> >> + - vio-supply
> >> +
> >> +additionalProperties: false
> >> +
> >> +examples:
> >> + - |
> >> + #include <dt-bindings/gpio/gpio.h>
> >> + #include <dt-bindings/interrupt-controller/irq.h>
> >> +
> >> + i2c {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >
> > Nit: most device trees tend to use 8-column indentation as with kernel code.
>
> If you meant DTS, then kernel coding style applies, which does not use
> spaces. But this is a binding, so please use indentation as defined by
> writing-schema: 2 or 4 spaces.
Good to know; I'll follow this definition moving forward.
>
>
> Best regards,
> Krzysztof
>
Kind regards,
Jeff LaBundy
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver
2024-03-11 10:30 ` Charles Keepax
@ 2024-03-14 15:15 ` Jeff LaBundy
0 siblings, 0 replies; 22+ messages in thread
From: Jeff LaBundy @ 2024-03-14 15:15 UTC (permalink / raw
To: Charles Keepax
Cc: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, lee, broonie, patches, linux-sound, linux-input,
devicetree
Hi Charles,
On Mon, Mar 11, 2024 at 10:30:53AM +0000, Charles Keepax wrote:
> On Sun, Mar 10, 2024 at 06:40:04PM -0500, Jeff LaBundy wrote:
> > On Fri, Mar 08, 2024 at 10:24:19PM +0000, James Ogletree wrote:
> > > +static void cs40l50_start_dsp(const struct firmware *bin, void *context)
> > > +{
> > > + struct cs40l50 *cs40l50 = context;
> > > + int err;
> > > +
> > > + /* Wavetable is optional; start DSP regardless */
> > > + cs40l50->bin = bin;
> > > +
> > > + mutex_lock(&cs40l50->lock);
> >
> > It seems the mutex is used only to prevent interrupt handling while the DSP
> > is being configured; can't we just call disable_irq() and enable_irq() here?
> >
>
> The trouble with diabling the IRQ is it masks the IRQ line. That
> means if the IRQ is shared with other devices you will be
> silencing their IRQs for the duration as well, which is slightly
> rude. Especially given the driver requests the IRQ with the
> SHARED flag I would be inclinded to stick with the mutex unless
> there are other reasons not to.
Ah, yes; I see we're using IRQF_SHARED here. Shared interrupts seem
like they made more sense for L+R audio amplifiers, and less for a
haptic driver; but if customers are indeed sharing this interrupt,
then the mutex is OK.
>
> > > +static int cs40l50_irq_init(struct cs40l50 *cs40l50)
> > > +{
> > > + struct device *dev = cs40l50->dev;
> > > + int err, i, virq;
> > > +
> > > + err = devm_regmap_add_irq_chip(dev, cs40l50->regmap, cs40l50->irq,
> > > + IRQF_ONESHOT | IRQF_SHARED, 0,
> > > + &cs40l50_irq_chip, &cs40l50->irq_data);
> > > + if (err) {
> > > + dev_err(dev, "Failed adding IRQ chip\n");
> >
> > I don't see any need for individual prints in this function, since the call
> > to cs40l50_irq_init() is already followed by a call to dev_err_probe().
>
> I would probably suggest keeping these individual prints here and
> removing the one in cs40l50_probe, it is nicer to know exactly
> what failed when something goes wrong. That said at least the
> devm_request_threaded_irq can probe defer so that print should be
> a dev_err_probe since this function is only called on the probe
> path.
Makes sense to me.
>
> > > + dev_info(cs40l50->dev, "Cirrus Logic CS40L50 rev. %02X\n", cs40l50->revid);
> >
> > This should be dev_dbg().
> >
>
> Not sure what the concenus is on this, but personally I greatly
> prefer these device ID prints being infos. Nice to always have
> some indication of the device and its version.
Right, but every other silicon vendor feels the same way too, and many
customers' dmesg is typically quite messy for the first couple minutes.
This is only one print, but my stance is that this type of information
belongs in debugfs; in fact for this case, we can get this information
from regmap if we know what we're looking for.
>
> Thanks,
> Charles
Kind regards,
Jeff LaBundy
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
2024-03-08 22:24 ` [PATCH v9 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver James Ogletree
@ 2024-03-14 15:54 ` Jeff LaBundy
2024-03-15 20:23 ` James Ogletree
0 siblings, 1 reply; 22+ messages in thread
From: Jeff LaBundy @ 2024-03-14 15:54 UTC (permalink / raw
To: James Ogletree
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, patches, linux-sound, linux-input, devicetree
Hi James,
On Fri, Mar 08, 2024 at 10:24:20PM +0000, James Ogletree wrote:
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
>
> The input driver provides the interface for control of
> haptic effects through the device.
>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
> v7: v8: v9:
> Please advise if playback stop is still misused with respect to not
> specifying an effect ID. The device can only play one effect at a
> time, but setting max effects for the EVIOCGEFFECTS ioctl to 1 would
> restrict the number of uploads to 1 as well. So I'm unsure of the correct
> approach here.
>
> MAINTAINERS | 1 +
> drivers/input/misc/Kconfig | 10 +
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/cs40l50-vibra.c | 578 +++++++++++++++++++++++++++++
> 4 files changed, 590 insertions(+)
> create mode 100644 drivers/input/misc/cs40l50-vibra.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 69a9e0a3b968..24cfb4f017bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4940,6 +4940,7 @@ M: Ben Bright <ben.bright@cirrus.com>
> L: patches@opensource.cirrus.com
> S: Supported
> F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> +F: drivers/input/misc/cs40l*
> F: drivers/mfd/cs40l*
> F: include/linux/mfd/cs40l*
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 6ba984d7f0b1..ee45dbb0636e 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -140,6 +140,16 @@ config INPUT_BMA150
> To compile this driver as a module, choose M here: the
> module will be called bma150.
>
> +config INPUT_CS40L50_VIBRA
> + tristate "CS40L50 Haptic Driver support"
> + depends on MFD_CS40L50_CORE
> + help
> + Say Y here to enable support for Cirrus Logic's CS40L50
> + haptic driver.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called cs40l50-vibra.
> +
> 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..88279de6d3d5 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -28,6 +28,7 @@ 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_VIBRA) += cs40l50-vibra.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-vibra.c b/drivers/input/misc/cs40l50-vibra.c
> new file mode 100644
> index 000000000000..c7e68862bbfe
> --- /dev/null
> +++ b/drivers/input/misc/cs40l50-vibra.c
> @@ -0,0 +1,578 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * CS40L50 Advanced Haptic Driver with waveform memory,
> + * integrated DSP, and closed-loop algorithms
> + *
> + * Copyright 2024 Cirrus Logic, Inc.
> + *
> + * Author: James Ogletree <james.ogletree@cirrus.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/input.h>
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +/* Wavetables */
> +#define CS40L50_RAM_INDEX_START 0x1000000
> +#define CS40L50_RAM_INDEX_END 0x100007F
> +#define CS40L50_RTH_INDEX_START 0x1400000
> +#define CS40L50_RTH_INDEX_END 0x1400001
> +#define CS40L50_ROM_INDEX_START 0x1800000
> +#define CS40L50_ROM_INDEX_END 0x180001A
> +#define CS40L50_TYPE_PCM 8
> +#define CS40L50_TYPE_PWLE 12
> +#define CS40L50_PCM_ID 0x0
> +#define CS40L50_OWT_CUSTOM_DATA_SIZE 2
> +
> +/* DSP */
> +#define CS40L50_GPIO_BASE 0x2804140
> +#define CS40L50_OWT_BASE 0x2805C34
> +#define CS40L50_OWT_SIZE 0x2805C38
> +#define CS40L50_OWT_NEXT 0x2805C3C
> +
> +/* GPIO */
> +#define CS40L50_GPIO_NUM_MASK GENMASK(14, 12)
> +#define CS40L50_GPIO_EDGE_MASK BIT(15)
> +#define CS40L50_GPIO_MAPPING_NONE 0
> +#define CS40L50_GPIO_DISABLE 0x1FF
> +
> +enum vibra_bank_type {
> + WVFRM_BANK_RAM,
> + WVFRM_BANK_ROM,
> + WVFRM_BANK_OWT,
> + WVFRM_BANK_NUM,
> +};
Please namespace everything, e.g. CS40L50_... and cs40l50_vibra_...
> +
> +/* Describes an area in DSP memory populated by effects */
> +struct vibra_bank {
> + enum vibra_bank_type bank;
> + u32 base_index;
> + u32 max_index;
> +};
> +
> +struct vibra_effect {
> + enum vibra_bank_type bank;
> + struct list_head list;
> + u32 gpio_reg;
> + u32 index;
> + int id;
> +};
> +
> +/* Describes haptic interface of loaded DSP firmware */
> +struct vibra_dsp {
> + struct vibra_bank *banks;
> + u32 gpio_base_reg;
> + u32 owt_offset_reg;
> + u32 owt_size_reg;
> + u32 owt_base_reg;
> + int (*write)(struct device *dev, struct regmap *regmap, u32 val);
> + u32 push_owt_cmd;
> + u32 delete_owt_cmd;
> + u32 stop_cmd;
> +};
> +
> +/* Describes configuration and state of haptic operations */
> +struct vibra_info {
> + struct device *dev;
> + struct regmap *regmap;
> + struct input_dev *input;
> + struct mutex lock;
> + struct workqueue_struct *vibe_wq;
> + struct list_head effect_head;
> + struct vibra_dsp dsp;
> +};
> +
> +struct vibra_work {
> + struct vibra_info *info;
> + struct ff_effect *effect;
> + struct work_struct work;
> + s16 *custom_data;
> + int custom_len;
> + int count;
> + int error;
> +};
> +
> +static struct vibra_bank cs40l50_banks[] = {
> + {
> + .bank = WVFRM_BANK_RAM,
> + .base_index = CS40L50_RAM_INDEX_START,
> + .max_index = CS40L50_RAM_INDEX_END,
> + },
> + {
> + .bank = WVFRM_BANK_ROM,
> + .base_index = CS40L50_ROM_INDEX_START,
> + .max_index = CS40L50_ROM_INDEX_END,
> + },
> + {
> + .bank = WVFRM_BANK_OWT,
> + .base_index = CS40L50_RTH_INDEX_START,
> + .max_index = CS40L50_RTH_INDEX_END,
> + },
> +};
> +
> +static struct vibra_dsp cs40l50_dsp = {
> + .banks = cs40l50_banks,
> + .gpio_base_reg = CS40L50_GPIO_BASE,
> + .owt_base_reg = CS40L50_OWT_BASE,
> + .owt_offset_reg = CS40L50_OWT_NEXT,
> + .owt_size_reg = CS40L50_OWT_SIZE,
> +
> + .push_owt_cmd = CS40L50_OWT_PUSH,
> + .delete_owt_cmd = CS40L50_OWT_DELETE,
> + .stop_cmd = CS40L50_STOP_PLAYBACK,
> +
> + .write = cs40l50_dsp_write,
> +};
Nit: I don't see any need for any newlines here.
> +
> +static struct vibra_effect *vibra_find_effect(int id, struct list_head *effect_head)
I recommend namespacing the function names too.
> +{
> + struct vibra_effect *effect;
> +
> + list_for_each_entry(effect, effect_head, list)
> + if (effect->id == id)
> + return effect;
> +
> + return NULL;
> +}
> +
> +static int vibra_effect_bank_set(struct vibra_work *work_data,
> + struct vibra_effect *effect)
> +{
> + s16 bank = work_data->custom_data[0] & 0xffffu;
This mask seems to get used in a couple different places; I would recommend
#defining it using GENMASK().
> +
> + if (bank >= WVFRM_BANK_NUM) {
> + dev_err(work_data->info->dev, "Invalid waveform bank: %d\n", bank);
> + return -EINVAL;
> + }
> +
> + if (work_data->custom_len > CS40L50_OWT_CUSTOM_DATA_SIZE)
> + effect->bank = WVFRM_BANK_OWT;
> + else
> + effect->bank = bank;
> +
> + return 0;
> +}
> +
> +static int vibra_effect_gpio_mapping_set(struct vibra_work *work_data,
> + struct vibra_effect *effect)
> +{
> + u16 button = work_data->effect->trigger.button;
> + struct vibra_info *info = work_data->info;
> + u32 gpio_num, gpio_edge;
> +
> + if (button) {
> + gpio_num = FIELD_GET(CS40L50_GPIO_NUM_MASK, button);
> + gpio_edge = FIELD_GET(CS40L50_GPIO_EDGE_MASK, button);
> + effect->gpio_reg = info->dsp.gpio_base_reg + (gpio_num * 8) - gpio_edge;
> +
> + return regmap_write(info->regmap, effect->gpio_reg, button);
> + }
> +
> + effect->gpio_reg = CS40L50_GPIO_MAPPING_NONE;
> +
> + return 0;
> +}
> +
> +static int vibra_effect_index_set(struct vibra_work *work_data,
> + struct vibra_effect *effect)
> +{
> + struct vibra_info *info = work_data->info;
> + struct vibra_effect *owt_effect;
> + u32 base_index, max_index;
> +
> + base_index = info->dsp.banks[effect->bank].base_index;
> + max_index = info->dsp.banks[effect->bank].max_index;
> +
> + effect->index = base_index;
> +
> + switch (effect->bank) {
> + case WVFRM_BANK_OWT:
> + list_for_each_entry(owt_effect, &info->effect_head, list)
> + if (owt_effect->bank == WVFRM_BANK_OWT)
> + effect->index++;
> + break;
> + case WVFRM_BANK_ROM:
> + case WVFRM_BANK_RAM:
> + effect->index += work_data->custom_data[1] & 0xffffu;
> + break;
> + default:
> + dev_err(info->dev, "Bank not supported: %d\n", effect->bank);
> + return -EINVAL;
> + }
> +
> + if (effect->index > max_index || effect->index < base_index) {
> + dev_err(info->dev, "Index out of bounds: %u\n", effect->index);
> + return -ENOSPC;
> + }
> +
> + return 0;
> +}
> +
> +/* Describes a header for an OWT effect */
> +struct owt_header {
> + u32 type;
> + u32 data_words;
> + u32 offset;
> +} __packed;
> +
> +static int vibra_upload_owt(struct vibra_work *work_data)
> +{
> + u32 len = 2 * work_data->custom_len, wt_offset, wt_size;
> + struct vibra_info *info = work_data->info;
> + struct owt_header header;
> + u8 *out_data;
> + int error;
> +
> + error = regmap_read(info->regmap, info->dsp.owt_size_reg, &wt_size);
> + if (error)
> + return error;
> +
> + if ((wt_size * sizeof(u32)) < sizeof(header) + len) {
> + dev_err(info->dev, "No space in OWT bank for effect\n");
> + return -ENOSPC;
> + }
> +
> + out_data = kzalloc(sizeof(header) + len, GFP_KERNEL);
> + if (!out_data)
> + return -ENOMEM;
> +
> + header.type = work_data->custom_data[0] == CS40L50_PCM_ID ? CS40L50_TYPE_PCM :
> + CS40L50_TYPE_PWLE;
> + header.offset = sizeof(header) / sizeof(u32);
> + header.data_words = len / sizeof(u32);
> +
> + memcpy(out_data, &header, sizeof(header));
> + memcpy(out_data + sizeof(header), work_data->custom_data, len);
> +
> + error = regmap_read(info->regmap, info->dsp.owt_offset_reg, &wt_offset);
> + if (error)
> + goto err_free;
> +
> + error = regmap_bulk_write(info->regmap, info->dsp.owt_base_reg +
> + (wt_offset * sizeof(u32)), out_data,
> + sizeof(header) + len);
> + if (error)
> + goto err_free;
> +
> + error = info->dsp.write(info->dev, info->regmap, info->dsp.push_owt_cmd);
> +err_free:
> + kfree(out_data);
> +
> + return error;
> +}
> +
> +static void vibra_add_worker(struct work_struct *work)
> +{
> + struct vibra_work *work_data = container_of(work, struct vibra_work, work);
> + struct vibra_info *info = work_data->info;
> + struct vibra_effect *effect;
> + bool is_new = false;
> + int error;
> +
> + error = pm_runtime_resume_and_get(info->dev);
> + if (error < 0)
> + goto err;
> +
> + mutex_lock(&info->lock);
> +
> + /* Update effect if already present, otherwise create new effect */
> + effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
> + if (!effect) {
> + effect = kzalloc(sizeof(*effect), GFP_KERNEL);
> + if (!effect) {
> + error = -ENOMEM;
> + goto err_mutex;
> + }
> +
> + effect->id = work_data->effect->id;
> + is_new = true;
> + }
> +
> + error = vibra_effect_bank_set(work_data, effect);
> + if (error)
> + goto err_free;
> +
> + error = vibra_effect_index_set(work_data, effect);
> + if (error)
> + goto err_free;
> +
> + error = vibra_effect_gpio_mapping_set(work_data, effect);
> + if (error)
> + goto err_free;
> +
> + if (effect->bank == WVFRM_BANK_OWT)
> + error = vibra_upload_owt(work_data);
> +err_free:
> + if (is_new) {
> + if (error)
> + kfree(effect);
> + else
> + list_add(&effect->list, &info->effect_head);
> + }
> +err_mutex:
> + mutex_unlock(&info->lock);
> +
> + pm_runtime_mark_last_busy(info->dev);
> + pm_runtime_put_autosuspend(info->dev);
> +err:
> + work_data->error = error;
> +}
> +
> +static int vibra_add(struct input_dev *dev, struct ff_effect *effect,
> + struct ff_effect *old)
> +{
> + struct ff_periodic_effect *periodic = &effect->u.periodic;
> + struct vibra_info *info = input_get_drvdata(dev);
> + u32 len = effect->u.periodic.custom_len;
> + struct vibra_work work_data;
> +
> + if (effect->type != FF_PERIODIC || periodic->waveform != FF_CUSTOM) {
> + dev_err(info->dev, "Type (%#X) or waveform (%#X) unsupported\n",
> + effect->type, periodic->waveform);
> + return -EINVAL;
> + }
> +
> + work_data.custom_data = kcalloc(len, sizeof(s16), GFP_KERNEL);
> + if (!work_data.custom_data)
> + return -ENOMEM;
> +
> + if (copy_from_user(work_data.custom_data, effect->u.periodic.custom_data,
> + sizeof(s16) * len)) {
> + work_data.error = -EFAULT;
> + goto err_free;
> + }
> +
> + work_data.custom_len = len;
> + work_data.info = info;
> + work_data.effect = effect;
> + INIT_WORK(&work_data.work, vibra_add_worker);
> +
> + /* Push to the workqueue to serialize with playbacks */
> + queue_work(info->vibe_wq, &work_data.work);
> + flush_work(&work_data.work);
> +err_free:
> + kfree(work_data.custom_data);
> +
> + return work_data.error;
> +}
> +
> +static void vibra_start_worker(struct work_struct *work)
> +{
> + struct vibra_work *work_data = container_of(work, struct vibra_work, work);
> + struct vibra_info *info = work_data->info;
> + struct vibra_effect *start_effect;
> +
> + if (pm_runtime_resume_and_get(info->dev) < 0)
> + goto err_free;
> +
> + mutex_lock(&info->lock);
> +
> + start_effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
> + if (start_effect) {
> + while (--work_data->count >= 0) {
> + info->dsp.write(info->dev, info->regmap, start_effect->index);
> + usleep_range(work_data->effect->replay.length,
> + work_data->effect->replay.length + 100);
> + }
> + }
> +
> + mutex_unlock(&info->lock);
> +
> + if (!start_effect)
> + dev_err(info->dev, "Effect to play not found\n");
> +
> + pm_runtime_mark_last_busy(info->dev);
> + pm_runtime_put_autosuspend(info->dev);
> +err_free:
> + kfree(work_data);
This business of allocating memory in one thread and freeing it in another
got pretty hard to follow, which means it will be hard to maintain. I know
there are some restrictions around writing into the HALO core, but I'd like
to see something simpler; the da7280 driver is nowhere near as complex.
How many s16 words do you realistically need for custom_data? Is it possible
to simply include an array in the cs40l50_vibra_info struct, and bleat if
user space tries to upload greater than the maximum size? This seems to be
the approach of the much simpler da7280 driver as well.
This array would go on the heap just the same, but you do not have to manage
it so carefully. Opinions may vary, but mine is that memory allocation and
freeing should be done in the same scope where possible.
> +}
> +
> +static void vibra_stop_worker(struct work_struct *work)
> +{
> + struct vibra_work *work_data = container_of(work, struct vibra_work, work);
> + struct vibra_info *info = work_data->info;
> +
> + if (pm_runtime_resume_and_get(info->dev) < 0)
> + return;
> +
> + mutex_lock(&info->lock);
> +
> + info->dsp.write(info->dev, info->regmap, info->dsp.stop_cmd);
> +
> + mutex_unlock(&info->lock);
> +
> + pm_runtime_mark_last_busy(info->dev);
> + pm_runtime_put_autosuspend(info->dev);
> +
> + kfree(work_data);
> +}
> +
> +static int vibra_playback(struct input_dev *dev, int effect_id, int val)
> +{
> + struct vibra_info *info = input_get_drvdata(dev);
> + struct vibra_work *work_data;
> +
> + work_data = kzalloc(sizeof(*work_data), GFP_ATOMIC);
> + if (!work_data)
> + return -ENOMEM;
> +
> + work_data->info = info;
> +
> + if (val > 0) {
> + work_data->effect = &dev->ff->effects[effect_id];
> + work_data->count = val;
> + INIT_WORK(&work_data->work, vibra_start_worker);
> + } else {
> + /* Stop the amplifier as device drives only one effect */
> + INIT_WORK(&work_data->work, vibra_stop_worker);
> + }
> +
> + queue_work(info->vibe_wq, &work_data->work);
> +
> + return 0;
> +}
> +
> +static void vibra_erase_worker(struct work_struct *work)
> +{
> + struct vibra_work *work_data = container_of(work, struct vibra_work, work);
> + struct vibra_effect *owt_effect, *erase_effect;
> + struct vibra_info *info = work_data->info;
> + int error;
> +
> + error = pm_runtime_resume_and_get(info->dev);
> + if (error < 0)
if (error)
> + goto err;
Please consider a more descriptive label.
> +
> + mutex_lock(&info->lock);
> +
> + erase_effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
> + if (!erase_effect) {
> + dev_err(info->dev, "Effect to erase not found\n");
> + error = -EINVAL;
> + goto err_mutex;
> + }
> +
> + if (erase_effect->gpio_reg != CS40L50_GPIO_MAPPING_NONE) {
> + error = regmap_write(info->regmap, erase_effect->gpio_reg,
> + CS40L50_GPIO_DISABLE);
> + if (error)
> + goto err_mutex;
> + }
> +
> + if (erase_effect->bank == WVFRM_BANK_OWT) {
> + error = info->dsp.write(info->dev, info->regmap,
> + info->dsp.delete_owt_cmd |
> + erase_effect->index);
> + if (error)
> + goto err_mutex;
> +
> + list_for_each_entry(owt_effect, &info->effect_head, list)
> + if (owt_effect->bank == WVFRM_BANK_OWT &&
> + owt_effect->index > erase_effect->index)
> + owt_effect->index--;
> + }
> +
> + list_del(&erase_effect->list);
> + kfree(erase_effect);
> +err_mutex:
> + mutex_unlock(&info->lock);
> +
> + pm_runtime_mark_last_busy(info->dev);
> + pm_runtime_put_autosuspend(info->dev);
> +err:
> + work_data->error = error;
> +}
> +
> +static int vibra_erase(struct input_dev *dev, int effect_id)
> +{
> + struct vibra_info *info = input_get_drvdata(dev);
> + struct vibra_work work_data;
> +
> + work_data.info = info;
> + work_data.effect = &dev->ff->effects[effect_id];
> +
> + INIT_WORK(&work_data.work, vibra_erase_worker);
> +
> + /* Push to workqueue to serialize with playbacks */
> + queue_work(info->vibe_wq, &work_data.work);
> + flush_work(&work_data.work);
> +
> + return work_data.error;
> +}
> +
> +static void vibra_remove_wq(void *data)
> +{
> + flush_workqueue(data);
> + destroy_workqueue(data);
> +}
> +
> +static int cs40l50_vibra_probe(struct platform_device *pdev)
> +{
> + struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
> + struct vibra_info *info;
> + int error;
> +
> + info = devm_kzalloc(pdev->dev.parent, sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return -ENOMEM;
> +
> + mutex_init(&info->lock);
> +
> + info->dev = cs40l50->dev;
> + info->regmap = cs40l50->regmap;
> + info->dsp = cs40l50_dsp;
> +
> + info->input = devm_input_allocate_device(info->dev);
> + if (!info->input)
> + return -ENOMEM;
> + info->input->id.product = cs40l50->devid & 0xFFFF;
> + info->input->id.version = cs40l50->revid;
> + info->input->name = "cs40l50_vibra";
> +
> + input_set_drvdata(info->input, info);
> + input_set_capability(info->input, EV_FF, FF_PERIODIC);
> + input_set_capability(info->input, EV_FF, FF_CUSTOM);
> +
> + error = input_ff_create(info->input, FF_MAX_EFFECTS);
> + if (error) {
> + dev_err(info->dev, "Failed to create input device\n");
> + return error;
> + }
> +
> + info->input->ff->upload = vibra_add;
> + info->input->ff->playback = vibra_playback;
> + info->input->ff->erase = vibra_erase;
> +
> + INIT_LIST_HEAD(&info->effect_head);
> +
> + info->vibe_wq = alloc_ordered_workqueue("vibe_wq", WQ_HIGHPRI);
> + if (!info->vibe_wq)
> + return -ENOMEM;
> +
> + error = devm_add_action_or_reset(info->dev, vibra_remove_wq, info->vibe_wq);
> + if (error)
> + return error;
> +
> + return input_register_device(info->input);
> +}
> +
> +static const struct platform_device_id vibra_id_match[] = {
> + { "cs40l50-vibra", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(platform, vibra_id_match);
> +
> +static struct platform_driver cs40l50_vibra_driver = {
> + .probe = cs40l50_vibra_probe,
> + .id_table = vibra_id_match,
> + .driver = {
> + .name = "cs40l50-vibra",
> + },
> +};
> +module_platform_driver(cs40l50_vibra_driver);
> +
> +MODULE_DESCRIPTION("CS40L50 Advanced Haptic Driver");
> +MODULE_AUTHOR("James Ogletree, Cirrus Logic Inc. <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>
Kind regards,
Jeff LaBundy
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
2024-03-08 22:24 ` [PATCH v9 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50 James Ogletree
@ 2024-03-14 16:05 ` Jeff LaBundy
2024-03-14 16:22 ` Mark Brown
2024-03-20 0:41 ` James Ogletree
0 siblings, 2 replies; 22+ messages in thread
From: Jeff LaBundy @ 2024-03-14 16:05 UTC (permalink / raw
To: James Ogletree
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, patches, linux-sound, linux-input, devicetree
Hi James,
This one looks pretty good; just a few nits since a new series is
coming anyway.
On Fri, Mar 08, 2024 at 10:24:21PM +0000, James Ogletree wrote:
> Introduce support for Cirrus Logic Device CS40L50: a
> haptic driver with waveform memory, integrated DSP,
> and closed-loop algorithms.
>
> The ASoC driver enables I2S streaming to the device.
>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
> MAINTAINERS | 1 +
> sound/soc/codecs/Kconfig | 11 ++
> sound/soc/codecs/Makefile | 2 +
> sound/soc/codecs/cs40l50-codec.c | 307 +++++++++++++++++++++++++++++++
> 4 files changed, 321 insertions(+)
> create mode 100644 sound/soc/codecs/cs40l50-codec.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 24cfb4f017bb..fca2454a7a38 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4943,6 +4943,7 @@ F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> F: drivers/input/misc/cs40l*
> F: drivers/mfd/cs40l*
> F: include/linux/mfd/cs40l*
> +F: sound/soc/codecs/cs40l*
>
> CIRRUS LOGIC DSP FIRMWARE DRIVER
> M: Simon Trimmer <simont@opensource.cirrus.com>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index f1e1dbc509f6..1a81bedfdbe3 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -73,6 +73,7 @@ config SND_SOC_ALL_CODECS
> imply SND_SOC_CS35L56_I2C
> imply SND_SOC_CS35L56_SPI
> imply SND_SOC_CS35L56_SDW
> + imply SND_SOC_CS40L50
> imply SND_SOC_CS42L42
> imply SND_SOC_CS42L42_SDW
> imply SND_SOC_CS42L43
> @@ -800,6 +801,16 @@ config SND_SOC_CS35L56_SDW
> help
> Enable support for Cirrus Logic CS35L56 boosted amplifier with SoundWire control
>
> +config SND_SOC_CS40L50
> + tristate "Cirrus Logic CS40L50 CODEC"
> + depends on MFD_CS40L50_CORE
> + help
> + This option enables support for I2S streaming to Cirrus Logic CS40L50.
> +
> + CS40L50 is a haptic driver with waveform memory, an integrated
> + DSP, and closed-loop algorithms. If built as a module, it will be
> + called snd-soc-cs40l50.
> +
> config SND_SOC_CS42L42_CORE
> tristate
>
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index a87e56938ce5..7e31f000774a 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -74,6 +74,7 @@ snd-soc-cs35l56-shared-objs := cs35l56-shared.o
> snd-soc-cs35l56-i2c-objs := cs35l56-i2c.o
> snd-soc-cs35l56-spi-objs := cs35l56-spi.o
> snd-soc-cs35l56-sdw-objs := cs35l56-sdw.o
> +snd-soc-cs40l50-objs := cs40l50-codec.o
> snd-soc-cs42l42-objs := cs42l42.o
> snd-soc-cs42l42-i2c-objs := cs42l42-i2c.o
> snd-soc-cs42l42-sdw-objs := cs42l42-sdw.o
> @@ -460,6 +461,7 @@ obj-$(CONFIG_SND_SOC_CS35L56_SHARED) += snd-soc-cs35l56-shared.o
> obj-$(CONFIG_SND_SOC_CS35L56_I2C) += snd-soc-cs35l56-i2c.o
> obj-$(CONFIG_SND_SOC_CS35L56_SPI) += snd-soc-cs35l56-spi.o
> obj-$(CONFIG_SND_SOC_CS35L56_SDW) += snd-soc-cs35l56-sdw.o
> +obj-$(CONFIG_SND_SOC_CS40L50) += snd-soc-cs40l50.o
> obj-$(CONFIG_SND_SOC_CS42L42_CORE) += snd-soc-cs42l42.o
> obj-$(CONFIG_SND_SOC_CS42L42) += snd-soc-cs42l42-i2c.o
> obj-$(CONFIG_SND_SOC_CS42L42_SDW) += snd-soc-cs42l42-sdw.o
> diff --git a/sound/soc/codecs/cs40l50-codec.c b/sound/soc/codecs/cs40l50-codec.c
> new file mode 100644
> index 000000000000..23299b8dacfb
> --- /dev/null
> +++ b/sound/soc/codecs/cs40l50-codec.c
> @@ -0,0 +1,307 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// CS40L50 Advanced Haptic Driver with waveform memory,
> +// integrated DSP, and closed-loop algorithms
> +//
> +// Copyright 2024 Cirrus Logic, Inc.
> +//
> +// Author: James Ogletree <james.ogletree@cirrus.com>
Typically we see a // C++ style comment for the SPDX identifier, and then
/* block comments */ for the rest, unless the maintainer has explicitly
asked for this style for this specific subsystem. The introductory block
in the rest of the series is much more common.
> +
> +#include <linux/bitfield.h>
> +#include <linux/mfd/cs40l50.h>
> +#include <linux/pm_runtime.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +
> +#define CS40L50_REFCLK_INPUT 0x2C04
> +#define CS40L50_ASP_CONTROL2 0x4808
> +#define CS40L50_ASP_DATA_CONTROL5 0x4840
> +
> +/* PLL Config */
> +#define CS40L50_PLL_REFCLK_BCLK 0x0
> +#define CS40L50_PLL_REFCLK_MCLK 0x5
> +#define CS40L50_PLL_REFCLK_LOOP_MASK BIT(11)
> +#define CS40L50_PLL_REFCLK_OPEN_LOOP 1
> +#define CS40L50_PLL_REFCLK_CLOSED_LOOP 0
> +#define CS40L50_PLL_REFCLK_LOOP_SHIFT 11
> +#define CS40L50_PLL_REFCLK_FREQ_MASK GENMASK(10, 5)
> +#define CS40L50_PLL_REFCLK_FREQ_SHIFT 5
> +#define CS40L50_PLL_REFCLK_SEL_MASK GENMASK(2, 0)
> +#define CS40L50_BCLK_RATIO_DEFAULT 32
> +
> +/* ASP Config */
> +#define CS40L50_ASP_RX_WIDTH_SHIFT 24
> +#define CS40L50_ASP_RX_WIDTH_MASK GENMASK(31, 24)
> +#define CS40L50_ASP_RX_WL_MASK GENMASK(5, 0)
> +#define CS40L50_ASP_FSYNC_INV_MASK BIT(2)
> +#define CS40L50_ASP_BCLK_INV_MASK BIT(6)
> +#define CS40L50_ASP_FMT_MASK GENMASK(10, 8)
> +#define CS40L50_ASP_FMT_I2S 0x2
> +
> +struct cs40l50_pll_config {
> + unsigned int freq;
> + unsigned int cfg;
> +};
> +
> +struct cs40l50_codec {
> + struct device *dev;
> + struct regmap *regmap;
> + unsigned int daifmt;
> + unsigned int bclk_ratio;
> + unsigned int rate;
> +};
> +
> +static const struct cs40l50_pll_config cs40l50_pll_cfg[] = {
> + {32768, 0x00},
> + {1536000, 0x1B},
> + {3072000, 0x21},
> + {6144000, 0x28},
> + {9600000, 0x30},
> + {12288000, 0x33},
Nit: please space these as is done elsewhere throughout the series, i.e.:
{ 12288000, 0x33 },
> +};
> +
> +static int cs40l50_get_clk_config(unsigned int freq, unsigned int *cfg)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(cs40l50_pll_cfg); i++) {
> + if (cs40l50_pll_cfg[i].freq == freq) {
> + *cfg = cs40l50_pll_cfg[i].cfg;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +
> +static int cs40l50_swap_ext_clk(struct cs40l50_codec *codec, unsigned int clk_src)
> +{
> + unsigned int cfg;
> + int ret;
> +
> + switch (clk_src) {
> + case CS40L50_PLL_REFCLK_BCLK:
> + ret = cs40l50_get_clk_config(codec->bclk_ratio * codec->rate, &cfg);
> + if (ret)
> + return ret;
> + break;
> + case CS40L50_PLL_REFCLK_MCLK:
> + cfg = 0x00;
0x00 seems special; please consider #defining it.
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> + CS40L50_PLL_REFCLK_LOOP_MASK,
> + CS40L50_PLL_REFCLK_OPEN_LOOP <<
> + CS40L50_PLL_REFCLK_LOOP_SHIFT);
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> + CS40L50_PLL_REFCLK_FREQ_MASK |
> + CS40L50_PLL_REFCLK_SEL_MASK,
> + (cfg << CS40L50_PLL_REFCLK_FREQ_SHIFT) | clk_src);
> + if (ret)
> + return ret;
> +
> + return regmap_update_bits(codec->regmap, CS40L50_REFCLK_INPUT,
> + CS40L50_PLL_REFCLK_LOOP_MASK,
> + CS40L50_PLL_REFCLK_CLOSED_LOOP <<
> + CS40L50_PLL_REFCLK_LOOP_SHIFT);
> +}
> +
> +static int cs40l50_clk_en(struct snd_soc_dapm_widget *w,
> + struct snd_kcontrol *kcontrol,
> + int event)
> +{
> + struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(comp);
> + int ret;
> +
> + switch (event) {
> + case SND_SOC_DAPM_POST_PMU:
> + ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_STOP_PLAYBACK);
> + if (ret)
> + return ret;
> +
> + ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_START_I2S);
> + if (ret)
> + return ret;
> +
> + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_BCLK);
> + if (ret)
> + return ret;
> + break;
> + case SND_SOC_DAPM_PRE_PMD:
> + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_MCLK);
> + if (ret)
> + return ret;
> + break;
Just for my own understanding, does the HALO core need to be told that I2S
streaming is about to stop? Or does the L50 gracefully park the class D
outputs when the PLL source is suddenly switched?
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dapm_widget cs40l50_dapm_widgets[] = {
> + SND_SOC_DAPM_SUPPLY_S("ASP PLL", 0, SND_SOC_NOPM, 0, 0, cs40l50_clk_en,
> + SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD),
> + SND_SOC_DAPM_AIF_IN("ASPRX1", NULL, 0, SND_SOC_NOPM, 0, 0),
> + SND_SOC_DAPM_AIF_IN("ASPRX2", NULL, 0, SND_SOC_NOPM, 0, 0),
> + SND_SOC_DAPM_OUTPUT("OUT"),
> +};
> +
> +static const struct snd_soc_dapm_route cs40l50_dapm_routes[] = {
> + { "ASP Playback", NULL, "ASP PLL" },
> + { "ASPRX1", NULL, "ASP Playback" },
> + { "ASPRX2", NULL, "ASP Playback" },
> +
> + { "OUT", NULL, "ASPRX1" },
> + { "OUT", NULL, "ASPRX2" },
> +};
> +
> +static int cs40l50_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(codec_dai->component);
> +
> + if ((fmt & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBC_CFC)
> + return -EINVAL;
> +
> + switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> + case SND_SOC_DAIFMT_NB_NF:
> + codec->daifmt = 0;
> + break;
> + case SND_SOC_DAIFMT_NB_IF:
> + codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK;
> + break;
> + case SND_SOC_DAIFMT_IB_NF:
> + codec->daifmt = CS40L50_ASP_BCLK_INV_MASK;
> + break;
> + case SND_SOC_DAIFMT_IB_IF:
> + codec->daifmt = CS40L50_ASP_FSYNC_INV_MASK | CS40L50_ASP_BCLK_INV_MASK;
> + break;
> + default:
> + dev_err(codec->dev, "Invalid clock invert\n");
> + return -EINVAL;
> + }
> +
> + switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> + case SND_SOC_DAIFMT_I2S:
> + codec->daifmt |= FIELD_PREP(CS40L50_ASP_FMT_MASK, CS40L50_ASP_FMT_I2S);
> + break;
> + default:
> + dev_err(codec->dev, "Unsupported DAI format\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int cs40l50_hw_params(struct snd_pcm_substream *substream,
> + struct snd_pcm_hw_params *params,
> + struct snd_soc_dai *dai)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
> + unsigned int asp_rx_wl = params_width(params);
> + int ret;
> +
> + codec->rate = params_rate(params);
> +
> + ret = regmap_update_bits(codec->regmap, CS40L50_ASP_DATA_CONTROL5,
> + CS40L50_ASP_RX_WL_MASK, asp_rx_wl);
> + if (ret)
> + return ret;
> +
> + codec->daifmt |= (asp_rx_wl << CS40L50_ASP_RX_WIDTH_SHIFT);
> +
> + return regmap_update_bits(codec->regmap, CS40L50_ASP_CONTROL2,
> + CS40L50_ASP_FSYNC_INV_MASK |
> + CS40L50_ASP_BCLK_INV_MASK |
> + CS40L50_ASP_FMT_MASK |
> + CS40L50_ASP_RX_WIDTH_MASK, codec->daifmt);
> +}
> +
> +static int cs40l50_set_dai_bclk_ratio(struct snd_soc_dai *dai, unsigned int ratio)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(dai->component);
> +
> + codec->bclk_ratio = ratio;
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_dai_ops cs40l50_dai_ops = {
> + .set_fmt = cs40l50_set_dai_fmt,
> + .set_bclk_ratio = cs40l50_set_dai_bclk_ratio,
> + .hw_params = cs40l50_hw_params,
> +};
> +
> +static struct snd_soc_dai_driver cs40l50_dai[] = {
> + {
> + .name = "cs40l50-pcm",
> + .id = 0,
> + .playback = {
> + .stream_name = "ASP Playback",
> + .channels_min = 1,
> + .channels_max = 2,
> + .rates = SNDRV_PCM_RATE_48000,
> + .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
> + },
> + .ops = &cs40l50_dai_ops,
> + },
> +};
> +
> +static int cs40l50_codec_probe(struct snd_soc_component *component)
> +{
> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(component);
> +
> + codec->bclk_ratio = CS40L50_BCLK_RATIO_DEFAULT;
> +
> + return 0;
> +}
> +
> +static const struct snd_soc_component_driver soc_codec_dev_cs40l50 = {
> + .probe = cs40l50_codec_probe,
> + .dapm_widgets = cs40l50_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(cs40l50_dapm_widgets),
> + .dapm_routes = cs40l50_dapm_routes,
> + .num_dapm_routes = ARRAY_SIZE(cs40l50_dapm_routes),
> +};
> +
> +static int cs40l50_codec_driver_probe(struct platform_device *pdev)
> +{
> + struct cs40l50 *cs40l50 = dev_get_drvdata(pdev->dev.parent);
> + struct cs40l50_codec *codec;
> +
> + codec = devm_kzalloc(&pdev->dev, sizeof(*codec), GFP_KERNEL);
> + if (!codec)
> + return -ENOMEM;
> +
> + codec->regmap = cs40l50->regmap;
> + codec->dev = &pdev->dev;
> +
> + return devm_snd_soc_register_component(&pdev->dev, &soc_codec_dev_cs40l50,
> + cs40l50_dai, ARRAY_SIZE(cs40l50_dai));
> +}
> +
> +static const struct platform_device_id cs40l50_id[] = {
> + { "cs40l50-codec", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(platform, cs40l50_id);
> +
> +static struct platform_driver cs40l50_codec_driver = {
> + .probe = cs40l50_codec_driver_probe,
> + .id_table = cs40l50_id,
> + .driver = {
> + .name = "cs40l50-codec",
> + },
> +};
> +module_platform_driver(cs40l50_codec_driver);
> +
> +MODULE_DESCRIPTION("ASoC CS40L50 driver");
> +MODULE_AUTHOR("James Ogletree <james.ogletree@cirrus.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1
>
Kind regards,
Jeff LaBundy
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
2024-03-14 16:05 ` Jeff LaBundy
@ 2024-03-14 16:22 ` Mark Brown
2024-03-20 0:41 ` James Ogletree
1 sibling, 0 replies; 22+ messages in thread
From: Mark Brown @ 2024-03-14 16:22 UTC (permalink / raw
To: Jeff LaBundy
Cc: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, lee, patches, linux-sound, linux-input, devicetree
[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]
On Thu, Mar 14, 2024 at 11:05:01AM -0500, Jeff LaBundy wrote:
> On Fri, Mar 08, 2024 at 10:24:21PM +0000, James Ogletree wrote:
> > Introduce support for Cirrus Logic Device CS40L50: a
> > haptic driver with waveform memory, integrated DSP,
> > and closed-loop algorithms.
Please delete unneeded context from mails when replying. Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// CS40L50 Advanced Haptic Driver with waveform memory,
> > +// integrated DSP, and closed-loop algorithms
> > +//
> > +// Copyright 2024 Cirrus Logic, Inc.
> > +//
> > +// Author: James Ogletree <james.ogletree@cirrus.com>
> Typically we see a // C++ style comment for the SPDX identifier, and then
> /* block comments */ for the rest, unless the maintainer has explicitly
> asked for this style for this specific subsystem. The introductory block
> in the rest of the series is much more common.
This is something I specifically request.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver
2024-03-10 23:40 ` Jeff LaBundy
2024-03-11 10:30 ` Charles Keepax
@ 2024-03-14 21:03 ` James Ogletree
1 sibling, 0 replies; 22+ messages in thread
From: James Ogletree @ 2024-03-14 21:03 UTC (permalink / raw
To: Jeff LaBundy
Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Mark Brown,
open list:CIRRUS LOGIC HAPTIC DRIVERS,
open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Jeff,
Thank you for the kind remarks and thorough review.
All of your points, here and across the series, you can consider
acknowledged and I will work to adopt them in the next version.
There were a few loose ends and questions throughout the series which I
intend to address over the coming days. The first one is below.
>> +
>> +static const struct reg_sequence cs40l50_irq_mask_override[] = {
>> + { CS40L50_IRQ1_MASK_2, CS40L50_IRQ_MASK_2_OVERRIDE },
>> + { CS40L50_IRQ1_MASK_20, CS40L50_IRQ_MASK_20_OVERRIDE },
>> +};
>> +
>> +static int cs40l50_configure_dsp(struct cs_dsp *dsp)
>> +{
>> + struct cs40l50 *cs40l50 = container_of(dsp, struct cs40l50, dsp);
>> + u32 nwaves;
>> + int err;
>> +
>> + /* Log number of effects if wavetable was loaded */
>> + if (cs40l50->bin) {
>
> Is there any other clue you can use to discern whether a wavetable was
> loaded? The memory at cs40l50->bin is gone now, and although you're not
> dereferencing it anymore, another contributor might be fooled into doing
> so down the road.
>
> Maybe a boolean would be more maintainable; I don't feel strongly about
> it though.
I will simply remove the conditional. If no wavetable was loaded, the
register will have a value of 0, which is no less worthwhile to report to the
user.
Best,
James
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface
2024-03-14 14:40 ` Jeff LaBundy
@ 2024-03-15 9:29 ` Charles Keepax
0 siblings, 0 replies; 22+ messages in thread
From: Charles Keepax @ 2024-03-15 9:29 UTC (permalink / raw
To: Jeff LaBundy
Cc: James Ogletree, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
conor+dt, lee, broonie, patches, linux-sound, linux-input,
devicetree
On Thu, Mar 14, 2024 at 09:40:34AM -0500, Jeff LaBundy wrote:
> On Mon, Mar 11, 2024 at 10:14:17AM +0000, Charles Keepax wrote:
> > On Sun, Mar 10, 2024 at 02:12:16PM -0500, Jeff LaBundy wrote:
> > > On Fri, Mar 08, 2024 at 10:24:17PM +0000, James Ogletree wrote:
> > I disagree here. This is the write function, there is no guarantee
> > this is even called at probe time. This is generic code going
> > into the DSP library and will presumably get re-used for different
> > purposes and on different parts in the future. Freeing on the error
> > path is much safer here.
>
> Agreed that this won't be called during probe; all I mean to say is
> that I don't see any issue in hanging on to a bit of memory while the
> device is in an error state, before the module is unloaded and devres
> unrolls all of the device-managed resources.
I think that's the problem the assumption that all users will
completely bork the device if this operation fails. Likely but
far from certain.
> It's also perfectly fine to leave this as-is; the existing method is
> functionally correct, and I understand the perspective of having the
> generic library clean up immediately. If that's the intent however,
> the same error handling needs to be applied in cs_dsp_populate_wseq();
> currently only cs_dsp_wseq_write() calls devm_kfree() on error. Since
> L50 uses asynchronous FW loading, neither are called until after the
> device probes.
>
Hmm... yes I guess in my head I wasn't thinking so much about
populate being called at runtime, but I am inclined to agree. We
should probably update populate to also free on the error path.
Thanks,
Charles
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver
2024-03-14 15:54 ` Jeff LaBundy
@ 2024-03-15 20:23 ` James Ogletree
0 siblings, 0 replies; 22+ messages in thread
From: James Ogletree @ 2024-03-15 20:23 UTC (permalink / raw
To: Jeff LaBundy
Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Mark Brown,
open list:CIRRUS LOGIC HAPTIC DRIVERS,
open list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM...,
open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
Hi Jeff,
> On Mar 14, 2024, at 10:54 AM, Jeff LaBundy <jeff@labundy.com> wrote:
>
> Hi James,
>
> On Fri, Mar 08, 2024 at 10:24:20PM +0000, James Ogletree wrote:
>>
>> +static void vibra_start_worker(struct work_struct *work)
>> +{
>> + struct vibra_work *work_data = container_of(work, struct vibra_work, work);
>> + struct vibra_info *info = work_data->info;
>> + struct vibra_effect *start_effect;
>> +
>> + if (pm_runtime_resume_and_get(info->dev) < 0)
>> + goto err_free;
>> +
>> + mutex_lock(&info->lock);
>> +
>> + start_effect = vibra_find_effect(work_data->effect->id, &info->effect_head);
>> + if (start_effect) {
>> + while (--work_data->count >= 0) {
>> + info->dsp.write(info->dev, info->regmap, start_effect->index);
>> + usleep_range(work_data->effect->replay.length,
>> + work_data->effect->replay.length + 100);
>> + }
>> + }
>> +
>> + mutex_unlock(&info->lock);
>> +
>> + if (!start_effect)
>> + dev_err(info->dev, "Effect to play not found\n");
>> +
>> + pm_runtime_mark_last_busy(info->dev);
>> + pm_runtime_put_autosuspend(info->dev);
>> +err_free:
>> + kfree(work_data);
>
> This business of allocating memory in one thread and freeing it in another
> got pretty hard to follow, which means it will be hard to maintain. I know
> there are some restrictions around writing into the HALO core, but I'd like
> to see something simpler; the da7280 driver is nowhere near as complex.
>
> How many s16 words do you realistically need for custom_data? Is it possible
> to simply include an array in the cs40l50_vibra_info struct, and bleat if
> user space tries to upload greater than the maximum size? This seems to be
> the approach of the much simpler da7280 driver as well.
>
> This array would go on the heap just the same, but you do not have to manage
> it so carefully. Opinions may vary, but mine is that memory allocation and
> freeing should be done in the same scope where possible.
The original issue was that the old implementation of playback effect had
a race condition which Dmitry pointed out. Consider when cs40l50_playback
is called: it populates effect data (including the index, say 1) and
schedules a trigger work to be run in the future. Before this work is
executed, a second call to cs40l50_playback is made, and again effect is
populated (say, index 2), and trigger work is scheduled again. If effect
is in shared data, then it will be overwritten by the second call and index
2 will be triggered twice.
So, I needed a separate work item for each playback to hold its individual
data so that effect cannot be overwritten by other playback calls. This
meant that I needed to take the playback work item and work-related data
out of shared memory (cs40l50_vibra_info), and create a structure
containing all the work data tied to that specific queue_work invocation,
as well as the work_struct itself so we can access that unique work data
via container_of() in the worker function.
Of course to have individuated work data, we need to allocate and populate
a new cs40l50_work with data in each input callback. Sometimes this
entails dynamic allocation. Why? Because sometimes we have no way of
knowing when we can let go of the work data. Contrast upload and erase:
these callbacks are not called in atomic context, so we can use flush_work
to guarantee the worker function returns, and therefore we can free the
work data in the callback. cs40l50_playback on the other hand is called
in atomic context, so we cannot use flush_work, and so we have to
"hold on" to the work data until such time known only by the worker
function itself. Hence we must free it there. So I think the complexity
here falls into the necessary bucket.
I don't think the above applies to the custom data memory, since that
particular memory is freed in the same function it is allocated in.
Looking at the possible use cases, the ceiling on the size of the data
is quite high if the customer wishes to upload a long sequence of
effects. So I prefer to keep that as it currently is.
It might be possible to use the "individuated work item" design for
playbacks only, and use the old "shared work item" for the other
callbacks. However, I decided that having one uniform implementation
would be better than mixing two schemes.
Best,
James
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50
2024-03-14 16:05 ` Jeff LaBundy
2024-03-14 16:22 ` Mark Brown
@ 2024-03-20 0:41 ` James Ogletree
1 sibling, 0 replies; 22+ messages in thread
From: James Ogletree @ 2024-03-20 0:41 UTC (permalink / raw
To: Jeff LaBundy
Cc: James Ogletree, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, lee@kernel.org, broonie@kernel.org,
patches@opensource.cirrus.com, linux-sound@vger.kernel.org,
linux-input@vger.kernel.org, devicetree@vger.kernel.org
> On Mar 14, 2024, at 11:05 AM, Jeff LaBundy <jeff@labundy.com> wrote:
>
> On Fri, Mar 08, 2024 at 10:24:21PM +0000, James Ogletree wrote:
>>
>> +static int cs40l50_clk_en(struct snd_soc_dapm_widget *w,
>> + struct snd_kcontrol *kcontrol,
>> + int event)
>> +{
>> + struct snd_soc_component *comp = snd_soc_dapm_to_component(w->dapm);
>> + struct cs40l50_codec *codec = snd_soc_component_get_drvdata(comp);
>> + int ret;
>> +
>> + switch (event) {
>> + case SND_SOC_DAPM_POST_PMU:
>> + ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_STOP_PLAYBACK);
>> + if (ret)
>> + return ret;
>> +
>> + ret = cs40l50_dsp_write(codec->dev, codec->regmap, CS40L50_START_I2S);
>> + if (ret)
>> + return ret;
>> +
>> + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_BCLK);
>> + if (ret)
>> + return ret;
>> + break;
>> + case SND_SOC_DAPM_PRE_PMD:
>> + ret = cs40l50_swap_ext_clk(codec, CS40L50_PLL_REFCLK_MCLK);
>> + if (ret)
>> + return ret;
>> + break;
>
> Just for my own understanding, does the HALO core need to be told that I2S
> streaming is about to stop? Or does the L50 gracefully park the class D
> outputs when the PLL source is suddenly switched?
The only time we enter this pre-power down DAPM event code path is if the
stream ends on its own, or if a stop has already been sent to the DSP. So the
former, but there is no need to tell the DSP anything here.
Best,
James
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2024-03-20 0:41 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-08 22:24 [PATCH v9 0/5] Add support for CS40L50 James Ogletree
2024-03-08 22:24 ` [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface James Ogletree
2024-03-10 19:12 ` Jeff LaBundy
2024-03-11 10:14 ` Charles Keepax
2024-03-14 14:40 ` Jeff LaBundy
2024-03-15 9:29 ` Charles Keepax
2024-03-08 22:24 ` [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding James Ogletree
2024-03-10 19:29 ` Jeff LaBundy
2024-03-11 6:31 ` Krzysztof Kozlowski
2024-03-14 14:42 ` Jeff LaBundy
2024-03-08 22:24 ` [PATCH v9 3/5] mfd: cs40l50: Add support for CS40L50 core driver James Ogletree
2024-03-10 23:40 ` Jeff LaBundy
2024-03-11 10:30 ` Charles Keepax
2024-03-14 15:15 ` Jeff LaBundy
2024-03-14 21:03 ` James Ogletree
2024-03-08 22:24 ` [PATCH v9 4/5] Input: cs40l50 - Add support for the CS40L50 haptic driver James Ogletree
2024-03-14 15:54 ` Jeff LaBundy
2024-03-15 20:23 ` James Ogletree
2024-03-08 22:24 ` [PATCH v9 5/5] ASoC: cs40l50: Support I2S streaming to CS40L50 James Ogletree
2024-03-14 16:05 ` Jeff LaBundy
2024-03-14 16:22 ` Mark Brown
2024-03-20 0:41 ` James Ogletree
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.