If the platform device has a clock associated with the tmio-mmc device,
use it.
Signed-off-by: Guennadi Liakhovetski <[email protected]>
Cc: Magnus Damm <[email protected]>
---
Depends (at least logically) on
http://marc.info/?l=linux-kernel&m=124782904228865&w=2
Sorry if I forgot anyone who volunteered to help out with MMC patches.
drivers/mmc/host/tmio_mmc.c | 21 +++++++++++++++++++++
drivers/mmc/host/tmio_mmc.h | 3 +++
include/linux/mfd/tmio.h | 1 +
3 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
index c246191..2a01572 100644
--- a/drivers/mmc/host/tmio_mmc.c
+++ b/drivers/mmc/host/tmio_mmc.c
@@ -32,6 +32,7 @@
#include <linux/mmc/host.h>
#include <linux/mfd/core.h>
#include <linux/mfd/tmio.h>
+#include <linux/clk.h>
#include "tmio_mmc.h"
@@ -57,10 +58,18 @@ static void tmio_mmc_clk_stop(struct tmio_mmc_host *host)
sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
msleep(10);
+ if (!IS_ERR(host->clk) && host->clk_is_running) {
+ clk_disable(host->clk);
+ host->clk_is_running = false;
+ }
}
static void tmio_mmc_clk_start(struct tmio_mmc_host *host)
{
+ if (!IS_ERR(host->clk) && !host->clk_is_running &&
+ !clk_enable(host->clk))
+ host->clk_is_running = true;
+
sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x0100 |
sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
msleep(10);
@@ -567,6 +576,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
goto unmap_cnf;
}
+ host->clk = clk_get(&dev->dev, pdata->clk_name);
+
+ if (!IS_ERR(host->clk) && !clk_enable(host->clk))
+ host->clk_is_running = true;
+
/* Enable the MMC/SD Control registers */
sd_config_write16(host, CNF_CMD, SDCREN);
sd_config_write32(host, CNF_CTL_BASE,
@@ -608,6 +622,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
return 0;
unmap_cnf:
+ if (!IS_ERR(host->clk)) {
+ clk_disable(host->clk);
+ host->clk_is_running = false;
+ clk_put(host->clk);
+ }
if (host->cnf)
iounmap(host->cnf);
unmap_ctl:
@@ -632,6 +651,8 @@ static int __devexit tmio_mmc_remove(struct platform_device *dev)
if (host->cnf)
iounmap(host->cnf);
mmc_free_host(mmc);
+ if (!IS_ERR(host->clk))
+ clk_put(host->clk);
}
return 0;
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 45f06aa..a76d237 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -108,6 +108,7 @@
sd_ctrl_write32((host), CTL_STATUS, mask); \
} while (0)
+struct clk;
struct tmio_mmc_host {
void __iomem *cnf;
@@ -117,6 +118,8 @@ struct tmio_mmc_host {
struct mmc_request *mrq;
struct mmc_data *data;
struct mmc_host *mmc;
+ struct clk *clk;
+ bool clk_is_running;
int irq;
/* pio related stuff */
diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index 6b9c5d0..e405895 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h
@@ -23,6 +23,7 @@
*/
struct tmio_mmc_data {
const unsigned int hclk;
+ const char *clk_name;
};
/*
--
1.6.2.4
On Tue, Jul 28, 2009 at 09:51:07AM +0200, Guennadi Liakhovetski wrote:
> If the platform device has a clock associated with the tmio-mmc device,
> use it.
>
> Signed-off-by: Guennadi Liakhovetski <[email protected]>
> Cc: Magnus Damm <[email protected]>
> ---
>
Looks fine to me, though it's probably best to wait for an ACK from Ian.
Reviewed-by: Matt Fleming <[email protected]>
Guennadi Liakhovetski wrote:
> If the platform device has a clock associated with the tmio-mmc device,
> use it.
As RMK has stated manny times before now, you cannot pass struct clks to
devices like this, they should be claimed using the clk api.
The problem for tmio-mmc is that the struct clk is tied to the
architecture code, so at present we cant create new clk providers. What
we need is for the clk api to be divorced from the arch code so that the
TMIO MFD driver can create its own clocks that can be claimed by its
children.
Guennadi Liakhovetski wrote:
> If the platform device has a clock associated with the tmio-mmc device,
> use it.
Sorry, I misread there.
I'm still not sure what to to about this though because we seem to be
collecting numerous ways of passing clocks to this driver, this is the
fourth, by my counting.
the clock API could cope with all of them by simply allowing the driver
to claim CLK_MMC (or such) from its parent, except that it cant cope
with both the platform and MFD code providing clocks. The parent could
be either the TMIO MFD core (for TMIO MFDs) or the CPU/SoC whatever, it
woudlnt matter.
In any case, still no, as with all the other TMIO clock code patches.
This needs to be done properly.
On Tue, 28 Jul 2009, Ian Molton wrote:
> Guennadi Liakhovetski wrote:
> > If the platform device has a clock associated with the tmio-mmc device, use
> > it.
>
> Sorry, I misread there.
>
> I'm still not sure what to to about this though because we seem to be
> collecting numerous ways of passing clocks to this driver, this is the fourth,
> by my counting.
>
> the clock API could cope with all of them by simply allowing the driver to
> claim CLK_MMC (or such) from its parent, except that it cant cope with both
> the platform and MFD code providing clocks. The parent could be either the
> TMIO MFD core (for TMIO MFDs) or the CPU/SoC whatever, it woudlnt matter.
>
> In any case, still no, as with all the other TMIO clock code patches. This
> needs to be done properly.
Hi Ian
Thanks for the review.
I understand your concerns. Of course, the _proper_ solution would be to
implement an architecture-independent clock API, something like what
clocklib was trying to do. So, yes, if clocklib were in place now, I
certainly would have used it.
I searched for those clocklib submission attempts (Dmitry added to CC).
Last one I can find (maybe I missed some) is from July 2008 - more than a
year ago. So, looks like our options currently are:
1. wait for new submissions of clocklib - if any are planned
2. develop a completely new arch-independent clock API approach
3. take over patches from Dmitry and bring them to a state acceptable for
mainline
(any more I missed)
No idea about 1, hopefully, Dmitry can tell if he has any near future
plans to resubmit his patches.
I personally don't have free (as in beer) time to work on 2 or 3. Anyone?
So, unless we hear, that one of the 1-3 is going to happen real soon now,
I think, it would be unfair to leave SuperH without a proper MMC driver in
the mainline for an indefinite time, when one can be trivially achieved.
As for your debugging concern, we could allow configuration-less operation
only on SuperH in tmio_mmc, how about that?
Thanks
Guennadi
---
Guennadi Liakhovetski
Guennadi Liakhovetski wrote:
> Hi Ian
Hi!
> Thanks for the review.
No problem - I hope you dont feel that I am picking on you in particular
here, this problem is not isolated to your patches.
> I understand your concerns. Of course, the _proper_ solution would be to
> implement an architecture-independent clock API, something like what
> clocklib was trying to do. So, yes, if clocklib were in place now, I
> certainly would have used it.
Which is of course a chicken/egg situation. I dealt with this a number
of times during the e-series development, and every time, integration
with mainline has gone more swiftly and with better quality, when the
job was done properly to begin with.
> I searched for those clocklib submission attempts (Dmitry added to CC).
Also adding RMK to the CC:
> Last one I can find (maybe I missed some) is from July 2008 - more than a
> year ago. So, looks like our options currently are:
>
> 1. wait for new submissions of clocklib - if any are planned
Dmitry: whats the current status of your clocklib work?
> 2. develop a completely new arch-independent clock API approach
No chance.
> 3. take over patches from Dmitry and bring them to a state acceptable for
> mainline
Take over / collaborate with. I'll happily help people push these patches.
> I personally don't have free (as in beer) time to work on 2 or 3. Anyone?
If I can find out what the current state of this stuff is, I'll help get
it working - I can test / update all the TC6x and T7x MFD devices (and
probably asic3 too)
> So, unless we hear, that one of the 1-3 is going to happen real soon now,
> I think, it would be unfair to leave SuperH without a proper MMC driver in
> the mainline for an indefinite time, when one can be trivially achieved.
Yes, because listening makes code write itself...
If the changes are so trivial, it wont hurt to maintain them as a patch.
certainly tmio-mmc doesnt change very rapidly so the patch wont need
regenerating much. You already have this patchset.
So since thats taken care of, we're all now free to work on updating
the clock API. And once thats done, you can drop your patch and add one
line to create / alias the clock in your superH platform code.
> As for your debugging concern, we could allow configuration-less operation
> only on SuperH in tmio_mmc, how about that?
No. The correct way to support optional parts of the hardware is to
simply not access them when they are not there.
As I said, I'll happily take a patch to abstract power control for the
tmio-mmc driver. This should remove most of the config area accesses
from the driver (and shrink your patch!). The remainder are all about
clock config, and will disappear once we sort the clock API.
Mainline isn't about 'fair' its about 'right'.
-Ian
On Thu, Jul 30, 2009 at 12:20 AM, pHilipp Zabel<[email protected]> wrote:
> On Tue, Jul 28, 2009 at 9:51 AM, Guennadi
> Liakhovetski<[email protected]> wrote:
>> If the platform device has a clock associated with the tmio-mmc device,
>> use it.
>>
>> Signed-off-by: Guennadi Liakhovetski <[email protected]>
>> Cc: Magnus Damm <[email protected]>
>> ---
>>
>> Depends (at least logically) on
>> http://marc.info/?l=linux-kernel&m=124782904228865&w=2
>>
>> Sorry if I forgot anyone who volunteered to help out with MMC patches.
>>
>> ?drivers/mmc/host/tmio_mmc.c | ? 21 +++++++++++++++++++++
>> ?drivers/mmc/host/tmio_mmc.h | ? ?3 +++
>> ?include/linux/mfd/tmio.h ? ?| ? ?1 +
>> ?3 files changed, 25 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
>> index c246191..2a01572 100644
>> --- a/drivers/mmc/host/tmio_mmc.c
>> +++ b/drivers/mmc/host/tmio_mmc.c
>> @@ -567,6 +576,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
>> ? ? ? ? ? ? ? ? ? ? ? ?goto unmap_cnf;
>> ? ? ? ?}
>>
>> + ? ? ? host->clk = clk_get(&dev->dev, pdata->clk_name);
>
> Instead of pdata->clk_name, this should be either NULL or better
> "HCLK" (to differentiate from CLK32, not sure if we want to be able to
> toggle that, too, in the future).
> Passing the clock consumer ID via pdata is just wrong and shouldn't be
> needed with clkdev.
Really? I think that depends on the clock framework implementation.
Remember that this needs to work on other architectures than just ARM.
Cheers,
/ magnus
On Wed, Jul 29, 2009 at 5:24 PM, Magnus Damm<[email protected]> wrote:
> On Thu, Jul 30, 2009 at 12:20 AM, pHilipp Zabel<[email protected]> wrote:
>> On Tue, Jul 28, 2009 at 9:51 AM, Guennadi
>> Liakhovetski<[email protected]> wrote:
>>> If the platform device has a clock associated with the tmio-mmc device,
>>> use it.
>>>
>>> Signed-off-by: Guennadi Liakhovetski <[email protected]>
>>> Cc: Magnus Damm <[email protected]>
>>> ---
>>>
>>> Depends (at least logically) on
>>> http://marc.info/?l=linux-kernel&m=124782904228865&w=2
>>>
>>> Sorry if I forgot anyone who volunteered to help out with MMC patches.
>>>
>>> ?drivers/mmc/host/tmio_mmc.c | ? 21 +++++++++++++++++++++
>>> ?drivers/mmc/host/tmio_mmc.h | ? ?3 +++
>>> ?include/linux/mfd/tmio.h ? ?| ? ?1 +
>>> ?3 files changed, 25 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
>>> index c246191..2a01572 100644
>>> --- a/drivers/mmc/host/tmio_mmc.c
>>> +++ b/drivers/mmc/host/tmio_mmc.c
>>> @@ -567,6 +576,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
>>> ? ? ? ? ? ? ? ? ? ? ? ?goto unmap_cnf;
>>> ? ? ? ?}
>>>
>>> + ? ? ? host->clk = clk_get(&dev->dev, pdata->clk_name);
>>
>> Instead of pdata->clk_name, this should be either NULL or better
>> "HCLK" (to differentiate from CLK32, not sure if we want to be able to
>> toggle that, too, in the future).
>> Passing the clock consumer ID via pdata is just wrong and shouldn't be
>> needed with clkdev.
>
> Really? I think that depends on the clock framework implementation.
> Remember that this needs to work on other architectures than just ARM.
Oh, clkdev is an ARM only thing. Maybe it would be worthwile to make
this even more generic instead?
regards
Philipp
On Wed, Jul 29, 2009 at 5:20 PM, pHilipp Zabel<[email protected]> wrote:
> On Tue, Jul 28, 2009 at 9:51 AM, Guennadi
> Liakhovetski<[email protected]> wrote:
>> If the platform device has a clock associated with the tmio-mmc device,
>> use it.
>>
>> Signed-off-by: Guennadi Liakhovetski <[email protected]>
>> Cc: Magnus Damm <[email protected]>
>> ---
>>
>> Depends (at least logically) on
>> http://marc.info/?l=linux-kernel&m=124782904228865&w=2
>>
>> Sorry if I forgot anyone who volunteered to help out with MMC patches.
>>
>> ?drivers/mmc/host/tmio_mmc.c | ? 21 +++++++++++++++++++++
>> ?drivers/mmc/host/tmio_mmc.h | ? ?3 +++
>> ?include/linux/mfd/tmio.h ? ?| ? ?1 +
>> ?3 files changed, 25 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
>> index c246191..2a01572 100644
>> --- a/drivers/mmc/host/tmio_mmc.c
>> +++ b/drivers/mmc/host/tmio_mmc.c
>> @@ -32,6 +32,7 @@
>> ?#include <linux/mmc/host.h>
>> ?#include <linux/mfd/core.h>
>> ?#include <linux/mfd/tmio.h>
>> +#include <linux/clk.h>
>>
>> ?#include "tmio_mmc.h"
>>
>> @@ -57,10 +58,18 @@ static void tmio_mmc_clk_stop(struct tmio_mmc_host *host)
>> ? ? ? ?sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
>> ? ? ? ? ? ? ? ?sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
>> ? ? ? ?msleep(10);
>> + ? ? ? if (!IS_ERR(host->clk) && host->clk_is_running) {
>> + ? ? ? ? ? ? ? clk_disable(host->clk);
>> + ? ? ? ? ? ? ? host->clk_is_running = false;
>> + ? ? ? }
>> ?}
>>
>> ?static void tmio_mmc_clk_start(struct tmio_mmc_host *host)
>> ?{
>> + ? ? ? if (!IS_ERR(host->clk) && !host->clk_is_running &&
>> + ? ? ? ? ? !clk_enable(host->clk))
>> + ? ? ? ? ? ? ? host->clk_is_running = true;
>> +
>> ? ? ? ?sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x0100 |
>> ? ? ? ? ? ? ? ?sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
>> ? ? ? ?msleep(10);
>> @@ -567,6 +576,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
>> ? ? ? ? ? ? ? ? ? ? ? ?goto unmap_cnf;
>> ? ? ? ?}
>>
>> + ? ? ? host->clk = clk_get(&dev->dev, pdata->clk_name);
>
> Instead of pdata->clk_name, this should be either NULL or better
> "HCLK" (to differentiate from CLK32, not sure if we want to be able to
> toggle that, too, in the future).
> Passing the clock consumer ID via pdata is just wrong and shouldn't be
> needed with clkdev.
>
>> +
>> + ? ? ? if (!IS_ERR(host->clk) && !clk_enable(host->clk))
>> + ? ? ? ? ? ? ? host->clk_is_running = true;
>> +
>> ? ? ? ?/* Enable the MMC/SD Control registers */
>> ? ? ? ?sd_config_write16(host, CNF_CMD, SDCREN);
>> ? ? ? ?sd_config_write32(host, CNF_CTL_BASE,
>> @@ -608,6 +622,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
>> ? ? ? ?return 0;
>>
>> ?unmap_cnf:
>> + ? ? ? if (!IS_ERR(host->clk)) {
>> + ? ? ? ? ? ? ? clk_disable(host->clk);
>> + ? ? ? ? ? ? ? host->clk_is_running = false;
>> + ? ? ? ? ? ? ? clk_put(host->clk);
>> + ? ? ? }
>> ? ? ? ?if (host->cnf)
>> ? ? ? ? ? ? ? ?iounmap(host->cnf);
>> ?unmap_ctl:
>> @@ -632,6 +651,8 @@ static int __devexit tmio_mmc_remove(struct platform_device *dev)
>> ? ? ? ? ? ? ? ?if (host->cnf)
>> ? ? ? ? ? ? ? ? ? ? ? ?iounmap(host->cnf);
>> ? ? ? ? ? ? ? ?mmc_free_host(mmc);
>> + ? ? ? ? ? ? ? if (!IS_ERR(host->clk))
>> + ? ? ? ? ? ? ? ? ? ? ? clk_put(host->clk);
>> ? ? ? ?}
>>
>> ? ? ? ?return 0;
>> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
>> index 45f06aa..a76d237 100644
>> --- a/drivers/mmc/host/tmio_mmc.h
>> +++ b/drivers/mmc/host/tmio_mmc.h
>> @@ -108,6 +108,7 @@
>> ? ? ? ? ? ? ? ?sd_ctrl_write32((host), CTL_STATUS, mask); \
>> ? ? ? ?} while (0)
>>
>> +struct clk;
>>
>> ?struct tmio_mmc_host {
>> ? ? ? ?void __iomem *cnf;
>> @@ -117,6 +118,8 @@ struct tmio_mmc_host {
>> ? ? ? ?struct mmc_request ? ? ?*mrq;
>> ? ? ? ?struct mmc_data ? ? ? ? *data;
>> ? ? ? ?struct mmc_host ? ? ? ? *mmc;
>> + ? ? ? struct clk ? ? ? ? ? ? ?*clk;
>> + ? ? ? bool ? ? ? ? ? ? ? ? ? ?clk_is_running;
>> ? ? ? ?int ? ? ? ? ? ? ? ? ? ? irq;
>>
>> ? ? ? ?/* pio related stuff */
>> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
>> index 6b9c5d0..e405895 100644
>> --- a/include/linux/mfd/tmio.h
>> +++ b/include/linux/mfd/tmio.h
>> @@ -23,6 +23,7 @@
>> ?*/
>> ?struct tmio_mmc_data {
>> ? ? ? ?const unsigned int ? ? ? ? ? ? ?hclk;
>> + ? ? ? const char ? ? ? ? ? ? ? ? ? ? ?*clk_name;
>
> Drop that.
Actually, this struct can be dropped completely and the HCLK rate can
be determined with clk_get_rate.
>> ?};
>>
>> ?/*
>> --
>> 1.6.2.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at ?http://www.tux.org/lkml/
>>
>
> Otherwise that patch doesn't seem wrong to me?
> As soon as we have the possibility for MFD drivers to provide their
> own clk API implementation, this should also work with ASIC3/TMIO.
> I'll try to register my ASIC3 HCLK with clkdev and see if this patch
> works.
>
> regards
> Philipp
>
On Tue, Jul 28, 2009 at 9:51 AM, Guennadi
Liakhovetski<[email protected]> wrote:
> If the platform device has a clock associated with the tmio-mmc device,
> use it.
>
> Signed-off-by: Guennadi Liakhovetski <[email protected]>
> Cc: Magnus Damm <[email protected]>
> ---
>
> Depends (at least logically) on
> http://marc.info/?l=linux-kernel&m=124782904228865&w=2
>
> Sorry if I forgot anyone who volunteered to help out with MMC patches.
>
> ?drivers/mmc/host/tmio_mmc.c | ? 21 +++++++++++++++++++++
> ?drivers/mmc/host/tmio_mmc.h | ? ?3 +++
> ?include/linux/mfd/tmio.h ? ?| ? ?1 +
> ?3 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
> index c246191..2a01572 100644
> --- a/drivers/mmc/host/tmio_mmc.c
> +++ b/drivers/mmc/host/tmio_mmc.c
> @@ -32,6 +32,7 @@
> ?#include <linux/mmc/host.h>
> ?#include <linux/mfd/core.h>
> ?#include <linux/mfd/tmio.h>
> +#include <linux/clk.h>
>
> ?#include "tmio_mmc.h"
>
> @@ -57,10 +58,18 @@ static void tmio_mmc_clk_stop(struct tmio_mmc_host *host)
> ? ? ? ?sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
> ? ? ? ? ? ? ? ?sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> ? ? ? ?msleep(10);
> + ? ? ? if (!IS_ERR(host->clk) && host->clk_is_running) {
> + ? ? ? ? ? ? ? clk_disable(host->clk);
> + ? ? ? ? ? ? ? host->clk_is_running = false;
> + ? ? ? }
> ?}
>
> ?static void tmio_mmc_clk_start(struct tmio_mmc_host *host)
> ?{
> + ? ? ? if (!IS_ERR(host->clk) && !host->clk_is_running &&
> + ? ? ? ? ? !clk_enable(host->clk))
> + ? ? ? ? ? ? ? host->clk_is_running = true;
> +
> ? ? ? ?sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x0100 |
> ? ? ? ? ? ? ? ?sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> ? ? ? ?msleep(10);
> @@ -567,6 +576,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
> ? ? ? ? ? ? ? ? ? ? ? ?goto unmap_cnf;
> ? ? ? ?}
>
> + ? ? ? host->clk = clk_get(&dev->dev, pdata->clk_name);
Instead of pdata->clk_name, this should be either NULL or better
"HCLK" (to differentiate from CLK32, not sure if we want to be able to
toggle that, too, in the future).
Passing the clock consumer ID via pdata is just wrong and shouldn't be
needed with clkdev.
> +
> + ? ? ? if (!IS_ERR(host->clk) && !clk_enable(host->clk))
> + ? ? ? ? ? ? ? host->clk_is_running = true;
> +
> ? ? ? ?/* Enable the MMC/SD Control registers */
> ? ? ? ?sd_config_write16(host, CNF_CMD, SDCREN);
> ? ? ? ?sd_config_write32(host, CNF_CTL_BASE,
> @@ -608,6 +622,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
> ? ? ? ?return 0;
>
> ?unmap_cnf:
> + ? ? ? if (!IS_ERR(host->clk)) {
> + ? ? ? ? ? ? ? clk_disable(host->clk);
> + ? ? ? ? ? ? ? host->clk_is_running = false;
> + ? ? ? ? ? ? ? clk_put(host->clk);
> + ? ? ? }
> ? ? ? ?if (host->cnf)
> ? ? ? ? ? ? ? ?iounmap(host->cnf);
> ?unmap_ctl:
> @@ -632,6 +651,8 @@ static int __devexit tmio_mmc_remove(struct platform_device *dev)
> ? ? ? ? ? ? ? ?if (host->cnf)
> ? ? ? ? ? ? ? ? ? ? ? ?iounmap(host->cnf);
> ? ? ? ? ? ? ? ?mmc_free_host(mmc);
> + ? ? ? ? ? ? ? if (!IS_ERR(host->clk))
> + ? ? ? ? ? ? ? ? ? ? ? clk_put(host->clk);
> ? ? ? ?}
>
> ? ? ? ?return 0;
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 45f06aa..a76d237 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -108,6 +108,7 @@
> ? ? ? ? ? ? ? ?sd_ctrl_write32((host), CTL_STATUS, mask); \
> ? ? ? ?} while (0)
>
> +struct clk;
>
> ?struct tmio_mmc_host {
> ? ? ? ?void __iomem *cnf;
> @@ -117,6 +118,8 @@ struct tmio_mmc_host {
> ? ? ? ?struct mmc_request ? ? ?*mrq;
> ? ? ? ?struct mmc_data ? ? ? ? *data;
> ? ? ? ?struct mmc_host ? ? ? ? *mmc;
> + ? ? ? struct clk ? ? ? ? ? ? ?*clk;
> + ? ? ? bool ? ? ? ? ? ? ? ? ? ?clk_is_running;
> ? ? ? ?int ? ? ? ? ? ? ? ? ? ? irq;
>
> ? ? ? ?/* pio related stuff */
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index 6b9c5d0..e405895 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -23,6 +23,7 @@
> ?*/
> ?struct tmio_mmc_data {
> ? ? ? ?const unsigned int ? ? ? ? ? ? ?hclk;
> + ? ? ? const char ? ? ? ? ? ? ? ? ? ? ?*clk_name;
Drop that.
> ?};
>
> ?/*
> --
> 1.6.2.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
>
Otherwise that patch doesn't seem wrong to me?
As soon as we have the possibility for MFD drivers to provide their
own clk API implementation, this should also work with ASIC3/TMIO.
I'll try to register my ASIC3 HCLK with clkdev and see if this patch
works.
regards
Philipp
On Tue, Jul 28, 2009 at 9:51 AM, Guennadi
Liakhovetski<[email protected]> wrote:
> If the platform device has a clock associated with the tmio-mmc device,
> use it.
>
> Signed-off-by: Guennadi Liakhovetski <[email protected]>
> Cc: Magnus Damm <[email protected]>
> ---
>
> Depends (at least logically) on
> http://marc.info/?l=linux-kernel&m=124782904228865&w=2
>
> Sorry if I forgot anyone who volunteered to help out with MMC patches.
>
> ?drivers/mmc/host/tmio_mmc.c | ? 21 +++++++++++++++++++++
> ?drivers/mmc/host/tmio_mmc.h | ? ?3 +++
> ?include/linux/mfd/tmio.h ? ?| ? ?1 +
> ?3 files changed, 25 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
> index c246191..2a01572 100644
> --- a/drivers/mmc/host/tmio_mmc.c
> +++ b/drivers/mmc/host/tmio_mmc.c
> @@ -32,6 +32,7 @@
> ?#include <linux/mmc/host.h>
> ?#include <linux/mfd/core.h>
> ?#include <linux/mfd/tmio.h>
> +#include <linux/clk.h>
>
> ?#include "tmio_mmc.h"
>
> @@ -57,10 +58,18 @@ static void tmio_mmc_clk_stop(struct tmio_mmc_host *host)
> ? ? ? ?sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~0x0100 &
> ? ? ? ? ? ? ? ?sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> ? ? ? ?msleep(10);
> + ? ? ? if (!IS_ERR(host->clk) && host->clk_is_running) {
> + ? ? ? ? ? ? ? clk_disable(host->clk);
> + ? ? ? ? ? ? ? host->clk_is_running = false;
> + ? ? ? }
> ?}
>
> ?static void tmio_mmc_clk_start(struct tmio_mmc_host *host)
> ?{
> + ? ? ? if (!IS_ERR(host->clk) && !host->clk_is_running &&
> + ? ? ? ? ? !clk_enable(host->clk))
> + ? ? ? ? ? ? ? host->clk_is_running = true;
> +
> ? ? ? ?sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, 0x0100 |
> ? ? ? ? ? ? ? ?sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL));
> ? ? ? ?msleep(10);
> @@ -567,6 +576,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
> ? ? ? ? ? ? ? ? ? ? ? ?goto unmap_cnf;
> ? ? ? ?}
>
> + ? ? ? host->clk = clk_get(&dev->dev, pdata->clk_name);
As long as we are going to ignore the error code, I think the rest of
the patch would be easier on the eyes with
if (IS_ERR(host->clk))
host->clk = NULL;
inserted here.
> + ? ? ? if (!IS_ERR(host->clk) && !clk_enable(host->clk))
> + ? ? ? ? ? ? ? host->clk_is_running = true;
> +
> ? ? ? ?/* Enable the MMC/SD Control registers */
> ? ? ? ?sd_config_write16(host, CNF_CMD, SDCREN);
> ? ? ? ?sd_config_write32(host, CNF_CTL_BASE,
> @@ -608,6 +622,11 @@ static int __devinit tmio_mmc_probe(struct platform_device *dev)
> ? ? ? ?return 0;
>
> ?unmap_cnf:
> + ? ? ? if (!IS_ERR(host->clk)) {
> + ? ? ? ? ? ? ? clk_disable(host->clk);
> + ? ? ? ? ? ? ? host->clk_is_running = false;
> + ? ? ? ? ? ? ? clk_put(host->clk);
> + ? ? ? }
> ? ? ? ?if (host->cnf)
> ? ? ? ? ? ? ? ?iounmap(host->cnf);
> ?unmap_ctl:
> @@ -632,6 +651,8 @@ static int __devexit tmio_mmc_remove(struct platform_device *dev)
> ? ? ? ? ? ? ? ?if (host->cnf)
> ? ? ? ? ? ? ? ? ? ? ? ?iounmap(host->cnf);
> ? ? ? ? ? ? ? ?mmc_free_host(mmc);
> + ? ? ? ? ? ? ? if (!IS_ERR(host->clk))
> + ? ? ? ? ? ? ? ? ? ? ? clk_put(host->clk);
> ? ? ? ?}
>
> ? ? ? ?return 0;
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 45f06aa..a76d237 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -108,6 +108,7 @@
> ? ? ? ? ? ? ? ?sd_ctrl_write32((host), CTL_STATUS, mask); \
> ? ? ? ?} while (0)
>
> +struct clk;
>
> ?struct tmio_mmc_host {
> ? ? ? ?void __iomem *cnf;
> @@ -117,6 +118,8 @@ struct tmio_mmc_host {
> ? ? ? ?struct mmc_request ? ? ?*mrq;
> ? ? ? ?struct mmc_data ? ? ? ? *data;
> ? ? ? ?struct mmc_host ? ? ? ? *mmc;
> + ? ? ? struct clk ? ? ? ? ? ? ?*clk;
> + ? ? ? bool ? ? ? ? ? ? ? ? ? ?clk_is_running;
> ? ? ? ?int ? ? ? ? ? ? ? ? ? ? irq;
>
> ? ? ? ?/* pio related stuff */
> diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
> index 6b9c5d0..e405895 100644
> --- a/include/linux/mfd/tmio.h
> +++ b/include/linux/mfd/tmio.h
> @@ -23,6 +23,7 @@
> ?*/
> ?struct tmio_mmc_data {
> ? ? ? ?const unsigned int ? ? ? ? ? ? ?hclk;
> + ? ? ? const char ? ? ? ? ? ? ? ? ? ? ?*clk_name;
> ?};
>
> ?/*
> --
> 1.6.2.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at ?http://www.tux.org/lkml/
Also I quickly tested this on ASIC3 and so far the device hangs if I
remove the code that enables hclk already in mfd_cell->enable (in
asic3.c). I'll try to find out why that happens.
regards
Philipp
pHilipp Zabel wrote:
>>> struct tmio_mmc_data {
>>> const unsigned int hclk;
>>> + const char *clk_name;
>> Drop that.
>
> Actually, this struct can be dropped completely and the HCLK rate can
> be determined with clk_get_rate.
Actually it cant because MFD devices have different hclk frequencies,
and the mmc driver needs to know this. MFD drivers cannot pass the hclk
rate in using the clk API at present.
-Ian