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=-9.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 47B5EC4363A for ; Thu, 8 Oct 2020 15:14:40 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 B835620782 for ; Thu, 8 Oct 2020 15:14:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="p3+0RC1O"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="s3+iGaEs" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B835620782 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0ImnzIV5mhi6hwYezGGg5CEOoiMjKatTKXH9r88MaSo=; b=p3+0RC1O4Q3YFf+MuGLwv7WP3 Da5L2YE0K8gRK1MX3dJQuwCYtMrFxwIGLASHeR5lQ9dUIG791bsGYzduLHD/NP4NgNlpd/ZiK0C4N PFCYV7cbzVPN3tAKgLmtAOV4tCdV/WJBesmJnH9kmJjsC5qScVRXCa++G2LUwrSOPl7ha7jYtmd4n xoULJFbH0R2qDLB86+/mTzS8acOfJjFtR1NWLaAUUlmp3U4kxvG5AdFtwxg/PGk9Ju5yu2FI0FYhe H5+ud/cFYMISIBp98riKyBo7BhvQUE+OvlJFZJdQgV9UCQ1eDum/poaX+KwhXqwLWAppiqFrj84wO aOHrThlLg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kQXbU-0004uz-Bb; Thu, 08 Oct 2020 15:13:12 +0000 Received: from mail-vk1-xa43.google.com ([2607:f8b0:4864:20::a43]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kQXbM-0004tu-V8 for linux-arm-kernel@lists.infradead.org; Thu, 08 Oct 2020 15:13:10 +0000 Received: by mail-vk1-xa43.google.com with SMTP id p5so738086vkm.4 for ; Thu, 08 Oct 2020 08:13:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=wsPwsTl/hAvDoxdVSq6VH5qAHlyEl+VKWN+4G8Q2WOQ=; b=s3+iGaEsVITyDA7YZovcQvYscw4EjPjWOCEJT6rKSq1KDPf+ha55v3LiLzlgk3arv3 f1t4gMJ1jg1x8B8c2TjMHfn6n98VLEiDa4W0kR175b3mrUz/3u+jS6Y11ta9gFMmZun4 uI/4ljpbnWGXI7aGQCKMOaujD8rnXq4yQTrvenvACQ49VN6X/fwVY+7OUi87dyauylOW JFVG/gaaoLasrA541feJDtIv9X+uS8PXYIUw+MRowHzzA4vrj80/G2Bg+ootk3eJ/jBN JEunId1ia3jNICZ6G9I2rookeL5+xZG7ere0myXr68+Eo1bxrU6rW3a8X3IHaqMzEG33 X5lg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=wsPwsTl/hAvDoxdVSq6VH5qAHlyEl+VKWN+4G8Q2WOQ=; b=ncbeITyFGzTd3OEwegYjOiFLXpEiTmKc9CtbT2PPVu0/2MEuyDMX1FfKoh0a7a4Zy6 VEUoTYBxoD53h4ISXSAlyfjCbRwlKJNY9PeQZmxhbvzlzD/Fo27OPlxOBHTui6MRwwVy /GSnuJ8xL29V/77wPNDEZjo96smC4NBaGyrztxk5SjjI/ORby1wdhLloJqtMNcMfCMGm Mw+Hs8iINWMuE5n2ARQGX0U59DoVczDbB51hNLNlc+sxkyBAE5YEs4PguZk3CR7QqpT8 AcG3v4CyD9RuhNberK5qfFkx3s7QQ6GlIHQsvvcKV/m4gBvqoCKoJQox98d+VS+k/LT9 70mQ== X-Gm-Message-State: AOAM533Pnl/8YT4VJppUOv10tohUnrtyqI5oVk4X84Cw8Q++fifePHsY 8SF80J+k1iqhyCZ50jtPzluLi7oHDgEIh6nNDh/Uxw== X-Google-Smtp-Source: ABdhPJwEjGnJf7ZCkTSVlSm0STvrBQYCiSWeuqvCe29hXNIC4YsoSyZw/x3HwLnhyDvaFcM5tgClpCqZEqy/KFuvln0= X-Received: by 2002:a1f:ae85:: with SMTP id x127mr1703271vke.8.1602169983032; Thu, 08 Oct 2020 08:13:03 -0700 (PDT) MIME-Version: 1.0 References: <20201008020936.19894-1-muhammad.husaini.zulkifli@intel.com> <20201008020936.19894-5-muhammad.husaini.zulkifli@intel.com> <35692f1c-62a4-6c71-d67a-2a216e97e7d5@intel.com> In-Reply-To: <35692f1c-62a4-6c71-d67a-2a216e97e7d5@intel.com> From: Ulf Hansson Date: Thu, 8 Oct 2020 17:12:26 +0200 Message-ID: Subject: Re: [PATCH v4 4/4] mmc: sdhci-of-arasan: Enable UHS-1 support for Keem Bay SOC To: Adrian Hunter X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201008_111305_300320_8DABD5B0 X-CRM114-Status: GOOD ( 30.76 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: andriy.shevchenko@intel.com, Arnd Bergmann , lakshmi.bai.raja.subramanian@intel.com, "linux-mmc@vger.kernel.org" , Linux Kernel Mailing List , Michal Simek , Wan Ahmad Zainie , muhammad.husaini.zulkifli@intel.com, Linux ARM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 8 Oct 2020 at 12:58, Adrian Hunter wrote: > > On 8/10/20 12:27 pm, Ulf Hansson wrote: > > On Thu, 8 Oct 2020 at 04:12, wrote: > >> > >> From: Muhammad Husaini Zulkifli > >> > >> Voltage switching sequence is needed to support UHS-1 interface. > >> There are 2 places to control the voltage. > >> 1) By setting the AON register using firmware driver calling > >> system-level platform management layer (SMC) to set the register. > >> 2) By controlling the GPIO expander value to drive either 1.8V or 3.3V > >> for power mux input. > >> > >> Signed-off-by: Muhammad Husaini Zulkifli > >> Reviewed-by: Andy Shevchenko > >> Reviewed-by: Adrian Hunter > >> --- > >> drivers/mmc/host/sdhci-of-arasan.c | 126 +++++++++++++++++++++++++++++ > >> 1 file changed, 126 insertions(+) > >> > >> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c > >> index 46aea6516133..ea2467b0073d 100644 > >> --- a/drivers/mmc/host/sdhci-of-arasan.c > >> +++ b/drivers/mmc/host/sdhci-of-arasan.c > >> @@ -16,6 +16,7 @@ > >> */ > >> > >> #include > >> +#include > >> #include > >> #include > >> #include > >> @@ -23,6 +24,7 @@ > >> #include > >> #include > >> #include > >> +#include > >> > >> #include "cqhci.h" > >> #include "sdhci-pltfm.h" > >> @@ -136,6 +138,7 @@ struct sdhci_arasan_clk_data { > >> * @soc_ctl_base: Pointer to regmap for syscon for soc_ctl registers. > >> * @soc_ctl_map: Map to get offsets into soc_ctl registers. > >> * @quirks: Arasan deviations from spec. > >> + * @uhs_gpio: Pointer to the uhs gpio. > >> */ > >> struct sdhci_arasan_data { > >> struct sdhci_host *host; > >> @@ -150,6 +153,7 @@ struct sdhci_arasan_data { > >> struct regmap *soc_ctl_base; > >> const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; > >> unsigned int quirks; > >> + struct gpio_desc *uhs_gpio; > >> > >> /* Controller does not have CD wired and will not function normally without */ > >> #define SDHCI_ARASAN_QUIRK_FORCE_CDTEST BIT(0) > >> @@ -361,6 +365,112 @@ static int sdhci_arasan_voltage_switch(struct mmc_host *mmc, > >> return -EINVAL; > >> } > >> > >> +static int sdhci_arasan_keembay_voltage_switch(struct mmc_host *mmc, > >> + struct mmc_ios *ios) > >> +{ > >> + struct sdhci_host *host = mmc_priv(mmc); > >> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > >> + struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host); > >> + u16 ctrl_2, clk; > >> + int ret; > >> + > >> + switch (ios->signal_voltage) { > >> + case MMC_SIGNAL_VOLTAGE_180: > >> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > >> + clk &= ~SDHCI_CLOCK_CARD_EN; > >> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL); > >> + > >> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL); > >> + if (clk & SDHCI_CLOCK_CARD_EN) > >> + return -EAGAIN; > >> + > >> + sdhci_writeb(host, SDHCI_POWER_ON | SDHCI_POWER_180, > >> + SDHCI_POWER_CONTROL); > >> + > >> + /* > >> + * Set VDDIO_B voltage to Low for 1.8V > >> + * which is controlling by GPIO Expander. > >> + */ > >> + gpiod_set_value_cansleep(sdhci_arasan->uhs_gpio, 0); > >> + > >> + /* > >> + * This is like a final gatekeeper. Need to ensure changed voltage > >> + * is settled before and after turn on this bit. > >> + */ > >> + usleep_range(1000, 1100); > >> + > >> + ret = keembay_sd_voltage_selection(KEEMBAY_SET_1V8_VOLT); > >> + if (ret) > >> + return ret; > >> + > >> + usleep_range(1000, 1100); > > > > No, sorry, but I don't like this. > > > > This looks like a GPIO regulator with an extension of using the > > keembay_sd_voltage_selection() thingy. I think you can model these > > things behind a regulator and hook it up as a vqmmc supply in DT > > instead. BTW, this is the common way we deal with these things for mmc > > host drivers. > > It seemed to me that would just result in calling regulator API instead of > GPIO API but the flow above would otherwise be unchanged i.e. no benefit > To me, the benefit is about avoiding platform specific code in drivers - but also about consistency. For I/O signal voltage, the common method here, is to model this as a GPIO regulator. This means we can use these available helpers from the core: mmc_regulator_set_vqmmc() mmc_regulator_get_supply() Kind regards Uffe _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel