lkml.org 
[lkml]   [2019]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v2 3/3] input: touchscreen mc13xxx: Add mc34708 support
Hi Lukasz,

On Fri, Jul 12, 2019 at 12:23:46AM +0200, Lukasz Majewski wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
>
> The mc34708 has a different bit to enable pen detection. This
> adds the driver data and devtype necessary to probe the device
> and to distinguish between the mc13783 and the mc34708.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
>
> ---
> Changes for v2:
> - Change nested if statements to a single one (with cr0 > ...)
> - Replace hardcoded max resistance value (4080) with a generic driver data
> value.
> - Introduce new include/linux/mfd/mc34708.h header file for mc34708 specific
> defines
> - Define as driver data mask and value for accessing mc13xxx registers
>
> Changes from the original patch:
> - Simplify the mcXXXXX_set_pen_detection functions
> - Fix checkpatch warnings
> ---
> drivers/input/touchscreen/mc13783_ts.c | 59 +++++++++++++++++++++++++++++++---
> 1 file changed, 55 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/touchscreen/mc13783_ts.c b/drivers/input/touchscreen/mc13783_ts.c
> index edd49e44e0c9..8fd3d0e47f57 100644
> --- a/drivers/input/touchscreen/mc13783_ts.c
> +++ b/drivers/input/touchscreen/mc13783_ts.c
> @@ -10,6 +10,7 @@
> */
> #include <linux/platform_device.h>
> #include <linux/mfd/mc13783.h>
> +#include <linux/mfd/mc34708.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/input.h>
> @@ -30,6 +31,8 @@ MODULE_PARM_DESC(sample_tolerance,
> "is supposed to be wrong and is discarded. Set to 0 to "
> "disable this check.");
>
> +struct mc13xxx_driver_data;

Why don't you define the structure here instead of sing forward
declaration? The structure is also commonly called as xxx_chip, so
struct mc13xxx_chip {
...
};

> +
> struct mc13783_ts_priv {
> struct input_dev *idev;
> struct mc13xxx *mc13xxx;
> @@ -37,6 +40,33 @@ struct mc13783_ts_priv {
> unsigned int sample[4];
> u8 ato;
> bool atox;
> + struct mc13xxx_driver_data *drvdata;

const struct mc13xxx_chip *chip;

> +};
> +
> +enum mc13xxx_type {
> + MC13XXX_TYPE_MC13783,
> + MC13XXX_TYPE_MC34708,
> +};
> +
> +struct mc13xxx_driver_data {
> + enum mc13xxx_type type;
> + int max_resistance;
> + u32 reg_mask;
> + u32 reg_value;
> +};
> +
> +static struct mc13xxx_driver_data mc13783_driver_data = {
> + .type = MC13XXX_TYPE_MC13783,
> .max_resistance = 4096,
> + .reg_mask = MC13XXX_ADC0_TSMOD_MASK,
> + .reg_value = MC13XXX_ADC0_TSMOD0,
> +};
> +
> +static struct mc13xxx_driver_data mc34708_driver_data = {
> + .type = MC13XXX_TYPE_MC34708,
> + .max_resistance = 4080,
> + .reg_mask = MC34708_ADC0_TSMASK,
> + .reg_value = MC34708_ADC0_TSPENDETEN,
> };

Have these 2 closer to the ID table.

>
> static irqreturn_t mc13783_ts_handler(int irq, void *data)
> @@ -93,6 +123,10 @@ static void mc13783_ts_report_sample(struct mc13783_ts_priv *priv)
>
> cr0 = (cr0 + cr1) / 2;
>
> + if (priv->drvdata->type == MC13XXX_TYPE_MC34708 &&
> + cr0 > priv->drvdata->max_resistance)
> + cr0 = 0;

I would like to avoid the type comparisons. Given that both cr0 and cr1
can't be more than 4095 (because we limit them when parsing sampling
data) I think we can simply say

if (cr0 > priv->chip->max_resistance)
cr0 = 0;

Thanks.

--
Dmitry
\
 
 \ /
  Last update: 2019-07-15 09:08    [W:0.074 / U:43.752 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site