linux-kernel - Re: [PATCH v6 2/2] iio: proximity: Add driver support for vcnl3020 proximity sensor

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 1 Apr 2020 19:35:19 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Ivan Mikhaylov <i.mikhaylov@...ro.com>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        linux-iio <linux-iio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Mark Rutland <mark.rutland@....com>,
        Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH v6 2/2] iio: proximity: Add driver support for vcnl3020
 proximity sensor

On Wed, Apr 1, 2020 at 7:24 PM Ivan Mikhaylov <i.mikhaylov@...ro.com> wrote:
>
> Proximity sensor driver based on light/vcnl4000.c code.
> For now supports only the single on-demand measurement.
>
> The VCNL3020 is a fully integrated proximity sensor. Fully
> integrated means that the infrared emitter is included in the
> package. It has 16-bit resolution. It includes a signal
> processing IC and features standard I2C communication
> interface. It features an interrupt function.

Thank you for an update, my comments below.

...

> +static int get_and_apply_property(struct vcnl3020_data *data, const char *prop,
> +                                 u32 reg)
> +{
> +       int rc;
> +       u32 val;
> +
> +       rc = device_property_read_u32(data->dev, prop, &val);
> +       if (rc)
> +               return 0;
> +
> +       rc = regmap_write(data->regmap, reg, val);
> +       if (rc)

> +               dev_err(data->dev, "Error (%d) setting property (%s)",
> +                       rc, prop);

This requires {} according to the coding style.

> +       return rc;
> +}

...

> +static int vcnl3020_probe(struct i2c_client *client)
> +{

> +       indio_dev->name = VCNL_DRV_NAME;

It's definitely not a driver name. You have to put part number here.

> +}

...

> +static struct i2c_driver vcnl3020_driver = {
> +       .driver = {

> +               .name   = VCNL_DRV_NAME,

Better to spell it explicitly.

> +               .of_match_table = vcnl3020_of_match,
> +       },
> +       .probe_new  = vcnl3020_probe,
> +};

-- 
With Best Regards,
Andy Shevchenko