From mboxrd@z Thu Jan 1 00:00:00 1970 From: Romain Izard Subject: Re: [PATCH v2] mmc: sdhci-sirf: fix 8bit width enable by overwriting set_bus_width Date: Wed, 20 Aug 2014 14:25:23 +0000 (UTC) Message-ID: References: <1408453153-8125-1-git-send-email-21cnbao@gmail.com> Return-path: Received: from plane.gmane.org ([80.91.229.3]:52872 "EHLO plane.gmane.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750703AbaHTOZx (ORCPT ); Wed, 20 Aug 2014 10:25:53 -0400 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1XK6pV-0004Aj-FA for linux-mmc@vger.kernel.org; Wed, 20 Aug 2014 16:25:37 +0200 Received: from 146.187.3.109.rev.sfr.net ([109.3.187.146]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 20 Aug 2014 16:25:37 +0200 Received: from romain.izard.pro by 146.187.3.109.rev.sfr.net with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Wed, 20 Aug 2014 16:25:37 +0200 Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: linux-mmc@vger.kernel.org ["Followup-To:" header set to gmane.linux.kernel.mmc.] On 2014-08-19, Barry Song <21cnbao@gmail.com> wrote: > From: Minda Chen > > 8bit-width enable bit of CSR MMC hosts is 3, while stardard hosts use > bit 5. this patch fixes the functionality of 8bit transfer in CSR mmc > controllers and improve performance for mmc0 a lot. > > Signed-off-by: Minda Chen > Signed-off-by: Barry Song > --- > -v2: check for host->version >= SDHCI_SPEC_300 > Hi Barry, Did you check the runtime behaviour of the device after this change ? >>From the SiRFprimaII/SiRFatlasVI data sheets, it appears that the implementation of the SDHCI controller is a modified version of the one described in the 1.0 specification, and not a normal 3.0 controller. This explains why the independent development of the 8-bit wide transfer bus does not use the same bit as the standard one. As a result, the description of the SPEC_VER field in the SD_SLOT_INT_STATUS register indicates a read-only value of 0, which corresponds to SDHCI_SPEC_100 in the "sdhci.h" header. On a real SiRFatlasVI chip, the value is 0 as well. I believe the correct change would be to remove the control on the version >= SDHCI_SPEC_300 in both 4-bit and 8-bit cases, instead of adding it in both cases. There are no supported SiRF SoCs where the 8-bit bus is not supported at the controller level. In my opinion, this should look like this: +static void sdhci_sirf_set_bus_width(struct sdhci_host *host, int width) +{ + u8 ctrl; + + ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL); + /* The 8-bit bus width configuration bit is not standard */ + ctrl &= ~(SDHCI_CTRL_4BITBUS | SDHCI_SIRF_8BITBUS); + if (width == MMC_BUS_WIDTH_8) { + ctrl |= SDHCI_SIRF_8BITBUS; + } else if (width == MMC_BUS_WIDTH_4) + ctrl |= SDHCI_CTRL_4BITBUS; + } + sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL); +} + You can use it verbatim with my signoff if you want. Signed-off-by: Romain Izard But you should try it on your board, as I do not have a setup to test the mainline kernel on SiRFatlasVI available. Best regards, -- Romain Izard