linux-kernel - Re: [PATCH 2/5] gpio: Add support for Intel LJCA USB GPIO 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: <Y/jy/iElbD8suxGY@ye-NUC7i7DNHE>
Date:   Sat, 25 Feb 2023 01:25:18 +0800
From:   "Ye, Xiang" <xiang.ye@...el.com>
To:     Linus Walleij <linus.walleij@...aro.org>
CC:     Lee Jones <lee@...nel.org>, Wolfram Sang <wsa@...nel.org>,
        Tyrone Ting <kfting@...oton.com>,
        Mark Brown <broonie@...nel.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        <linux-i2c@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-spi@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
        <srinivas.pandruvada@...el.com>, <heikki.krogerus@...ux.intel.com>,
        <andriy.shevchenko@...ux.intel.com>,
        <sakari.ailus@...ux.intel.com>, <zhifeng.wang@...el.com>,
        <wentong.wu@...el.com>, <lixu.zhang@...el.com>
Subject: Re: [PATCH 2/5] gpio: Add support for Intel LJCA USB GPIO driver

Hi Walleij,

Thanks for the review.

On Fri, Feb 24, 2023 at 11:48:08AM +0100, Linus Walleij wrote:
> Hi Ye,
> 
> thanks for your patch!
> 
> I think your colleague Andy Shevchenko will provide the most detailed
> and deep feedback, but here are some things I spotted immediately:
> 
> On Sun, Feb 19, 2023 at 7:31 PM Ye Xiang <xiang.ye@...el.com> wrote:
> 
> > This patch implements the GPIO function of Intel USB-I2C/GPIO/SPI adapter
> > device named "La Jolla Cove Adapter" (LJCA). It communicate with LJCA
> > GPIO module with specific protocol through interfaces exported by LJCA USB
> > driver.
> >
> > Signed-off-by: Ye Xiang <xiang.ye@...el.com>
> (...)
> 
> > +config GPIO_LJCA
> > +       tristate "INTEL La Jolla Cove Adapter GPIO support"
> > +       depends on MFD_LJCA
> 
> I would add
> 
>    default MFD_LJCA
> 
> so if you activate the MFD you get this subdriver by default
> as module or built-in depending on what the MFD is built
> as.
> 
> (Same goes for the other subdrivers I guess)
Agree, I will add "default MFD_LJCA" on all the LJCA subdrivers.
> 
> In addition you need:
> 
> select GPIOLIB_IRQCHIP
> 
> since you use this facility.
> 
> > +static struct irq_chip ljca_gpio_irqchip = {
> 
> static const ...
Yes, I will add "select GPIOLIB_IRQCHIP" and use "static const" here.
> 
> > +       .name = "ljca-irq",
> > +       .irq_mask = ljca_irq_mask,
> > +       .irq_unmask = ljca_irq_unmask,
> > +       .irq_set_type = ljca_irq_set_type,
> > +       .irq_bus_lock = ljca_irq_bus_lock,
> > +       .irq_bus_sync_unlock = ljca_irq_bus_unlock,
> > +       .flags = IRQCHIP_IMMUTABLE,
> > +       GPIOCHIP_IRQ_RESOURCE_HELPERS,
> > +};
> 
> Yours,
> Linus Walleij

Thanks
Ye Xiang