Re: [PATCH v3 3/5] IIO: Ingenic JZ47xx: Add touchscreen mode.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Jonathan,


Le sam., mars 7, 2020 at 12:47, Jonathan Cameron <jic23@xxxxxxxxxxxxxxxxxxxxx> a écrit :
On Sun,  1 Mar 2020 16:09:18 +0100
Artur Rojek <contact@xxxxxxxxxxxxxx> wrote:

 The SADC component in JZ47xx SoCs provides support for touchscreen
 operations (pen position and pen down pressure) in single-ended and
 differential modes.

Of the known hardware to use this controller, GCW Zero and Anbernic RG-350 utilize the touchscreen mode by having their joystick(s) attached to the
 X/Y positive/negative input pins.
GCW Zero comes with a single joystick and is sufficiently handled with the
 currently implemented single-ended mode. Support for boards with two
joysticks, where one is hooked up to Xn/Yn and the other to Xp/Yp channels
 will need to be provided in the future.

The touchscreen component of SADC takes a significant time to stabilize after first receiving the clock and a delay of 50ms has been empirically
 proven to be a safe value before data sampling can begin.

 Signed-off-by: Artur Rojek <contact@xxxxxxxxxxxxxx>
 Tested-by: Paul Cercueil <paul@xxxxxxxxxxxxxxx>

This relies on the use of an irq that wasn't previously used. Was that
always hooked up?  If not we need to still work as before when it
isn't provided.

Otherwise this looks fine to me.

All the boards that probe the ingenic-adc driver have the interrupt provided from devicetree, yes.

Cheers,
-Paul


 ---

  Changes:

  v2: - improve description of the touchscreen mode,
      - get rid of the unneeded kfifo,
      - drop IIO_BUFFER_CB from Kconfig,
      - remove extended names from the touchscreen channels

  v3: remove unneeded `linux/iio/kfifo_buf.h` include

  drivers/iio/adc/Kconfig       |   1 +
drivers/iio/adc/ingenic-adc.c | 109 +++++++++++++++++++++++++++++++++-
  2 files changed, 108 insertions(+), 2 deletions(-)

 diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
 index 82e33082958c..d3fd4b6e2d47 100644
 --- a/drivers/iio/adc/Kconfig
 +++ b/drivers/iio/adc/Kconfig
 @@ -453,6 +453,7 @@ config INA2XX_ADC
  config INGENIC_ADC
  	tristate "Ingenic JZ47xx SoCs ADC driver"
  	depends on MIPS || COMPILE_TEST
 +	select IIO_BUFFER
  	help
Say yes here to build support for the Ingenic JZ47xx SoCs ADC unit.

diff --git a/drivers/iio/adc/ingenic-adc.c b/drivers/iio/adc/ingenic-adc.c
 index 7a24bc1dabe1..0dafc8d5d0d8 100644
 --- a/drivers/iio/adc/ingenic-adc.c
 +++ b/drivers/iio/adc/ingenic-adc.c
 @@ -8,7 +8,9 @@

  #include <dt-bindings/iio/adc/ingenic,adc.h>
  #include <linux/clk.h>
 +#include <linux/iio/buffer.h>
  #include <linux/iio/iio.h>
 +#include <linux/interrupt.h>
  #include <linux/io.h>
  #include <linux/iopoll.h>
  #include <linux/kernel.h>
 @@ -20,6 +22,8 @@
  #define JZ_ADC_REG_CFG			0x04
  #define JZ_ADC_REG_CTRL			0x08
  #define JZ_ADC_REG_STATUS		0x0c
 +#define JZ_ADC_REG_ADSAME		0x10
 +#define JZ_ADC_REG_ADWAIT		0x14
  #define JZ_ADC_REG_ADTCH		0x18
  #define JZ_ADC_REG_ADBDAT		0x1c
  #define JZ_ADC_REG_ADSDAT		0x20
 @@ -28,6 +32,9 @@
  #define JZ_ADC_REG_ENABLE_PD		BIT(7)
  #define JZ_ADC_REG_CFG_AUX_MD		(BIT(0) | BIT(1))
  #define JZ_ADC_REG_CFG_BAT_MD		BIT(4)
 +#define JZ_ADC_REG_CFG_PULL_UP(n)	((n) << 16)
 +#define JZ_ADC_REG_CFG_SAMPLE_NUM(n)	((n) << 10)
 +#define JZ_ADC_REG_CFG_TOUCH_OPS_MASK	(BIT(31) | GENMASK(23, 10))
  #define JZ_ADC_REG_ADCLK_CLKDIV_LSB	0
  #define JZ4725B_ADC_REG_ADCLK_CLKDIV10US_LSB	16
  #define JZ4770_ADC_REG_ADCLK_CLKDIV10US_LSB	8
 @@ -44,6 +51,14 @@
  #define JZ4770_ADC_BATTERY_VREF			6600
  #define JZ4770_ADC_BATTERY_VREF_BITS		12

 +#define JZ_ADC_IRQ_AUX			BIT(0)
 +#define JZ_ADC_IRQ_BATTERY		BIT(1)
 +#define JZ_ADC_IRQ_TOUCH		BIT(2)
 +#define JZ_ADC_IRQ_PEN_DOWN		BIT(3)
 +#define JZ_ADC_IRQ_PEN_UP		BIT(4)
 +#define JZ_ADC_IRQ_PEN_DOWN_SLEEP	BIT(5)
 +#define JZ_ADC_IRQ_SLEEP		BIT(7)
 +
  struct ingenic_adc;

  struct ingenic_adc_soc_data {
@@ -411,6 +426,28 @@ static const struct iio_info ingenic_adc_info = {
  };

  static const struct iio_chan_spec ingenic_channels[] = {
 +	{
 +		.type = IIO_POSITIONRELATIVE,
 +		.indexed = 1,
 +		.channel = INGENIC_ADC_TOUCH_XP,
 +		.scan_index = 0,
 +		.scan_type = {
 +			.sign = 'u',
 +			.realbits = 12,
 +			.storagebits = 16
 +		},
 +	},
 +	{
 +		.type = IIO_POSITIONRELATIVE,
 +		.indexed = 1,
 +		.channel = INGENIC_ADC_TOUCH_YP,
 +		.scan_index = 1,
 +		.scan_type = {
 +			.sign = 'u',
 +			.realbits = 12,
 +			.storagebits = 16
 +		},
 +	},
  	{
  		.extend_name = "aux",
  		.type = IIO_VOLTAGE,
@@ -418,6 +455,7 @@ static const struct iio_chan_spec ingenic_channels[] = {
  				      BIT(IIO_CHAN_INFO_SCALE),
  		.indexed = 1,
  		.channel = INGENIC_ADC_AUX,
 +		.scan_index = -1
  	},
  	{
  		.extend_name = "battery",
@@ -428,6 +466,7 @@ static const struct iio_chan_spec ingenic_channels[] = {
  						BIT(IIO_CHAN_INFO_SCALE),
  		.indexed = 1,
  		.channel = INGENIC_ADC_BATTERY,
 +		.scan_index = -1
  	},
  	{ /* Must always be last in the array. */
  		.extend_name = "aux2",
@@ -436,16 +475,69 @@ static const struct iio_chan_spec ingenic_channels[] = {
  				      BIT(IIO_CHAN_INFO_SCALE),
  		.indexed = 1,
  		.channel = INGENIC_ADC_AUX2,
 +		.scan_index = -1
  	},
  };

 +static int ingenic_adc_buffer_enable(struct iio_dev *iio_dev)
 +{
 +	struct ingenic_adc *adc = iio_priv(iio_dev);
 +
 +	clk_enable(adc->clk);
+ /* It takes significant time for the touchscreen hw to stabilize. */
 +	msleep(50);
 +	ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_TOUCH_OPS_MASK,
 +			       JZ_ADC_REG_CFG_SAMPLE_NUM(4) |
 +			       JZ_ADC_REG_CFG_PULL_UP(4));
 +	writew(80, adc->base + JZ_ADC_REG_ADWAIT);
 +	writew(2, adc->base + JZ_ADC_REG_ADSAME);
 +	writeb((u8)~JZ_ADC_IRQ_TOUCH, adc->base + JZ_ADC_REG_CTRL);
 +	writel(0, adc->base + JZ_ADC_REG_ADTCH);
 +	ingenic_adc_enable(adc, 2, true);
 +
 +	return 0;
 +}
 +
 +static int ingenic_adc_buffer_disable(struct iio_dev *iio_dev)
 +{
 +	struct ingenic_adc *adc = iio_priv(iio_dev);
 +
 +	ingenic_adc_enable(adc, 2, false);
 +	writeb(0xff, adc->base + JZ_ADC_REG_CTRL);
 +	writeb(0xff, adc->base + JZ_ADC_REG_STATUS);
 +	ingenic_adc_set_config(adc, JZ_ADC_REG_CFG_TOUCH_OPS_MASK, 0);
 +	writew(0, adc->base + JZ_ADC_REG_ADSAME);
 +	writew(0, adc->base + JZ_ADC_REG_ADWAIT);
 +	clk_disable(adc->clk);
 +
 +	return 0;
 +}
 +
+static const struct iio_buffer_setup_ops ingenic_buffer_setup_ops = {
 +	.postenable = &ingenic_adc_buffer_enable,
 +	.predisable = &ingenic_adc_buffer_disable
 +};
 +
 +static irqreturn_t ingenic_adc_irq(int irq, void *data)
 +{
 +	struct iio_dev *iio_dev = data;
 +	struct ingenic_adc *adc = iio_priv(iio_dev);
 +	u32 tdat;
 +
 +	tdat = readl(adc->base + JZ_ADC_REG_ADTCH);
 +	iio_push_to_buffers(iio_dev, &tdat);
 +	writeb(JZ_ADC_IRQ_TOUCH, adc->base + JZ_ADC_REG_STATUS);
 +
 +	return IRQ_HANDLED;
 +}
 +
  static int ingenic_adc_probe(struct platform_device *pdev)
  {
  	struct device *dev = &pdev->dev;
  	struct iio_dev *iio_dev;
  	struct ingenic_adc *adc;
  	const struct ingenic_adc_soc_data *soc_data;
 -	int ret;
 +	int irq, ret;

  	soc_data = device_get_match_data(dev);
  	if (!soc_data)
@@ -460,6 +552,18 @@ static int ingenic_adc_probe(struct platform_device *pdev)
  	mutex_init(&adc->aux_lock);
  	adc->soc_data = soc_data;

 +	irq = platform_get_irq(pdev, 0);

Do we need a fallback path if there is no irq provided? We can't break existing
supported devices that don't specify one.

 +	if (irq < 0) {
 +		dev_err(dev, "Failed to get irq: %d\n", irq);
 +		return irq;
 +	}
 +	ret = devm_request_irq(dev, irq, ingenic_adc_irq, 0,
 +			       dev_name(dev), iio_dev);
 +	if (ret < 0) {
 +		dev_err(dev, "Failed to request irq: %d\n", ret);
 +		return ret;
 +	}
 +
  	adc->base = devm_platform_ioremap_resource(pdev, 0);
  	if (IS_ERR(adc->base))
  		return PTR_ERR(adc->base);
@@ -499,7 +603,8 @@ static int ingenic_adc_probe(struct platform_device *pdev)

  	iio_dev->dev.parent = dev;
  	iio_dev->name = "jz-adc";
 -	iio_dev->modes = INDIO_DIRECT_MODE;
 +	iio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
 +	iio_dev->setup_ops = &ingenic_buffer_setup_ops;
  	iio_dev->channels = ingenic_channels;
  	iio_dev->num_channels = ARRAY_SIZE(ingenic_channels);
  	/* Remove AUX2 from the list of supported channels. */







[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux