2023-05-30 12:24:08

by Charles Keepax

[permalink] [raw]
Subject: [PATCH v2 5/6] spi: cs42l43: Add SPI controller support

From: Lucas Tanure <[email protected]>

The CS42L43 is an audio CODEC with integrated MIPI SoundWire interface
(Version 1.2.1 compliant), I2C, SPI, and I2S/TDM interfaces designed
for portable applications. It provides a high dynamic range, stereo
DAC for headphone output, two integrated Class D amplifiers for
loudspeakers, and two ADCs for wired headset microphone input or
stereo line input. PDM inputs are provided for digital microphones.

The SPI component incorporates a SPI controller interface for
communication with other peripheral components.

Signed-off-by: Lucas Tanure <[email protected]>
Signed-off-by: Maciej Strozek <[email protected]>
Signed-off-by: Charles Keepax <[email protected]>
---

Changes since v1:
- Use HZ_PER_MHZ
- Return ret in un/prepare_transfer_hardware
- Use SPI_MODE_X_MASK
- Use devm_pm_runtime_enable
- Switch to using a platform_device_id table
- Use device_set_node
- Use min rather than min_t, and updating some consts to make it work

Thanks,
Charles

MAINTAINERS | 1 +
drivers/spi/Kconfig | 7 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-cs42l43.c | 279 ++++++++++++++++++++++++++++++++++++++
4 files changed, 288 insertions(+)
create mode 100644 drivers/spi/spi-cs42l43.c

diff --git a/MAINTAINERS b/MAINTAINERS
index ed26f63087342..a30dfbeb4f57a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4930,6 +4930,7 @@ F: Documentation/devicetree/bindings/mfd/cirrus,cs*
F: Documentation/devicetree/bindings/sound/cirrus,cs*
F: drivers/mfd/cs42l43*
F: drivers/pinctrl/cirrus/pinctrl-cs42l43*
+F: drivers/spi/spi-cs42l43*
F: include/dt-bindings/sound/cs*
F: include/linux/mfd/cs42l43*
F: include/sound/cs*
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 3de2ebe8294aa..f6ce06de41051 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -281,6 +281,13 @@ config SPI_COLDFIRE_QSPI
This enables support for the Coldfire QSPI controller in master
mode.

+config SPI_CS42L43
+ tristate "Cirrus Logic CS42L43 SPI controller"
+ depends on MFD_CS42L43 && PINCTRL_CS42L43
+ help
+ This enables support for the SPI controller inside the Cirrus Logic
+ CS42L43 audio codec.
+
config SPI_DAVINCI
tristate "Texas Instruments DaVinci/DA8x/OMAP-L/AM1x SoC SPI controller"
depends on ARCH_DAVINCI || ARCH_KEYSTONE || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 28c4817a8a74a..49937ea0d73d0 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_SPI_CADENCE_QUADSPI) += spi-cadence-quadspi.o
obj-$(CONFIG_SPI_CADENCE_XSPI) += spi-cadence-xspi.o
obj-$(CONFIG_SPI_CLPS711X) += spi-clps711x.o
obj-$(CONFIG_SPI_COLDFIRE_QSPI) += spi-coldfire-qspi.o
+obj-$(CONFIG_SPI_CS42L43) += spi-cs42l43.o
obj-$(CONFIG_SPI_DAVINCI) += spi-davinci.o
obj-$(CONFIG_SPI_DLN2) += spi-dln2.o
obj-$(CONFIG_SPI_DESIGNWARE) += spi-dw.o
diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c
new file mode 100644
index 0000000000000..5f86f4feb22c8
--- /dev/null
+++ b/drivers/spi/spi-cs42l43.c
@@ -0,0 +1,279 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// CS42L43 SPI Controller Driver
+//
+// Copyright (C) 2022-2023 Cirrus Logic, Inc. and
+// Cirrus Logic International Semiconductor Ltd.
+
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/mfd/cs42l43.h>
+#include <linux/mfd/cs42l43-regs.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/units.h>
+
+#define CS42L43_FIFO_SIZE 16
+#define CS42L43_SPI_ROOT_HZ (4 * HZ_PER_MHZ)
+#define CS42L43_SPI_MAX_LENGTH 65532
+
+enum cs42l43_spi_cmd {
+ CS42L43_WRITE,
+ CS42L43_READ
+};
+
+struct cs42l43_spi {
+ struct device *dev;
+ struct regmap *regmap;
+ struct spi_controller *ctlr;
+};
+
+static unsigned int cs42l43_clock_divs[16] = {
+ 2, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30
+};
+
+static int cs42l43_spi_tx(struct regmap *regmap, const u8 *buf, unsigned int len)
+{
+ const u8 *end = buf + len;
+ u32 val;
+ int ret;
+
+ while (buf < end) {
+ const u8 *block = min(buf + CS42L43_FIFO_SIZE, end);
+
+ for (; buf < block - (sizeof(u32) - 1); buf += sizeof(u32))
+ regmap_write(regmap, CS42L43_TX_DATA, *(const u32 *)buf);
+
+ if (buf < block) {
+ unsigned int residue = end - buf;
+
+ memcpy(&val, buf, residue);
+ regmap_write(regmap, CS42L43_TX_DATA, val);
+ buf += residue;
+ }
+
+ regmap_write(regmap, CS42L43_TRAN_CONFIG8, CS42L43_SPI_TX_DONE_MASK);
+
+ ret = regmap_read_poll_timeout(regmap, CS42L43_TRAN_STATUS1,
+ val, (val & CS42L43_SPI_TX_REQUEST_MASK),
+ 1000, 5000);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int cs42l43_spi_rx(struct regmap *regmap, u8 *buf, unsigned int len)
+{
+ u8 *end = buf + len;
+ u32 val;
+ int ret;
+
+ while (buf < end) {
+ const u8 *block = min(buf + CS42L43_FIFO_SIZE, end);
+
+ ret = regmap_read_poll_timeout(regmap, CS42L43_TRAN_STATUS1,
+ val, (val & CS42L43_SPI_RX_REQUEST_MASK),
+ 1000, 5000);
+ if (ret)
+ return ret;
+
+ for (; buf < block - (sizeof(u32) - 1); buf += sizeof(u32)) {
+ ret = regmap_read(regmap, CS42L43_RX_DATA, (u32 *)buf);
+ if (ret)
+ return ret;
+ }
+
+ if (buf < block) {
+ unsigned int residue = end - buf;
+
+ ret = regmap_read(regmap, CS42L43_RX_DATA, &val);
+ if (ret)
+ return ret;
+
+ memcpy(buf, &val, residue);
+ buf += residue;
+ }
+
+ regmap_write(regmap, CS42L43_TRAN_CONFIG8, CS42L43_SPI_RX_DONE_MASK);
+ }
+
+ return 0;
+}
+
+static int cs42l43_transfer_one(struct spi_controller *ctlr, struct spi_device *spi,
+ struct spi_transfer *tfr)
+{
+ struct cs42l43_spi *priv = spi_controller_get_devdata(spi->controller);
+ int i, ret = -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(cs42l43_clock_divs); i++) {
+ if (CS42L43_SPI_ROOT_HZ / cs42l43_clock_divs[i] <= tfr->speed_hz)
+ break;
+ }
+
+ if (i == ARRAY_SIZE(cs42l43_clock_divs))
+ return -EINVAL;
+
+ regmap_write(priv->regmap, CS42L43_SPI_CLK_CONFIG1, i);
+
+ if (tfr->tx_buf) {
+ regmap_write(priv->regmap, CS42L43_TRAN_CONFIG3, CS42L43_WRITE);
+ regmap_write(priv->regmap, CS42L43_TRAN_CONFIG4, tfr->len - 1);
+ } else if (tfr->rx_buf) {
+ regmap_write(priv->regmap, CS42L43_TRAN_CONFIG3, CS42L43_READ);
+ regmap_write(priv->regmap, CS42L43_TRAN_CONFIG5, tfr->len - 1);
+ }
+
+ regmap_write(priv->regmap, CS42L43_TRAN_CONFIG1, CS42L43_SPI_START_MASK);
+
+ if (tfr->tx_buf)
+ ret = cs42l43_spi_tx(priv->regmap, (const u8 *)tfr->tx_buf, tfr->len);
+ else if (tfr->rx_buf)
+ ret = cs42l43_spi_rx(priv->regmap, (u8 *)tfr->rx_buf, tfr->len);
+
+ return ret;
+}
+
+static void cs42l43_set_cs(struct spi_device *spi, bool is_high)
+{
+ struct cs42l43_spi *priv = spi_controller_get_devdata(spi->controller);
+
+ if (spi->chip_select == 0)
+ regmap_write(priv->regmap, CS42L43_SPI_CONFIG2, !is_high);
+}
+
+static int cs42l43_prepare_message(struct spi_controller *ctlr, struct spi_message *msg)
+{
+ struct cs42l43_spi *priv = spi_controller_get_devdata(ctlr);
+ struct spi_device *spi = msg->spi;
+ unsigned int spi_config1 = 0;
+
+ /* select another internal CS, which doesn't exist, so CS 0 is not used */
+ if (spi->cs_gpiod)
+ spi_config1 |= 1 << CS42L43_SPI_SS_SEL_SHIFT;
+ if (spi->mode & SPI_CPOL)
+ spi_config1 |= CS42L43_SPI_CPOL_MASK;
+ if (spi->mode & SPI_CPHA)
+ spi_config1 |= CS42L43_SPI_CPHA_MASK;
+ if (spi->mode & SPI_3WIRE)
+ spi_config1 |= CS42L43_SPI_THREE_WIRE_MASK;
+
+ regmap_write(priv->regmap, CS42L43_SPI_CONFIG1, spi_config1);
+
+ return 0;
+}
+
+static int cs42l43_prepare_transfer_hardware(struct spi_controller *ctlr)
+{
+ struct cs42l43_spi *priv = spi_controller_get_devdata(ctlr);
+ int ret;
+
+ ret = regmap_write(priv->regmap, CS42L43_BLOCK_EN2, CS42L43_SPI_MSTR_EN_MASK);
+ if (ret)
+ dev_err(priv->dev, "Failed to enable SPI controller: %d\n", ret);
+
+ return ret;
+}
+
+static int cs42l43_unprepare_transfer_hardware(struct spi_controller *ctlr)
+{
+ struct cs42l43_spi *priv = spi_controller_get_devdata(ctlr);
+ int ret;
+
+ ret = regmap_write(priv->regmap, CS42L43_BLOCK_EN2, 0);
+ if (ret)
+ dev_err(priv->dev, "Failed to disable SPI controller: %d\n", ret);
+
+ return ret;
+}
+
+static size_t cs42l43_spi_max_length(struct spi_device *spi)
+{
+ return CS42L43_SPI_MAX_LENGTH;
+}
+
+static int cs42l43_spi_probe(struct platform_device *pdev)
+{
+ struct cs42l43 *cs42l43 = dev_get_drvdata(pdev->dev.parent);
+ struct cs42l43_spi *priv;
+ struct fwnode_handle *fwnode = dev_fwnode(cs42l43->dev);
+ int ret;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->ctlr = devm_spi_alloc_master(&pdev->dev, sizeof(*priv->ctlr));
+ if (!priv->ctlr)
+ return -ENOMEM;
+
+ spi_controller_set_devdata(priv->ctlr, priv);
+
+ priv->dev = &pdev->dev;
+ priv->regmap = cs42l43->regmap;
+
+ priv->ctlr->prepare_message = cs42l43_prepare_message;
+ priv->ctlr->prepare_transfer_hardware = cs42l43_prepare_transfer_hardware;
+ priv->ctlr->unprepare_transfer_hardware = cs42l43_unprepare_transfer_hardware;
+ priv->ctlr->transfer_one = cs42l43_transfer_one;
+ priv->ctlr->set_cs = cs42l43_set_cs;
+ priv->ctlr->max_transfer_size = cs42l43_spi_max_length;
+
+ if (is_of_node(fwnode))
+ fwnode = fwnode_get_named_child_node(fwnode, "spi");
+
+ device_set_node(&priv->ctlr->dev, fwnode);
+
+ priv->ctlr->mode_bits = SPI_3WIRE | SPI_MODE_X_MASK;
+ priv->ctlr->flags = SPI_CONTROLLER_HALF_DUPLEX;
+ priv->ctlr->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16) |
+ SPI_BPW_MASK(32);
+ priv->ctlr->min_speed_hz = CS42L43_SPI_ROOT_HZ /
+ cs42l43_clock_divs[ARRAY_SIZE(cs42l43_clock_divs) - 1];
+ priv->ctlr->max_speed_hz = CS42L43_SPI_ROOT_HZ / cs42l43_clock_divs[0];
+ priv->ctlr->use_gpio_descriptors = true;
+ priv->ctlr->auto_runtime_pm = true;
+
+ devm_pm_runtime_enable(priv->dev);
+ pm_runtime_idle(priv->dev);
+
+ regmap_write(priv->regmap, CS42L43_TRAN_CONFIG6, CS42L43_FIFO_SIZE - 1);
+ regmap_write(priv->regmap, CS42L43_TRAN_CONFIG7, CS42L43_FIFO_SIZE - 1);
+
+ // Disable Watchdog timer and enable stall
+ regmap_write(priv->regmap, CS42L43_SPI_CONFIG3, 0);
+ regmap_write(priv->regmap, CS42L43_SPI_CONFIG4, CS42L43_SPI_STALL_ENA_MASK);
+
+ ret = devm_spi_register_controller(priv->dev, priv->ctlr);
+ if (ret) {
+ pm_runtime_disable(priv->dev);
+ dev_err(priv->dev, "Failed to register SPI controller: %d\n", ret);
+ }
+
+ return ret;
+}
+
+static const struct platform_device_id cs42l43_spi_id_table[] = {
+ { "cs42l43-spi", },
+ {}
+};
+MODULE_DEVICE_TABLE(platform, cs42l43_spi_id_table);
+
+static struct platform_driver cs42l43_spi_driver = {
+ .driver = {
+ .name = "cs42l43-spi",
+ },
+ .probe = cs42l43_spi_probe,
+ .id_table = cs42l43_spi_id_table,
+};
+module_platform_driver(cs42l43_spi_driver);
+
+MODULE_DESCRIPTION("CS42L43 SPI Driver");
+MODULE_AUTHOR("Lucas Tanure <[email protected]>");
+MODULE_AUTHOR("Maciej Strozek <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.30.2



2023-05-30 21:48:07

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] spi: cs42l43: Add SPI controller support

On Tue, May 30, 2023 at 01:21:11PM +0100, Charles Keepax wrote:

A couple of small things:

> +static unsigned int cs42l43_clock_divs[16] = {
> + 2, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30
> +};

Do we need to specify the size of the array? I just had to count the
number of initialisers :( Should probably also be const.

> + for (; buf < block - (sizeof(u32) - 1); buf += sizeof(u32))
> + regmap_write(regmap, CS42L43_TX_DATA, *(const u32 *)buf);

We're passing a byte stream through a u32 here - are you sure this is
endian safe?


Attachments:
(No filename) (570.00 B)
signature.asc (499.00 B)
Download all attachments

2023-05-31 05:32:32

by Mahapatra, Amit Kumar

[permalink] [raw]
Subject: RE: [PATCH v2 5/6] spi: cs42l43: Add SPI controller support

Hello,


> -----Original Message-----
> From: Charles Keepax <[email protected]>
> Sent: Tuesday, May 30, 2023 5:51 PM
> To: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; yung-
> [email protected]; [email protected]; pierre-
> [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: [PATCH v2 5/6] spi: cs42l43: Add SPI controller support
>
> CAUTION: This message has originated from an External Source. Please use
> proper judgment and caution when opening attachments, clicking links, or
> responding to this email.
>
>
> From: Lucas Tanure <[email protected]>
>
> The CS42L43 is an audio CODEC with integrated MIPI SoundWire interface
> (Version 1.2.1 compliant), I2C, SPI, and I2S/TDM interfaces designed for
> portable applications. It provides a high dynamic range, stereo DAC for
> headphone output, two integrated Class D amplifiers for loudspeakers, and
> two ADCs for wired headset microphone input or stereo line input. PDM
> inputs are provided for digital microphones.
>
> The SPI component incorporates a SPI controller interface for communication
> with other peripheral components.
>
> Signed-off-by: Lucas Tanure <[email protected]>
> Signed-off-by: Maciej Strozek <[email protected]>
> Signed-off-by: Charles Keepax <[email protected]>
> ---
>
> Changes since v1:
> - Use HZ_PER_MHZ
> - Return ret in un/prepare_transfer_hardware
> - Use SPI_MODE_X_MASK
> - Use devm_pm_runtime_enable
> - Switch to using a platform_device_id table
> - Use device_set_node
> - Use min rather than min_t, and updating some consts to make it work
>
> Thanks,
> Charles
>
> MAINTAINERS | 1 +
> drivers/spi/Kconfig | 7 +
> drivers/spi/Makefile | 1 +
> drivers/spi/spi-cs42l43.c | 279 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 288 insertions(+)
> create mode 100644 drivers/spi/spi-cs42l43.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ed26f63087342..a30dfbeb4f57a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4930,6 +4930,7 @@ F:
> Documentation/devicetree/bindings/mfd/cirrus,cs*
> F: Documentation/devicetree/bindings/sound/cirrus,cs*
> F: drivers/mfd/cs42l43*
> F: drivers/pinctrl/cirrus/pinctrl-cs42l43*
> +F: drivers/spi/spi-cs42l43*
> F: include/dt-bindings/sound/cs*
> F: include/linux/mfd/cs42l43*
> F: include/sound/cs*
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
> 3de2ebe8294aa..f6ce06de41051 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -281,6 +281,13 @@ config SPI_COLDFIRE_QSPI
> This enables support for the Coldfire QSPI controller in master
> mode.
>
> +config SPI_CS42L43
> + tristate "Cirrus Logic CS42L43 SPI controller"
> + depends on MFD_CS42L43 && PINCTRL_CS42L43
> + help
> + This enables support for the SPI controller inside the Cirrus Logic
> + CS42L43 audio codec.
> +
> config SPI_DAVINCI
> tristate "Texas Instruments DaVinci/DA8x/OMAP-L/AM1x SoC SPI
> controller"
> depends on ARCH_DAVINCI || ARCH_KEYSTONE || COMPILE_TEST diff --
> git a/drivers/spi/Makefile b/drivers/spi/Makefile index
> 28c4817a8a74a..49937ea0d73d0 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -40,6 +40,7 @@ obj-$(CONFIG_SPI_CADENCE_QUADSPI) += spi-
> cadence-quadspi.o
> obj-$(CONFIG_SPI_CADENCE_XSPI) += spi-cadence-xspi.o
> obj-$(CONFIG_SPI_CLPS711X) += spi-clps711x.o
> obj-$(CONFIG_SPI_COLDFIRE_QSPI) += spi-coldfire-qspi.o
> +obj-$(CONFIG_SPI_CS42L43) += spi-cs42l43.o
> obj-$(CONFIG_SPI_DAVINCI) += spi-davinci.o
> obj-$(CONFIG_SPI_DLN2) += spi-dln2.o
> obj-$(CONFIG_SPI_DESIGNWARE) += spi-dw.o
> diff --git a/drivers/spi/spi-cs42l43.c b/drivers/spi/spi-cs42l43.c new file mode
> 100644 index 0000000000000..5f86f4feb22c8
> --- /dev/null
> +++ b/drivers/spi/spi-cs42l43.c
> @@ -0,0 +1,279 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// CS42L43 SPI Controller Driver
> +//
> +// Copyright (C) 2022-2023 Cirrus Logic, Inc. and
> +// Cirrus Logic International Semiconductor Ltd.
> +
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/mfd/cs42l43.h>
> +#include <linux/mfd/cs42l43-regs.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/spi/spi.h>
> +#include <linux/units.h>
> +
> +#define CS42L43_FIFO_SIZE 16
> +#define CS42L43_SPI_ROOT_HZ (4 * HZ_PER_MHZ)
> +#define CS42L43_SPI_MAX_LENGTH 65532
> +
> +enum cs42l43_spi_cmd {
> + CS42L43_WRITE,
> + CS42L43_READ
> +};
> +
> +struct cs42l43_spi {
> + struct device *dev;
> + struct regmap *regmap;
> + struct spi_controller *ctlr;
> +};
> +
> +static unsigned int cs42l43_clock_divs[16] = {
> + 2, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30 };
> +
> +static int cs42l43_spi_tx(struct regmap *regmap, const u8 *buf,
> +unsigned int len) {
> + const u8 *end = buf + len;
> + u32 val;
> + int ret;
> +
> + while (buf < end) {
> + const u8 *block = min(buf + CS42L43_FIFO_SIZE, end);
> +
> + for (; buf < block - (sizeof(u32) - 1); buf += sizeof(u32))
> + regmap_write(regmap, CS42L43_TX_DATA, *(const
> + u32 *)buf);
> +
> + if (buf < block) {
> + unsigned int residue = end - buf;
> +
> + memcpy(&val, buf, residue);
> + regmap_write(regmap, CS42L43_TX_DATA, val);
> + buf += residue;
> + }
> +
> + regmap_write(regmap, CS42L43_TRAN_CONFIG8,
> + CS42L43_SPI_TX_DONE_MASK);
> +
> + ret = regmap_read_poll_timeout(regmap, CS42L43_TRAN_STATUS1,
> + val, (val & CS42L43_SPI_TX_REQUEST_MASK),
> + 1000, 5000);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int cs42l43_spi_rx(struct regmap *regmap, u8 *buf, unsigned int
> +len) {
> + u8 *end = buf + len;
> + u32 val;
> + int ret;
> +
> + while (buf < end) {
> + const u8 *block = min(buf + CS42L43_FIFO_SIZE, end);
> +
> + ret = regmap_read_poll_timeout(regmap, CS42L43_TRAN_STATUS1,
> + val, (val & CS42L43_SPI_RX_REQUEST_MASK),
> + 1000, 5000);
> + if (ret)
> + return ret;
> +
> + for (; buf < block - (sizeof(u32) - 1); buf += sizeof(u32)) {
> + ret = regmap_read(regmap, CS42L43_RX_DATA, (u32 *)buf);
> + if (ret)
> + return ret;
> + }
> +
> + if (buf < block) {
> + unsigned int residue = end - buf;
> +
> + ret = regmap_read(regmap, CS42L43_RX_DATA, &val);
> + if (ret)
> + return ret;
> +
> + memcpy(buf, &val, residue);
> + buf += residue;
> + }
> +
> + regmap_write(regmap, CS42L43_TRAN_CONFIG8,
> CS42L43_SPI_RX_DONE_MASK);
> + }
> +
> + return 0;
> +}
> +
> +static int cs42l43_transfer_one(struct spi_controller *ctlr, struct spi_device
> *spi,
> + struct spi_transfer *tfr) {
> + struct cs42l43_spi *priv = spi_controller_get_devdata(spi->controller);
> + int i, ret = -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(cs42l43_clock_divs); i++) {
> + if (CS42L43_SPI_ROOT_HZ / cs42l43_clock_divs[i] <= tfr->speed_hz)
> + break;
> + }
> +
> + if (i == ARRAY_SIZE(cs42l43_clock_divs))
> + return -EINVAL;
> +
> + regmap_write(priv->regmap, CS42L43_SPI_CLK_CONFIG1, i);
> +
> + if (tfr->tx_buf) {
> + regmap_write(priv->regmap, CS42L43_TRAN_CONFIG3,
> CS42L43_WRITE);
> + regmap_write(priv->regmap, CS42L43_TRAN_CONFIG4, tfr->len - 1);
> + } else if (tfr->rx_buf) {
> + regmap_write(priv->regmap, CS42L43_TRAN_CONFIG3,
> CS42L43_READ);
> + regmap_write(priv->regmap, CS42L43_TRAN_CONFIG5, tfr->len - 1);
> + }
> +
> + regmap_write(priv->regmap, CS42L43_TRAN_CONFIG1,
> + CS42L43_SPI_START_MASK);
> +
> + if (tfr->tx_buf)
> + ret = cs42l43_spi_tx(priv->regmap, (const u8 *)tfr->tx_buf, tfr->len);
> + else if (tfr->rx_buf)
> + ret = cs42l43_spi_rx(priv->regmap, (u8 *)tfr->rx_buf,
> + tfr->len);
> +
> + return ret;
> +}
> +
> +static void cs42l43_set_cs(struct spi_device *spi, bool is_high) {
> + struct cs42l43_spi *priv =
> +spi_controller_get_devdata(spi->controller);
> +
> + if (spi->chip_select == 0)

New set/get APIs for accessing spi->chip_select were introduced by
https://github.com/torvalds/linux/commit/303feb3cc06ac0665d0ee9c1414941200e60e8a3
please use these APIs instead of accessing spi->chip_select directly.

> + regmap_write(priv->regmap, CS42L43_SPI_CONFIG2,
> +!is_high); }
> +
> +static int cs42l43_prepare_message(struct spi_controller *ctlr, struct
> +spi_message *msg) {
> + struct cs42l43_spi *priv = spi_controller_get_devdata(ctlr);
> + struct spi_device *spi = msg->spi;
> + unsigned int spi_config1 = 0;
> +
> + /* select another internal CS, which doesn't exist, so CS 0 is not used */
> + if (spi->cs_gpiod)

Same here for spi->cs_gpio

Regards,
Amit
> + spi_config1 |= 1 << CS42L43_SPI_SS_SEL_SHIFT;
> + if (spi->mode & SPI_CPOL)
> + spi_config1 |= CS42L43_SPI_CPOL_MASK;
> + if (spi->mode & SPI_CPHA)
> + spi_config1 |= CS42L43_SPI_CPHA_MASK;
> + if (spi->mode & SPI_3WIRE)
> + spi_config1 |= CS42L43_SPI_THREE_WIRE_MASK;
> +
> + regmap_write(priv->regmap, CS42L43_SPI_CONFIG1, spi_config1);
> +
> + return 0;
> +}
> +
> +static int cs42l43_prepare_transfer_hardware(struct spi_controller
> +*ctlr) {
> + struct cs42l43_spi *priv = spi_controller_get_devdata(ctlr);
> + int ret;
> +
> + ret = regmap_write(priv->regmap, CS42L43_BLOCK_EN2,
> CS42L43_SPI_MSTR_EN_MASK);
> + if (ret)
> + dev_err(priv->dev, "Failed to enable SPI controller:
> + %d\n", ret);
> +
> + return ret;
> +}
> +
> +static int cs42l43_unprepare_transfer_hardware(struct spi_controller
> +*ctlr) {
> + struct cs42l43_spi *priv = spi_controller_get_devdata(ctlr);
> + int ret;
> +
> + ret = regmap_write(priv->regmap, CS42L43_BLOCK_EN2, 0);
> + if (ret)
> + dev_err(priv->dev, "Failed to disable SPI controller:
> + %d\n", ret);
> +
> + return ret;
> +}
> +
> +static size_t cs42l43_spi_max_length(struct spi_device *spi) {
> + return CS42L43_SPI_MAX_LENGTH;
> +}
> +
> +static int cs42l43_spi_probe(struct platform_device *pdev) {
> + struct cs42l43 *cs42l43 = dev_get_drvdata(pdev->dev.parent);
> + struct cs42l43_spi *priv;
> + struct fwnode_handle *fwnode = dev_fwnode(cs42l43->dev);
> + int ret;
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->ctlr = devm_spi_alloc_master(&pdev->dev, sizeof(*priv->ctlr));
> + if (!priv->ctlr)
> + return -ENOMEM;
> +
> + spi_controller_set_devdata(priv->ctlr, priv);
> +
> + priv->dev = &pdev->dev;
> + priv->regmap = cs42l43->regmap;
> +
> + priv->ctlr->prepare_message = cs42l43_prepare_message;
> + priv->ctlr->prepare_transfer_hardware =
> cs42l43_prepare_transfer_hardware;
> + priv->ctlr->unprepare_transfer_hardware =
> cs42l43_unprepare_transfer_hardware;
> + priv->ctlr->transfer_one = cs42l43_transfer_one;
> + priv->ctlr->set_cs = cs42l43_set_cs;
> + priv->ctlr->max_transfer_size = cs42l43_spi_max_length;
> +
> + if (is_of_node(fwnode))
> + fwnode = fwnode_get_named_child_node(fwnode, "spi");
> +
> + device_set_node(&priv->ctlr->dev, fwnode);
> +
> + priv->ctlr->mode_bits = SPI_3WIRE | SPI_MODE_X_MASK;
> + priv->ctlr->flags = SPI_CONTROLLER_HALF_DUPLEX;
> + priv->ctlr->bits_per_word_mask = SPI_BPW_MASK(8) |
> SPI_BPW_MASK(16) |
> + SPI_BPW_MASK(32);
> + priv->ctlr->min_speed_hz = CS42L43_SPI_ROOT_HZ /
> + cs42l43_clock_divs[ARRAY_SIZE(cs42l43_clock_divs) - 1];
> + priv->ctlr->max_speed_hz = CS42L43_SPI_ROOT_HZ /
> cs42l43_clock_divs[0];
> + priv->ctlr->use_gpio_descriptors = true;
> + priv->ctlr->auto_runtime_pm = true;
> +
> + devm_pm_runtime_enable(priv->dev);
> + pm_runtime_idle(priv->dev);
> +
> + regmap_write(priv->regmap, CS42L43_TRAN_CONFIG6,
> CS42L43_FIFO_SIZE - 1);
> + regmap_write(priv->regmap, CS42L43_TRAN_CONFIG7,
> + CS42L43_FIFO_SIZE - 1);
> +
> + // Disable Watchdog timer and enable stall
> + regmap_write(priv->regmap, CS42L43_SPI_CONFIG3, 0);
> + regmap_write(priv->regmap, CS42L43_SPI_CONFIG4,
> + CS42L43_SPI_STALL_ENA_MASK);
> +
> + ret = devm_spi_register_controller(priv->dev, priv->ctlr);
> + if (ret) {
> + pm_runtime_disable(priv->dev);
> + dev_err(priv->dev, "Failed to register SPI controller: %d\n", ret);
> + }
> +
> + return ret;
> +}
> +
> +static const struct platform_device_id cs42l43_spi_id_table[] = {
> + { "cs42l43-spi", },
> + {}
> +};
> +MODULE_DEVICE_TABLE(platform, cs42l43_spi_id_table);
> +
> +static struct platform_driver cs42l43_spi_driver = {
> + .driver = {
> + .name = "cs42l43-spi",
> + },
> + .probe = cs42l43_spi_probe,
> + .id_table = cs42l43_spi_id_table,
> +};
> +module_platform_driver(cs42l43_spi_driver);
> +
> +MODULE_DESCRIPTION("CS42L43 SPI Driver"); MODULE_AUTHOR("Lucas
> Tanure
> +<[email protected]>");
> +MODULE_AUTHOR("Maciej Strozek <[email protected]>");
> +MODULE_LICENSE("GPL");
> --
> 2.30.2


2023-05-31 08:50:28

by Charles Keepax

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] spi: cs42l43: Add SPI controller support

On Tue, May 30, 2023 at 10:30:46PM +0100, Mark Brown wrote:
> On Tue, May 30, 2023 at 01:21:11PM +0100, Charles Keepax wrote:
>
> A couple of small things:
>
> > +static unsigned int cs42l43_clock_divs[16] = {
> > + 2, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30
> > +};
>
> Do we need to specify the size of the array? I just had to count the
> number of initialisers :( Should probably also be const.
>
> > + for (; buf < block - (sizeof(u32) - 1); buf += sizeof(u32))
> > + regmap_write(regmap, CS42L43_TX_DATA, *(const u32 *)buf);
>
> We're passing a byte stream through a u32 here - are you sure this is
> endian safe?

Ah shoot, yeah Andy made some comments on this that seem to have
got lost in my mass of fixups. I will fixup for a v3.

Thanks,
Charles

2023-05-31 08:58:54

by Charles Keepax

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] spi: cs42l43: Add SPI controller support

On Wed, May 31, 2023 at 05:26:46AM +0000, Mahapatra, Amit Kumar wrote:
> > + if (spi->chip_select == 0)
>
> New set/get APIs for accessing spi->chip_select were introduced by
> https://github.com/torvalds/linux/commit/303feb3cc06ac0665d0ee9c1414941200e60e8a3
> please use these APIs instead of accessing spi->chip_select directly.
>
> > + /* select another internal CS, which doesn't exist, so CS 0 is not used */
> > + if (spi->cs_gpiod)
>
> Same here for spi->cs_gpio
>

Thanks, I will get these fixed up for v3.

Thanks,
Charles