From patchwork Thu Mar 28 22:27:28 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Neil Greatorex X-Patchwork-Id: 2359921 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork2.kernel.org (Postfix) with ESMTP id 7CA16DF2A1 for ; Thu, 28 Mar 2013 22:34:00 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ULLIP-0002t7-EU; Thu, 28 Mar 2013 22:27:45 +0000 Received: from mail-wg0-x229.google.com ([2a00:1450:400c:c00::229]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1ULLIL-0002sb-Vu for linux-arm-kernel@lists.infradead.org; Thu, 28 Mar 2013 22:27:43 +0000 Received: by mail-wg0-f41.google.com with SMTP id y10so61742wgg.2 for ; Thu, 28 Mar 2013 15:27:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fatboyfat.co.uk; s=google; h=x-received:date:from:x-x-sender:to:cc:subject:in-reply-to :message-id:references:user-agent:mime-version:content-type; bh=PqP1NQNF1CqgdGDPink6+92YuEfT/if9ph4Zodj2RCQ=; b=bycLV/2ZNdHh+fxrCApBp4q/wpC1PdPqX+pehSrACzJthGy/ZtGud93pYZqgZmsRQx GrzVy5f2ybdivUaSBQFec3bXzTeTD86QZa5jv5Xxplm+9T9q2WwYOyt+0t6e8BsW7Fwe CZeWcAwZ/BJtoOebjudVY6dx9aOlbgtfoE1cs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:date:from:x-x-sender:to:cc:subject:in-reply-to :message-id:references:user-agent:mime-version:content-type :x-gm-message-state; bh=PqP1NQNF1CqgdGDPink6+92YuEfT/if9ph4Zodj2RCQ=; b=ms3NLhlKCV9oNIqnIW59iOtoQqLFDUV3POF0yhxEPJVgkRjK4fWXZAP7N4KMhUl5aW RoSj46RRsotj5VsEhSk8GsJ9KqbSVLmZqI95RBwT2IPV8uChWVl/eUyFHcv23RtUtrwX 29yQH4WFirq+if8xLOu9g9uUG8FUIRVlysdydU81BBzkoMu8grCsXUppSUNnjrnU4Nez IZUXDbrmkGA8evukii/L75p4Rz1aD+Vz8Tgo2tmf5TI40bQ/y1nC/bcFc11lRhimJw+G Bw0973c/Di+gzOs4YKi3RMhgN6yQe321TbDMFYBLADpsowcE8M18g4EmcKCTbiwT+LGX S8mw== X-Received: by 10.194.22.5 with SMTP id z5mr493383wje.5.1364509659900; Thu, 28 Mar 2013 15:27:39 -0700 (PDT) Received: from vroombuntu.lan ([151.230.237.226]) by mx.google.com with ESMTPS id fp2sm16805165wib.7.2013.03.28.15.27.38 (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 28 Mar 2013 15:27:38 -0700 (PDT) Date: Thu, 28 Mar 2013 22:27:28 +0000 (GMT) From: Neil Greatorex X-X-Sender: neil@vroombuntu To: Thomas Petazzoni Subject: Re: [PATCH v3 for 3.10] Introduce a Marvell EBU MBus driver In-Reply-To: <20130328083306.3c2a23de@skate> Message-ID: References: <1363885160-6870-1-git-send-email-thomas.petazzoni@free-electrons.com> <201303211730.23247.arnd@arndb.de> <20130321184641.GB13280@titan.lakedaemon.net> <20130328083306.3c2a23de@skate> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 X-Gm-Message-State: ALoCoQl/r7ktlHAKAeNLTJItn4jRPjFqzcABDGMQTcElZ1+1PtASIzFlLqJvmrzmtetIJ0463nzE X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130328_182742_275783_9DE07944 X-CRM114-Status: GOOD ( 34.03 ) X-Spam-Score: -2.0 (--) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-2.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature Cc: Lior Amsalem , Andrew Lunn , Jason Cooper , Arnd Bergmann , Olof Johansson , Maen Suleiman , Ezequiel Garcia , Gregory Clement , linux-arm-kernel , Sebastian Hesselbarth 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+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Thomas, On Thu, 28 Mar 2013, Thomas Petazzoni wrote: > Dear Neil Greatorex, > > On Thu, 28 Mar 2013 01:32:11 +0000, Neil Greatorex wrote: > >> This set of patches seems to break the mvsdio driver (and hence >> mwifiex_sdio) on the Globalscale Mirabox. I believe this is because >> mvsdio.c contains a function mv_conf_mbus_windows that seems to >> conflict with the new mbus driver. I don't understand enough about the >> hardware (without the datasheets anyway) to be able to understand the >> exact problem, but I hope that this is enough to point you (or someone >> else) to it! > > Thanks for reporting this problem. As Ryan Press said in reply to your > e-mail, it is well known that the mvsdio driver doesn't handle DMA > properly with SDIO devices that are not SD cards, and this, regardless > of whether the mvebu-mbus driver patches are applied or not. > > Therefore, are you sure that this problem is really introduced by the > mvebu-mbus driver, and doesn't exist without applying the patches? > > The mv_conf_mbus_windows() function of the mvsdio driver doesn't > conflict with the mvebu-mbus driver. What the mvsdio driver does is > request the list of DRAM chip-selects using mv_mbus_dram_info(), and > configure the SDIO unit with those informations in > mv_conf_mbus_windows(). But yes, the list of DRAM chip-selects is > provided by mv_mbus_dram_info() and this function has changed by the > introduction of the mvebu-mbus driver. > > I'm on the move right now, so I don't have hardware around to test > this. Would it be possible for you to test the Mirabox with the > following patch applied on mvsdio, and do a run with and without the > patch series that introduces mvebu-mbus? The idea is that the > informations listed should be identical. If not, then indeed there is > potentially a problem with the mvebu-mbus driver. > Thanks for your information. I have managed to track down the bug. It seems that two crucial lines were missed in the conversion to the mvebu-mbus driver - the lines that set the hw_io_coherency flag if a marvell,coherency-fabric compatible node is in the device tree. This leads to the differences below that I found using your printk patch: Before mvebu-mbus patches: [SD] root@mirabox:~# modprobe mvsdio [ 134.595899] mvsdio: CS[0], base = 0x0, size = 0x20000000, mbus_attr = 0x1e [ 134.602821] mvsdio: CS[1], base = 0x20000000, size = 0x20000000, mbus_attr = 0x1d [ 134.643933] mmc0: mvsdio driver initialized, lacking card detect (fall back to polling) [ 134.660814] mmc0: new high speed SDIO card at address 0001 After mvebu-mbus patches: [SD] root@mirabox:~# modprobe mvsdio [ 107.462881] mvsdio: CS[0], base = 0x0, size = 0x20000000, mbus_attr = 0xe [ 107.469766] mvsdio: CS[1], base = 0x20000000, size = 0x20000000, mbus_attr = 0xd [ 107.516219] mmc0: mvsdio driver initialized, lacking card detect (fall back to polling) [ 107.533029] mmc0: new high speed SDIO card at address 0001 Here is a patch that restores the behaviour to how it was before the mbus patches, but it obviously needs testing on other hardware than the Mirabox. It has been based on your marvell-mvebu-mbus-v3 branch from Github: -- >8 -- Subject: [PATCH] bus: mvebu-mbus: Restore checking for coherency fabric hardware The new mvebu-mbus driver was not checking the device tree for coherency fabric hardware and hence was not setting the hw_io_coherency flag in mbus_state. This prevented the mvsdio driver from operating correctly. This patch restores the check. --- drivers/bus/mvebu-mbus.c | 3 +++ 1 file changed, 3 insertions(+) mvebu_mbus_disable_window(mbus, win); diff --git a/drivers/bus/mvebu-mbus.c b/drivers/bus/mvebu-mbus.c index 586d03e..a0250c6 100644 --- a/drivers/bus/mvebu-mbus.c +++ b/drivers/bus/mvebu-mbus.c @@ -858,6 +858,9 @@ int __init mvebu_mbus_init(const char *soc, phys_addr_t mbuswins_phys_base, return -ENOMEM; } + if (of_find_compatible_node(NULL, NULL, "marvell,coherency-fabric")) + mbus->hw_io_coherency = 1; + for (win = 0; win < mbus->soc->num_wins; win++)