| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <bd4c38ceed9efb407a64f9763a9e5a20567e1a3c.camel@gmail.com>
Date: Fri, 24 Feb 2023 08:56:33 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: “Ryan <ryan.lee.analog@...il.com>,
lgirdwood@...il.com, broonie@...nel.org, perex@...ex.cz,
tiwai@...e.com, krzysztof.kozlowski@...aro.org,
rf@...nsource.cirrus.com, ckeepax@...nsource.cirrus.com,
pierre-louis.bossart@...ux.intel.com, herve.codina@...tlin.com,
wangweidong.a@...nic.com, james.schulman@...rus.com,
ajye_huang@...pal.corp-partner.google.com, shumingf@...ltek.com,
povik+lin@...ebit.org, flatmax@...tmax.com,
linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
robh+dt@...nel.org, devicetree@...r.kernel.org,
ryans.lee@...log.com
Subject: Re: [PATCH 1/2] ASoC: max98363: add soundwire amplifier driver
On Thu, 2023-02-23 at 17:08 -0800, “Ryan wrote:
> From: Ryan Lee <ryans.lee@...log.com>
>
> Added Analog Devices MAX98363 SoundWire Amplifier Driver.
> The MAX98363 is a SoundWire peripheral device that supports
> MIPI SoundWire v1.2-compatible digital interface for audio and
> control data.
>
> Signed-off-by: Ryan Lee <ryans.lee@...log.com>
> ---
> sound/soc/codecs/Kconfig | 11 +
> sound/soc/codecs/Makefile | 2 +
> sound/soc/codecs/max98363.c | 699
> ++++++++++++++++++++++++++++++++++++
> sound/soc/codecs/max98363.h | 108 ++++++
> 4 files changed, 820 insertions(+)
> create mode 100644 sound/soc/codecs/max98363.c
> create mode 100644 sound/soc/codecs/max98363.h
>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 4621674e68bf..0638b27c6494 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -130,6 +130,7 @@ config SND_SOC_ALL_CODECS
> imply SND_SOC_MAX98925
> imply SND_SOC_MAX98926
> imply SND_SOC_MAX98927
> + imply SND_SOC_MAX98363
> imply SND_SOC_MAX98373_I2C
> imply SND_SOC_MAX98373_SDW
> imply SND_SOC_MAX98390
> @@ -1094,6 +1095,16 @@ config SND_SOC_MAX98520
>
> To compile this driver as a module, choose M here.
>
> +config SND_SOC_MAX98363
> + tristate "Analog Devices MAX98363 Soundwire Speaker
> Amplifier"
> + depends on SOUNDWIRE
> + select REGMAP_SOUNDWIRE
> + help
> + Enable support for Analog Devices MAX98363 Soundwire
> + amplifier. MAX98363 supports the MIPI SoundWire v1.2
> + compatible interface for audio and control data.
> + This amplifier does not support I2C and I2S.
> +
> config SND_SOC_MAX98373
> tristate
>
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index a0a61554548e..f4fbeceed816 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -144,6 +144,7 @@ snd-soc-max98925-objs := max98925.o
> snd-soc-max98926-objs := max98926.o
> snd-soc-max98927-objs := max98927.o
> snd-soc-max98520-objs := max98520.o
> +snd-soc-max98363-objs := max98363.o
> snd-soc-max98373-objs := max98373.o
> snd-soc-max98373-i2c-objs := max98373-i2c.o
> snd-soc-max98373-sdw-objs := max98373-sdw.o
> @@ -507,6 +508,7 @@ obj-$(CONFIG_SND_SOC_MAX98925) += snd-soc-
> max98925.o
> obj-$(CONFIG_SND_SOC_MAX98926) += snd-soc-max98926.o
> obj-$(CONFIG_SND_SOC_MAX98927) += snd-soc-max98927.o
> obj-$(CONFIG_SND_SOC_MAX98520) += snd-soc-max98520.o
> +obj-$(CONFIG_SND_SOC_MAX98363) += snd-soc-max98363.o
> obj-$(CONFIG_SND_SOC_MAX98373) += snd-soc-max98373.o
> obj-$(CONFIG_SND_SOC_MAX98373_I2C) += snd-soc-max98373-i2c.o
> obj-$(CONFIG_SND_SOC_MAX98373_SDW) += snd-soc-max98373-sdw.o
> diff --git a/sound/soc/codecs/max98363.c
> b/sound/soc/codecs/max98363.c
> new file mode 100644
> index 000000000000..e766c733f8e4
> --- /dev/null
> +++ b/sound/soc/codecs/max98363.c
> @@ -0,0 +1,699 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright (c) 2022, Analog Devices Inc.
> +
> +#include <linux/acpi.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +#include <sound/tlv.h>
> +#include <linux/of.h>
> +#include <linux/soundwire/sdw.h>
> +#include <linux/soundwire/sdw_type.h>
> +#include <linux/soundwire/sdw_registers.h>
> +#include <linux/regulator/consumer.h>
> +#include "max98363.h"
> +
> +struct sdw_stream_data {
> + struct sdw_stream_runtime *sdw_stream;
> +};
> +
> +static struct reg_default max98363_reg[] = {
> + {MAX98363_R0040_SCP_INIT_STAT_1, 0x00},
> + {MAX98363_R0041_SCP_INIT_MASK_1, 0x00},
> + {MAX98363_R0042_SCP_INIT_STAT_2, 0x00},
> + {MAX98363_R0044_SCP_CTRL, 0x00},
> + {MAX98363_R0045_SCP_SYSTEM_CTRL, 0x00},
> + {MAX98363_R0046_SCP_DEV_NUMBER, 0x00},
> + {MAX98363_R004D_SCP_BUS_CLK, 0x00},
> + {MAX98363_R0050_SCP_DEV_ID_0, 0x21},
> + {MAX98363_R0051_SCP_DEV_ID_1, 0x01},
> + {MAX98363_R0052_SCP_DEV_ID_2, 0x9F},
> + {MAX98363_R0053_SCP_DEV_ID_3, 0x87},
> + {MAX98363_R0054_SCP_DEV_ID_4, 0x08},
> + {MAX98363_R0055_SCP_DEV_ID_5, 0x00},
> + {MAX98363_R0060_SCP_FRAME_CTRL, 0x00},
> + {MAX98363_R0062_SCP_CLK_SCALE_BANK0, 0x00},
> + {MAX98363_R0070_SCP_FRAME_CTRL, 0x00},
> + {MAX98363_R0072_SCP_CLK_SCALE_BANK1, 0x00},
> + {MAX98363_R0080_SCP_PHYOUTCTRL_0, 0x00},
> + {MAX98363_R0100_DP1_INIT_STAT, 0x00},
> + {MAX98363_R0101_DP1_INIT_MASK, 0x00},
> + {MAX98363_R0102_DP1_PORT_CTRL, 0x00},
> + {MAX98363_R0103_DP1_BLOCK_CTRL_1, 0x00},
> + {MAX98363_R0104_DP1_PREPARE_STATUS, 0x00},
> + {MAX98363_R0105_DP1_PREPARE_CTRL, 0x00},
> + {MAX98363_R0120_DP1_CHANNEL_EN, 0x00},
> + {MAX98363_R0122_DP1_SAMPLE_CTRL1, 0x00},
> + {MAX98363_R0123_DP1_SAMPLE_CTRL2, 0x00},
> + {MAX98363_R0124_DP1_OFFSET_CTRL1, 0x00},
> + {MAX98363_R0125_DP1_OFFSET_CTRL2, 0x00},
> + {MAX98363_R0126_DP1_HCTRL, 0x00},
> + {MAX98363_R0127_DP1_BLOCK_CTRL3, 0x00},
> + {MAX98363_R0130_DP1_CHANNEL_EN, 0x00},
> + {MAX98363_R0132_DP1_SAMPLE_CTRL1, 0x00},
> + {MAX98363_R0133_DP1_SAMPLE_CTRL2, 0x00},
> + {MAX98363_R0134_DP1_OFFSET_CTRL1, 0x00},
> + {MAX98363_R0135_DP1_OFFSET_CTRL2, 0x00},
> + {MAX98363_R0136_DP1_HCTRL, 0x0136},
> + {MAX98363_R0137_DP1_BLOCK_CTRL3, 0x00},
> + {MAX98363_R2001_INTR_RAW, 0x0},
> + {MAX98363_R2003_INTR_STATE, 0x0},
> + {MAX98363_R2005_INTR_FALG, 0x0},
> + {MAX98363_R2007_INTR_EN, 0x0},
> + {MAX98363_R2009_INTR_CLR, 0x0},
> + {MAX98363_R2021_ERR_MON_CTRL, 0x0},
> + {MAX98363_R2022_SPK_MON_THRESH, 0x0},
> + {MAX98363_R2023_SPK_MON_DURATION, 0x0},
> + {MAX98363_R2030_TONE_GEN_CFG, 0x0},
> + {MAX98363_R203F_TONE_GEN_EN, 0x0},
> + {MAX98363_R2040_AMP_VOL, 0x0},
> + {MAX98363_R2041_AMP_GAIN, 0x5},
> + {MAX98363_R2042_DSP_CFG, 0x0},
> + {MAX98363_R21FF_REV_ID, 0x0},
> +};
> +
> +static bool max98363_readable_register(struct device *dev, unsigned
> int reg)
> +{
> + switch (reg) {
> + /* SoundWire Control Port Registers */
> + case MAX98363_R0040_SCP_INIT_STAT_1 ...
> MAX98363_R0046_SCP_DEV_NUMBER:
> + case MAX98363_R004D_SCP_BUS_CLK:
> + case MAX98363_R0050_SCP_DEV_ID_0 ...
> MAX98363_R0055_SCP_DEV_ID_5:
> + case MAX98363_R0062_SCP_CLK_SCALE_BANK0:
> + case MAX98363_R0072_SCP_CLK_SCALE_BANK1:
> + case MAX98363_R0080_SCP_PHYOUTCTRL_0:
> + /* Soundwire Data Port 1 Registers */
> + case MAX98363_R0100_DP1_INIT_STAT ...
> MAX98363_R0105_DP1_PREPARE_CTRL:
> + case MAX98363_R0120_DP1_CHANNEL_EN ...
> MAX98363_R0127_DP1_BLOCK_CTRL3:
> + case MAX98363_R0130_DP1_CHANNEL_EN:
> + case MAX98363_R0132_DP1_SAMPLE_CTRL1...
> MAX98363_R0137_DP1_BLOCK_CTRL3:
> + /* MAX98363 Amp Control Registers */
> + case MAX98363_R2001_INTR_RAW:
> + case MAX98363_R2003_INTR_STATE:
> + case MAX98363_R2005_INTR_FALG:
> + case MAX98363_R2007_INTR_EN:
> + case MAX98363_R2009_INTR_CLR:
> + case MAX98363_R2021_ERR_MON_CTRL ...
> MAX98363_R2023_SPK_MON_DURATION:
> + case MAX98363_R2030_TONE_GEN_CFG:
> + case MAX98363_R203F_TONE_GEN_EN:
> + case MAX98363_R2040_AMP_VOL:
> + case MAX98363_R2041_AMP_GAIN:
> + case MAX98363_R2042_DSP_CFG:
> + case MAX98363_R21FF_REV_ID:
> + return true;
> + default:
> + return false;
> + }
> +};
> +
> +static bool max98363_volatile_reg(struct device *dev, unsigned int
> reg)
> +{
> + switch (reg) {
> + /* SoundWire Control Port Registers */
> + case MAX98363_R0040_SCP_INIT_STAT_1 ...
> MAX98363_R0046_SCP_DEV_NUMBER:
> + case MAX98363_R004D_SCP_BUS_CLK:
> + case MAX98363_R0050_SCP_DEV_ID_0 ...
> MAX98363_R0055_SCP_DEV_ID_5:
> + case MAX98363_R0062_SCP_CLK_SCALE_BANK0:
> + case MAX98363_R0072_SCP_CLK_SCALE_BANK1:
> + case MAX98363_R0080_SCP_PHYOUTCTRL_0:
> + /* Soundwire Data Port 1 Registers */
> + case MAX98363_R0100_DP1_INIT_STAT ...
> MAX98363_R0105_DP1_PREPARE_CTRL:
> + case MAX98363_R0120_DP1_CHANNEL_EN ...
> MAX98363_R0127_DP1_BLOCK_CTRL3:
> + case MAX98363_R0130_DP1_CHANNEL_EN:
> + case MAX98363_R0132_DP1_SAMPLE_CTRL1...
> MAX98363_R0137_DP1_BLOCK_CTRL3:
> + /* MAX98363 Amp Control Registers */
> + case MAX98363_R2001_INTR_RAW:
> + case MAX98363_R2003_INTR_STATE:
> + case MAX98363_R2005_INTR_FALG:
> + case MAX98363_R2007_INTR_EN:
> + case MAX98363_R2009_INTR_CLR:
> + case MAX98363_R21FF_REV_ID:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct regmap_config max98363_sdw_regmap = {
> + .reg_bits = 32,
> + .val_bits = 8,
> + .max_register = MAX98363_R21FF_REV_ID,
> + .reg_defaults = max98363_reg,
> + .num_reg_defaults = ARRAY_SIZE(max98363_reg),
> + .readable_reg = max98363_readable_register,
> + .volatile_reg = max98363_volatile_reg,
> + .cache_type = REGCACHE_RBTREE,
> + .use_single_read = true,
> + .use_single_write = true,
> +};
> +
> +static __maybe_unused int max98363_suspend(struct device *dev)
can drop '__maybe_unused'...
> +{
> + struct max98363_priv *max98363 = dev_get_drvdata(dev);
> +
> + regcache_cache_only(max98363->regmap, true);
> + regcache_mark_dirty(max98363->regmap);
> +
> + if (max98363->dvddio)
> + regulator_disable(max98363->dvddio);
> +
> + if (max98363->vdd)
> + regulator_disable(max98363->vdd);
> +
> + return 0;
> +}
> +
> +#define MAX98363_PROBE_TIMEOUT 5000
> +
> +static __maybe_unused int max98363_resume(struct device *dev)
ditto...
> +{
> + struct sdw_slave *slave = dev_to_sdw_dev(dev);
> + struct max98363_priv *max98363 = dev_get_drvdata(dev);
> + unsigned long time;
> + int ret;
> +
> + if (!max98363->first_hw_init)
> + return 0;
> +
> + if (!slave->unattach_request)
> + goto regmap_sync;
> +
> + time = wait_for_completion_timeout(&slave-
> >initialization_complete,
> +
> msecs_to_jiffies(MAX98363_PROBE_TIMEOUT));
> + if (!time) {
> + dev_err(dev, "Initialization not complete, timed
> out\n");
> + return -ETIMEDOUT;
> + }
> +
> +regmap_sync:
> +
> + if (max98363->dvddio) {
> + ret = regulator_enable(max98363->dvddio);
> + if (ret < 0)
> + return ret;
> + }
> +
> + if (max98363->vdd) {
> + ret = regulator_enable(max98363->vdd);
> + if (ret < 0)
> + return ret;
> + }
> +
> + slave->unattach_request = 0;
> + regcache_cache_only(max98363->regmap, false);
> + regcache_sync(max98363->regmap);
> +
> + return 0;
> +}
> +
> +static const struct dev_pm_ops max98363_pm = {
> + SET_SYSTEM_SLEEP_PM_OPS(max98363_suspend, max98363_resume)
> + SET_RUNTIME_PM_OPS(max98363_suspend, max98363_resume, NULL)
> +};
> +
DEFINE_RUNTIME_DEV_PM_OPS()...
> +static int max98363_read_prop(struct sdw_slave *slave)
> +{
> + struct sdw_slave_prop *prop = &slave->prop;
> + int nval, i;
> + u32 bit;
> + unsigned long addr;
> + struct sdw_dpn_prop *dpn;
> +
...
> +static int max98363_init(struct sdw_slave *slave, struct regmap
> *regmap)
> +{
> + struct max98363_priv *max98363;
> + int ret;
> + struct device *dev = &slave->dev;
> +
> + /* Allocate and assign private driver data structure */
> + max98363 = devm_kzalloc(dev, sizeof(*max98363), GFP_KERNEL);
> + if (!max98363)
> + return -ENOMEM;
> +
> + dev_set_drvdata(dev, max98363);
> + max98363->regmap = regmap;
> + max98363->slave = slave;
> +
> + max98363->hw_init = false;
> + max98363->first_hw_init = false;
> +
> + max98363->vdd = devm_regulator_get_optional(dev, "vdd");
vdd is not an optional regulator, right?
> + if (IS_ERR(max98363->vdd)) {
> + if (PTR_ERR(max98363->vdd) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + max98363->vdd = NULL;
> + }
> + max98363->dvddio = devm_regulator_get_optional(dev,
> "dvddio");
ditto...
> + if (IS_ERR(max98363->dvddio)) {
> + if (PTR_ERR(max98363->dvddio) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> +
> + max98363->dvddio = NULL;
> + }
> +
> + if (max98363->vdd) {
> + ret = regulator_enable(max98363->vdd);
> + if (ret < 0)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev,
> +
> max98363_supply_disable,
> + max98363->vdd);
> + if (ret < 0)
> + return ret;
> + }
> +
> + if (max98363->dvddio) {
> + ret = regulator_enable(max98363->dvddio);
> + if (ret < 0)
> + return ret;
> +
> + ret = devm_add_action_or_reset(dev,
> +
> max98363_supply_disable,
> + max98363->dvddio);
> + if (ret < 0)
> + return ret;
> + }
> +
Being said the regulators are not optional, you can use
devm_regulator_get_enable(). If there's no constrains in power up/down
ordering, consider devm_regulator_bulk_get_enable().
> + /* codec registration */
> + ret = devm_snd_soc_register_component(dev,
> &soc_codec_dev_max98363,
> + max98363_dai,
> +
> ARRAY_SIZE(max98363_dai));
> + if (ret < 0)
> + dev_err(dev, "Failed to register codec: %d\n", ret);
> +
> + return ret;
> +}
> +
> +static int max98363_sdw_probe(struct sdw_slave *slave,
> + const struct sdw_device_id *id)
> +{
> + struct regmap *regmap;
> +
> + /* Regmap Initialization */
> + regmap = devm_regmap_init_sdw(slave, &max98363_sdw_regmap);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + return max98363_init(slave, regmap);
> +}
> +
> +#if defined(CONFIG_OF)
no need for this guard...
> +static const struct of_device_id max98363_of_match[] = {
> + { .compatible = "adi,max98363", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, max98363_of_match);
> +#endif
> +
> +#ifdef CONFIG_ACPI
and this. See below
> +static const struct acpi_device_id max98363_acpi_match[] = {
> + { "ADS8363", 0 },
> + {},
> +};
> +MODULE_DEVICE_TABLE(acpi, max98363_acpi_match);
> +#endif
> +
> +static const struct sdw_device_id max98363_id[] = {
> + SDW_SLAVE_ENTRY(0x019F, 0x8363, 0),
> + {},
> +};
> +MODULE_DEVICE_TABLE(sdw, max98363_id);
> +
> +static struct sdw_driver max98363_sdw_driver = {
> + .driver = {
> + .name = "max98363",
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(max98363_of_match),
I think you can drop 'acpi_match_table'. If you drop of_match_ptr() and
always define the of_device_id table, ACPI can use it to probe the
device.
> + .acpi_match_table = ACPI_PTR(max98363_acpi_match),
> + .pm = &max98363_pm,
use pm_ptr(). With this macro, no need for '__maybe_unused'
annotations.
> + },
> + .probe = max98363_sdw_probe,
> + .remove = NULL,
no need for this.
> + .ops = &max98363_slave_ops,
> + .id_table = max98363_id,
> +};
> +
> +module_sdw_driver(max98363_sdw_driver);
> +
>
- Nuno Sá
Powered by blists - more mailing lists