Date:   Sat, 25 Feb 2023 01:25:18 +0800
From:   "Ye, Xiang" <xiang.ye@intel.com>
To:     Linus Walleij <linus.walleij@linaro.org>
CC:     Lee Jones <lee@kernel.org>, Wolfram Sang <wsa@kernel.org>,
        Tyrone Ting <kfting@nuvoton.com>,
        Mark Brown <broonie@kernel.org>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        <linux-i2c@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-spi@vger.kernel.org>, <linux-gpio@vger.kernel.org>,
        <srinivas.pandruvada@intel.com>, <heikki.krogerus@linux.intel.com>,
        <andriy.shevchenko@linux.intel.com>,
        <sakari.ailus@linux.intel.com>, <zhifeng.wang@intel.com>,
        <wentong.wu@intel.com>, <lixu.zhang@intel.com>
Subject: Re: [PATCH 2/5] gpio: Add support for Intel LJCA USB GPIO driver
Message-ID: <Y/jy/iElbD8suxGY@ye-NUC7i7DNHE>
References: <20230219183059.1029525-1-xiang.ye@intel.com>
 <20230219183059.1029525-3-xiang.ye@intel.com>
 <CACRpkdYdDE+B=SF+kAuQBiwUi31y_xELLXuDK+9G+rqD9MQBGg@mail.gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <CACRpkdYdDE+B=SF+kAuQBiwUi31y_xELLXuDK+9G+rqD9MQBGg@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Mf31/s/Zw7YYRY6OTvw5Tb2FqkGxvBURSdsxRVpDVnnCw1ky8b5u6nPZqJiL?=
 =?us-ascii?Q?Iiy/2cLC8XR+YouioHNKjojQFyYSWhkvHLAGraLR0r25ZLoVDoHcSbrsvKZV?=
 =?us-ascii?Q?KsmuGTBYwokgvWx3CTTIMxW3SSU8QEfRAvsm6wTn1VrWDUcyBVuiqgK60vfi?=
 =?us-ascii?Q?dwKKAw2dM9rAMEjBROXq+NAZHLjmxPw87UZku9P3oJDdDChUsjSKDRXeBwE2?=
 =?us-ascii?Q?Soraz7pYn8Sl34Zg8PjhBA+wErFkcee7JfHHAofWlMgCK3Al0DXqgXPFZKxH?=
 =?us-ascii?Q?iAOuyQoyl3pX28r++hgaz9EtT5BPZsiTSUzwedWAxjiUm+TYODCLdbAPFw/B?=
 =?us-ascii?Q?kR/cVsGRkeRjUe1APuTO5upncqmK3//GO3NIrj+Wda+7KHOW8LwQLsL56RST?=
 =?us-ascii?Q?KBykJsIawxsA4uTg+ljoMcYqKdXA10g6QhSDz1Y1r10R4S9ecEqIoIK1ygy7?=
 =?us-ascii?Q?bMevOSL+3vcdgbEaDDmhCZ6BK7uEyve5lgEjgmfWIfHuv32ndhRmAJlkjoyZ?=
 =?us-ascii?Q?UUNJQQPorUFAhJSE2xnFbpuSgQRrD2e8KEFhQfGuGWJRA6K9OVvi87POQJGk?=
 =?us-ascii?Q?30Yva94tRnkcA/IT4WU27WCcBQZ7wuycbysPaBETM1ww678Q0k0rlNwSkJtk?=
 =?us-ascii?Q?c6GJkAjoiJHqp8mWTVt3UiZQtHDg3h9a98PQXXn/szdtaF8T6tZrSyWCaFaq?=
 =?us-ascii?Q?GBZtRW0rE269IedRM3afwLudFXcpbT37qeTW/N3fr7sXkQFl1ujr9KPoTA8R?=
 =?us-ascii?Q?QigMci6ICi/n8DYLTAjTawq56kNopBU3m5VvJkcwlgouHaYBBDiaTd5tojY7?=
 =?us-ascii?Q?RulG10Ul6aWzu63TWL6x5EzhCJf7jgGbQMqCCGOqYN7dd9GrYNaoJ0uEzraR?=
 =?us-ascii?Q?0S3+OuAKXN0+hZVHYu69JziNKU0rVg06wz08tYUvUk/bboJUP9h9KltJoXdA?=
 =?us-ascii?Q?dO3NVwr4d74wtDZLK4Wxyw0hUOuhVK+kRumLef9ggvR469sotgMDyrJajuLW?=
 =?us-ascii?Q?yvm041HVRGdoEQ3onIRc7pXxJDK42xEp7DFk0gAhOCZoxrC8j6+WIeu98U8u?=
 =?us-ascii?Q?Tok3wZXKnNHwuGGzI4VgjjpzEyqbfIf1KLdRT1tMIbFprPSMalalzEcqi0hA?=
 =?us-ascii?Q?AhDNQtGrdJwaw+l3xoDRRxqCS5iELQjMngjW5mOShOrDQ6OgkTB94/FXndmS?=
 =?us-ascii?Q?uy9X51maZRoHKPu3T+UFCEQ40xoUbBp1FcV3f4jImbTD7A5ShXFmcQ51c5VY?=
 =?us-ascii?Q?hmnroArY1exvACfCItoSSfqSM+PSfbSjYK2vISpBeqOXHJhSWj22hWfC/tgC?=
 =?us-ascii?Q?Sk90jbEIjLOTE708WgexFsDHW8MAiiCGTxzcFj+pvlKCKJAbS++mSMHhVZ0i?=
 =?us-ascii?Q?e2vMKoh5oVF25ZyBJx/ZuhOVGai3EenHCG0aA30s4s/QgQOebmYYBFRJ1gnx?=
 =?us-ascii?Q?uebWWYJS1kvqDq/MAiUKgPkHFDiBnNZPp9ANx3ufO5cF9S9rDeWL9pLk9h/W?=
 =?us-ascii?Q?49OB0r+MgeKcJTstOTjGmwuMkpjeJj3ApXySwirC8IZu9VwWqHlpGytXFnlz?=
 =?us-ascii?Q?b3d6J/W3dWecwV6tGIJy9+ZsWdX8qgyg63XVQjyI?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2d90851b-c941-421e-88f7-08db168c1f70
X-MS-Exchange-CrossTenant-AuthSource: DM5PR11MB1418.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Feb 2023 17:25:28.4258
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9A8S6rbJhxsvM95oDYIy4/MHKtdunyqqP3iKnFoQBcSoniG+j8VjRd3FKj7FkM6QNg/YHAVek+c5Hf0GKWUcuQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR11MB6088
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@intel.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@intel.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