by Dmitry Torokhov

[permalink] [raw]

Subject: Re: [PATCH v2 06/12] Input: wm97xx: add new AC97 bus support

On Mon, Jun 19, 2017 at 09:27:03AM +0200, Robert Jarzmik wrote:
> This adds support for the new AC97 bus code, which discovers the devices
> rather than uses platform data.
>
> As part of this discovery, it enables a multi-function device wm97xx,
> which supports touchscreen, battery, ADC and an audio codec. This patch
> adds the code to bind the touchscreen "cell" as the touchscreen driver.
>
> This was tested on the pxa architecture with a pxa270 + wm9713 + the
> mioa701 touchscreen.
>
> Signed-off-by: Robert Jarzmik <[email protected]>
> Acked-by: Charles Keepax <[email protected]>

Acked-by: Dmitry Torokhov <[email protected]>

> ---
> drivers/input/touchscreen/Kconfig | 2 +-
> drivers/input/touchscreen/wm97xx-core.c | 56 ++++++++++++++++++++++++++++++++-
> 2 files changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 033599777651..22257f3e1059 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -725,7 +725,7 @@ config TOUCHSCREEN_WM831X
>
> config TOUCHSCREEN_WM97XX
> tristate "Support for WM97xx AC97 touchscreen controllers"
> - depends on AC97_BUS
> + depends on AC97_BUS || AC97_BUS_NEW
> help
> Say Y here if you have a Wolfson Microelectronics WM97xx
> touchscreen connected to your system. Note that this option
> diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
> index 39869ffdc4fa..fd714ee881f7 100644
> --- a/drivers/input/touchscreen/wm97xx-core.c
> +++ b/drivers/input/touchscreen/wm97xx-core.c
> @@ -44,6 +44,7 @@
> #include <linux/pm.h>
> #include <linux/interrupt.h>
> #include <linux/bitops.h>
> +#include <linux/mfd/wm97xx.h>
> #include <linux/workqueue.h>
> #include <linux/wm97xx.h>
> #include <linux/uaccess.h>
> @@ -766,6 +767,39 @@ static int wm97xx_remove(struct device *dev)
> return 0;
> }
>
> +static int wm97xx_mfd_probe(struct platform_device *pdev)
> +{
> + struct wm97xx *wm;
> + struct wm97xx_platform_data *mfd_pdata = dev_get_platdata(&pdev->dev);
> + int ret;
> +
> + wm = devm_kzalloc(&pdev->dev, sizeof(struct wm97xx), GFP_KERNEL);
> + if (!wm)
> + return -ENOMEM;
> +
> + wm->dev = &pdev->dev;
> + wm->ac97 = mfd_pdata->ac97;
> +
> + ret = _wm97xx_probe(wm);
> + if (ret)
> + return ret;
> +
> + ret = wm97xx_add_battery(wm, mfd_pdata->batt_pdata);
> + if (ret < 0)
> + goto batt_err;
> +
> + return ret;
> +
> +batt_err:
> + wm97xx_unregister_touch(wm);
> + return ret;
> +}
> +
> +static int wm97xx_mfd_remove(struct platform_device *pdev)
> +{
> + return wm97xx_remove(&pdev->dev);
> +}
> +
> static int __maybe_unused wm97xx_suspend(struct device *dev)
> {
> struct wm97xx *wm = dev_get_drvdata(dev);
> @@ -862,21 +896,41 @@ EXPORT_SYMBOL_GPL(wm97xx_unregister_mach_ops);
>
> static struct device_driver wm97xx_driver = {
> .name = "wm97xx-ts",
> +#ifdef CONFIG_AC97_BUS
> .bus = &ac97_bus_type,
> +#endif
> .owner = THIS_MODULE,
> .probe = wm97xx_probe,
> .remove = wm97xx_remove,
> .pm = &wm97xx_pm_ops,
> };
>
> +static struct platform_driver wm97xx_mfd_driver = {
> + .driver = {
> + .name = "wm97xx-ts",
> + .pm = &wm97xx_pm_ops,
> + },
> + .probe = wm97xx_mfd_probe,
> + .remove = wm97xx_mfd_remove,
> +};
> +
> static int __init wm97xx_init(void)
> {
> - return driver_register(&wm97xx_driver);
> + int ret;
> +
> + ret = platform_driver_register(&wm97xx_mfd_driver);
> + if (ret)
> + return ret;
> +
> + if (IS_BUILTIN(CONFIG_AC97_BUS))
> + ret = driver_register(&wm97xx_driver);
> + return ret;
> }
>
> static void __exit wm97xx_exit(void)
> {
> driver_unregister(&wm97xx_driver);
> + platform_driver_unregister(&wm97xx_mfd_driver);
> }
>
> module_init(wm97xx_init);
> --
> 2.1.4
>

--
Dmitry

2017-06-28 19:54:37

The patch

Input: wm97xx: split out touchscreen registering

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From c72f61e7407335dfa5fe5947827777b1429cd883 Mon Sep 17 00:00:00 2001
From: Robert Jarzmik <[email protected]>
Date: Wed, 13 Sep 2017 21:37:16 +0200
Subject: [PATCH] Input: wm97xx: split out touchscreen registering

wm97xx-core does several things in it initialization :
- touchscreen input device setup
- battery device creation

As the wm97xx is actually a multi-function device handling an audio
codec, a touchscreen, a gpio block and an ADC, reshape the probing to
isolate what is truly input/touchscreen specific from the remaining
part.

This is only code shuffling, there is no functional change.

Signed-off-by: Robert Jarzmik <[email protected]>
Acked-by: Dmitry Torokhov <[email protected]>
Acked-by: Charles Keepax <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/input/touchscreen/wm97xx-core.c | 196 +++++++++++++++++++-------------
1 file changed, 115 insertions(+), 81 deletions(-)

diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c
index c9d1c91e1887..39869ffdc4fa 100644
--- a/drivers/input/touchscreen/wm97xx-core.c
+++ b/drivers/input/touchscreen/wm97xx-core.c
@@ -581,27 +581,85 @@ static void wm97xx_ts_input_close(struct input_dev *idev)
wm->codec->acc_enable(wm, 0);
}

-static int wm97xx_probe(struct device *dev)
+static int wm97xx_register_touch(struct wm97xx *wm)
{
- struct wm97xx *wm;
- struct wm97xx_pdata *pdata = dev_get_platdata(dev);
- int ret = 0, id = 0;
+ struct wm97xx_pdata *pdata = dev_get_platdata(wm->dev);
+ int ret;

- wm = kzalloc(sizeof(struct wm97xx), GFP_KERNEL);
- if (!wm)
+ wm->input_dev = devm_input_allocate_device(wm->dev);
+ if (wm->input_dev == NULL)
return -ENOMEM;
- mutex_init(&wm->codec_mutex);

- wm->dev = dev;
- dev_set_drvdata(dev, wm);
- wm->ac97 = to_ac97_t(dev);
+ /* set up touch configuration */
+ wm->input_dev->name = "wm97xx touchscreen";
+ wm->input_dev->phys = "wm97xx";
+ wm->input_dev->open = wm97xx_ts_input_open;
+ wm->input_dev->close = wm97xx_ts_input_close;
+
+ __set_bit(EV_ABS, wm->input_dev->evbit);
+ __set_bit(EV_KEY, wm->input_dev->evbit);
+ __set_bit(BTN_TOUCH, wm->input_dev->keybit);
+
+ input_set_abs_params(wm->input_dev, ABS_X, abs_x[0], abs_x[1],
+ abs_x[2], 0);
+ input_set_abs_params(wm->input_dev, ABS_Y, abs_y[0], abs_y[1],
+ abs_y[2], 0);
+ input_set_abs_params(wm->input_dev, ABS_PRESSURE, abs_p[0], abs_p[1],
+ abs_p[2], 0);
+
+ input_set_drvdata(wm->input_dev, wm);
+ wm->input_dev->dev.parent = wm->dev;
+
+ ret = input_register_device(wm->input_dev);
+ if (ret)
+ return ret;
+
+ /*
+ * register our extended touch device (for machine specific
+ * extensions)
+ */
+ wm->touch_dev = platform_device_alloc("wm97xx-touch", -1);
+ if (!wm->touch_dev) {
+ ret = -ENOMEM;
+ goto touch_err;
+ }
+ platform_set_drvdata(wm->touch_dev, wm);
+ wm->touch_dev->dev.parent = wm->dev;
+ wm->touch_dev->dev.platform_data = pdata;
+ ret = platform_device_add(wm->touch_dev);
+ if (ret < 0)
+ goto touch_reg_err;
+
+ return 0;
+touch_reg_err:
+ platform_device_put(wm->touch_dev);
+touch_err:
+ input_unregister_device(wm->input_dev);
+ wm->input_dev = NULL;
+
+ return ret;
+}
+
+static void wm97xx_unregister_touch(struct wm97xx *wm)
+{
+ platform_device_unregister(wm->touch_dev);
+ input_unregister_device(wm->input_dev);
+ wm->input_dev = NULL;
+}
+
+static int _wm97xx_probe(struct wm97xx *wm)
+{
+ int id = 0;
+
+ mutex_init(&wm->codec_mutex);
+ dev_set_drvdata(wm->dev, wm);

/* check that we have a supported codec */
id = wm97xx_reg_read(wm, AC97_VENDOR_ID1);
if (id != WM97XX_ID1) {
- dev_err(dev, "Device with vendor %04x is not a wm97xx\n", id);
- ret = -ENODEV;
- goto alloc_err;
+ dev_err(wm->dev,
+ "Device with vendor %04x is not a wm97xx\n", id);
+ return -ENODEV;
}

wm->id = wm97xx_reg_read(wm, AC97_VENDOR_ID2);
@@ -629,8 +687,7 @@ static int wm97xx_probe(struct device *dev)
default:
dev_err(wm->dev, "Support for wm97%02x not compiled in.\n",
wm->id & 0xff);
- ret = -ENODEV;
- goto alloc_err;
+ return -ENODEV;
}

/* set up physical characteristics */
@@ -644,79 +701,58 @@ static int wm97xx_probe(struct device *dev)
wm->gpio[4] = wm97xx_reg_read(wm, AC97_GPIO_STATUS);
wm->gpio[5] = wm97xx_reg_read(wm, AC97_MISC_AFE);

- wm->input_dev = input_allocate_device();
- if (wm->input_dev == NULL) {
- ret = -ENOMEM;
- goto alloc_err;
- }
-
- /* set up touch configuration */
- wm->input_dev->name = "wm97xx touchscreen";
- wm->input_dev->phys = "wm97xx";
- wm->input_dev->open = wm97xx_ts_input_open;
- wm->input_dev->close = wm97xx_ts_input_close;
-
- __set_bit(EV_ABS, wm->input_dev->evbit);
- __set_bit(EV_KEY, wm->input_dev->evbit);
- __set_bit(BTN_TOUCH, wm->input_dev->keybit);
-
- input_set_abs_params(wm->input_dev, ABS_X, abs_x[0], abs_x[1],
- abs_x[2], 0);
- input_set_abs_params(wm->input_dev, ABS_Y, abs_y[0], abs_y[1],
- abs_y[2], 0);
- input_set_abs_params(wm->input_dev, ABS_PRESSURE, abs_p[0], abs_p[1],
- abs_p[2], 0);
+ return wm97xx_register_touch(wm);
+}

- input_set_drvdata(wm->input_dev, wm);
- wm->input_dev->dev.parent = dev;
+static void wm97xx_remove_battery(struct wm97xx *wm)
+{
+ platform_device_unregister(wm->battery_dev);
+}

- ret = input_register_device(wm->input_dev);
- if (ret < 0)
- goto dev_alloc_err;
+static int wm97xx_add_battery(struct wm97xx *wm,
+ struct wm97xx_batt_pdata *pdata)
+{
+ int ret;

- /* register our battery device */
wm->battery_dev = platform_device_alloc("wm97xx-battery", -1);
- if (!wm->battery_dev) {
- ret = -ENOMEM;
- goto batt_err;
- }
+ if (!wm->battery_dev)
+ return -ENOMEM;
+
platform_set_drvdata(wm->battery_dev, wm);
- wm->battery_dev->dev.parent = dev;
- wm->battery_dev->dev.platform_data = pdata ? pdata->batt_pdata : NULL;
+ wm->battery_dev->dev.parent = wm->dev;
+ wm->battery_dev->dev.platform_data = pdata;
ret = platform_device_add(wm->battery_dev);
- if (ret < 0)
- goto batt_reg_err;
+ if (ret)
+ platform_device_put(wm->battery_dev);

- /* register our extended touch device (for machine specific
- * extensions) */
- wm->touch_dev = platform_device_alloc("wm97xx-touch", -1);
- if (!wm->touch_dev) {
- ret = -ENOMEM;
- goto touch_err;
- }
- platform_set_drvdata(wm->touch_dev, wm);
- wm->touch_dev->dev.parent = dev;
- wm->touch_dev->dev.platform_data = pdata;
- ret = platform_device_add(wm->touch_dev);
+ return ret;
+}
+
+static int wm97xx_probe(struct device *dev)
+{
+ struct wm97xx *wm;
+ int ret;
+ struct wm97xx_pdata *pdata = dev_get_platdata(dev);
+
+ wm = devm_kzalloc(dev, sizeof(struct wm97xx), GFP_KERNEL);
+ if (!wm)
+ return -ENOMEM;
+
+ wm->dev = dev;
+ wm->ac97 = to_ac97_t(dev);
+
+ ret = _wm97xx_probe(wm);
+ if (ret)
+ return ret;
+
+ ret = wm97xx_add_battery(wm, pdata ? pdata->batt_pdata : NULL);
if (ret < 0)
- goto touch_reg_err;
+ goto batt_err;

return ret;

- touch_reg_err:
- platform_device_put(wm->touch_dev);
- touch_err:
- platform_device_del(wm->battery_dev);
- batt_reg_err:
- platform_device_put(wm->battery_dev);
- batt_err:
- input_unregister_device(wm->input_dev);
- wm->input_dev = NULL;
- dev_alloc_err:
- input_free_device(wm->input_dev);
- alloc_err:
- kfree(wm);
-
+batt_err:
+ wm97xx_unregister_touch(wm);
return ret;
}

@@ -724,10 +760,8 @@ static int wm97xx_remove(struct device *dev)
{
struct wm97xx *wm = dev_get_drvdata(dev);

- platform_device_unregister(wm->battery_dev);
- platform_device_unregister(wm->touch_dev);
- input_unregister_device(wm->input_dev);
- kfree(wm);
+ wm97xx_remove_battery(wm);
+ wm97xx_unregister_touch(wm);

return 0;
}
--
2.14.1