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 6C7D9C76196 for ; Fri, 7 Apr 2023 18:36:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229822AbjDGSgv (ORCPT ); Fri, 7 Apr 2023 14:36:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51558 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229588AbjDGSgu (ORCPT ); Fri, 7 Apr 2023 14:36:50 -0400 Received: from fgw21-7.mail.saunalahti.fi (fgw21-7.mail.saunalahti.fi [62.142.5.82]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B9A4010DA for ; Fri, 7 Apr 2023 11:36:49 -0700 (PDT) Received: from localhost (88-113-24-128.elisa-laajakaista.fi [88.113.24.128]) by fgw21.mail.saunalahti.fi (Halon) with ESMTP id 260eab40-d573-11ed-abf4-005056bdd08f; Fri, 07 Apr 2023 21:36:46 +0300 (EEST) From: andy.shevchenko@gmail.com Date: Fri, 7 Apr 2023 21:36:45 +0300 To: Linus Walleij Cc: Michael Walle , "Sahin, Okan" , Bartosz Golaszewski , Rob Herring , Krzysztof Kozlowski , linux-gpio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 2/2] gpio: ds4520: Add ADI DS4520 Regulator Support Message-ID: References: <20230327130010.8342-1-okan.sahin@analog.com> <20230327130010.8342-3-okan.sahin@analog.com> <25e1fda4b6df2d10444d7eca3cd0e387@walle.cc> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org Fri, Apr 07, 2023 at 03:48:25PM +0200, Linus Walleij kirjoitti: > On Wed, Apr 5, 2023 at 3:57 PM Michael Walle wrote: > > > OTOH I'm not sure the driver is doing it correctly, because it also > > seems to switch the pullup resisters together with the direction. > > I'm not sure that is correct. So there might be just one register > > involved after all and the GPIO_REGMAP should work again. > > I'm pretty sure that should be in the .set_config() callback. > > > Also, according to the datasheet this has some nv memory (to set the > > initial state of the GPIOs [?]). So it should really be a multi-function > > device. I'm not sure if this has to be considered right from the > > beginning or if the device support can start with GPIO only and later > > be transitioned to a full featured MFD (probably with nvmem support). > > That's a bit of a soft definition. > > If the chip is *only* doing GPIO and nvram it can be a GPIO-only > device I think. > > The precedent is a ton of ethernet drivers with nvram for storing > e.g. the MAC address. We don't make all of those into MFDs, > as the nvram is closely tied to the one and only function of the > block. I agree with Linus. This should be part of the actual (main) driver for the chip as many do (like USB to serial adapters that have GPIO capability). Also this code lacks of proper locking and has style issues. -- With Best Regards, Andy Shevchenko