All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peng Fan <van.freenix@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model
Date: Sat, 12 Mar 2016 12:21:39 +0800	[thread overview]
Message-ID: <20160312042137.GA7333@linux-7smt.suse> (raw)
In-Reply-To: <CAPnjgZ14LvMa-3tt18zBmVQSy0Hx7LbCge=H_vP0GGbZnR2N9w@mail.gmail.com>

Hi Simon,

On Fri, Mar 11, 2016 at 05:33:05PM -0700, Simon Glass wrote:
>Hi Peng,
>
>On 10 March 2016 at 01:57, Peng Fan <van.freenix@gmail.com> wrote:
>> Support Driver Model for fsl esdhc driver.
>>
>> In order to minimize the change, reuse the fsl_esdhc_initialize function.
>> This new way is to fill an fsl_esdhc_cfg struture and pass it
>> to fsl_esdhc_initialize, just like the code in different board codes.
>>
>> Introduce a 'struct mmc *mmc' entry in fsl_esdhc_cfg structure,
>> otherwise fsl_esdhc_initialize may need to be restructured which will
>> cause lots changes for board code.
>>
>> Since clk driver is not implemented, use mxc_get_clock here to
>> fill cfg->sdhc_clk. Anyway we can utilize the pinctrl imx driver
>> now, except the SPL part, we can drop the pinmux settings from
>> board file for mmc.
>>
>> There are so many "ifdef" in the file, maybe we can use driver data
>> or quirks to cover these. But to minimize changes for this patch,
>> these are not included. Later we can try to discard the nasty
>> ifdef.
>>
>> Has been tested on i.MX6UL 9x9 EVK board:
>> "
>> =>dm tree
>> ....
>>  simple_bus  [ + ]    |   `-- aips-bus at 02100000
>>   mmc        [ + ]    |       |-- usdhc at 02190000
>>   mmc        [ + ]    |       |-- usdhc at 02194000
>> ....
>> => mmc list
>> FSL_SDHC: 0 (SD)
>> FSL_SDHC: 1 (SD)
>> "
>>
>> Signed-off-by: Peng Fan <van.freenix@gmail.com>
>> Cc: York Sun <york.sun@nxp.com>
>> Cc: Yangbo Lu <yangbo.lu@nxp.com>
>> Cc: Hector Palacios <hector.palacios@digi.com>
>> Cc: Eric Nelson <eric@nelint.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> ---
>>  drivers/mmc/fsl_esdhc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  include/fsl_esdhc.h     |  1 +
>>  2 files changed, 82 insertions(+)
>
>I'm nervous about this patch. It is calling board code from a driver,
>which we should avoid.

No. I may need to write more in the commit log.
The current way without driver model is
board code fill fsl_esdhc_cfg strucure and then call fsl_esdhc_initialize
which is in driver/mmc/fsl_esdhc.c.

In order to minimize changes, in this patch, the probe function
will fill fsl_esdhc_cfg structure and call fsl_esdhc_initialize.

Then we can drop the function call to fsl_esdhc_initialize in different board code.

I originally want to rewrite fsl_esdhc_cfg structure, may give it a new name
such as fsl_esdhc_platdata or priv, but this may incur lots changes in
board code, because board code also refers to this structure. So I keep in.

>Perhaps it would be better to start by refactoring things to fix the
>problems you mention, and then come back to this patch? I'm worried
>we'll end up with a driver model conversion that never gets properly
>finished if we do things in the wrong order.

Please kind take my upper words. I am still thinking add "non-removable",
"cd-gpios" related code in this patch, so I'll write a new version patch. 

Thanks,
Peng.

>
>Regards,
>Simon

  reply	other threads:[~2016-03-12  4:21 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-10  8:57 [U-Boot] [PATCH 1/2] fsl: esdhc: consolidate fsl_esdhc_cfg structure Peng Fan
2016-03-10  8:57 ` [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model Peng Fan
2016-03-12  0:33   ` Simon Glass
2016-03-12  4:21     ` Peng Fan [this message]
2016-03-13  2:51       ` Simon Glass
2016-03-13  9:22         ` Peng Fan

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=20160312042137.GA7333@linux-7smt.suse \
    --to=van.freenix@gmail.com \
    --cc=u-boot@lists.denx.de \
    /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.