From: Marcus Folkesson <marcus.folkesson-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: "Mylène Josserand"
<mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Cc: dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
mark.rutland-5wv7dgnIgG8@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org
Subject: Re: [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen
Date: Mon, 22 Jan 2018 21:11:09 +0100 [thread overview]
Message-ID: <20180122201109.GA651@gmail.com> (raw)
In-Reply-To: <20171201153957.13053-2-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6537 bytes --]
Mylene,
On Fri, Dec 01, 2017 at 04:39:56PM +0100, Mylène Josserand wrote:
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -0,0 +1,1110 @@
> +/*
> + * Parade TrueTouch(TM) Standard Product V5 Module.
> + * For use with Parade touchscreen controllers.
> + *
> + * Copyright (C) 2015 Parade Technologies
> + * Copyright (C) 2012-2015 Cypress Semiconductor
> + * Copyright (C) 2017 Free Electrons
> + *
> + * Author: Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2, and only version 2, as published by the
> + * Free Software Foundation.
Please use SPDX license Identifier to describe the license.
E.g.
SPDX-License-Identifier: GPL-2.0
Also, the MODULE_LICENSE means GPL 2.0 or later per module.h and this does
not match the license description.
Could you make sure they are in sync?
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <asm/unaligned.h>
> +#include <linux/crc-itu-t.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/gpio.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/interrupt.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
#include <linux/bitops.h>
since you are using BIT()
[snip]
> +static int cyttsp5_setup_input_device(struct device *dev)
> +{
> + struct cyttsp5 *ts = dev_get_drvdata(dev);
> + struct cyttsp5_sysinfo *si = &ts->sysinfo;
> + int max_x, max_y, max_p;
> + int max_x_tmp, max_y_tmp;
> + int rc;
> +
> + __set_bit(EV_ABS, ts->input->evbit);
> + __set_bit(EV_REL, ts->input->evbit);
> + __set_bit(EV_KEY, ts->input->evbit);
> +
> + max_x_tmp = si->sensing_conf_data.res_x;
> + max_y_tmp = si->sensing_conf_data.res_y;
> + max_x = max_y_tmp - 1;
> + max_y = max_x_tmp - 1;
> + max_p = si->sensing_conf_data.max_z;
> +
> + input_mt_init_slots(ts->input, si->tch_abs[CY_TCH_T].max, 0);
> +
> + __set_bit(ABS_MT_POSITION_X, ts->input->absbit);
> + __set_bit(ABS_MT_POSITION_Y, ts->input->absbit);
> + __set_bit(ABS_MT_PRESSURE, ts->input->absbit);
Not needed, input_set_abs_params() will set the bits for you below.
> +
> + input_set_abs_params(ts->input, ABS_MT_POSITION_X, 0, max_x, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_POSITION_Y, 0, max_y, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_PRESSURE, 0, max_p, 0, 0);
> +
> + input_set_abs_params(ts->input, ABS_MT_TOUCH_MAJOR, 0, MAX_AREA, 0, 0);
> + input_set_abs_params(ts->input, ABS_MT_TOUCH_MINOR, 0, MAX_AREA, 0, 0);
> +
> + rc = input_register_device(ts->input);
> + if (rc < 0)
> + dev_err(dev, "Error, failed register input device r=%d\n", rc);
> +
> + return rc;
> +}
> +
> +
[snip]
> +static int cyttsp5_get_hid_descriptor(struct cyttsp5 *ts,
> + struct cyttsp5_hid_desc *desc)
> +{
> + struct device *dev = ts->dev;
> + __le16 hid_desc_register = HID_DESC_REG;
> + int rc;
> + u8 cmd[2];
> +
> + /* Read HID descriptor length and version */
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_BUSY;
> + mutex_unlock(&ts->system_lock);
> +
> + /* Set HID descriptor register */
> + memcpy(cmd, &hid_desc_register, sizeof(hid_desc_register));
> +
> + rc = cyttsp5_write(ts, HID_DESC_REG, NULL, 0);
> + if (rc < 0) {
> + dev_err(dev, "Failed to get HID descriptor, rc=%d\n", rc);
> + goto error;
> + }
> +
> + rc = wait_event_timeout(ts->wait_q, (ts->hid_cmd_state == HID_CMD_DONE),
> + msecs_to_jiffies(CY_HID_GET_HID_DESCRIPTOR_TIMEOUT));
> + if (!rc) {
> + dev_err(ts->dev, "HID get descriptor timed out\n");
> + rc = -ETIME;
> + goto error;
> + }
> +
> + memcpy(desc, ts->response_buf, sizeof(struct cyttsp5_hid_desc));
> +
> + /* Check HID descriptor length and version */
> + if (le16_to_cpu(desc->hid_desc_len) != sizeof(*desc) ||
> + le16_to_cpu(desc->bcd_version) != HID_VERSION) {
> + dev_err(dev, "Unsupported HID version\n");
> + return -ENODEV;
Maybe it is supposed to return here, but all other faults use "goto
error".
> + }
> +
> + goto exit;
> +
> +error:
> + mutex_lock(&ts->system_lock);
> + ts->hid_cmd_state = HID_CMD_DONE;
> + mutex_unlock(&ts->system_lock);
> +exit:
> + return rc;
> +}
> +
[snip]
> +static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
> + const char *name)
> +{
> + struct cyttsp5 *ts;
> + struct cyttsp5_sysinfo *si;
> + int rc = 0, i;
> +
> + ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + /* Initialize device info */
> + ts->regmap = regmap;
> + ts->dev = dev;
> + si = &ts->sysinfo;
> + dev_set_drvdata(dev, ts);
> +
> + /* Initialize mutexes and spinlocks */
No spinlocks here :-)
> + mutex_init(&ts->system_lock);
> +
> + /* Initialize wait queue */
> + init_waitqueue_head(&ts->wait_q);
> +
> + /* Reset the gpio to be in a reset state */
> + ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(ts->reset_gpio)) {
> + rc = PTR_ERR(ts->reset_gpio);
> + dev_err(dev, "Failed to request reset gpio, error %d\n", rc);
> + return rc;
> + }
> + gpiod_set_value(ts->reset_gpio, 1);
> +
[snip]
> +static struct i2c_driver cyttsp5_i2c_driver = {
> + .driver = {
> + .name = CYTTSP5_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = of_match_ptr(cyttsp5_of_match),
> + },
> + .probe = cyttsp5_i2c_probe,
> + .remove = cyttsp5_i2c_remove,
> + .id_table = cyttsp5_i2c_id,
> +};
> +
> +module_i2c_driver(cyttsp5_i2c_driver);
> +
> +MODULE_LICENSE("GPL");
From linux/module.h:
/*
* The following license idents are currently accepted as indicating free
* software modules
*
* "GPL" [GNU Public License v2 or later]
* "GPL v2" [GNU Public License v2]
> +MODULE_DESCRIPTION("Touchscreen driver for Cypress TrueTouch Gen 5 Product");
> +MODULE_AUTHOR("Mylène Josserand <mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>");
> --
> 2.11.0
>
Best regards
Marcus Folkesson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2018-01-22 20:11 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-01 15:39 [PATCH v4 0/2] Input: Add Cypress Gen5 Touchscreen driver Mylène Josserand
2017-12-01 15:39 ` [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen Mylène Josserand
2017-12-03 14:29 ` [PATCH] Input: fix platform_no_drv_owner.cocci warnings kbuild test robot
2017-12-03 14:29 ` [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen kbuild test robot
2017-12-13 11:37 ` kbuild test robot
2017-12-13 11:37 ` [RFC PATCH] Input: hid_cmd_state can be static kbuild test robot
2018-01-22 12:50 ` [PATCH v4 1/2] Input: Add driver for Cypress Generation 5 touchscreen Maxime Ripard
[not found] ` <20171201153957.13053-2-mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2018-01-22 20:11 ` Marcus Folkesson [this message]
2018-01-23 9:33 ` Mylene Josserand
2018-01-23 4:15 ` Dmitry Torokhov
2018-01-23 10:44 ` Mylene Josserand
[not found] ` <20180123114459.07cc3404-K8i4uRanGBt8XcdJbWeDu7NAH6kLmebB@public.gmane.org>
2018-01-23 10:50 ` landyn.lawrence-jHc5sQ61u836K7/ahGyk6A
2018-01-23 17:30 ` Dmitry Torokhov
2017-12-01 15:39 ` [PATCH v4 2/2] dt-bindings: input: Add documentation for cyttsp5 Mylène Josserand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180122201109.GA651@gmail.com \
--to=marcus.folkesson-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
--cc=linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
--cc=maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
--cc=mylene.josserand-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).