Re: [PATCH v2] mmc: sdhci-sirf: fix 8bit width enable by overwriting set_bus_width

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2014-08-20 22:25 GMT+08:00 Romain Izard <romain.izard.pro@xxxxxxxxx>:
> ["Followup-To:" header set to gmane.linux.kernel.mmc.]
> On 2014-08-19, Barry Song <21cnbao@xxxxxxxxx> wrote:
>> From: Minda Chen <Minda.Chen@xxxxxxx>
>>
>> 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 <Minda.Chen@xxxxxxx>
>> Signed-off-by: Barry Song <Baohua.Song@xxxxxxx>
>> ---
>>  -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);
> +}
> +

hi Romain,
thanks very much! Minda will double-check the HW behaviour and confirm
what you said.

>
> You can use it verbatim with my signoff if you want.
> Signed-off-by: Romain Izard <romain.izard.pro@xxxxxxxxx>
>
>
> 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
>
-barry
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux