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 X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C75DC4320E for ; Sun, 29 Aug 2021 22:13:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D811A60F56 for ; Sun, 29 Aug 2021 22:13:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236083AbhH2WOi (ORCPT ); Sun, 29 Aug 2021 18:14:38 -0400 Received: from dnyon.com ([82.223.165.189]:56182 "EHLO dnyon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235370AbhH2WOg (ORCPT ); Sun, 29 Aug 2021 18:14:36 -0400 X-Greylist: delayed 23025 seconds by postgrey-1.27 at vger.kernel.org; Sun, 29 Aug 2021 18:14:36 EDT Received: from dnyon.com (45.74.222.87.dynamic.jazztel.es [87.222.74.45]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dnyon.com (Postfix) with ESMTPSA id B9FFC40442; Sun, 29 Aug 2021 22:13:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=dnyon.com; s=mail; t=1630275223; bh=aw1SvFqyWFMuJYWbD3SZOO+rClzP6Lw/Acya+RIY0Z4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=0Rm6SJJkrMKPuF+qpFkge2kUEP2OnceG6Oj82j5mVk5rpJ5ifwekPM3xIcBdpVKax guOds/fzBIIN3+sEkB4s1Fp9HtUlr1/h4Uv08V2rinlaUaTJc8lL29/5Ha4F+dYDgx B4E7w15wun17uk/RXfxooBui4QdPCc60SOfzBdHIo1KCXqPLLFMbpQ+2y7hAMTWFLB iOTzEORaqZO9+ueEg+7vyGzdApzmR6Bk6ybW2A3J1ZTkk3eGxfslMVDbb0kY3y4g9w yOlJCrN23bjtoj0b8zE40rkjOmMtmAq5HPN7d7TbuLOyS3D+aQvDURkklfNeMu5IZu +xhjR9EBwcRbA== Date: Mon, 30 Aug 2021 00:13:41 +0200 From: Alejandro Tafalla To: Andy Shevchenko Cc: Liam Girdwood , Mark Brown , "alsa-devel@alsa-project.org" , Rob Herring , Jaroslav Kysela , Takashi Iwai , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c Message-ID: References: <20210829170019.384632-1-atafalla@dnyon.com> <20210829170019.384632-2-atafalla@dnyon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=mensaje In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello Andy, On Sun, Aug 29, 2021 at 11:22:35PM +0300, Andy Shevchenko wrote: > > + max98927->reset_gpio > > + = devm_gpiod_get_optional(&i2c->dev, "reset", > > GPIOD_OUT_HIGH); > > + if (IS_ERR(max98927->reset_gpio)) { > > + ret = PTR_ERR(max98927->reset_gpio); > > + dev_err(&i2c->dev, > > + "Failed to request GPIO reset pin, error %d\n", > > ret); > > + return ret; > > > > Spamming logs is not good. Use > > return dev_err_probe(...); Okay. > > + } > > + > > + if (max98927->reset_gpio) { > > + gpiod_set_value_cansleep(max98927->reset_gpio, 0); > > > > You may request the pin in a proper state, also with current code you seems > mishandle the conception of the logical pin level vs. physical one. Right, i made the mistake of basing off an old driver that use legacy functions. > > diff --git a/sound/soc/codecs/max98927.h b/sound/soc/codecs/max98927.h > > index 05f495db914d..5c04bf38e24a 100644 > > --- a/sound/soc/codecs/max98927.h > > +++ b/sound/soc/codecs/max98927.h > > @@ -255,6 +255,7 @@ struct max98927_priv { > > struct regmap *regmap; > > struct snd_soc_component *component; > > struct max98927_pdata *pdata; > > > > > + struct gpio_desc *reset_gpio; > > > Why? Are you using it outside of ->probe()? No, I'll delete it and use a local variable. > With Best Regards, > Andy Shevchenko Thank you for the feedback, I'll address all the issues in a V2. Alejandro Tafalla 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 X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DF4BFC432BE for ; Sun, 29 Aug 2021 22:14:46 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7593D60F46 for ; Sun, 29 Aug 2021 22:14:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 7593D60F46 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=dnyon.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 3A41716B5; Mon, 30 Aug 2021 00:13:53 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 3A41716B5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1630275283; bh=aw1SvFqyWFMuJYWbD3SZOO+rClzP6Lw/Acya+RIY0Z4=; h=Date:From:To:Subject:References:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=tiFRT56ZicuPAHNP/4tExWihHw84fzfiUq0AecY9pDbwEYH2XEeDSLISIdM41pxVa r30sFNzykga9PP7vDuqUk53mkKEAzQWe7QsfU+c/mpEu+ZKpO4/JGFFmF88SDSz7I2 wJeHLD7PFPVy2B8UOeHaqB8GoQY4rgTlV2rILs4E= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id CB1ACF80254; Mon, 30 Aug 2021 00:13:52 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id CDD9EF8025B; Mon, 30 Aug 2021 00:13:51 +0200 (CEST) Received: from dnyon.com (dnyon.com [82.223.165.189]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id 6EC17F80246 for ; Mon, 30 Aug 2021 00:13:48 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz 6EC17F80246 Authentication-Results: alsa1.perex.cz; dkim=pass (2048-bit key) header.d=dnyon.com header.i=@dnyon.com header.b="0Rm6SJJk" Received: from dnyon.com (45.74.222.87.dynamic.jazztel.es [87.222.74.45]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dnyon.com (Postfix) with ESMTPSA id B9FFC40442; Sun, 29 Aug 2021 22:13:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=dnyon.com; s=mail; t=1630275223; bh=aw1SvFqyWFMuJYWbD3SZOO+rClzP6Lw/Acya+RIY0Z4=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=0Rm6SJJkrMKPuF+qpFkge2kUEP2OnceG6Oj82j5mVk5rpJ5ifwekPM3xIcBdpVKax guOds/fzBIIN3+sEkB4s1Fp9HtUlr1/h4Uv08V2rinlaUaTJc8lL29/5Ha4F+dYDgx B4E7w15wun17uk/RXfxooBui4QdPCc60SOfzBdHIo1KCXqPLLFMbpQ+2y7hAMTWFLB iOTzEORaqZO9+ueEg+7vyGzdApzmR6Bk6ybW2A3J1ZTkk3eGxfslMVDbb0kY3y4g9w yOlJCrN23bjtoj0b8zE40rkjOmMtmAq5HPN7d7TbuLOyS3D+aQvDURkklfNeMu5IZu +xhjR9EBwcRbA== Date: Mon, 30 Aug 2021 00:13:41 +0200 From: Alejandro Tafalla To: Andy Shevchenko Subject: Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c Message-ID: References: <20210829170019.384632-1-atafalla@dnyon.com> <20210829170019.384632-2-atafalla@dnyon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=mensaje In-Reply-To: Cc: "devicetree@vger.kernel.org" , "alsa-devel@alsa-project.org" , "linux-kernel@vger.kernel.org" , Takashi Iwai , Rob Herring , Liam Girdwood , Mark Brown X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" Hello Andy, On Sun, Aug 29, 2021 at 11:22:35PM +0300, Andy Shevchenko wrote: > > + max98927->reset_gpio > > + = devm_gpiod_get_optional(&i2c->dev, "reset", > > GPIOD_OUT_HIGH); > > + if (IS_ERR(max98927->reset_gpio)) { > > + ret = PTR_ERR(max98927->reset_gpio); > > + dev_err(&i2c->dev, > > + "Failed to request GPIO reset pin, error %d\n", > > ret); > > + return ret; > > > > Spamming logs is not good. Use > > return dev_err_probe(...); Okay. > > + } > > + > > + if (max98927->reset_gpio) { > > + gpiod_set_value_cansleep(max98927->reset_gpio, 0); > > > > You may request the pin in a proper state, also with current code you seems > mishandle the conception of the logical pin level vs. physical one. Right, i made the mistake of basing off an old driver that use legacy functions. > > diff --git a/sound/soc/codecs/max98927.h b/sound/soc/codecs/max98927.h > > index 05f495db914d..5c04bf38e24a 100644 > > --- a/sound/soc/codecs/max98927.h > > +++ b/sound/soc/codecs/max98927.h > > @@ -255,6 +255,7 @@ struct max98927_priv { > > struct regmap *regmap; > > struct snd_soc_component *component; > > struct max98927_pdata *pdata; > > > > > + struct gpio_desc *reset_gpio; > > > Why? Are you using it outside of ->probe()? No, I'll delete it and use a local variable. > With Best Regards, > Andy Shevchenko Thank you for the feedback, I'll address all the issues in a V2. Alejandro Tafalla