* [PATCH] mmc: sdhci-of-arasan: Properly set corecfg_clockmultipler on rk3399
@ 2016-08-26 3:22 Shawn Lin
[not found] ` <1472181736-17848-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Shawn Lin @ 2016-08-26 3:22 UTC (permalink / raw)
To: Ulf Hansson
Cc: Shawn Lin, xzy.xu-TNX95d0MmH7DzftRWevZcw,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, Adrian Hunter, Douglas Anderson,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
corecfg_clockmultipler indicates clock multiplier value of
programmable clock generator which should be the same value
of SDHCI_CAPABILITIES_1. The default value of the register,
corecfg_clockmultipler, is 0x10. But actually it is a mistake
by designer as our intention was to set it to be zero which
means we don't support programmable clock generator. So we have
to made it to be zero on bootloader which seems work fine until
now. But now we find a issue that when deploying genpd support
for it, the remove callback will trigger the genpd to poweroff the
power domain for sdhci-of-arasan which magage the controller, phy
and corecfg_* stuff.
So when we do bind/unbind the driver, we have already reinit
the controller and phy, but without doing that for corecfg_*.
Regarding to only the corecfg_clockmultipler is wrong, let's
fix it by explicitly marking it to be zero when probing. With
this change, we could do bind/unbind successfully.
Reported-by: Ziyuan Xu <xzy.xu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---
drivers/mmc/host/sdhci-of-arasan.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 0b3a9cf..c1eb263 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -67,10 +67,12 @@ struct sdhci_arasan_soc_ctl_field {
* accessible via the syscon API.
*
* @baseclkfreq: Where to find corecfg_baseclkfreq
+ * @clockmultiplier: Where to find corecfg_clockmultiplier
* @hiword_update: If true, use HIWORD_UPDATE to access the syscon
*/
struct sdhci_arasan_soc_ctl_map {
struct sdhci_arasan_soc_ctl_field baseclkfreq;
+ struct sdhci_arasan_soc_ctl_field clockmultiplier;
bool hiword_update;
};
@@ -100,6 +102,7 @@ struct sdhci_arasan_data {
static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
.baseclkfreq = { .reg = 0xf000, .width = 8, .shift = 8 },
+ .clockmultiplier = { .reg = 0xf02c, .width = 8, .shit = 0},
.hiword_update = true,
};
@@ -528,6 +531,12 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
ret);
goto err_pltfm_free;
}
+
+ if (of_device_is_compatible(pdev->dev.of_node,
+ "rockchip,rk3399-sdhci-5.1"))
+ sdhci_arasan_syscon_write(host,
+ &soc_ctl_map->clockmultiplier,
+ 0);
}
sdhci_arasan->clk_ahb = devm_clk_get(&pdev->dev, "clk_ahb");
--
2.3.7
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Properly set corecfg_clockmultipler on rk3399
[not found] ` <1472181736-17848-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2016-08-26 6:38 ` Ziyuan Xu
2016-08-26 7:50 ` Shawn Lin
0 siblings, 1 reply; 7+ messages in thread
From: Ziyuan Xu @ 2016-08-26 6:38 UTC (permalink / raw)
To: Shawn Lin, Ulf Hansson
Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, Adrian Hunter, Douglas Anderson
Hi Shawn,
On 2016年08月26日 11:22, Shawn Lin wrote:
> corecfg_clockmultipler indicates clock multiplier value of
> programmable clock generator which should be the same value
> of SDHCI_CAPABILITIES_1. The default value of the register,
> corecfg_clockmultipler, is 0x10. But actually it is a mistake
corecfg_clockmultipler==>corecfg_clockmultiplier
> by designer as our intention was to set it to be zero which
> means we don't support programmable clock generator. So we have
> to made it to be zero on bootloader which seems work fine until
made==>make
> now. But now we find a issue that when deploying genpd support
a issue==>an issue
> for it, the remove callback will trigger the genpd to poweroff the
> power domain for sdhci-of-arasan which magage the controller, phy
magage==>manage
> and corecfg_* stuff.
>
> So when we do bind/unbind the driver, we have already reinit
do==>doing
> the controller and phy, but without doing that for corecfg_*.
> Regarding to only the corecfg_clockmultipler is wrong, let's
> fix it by explicitly marking it to be zero when probing. With
> this change, we could do bind/unbind successfully.
>
> Reported-by: Ziyuan Xu <xzy.xu@rock-chips.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
>
> drivers/mmc/host/sdhci-of-arasan.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 0b3a9cf..c1eb263 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -67,10 +67,12 @@ struct sdhci_arasan_soc_ctl_field {
> * accessible via the syscon API.
> *
> * @baseclkfreq: Where to find corecfg_baseclkfreq
> + * @clockmultiplier: Where to find corecfg_clockmultiplier
> * @hiword_update: If true, use HIWORD_UPDATE to access the syscon
> */
> struct sdhci_arasan_soc_ctl_map {
> struct sdhci_arasan_soc_ctl_field baseclkfreq;
> + struct sdhci_arasan_soc_ctl_field clockmultiplier;
> bool hiword_update;
> };
>
> @@ -100,6 +102,7 @@ struct sdhci_arasan_data {
>
> static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
> .baseclkfreq = { .reg = 0xf000, .width = 8, .shift = 8 },
> + .clockmultiplier = { .reg = 0xf02c, .width = 8, .shit = 0},
> .hiword_update = true,
> };
>
> @@ -528,6 +531,12 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
> ret);
> goto err_pltfm_free;
> }
> +
> + if (of_device_is_compatible(pdev->dev.of_node,
> + "rockchip,rk3399-sdhci-5.1"))
> + sdhci_arasan_syscon_write(host,
> + &soc_ctl_map->clockmultiplier,
> + 0);
Variable soc_ctl_map is undefined in sdhci_arasan_probe(), please fix it.
Moreover, we can not access grf_emmccore/grf_emmcphy prior to enable
clk_ahb, so do it after clock enable.
> }
>
> sdhci_arasan->clk_ahb = devm_clk_get(&pdev->dev, "clk_ahb");
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Properly set corecfg_clockmultipler on rk3399
2016-08-26 6:38 ` Ziyuan Xu
@ 2016-08-26 7:50 ` Shawn Lin
[not found] ` <d6c3b728-af01-e724-3278-1bc0801f8fb4-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
0 siblings, 1 reply; 7+ messages in thread
From: Shawn Lin @ 2016-08-26 7:50 UTC (permalink / raw)
To: Ziyuan Xu, Ulf Hansson
Cc: shawn.lin, linux-rockchip, linux-mmc, Adrian Hunter,
Douglas Anderson, Heiko Stübner
+ Heiko
在 2016/8/26 14:38, Ziyuan Xu 写道:
> Hi Shawn,
>
>
> On 2016年08月26日 11:22, Shawn Lin wrote:
>> corecfg_clockmultipler indicates clock multiplier value of
>> programmable clock generator which should be the same value
>> of SDHCI_CAPABILITIES_1. The default value of the register,
>> corecfg_clockmultipler, is 0x10. But actually it is a mistake
> corecfg_clockmultipler==>corecfg_clockmultiplier
>> by designer as our intention was to set it to be zero which
>> means we don't support programmable clock generator. So we have
>> to made it to be zero on bootloader which seems work fine until
> made==>make
>> now. But now we find a issue that when deploying genpd support
> a issue==>an issue
>> for it, the remove callback will trigger the genpd to poweroff the
>> power domain for sdhci-of-arasan which magage the controller, phy
> magage==>manage
>> and corecfg_* stuff.
>>
>> So when we do bind/unbind the driver, we have already reinit
> do==>doing
>> the controller and phy, but without doing that for corecfg_*.
>> Regarding to only the corecfg_clockmultipler is wrong, let's
>> fix it by explicitly marking it to be zero when probing. With
>> this change, we could do bind/unbind successfully.
>>
>> Reported-by: Ziyuan Xu <xzy.xu@rock-chips.com>
>> Cc: Douglas Anderson <dianders@chromium.org>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> ---
>>
>> drivers/mmc/host/sdhci-of-arasan.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-of-arasan.c
>> b/drivers/mmc/host/sdhci-of-arasan.c
>> index 0b3a9cf..c1eb263 100644
>> --- a/drivers/mmc/host/sdhci-of-arasan.c
>> +++ b/drivers/mmc/host/sdhci-of-arasan.c
>> @@ -67,10 +67,12 @@ struct sdhci_arasan_soc_ctl_field {
>> * accessible via the syscon API.
>> *
>> * @baseclkfreq: Where to find corecfg_baseclkfreq
>> + * @clockmultiplier: Where to find corecfg_clockmultiplier
>> * @hiword_update: If true, use HIWORD_UPDATE to access the syscon
>> */
>> struct sdhci_arasan_soc_ctl_map {
>> struct sdhci_arasan_soc_ctl_field baseclkfreq;
>> + struct sdhci_arasan_soc_ctl_field clockmultiplier;
>> bool hiword_update;
>> };
>> @@ -100,6 +102,7 @@ struct sdhci_arasan_data {
>> static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
>> .baseclkfreq = { .reg = 0xf000, .width = 8, .shift = 8 },
>> + .clockmultiplier = { .reg = 0xf02c, .width = 8, .shit = 0},
>> .hiword_update = true,
>> };
>> @@ -528,6 +531,12 @@ static int sdhci_arasan_probe(struct
>> platform_device *pdev)
>> ret);
>> goto err_pltfm_free;
>> }
>> +
>> + if (of_device_is_compatible(pdev->dev.of_node,
>> + "rockchip,rk3399-sdhci-5.1"))
>> + sdhci_arasan_syscon_write(host,
>> + &soc_ctl_map->clockmultiplier,
>> + 0);
> Variable soc_ctl_map is undefined in sdhci_arasan_probe(), please fix it.
Woops, I change the code a bit after generating patch, I will fix it.
> Moreover, we can not access grf_emmccore/grf_emmcphy prior to enable
> clk_ahb, so do it after clock enable.
Hrmm, we configure the grf vai CPU and the clock for accessing
grf is aclk_emmc_grf( I saw aclk_emmc_noc is critical and it seems we
should treat aclk_emmc_* the same way). I could move this after
enabling aclk_emmc, but that is a bogus critical for aclk_emmc_* to me
since it's off when disabling aclk_emmc, no?
>> }
>> sdhci_arasan->clk_ahb = devm_clk_get(&pdev->dev, "clk_ahb");
>
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
--
Best Regards
Shawn Lin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Properly set corecfg_clockmultipler on rk3399
[not found] ` <d6c3b728-af01-e724-3278-1bc0801f8fb4-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
@ 2016-08-26 8:31 ` Heiko Stübner
2016-08-26 8:36 ` Shawn Lin
2016-08-26 15:46 ` Doug Anderson
0 siblings, 2 replies; 7+ messages in thread
From: Heiko Stübner @ 2016-08-26 8:31 UTC (permalink / raw)
To: Shawn Lin
Cc: Ulf Hansson, Ziyuan Xu, linux-mmc-u79uwXL29TY76Z2rM5mHXA,
Adrian Hunter, Douglas Anderson,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Am Freitag, 26. August 2016, 15:50:57 schrieb Shawn Lin:
> + Heiko
>
> 在 2016/8/26 14:38, Ziyuan Xu 写道:
> > Hi Shawn,
> >
> > On 2016年08月26日 11:22, Shawn Lin wrote:
> >> corecfg_clockmultipler indicates clock multiplier value of
> >> programmable clock generator which should be the same value
> >> of SDHCI_CAPABILITIES_1. The default value of the register,
> >> corecfg_clockmultipler, is 0x10. But actually it is a mistake
> >
> > corecfg_clockmultipler==>corecfg_clockmultiplier
> >
> >> by designer as our intention was to set it to be zero which
> >> means we don't support programmable clock generator. So we have
> >> to made it to be zero on bootloader which seems work fine until
> >
> > made==>make
> >
> >> now. But now we find a issue that when deploying genpd support
> >
> > a issue==>an issue
> >
> >> for it, the remove callback will trigger the genpd to poweroff the
> >> power domain for sdhci-of-arasan which magage the controller, phy
> >
> > magage==>manage
> >
> >> and corecfg_* stuff.
> >>
> >> So when we do bind/unbind the driver, we have already reinit
> >
> > do==>doing
> >
> >> the controller and phy, but without doing that for corecfg_*.
> >> Regarding to only the corecfg_clockmultipler is wrong, let's
> >> fix it by explicitly marking it to be zero when probing. With
> >> this change, we could do bind/unbind successfully.
> >>
> >> Reported-by: Ziyuan Xu <xzy.xu@rock-chips.com>
> >> Cc: Douglas Anderson <dianders@chromium.org>
> >> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> >> ---
> >>
> >> drivers/mmc/host/sdhci-of-arasan.c | 9 +++++++++
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/mmc/host/sdhci-of-arasan.c
> >> b/drivers/mmc/host/sdhci-of-arasan.c
> >> index 0b3a9cf..c1eb263 100644
> >> --- a/drivers/mmc/host/sdhci-of-arasan.c
> >> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> >> @@ -67,10 +67,12 @@ struct sdhci_arasan_soc_ctl_field {
> >>
> >> * accessible via the syscon API.
> >> *
> >> * @baseclkfreq: Where to find corecfg_baseclkfreq
> >>
> >> + * @clockmultiplier: Where to find corecfg_clockmultiplier
> >>
> >> * @hiword_update: If true, use HIWORD_UPDATE to access the syscon
> >> */
> >>
> >> struct sdhci_arasan_soc_ctl_map {
> >>
> >> struct sdhci_arasan_soc_ctl_field baseclkfreq;
> >>
> >> + struct sdhci_arasan_soc_ctl_field clockmultiplier;
> >>
> >> bool hiword_update;
> >>
> >> };
> >> @@ -100,6 +102,7 @@ struct sdhci_arasan_data {
> >>
> >> static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
> >>
> >> .baseclkfreq = { .reg = 0xf000, .width = 8, .shift = 8 },
> >>
> >> + .clockmultiplier = { .reg = 0xf02c, .width = 8, .shit = 0},
> >>
> >> .hiword_update = true,
> >>
> >> };
> >> @@ -528,6 +531,12 @@ static int sdhci_arasan_probe(struct
> >>
> >> platform_device *pdev)
> >>
> >> ret);
> >>
> >> goto err_pltfm_free;
> >>
> >> }
> >>
> >> +
> >> + if (of_device_is_compatible(pdev->dev.of_node,
> >> + "rockchip,rk3399-sdhci-5.1"))
> >> + sdhci_arasan_syscon_write(host,
> >> + &soc_ctl_map->clockmultiplier,
> >> + 0);
> >
> > Variable soc_ctl_map is undefined in sdhci_arasan_probe(), please fix it.
>
> Woops, I change the code a bit after generating patch, I will fix it.
>
> > Moreover, we can not access grf_emmccore/grf_emmcphy prior to enable
> > clk_ahb, so do it after clock enable.
>
> Hrmm, we configure the grf vai CPU and the clock for accessing
> grf is aclk_emmc_grf( I saw aclk_emmc_noc is critical and it seems we
> should treat aclk_emmc_* the same way).
No :-)
The aclk_emmc_grf is (similar to aclk_vio_grf etc) part of the emmc block.
The noc clocks (as far as I understand) are the interconnect clocks that
supply the link between interconnect and peripheral, so are part of the
interconnect block.
We don't model the interconnect on any rockchip soc right now, so the noc
clocks are rightfully marked critical, but we of course do model the emmc
block, so it should handle its grf clock, similar to how the drm driver is
(planning to) doing it right now.
Heiko
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Properly set corecfg_clockmultipler on rk3399
2016-08-26 8:31 ` Heiko Stübner
@ 2016-08-26 8:36 ` Shawn Lin
2016-08-26 15:46 ` Doug Anderson
1 sibling, 0 replies; 7+ messages in thread
From: Shawn Lin @ 2016-08-26 8:36 UTC (permalink / raw)
To: Heiko Stübner
Cc: Ulf Hansson, shawn.lin-TNX95d0MmH7DzftRWevZcw, Ziyuan Xu,
linux-mmc-u79uwXL29TY76Z2rM5mHXA, Adrian Hunter, Douglas Anderson,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
在 2016/8/26 16:31, Heiko Stübner 写道:
> Am Freitag, 26. August 2016, 15:50:57 schrieb Shawn Lin:
>> + Heiko
>>
>> 在 2016/8/26 14:38, Ziyuan Xu 写道:
>>> Hi Shawn,
>>>
>>> On 2016年08月26日 11:22, Shawn Lin wrote:
>>>> corecfg_clockmultipler indicates clock multiplier value of
>>>> programmable clock generator which should be the same value
>>>> of SDHCI_CAPABILITIES_1. The default value of the register,
>>>> corecfg_clockmultipler, is 0x10. But actually it is a mistake
>>>
>>> corecfg_clockmultipler==>corecfg_clockmultiplier
>>>
>>>> by designer as our intention was to set it to be zero which
>>>> means we don't support programmable clock generator. So we have
>>>> to made it to be zero on bootloader which seems work fine until
>>>
>>> made==>make
>>>
>>>> now. But now we find a issue that when deploying genpd support
>>>
>>> a issue==>an issue
>>>
>>>> for it, the remove callback will trigger the genpd to poweroff the
>>>> power domain for sdhci-of-arasan which magage the controller, phy
>>>
>>> magage==>manage
>>>
>>>> and corecfg_* stuff.
>>>>
>>>> So when we do bind/unbind the driver, we have already reinit
>>>
>>> do==>doing
>>>
>>>> the controller and phy, but without doing that for corecfg_*.
>>>> Regarding to only the corecfg_clockmultipler is wrong, let's
>>>> fix it by explicitly marking it to be zero when probing. With
>>>> this change, we could do bind/unbind successfully.
>>>>
>>>> Reported-by: Ziyuan Xu <xzy.xu@rock-chips.com>
>>>> Cc: Douglas Anderson <dianders@chromium.org>
>>>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>>> ---
>>>>
>>>> drivers/mmc/host/sdhci-of-arasan.c | 9 +++++++++
>>>> 1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci-of-arasan.c
>>>> b/drivers/mmc/host/sdhci-of-arasan.c
>>>> index 0b3a9cf..c1eb263 100644
>>>> --- a/drivers/mmc/host/sdhci-of-arasan.c
>>>> +++ b/drivers/mmc/host/sdhci-of-arasan.c
>>>> @@ -67,10 +67,12 @@ struct sdhci_arasan_soc_ctl_field {
>>>>
>>>> * accessible via the syscon API.
>>>> *
>>>> * @baseclkfreq: Where to find corecfg_baseclkfreq
>>>>
>>>> + * @clockmultiplier: Where to find corecfg_clockmultiplier
>>>>
>>>> * @hiword_update: If true, use HIWORD_UPDATE to access the syscon
>>>> */
>>>>
>>>> struct sdhci_arasan_soc_ctl_map {
>>>>
>>>> struct sdhci_arasan_soc_ctl_field baseclkfreq;
>>>>
>>>> + struct sdhci_arasan_soc_ctl_field clockmultiplier;
>>>>
>>>> bool hiword_update;
>>>>
>>>> };
>>>> @@ -100,6 +102,7 @@ struct sdhci_arasan_data {
>>>>
>>>> static const struct sdhci_arasan_soc_ctl_map rk3399_soc_ctl_map = {
>>>>
>>>> .baseclkfreq = { .reg = 0xf000, .width = 8, .shift = 8 },
>>>>
>>>> + .clockmultiplier = { .reg = 0xf02c, .width = 8, .shit = 0},
>>>>
>>>> .hiword_update = true,
>>>>
>>>> };
>>>> @@ -528,6 +531,12 @@ static int sdhci_arasan_probe(struct
>>>>
>>>> platform_device *pdev)
>>>>
>>>> ret);
>>>>
>>>> goto err_pltfm_free;
>>>>
>>>> }
>>>>
>>>> +
>>>> + if (of_device_is_compatible(pdev->dev.of_node,
>>>> + "rockchip,rk3399-sdhci-5.1"))
>>>> + sdhci_arasan_syscon_write(host,
>>>> + &soc_ctl_map->clockmultiplier,
>>>> + 0);
>>>
>>> Variable soc_ctl_map is undefined in sdhci_arasan_probe(), please fix it.
>>
>> Woops, I change the code a bit after generating patch, I will fix it.
>>
>>> Moreover, we can not access grf_emmccore/grf_emmcphy prior to enable
>>> clk_ahb, so do it after clock enable.
>>
>> Hrmm, we configure the grf vai CPU and the clock for accessing
>> grf is aclk_emmc_grf( I saw aclk_emmc_noc is critical and it seems we
>> should treat aclk_emmc_* the same way).
>
> No :-)
>
> The aclk_emmc_grf is (similar to aclk_vio_grf etc) part of the emmc block.
>
> The noc clocks (as far as I understand) are the interconnect clocks that
> supply the link between interconnect and peripheral, so are part of the
> interconnect block.
>
> We don't model the interconnect on any rockchip soc right now, so the noc
> clocks are rightfully marked critical, but we of course do model the emmc
> block, so it should handle its grf clock, similar to how the drm driver is
> (planning to) doing it right now.
Thanks for these details. Seems we should plan to handle this inside the
sdhci-of-arasan for rockchip soc.
>
>
> Heiko
>
>
>
--
Best Regards
Shawn Lin
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Properly set corecfg_clockmultipler on rk3399
2016-08-26 8:31 ` Heiko Stübner
2016-08-26 8:36 ` Shawn Lin
@ 2016-08-26 15:46 ` Doug Anderson
[not found] ` <CAD=FV=UD-9PwQFm4x4Uqx5mQjEirD9icyYHPDaVjU4tRgk-f+g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
1 sibling, 1 reply; 7+ messages in thread
From: Doug Anderson @ 2016-08-26 15:46 UTC (permalink / raw)
To: Heiko Stübner
Cc: Shawn Lin, Ziyuan Xu, Ulf Hansson, open list:ARM/Rockchip SoC...,
linux-mmc@vger.kernel.org, Adrian Hunter
Hi,
On Fri, Aug 26, 2016 at 1:31 AM, Heiko Stübner <heiko@sntech.de> wrote:
> The aclk_emmc_grf is (similar to aclk_vio_grf etc) part of the emmc block.
Right now aclk_emmc_grf is set as CLK_IGNORE_UNUSED, which is my least
favorite clock property. ;)
Due to the parenting structure and the fact that we just added
"aclk_emmc_noc" as a critical clock, I believe that this means that
aclk_emmc_grf will never turn off. It would be awfully nice if you
could send up a patch to actually turn the "grf" clock on when needed
and turn it off when not needed. That would let us remove the
"CLK_IGNORE_UNUSED" from all of the eMMC clocks in rk3399.
I noticed you send v2 of your patch series and that looks fine to me.
The issue with not handling the "grf" clock is not a new one and
shouldn't block your patch. ...but it would still be nice to handle
properly.
-Doug
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Properly set corecfg_clockmultipler on rk3399
[not found] ` <CAD=FV=UD-9PwQFm4x4Uqx5mQjEirD9icyYHPDaVjU4tRgk-f+g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2016-08-27 2:10 ` Shawn Lin
0 siblings, 0 replies; 7+ messages in thread
From: Shawn Lin @ 2016-08-27 2:10 UTC (permalink / raw)
To: Doug Anderson, Heiko Stübner
Cc: Ulf Hansson, shawn.lin-TNX95d0MmH7DzftRWevZcw, Ziyuan Xu,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Adrian Hunter,
open list:ARM/Rockchip SoC...
On 2016/8/26 23:46, Doug Anderson wrote:
> Hi,
>
> On Fri, Aug 26, 2016 at 1:31 AM, Heiko Stübner <heiko@sntech.de> wrote:
>> The aclk_emmc_grf is (similar to aclk_vio_grf etc) part of the emmc block.
>
> Right now aclk_emmc_grf is set as CLK_IGNORE_UNUSED, which is my least
> favorite clock property. ;)
>
> Due to the parenting structure and the fact that we just added
> "aclk_emmc_noc" as a critical clock, I believe that this means that
> aclk_emmc_grf will never turn off. It would be awfully nice if you
It just means we don't gate aclk_emmc_grf/aclk_emmc_noc if not
referenced, but actually we don't prevent clk driver to gate its parent
which means it isn't really CLK_IGNORE_UNUSED or critical to a certain
degree from my personal view as we may need to propagate this feature
forward to its parents.
Anyway, that seems a bit beyond the topic of what $SUBJECT wanna handle
with. But I agree that we should make this dependency relationship more
exlicit to users anyway.
I will do it once $SUBJECT applied. :)
Thanks.
> could send up a patch to actually turn the "grf" clock on when needed
> and turn it off when not needed. That would let us remove the
> "CLK_IGNORE_UNUSED" from all of the eMMC clocks in rk3399.
>
> I noticed you send v2 of your patch series and that looks fine to me.
> The issue with not handling the "grf" clock is not a new one and
> shouldn't block your patch. ...but it would still be nice to handle
> properly.
>
> -Doug
>
>
>
--
Best Regards
Shawn Lin
_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-08-27 2:10 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-26 3:22 [PATCH] mmc: sdhci-of-arasan: Properly set corecfg_clockmultipler on rk3399 Shawn Lin
[not found] ` <1472181736-17848-1-git-send-email-shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-08-26 6:38 ` Ziyuan Xu
2016-08-26 7:50 ` Shawn Lin
[not found] ` <d6c3b728-af01-e724-3278-1bc0801f8fb4-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
2016-08-26 8:31 ` Heiko Stübner
2016-08-26 8:36 ` Shawn Lin
2016-08-26 15:46 ` Doug Anderson
[not found] ` <CAD=FV=UD-9PwQFm4x4Uqx5mQjEirD9icyYHPDaVjU4tRgk-f+g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2016-08-27 2:10 ` Shawn Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox