linux-kernel - Re: [PATCH] drivers: ddcci: upstream DDCCI 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a3TWx71xT83003LSFsu1eF1p75hCxvpCB_FZf1SrNjQJw@mail.gmail.com>
Date:   Sat, 26 Feb 2022 10:38:48 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Yusuf Khan <yusisamerican@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jason Wang <jasowang@...hat.com>,
        Michael Kelley <mikelley@...rosoft.com>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        gregkh <gregkh@...uxfoundation.org>, javier@...igon.com,
        Arnd Bergmann <arnd@...db.de>, Will Deacon <will@...nel.org>,
        Jens Axboe <axboe@...nel.dk>,
        Bjorn Andersson <bjorn.andersson@...aro.org>
Subject: Re: [PATCH] drivers: ddcci: upstream DDCCI driver

with the other ones.On Sat, Feb 26, 2022 at 3:20 AM Yusuf Khan
<yusisamerican@...il.com> wrote:
>
> This patch upstreams the DDCCI driver by Christoph Grenz into
> the kernel. The original gitlab page is loacted at https://gitlab
> .com/ddcci-driver-linux/ddcci-driver-linux/-/tree/master.
>
> Signed-off-by: Yusuf Khan <yusisamerican@...il.com>
> ---
>  drivers/Kconfig                 |    2 +
>  drivers/Makefile                |    1 +
>  drivers/ddcci/Kconfig           |    3 +
>  drivers/ddcci/Makefile          |    3 +
>  drivers/ddcci/ddcci-backlight.c |  413 +++++++
>  drivers/ddcci/ddcci.c           | 1895 +++++++++++++++++++++++++++++++
>  include/linux/ddcci.h           |  164 +++

If this is a backlight driver, I think it should go into
drivers/video/backlight/,
no need for a top-level subsystem.

> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +#include <asm-generic/fcntl.h>

Including the asm-generic version causes the build failures. If you need
the contents, use <linux/fcntl.h>, otherwise leave it out.

> +static dev_t ddcci_cdev_first;
> +static dev_t ddcci_cdev_next;
> +static dev_t ddcci_cdev_end;
> +static DEFINE_SEMAPHORE(core_lock);

No new semaphores please, this should probably be a mutex.


> +struct bus_type ddcci_bus_type;
> +EXPORT_SYMBOL_GPL(ddcci_bus_type);
> +
> +/* Assert neccessary string array sizes  */
> +#ifndef sizeof_field
> +# define sizeof_field(t,m) FIELD_SIZEOF(t,m)
> +#endif
> +static_assert(sizeof_field(struct ddcci_device, prot) > 8);
> +static_assert(sizeof_field(struct ddcci_device, type) > 8);
> +static_assert(sizeof_field(struct ddcci_device, model) > 8);
> +static_assert(sizeof_field(struct ddcci_device, vendor) > 8);
> +static_assert(sizeof_field(struct ddcci_device, module) > 8);
> +
> +/* Internal per-i2c-client driver data */
> +struct ddcci_bus_drv_data {
> +       unsigned long quirks;
> +       struct i2c_client *i2c_dev;
> +       struct semaphore sem;
> +       unsigned char recv_buffer[DDCCI_RECV_BUFFER_SIZE];
> +};

Same here.

> +static const struct file_operations ddcci_fops = {
> +       .owner = THIS_MODULE,
> +       .read = ddcci_cdev_read,
> +       .write = ddcci_cdev_write,
> +       .open = ddcci_cdev_open,
> +       .release = ddcci_cdev_close,
> +       .llseek = ddcci_cdev_seek
> +};

It looks like this adds low-level access to a bus that is already managed by
the drm (or older framebuffer) drivers. How do you prevent these two
from stepping on each other's toes?

        Arnd