From: Ulf Hansson <ulf.hansson@linaro.org>
To: Peter Griffin <peter.griffin@linaro.org>
Cc: "linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Maxime Coquelin <maxime.coquelin@st.com>,
Patrice CHOTARD <patrice.chotard@st.com>,
Srinivas Kandagatla <srinivas.kandagatla@gmail.com>,
Chris Ball <chris@printf.net>,
Ulf Hansson <ulf.hansson@linaro.org>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
kernel@stlinux.com, linux-mmc <linux-mmc@vger.kernel.org>,
Giuseppe Cavallaro <peppe.cavallaro@st.com>,
Lee Jones <lee.jones@linaro.org>
Subject: Re: [PATCH 1/8] mmc: sdhci-st: Intial support for ST SDHCI controller
Date: Wed, 4 Jun 2014 13:06:07 +0200 [thread overview]
Message-ID: <CAPDyKFqvQca9RfsVVyfeC9r=pEDM2mBkfS7SzpYZHHXMnHfyUQ@mail.gmail.com> (raw)
In-Reply-To: <20140604084831.GB23469@griffinp-ThinkPad-X1-Carbon-2nd>
On 4 June 2014 10:48, Peter Griffin <peter.griffin@linaro.org> wrote:
> Hi Ulf,
>
> Thanks for reviewing, see my comments below: -
>
> On Fri, 23 May 2014, Ulf Hansson wrote:
>> > +static unsigned int sdhci_st_get_max_clk(struct sdhci_host *host)
>> > +{
>> > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> > +
>> > + return clk_get_rate(pltfm_host->clk);
>> > +}
>>
>> There are sdhci library function for the above:
>> sdhci_pltfm_clk_get_max_clock()
>
> I've fixed in v2 to use the library function
>
>> > + host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_BUS_WIDTH_TEST
>> > + | MMC_CAP_1_8V_DDR;
>>
>> Comment below.
>>
>> > +
>> > + if (of_property_read_bool(np, "non-removable"))
>> > + host->mmc->caps |= MMC_CAP_NONREMOVABLE;
>>
>> MMC_CAP_1_8V_DDR, MMC_CAP_8_BIT_DATA, MMC_CAP_NONREMOVABLE aren’t
>> those board specific capabilities?
>
> Yep
>>
>> Unless there are something that prevents you from using the common mmc
>> DT parser, I would suggest you to use it. mmc_of_parse(). Thus you can
>> provide these capabilities through DT instead.
>
> Thanks for pointing that out, I've switched over to using mmc_of_parse,
> and everything works as expected.
>
> Also as an added bonus this will simplify support for the next SoC which
> needs access to the high speed ddr / sdr bindings which
> mmc_of_parse already supports :-)
>
> In v2 I've also removed the reset controller code from this platform driver
> with the intention of adding it back in, in the generic code. The idea
> would be that an additional 'reset' binding could be provided, which if
> present would be used to deassert the IP reset line of the controller at
> probe / resume, and assert reset at remove / sleep.
>
> Is this something you view as useful, if so I will prepare some RFC patches?
Sounds useful. Please go ahead and send patches! :-)
Kind regards
Uffe
next prev parent reply other threads:[~2014-06-04 11:06 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-22 15:18 Add SDHCI support for STMicroelectronics SoCs Peter Griffin
2014-05-22 15:18 ` [PATCH 1/8] mmc: sdhci-st: Intial support for ST SDHCI controller Peter Griffin
2014-05-22 15:41 ` Maxime Coquelin
2014-06-04 11:05 ` Peter Griffin
2014-05-22 16:50 ` Lee Jones
2014-06-02 11:06 ` Peter Griffin
2014-05-23 6:18 ` Srinivas Kandagatla
2014-06-04 8:37 ` Peter Griffin
2014-05-23 7:34 ` Ulf Hansson
[not found] ` <20140604084831.GB23469@griffinp-ThinkPad-X1-Carbon-2nd>
2014-06-04 11:06 ` Ulf Hansson [this message]
2014-05-22 15:18 ` [PATCH 2/8] mmc: sdhci-st: STMicroelectronics SDHCI binding documentation Peter Griffin
2014-05-22 15:46 ` Maxime Coquelin
2014-05-22 16:06 ` Lee Jones
2014-05-23 6:57 ` Ulf Hansson
2014-05-22 15:18 ` [PATCH 3/8] ARM: STi: DT: Add sdhci pins for stih416 Peter Griffin
2014-05-22 15:50 ` Maxime Coquelin
2014-05-22 15:59 ` Lee Jones
2014-05-22 15:18 ` [PATCH 4/8] ARM: STi: DT: Add sdhci controller " Peter Griffin
2014-05-22 15:58 ` Lee Jones
2014-05-22 15:18 ` [PATCH 5/8] ARM: STi: DT: Add sdhci pin configuration for stih415 Peter Griffin
2014-05-22 15:57 ` Lee Jones
2014-05-22 15:18 ` [PATCH 6/8] ARM: STi: DT: Enable mmc0 for both stih415 and stih416 SoCs Peter Griffin
2014-05-22 15:41 ` Lee Jones
2014-05-22 15:18 ` [PATCH 7/8] ARM: STi: DT: Enable second sdhci controller for stih416 b2020 boards Peter Griffin
2014-05-22 15:39 ` Lee Jones
2014-05-22 15:18 ` [PATCH 8/8] ARM: update multi_v7_defconfig for STI Peter Griffin
2014-05-22 15:36 ` Lee Jones
2014-05-22 15:44 ` Add SDHCI support for STMicroelectronics SoCs Maxime Coquelin
2014-06-02 10:29 ` Peter Griffin
2014-06-02 13:53 ` Lee Jones
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='CAPDyKFqvQca9RfsVVyfeC9r=pEDM2mBkfS7SzpYZHHXMnHfyUQ@mail.gmail.com' \
--to=ulf.hansson@linaro.org \
--cc=chris@printf.net \
--cc=devicetree@vger.kernel.org \
--cc=kernel@stlinux.com \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mmc@vger.kernel.org \
--cc=maxime.coquelin@st.com \
--cc=patrice.chotard@st.com \
--cc=peppe.cavallaro@st.com \
--cc=peter.griffin@linaro.org \
--cc=srinivas.kandagatla@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).