Message ID | 20230420101617.142225-2-krzysztof.kozlowski@linaro.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp224451vqo; Thu, 20 Apr 2023 03:25:00 -0700 (PDT) X-Google-Smtp-Source: AKy350aSQ0gKgWltK4RY9hEBm06q7q7eG9cwrttIwwpYLtcw4Vt9IK1e03gJTOEH9mxsmBG6dm13 X-Received: by 2002:a17:903:1d1:b0:1a6:3c64:513c with SMTP id e17-20020a17090301d100b001a63c64513cmr1466938plh.37.1681986300579; Thu, 20 Apr 2023 03:25:00 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1681986300; cv=none; d=google.com; s=arc-20160816; b=S/986hBDJv4YCq+Qcx2uYAXACpxv2UrjuMCZzt0TrGYObMyx3swNPUlNr3CxEp6SSj yvFRqovsN0aZSgI+N1qQsMC2QLYP47ARtemfBs6EHUks96buW7Bw0CnjTexaKVaRgmoQ cmYnZ9ARNAz+Sv8lCCUbNY/Lpxn7tJk+ehEmLhQB9vjTN7S4ierHr4KBYnWWSuvYk+wB Et4n4g+QpF26SozC1zLfu122ufJULluouRNQvVKS601GkZ1t9+pIGFYr+EXZ/D2ieZGe tLs/Cy6JbtIHj55MJaWB03QZ7aEq3k/d5pVrP84W1SUN5iFNLK7T/mPeLejszY2o6W1V 8PyA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=/rqhyU6uXoU+/LhRADh+Yrxp3II6kKdCrObNCj5zZIk=; b=hHvhreoSSuzvsTZdcDDEQ55IJE1V7K4aJWdIXNlhEGOkiy8j/1ZRunO2cOcQRDW8JZ MpkYt4idfyV1sL3yjnuDHrDKn1/JNbLl8WPz9JE86CdIz3/QRvK1/nupzL+KFLeCbkHF 8/wLZwlGl1sUqpONATSAQQM9Lrwa88D3H2rdGJT/E03zgQ3OQPIRU8o9QPfH5bYS0wSY fuFLHLiXqM0ug84TPCeiT77fzw0dJHc1YgItUdENIsw42XaO9ChV3O4LrGdK0G9aQHgh 6Fnl+X+W+kF3WmrZAElsz96mna7N6nPHnX+/Ipc7JGlofo4HEOye+GH8yWxJpCN6k75F R6uw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b="Ut6iQ/+C"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id c15-20020a170902d48f00b001a6dba52e52si1672137plg.390.2023.04.20.03.24.48; Thu, 20 Apr 2023 03:25:00 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b="Ut6iQ/+C"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234412AbjDTKSF (ORCPT <rfc822;cjcooper78@gmail.com> + 99 others); Thu, 20 Apr 2023 06:18:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58772 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234328AbjDTKRj (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 20 Apr 2023 06:17:39 -0400 Received: from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com [IPv6:2a00:1450:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 874134C30 for <linux-kernel@vger.kernel.org>; Thu, 20 Apr 2023 03:16:25 -0700 (PDT) Received: by mail-ej1-x62c.google.com with SMTP id a640c23a62f3a-94ed7e49541so49813666b.1 for <linux-kernel@vger.kernel.org>; Thu, 20 Apr 2023 03:16:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1681985784; x=1684577784; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=/rqhyU6uXoU+/LhRADh+Yrxp3II6kKdCrObNCj5zZIk=; b=Ut6iQ/+CoIv1rPMxqnZvK0NSWdqeCFUzd6OEx1lKkUgb/r52SzTay59vOp6CZkva05 8SGuiVyeXnxzYx/n6GIJRR8oUGw9tjeCMCLAcQATcoAebowSItrF3GwVrDQDCqTYJTfv nVq35AQtK5pxpUcHSNBA7H0d14o3p7Dzu0lQ4p8r82XQdBVUBsvr4GJ7X4HY22Xt/IIt bUMDdwpLFNd8eyAy7mD+F1M84wUS4ga+3/PganPMZM3Cln5eEY8lqVR/OApIs0lFZn9M pVShqreCbx9e8O1gOETlPSNg8pzNbkIaLUyskwjwBn/1ifnBr+LoakoKoqAWmiDuDezk A3pg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681985784; x=1684577784; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/rqhyU6uXoU+/LhRADh+Yrxp3II6kKdCrObNCj5zZIk=; b=R0TWbzthFA8nHaLA2jp/V/ZJr65IqYEoVPmKBYUfFGJYFPDIc7SSUj0siiCHu0QNED 9V/wkSle0kKKKFlEy3KCTQP4tdLwNpmBYZQtOzKyB0Pb5jmBT7/zluWoNuEa4CAPeKlz w2E7Tv22Iizp7h6oKHl28qeSP/ikYAixrY5u2ulSBqZV/uDxHkzb+V6bu+R3QuzNzsc6 F9cvqmdNSRIOjs5co6StNZByLqjTMWQhSiLLORVMARd1BH2Sn5VXdp53dlvAIxiq9R6o NzytVovZPMwLfyvJpvUFi/O8Ga8AqRt1S39W35dAOLHWkAVbTzrSgb8VwMXMb73PJRZZ eymA== X-Gm-Message-State: AAQBX9dY0aQ6eMCDT1lhvHDLo6LOwjU/ItmBoqjqdsbM1wSi5T4jlJ/a XEFtN5msAQ93TRZXajTH+qcqxQ== X-Received: by 2002:aa7:d81a:0:b0:504:98f1:464c with SMTP id v26-20020aa7d81a000000b0050498f1464cmr1260446edq.23.1681985783905; Thu, 20 Apr 2023 03:16:23 -0700 (PDT) Received: from krzk-bin.. ([2a02:810d:15c0:828:bcb8:77e6:8f45:4771]) by smtp.gmail.com with ESMTPSA id l22-20020aa7c3d6000000b00506be898998sm588954edr.29.2023.04.20.03.16.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Apr 2023 03:16:23 -0700 (PDT) From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> To: Vinod Koul <vkoul@kernel.org>, Bard Liao <yung-chuan.liao@linux.intel.com>, Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>, Sanyog Kale <sanyog.r.kale@intel.com>, Andy Gross <agross@kernel.org>, Bjorn Andersson <andersson@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, Liam Girdwood <lgirdwood@gmail.com>, Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>, Patrick Lai <quic_plai@quicinc.com> Subject: [PATCH 1/6] ASoC: wcd938x: switch to using gpiod API Date: Thu, 20 Apr 2023 12:16:12 +0200 Message-Id: <20230420101617.142225-2-krzysztof.kozlowski@linaro.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230420101617.142225-1-krzysztof.kozlowski@linaro.org> References: <20230420101617.142225-1-krzysztof.kozlowski@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1763690467273319141?= X-GMAIL-MSGID: =?utf-8?q?1763690467273319141?= |
Series |
ASoC/soundwire: qcom: correctly probe devices after link init
|
|
Commit Message
Krzysztof Kozlowski
April 20, 2023, 10:16 a.m. UTC
From: Dmitry Torokhov <dmitry.torokhov@gmail.com> Switch the driver from legacy gpio API that is deprecated to the newer gpiod API that respects line polarities described in ACPI/DT. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Tested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> [krzysztof: rebased on recent dev_err_probe() changes] Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- Cc: Patrick Lai <quic_plai@quicinc.com> --- sound/soc/codecs/wcd938x.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
Comments
On Thu, Apr 20, 2023 at 12:16:12PM +0200, Krzysztof Kozlowski wrote: > - gpio_direction_output(wcd938x->reset_gpio, 0); > - /* 20us sleep required after pulling the reset gpio to LOW */ > + gpiod_set_value_cansleep(wcd938x->reset_gpio, 1); > + /* 20us sleep required after asserting the reset gpio */ This is inverting the sense of the GPIO in the API from active low to active high which will mean we're introducing a new reliance on having the signal described as active low in DT. That's an ABI concern. I remain deeply unconvinced that remapping active low outputs like this in the GPIO API is helping.
On 20/04/2023 13:58, Mark Brown wrote: > On Thu, Apr 20, 2023 at 12:16:12PM +0200, Krzysztof Kozlowski wrote: > >> - gpio_direction_output(wcd938x->reset_gpio, 0); >> - /* 20us sleep required after pulling the reset gpio to LOW */ >> + gpiod_set_value_cansleep(wcd938x->reset_gpio, 1); >> + /* 20us sleep required after asserting the reset gpio */ > > This is inverting the sense of the GPIO in the API from active low to > active high which will mean we're introducing a new reliance on having > the signal described as active low in DT. That's an ABI concern. It's bringing it to the correct level. Old code was not respecting the DTS thus if such DTS came with inverted design, the driver would not work. We were already fixing the upstream DTS users and I thought all of them are fixed since long time (half a year) or even correct from the beginning. Now I found one more case with incorrect level, which I will fix. > > I remain deeply unconvinced that remapping active low outputs like this > in the GPIO API is helping. The code is mapping them to correct state. The previous state was incorrect and did not allow to handle active high (which can happen). This is the effort to make code correct - driver and DTS. Best regards, Krzysztof
On 20/04/2023 14:30, Krzysztof Kozlowski wrote: > On 20/04/2023 13:58, Mark Brown wrote: >> On Thu, Apr 20, 2023 at 12:16:12PM +0200, Krzysztof Kozlowski wrote: >> >>> - gpio_direction_output(wcd938x->reset_gpio, 0); >>> - /* 20us sleep required after pulling the reset gpio to LOW */ >>> + gpiod_set_value_cansleep(wcd938x->reset_gpio, 1); >>> + /* 20us sleep required after asserting the reset gpio */ >> >> This is inverting the sense of the GPIO in the API from active low to >> active high which will mean we're introducing a new reliance on having >> the signal described as active low in DT. That's an ABI concern. > > It's bringing it to the correct level. Old code was not respecting the > DTS thus if such DTS came with inverted design, the driver would not work. > > We were already fixing the upstream DTS users and I thought all of them > are fixed since long time (half a year) or even correct from the > beginning. Now I found one more case with incorrect level, which I will fix. No, my bad - all upstream DTSes are corrected since half year. > >> >> I remain deeply unconvinced that remapping active low outputs like this >> in the GPIO API is helping. > > The code is mapping them to correct state. The previous state was > incorrect and did not allow to handle active high (which can happen). > This is the effort to make code correct - driver and DTS. Best regards, Krzysztof
On Thu, Apr 20, 2023 at 02:30:17PM +0200, Krzysztof Kozlowski wrote: > On 20/04/2023 13:58, Mark Brown wrote: > > On Thu, Apr 20, 2023 at 12:16:12PM +0200, Krzysztof Kozlowski wrote: > >> - gpio_direction_output(wcd938x->reset_gpio, 0); > >> - /* 20us sleep required after pulling the reset gpio to LOW */ > >> + gpiod_set_value_cansleep(wcd938x->reset_gpio, 1); > >> + /* 20us sleep required after asserting the reset gpio */ > > This is inverting the sense of the GPIO in the API from active low to > > active high which will mean we're introducing a new reliance on having > > the signal described as active low in DT. That's an ABI concern. > It's bringing it to the correct level. Old code was not respecting the > DTS thus if such DTS came with inverted design, the driver would not work. Sure, but OTOH if the user didn't bother specifying as active low it would work. I suspect it's more likely that someone missed a flag that had no practical impact in DT than that someone would add an inverter to their design. > We were already fixing the upstream DTS users and I thought all of them > are fixed since long time (half a year) or even correct from the > beginning. Now I found one more case with incorrect level, which I will fix. That's just upstream, what about any downstream users? > > I remain deeply unconvinced that remapping active low outputs like this > > in the GPIO API is helping. > The code is mapping them to correct state. The previous state was > incorrect and did not allow to handle active high (which can happen). > This is the effort to make code correct - driver and DTS. We could handle inversions through an explicit property if that were needed, that would be a less problematic transition and clearer in the consumer code.
On 20/04/2023 15:00, Mark Brown wrote: > On Thu, Apr 20, 2023 at 02:30:17PM +0200, Krzysztof Kozlowski wrote: >> On 20/04/2023 13:58, Mark Brown wrote: >>> On Thu, Apr 20, 2023 at 12:16:12PM +0200, Krzysztof Kozlowski wrote: > >>>> - gpio_direction_output(wcd938x->reset_gpio, 0); >>>> - /* 20us sleep required after pulling the reset gpio to LOW */ >>>> + gpiod_set_value_cansleep(wcd938x->reset_gpio, 1); >>>> + /* 20us sleep required after asserting the reset gpio */ > >>> This is inverting the sense of the GPIO in the API from active low to >>> active high which will mean we're introducing a new reliance on having >>> the signal described as active low in DT. That's an ABI concern. > >> It's bringing it to the correct level. Old code was not respecting the >> DTS thus if such DTS came with inverted design, the driver would not work. > > Sure, but OTOH if the user didn't bother specifying as active low it > would work. I suspect it's more likely that someone missed a flag that > had no practical impact in DT than that someone would add an inverter to > their design. > >> We were already fixing the upstream DTS users and I thought all of them >> are fixed since long time (half a year) or even correct from the >> beginning. Now I found one more case with incorrect level, which I will fix. > > That's just upstream, what about any downstream users? Life of downstream. We all know the drill: merge your DTS or suffer. The WCD938x codecs are moderately new, so I do not expect many downstream users. They are in theory possible, because driver was merged in v5.14-rc1 and for the newest products Qualcomm uses v5.15. Although now it is v5.15, but the time driver was merged, maybe it was v5.10. I could rework this patch to provide backwards compatible solution like I did for WSA: https://lore.kernel.org/all/20230102114152.297305-4-krzysztof.kozlowski@linaro.org/ There are downsides of it, but as you pointed out - it's actually very rare to have the signal inverted in hardware. > >>> I remain deeply unconvinced that remapping active low outputs like this >>> in the GPIO API is helping. > >> The code is mapping them to correct state. The previous state was >> incorrect and did not allow to handle active high (which can happen). >> This is the effort to make code correct - driver and DTS. > > We could handle inversions through an explicit property if that were > needed, that would be a less problematic transition and clearer in the > consumer code. I am not sure if it is worth. The DTS is supposed to describe hardware, so even if reset pin flag was not effective, it is a mistake to describe it as ACTIVE_HIGH. Do we care about keeping broken code happy? If yes, then property is the way to go. If partially, then I can add backwards-compatible approach like I mentioned above. Best regards, Krzysztof
On Thu, Apr 20, 2023 at 04:16:59PM +0200, Krzysztof Kozlowski wrote: > On 20/04/2023 15:00, Mark Brown wrote: > > That's just upstream, what about any downstream users? > Life of downstream. We all know the drill: merge your DTS or suffer. The No, the DT is supposed to be an ABI. The point in having a domain specific language with a compiler is to allow device trees to be distributed independently of the kernel. > I could rework this patch to provide backwards compatible solution like > I did for WSA: > https://lore.kernel.org/all/20230102114152.297305-4-krzysztof.kozlowski@linaro.org/ There we go... > > We could handle inversions through an explicit property if that were > > needed, that would be a less problematic transition and clearer in the > > consumer code. > I am not sure if it is worth. The DTS is supposed to describe hardware, > so even if reset pin flag was not effective, it is a mistake to describe > it as ACTIVE_HIGH. Do we care about keeping broken code happy? If yes, > then property is the way to go. If partially, then I can add > backwards-compatible approach like I mentioned above. It's not just this individual transition, it's the whole thing with encoding the polarity of the signal at all - it's a layer of abstraction that feels like it introduces at least as many problems as it solves, and requiring configuration on every single system integration doesn't feel like the right choice in general.
On 20/04/2023 18:28, Mark Brown wrote: > On Thu, Apr 20, 2023 at 04:16:59PM +0200, Krzysztof Kozlowski wrote: >> On 20/04/2023 15:00, Mark Brown wrote: > >>> That's just upstream, what about any downstream users? > >> Life of downstream. We all know the drill: merge your DTS or suffer. The > > No, the DT is supposed to be an ABI. No, the DT bindings are the ABI. We are supposed not to break user-space, but out-of-tree users of drivers are not ABI by itself. Bindings are. If out-of-tree users make mistakes in their DTS and do not want to upstream it, it's their choice but it does not come for free. > The point in having a domain > specific language with a compiler is to allow device trees to be > distributed independently of the kernel. When it is written incorrectly - wrong flag used for GPIO - there is no requirement to support it. >> I could rework this patch to provide backwards compatible solution like >> I did for WSA: >> https://lore.kernel.org/all/20230102114152.297305-4-krzysztof.kozlowski@linaro.org/ > > There we go... > >>> We could handle inversions through an explicit property if that were >>> needed, that would be a less problematic transition and clearer in the >>> consumer code. > >> I am not sure if it is worth. The DTS is supposed to describe hardware, >> so even if reset pin flag was not effective, it is a mistake to describe >> it as ACTIVE_HIGH. Do we care about keeping broken code happy? If yes, >> then property is the way to go. If partially, then I can add >> backwards-compatible approach like I mentioned above. > > It's not just this individual transition, it's the whole thing with > encoding the polarity of the signal at all - it's a layer of abstraction > that feels like it introduces at least as many problems as it solves, > and requiring configuration on every single system integration doesn't > feel like the right choice in general. Choosing appropriate flag for GPIO in DTS is not difficult. It was skipped because we rarely cared in the drivers, but it should have been chosen correctly. The same about interrupt flags. We had many DTS for many times marking all possible interrupts as IRQ_TYPE_NONE. Did it matter for many drivers and setups? No, was perfectly "fine". Is it correct from DTS point of view. Also no. Best regards, Krzysztof
On Thu, Apr 20, 2023 at 07:51:27PM +0200, Krzysztof Kozlowski wrote: > On 20/04/2023 18:28, Mark Brown wrote: > > On Thu, Apr 20, 2023 at 04:16:59PM +0200, Krzysztof Kozlowski wrote: > >> Life of downstream. We all know the drill: merge your DTS or suffer. The > > No, the DT is supposed to be an ABI. > No, the DT bindings are the ABI. We are supposed not to break > user-space, but out-of-tree users of drivers are not ABI by itself. > Bindings are. If out-of-tree users make mistakes in their DTS and do not > want to upstream it, it's their choice but it does not come for free. This is absolutely not the case, users should be able to ship DTs without upstreaming them and run multiple operating systems on top of a single DT - ideally boards would ship with DTs in firmware and people would be able to install generic OSs onto them with just off the shelf install media. This is even a thing that people have actually done, both non-FDT systems like SPARC and the PowerPC systems from Apple and a few FDT ones like Synquacer. The enormous costs of DT would hardly be worth it if it were purely an in tree thing. > > The point in having a domain > > specific language with a compiler is to allow device trees to be > > distributed independently of the kernel. > When it is written incorrectly - wrong flag used for GPIO - there is no > requirement to support it. If it worked was it ever really wrong (and note that the bindings may not always be super clear...)? While there is a point at which things never worked, can be fixed and we don't need to care about it or where we know the userbase well enough to know there won't be any issue those shouldn't be the default and should generally be avoided. Where there is a good reason to break compatibility it should be something we're actively deciding to do for a clear reason having considered the tradeoffs, not something that gets done on a whim without even mentioning it. > > It's not just this individual transition, it's the whole thing with > > encoding the polarity of the signal at all - it's a layer of abstraction > > that feels like it introduces at least as many problems as it solves, > > and requiring configuration on every single system integration doesn't > > feel like the right choice in general. > Choosing appropriate flag for GPIO in DTS is not difficult. It was > skipped because we rarely cared in the drivers, but it should have been > chosen correctly. The same about interrupt flags. We had many DTS for > many times marking all possible interrupts as IRQ_TYPE_NONE. Did it > matter for many drivers and setups? No, was perfectly "fine". Is it > correct from DTS point of view. Also no. There's no natural definition of "correct" here though - it's just picking something in a binding. If someone for example flips the label on a signal from reset to enable (perhaps during review) that ends up changing active high to active low, and really I'm not sure how much we're really winning compared to just having code in the end consumer which just directly says what value it wants the physical signal to have. My point is not that we haven't defined things such that the user has to specify if something is active high or active low, it's that it feels like it's more trouble han it's worth.
diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c index 11b264a63b04..33fd8fdde9fd 100644 --- a/sound/soc/codecs/wcd938x.c +++ b/sound/soc/codecs/wcd938x.c @@ -6,12 +6,14 @@ #include <linux/platform_device.h> #include <linux/device.h> #include <linux/delay.h> +#include <linux/err.h> #include <linux/gpio/consumer.h> +#include <linux/irq.h> +#include <linux/irqdomain.h> #include <linux/kernel.h> #include <linux/pm_runtime.h> #include <linux/component.h> #include <sound/tlv.h> -#include <linux/of_gpio.h> #include <linux/of.h> #include <sound/jack.h> #include <sound/pcm.h> @@ -194,7 +196,7 @@ struct wcd938x_priv { int flyback_cur_det_disable; int ear_rx_path; int variant; - int reset_gpio; + struct gpio_desc *reset_gpio; struct gpio_desc *us_euro_gpio; u32 micb1_mv; u32 micb2_mv; @@ -4234,16 +4236,16 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device struct wcd_mbhc_config *cfg = &wcd938x->mbhc_cfg; int ret; - wcd938x->reset_gpio = of_get_named_gpio(dev->of_node, "reset-gpios", 0); - if (wcd938x->reset_gpio < 0) - return dev_err_probe(dev, wcd938x->reset_gpio, - "Failed to get reset gpio\n"); + wcd938x->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_ASIS); + ret = PTR_ERR_OR_ZERO(wcd938x->reset_gpio); + if (ret) + return dev_err_probe(dev, ret, "Failed to get reset gpio\n"); wcd938x->us_euro_gpio = devm_gpiod_get_optional(dev, "us-euro", GPIOD_OUT_LOW); - if (IS_ERR(wcd938x->us_euro_gpio)) - return dev_err_probe(dev, PTR_ERR(wcd938x->us_euro_gpio), - "us-euro swap Control GPIO not found\n"); + ret = PTR_ERR_OR_ZERO(wcd938x->us_euro_gpio); + if (ret) + return dev_err_probe(dev, ret, "us-euro swap Control GPIO not found\n"); cfg->swap_gnd_mic = wcd938x_swap_gnd_mic; @@ -4278,11 +4280,11 @@ static int wcd938x_populate_dt_data(struct wcd938x_priv *wcd938x, struct device static int wcd938x_reset(struct wcd938x_priv *wcd938x) { - gpio_direction_output(wcd938x->reset_gpio, 0); - /* 20us sleep required after pulling the reset gpio to LOW */ + gpiod_set_value_cansleep(wcd938x->reset_gpio, 1); + /* 20us sleep required after asserting the reset gpio */ usleep_range(20, 30); - gpio_set_value(wcd938x->reset_gpio, 1); - /* 20us sleep required after pulling the reset gpio to HIGH */ + gpiod_set_value_cansleep(wcd938x->reset_gpio, 0); + /* 20us sleep required after releasing the reset gpio */ usleep_range(20, 30); return 0;