Received: by 2002:a05:6a10:6744:0:0:0:0 with SMTP id w4csp1302539pxu; Thu, 8 Oct 2020 08:15:45 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwId+w1Dwu5EpJJtOcR7WG12qScSEO5n6p2keMlK++6R2xirjjNQd65qGw8KIDPgg/1yF3A X-Received: by 2002:a05:6402:1298:: with SMTP id w24mr9116235edv.280.1602170145655; Thu, 08 Oct 2020 08:15:45 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1602170145; cv=none; d=google.com; s=arc-20160816; b=UoV5ifXdBwQ7bu7EzUWRtziuzyJgb9DAdO6VzDkf3j65vUtMU/CH44etpP1wzD90yG s3d1wX0hGjoTVLDgQsrjaHdQRk8YFVumKX60HRcmT9NYE9zw/gRZy3qI6/jVcZiiQmGy NtHLJYJqs/EGSseyUYw4bn9V5tOLUuWCC9JxS8LaXNpiGiGgF3gR1Mk9iWCpx3+Glc9v kMfaXAGrIw6yPJziYggmruwNPzCWnHHdonAlIwtPVodQMTd9Q3uVuZMVNQjxI774HNCj +bxP/J90s6Z5xjGfJl5l7DdxsE6WcokoDZvmu+QQLYIO22mZw0pRKiaKhfzhEtQtrOpg yJZQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=wsPwsTl/hAvDoxdVSq6VH5qAHlyEl+VKWN+4G8Q2WOQ=; b=U7P8kzmUzYrJGU/an2g9ZMGFZHkAY82OXPVkiScjf4dPZUs0b2kxadFZ+MA/FraUi0 Tj/XRSp/7TYle4DW7SzZ3aYptSg3rF4WOGhGDFMxrtiJhMag+gKXWsexdDnj4s92nkN+ hqKc4tHmW/b9cm9xR1/VV2CCpT88Ptqq+cU51id6VzWF8hpVBvC7I8h2n6niyCMXl9Gl xQzK3EGS7wZLcYc+q7u1eEO4l1iO7IKMy9nXpBHhszrUXsk+43EcCg1UEb7DFOR90d9b cBZR9KM053XcIxsfJcy/bt4OEHzHWcOdZbVy02S4tL/YPwtqGUkh1NPD2ywJwynKBGBF QoMQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=s3+iGaEs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id n24si3957282ejz.503.2020.10.08.08.15.18; Thu, 08 Oct 2020 08:15:45 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=s3+iGaEs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 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 S1730956AbgJHPNF (ORCPT + 99 others); Thu, 8 Oct 2020 11:13:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35146 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730953AbgJHPNE (ORCPT ); Thu, 8 Oct 2020 11:13:04 -0400 Received: from mail-vk1-xa41.google.com (mail-vk1-xa41.google.com [IPv6:2607:f8b0:4864:20::a41]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 35618C0613D2 for ; Thu, 8 Oct 2020 08:13:04 -0700 (PDT) Received: by mail-vk1-xa41.google.com with SMTP id d2so1380589vkd.13 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=FmZHRSnT4fBM4/qUoJ4v+nIv2bqMdexmmaPJJHkPowrLC38XVeHytNTLzmSSTLAh5z 7PJ+NhCCTcIZiM4WTTaSfqra03gvMjo5wqIu1JubxGlCe9IX9N8Pu04FGCNXf6Vb/dZ4 yn92zBn6Fh0BXmoHHAxN98OpOXrSKsU/+db5YFh+JAaofvx1Cn5Ci1gykSylcDZ24gGF NBaMqnTmKKUlnYruSK1Aa3XgE22QWbFvoC2T+ULvDaeaU5QtZQlNt1qNpGUfYR08Zqmn LOd/FXcBrt1u5ZTAEJos4LbB5jA1LlEDKrE3vzudbIeTapzRlu9L7wff4C1sJppx9Suv veyQ== X-Gm-Message-State: AOAM533sY1/5sCyKP+P3y6f+u2t9iBwbAgklsEYhWnCGl9Bn3zt9xPrb 5ITNm3hR/8EwMOpqbvDUiw6esQwbclTmjlfGrLpf/sloiH7GKdBK 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 Cc: muhammad.husaini.zulkifli@intel.com, Michal Simek , andriy.shevchenko@intel.com, "linux-mmc@vger.kernel.org" , Linux ARM , Linux Kernel Mailing List , lakshmi.bai.raja.subramanian@intel.com, Wan Ahmad Zainie , Arnd Bergmann Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.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