All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kukjin Kim <kgene@kernel.org>
To: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Alim Akhtar <alim.akhtar@samsung.com>,
	linux-kernel@vger.kernel.org,
	Krzysztof Kozlowski <k.kozlowski@samsung.com>,
	Kukjin Kim <kgene@kernel.org>,
	Douglas Anderson <dianders@chromium.org>,
	Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org
Subject: Re: [PATCH v2 0/6] ARM: dts: Use correct CD properties for SDIO and eMMC in Snow and Peach
Date: Sat, 17 Oct 2015 08:46:28 +0900	[thread overview]
Message-ID: <56218C54.3050206@kernel.org> (raw)
In-Reply-To: <5621002E.4080906@osg.samsung.com>

On 10/16/15 22:48, Javier Martinez Canillas wrote:
> Hello Alim,
> 
> On 10/16/2015 01:37 PM, Alim Akhtar wrote:
>> Hi Javier,
>>
>> On 10/15/2015 10:21 PM, Javier Martinez Canillas wrote:
>>> Hello,
>>>
>>> The Exynos Chromebooks DTS don't use the correct card detection properties
>>> since these were carried from the vendor tree that had a reason to do so.
>>>
>>> There are two things that I noticed:
>>>
>>> 1) The Marvell WiFi SDIO is marked as broken-cd instead of non-removable:
>>>
>>> This causes the device to be removed when the system enters into a suspend
>>> state which leads to the following warning when the system is resumed:
>>>
>>> [  181.944636] mmc2: error -2 during resume (card was removed?)
>>>
>>> The rationale for using broken-cd is explained in downstream commit [0] and
>>> was that using the non-removable property caused issues with the mwifiex
>>> driver since the reset logic called the mmc_{remove,add}_host() functions.
>>>
>>> But the reset logic in the mwifiex mainline driver has changed and this is
>>> no longer the case so it's safe to use the non-removable property AFAICT.
>>>
>> Good to know it is fixed now. This is fixed in firmware or in the driver?
>>
> 
> The fix is only in the driver. In fact, I'm using the same firmware from
> the ChromeOS rootfs for both the v3.8 vendor tree and mainline.
> 
> So IIUC, the problem with the vendor tree was that the mwifiex SDIO driver
> mwifiex_sdio_reset_work() function did a card reset by calling the functions
> mmc_remove_host() and then mmc_add_host().
> 
> But this didn't play nice with MMC_CAP_NONREMOVABLE since mmc_rescan() just
> returns if the flag is set so the card wouldn't be tried to be detected again.
> 
> After commit ("b4336a282db8 mwifiex: sdio: reset adapter using mmc_hw_reset"),
> the card is reset by doing a power cycle so that's why "non-removable" can be
> used now instead of "broken-cd".
> 
>>> 2) The eMMC node has both broken-cd and non-removable but the MMC DT binding
>>> doc (Documentation/devicetree/bindings/mmc/mmc.txt) mentions that only one
>>> of these card detection properties should be used.
>>>
>> This change looks ok, will take a closer look once I am back to my work station.
>>
Actually, looks good to me +1.
Let me take this series once get the feedback from Alim.

Thanks,
Kukjin

WARNING: multiple messages have this Message-ID (diff)
From: kgene@kernel.org (Kukjin Kim)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 0/6] ARM: dts: Use correct CD properties for SDIO and eMMC in Snow and Peach
Date: Sat, 17 Oct 2015 08:46:28 +0900	[thread overview]
Message-ID: <56218C54.3050206@kernel.org> (raw)
In-Reply-To: <5621002E.4080906@osg.samsung.com>

On 10/16/15 22:48, Javier Martinez Canillas wrote:
> Hello Alim,
> 
> On 10/16/2015 01:37 PM, Alim Akhtar wrote:
>> Hi Javier,
>>
>> On 10/15/2015 10:21 PM, Javier Martinez Canillas wrote:
>>> Hello,
>>>
>>> The Exynos Chromebooks DTS don't use the correct card detection properties
>>> since these were carried from the vendor tree that had a reason to do so.
>>>
>>> There are two things that I noticed:
>>>
>>> 1) The Marvell WiFi SDIO is marked as broken-cd instead of non-removable:
>>>
>>> This causes the device to be removed when the system enters into a suspend
>>> state which leads to the following warning when the system is resumed:
>>>
>>> [  181.944636] mmc2: error -2 during resume (card was removed?)
>>>
>>> The rationale for using broken-cd is explained in downstream commit [0] and
>>> was that using the non-removable property caused issues with the mwifiex
>>> driver since the reset logic called the mmc_{remove,add}_host() functions.
>>>
>>> But the reset logic in the mwifiex mainline driver has changed and this is
>>> no longer the case so it's safe to use the non-removable property AFAICT.
>>>
>> Good to know it is fixed now. This is fixed in firmware or in the driver?
>>
> 
> The fix is only in the driver. In fact, I'm using the same firmware from
> the ChromeOS rootfs for both the v3.8 vendor tree and mainline.
> 
> So IIUC, the problem with the vendor tree was that the mwifiex SDIO driver
> mwifiex_sdio_reset_work() function did a card reset by calling the functions
> mmc_remove_host() and then mmc_add_host().
> 
> But this didn't play nice with MMC_CAP_NONREMOVABLE since mmc_rescan() just
> returns if the flag is set so the card wouldn't be tried to be detected again.
> 
> After commit ("b4336a282db8 mwifiex: sdio: reset adapter using mmc_hw_reset"),
> the card is reset by doing a power cycle so that's why "non-removable" can be
> used now instead of "broken-cd".
> 
>>> 2) The eMMC node has both broken-cd and non-removable but the MMC DT binding
>>> doc (Documentation/devicetree/bindings/mmc/mmc.txt) mentions that only one
>>> of these card detection properties should be used.
>>>
>> This change looks ok, will take a closer look once I am back to my work station.
>>
Actually, looks good to me +1.
Let me take this series once get the feedback from Alim.

Thanks,
Kukjin

  reply	other threads:[~2015-10-16 23:46 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-15 16:51 [PATCH v2 0/6] ARM: dts: Use correct CD properties for SDIO and eMMC in Snow and Peach Javier Martinez Canillas
2015-10-15 16:51 ` Javier Martinez Canillas
2015-10-15 16:51 ` Javier Martinez Canillas
2015-10-15 16:51 ` [PATCH v2 1/6] ARM: dts: Mark SDIO as non-removable in exynos5800-peach-pi Javier Martinez Canillas
2015-10-15 16:51   ` Javier Martinez Canillas
2015-10-19  0:10   ` Krzysztof Kozlowski
2015-10-19  0:10     ` Krzysztof Kozlowski
2015-10-15 16:51 ` [PATCH v2 2/6] ARM: dts: Mark SDIO as non-removable in exynos5420-peach-pit Javier Martinez Canillas
2015-10-15 16:51   ` Javier Martinez Canillas
2015-10-19  0:10   ` Krzysztof Kozlowski
2015-10-19  0:10     ` Krzysztof Kozlowski
2015-10-15 16:51 ` [RFT PATCH v2 3/6] ARM: dts: Mark SDIO as non-removable in exynos5250-snow-common Javier Martinez Canillas
2015-10-15 16:51   ` Javier Martinez Canillas
2015-10-16 12:33   ` Tomeu Vizoso
2015-10-16 12:33     ` Tomeu Vizoso
2015-10-19  0:10   ` Krzysztof Kozlowski
2015-10-19  0:10     ` Krzysztof Kozlowski
2015-10-15 16:51 ` [PATCH v2 4/6] ARM: dts: Remove broken-cd from eMMC node in exynos5800-peach-pi Javier Martinez Canillas
2015-10-15 16:51   ` Javier Martinez Canillas
2015-10-19  0:11   ` Krzysztof Kozlowski
2015-10-19  0:11     ` Krzysztof Kozlowski
2015-10-15 16:51 ` [PATCH v2 5/6] ARM: dts: Remove broken-cd from eMMC node in exynos5420-peach-pi Javier Martinez Canillas
2015-10-15 16:51   ` Javier Martinez Canillas
2015-10-19  0:11   ` Krzysztof Kozlowski
2015-10-19  0:11     ` Krzysztof Kozlowski
2015-10-15 16:51 ` [RFT PATCH v2 6/6] ARM: dts: Mark eMMC as non-removable in exynos5250-snow-common Javier Martinez Canillas
2015-10-15 16:51   ` Javier Martinez Canillas
2015-10-16 12:33   ` Tomeu Vizoso
2015-10-16 12:33     ` Tomeu Vizoso
2015-10-19  0:12   ` Krzysztof Kozlowski
2015-10-19  0:12     ` Krzysztof Kozlowski
2015-10-16 11:37 ` [PATCH v2 0/6] ARM: dts: Use correct CD properties for SDIO and eMMC in Snow and Peach Alim Akhtar
2015-10-16 11:37   ` Alim Akhtar
2015-10-16 13:48   ` Javier Martinez Canillas
2015-10-16 13:48     ` Javier Martinez Canillas
2015-10-16 23:46     ` Kukjin Kim [this message]
2015-10-16 23:46       ` Kukjin Kim
2015-10-19  3:04 ` Alim Akhtar
2015-10-19  3:04   ` Alim Akhtar
2015-11-08 14:48   ` Javier Martinez Canillas
2015-11-08 14:48     ` Javier Martinez Canillas

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=56218C54.3050206@kernel.org \
    --to=kgene@kernel.org \
    --cc=alim.akhtar@samsung.com \
    --cc=dianders@chromium.org \
    --cc=javier@osg.samsung.com \
    --cc=k.kozlowski@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=tomeu.vizoso@collabora.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 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.