Date:	Tue, 6 Oct 2015 11:18:13 +0200
From:	Ulf Hansson <ulf.hansson@...aro.org>
To:	Hongtao Wu <wuht06@...il.com>
Cc:	linux-mmc <linux-mmc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Paweł Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Shawn Lin <shawn.lin@...k-chips.com>,
	"Orson.Zhai@...eadtrum.com" <Orson.Zhai@...eadtrum.com>,
	"Chunyan.Zhang@...eadtrum.com" <Chunyan.Zhang@...eadtrum.com>,
	"Jason.Wu@...eadtrum.com" <Jason.Wu@...eadtrum.com>,
	Russell King <rmk+kernel@....linux.org.uk>
Subject: Re: [RFC PATCH v3 1/3] mmc: sprd: Add MMC host driver for Spreadtrum SoC

+Russell

On 28 September 2015 at 09:18, Hongtao Wu <wuht06@...il.com> wrote:
>>>
>
> On Thu, Sep 10, 2015 at 9:28 PM, Ulf Hansson <ulf.hansson@...aro.org> wrote:
>>
>> On 14 August 2015 at 18:55, Hongtao Wu <wuht06@...il.com> wrote:
>> > the Spreadtrum MMC host driver is used to supply EMMC, SD, and
>> > SDIO types of memory cards
>>
>> Perhaps some more information about the controller. Are there any
>> specific features it support or doesn't support!?
>
> Thanks for kindly reply.
> Yes, spreadtrum MMC host controllers have some specific features as follows:
> (1) We don't have controls for sampling clock tuning and re-tuning, we take
>       place of them with three registers as follows:
>        (a) CLK_WR_DL(Offset 080h): Data write clock delay line.
>        (b) CLK_RD_POS_DL(Offset 0x84h): Posedge data read clock delay line.
>        (c) CLK_RD_NEG_DL(Offset 088h): Negedge data read clock delay line.
>
> (2) We don't have Power Control Register(Offset 029h), all our controller's
> power
>       come from PMIC, rather than directly from CPU.
>
> (3) We don't have bit[6](Card Insertion), bit[7] (Card Removal), bit[8]
> (Card Interrupt)
> and bit[12:9] in Normal Interrupt Status Register(Offset 030h). Because the
> detect
> gpio pin doesn't connect to the register of our host controller. So we can't
> operate bit[18:16](Card Detect Pin Level, Card State stable and Card
> Inserted)
> in Present State Register(Offset 024h).

Thanks for clarifying!

You have some differences towards the "standard" sdhci variant, but
that doesn't mean you should go off and implement a new driver from
scratch, instead you should create a new sdhci variant and re-use code
from the generic sdhci driver.

The current problem with such approach, is that the sdhci driver isn't
designed as a library but instead a driver consisting of too many
quirks and callbacks. While you start to adopt your driver towards
sdhci, you will need to add yet another bunch of new quirks and
callbacks to suite your hw.

Now, as the number of callbacks and quirks continues to increase I
will sooner or later give up maintaining it, as each line of code will
depend on a quirk. So, we need to start turning sdhci into a library
*right now*! Russell King, has pointed out this several times as well,
but unfortunate I haven't yet seen anyone willing to help out in this
field.

I would of course be very happy if you would like to have a look at
that, but I realize it's a difficult task, So, unless you are happy
with taking on such a challenge, I suggest you go for an intermediate
step, which thus means convert your driver to a sdhci variant driver
and add the quirks/callbacks you need to suite your hw.

[...]

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists