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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3395CC4345F for ; Sat, 27 Apr 2024 11:21:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:Date:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=yMKB45DWztKCsqstag2JnbS2apf6Z1DnY/l+EMU7EYY=; b=xXZFIgJuwyQSuw aCPdbZozV/RDLucDxCVmmhr27+m9+qm16w5c3i/DcX9pcFh+4vPeaa5jqt2gXQIii4EhF7oFGnu+/ zQWYQ9Zq15p66mm7Zm/tPczPRY6Ag3B6wRePC+uCCxBDVBro7Q7DhO7YyrLhZbS1HR4cC8CnddQmw 5M2C8+LjExNO0CkqyrcYwJ3TuNJ6tLgjMBHMPSGE3Ukvvdpmiav21W4tb407uFwjb2yOfiGosMDAG w6zmDRhWCWV4CdS56gLYB8qrJsB8mKjd4dZUCv+ljnzAzJRTOTSOQNpujqSlzqF0+K6I8sr/1mLZL X/kWttE/d5u3YsstcSAA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1s0g7B-0000000FPWR-2m16; Sat, 27 Apr 2024 11:21:09 +0000 Received: from mail-ed1-x531.google.com ([2a00:1450:4864:20::531]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1s0g78-0000000FPVN-2WCR for linux-arm-kernel@lists.infradead.org; Sat, 27 Apr 2024 11:21:08 +0000 Received: by mail-ed1-x531.google.com with SMTP id 4fb4d7f45d1cf-56e69888a36so3201356a12.3 for ; Sat, 27 Apr 2024 04:21:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1714216865; x=1714821665; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=IRPW+GikaAPs83DoedFde9bOURsu7XsMsV7lagjHs7E=; b=R5ofcZEKUStm3LivjIa+phCcy9k8AOnCzoI546l9FCZ+ug4lMN1XjzZ6fEvVomRQfW oJDV8bwlFVrN0s7QkCfClb1QHHaSPZAEZkSfeYd/ktMo+vMhSGGmzLMalD+irFyXus/M D5f5xVXIFTovGmDSg1j5ZT42vrbAYMNsNv06hUQm3h7K9TdfAZ3RGH/CBaul6qaSjbrs 4ufAlZBQWJWkXmAtKvAJWhDcFW/sl1G8Taihg41bzE0lcYg8q9/YWkNvUgAYlFAYa70u ik//+rpexqCSArKSM52d2pcsem1y3P0HxrVRrRJ3oZzB/tChWIDne6DzUfM8WgPiepPP ubRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714216865; x=1714821665; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=IRPW+GikaAPs83DoedFde9bOURsu7XsMsV7lagjHs7E=; b=vkJtseWP7dq9oKncdcirjbaku2JgKFKdiywAnDMB7DzNAziLcYU7/ETy+h0yrhqF31 XeO9y/k3i0INsvDZiFjIm176PVAlioib3rkHkG0zioli6caAH3/h5ZAm2vt/pfgM7KSE xZGwLhcYej5xGvakiKCU9ZrON+T6m4CJ8PRAxxrKOkrntCVPBanFM19rEJOCbUp2wyyK DuwZVsQm4jrIui4M2R8yYC4589hU7bs0lVbY8UJkNrW3OoHYQ+Za9qkXJ7q7kg6qVa3q thze6LlGCxdDZjfigXp57F5Vn7uSV0kfgpFS5HiNFRqAM+H+v5z/FkOulgQe35J9opZS k9vg== X-Forwarded-Encrypted: i=1; AJvYcCU2cHswTzoGwhk4yobDL3C5VxilkcIHoOO2PML3Z7mJP6f9BXRgSQG3esfI+8FS5cIMa7kZZl2QWwm2eJILU3hqwRD96VPdHCuDpo4j4zhsZXLw/vQ= X-Gm-Message-State: AOJu0YxQnFztw5gmQFsKgz7hIBftD3EbFeLi7NJZ7ZZnB35MwMDzGJFj 0ERlP5+A8GQxjGQxRgK0uckpHebXYsYSeBYMhBV+lyvi97wQJaPDgZf8QjkkRh0= X-Google-Smtp-Source: AGHT+IF2riIwu/t+oL7gyIiwcDjB2WQ1NvfVhyty6/q7huEYLjYW7wFDCHoweSqHgFZ70bFqTKHI1A== X-Received: by 2002:a50:8e5e:0:b0:56f:e58f:31f7 with SMTP id 30-20020a508e5e000000b0056fe58f31f7mr3477624edx.28.1714216864903; Sat, 27 Apr 2024 04:21:04 -0700 (PDT) Received: from localhost (host-87-1-234-99.retail.telecomitalia.it. [87.1.234.99]) by smtp.gmail.com with ESMTPSA id ig1-20020a056402458100b0057272ff56f3sm89762edb.93.2024.04.27.04.21.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 27 Apr 2024 04:21:04 -0700 (PDT) From: Andrea della Porta X-Google-Original-From: Andrea della Porta Date: Sat, 27 Apr 2024 13:21:06 +0200 To: Florian Fainelli Cc: Andrea della Porta , Ulf Hansson , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Broadcom internal kernel review list , Linus Walleij , Adrian Hunter , Kamal Dasu , Al Cooper , linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, Jonathan Bell , Phil Elwell Subject: Re: [PATCH 6/6] mmc: sdhci-brcmstb: Add BCM2712 SD Express support Message-ID: Mail-Followup-To: Florian Fainelli , Andrea della Porta , Ulf Hansson , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Broadcom internal kernel review list , Linus Walleij , Adrian Hunter , Kamal Dasu , Al Cooper , linux-mmc@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, Jonathan Bell , Phil Elwell References: <6042c0c7-bb8a-4898-8bed-92155b8e9c4f@broadcom.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <6042c0c7-bb8a-4898-8bed-92155b8e9c4f@broadcom.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240427_042106_672485_CB266C50 X-CRM114-Status: GOOD ( 36.89 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 08:55 Sun 14 Apr , Florian Fainelli wrote: > > > On 4/13/2024 3:14 PM, Andrea della Porta wrote: > > Broadcom BCM2712 SDHCI controller is SD Express capable. Add support > > for sde capability where the implementation is based on downstream driver, > > diverging from it in the way that init_sd_express callback is invoked: > > in downstream the sdhci_ops structure has been augmented with a new > > function ptr 'init_sd_express' that just propagate the call to the > > driver specific callback so that the callstack from a structure > > standpoint is mmc_host_ops -> sdhci_ops. The drawback here is in the > > added level of indirection (the newly added init_sd_express is > > redundant) and the sdhci_ops structure declaration has to be changed. > > To overcome this the presented approach consist in patching the mmc_host_ops > > init_sd_express callback to point directly to the custom function defined in > > this driver (see struct brcmstb_match_priv). > > > > Signed-off-by: Andrea della Porta > > --- > > drivers/mmc/host/Kconfig | 1 + > > drivers/mmc/host/sdhci-brcmstb.c | 147 ++++++++++++++++++++++++++++++- > > 2 files changed, 147 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig > > index aebc587f77a7..343ccac1a4e4 100644 > > --- a/drivers/mmc/host/Kconfig > > +++ b/drivers/mmc/host/Kconfig > > @@ -1018,6 +1018,7 @@ config MMC_SDHCI_BRCMSTB > > depends on ARCH_BRCMSTB || BMIPS_GENERIC || COMPILE_TEST > > depends on MMC_SDHCI_PLTFM > > select MMC_CQHCI > > + select OF_DYNAMIC > > default ARCH_BRCMSTB || BMIPS_GENERIC > > help > > This selects support for the SDIO/SD/MMC Host Controller on > > diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c > > index 907a4947abe5..56fb34a75ec2 100644 > > --- a/drivers/mmc/host/sdhci-brcmstb.c > > +++ b/drivers/mmc/host/sdhci-brcmstb.c > > @@ -29,6 +29,7 @@ > > #define BRCMSTB_PRIV_FLAGS_HAS_CQE BIT(0) > > #define BRCMSTB_PRIV_FLAGS_GATE_CLOCK BIT(1) > > +#define BRCMSTB_PRIV_FLAGS_HAS_SD_EXPRESS BIT(2) > > #define SDHCI_ARASAN_CQE_BASE_ADDR 0x200 > > @@ -50,13 +51,19 @@ struct sdhci_brcmstb_priv { > > unsigned int flags; > > struct clk *base_clk; > > u32 base_freq_hz; > > + struct regulator *sde_1v8; > > + struct device_node *sde_pcie; > > + void *__iomem sde_ioaddr; > > + void *__iomem sde_ioaddr2; > > struct pinctrl *pinctrl; > > struct pinctrl_state *pins_default; > > + struct pinctrl_state *pins_sdex; > > }; > > struct brcmstb_match_priv { > > void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios); > > void (*cfginit)(struct sdhci_host *host); > > + int (*init_sd_express)(struct mmc_host *mmc, struct mmc_ios *ios); > > struct sdhci_ops *ops; > > const unsigned int flags; > > }; > > @@ -263,6 +270,105 @@ static void sdhci_brcmstb_cfginit_2712(struct sdhci_host *host) > > } > > } > > +static int bcm2712_init_sd_express(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_brcmstb_priv *brcmstb_priv = sdhci_pltfm_priv(pltfm_host); > > + struct device *dev = host->mmc->parent; > > + u32 ctrl_val; > > + u32 present_state; > > + int ret; > > + > > + if (!brcmstb_priv->sde_ioaddr || !brcmstb_priv->sde_ioaddr2) > > + return -EINVAL; > > + > > + if (!brcmstb_priv->pinctrl) > > + return -EINVAL; > > + > > + /* Turn off the SD clock first */ > > + sdhci_set_clock(host, 0); > > + > > + /* Disable SD DAT0-3 pulls */ > > + pinctrl_select_state(brcmstb_priv->pinctrl, brcmstb_priv->pins_sdex); > > + > > + ctrl_val = readl(brcmstb_priv->sde_ioaddr); > > + dev_dbg(dev, "ctrl_val 1 %08x\n", ctrl_val); > > + > > + /* Tri-state the SD pins */ > > + ctrl_val |= 0x1ff8; > > No magic values please. Ack. > > > + writel(ctrl_val, brcmstb_priv->sde_ioaddr); > > + dev_dbg(dev, "ctrl_val 1->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr)); > > + /* Let voltages settle */ > > + udelay(100); > > Why not usleep_range()? No real reason. I assume only the lower boundary is critical so I can use usleep_range instead. Will be fixed in a future patch, the SD express support will be drpped in V2 since nto strictly necessary. > > > + > > + /* Enable the PCIe sideband pins */ > > + ctrl_val &= ~0x6000; > > No magic values please. > > > + writel(ctrl_val, brcmstb_priv->sde_ioaddr); > > + dev_dbg(dev, "ctrl_val 1->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr)); > > + /* Let voltages settle */ > > + udelay(100); > > Likewise. Ditto. > > > + > > + /* Turn on the 1v8 VDD2 regulator */ > > + ret = regulator_enable(brcmstb_priv->sde_1v8); > > + if (ret) > > + return ret; > > + > > + /* Wait for Tpvcrl */ > > + msleep(1); > > + > > + /* Sample DAT2 (CLKREQ#) - if low, card is in PCIe mode */ > > + present_state = sdhci_readl(host, SDHCI_PRESENT_STATE); > > + present_state = (present_state & SDHCI_DATA_LVL_MASK) >> SDHCI_DATA_LVL_SHIFT; > > + dev_dbg(dev, "state = 0x%08x\n", present_state); > > + > > + if (present_state & BIT(2)) { > > Likewise, replace with constant. Ack. > > > + dev_err(dev, "DAT2 still high, abandoning SDex switch\n"); > > + return -ENODEV; > > + } > > + > > + /* Turn on the LCPLL PTEST mux */ > > + ctrl_val = readl(brcmstb_priv->sde_ioaddr2 + 20); // misc5 > > + ctrl_val &= ~(0x7 << 7); > > + ctrl_val |= 3 << 7; > > + writel(ctrl_val, brcmstb_priv->sde_ioaddr2 + 20); > > + dev_dbg(dev, "misc 5->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr2 + 20)); > > + > > + /* PTEST diff driver enable */ > > + ctrl_val = readl(brcmstb_priv->sde_ioaddr2); > > + ctrl_val |= BIT(21); > > + writel(ctrl_val, brcmstb_priv->sde_ioaddr2); > > + > > + dev_dbg(dev, "misc 0->%08x (%08x)\n", ctrl_val, readl(brcmstb_priv->sde_ioaddr2)); > > + > > + /* Wait for more than the minimum Tpvpgl time */ > > + msleep(100); > > + > > + if (brcmstb_priv->sde_pcie) { > > + struct of_changeset changeset; > > + static struct property okay_property = { > > + .name = "status", > > + .value = "okay", > > + .length = 5, > > + }; > > + > > + /* Enable the pcie controller */ > > + of_changeset_init(&changeset); > > + ret = of_changeset_update_property(&changeset, > > + brcmstb_priv->sde_pcie, > > + &okay_property); > > + if (ret) { > > + dev_err(dev, "%s: failed to update property - %d\n", __func__, > > + ret); > > + return -ENODEV; > > + } > > + ret = of_changeset_apply(&changeset); > > + } > > Why are you doing this? Cannot the firmware enable/disable the node > according to the boot mode or something else? > > This is not going to fly for upstream, sorry. > -- > Florian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel