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 8804ACCA480 for ; Thu, 30 Jun 2022 14:53:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235788AbiF3Oxx convert rfc822-to-8bit (ORCPT ); Thu, 30 Jun 2022 10:53:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60176 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235777AbiF3Oxw (ORCPT ); Thu, 30 Jun 2022 10:53:52 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 466ED1B7A4 for ; Thu, 30 Jun 2022 07:53:51 -0700 (PDT) Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1o6vXc-0002Re-Vr; Thu, 30 Jun 2022 16:53:13 +0200 Received: from [2a0a:edc0:0:900:1d::4e] (helo=lupine) by drehscheibe.grey.stw.pengutronix.de with esmtp (Exim 4.94.2) (envelope-from ) id 1o6vXO-003bqn-HG; Thu, 30 Jun 2022 16:53:02 +0200 Received: from pza by lupine with local (Exim 4.94.2) (envelope-from ) id 1o6vXR-000AqK-Co; Thu, 30 Jun 2022 16:53:01 +0200 Message-ID: Subject: Re: [PATCH v6 08/17] reset: npcm: using syscon instead of device data From: Philipp Zabel To: Tomer Maimon Cc: Avi Fishman , Tali Perry , Joel Stanley , Patrick Venture , Nancy Yuen , Benjamin Fair , Rob Herring , Krzysztof Kozlowski , Michael Turquette , Stephen Boyd , Greg KH , Daniel Lezcano , Thomas Gleixner , Wim Van Sebroeck , Guenter Roeck , Catalin Marinas , Will Deacon , Arnd Bergmann , Olof Johansson , Jiri Slaby , Shawn Guo , Bjorn Andersson , Geert Uytterhoeven , Marcel Ziswiler , Vinod Koul , Biju Das , Nobuhiro Iwamatsu , Robert Hancock , Jonathan =?ISO-8859-1?Q?Neusch=E4fer?= , Lubomir Rintel , devicetree , Linux Kernel Mailing List , linux-clk , "open list:SERIAL DRIVERS" , LINUXWATCHDOG , Linux ARM Date: Thu, 30 Jun 2022 16:53:01 +0200 In-Reply-To: References: <20220630103606.83261-1-tmaimon77@gmail.com> <20220630103606.83261-9-tmaimon77@gmail.com> <63f8d70ad9c657890669e9c32775632af4e36995.camel@pengutronix.de> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT User-Agent: Evolution 3.38.3-1 MIME-Version: 1.0 X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 X-SA-Exim-Mail-From: p.zabel@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-serial@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-serial@vger.kernel.org On Do, 2022-06-30 at 14:20 +0300, Tomer Maimon wrote: > Hi Philipp, > > Thanks for your comment. > > On Thu, 30 Jun 2022 at 13:59, Philipp Zabel wrote: > > > > Hi Tomer, > > > > On Do, 2022-06-30 at 13:35 +0300, Tomer Maimon wrote: > > Using syscon device tree property instead of device data to handle the > > NPCM general control registers. > > > > In case the syscon not found the code still search for nuvoton,npcm750-gcr > > to support DTS backward compatibility. > > > > Signed-off-by: Tomer Maimon > > --- > >  drivers/reset/reset-npcm.c | 17 ++++++++--------- > >  1 file changed, 8 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/reset/reset-npcm.c b/drivers/reset/reset-npcm.c > > index 2ea4d3136e15..431ff2b602c5 100644 > > --- a/drivers/reset/reset-npcm.c > > +++ b/drivers/reset/reset-npcm.c > > @@ -138,8 +138,7 @@ static int npcm_reset_xlate(struct reset_controller_dev *rcdev, > >  } > > > > > >  static const struct of_device_id npcm_rc_match[] = { > > - { .compatible = "nuvoton,npcm750-reset", > > - .data = (void *)"nuvoton,npcm750-gcr" }, > > + { .compatible = "nuvoton,npcm750-reset"}, > > > > Add a space. ^^ > Will modify in V7 > > > >         { } > >  }; > > > > > > @@ -155,15 +154,15 @@ static int npcm_usb_reset(struct platform_device *pdev, struct npcm_rc_data *rc) > >         u32 ipsrst1_bits = 0; > >         u32 ipsrst2_bits = NPCM_IPSRST2_USB_HOST; > >         u32 ipsrst3_bits = 0; > > - const char *gcr_dt; > > > > > > - gcr_dt = (const char *) > > - of_match_device(dev->driver->of_match_table, dev)->data; > > - > > - gcr_regmap = syscon_regmap_lookup_by_compatible(gcr_dt); > > + gcr_regmap = syscon_regmap_lookup_by_phandle(dev->of_node, "nuvoton,sysgcr"); > >         if (IS_ERR(gcr_regmap)) { > > - dev_err(&pdev->dev, "Failed to find %s\n", gcr_dt); > > - return PTR_ERR(gcr_regmap); > > + dev_warn(&pdev->dev, "Failed to find nuvoton,sysgcr search for nuvoton,npcm750-gcr for Poleg backward compatibility"); > > > > Is this warning useful to the user? Maybe add suggestion like "please > > update the device tree". Also there is no further message if > > nuvoton,npcm750-gcr is found and all is well. > > O.K. > I think about two options: > > 1. Modify the message "Failed to find nuvoton,sysgcr property, please > update the device tree\n Search for nuvoton,npcm750-gcr for Poleg > backward compatibility" I would replace "Search for" with "Using" The second line probably should be dev_info() level. > OR > > 2. >         if (IS_ERR(rc->gcr_regmap)) { >                 dev_warn(&pdev->dev, "Failed to find nuvoton,sysgcr > please update the device tree"); >                 rc->gcr_regmap = > syscon_regmap_lookup_by_compatible("nuvoton,npcm750-gcr"); >                 if (IS_ERR(rc->gcr_regmap)) { >                         dev_err(&pdev->dev, "Failed to find > nuvoton,npcm750-gcr"); >                         return PTR_ERR(rc->gcr_regmap); >                 } >                  dev_info(&pdev->dev, "found nuvoton,npcm750-gcr for > Poleg backward compatibility"); >         } > > The only problem that I have with option 2 is if our customers will > use the latest reset driver and they will not update their device tree > they will see all the time the dev_info message. > > What do you think? I'm fine with either. With the "please update DT" prompt it's clear how to get rid of the warning. regards Philipp