On Sat, 02 Sep 2017, Robert Jarzmik wrote:
The WM9705, WM9712 and WM9713 are highly integrated codecs, with an audio codec, DAC and ADC, GPIO unit and a touchscreen interface.
Historically the support was spread across drivers/input/touchscreen and sound/soc/codecs. The sharing was done through ac97 bus sharing. This model will not withstand the new AC97 bus model, where codecs are discovered on runtime.
Signed-off-by: Robert Jarzmik robert.jarzmik@free.fr Acked-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com
Since v3:
- added a "depends on AC97_BUS_NEW" Kconfig statement
- added default values for wm9705, wm9712 per Charles's comment
Since v4:
- added Charles's ack
Since v5:
- took into account Lee's comments
drivers/mfd/Kconfig | 14 ++ drivers/mfd/Makefile | 1 + drivers/mfd/wm97xx-core.c | 379 +++++++++++++++++++++++++++++++++++++++++++++ include/linux/mfd/wm97xx.h | 25 +++ 4 files changed, 419 insertions(+) create mode 100644 drivers/mfd/wm97xx-core.c create mode 100644 include/linux/mfd/wm97xx.h
[...]
+static struct mfd_cell wm9705_cells[] = {
- {
.name = "wm9705-codec",- },
- {
.name = "wm97xx-ts",- },
+};
Nit: These should be one line entries:
{ .name = "wm9705-codec" }, { .name = "wm97xx-ts" },
[...]
+static struct mfd_cell wm9712_cells[] = {
- {
.name = "wm9712-codec",- },
- {
.name = "wm97xx-ts",- },
+};
As above.
[...]
+static struct mfd_cell wm9713_cells[] = {
- {
.name = "wm9713-codec",- },
- {
.name = "wm97xx-ts",- },
+};
And again.
+static int wm97xx_ac97_probe(struct ac97_codec_device *adev) +{
- struct wm97xx_priv *wm97xx;
- const struct regmap_config *config;
- struct wm97xx_platform_data *codec_pdata;
- struct mfd_cell *cells;
- int ret = 0, nb_cells, i;
- struct wm97xx_pdata *pdata = snd_ac97_codec_get_platdata(adev);
- wm97xx = devm_kzalloc(ac97_codec_dev2dev(adev),
sizeof(*wm97xx), GFP_KERNEL);- if (!wm97xx)
return -ENOMEM;- wm97xx->dev = ac97_codec_dev2dev(adev);
- wm97xx->ac97 = snd_ac97_compat_alloc(adev);
- if (IS_ERR(wm97xx->ac97))
return PTR_ERR(wm97xx->ac97);- ac97_set_drvdata(adev, wm97xx);
- dev_info(wm97xx->dev, "wm97xx core found, id=0x%x\n",
adev->vendor_id);- codec_pdata = &wm97xx->codec_pdata;
- codec_pdata->ac97 = wm97xx->ac97;
- codec_pdata->batt_pdata = pdata->batt_pdata;
- switch (adev->vendor_id) {
- case WM9705_VENDOR_ID:
config = &wm9705_regmap_config;cells = wm9705_cells;nb_cells = ARRAY_SIZE(wm9705_cells);break;- case WM9712_VENDOR_ID:
config = &wm9712_regmap_config;cells = wm9712_cells;nb_cells = ARRAY_SIZE(wm9712_cells);break;- case WM9713_VENDOR_ID:
config = &wm9713_regmap_config;cells = wm9713_cells;nb_cells = ARRAY_SIZE(wm9713_cells);break;- default:
config = NULL;
goto an error label here.
- }
- for (i = 0; i < nb_cells; i++) {
cells[i].platform_data = codec_pdata;cells[i].pdata_size = sizeof(*codec_pdata);- }
- if (config) {
codec_pdata->regmap =devm_regmap_init_ac97(wm97xx->ac97, config);if (IS_ERR(codec_pdata->regmap))ret = PTR_ERR(codec_pdata->regmap);
Remove the if and do this regardless.
- } else {
ret = -ENODEV;- }
remove this.
- if (!ret)
ret = devm_mfd_add_devices(wm97xx->dev, PLATFORM_DEVID_NONE,cells, nb_cells, NULL, 0, NULL);
Remove the if and do this regardless.
No need to allocate to a variable. Just return the result directly.
- if (ret)
snd_ac97_compat_release(wm97xx->ac97);
Place this in the error path, under the goto label.
- return ret;
+}
Once fixed, please apply my:
For my own reference: Acked-for-MFD-by: Lee Jones lee.jones@linaro.org