Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hi Michal, On Mon, Aug 17, 2015 at 8:25 PM, Michal Suchanek wrote: > Hello, > > On 17 August 2015 at 16:42, Alim Akhtar wrote: >> HI >> >> On Mon, Aug 17, 2015 at 4:56 PM, Jaehoon Chung >> wrote: >>> On 08/17/2015 02:52 PM, Michal Suchanek wrote: Hello, On 17 August 2015 at 03:55, Jaehoon Chung wrote: > Hi, Michal. > > On 08/12/2015 09:23 PM, Michal Suchanek wrote: >> The driver has open-coded test for SDIO cards. Use the mmc core provided >> MMC_QUIRK_BROKEN_CLK_GATING flag instead. > > Did you use the clock-gating for SDIO cards? > Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken? > Could you explain to me more? The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING. >>> >>> I understood your intention. And i read the comment into mmc/core/quirks.c >>> I will test SDIO card with this patch. Thanks. >>> >> When you test, please check if SDIO IRQ still works, we need to put >> dw_mmc in low_power mode otherwise SDIO IRQ will be not be generated >> by dw_mmc host controller. >> > > As far as I understand the logic which is removed in this patch and > the core logic which replaces it is the same - low power by means of > clock gating is *not* enabled for SDIO cards in either case. > > The original code also checks for SDIO IRQ and disables clock gating > regardless of card type which is probably redundant. If not it should > be fixed in mmc core. > > My recent kernel builds which I run on a system with mwifiex card > probably include this patch. > I have no objection to this patch as such, it just that some extra testing on few other boards will be good, which I am sure Jeahoon will take care. Thanks!! > Thanks > > Michal -- Regards, Alim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hello, On 17 August 2015 at 16:42, Alim Akhtar wrote: > HI > > On Mon, Aug 17, 2015 at 4:56 PM, Jaehoon Chung wrote: >> On 08/17/2015 02:52 PM, Michal Suchanek wrote: >>> Hello, >>> >>> On 17 August 2015 at 03:55, Jaehoon Chung wrote: Hi, Michal. On 08/12/2015 09:23 PM, Michal Suchanek wrote: > The driver has open-coded test for SDIO cards. Use the mmc core provided > MMC_QUIRK_BROKEN_CLK_GATING flag instead. Did you use the clock-gating for SDIO cards? Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken? Could you explain to me more? >>> >>> The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING. >> >> I understood your intention. And i read the comment into mmc/core/quirks.c >> I will test SDIO card with this patch. Thanks. >> > When you test, please check if SDIO IRQ still works, we need to put > dw_mmc in low_power mode otherwise SDIO IRQ will be not be generated > by dw_mmc host controller. > As far as I understand the logic which is removed in this patch and the core logic which replaces it is the same - low power by means of clock gating is *not* enabled for SDIO cards in either case. The original code also checks for SDIO IRQ and disables clock gating regardless of card type which is probably redundant. If not it should be fixed in mmc core. My recent kernel builds which I run on a system with mwifiex card probably include this patch. Thanks Michal -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
HI
On Mon, Aug 17, 2015 at 4:56 PM, Jaehoon Chung wrote:
> On 08/17/2015 02:52 PM, Michal Suchanek wrote:
>> Hello,
>>
>> On 17 August 2015 at 03:55, Jaehoon Chung wrote:
>>> Hi, Michal.
>>>
>>> On 08/12/2015 09:23 PM, Michal Suchanek wrote:
The driver has open-coded test for SDIO cards. Use the mmc core provided
MMC_QUIRK_BROKEN_CLK_GATING flag instead.
>>>
>>> Did you use the clock-gating for SDIO cards?
>>> Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken?
>>> Could you explain to me more?
>>
>> The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING.
>
> I understood your intention. And i read the comment into mmc/core/quirks.c
> I will test SDIO card with this patch. Thanks.
>
When you test, please check if SDIO IRQ still works, we need to put
dw_mmc in low_power mode otherwise SDIO IRQ will be not be generated
by dw_mmc host controller.
> Best Regards,
> Jaehoon Chung
>
>>
>> It may coincide with MMC_CAP_SDIO_IRQ but that's different flag for
>> different purpose.
>>
>> MMC_QUIRK_BROKEN_CLK_GATING is currently set for all SDIO cards except
>> for cards on a whitelist.
>>
>> I don't know any particular SDIO card that has problems when the clock
>> is off and does not use an IRQ.
>>
>> Thanks
>>
>> Michal
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
As a bonus this may enable clock gating on SDIO cards that are known to
work with it.
Signed-off-by: Michal Suchanek
---
drivers/mmc/host/dw_mmc.c | 33 +++--
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 40e9d8e..3bc9fd7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1335,27 +1335,24 @@ static void dw_mci_init_card(struct mmc_host *mmc,
struct mmc_card *card)
* description of the CLKENA register we should disable low power
mode
* for SDIO cards if we need SDIO interrupts to work.
*/
- if (mmc->caps & MMC_CAP_SDIO_IRQ) {
- const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
- u32 clk_en_a_old;
- u32 clk_en_a;
+ const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
+ u32 clk_en_a_old;
+ u32 clk_en_a;
- clk_en_a_old = mci_readl(host, CLKENA);
+ clk_en_a_old = mci_readl(host, CLKENA);
- if (card->type == MMC_TYPE_SDIO ||
- card->type == MMC_TYPE_SD_COMBO) {
- set_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
- clk_en_a = clk_en_a_old & ~clken_low_pwr;
- } else {
- clear_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
- clk_en_a = clk_en_a_old | clken_low_pwr;
- }
+ if (card->quirks & MMC_QUIRK_BROKEN_CLK_GATING) {
+ set_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
+ clk_en_a = clk_en_a_old & ~clken_low_pwr;
+ } else {
+ clear_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
+ clk_en_a = clk_en_a_old | clken_low_pwr;
+ }
- if (clk_en_a != clk_en_a_old) {
- mci_writel(host, CLKENA, clk_en_a);
- mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
- SDMMC_CMD_PRV_DAT_WAIT, 0);
- }
+ if (clk_en_a != clk_en_a_old) {
+ mci_writel(host, CLKENA, clk_en_a);
+ mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+ SDMMC_CMD_PRV_DAT_WAIT, 0);
}
}
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Regards,
Alim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
On 08/17/2015 02:52 PM, Michal Suchanek wrote:
> Hello,
>
> On 17 August 2015 at 03:55, Jaehoon Chung wrote:
>> Hi, Michal.
>>
>> On 08/12/2015 09:23 PM, Michal Suchanek wrote:
>>> The driver has open-coded test for SDIO cards. Use the mmc core provided
>>> MMC_QUIRK_BROKEN_CLK_GATING flag instead.
>>
>> Did you use the clock-gating for SDIO cards?
>> Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken?
>> Could you explain to me more?
>
> The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING.
I understood your intention. And i read the comment into mmc/core/quirks.c
I will test SDIO card with this patch. Thanks.
Best Regards,
Jaehoon Chung
>
> It may coincide with MMC_CAP_SDIO_IRQ but that's different flag for
> different purpose.
>
> MMC_QUIRK_BROKEN_CLK_GATING is currently set for all SDIO cards except
> for cards on a whitelist.
>
> I don't know any particular SDIO card that has problems when the clock
> is off and does not use an IRQ.
>
> Thanks
>
> Michal
>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> As a bonus this may enable clock gating on SDIO cards that are known to
>>> work with it.
>>>
>>> Signed-off-by: Michal Suchanek
>>> ---
>>> drivers/mmc/host/dw_mmc.c | 33 +++--
>>> 1 file changed, 15 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 40e9d8e..3bc9fd7 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -1335,27 +1335,24 @@ static void dw_mci_init_card(struct mmc_host *mmc,
>>> struct mmc_card *card)
>>>* description of the CLKENA register we should disable low power mode
>>>* for SDIO cards if we need SDIO interrupts to work.
>>>*/
>>> - if (mmc->caps & MMC_CAP_SDIO_IRQ) {
>>> - const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
>>> - u32 clk_en_a_old;
>>> - u32 clk_en_a;
>>> + const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
>>> + u32 clk_en_a_old;
>>> + u32 clk_en_a;
>>>
>>> - clk_en_a_old = mci_readl(host, CLKENA);
>>> + clk_en_a_old = mci_readl(host, CLKENA);
>>>
>>> - if (card->type == MMC_TYPE_SDIO ||
>>> - card->type == MMC_TYPE_SD_COMBO) {
>>> - set_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
>>> - clk_en_a = clk_en_a_old & ~clken_low_pwr;
>>> - } else {
>>> - clear_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
>>> - clk_en_a = clk_en_a_old | clken_low_pwr;
>>> - }
>>> + if (card->quirks & MMC_QUIRK_BROKEN_CLK_GATING) {
>>> + set_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
>>> + clk_en_a = clk_en_a_old & ~clken_low_pwr;
>>> + } else {
>>> + clear_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
>>> + clk_en_a = clk_en_a_old | clken_low_pwr;
>>> + }
>>>
>>> - if (clk_en_a != clk_en_a_old) {
>>> - mci_writel(host, CLKENA, clk_en_a);
>>> - mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
>>> - SDMMC_CMD_PRV_DAT_WAIT, 0);
>>> - }
>>> + if (clk_en_a != clk_en_a_old) {
>>> + mci_writel(host, CLKENA, clk_en_a);
>>> + mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
>>> + SDMMC_CMD_PRV_DAT_WAIT, 0);
>>> }
>>> }
>>>
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
On 08/17/2015 02:52 PM, Michal Suchanek wrote:
Hello,
On 17 August 2015 at 03:55, Jaehoon Chung jh80.ch...@samsung.com wrote:
Hi, Michal.
On 08/12/2015 09:23 PM, Michal Suchanek wrote:
The driver has open-coded test for SDIO cards. Use the mmc core provided
MMC_QUIRK_BROKEN_CLK_GATING flag instead.
Did you use the clock-gating for SDIO cards?
Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken?
Could you explain to me more?
The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING.
I understood your intention. And i read the comment into mmc/core/quirks.c
I will test SDIO card with this patch. Thanks.
Best Regards,
Jaehoon Chung
It may coincide with MMC_CAP_SDIO_IRQ but that's different flag for
different purpose.
MMC_QUIRK_BROKEN_CLK_GATING is currently set for all SDIO cards except
for cards on a whitelist.
I don't know any particular SDIO card that has problems when the clock
is off and does not use an IRQ.
Thanks
Michal
Best Regards,
Jaehoon Chung
As a bonus this may enable clock gating on SDIO cards that are known to
work with it.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mmc/host/dw_mmc.c | 33 +++--
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 40e9d8e..3bc9fd7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1335,27 +1335,24 @@ static void dw_mci_init_card(struct mmc_host *mmc,
struct mmc_card *card)
* description of the CLKENA register we should disable low power mode
* for SDIO cards if we need SDIO interrupts to work.
*/
- if (mmc-caps MMC_CAP_SDIO_IRQ) {
- const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
- u32 clk_en_a_old;
- u32 clk_en_a;
+ const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
+ u32 clk_en_a_old;
+ u32 clk_en_a;
- clk_en_a_old = mci_readl(host, CLKENA);
+ clk_en_a_old = mci_readl(host, CLKENA);
- if (card-type == MMC_TYPE_SDIO ||
- card-type == MMC_TYPE_SD_COMBO) {
- set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old ~clken_low_pwr;
- } else {
- clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old | clken_low_pwr;
- }
+ if (card-quirks MMC_QUIRK_BROKEN_CLK_GATING) {
+ set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old ~clken_low_pwr;
+ } else {
+ clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old | clken_low_pwr;
+ }
- if (clk_en_a != clk_en_a_old) {
- mci_writel(host, CLKENA, clk_en_a);
- mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
- SDMMC_CMD_PRV_DAT_WAIT, 0);
- }
+ if (clk_en_a != clk_en_a_old) {
+ mci_writel(host, CLKENA, clk_en_a);
+ mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+ SDMMC_CMD_PRV_DAT_WAIT, 0);
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
HI
On Mon, Aug 17, 2015 at 4:56 PM, Jaehoon Chung jh80.ch...@samsung.com wrote:
On 08/17/2015 02:52 PM, Michal Suchanek wrote:
Hello,
On 17 August 2015 at 03:55, Jaehoon Chung jh80.ch...@samsung.com wrote:
Hi, Michal.
On 08/12/2015 09:23 PM, Michal Suchanek wrote:
The driver has open-coded test for SDIO cards. Use the mmc core provided
MMC_QUIRK_BROKEN_CLK_GATING flag instead.
Did you use the clock-gating for SDIO cards?
Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken?
Could you explain to me more?
The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING.
I understood your intention. And i read the comment into mmc/core/quirks.c
I will test SDIO card with this patch. Thanks.
When you test, please check if SDIO IRQ still works, we need to put
dw_mmc in low_power mode otherwise SDIO IRQ will be not be generated
by dw_mmc host controller.
Best Regards,
Jaehoon Chung
It may coincide with MMC_CAP_SDIO_IRQ but that's different flag for
different purpose.
MMC_QUIRK_BROKEN_CLK_GATING is currently set for all SDIO cards except
for cards on a whitelist.
I don't know any particular SDIO card that has problems when the clock
is off and does not use an IRQ.
Thanks
Michal
Best Regards,
Jaehoon Chung
As a bonus this may enable clock gating on SDIO cards that are known to
work with it.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mmc/host/dw_mmc.c | 33 +++--
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 40e9d8e..3bc9fd7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1335,27 +1335,24 @@ static void dw_mci_init_card(struct mmc_host *mmc,
struct mmc_card *card)
* description of the CLKENA register we should disable low power
mode
* for SDIO cards if we need SDIO interrupts to work.
*/
- if (mmc-caps MMC_CAP_SDIO_IRQ) {
- const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
- u32 clk_en_a_old;
- u32 clk_en_a;
+ const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
+ u32 clk_en_a_old;
+ u32 clk_en_a;
- clk_en_a_old = mci_readl(host, CLKENA);
+ clk_en_a_old = mci_readl(host, CLKENA);
- if (card-type == MMC_TYPE_SDIO ||
- card-type == MMC_TYPE_SD_COMBO) {
- set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old ~clken_low_pwr;
- } else {
- clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old | clken_low_pwr;
- }
+ if (card-quirks MMC_QUIRK_BROKEN_CLK_GATING) {
+ set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old ~clken_low_pwr;
+ } else {
+ clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old | clken_low_pwr;
+ }
- if (clk_en_a != clk_en_a_old) {
- mci_writel(host, CLKENA, clk_en_a);
- mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
- SDMMC_CMD_PRV_DAT_WAIT, 0);
- }
+ if (clk_en_a != clk_en_a_old) {
+ mci_writel(host, CLKENA, clk_en_a);
+ mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+ SDMMC_CMD_PRV_DAT_WAIT, 0);
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Regards,
Alim
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hi Michal, On Mon, Aug 17, 2015 at 8:25 PM, Michal Suchanek hramr...@gmail.com wrote: Hello, On 17 August 2015 at 16:42, Alim Akhtar alim.akh...@gmail.com wrote: HI On Mon, Aug 17, 2015 at 4:56 PM, Jaehoon Chung jh80.ch...@samsung.com wrote: On 08/17/2015 02:52 PM, Michal Suchanek wrote: Hello, On 17 August 2015 at 03:55, Jaehoon Chung jh80.ch...@samsung.com wrote: Hi, Michal. On 08/12/2015 09:23 PM, Michal Suchanek wrote: The driver has open-coded test for SDIO cards. Use the mmc core provided MMC_QUIRK_BROKEN_CLK_GATING flag instead. Did you use the clock-gating for SDIO cards? Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken? Could you explain to me more? The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING. I understood your intention. And i read the comment into mmc/core/quirks.c I will test SDIO card with this patch. Thanks. When you test, please check if SDIO IRQ still works, we need to put dw_mmc in low_power mode otherwise SDIO IRQ will be not be generated by dw_mmc host controller. As far as I understand the logic which is removed in this patch and the core logic which replaces it is the same - low power by means of clock gating is *not* enabled for SDIO cards in either case. The original code also checks for SDIO IRQ and disables clock gating regardless of card type which is probably redundant. If not it should be fixed in mmc core. My recent kernel builds which I run on a system with mwifiex card probably include this patch. I have no objection to this patch as such, it just that some extra testing on few other boards will be good, which I am sure Jeahoon will take care. Thanks!! Thanks Michal -- Regards, Alim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hello, On 17 August 2015 at 16:42, Alim Akhtar alim.akh...@gmail.com wrote: HI On Mon, Aug 17, 2015 at 4:56 PM, Jaehoon Chung jh80.ch...@samsung.com wrote: On 08/17/2015 02:52 PM, Michal Suchanek wrote: Hello, On 17 August 2015 at 03:55, Jaehoon Chung jh80.ch...@samsung.com wrote: Hi, Michal. On 08/12/2015 09:23 PM, Michal Suchanek wrote: The driver has open-coded test for SDIO cards. Use the mmc core provided MMC_QUIRK_BROKEN_CLK_GATING flag instead. Did you use the clock-gating for SDIO cards? Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken? Could you explain to me more? The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING. I understood your intention. And i read the comment into mmc/core/quirks.c I will test SDIO card with this patch. Thanks. When you test, please check if SDIO IRQ still works, we need to put dw_mmc in low_power mode otherwise SDIO IRQ will be not be generated by dw_mmc host controller. As far as I understand the logic which is removed in this patch and the core logic which replaces it is the same - low power by means of clock gating is *not* enabled for SDIO cards in either case. The original code also checks for SDIO IRQ and disables clock gating regardless of card type which is probably redundant. If not it should be fixed in mmc core. My recent kernel builds which I run on a system with mwifiex card probably include this patch. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hello,
On 17 August 2015 at 03:55, Jaehoon Chung wrote:
> Hi, Michal.
>
> On 08/12/2015 09:23 PM, Michal Suchanek wrote:
>> The driver has open-coded test for SDIO cards. Use the mmc core provided
>> MMC_QUIRK_BROKEN_CLK_GATING flag instead.
>
> Did you use the clock-gating for SDIO cards?
> Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken?
> Could you explain to me more?
The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING.
It may coincide with MMC_CAP_SDIO_IRQ but that's different flag for
different purpose.
MMC_QUIRK_BROKEN_CLK_GATING is currently set for all SDIO cards except
for cards on a whitelist.
I don't know any particular SDIO card that has problems when the clock
is off and does not use an IRQ.
Thanks
Michal
>
> Best Regards,
> Jaehoon Chung
>
>>
>> As a bonus this may enable clock gating on SDIO cards that are known to
>> work with it.
>>
>> Signed-off-by: Michal Suchanek
>> ---
>> drivers/mmc/host/dw_mmc.c | 33 +++--
>> 1 file changed, 15 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 40e9d8e..3bc9fd7 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -1335,27 +1335,24 @@ static void dw_mci_init_card(struct mmc_host *mmc,
>> struct mmc_card *card)
>>* description of the CLKENA register we should disable low power mode
>>* for SDIO cards if we need SDIO interrupts to work.
>>*/
>> - if (mmc->caps & MMC_CAP_SDIO_IRQ) {
>> - const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
>> - u32 clk_en_a_old;
>> - u32 clk_en_a;
>> + const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
>> + u32 clk_en_a_old;
>> + u32 clk_en_a;
>>
>> - clk_en_a_old = mci_readl(host, CLKENA);
>> + clk_en_a_old = mci_readl(host, CLKENA);
>>
>> - if (card->type == MMC_TYPE_SDIO ||
>> - card->type == MMC_TYPE_SD_COMBO) {
>> - set_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
>> - clk_en_a = clk_en_a_old & ~clken_low_pwr;
>> - } else {
>> - clear_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
>> - clk_en_a = clk_en_a_old | clken_low_pwr;
>> - }
>> + if (card->quirks & MMC_QUIRK_BROKEN_CLK_GATING) {
>> + set_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
>> + clk_en_a = clk_en_a_old & ~clken_low_pwr;
>> + } else {
>> + clear_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
>> + clk_en_a = clk_en_a_old | clken_low_pwr;
>> + }
>>
>> - if (clk_en_a != clk_en_a_old) {
>> - mci_writel(host, CLKENA, clk_en_a);
>> - mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
>> - SDMMC_CMD_PRV_DAT_WAIT, 0);
>> - }
>> + if (clk_en_a != clk_en_a_old) {
>> + mci_writel(host, CLKENA, clk_en_a);
>> + mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
>> + SDMMC_CMD_PRV_DAT_WAIT, 0);
>> }
>> }
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hi, Michal.
On 08/12/2015 09:23 PM, Michal Suchanek wrote:
> The driver has open-coded test for SDIO cards. Use the mmc core provided
> MMC_QUIRK_BROKEN_CLK_GATING flag instead.
Did you use the clock-gating for SDIO cards?
Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken?
Could you explain to me more?
Best Regards,
Jaehoon Chung
>
> As a bonus this may enable clock gating on SDIO cards that are known to
> work with it.
>
> Signed-off-by: Michal Suchanek
> ---
> drivers/mmc/host/dw_mmc.c | 33 +++--
> 1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 40e9d8e..3bc9fd7 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -1335,27 +1335,24 @@ static void dw_mci_init_card(struct mmc_host *mmc,
> struct mmc_card *card)
>* description of the CLKENA register we should disable low power mode
>* for SDIO cards if we need SDIO interrupts to work.
>*/
> - if (mmc->caps & MMC_CAP_SDIO_IRQ) {
> - const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
> - u32 clk_en_a_old;
> - u32 clk_en_a;
> + const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR << slot->id;
> + u32 clk_en_a_old;
> + u32 clk_en_a;
>
> - clk_en_a_old = mci_readl(host, CLKENA);
> + clk_en_a_old = mci_readl(host, CLKENA);
>
> - if (card->type == MMC_TYPE_SDIO ||
> - card->type == MMC_TYPE_SD_COMBO) {
> - set_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
> - clk_en_a = clk_en_a_old & ~clken_low_pwr;
> - } else {
> - clear_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
> - clk_en_a = clk_en_a_old | clken_low_pwr;
> - }
> + if (card->quirks & MMC_QUIRK_BROKEN_CLK_GATING) {
> + set_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
> + clk_en_a = clk_en_a_old & ~clken_low_pwr;
> + } else {
> + clear_bit(DW_MMC_CARD_NO_LOW_PWR, >flags);
> + clk_en_a = clk_en_a_old | clken_low_pwr;
> + }
>
> - if (clk_en_a != clk_en_a_old) {
> - mci_writel(host, CLKENA, clk_en_a);
> - mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
> - SDMMC_CMD_PRV_DAT_WAIT, 0);
> - }
> + if (clk_en_a != clk_en_a_old) {
> + mci_writel(host, CLKENA, clk_en_a);
> + mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
> + SDMMC_CMD_PRV_DAT_WAIT, 0);
> }
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hi, Michal.
On 08/12/2015 09:23 PM, Michal Suchanek wrote:
The driver has open-coded test for SDIO cards. Use the mmc core provided
MMC_QUIRK_BROKEN_CLK_GATING flag instead.
Did you use the clock-gating for SDIO cards?
Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken?
Could you explain to me more?
Best Regards,
Jaehoon Chung
As a bonus this may enable clock gating on SDIO cards that are known to
work with it.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mmc/host/dw_mmc.c | 33 +++--
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 40e9d8e..3bc9fd7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1335,27 +1335,24 @@ static void dw_mci_init_card(struct mmc_host *mmc,
struct mmc_card *card)
* description of the CLKENA register we should disable low power mode
* for SDIO cards if we need SDIO interrupts to work.
*/
- if (mmc-caps MMC_CAP_SDIO_IRQ) {
- const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
- u32 clk_en_a_old;
- u32 clk_en_a;
+ const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
+ u32 clk_en_a_old;
+ u32 clk_en_a;
- clk_en_a_old = mci_readl(host, CLKENA);
+ clk_en_a_old = mci_readl(host, CLKENA);
- if (card-type == MMC_TYPE_SDIO ||
- card-type == MMC_TYPE_SD_COMBO) {
- set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old ~clken_low_pwr;
- } else {
- clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old | clken_low_pwr;
- }
+ if (card-quirks MMC_QUIRK_BROKEN_CLK_GATING) {
+ set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old ~clken_low_pwr;
+ } else {
+ clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old | clken_low_pwr;
+ }
- if (clk_en_a != clk_en_a_old) {
- mci_writel(host, CLKENA, clk_en_a);
- mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
- SDMMC_CMD_PRV_DAT_WAIT, 0);
- }
+ if (clk_en_a != clk_en_a_old) {
+ mci_writel(host, CLKENA, clk_en_a);
+ mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+ SDMMC_CMD_PRV_DAT_WAIT, 0);
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] mmc: dw_mmc: simplify the SDMMC_CLKEN_LOW_PWR logic
Hello,
On 17 August 2015 at 03:55, Jaehoon Chung jh80.ch...@samsung.com wrote:
Hi, Michal.
On 08/12/2015 09:23 PM, Michal Suchanek wrote:
The driver has open-coded test for SDIO cards. Use the mmc core provided
MMC_QUIRK_BROKEN_CLK_GATING flag instead.
Did you use the clock-gating for SDIO cards?
Doesn't MMC_CAP_SDIO_IRQ bit set? Which case is broken?
Could you explain to me more?
The core flag for disabling power saving is MMC_QUIRK_BROKEN_CLK_GATING.
It may coincide with MMC_CAP_SDIO_IRQ but that's different flag for
different purpose.
MMC_QUIRK_BROKEN_CLK_GATING is currently set for all SDIO cards except
for cards on a whitelist.
I don't know any particular SDIO card that has problems when the clock
is off and does not use an IRQ.
Thanks
Michal
Best Regards,
Jaehoon Chung
As a bonus this may enable clock gating on SDIO cards that are known to
work with it.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
drivers/mmc/host/dw_mmc.c | 33 +++--
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 40e9d8e..3bc9fd7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1335,27 +1335,24 @@ static void dw_mci_init_card(struct mmc_host *mmc,
struct mmc_card *card)
* description of the CLKENA register we should disable low power mode
* for SDIO cards if we need SDIO interrupts to work.
*/
- if (mmc-caps MMC_CAP_SDIO_IRQ) {
- const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
- u32 clk_en_a_old;
- u32 clk_en_a;
+ const u32 clken_low_pwr = SDMMC_CLKEN_LOW_PWR slot-id;
+ u32 clk_en_a_old;
+ u32 clk_en_a;
- clk_en_a_old = mci_readl(host, CLKENA);
+ clk_en_a_old = mci_readl(host, CLKENA);
- if (card-type == MMC_TYPE_SDIO ||
- card-type == MMC_TYPE_SD_COMBO) {
- set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old ~clken_low_pwr;
- } else {
- clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
- clk_en_a = clk_en_a_old | clken_low_pwr;
- }
+ if (card-quirks MMC_QUIRK_BROKEN_CLK_GATING) {
+ set_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old ~clken_low_pwr;
+ } else {
+ clear_bit(DW_MMC_CARD_NO_LOW_PWR, slot-flags);
+ clk_en_a = clk_en_a_old | clken_low_pwr;
+ }
- if (clk_en_a != clk_en_a_old) {
- mci_writel(host, CLKENA, clk_en_a);
- mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
- SDMMC_CMD_PRV_DAT_WAIT, 0);
- }
+ if (clk_en_a != clk_en_a_old) {
+ mci_writel(host, CLKENA, clk_en_a);
+ mci_send_cmd(slot, SDMMC_CMD_UPD_CLK |
+ SDMMC_CMD_PRV_DAT_WAIT, 0);
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
