From patchwork Mon Mar 24 13:28:05 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sekhar Nori X-Patchwork-Id: 26928 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-pa0-f70.google.com (mail-pa0-f70.google.com [209.85.220.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id A61F820143 for ; Mon, 24 Mar 2014 13:28:55 +0000 (UTC) Received: by mail-pa0-f70.google.com with SMTP id lj1sf14659796pab.9 for ; Mon, 24 Mar 2014 06:28:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:date:from:user-agent :mime-version:to:subject:references:in-reply-to:cc:precedence :list-id:list-unsubscribe:list-archive:list-post:list-help :list-subscribe:sender:errors-to:x-original-sender :x-original-authentication-results:mailing-list:content-type :content-transfer-encoding; bh=ftISlwIVHWCjvuDz432LR9lcm0VDqNKjReXm4/AVc/A=; b=RBv0oL9I17CKmm8iVWlsS0RjFXN/hAUvOT105BjWyvdoLqmzQ88qnj4qgf19lSR20E W4IsAR+VY02IgFM1VUlLKqM2lXclXLY3oDCkpP1pikA1qXPCk/zrATQsNkDt0R3wjenP 1OxTiBiAwuE0HFJvcOiUilCHmzvtV1lCu7Nvw7aEG7BHF8vSaMkOLyN3qSPSztf4Lfz0 dmOwYiNm2BpdiOV7F5pYPIaryOUjD5W3xHNGcXVEVbGMv3t5bljzFD6tlfwwajwC85bt z7/OsZXTZXx0D/4h8Ia+Hr74fKAUIcNA+p1PHVFG6+i11ZPsK/BkES9MowuTPrc4oTTA aMvw== X-Gm-Message-State: ALoCoQkWjYB0cYaVUDuJMr94lu3Hx03JvbeFoajfzyq5KYm9uoWvyZTXUEWOaactIP0igM4y+cT+ X-Received: by 10.66.227.103 with SMTP id rz7mr24672798pac.37.1395667734760; Mon, 24 Mar 2014 06:28:54 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.97.166 with SMTP id m35ls53996qge.87.gmail; Mon, 24 Mar 2014 06:28:54 -0700 (PDT) X-Received: by 10.52.18.70 with SMTP id u6mr43613169vdd.11.1395667734552; Mon, 24 Mar 2014 06:28:54 -0700 (PDT) Received: from mail-vc0-f176.google.com (mail-vc0-f176.google.com [209.85.220.176]) by mx.google.com with ESMTPS id 2si2987228vcd.95.2014.03.24.06.28.54 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 24 Mar 2014 06:28:54 -0700 (PDT) Received-SPF: neutral (google.com: 209.85.220.176 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) client-ip=209.85.220.176; Received: by mail-vc0-f176.google.com with SMTP id lc6so5772675vcb.35 for ; Mon, 24 Mar 2014 06:28:54 -0700 (PDT) X-Received: by 10.220.99.72 with SMTP id t8mr50756459vcn.10.1395667734373; Mon, 24 Mar 2014 06:28:54 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.220.78.9 with SMTP id i9csp222761vck; Mon, 24 Mar 2014 06:28:53 -0700 (PDT) X-Received: by 10.180.19.69 with SMTP id c5mr14643006wie.7.1395667732829; Mon, 24 Mar 2014 06:28:52 -0700 (PDT) Received: from casper.infradead.org (casper.infradead.org. [2001:770:15f::2]) by mx.google.com with ESMTPS id gd7si85660wic.6.2014.03.24.06.28.52 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 24 Mar 2014 06:28:52 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 2001:770:15f::2 as permitted sender) client-ip=2001:770:15f::2; Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WS4vi-0001BN-V5; Mon, 24 Mar 2014 13:28:43 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WS4vg-00088f-IC; Mon, 24 Mar 2014 13:28:40 +0000 Received: from arroyo.ext.ti.com ([192.94.94.40]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WS4vd-00087y-IG for linux-arm-kernel@lists.infradead.org; Mon, 24 Mar 2014 13:28:38 +0000 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id s2ODSAKF004439; Mon, 24 Mar 2014 08:28:10 -0500 Received: from DLEE71.ent.ti.com (dlee71.ent.ti.com [157.170.170.114]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id s2ODSA60027047; Mon, 24 Mar 2014 08:28:10 -0500 Received: from dlep32.itg.ti.com (157.170.170.100) by DLEE71.ent.ti.com (157.170.170.114) with Microsoft SMTP Server id 14.3.174.1; Mon, 24 Mar 2014 08:28:09 -0500 Received: from [172.24.190.153] (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep32.itg.ti.com (8.14.3/8.13.8) with ESMTP id s2ODS5si030880; Mon, 24 Mar 2014 08:28:06 -0500 Message-ID: <533032E5.2090004@ti.com> Date: Mon, 24 Mar 2014 18:58:05 +0530 From: Sekhar Nori User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: Bartlomiej Zolnierkiewicz , Tejun Heo Subject: Re: [PATCH v2 3/3] ata: add new-style AHCI platform driver for DaVinci DA850 AHCI controller References: <1395340026-3969-1-git-send-email-b.zolnierkie@samsung.com> <1395340026-3969-4-git-send-email-b.zolnierkie@samsung.com> In-Reply-To: <1395340026-3969-4-git-send-email-b.zolnierkie@samsung.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140324_092837_687434_277E6751 X-CRM114-Status: GOOD ( 25.21 ) X-Spam-Score: -7.4 (-------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-7.4 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 SPF_PASS SPF: sender matches SPF record -0.5 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [192.94.94.40 listed in list.dnswl.org] -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: davinci-linux-open-source@linux.davincidsp.com, linux-ide@vger.kernel.org, Kevin Hilman , Shiraz Hashim , spear-devel@list.st.com, linux-kernel@vger.kernel.org, Hans de Goede , Viresh Kumar , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: nsekhar@ti.com X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.220.176 is neither permitted nor denied by best guess record for domain of patch+caf_=patchwork-forward=linaro.org@linaro.org) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 On Thursday 20 March 2014 11:57 PM, Bartlomiej Zolnierkiewicz wrote: > Add the new ahci_da850 host driver and remove the deprecated > ahci_platform_data platform code. > > Please note that the new driver doesn't have the superfluous > clock control code as clock is already handled by the generic > AHCI platform library code. > > Cc: Sekhar Nori > Cc: Kevin Hilman > Cc: Hans de Goede > Signed-off-by: Bartlomiej Zolnierkiewicz Looks much better now and re-tested it on DA850 EVM. Few issues pointed out below. > --- > v2: > - dropped the experimental tag from the config option help > - fixed SYSCFG1 memory resource handling > - hardcoded the multiplier data and added a note about it > - used readl()/writel() instead of __raw_readl()/__raw_writel() > - dropped the superflous clock control > - cleaned up da850_sata_init() > - changed the platform device name and removed the platform ids table > - removed the deprecated ahci_platform_data platform code > - updated the patch description > > arch/arm/mach-davinci/devices-da8xx.c | 99 +++-------------------------- > drivers/ata/Kconfig | 9 +++ > drivers/ata/Makefile | 1 + > drivers/ata/ahci_da850.c | 116 ++++++++++++++++++++++++++++++++++ > 4 files changed, 134 insertions(+), 91 deletions(-) I prefer not to mix platform and driver together in one patch. If you separate out the platform changes, I can take then through my tree with out risk of conflicts. The platform changes can come after the driver is introduced so there is no breakage. > create mode 100644 drivers/ata/ahci_da850.c > > diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c > index 0486cdf2..72bb8d6 100644 > --- a/arch/arm/mach-davinci/devices-da8xx.c > +++ b/arch/arm/mach-davinci/devices-da8xx.c > @@ -1020,7 +1020,6 @@ int __init da8xx_register_spi_bus(int instance, unsigned num_chipselect) > } > > #ifdef CONFIG_ARCH_DAVINCI_DA850 > - > static struct resource da850_sata_resources[] = { > { > .start = DA850_SATA_BASE, > @@ -1028,103 +1027,22 @@ static struct resource da850_sata_resources[] = { > .flags = IORESOURCE_MEM, > }, > { > + .start = DA8XX_SYSCFG1_BASE, > + .end = DA8XX_SYSCFG1_BASE + SZ_4K - 1, > + .flags = IORESOURCE_MEM, This is not correct. The entire SYSCFG is not owned by SATA driver. Its just that one PWRDN register. Here is a patch which applies on top of your patch patch fixing it. Feel free to roll it in. ---8<--- --- Thanks, Sekhar diff --git a/arch/arm/mach-davinci/devices-da8xx.c b/arch/arm/mach-davinci/devices-da8xx.c index 72bb8d6..56ea41d 100644 --- a/arch/arm/mach-davinci/devices-da8xx.c +++ b/arch/arm/mach-davinci/devices-da8xx.c @@ -1027,8 +1027,8 @@ static struct resource da850_sata_resources[] = { .flags = IORESOURCE_MEM, }, { - .start = DA8XX_SYSCFG1_BASE, - .end = DA8XX_SYSCFG1_BASE + SZ_4K - 1, + .start = DA8XX_SYSCFG1_BASE + DA8XX_PWRDN_REG, + .end = DA8XX_SYSCFG1_BASE + DA8XX_PWRDN_REG + 0x3, .flags = IORESOURCE_MEM, }, { diff --git a/drivers/ata/ahci_da850.c b/drivers/ata/ahci_da850.c index 899270a..2c83613 100644 --- a/drivers/ata/ahci_da850.c +++ b/drivers/ata/ahci_da850.c @@ -16,8 +16,6 @@ #include #include "ahci.h" -#define DA8XX_PWRDN_REG 0x18 - /* SATA PHY Control Register offset from AHCI base */ #define SATA_P0PHYCR_REG 0x178 @@ -37,15 +35,15 @@ */ #define DA850_SATA_CLK_MULTIPLIER 7 -static void da850_sata_init(struct device *dev, void __iomem *syscfg1_base, +static void da850_sata_init(struct device *dev, void __iomem *pwrdn_reg, void __iomem *ahci_base) { unsigned int val; /* Enable SATA clock receiver */ - val = readl(syscfg1_base + DA8XX_PWRDN_REG); + val = readl(pwrdn_reg); val &= ~BIT(0); - writel(val, syscfg1_base + DA8XX_PWRDN_REG); + writel(val, pwrdn_reg); val = SATA_PHY_MPY(DA850_SATA_CLK_MULTIPLIER + 1) | SATA_PHY_LOS(1) | SATA_PHY_RXCDR(4) | SATA_PHY_RXEQ(1) | SATA_PHY_TXSWING(3) | @@ -66,7 +64,7 @@ static int ahci_da850_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct ahci_host_priv *hpriv; struct resource *res; - void __iomem *syscfg1_base; + void __iomem *pwrdn_reg; int rc; hpriv = ahci_platform_get_resources(pdev); @@ -81,11 +79,11 @@ static int ahci_da850_probe(struct platform_device *pdev) if (!res) goto disable_resources; - syscfg1_base = devm_ioremap(dev, res->start, resource_size(res)); - if (!syscfg1_base) + pwrdn_reg = devm_ioremap(dev, res->start, resource_size(res)); + if (!pwrdn_reg) goto disable_resources; - da850_sata_init(dev, syscfg1_base, hpriv->mmio); + da850_sata_init(dev, pwrdn_reg, hpriv->mmio); rc = ahci_platform_init_host(pdev, hpriv, &ahci_da850_port_info, 0, 0); if (rc) --- Also, there is an additional change required in platform side to make sure clock look-up succeeds. Here is the change needed, please roll it into your platform changes patch. ---8<--- diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c index 2ab0043..85399c9 100644 --- a/arch/arm/mach-davinci/da850.c +++ b/arch/arm/mach-davinci/da850.c @@ -472,7 +472,7 @@ static struct clk_lookup da850_clks[] = { CLK("spi_davinci.0", NULL, &spi0_clk), CLK("spi_davinci.1", NULL, &spi1_clk), CLK("vpif", NULL, &vpif_clk), - CLK("ahci", NULL, &sata_clk), + CLK("ahci_da850", NULL, &sata_clk), CLK("davinci-rproc.0", NULL, &dsp_clk), CLK("ehrpwm", "fck", &ehrpwm_clk), CLK("ehrpwm", "tbclk", &ehrpwm_tbclk),