From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A82B4C77B73 for ; Tue, 2 May 2023 15:55:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234617AbjEBPzJ (ORCPT ); Tue, 2 May 2023 11:55:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33642 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233724AbjEBPzI (ORCPT ); Tue, 2 May 2023 11:55:08 -0400 Received: from fgw23-7.mail.saunalahti.fi (fgw23-7.mail.saunalahti.fi [62.142.5.84]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 930441B6 for ; Tue, 2 May 2023 08:55:07 -0700 (PDT) Received: from localhost (88-113-26-95.elisa-laajakaista.fi [88.113.26.95]) by fgw23.mail.saunalahti.fi (Halon) with ESMTP id b418ae39-e901-11ed-b972-005056bdfda7; Tue, 02 May 2023 18:55:04 +0300 (EEST) From: andy.shevchenko@gmail.com Date: Tue, 2 May 2023 18:55:04 +0300 To: Okan Sahin Cc: Linus Walleij , Bartosz Golaszewski , Rob Herring , Krzysztof Kozlowski , linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/2] gpio: ds4520: Add ADI DS4520 GPIO Expander Support Message-ID: References: <20230501230517.4491-1-okan.sahin@analog.com> <20230501230517.4491-3-okan.sahin@analog.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230501230517.4491-3-okan.sahin@analog.com> Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org Tue, May 02, 2023 at 02:05:16AM +0300, Okan Sahin kirjoitti: > The DS4520 is a 9-bit nonvolatile (NV) I/O expander. > It offers users a digitally programmable alternative > to hardware jumpers and mechanical switches that are > being used to control digital logic node. ... > +#include > +#include > +#include > +#include Missing property.h. > +#include ... > +#define NUMBER_OF_GPIO 9 > + > +#define PULLUP0 0xF0 > +#define IO_CONTROL0 0xF2 > +#define IO_STATUS0 0xF8 No namespace for the above? ... > + struct gpio_regmap_config config = {0}; 0 is not needed. > + ngpio = NUMBER_OF_GPIO; Do you really need this? Can Device Tree be sufficient here? (We have a GPIO-wide property for that). ... > + ret = device_property_read_u32(dev, "reg", &base); > + if (ret) > + return -EINVAL; Why shadowing error? ... > + regmap = devm_regmap_init_i2c(client, &ds4520_regmap_config); > + if (IS_ERR(regmap)) { > + ret = PTR_ERR(regmap); > + dev_err_probe(dev, ret, > + "Failed to allocate register map\n"); > + return ret; return dev_err_probe(); > + } ... > + config.ngpio = ngpio; Why do you use temporary variable ngpio and not assign directly here? -- With Best Regards, Andy Shevchenko