All of lore.kernel.org
 help / color / mirror / Atom feed
From: Philip Rakity <prakity@marvell.com>
To: zhangfei gao <zhangfei.gao@gmail.com>
Cc: Wolfram Sang <w.sang@pengutronix.de>, Chris Ball <cjb@laptop.org>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	Jun Nie <njun@marvell.com>, Raymond Wu <xywu@marvell.com>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	"arnd@arndb.de Bergmann" <arnd@arndb.de>,
	Mark Brown <mark.brown314@gmail.com>
Subject: Re: [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa
Date: Mon, 16 May 2011 21:27:48 -0700	[thread overview]
Message-ID: <08119C91-F5AA-4E6A-99D4-CFB253B1AE7B@marvell.com> (raw)
In-Reply-To: <BANLkTi=+wXUgP2hQH1q1mZKF9gRjKYP2vA@mail.gmail.com>


On May 16, 2011, at 7:02 PM, zhangfei gao wrote:

> On Mon, May 16, 2011 at 9:51 AM, Philip Rakity <prakity@marvell.com> wrote:
>> 
>> On May 15, 2011, at 11:26 PM, zhangfei gao wrote:
>> 
>>> On Sat, May 14, 2011 at 1:01 PM, Philip Rakity <prakity@marvell.com> wrote:
>>>> 
>>>> On May 13, 2011, at 10:11 PM, zhangfei gao wrote:
>>>> 
>>>>> On Fri, May 13, 2011 at 9:47 PM, Chris Ball <cjb@laptop.org> wrote:
>>>>>> Hi,
>>>>>> 
>>>>>> On Thu, May 12 2011, Philip Rakity wrote:
>>>>>>> All other platform specific code is in the host/ directory.
>>>>>>> 
>>>>>>> This moves it to arch/arm
>>>>>>> 
>>>>>>> If that is the direction the group wants to go in --> then the patch
>>>>>>> is fine provided the mmc group can review the patches.  Otherwise they
>>>>>>> are handled by the arm maintainer.
>>>>>> 
>>>>>> Thanks.  Wolfram, do you have any ideas on what the best design is for
>>>>>> these SoC-specific changes to sdhci-pxa?
>>>>>> 
>>>>>> - Chris.
>>>>> 
>>>>> The code in arch/arm is
>>>>> 1. Accessing private register, take pxa910 and mmp2 we want to support
>>>>> as example, there are several private registers differece, though they
>>>>> are same ip, with same issues and quirk.
>>>>> 2. Handle platform difference, for example, mmp2 used in two different
>>>>> platform, one use wp pin, the other does not.
>>>> 
>>>> The situation is a little more complicated.
>>> 
>>> The interface is used for long time among mmp2, pxa910 and mmp3.
>>> Also could be used for new controller with minor register difference
>>> but same ip, without adding new specific driver.
>> 
>> applies to both approaches.  The mmp2 specific code can be applied to other marvell
>> platforms that share the same controller.  Just change Kconfig and the Makefile to use
>> the mmp2 code.  The name of the file is not the important thing.  It is what it does.
>> 
>> The pxa168 and pxa910 code are a little less sharable due to io accessors and some
>> other quirks.
>> 
>> For other following the discussion we are probably talking about 100 liens of code.
>> 
>> The philosophical location of where the code belongs for host specific drivers is
>> to me more important.
>> 
>> Is it in arch/arm and not directly visible to the mmc list or is it like all other platform drivers
>> in drivers/mmc/host.
> 
> The code handle several register difference are located at
> arch/arm/mach-mmp/mmp2.c is for mmp2
> arch/arm/mach-mmp/pxa910.c is for pxa910 serious,
> arch/arm/mach-mmp/mmp3.c is for mmp3 serious, considering there may
> still registers changing.
> 
> The board difference are directly put in board.c
> For example ttc_dkb do not use wp pin, so get_ro is provided.

embedding the code in these chip files stops code sharing.  For example,
mmp3 has same controller as mmp2. 
> 
>> 
>> 
>> 
>>> 
>>>> 
>>>> pxa168 and pxa910 share (almost) the same IP  The are both based on SD 2.0
>>>> controller spec with extensions.  The pxa910 controller has fixes to the
>>>> pxa168 controller.  They share the same private registers that allow support
>>>> for clock gating and timing adjustments.
>>>> 
>>>> mmp2 is based on SD 3.0 spec.  The private register space is different.
>>>> 
>>>> mmc/host/Kconfig takes no account of these differences.  mmp2 and pxa168/910
>>>> cannot co-exist.   What is currently submitted does not work.  One cannot
>>>> compile mmp2 and pxa910  nor would they work if one could.
>>>> 
>>>> Mark Brown and I submitted patches to fix this.  We added code to the host/ directory
>>>> that took into account these differences.   It provided a common interface layer
>>>> that then used platform specific code in the host/ directory to handle the different
>>>> behavior.
>>>> 
>>>> Arng Bergmann provided advice and reviewed the patches to allow explicit selection of the
>>>> pxa familty controller and board.  Based on this comments we submitted a patch
>>>> to allow selection if the appropriate SoC.
>>>> 
>>>> These are two approaches.
>>>> 
>>>>> 
>>>>> Thanks
>>>>>> --
>>>>>> Chris Ball   <cjb@laptop.org>   <http://printf.net/>
>>>>>> One Laptop Per Child
>>>>>> 
>>>> 
>>>> 
>> 
>> 


  reply	other threads:[~2011-05-17  4:28 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-29  3:57 [PATCH 2/2] sdhci-pxa: add call back interface to share sdhci-pxa zhangfei gao
2011-05-12 22:25 ` Chris Ball
2011-05-12 22:58   ` Philip Rakity
2011-05-13 13:47     ` Chris Ball
2011-05-14  5:11       ` zhangfei gao
2011-05-14 17:01         ` Philip Rakity
2011-05-15 21:32           ` Philip Rakity
2011-05-16  6:26           ` zhangfei gao
2011-05-16 13:51             ` Philip Rakity
2011-05-17  2:02               ` zhangfei gao
2011-05-17  4:27                 ` Philip Rakity [this message]
2011-05-17  5:39                   ` zhangfei gao
2011-05-18 20:38                     ` Arnd Bergmann
2011-05-19 11:34                       ` zhangfei gao
2011-05-19 13:04                         ` Arnd Bergmann
2011-05-23 13:13                           ` zhangfei gao
2011-05-23 14:56                             ` Arnd Bergmann
2011-05-19 18:24                         ` Nicolas Pitre
2011-05-21  1:50                           ` zhangfei gao
  -- strict thread matches above, loose matches on Subject: below --
2010-12-03  6:31 [PATCH 2/2] sdhci-pxa " zhangfei gao
2010-12-07  5:56 ` Raymond Wu
2010-12-07 15:26   ` Philip Rakity
2010-12-07 15:38     ` zhangfei gao
2010-12-07 15:48       ` Philip Rakity
2010-12-07 15:58         ` zhangfei gao
2010-12-27  7:09 ` zhangfei gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=08119C91-F5AA-4E6A-99D4-CFB253B1AE7B@marvell.com \
    --to=prakity@marvell.com \
    --cc=arnd@arndb.de \
    --cc=cjb@laptop.org \
    --cc=haojian.zhuang@gmail.com \
    --cc=linux-mmc@vger.kernel.org \
    --cc=mark.brown314@gmail.com \
    --cc=njun@marvell.com \
    --cc=w.sang@pengutronix.de \
    --cc=xywu@marvell.com \
    --cc=zhangfei.gao@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.