linux-kernel - Re: [PATCH v7 2/3] Input: Add Novatek NT36xxx touchscreen driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHp75VeRagDvpZBFZ4+T8JgsjOKxuZjxRHw_azXKQx5O3Cx5yw@mail.gmail.com>
Date:   Mon, 26 Oct 2020 20:24:39 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     kholk11@...il.com
Cc:     Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Henrik Rydberg <rydberg@...math.org>, priv.luk@...il.com,
        linux-input <linux-input@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        marijns95@...il.com, Konrad Dybcio <konradybcio@...il.com>,
        Martin Botka <martin.botka1@...il.com>,
        phone-devel@...r.kernel.org,
        devicetree <devicetree@...r.kernel.org>,
        Krzysztof Kozlowski <krzk@...nel.org>
Subject: Re: [PATCH v7 2/3] Input: Add Novatek NT36xxx touchscreen driver

On Sat, Oct 24, 2020 at 1:51 PM <kholk11@...il.com> wrote:
>
> From: AngeloGioacchino Del Regno <kholk11@...il.com>
>
> This is a driver for the Novatek in-cell touch controller and
> supports various chips from the NT36xxx family, currently
> including NT36525, NT36672A, NT36676F, NT36772 and NT36870.
>
> Functionality like wake gestures and firmware flashing is not
> included: I am not aware of any of these DrIC+Touch combo
> chips not including a non-volatile memory and it should be
> highly unlikely to find one, since the touch firmware is
> embedded into the DriverIC one, which is obviously necessary
> to drive the display unit.
>
> However, the necessary address for the firmware update
> procedure was included into the address table in this driver
> so, in the event that someone finds the need to implement it
> for a reason or another, it will be pretty straightforward to.
>
> This driver is lightly based on the downstream implementation [1].
> [1] https://github.com/Rasenkai/caf-tsoft-Novatek-nt36xxx

(From drafts, didn't have time to finish and see already v8, so please revisit)

...

> +#include <asm/unaligned.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/interrupt.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/regulator/consumer.h>

Perhaps sorted? Also, asm/* usually go after linux/*.

...

> +#define TOUCH_MAX_PRESSURE      1000

Any units?

...

> +       ts = devm_kzalloc(&hw_client->dev, sizeof(struct nt36xxx_i2c), GFP_KERNEL);

sizeof(*ts) ?

> +       if (!ts)
> +               return -ENOMEM;
> +
> +       ts->supplies = devm_kcalloc(&hw_client->dev,
> +                                   NT36XXX_NUM_SUPPLIES,

> +                                   sizeof(struct regulator_bulk_data),

sizeof(*ts->supplies) ?

> +                                   GFP_KERNEL);
> +       if (!ts->supplies)
> +               return -ENOMEM;

...

> +       ret = devm_regulator_bulk_get(&hw_client->dev,
> +                                     NT36XXX_NUM_SUPPLIES,
> +                                     ts->supplies);

> +       if (ret != 0) {

if (ret)

> +               if (ret != -EPROBE_DEFER)
> +                       dev_err(&hw_client->dev,
> +                               "Cannot get supplies: %d\n", ret);
> +               return ret;
> +       }

...

> +       input->phys = devm_kasprintf(&hw_client->dev, GFP_KERNEL,
> +                                    "%s/input0", dev_name(&hw_client->dev));

NULL check?

...

> +       ret = devm_request_threaded_irq(&hw_client->dev, hw_client->irq, NULL,
> +                                       nt36xxx_i2c_irq_handler, IRQF_ONESHOT,
> +                                       hw_client->name, ts);
> +       if (ret) {
> +               dev_err(&hw_client->dev, "request irq failed: %d\n", ret);

> +               return ret;
> +       }
> +
> +       return 0;

return ret; ?

> +}


--
With Best Regards,
Andy Shevchenko