* Re: [PATCH] mmc: block: delete packed command support
From: Arnd Bergmann @ 2016-11-21 11:11 UTC (permalink / raw)
To: Linus Walleij
Cc: linux-mmc, Ulf Hansson, Chunyan Zhang, Baolin Wang, Jaehoon Chung,
Namjae Jeon, Maya Erez, Luca Porzio, Alex Lemberg
In-Reply-To: <1479722937-19551-1-git-send-email-linus.walleij@linaro.org>
On Monday, November 21, 2016 11:08:57 AM CET Linus Walleij wrote:
> I've had it with this code now.
>
> The packed command support is a complex hurdle in the MMC/SD block
> layer, around 500+ lines of code which was introduced in 2013 in
> commits
>
> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>
> ...and since then it has been rotting. The original author of the
> code has disappeared from the community and the mail address is
> bouncing.
>
> For the code to be exercised the host must flag that it supports
> packed commands, so in mmc_blk_prep_packed_list() which is called for
> every single request, the following construction appears:
>
> u8 max_packed_rw = 0;
>
> if ((rq_data_dir(cur) == WRITE) &&
> mmc_host_packed_wr(card->host))
> max_packed_rw = card->ext_csd.max_packed_writes;
>
> if (max_packed_rw == 0)
> goto no_packed;
>
> This has the following logical deductions:
>
> - Only WRITE commands can really be packed, so the solution is
> only half-done: we support packed WRITE but not packed READ.
> The packed command support has not been finalized by supporting
> reads in three years!
Packed reads don't make a lot of sense, there is very little
for an MMC to optimize in reads that it can't already do without
the packing. For writes, packing makes could be an important
performance optimization, if the eMMC supports it.
I've added Luca Porzio and Alex Lemberg to Cc. I think they
are subscribed to the list already, but it would be good to
get some estimate from them too about how common the packed
write support is on existing hardware from their respective
employers before we kill it off.
If none of Samsung/Micron/Sandisk are currently shipping
devices that support eMMC-4.5 packed commands but don't
support the eMMC-5.1 command queuing (which IIRC is a more
general way to achieve the same), we probably don't need to
worry about it too much.
> - mmc_host_packed_wr() is just a static inline that checks
> host->caps2 & MMC_CAP2_PACKED_WR. The problem with this is
> that NO upstream host sets this capability flag! No driver
> in the kernel is using it, and we can't test it. Packed
> command may be supported in out-of-tree code, but I doubt
> it. I doubt that the code is even working anymore due to
> other refactorings in the MMC block layer, who would
> notice if patches affecting it broke packed commands?
> No one.
This is a very good indication that it's not really used.
It would be useful to also check out the Android AOSP
kernel tree to see if it's in there, if anything important
supports packed commands, it's probably in there.
Arnd
^ permalink raw reply
* Re: [PATCH v9 00/16] mmc: sdhci-msm: Add clk-rates, DDR, HS400 support
From: Ritesh Harjani @ 2016-11-21 11:42 UTC (permalink / raw)
To: Ulf Hansson, Stephen Boyd, Andy Gross
Cc: linux-mmc, Adrian Hunter, Shawn Lin, devicetree@vger.kernel.org,
linux-clk, David Brown, linux-arm-msm@vger.kernel.org,
Georgi Djakov, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Asutosh Das, David Griego, Sahitya Tummala, Venkat Gopalakrishnan,
Rajendra Nayak, Pramod Gurav, jeremymc
In-Reply-To: <CAPDyKFrRbf0+G5K=-jsaevAGbTqhTAss2zJv3QdiyCrv0BG-zA@mail.gmail.com>
On 11/21/2016 3:36 PM, Ulf Hansson wrote:
> On 21 November 2016 at 07:37, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>> Hi,
>>
>> This is v9 version of the patch series which adds support for MSM8996.
>> Adds HS400 driver support as well.
>> These are tested on internal msm8996 & db410c HW.
>>
>> The patch series is ready. Do we think we can apply these
>> patches for next now?
>
> I guess the DTS changes can be picked up by Andy, so they can go via arm-soc?
Yes.
>
> Then, does the mmc changes depend on the clock changes? If so, I can
> pick them as well, but then I need an ack from Stephen....
Ideal and preferable, would be that clk & mmc changes go in together.
But either ways should be fine.
>
> Kind regards
> Uffe
>
>>
>> There are only minor changes in v9.
>> 1. From <&xo_board 0> -> <&xo_board>.
>> 2. Addressed Adrian minor comments on 009.
>> 3. Other minor changes.
>>
>>
>> Older history:-
>>
>> Changes from v7 -> v8 :-
>> 1. Added patch 005 to add dt bindings for xo_clock.
>> 2. Added patch 009 to factor out sdhci_enable_clock as discussed on v7 series.
>> 2.a. Modified patch 010 by making use of sdhci_enable_clock.
>> 2.b. Addressed Stephen's comment on patch 010 to call clk_set_rate unconditionally.
>> 3. Addressed Stephen comments to remove unncessary one line comments, braces and other
>> minor comments.
>> 4. Added changes from Jeremy in patch 002 for gcc-msm8994 as well for sdcc clk_rcg2_floor_ops.
>> minor comments.
>>
>> v7 was verified on my Nexus 5X (msm8992).
>>
>> Older history :-
>> Below are the changes in v7.
>>
>> Changes from v6 -> v7 :-
>> 1. Removed patch "clk: Add clk_hw_get_clk() helper API to be used by clk providers"
>> in v7 as it was not required.
>> 2. Addressed Stephen review comments on -
>> "clk: qcom: Add rcg ops to return floor value closest to the requested rate"
>> 3. Addressed comments from Stephen to add xo_clock entry in the sdhc clock node.
>> Using the same xo_clock entry from DT to get the clk_rate of xo_clock used in
>> sdhci-msm driver. Patch 04 adds this entry into DT.
>> Patch 05 adds the driver support for xo_clock mentioned above.
>> Hence there is a minor change in Patch05, which can be reviewed and taken
>> into the tree.
>>
>> IMHO, almost all patches are almost done and are ready to be accepted.
>> Will below process work out?
>> Patches 001 & 002 :- (clock changes) - Can go via Stephen's Boyd Tree.
>> Patches 004 & 010 :- (DTS changes) - Can go via Andy Gross.
>> Patches 003, 005-009 & 011-014 :- (sdhci-msm changes) - Adrian's tree.
>>
>> Please let me know in case if anything else is required on above.
>>
>>
>> Changes from v5 -> v6 :-
>> 1. Earlier in v5 series DT node was added to get the clk-rates table
>> needed for sdhci-msm driver. But this is removed in this(v6) patch series
>> and instead the clk changes are done in the clk driver as per Rob H comment.
>>
>> 2. Added clk driver changes(patch 1-3) to provide floor rate values of requested
>> clock for sdhc client.
>> For following boards- apq8084, msm8996, msm8916, msm8974.
>>
>> 3. Other minor patch comments were addressed.
>>
>> Changes from v4 -> v5 :-
>> 1. Added HS400 sdhci-msm controller specific changes:- (Patch 10, 11, 12)
>> 2. Addressed comment from Adrian on Patch 07 @[3].
>> 3. Addressed comment from Arnd on Patch 03, to directly add
>> clk_table into sdhci_msm_host. [4]
>> 4. Addressed comment from Bjorn to not enforce having clk-rates property
>> in DT for older targets based on discussion at [5]
>> 5. Retained Acks from Adrian on patches (01 & 02 & 06) where there were no
>> changes made while addressing above comments.
>>
>> Older history:-
>> This is v4 version of the patch series.
>> Patches 01, 02, 05 & 06 were Acked-by Adrian.
>>
>> Changes from v3 -> v4 :-
>> 1. Addressed comments from Adrian on Patch 03, 07, 08.
>> 2. Addressed comments from Bjorn on Patch 03.
>> 3. Added clk-rate support for sdhc DT nodes to all MSM platforms.
>> in Pacth 04.
>> 4. Rebased on next branch of Ulf.
>>
>> Changes from v2 -> v3 :-
>> 1. Addded Patch 01 based on Bjorn comment[2] -
>> This fixes/unrolls the poor coding style of read/writes of
>> registers from base sdhci-msm driver.
>>
>> 2. Fixed/unrolled poor style of reads/writes of registers in Patch 02,
>> based on Bjorn comment[2]. Also changed name of flag from
>> use_updated_dll_reset -> use_14lpp_dll_reset.
>>
>> Changes from v1->v2 :-
>> 1. Removed patch 06 & 08 from v1 patch series[1]
>> (which were introducing unnecessary quirks).
>> Instead have implemented __sdhci_msm_set_clock version of
>> sdhci_set_clock in sdhci_msm driver itself in patch 07 of
>> this patch series.
>> 2. Enabled extra quirk (SDHCI_QUIRK2_PRESET_VALUE_BROKEN) in
>> patch 05 of this patch series.
>>
>>
>> Description of patches :-
>> This patchset adds clk-rates & other required changes to
>> upstream sdhci-msm driver from codeaurora tree.
>> It has been tested on a db410c Dragonboard and msm8996 based
>> platform.
>>
>> Patch 0001-0003- Adds support in qcom clk driver to return
>> floor value of requested clock rate instead of ceil rate
>> for sdhc clients.
>>
>> Patch 0004- Adds updated dll sequence for newer controllers
>> which has minor_version >= 0x42. This is required for msm8996.
>>
>> MSM controller HW recommendation is to use the base MCI clock
>> and directly control this MCI clock at GCC in order to
>> change the clk-rate.
>> Patches 06-08 bring in required change for this to
>> sdhci-msm.
>>
>> MSM controller would require 2x clock rate from source
>> for DDR bus speed modes. Patch 09 adds this support.
>>
>> Patch 0010- adds DDR support in DT for sdhc1 of msm8916.
>>
>> Patches 0011-0014- Adds HS400 support to sdhci-msm.
>>
>>
>> [1]:- http://www.spinics.net/lists/linux-mmc/msg38467.html
>> [2]:- http://www.spinics.net/lists/linux-mmc/msg38578.html
>> [3]:- https://patchwork.kernel.org/patch/9289345/
>> [4]:- https://www.spinics.net/lists/linux-mmc/msg39107.html
>> [5]:- http://www.spinics.net/lists/linux-mmc/msg38749.html
>> [6]:- https://patchwork.kernel.org/patch/9297381/
>>
>>
>> Rajendra Nayak (2):
>> clk: qcom: Add rcg ops to return floor value closest to the requested
>> rate
>> clk: qcom: Move all sdcc rcgs to use clk_rcg2_floor_ops
>>
>> Ritesh Harjani (12):
>> mmc: sdhci-msm: Change poor style writel/readl of registers
>> ARM: dts: Add xo to sdhc clock node on qcom platforms
>> dt-bindings: sdhci-msm: Add xo value
>> mmc: sdhci-msm: Add get_min_clock() and get_max_clock() callback
>> mmc: sdhci-msm: Enable few quirks
>> mmc: sdhci: Factor out sdhci_enable_clk
>> mmc: sdhci-msm: Implement set_clock callback for sdhci-msm
>> mmc: sdhci-msm: Add clock changes for DDR mode.
>> arm64: dts: qcom: msm8916: Add ddr support to sdhc1
>> mmc: sdhci-msm: Save the calculated tuning phase
>> mmc: sdhci-msm: Add calibration tuning for CDCLP533 circuit
>> sdhci: sdhci-msm: update dll configuration
>>
>> Venkat Gopalakrishnan (2):
>> mmc: sdhci-msm: Update DLL reset sequence
>> mmc: sdhci-msm: Add HS400 platform support
>>
>> .../devicetree/bindings/mmc/sdhci-msm.txt | 1 +
>> arch/arm/boot/dts/qcom-apq8084.dtsi | 16 +-
>> arch/arm/boot/dts/qcom-msm8974.dtsi | 16 +-
>> arch/arm64/boot/dts/qcom/msm8916.dtsi | 11 +-
>> arch/arm64/boot/dts/qcom/msm8996.dtsi | 9 +-
>> drivers/clk/qcom/clk-rcg.h | 1 +
>> drivers/clk/qcom/clk-rcg2.c | 76 ++-
>> drivers/clk/qcom/common.c | 16 +
>> drivers/clk/qcom/common.h | 2 +
>> drivers/clk/qcom/gcc-apq8084.c | 8 +-
>> drivers/clk/qcom/gcc-msm8916.c | 4 +-
>> drivers/clk/qcom/gcc-msm8974.c | 8 +-
>> drivers/clk/qcom/gcc-msm8994.c | 8 +-
>> drivers/clk/qcom/gcc-msm8996.c | 8 +-
>> drivers/mmc/host/sdhci-msm.c | 626 +++++++++++++++++++--
>> drivers/mmc/host/sdhci.c | 28 +-
>> drivers/mmc/host/sdhci.h | 1 +
>> 17 files changed, 739 insertions(+), 100 deletions(-)
>>
>> --
>> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>> a Linux Foundation Collaborative Project.
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Adrian Hunter @ 2016-11-21 12:44 UTC (permalink / raw)
To: Linus Walleij
Cc: Arnd Bergmann, linux-mmc, Ulf Hansson, Chunyan Zhang, Baolin Wang,
Jaehoon Chung, Namjae Jeon, Maya Erez, Luca Porzio, Alex Lemberg
In-Reply-To: <2697103.lML87IM8sj@wuerfel>
On 21/11/16 13:11, Arnd Bergmann wrote:
> On Monday, November 21, 2016 11:08:57 AM CET Linus Walleij wrote:
>> I've had it with this code now.
>>
>> The packed command support is a complex hurdle in the MMC/SD block
>> layer, around 500+ lines of code which was introduced in 2013 in
>> commits
>>
>> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
>> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>>
>> ...and since then it has been rotting. The original author of the
>> code has disappeared from the community and the mail address is
>> bouncing.
>>
>> For the code to be exercised the host must flag that it supports
>> packed commands, so in mmc_blk_prep_packed_list() which is called for
>> every single request, the following construction appears:
>>
>> u8 max_packed_rw = 0;
>>
>> if ((rq_data_dir(cur) == WRITE) &&
>> mmc_host_packed_wr(card->host))
>> max_packed_rw = card->ext_csd.max_packed_writes;
>>
>> if (max_packed_rw == 0)
>> goto no_packed;
>>
>> This has the following logical deductions:
>>
>> - Only WRITE commands can really be packed, so the solution is
>> only half-done: we support packed WRITE but not packed READ.
>> The packed command support has not been finalized by supporting
>> reads in three years!
>
> Packed reads don't make a lot of sense, there is very little
> for an MMC to optimize in reads that it can't already do without
> the packing. For writes, packing makes could be an important
> performance optimization, if the eMMC supports it.
>
> I've added Luca Porzio and Alex Lemberg to Cc. I think they
> are subscribed to the list already, but it would be good to
> get some estimate from them too about how common the packed
> write support is on existing hardware from their respective
> employers before we kill it off.
>
> If none of Samsung/Micron/Sandisk are currently shipping
> devices that support eMMC-4.5 packed commands but don't
> support the eMMC-5.1 command queuing (which IIRC is a more
> general way to achieve the same), we probably don't need to
> worry about it too much.
>
>> - mmc_host_packed_wr() is just a static inline that checks
>> host->caps2 & MMC_CAP2_PACKED_WR. The problem with this is
>> that NO upstream host sets this capability flag! No driver
>> in the kernel is using it, and we can't test it. Packed
>> command may be supported in out-of-tree code, but I doubt
>> it. I doubt that the code is even working anymore due to
>> other refactorings in the MMC block layer, who would
>> notice if patches affecting it broke packed commands?
>> No one.
>
> This is a very good indication that it's not really used.
> It would be useful to also check out the Android AOSP
> kernel tree to see if it's in there, if anything important
> supports packed commands, it's probably in there.
>
>> - There is no Device Tree binding or code to mark a host as
>> supporting packed read or write commands, just this flag
>> in caps2, so for sure there are not any DT systems using
>> it either.
>>
>> It has other problems as well: mmc_blk_prep_packed_list() is
>> speculatively picking requests out of the request queue with
>> blk_fetch_request() making the MMC/SD stack harder to convert
>> to the multiqueue block layer. By this we get rid of an
>> obstacle.
SDHCIv4 has a feature (ADMA3) which is slightly similar to packed
commands but it does not require card support.
Why is it a problem for blk-mq to allow some extra requests to
pick from for packing?
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Ulf Hansson @ 2016-11-21 14:02 UTC (permalink / raw)
To: Adrian Hunter
Cc: Linus Walleij, Arnd Bergmann, linux-mmc, Chunyan Zhang,
Baolin Wang, Jaehoon Chung, Namjae Jeon, Maya Erez, Luca Porzio,
Alex Lemberg
In-Reply-To: <d0722644-2922-b580-9bfe-01dd3c38ec67@intel.com>
On 21 November 2016 at 13:44, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 21/11/16 13:11, Arnd Bergmann wrote:
>> On Monday, November 21, 2016 11:08:57 AM CET Linus Walleij wrote:
>>> I've had it with this code now.
>>>
>>> The packed command support is a complex hurdle in the MMC/SD block
>>> layer, around 500+ lines of code which was introduced in 2013 in
>>> commits
>>>
>>> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
>>> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>>>
>>> ...and since then it has been rotting. The original author of the
>>> code has disappeared from the community and the mail address is
>>> bouncing.
>>>
>>> For the code to be exercised the host must flag that it supports
>>> packed commands, so in mmc_blk_prep_packed_list() which is called for
>>> every single request, the following construction appears:
>>>
>>> u8 max_packed_rw = 0;
>>>
>>> if ((rq_data_dir(cur) == WRITE) &&
>>> mmc_host_packed_wr(card->host))
>>> max_packed_rw = card->ext_csd.max_packed_writes;
>>>
>>> if (max_packed_rw == 0)
>>> goto no_packed;
>>>
>>> This has the following logical deductions:
>>>
>>> - Only WRITE commands can really be packed, so the solution is
>>> only half-done: we support packed WRITE but not packed READ.
>>> The packed command support has not been finalized by supporting
>>> reads in three years!
>>
>> Packed reads don't make a lot of sense, there is very little
>> for an MMC to optimize in reads that it can't already do without
>> the packing. For writes, packing makes could be an important
>> performance optimization, if the eMMC supports it.
>>
>> I've added Luca Porzio and Alex Lemberg to Cc. I think they
>> are subscribed to the list already, but it would be good to
>> get some estimate from them too about how common the packed
>> write support is on existing hardware from their respective
>> employers before we kill it off.
>>
>> If none of Samsung/Micron/Sandisk are currently shipping
>> devices that support eMMC-4.5 packed commands but don't
>> support the eMMC-5.1 command queuing (which IIRC is a more
>> general way to achieve the same), we probably don't need to
>> worry about it too much.
>>
>>> - mmc_host_packed_wr() is just a static inline that checks
>>> host->caps2 & MMC_CAP2_PACKED_WR. The problem with this is
>>> that NO upstream host sets this capability flag! No driver
>>> in the kernel is using it, and we can't test it. Packed
>>> command may be supported in out-of-tree code, but I doubt
>>> it. I doubt that the code is even working anymore due to
>>> other refactorings in the MMC block layer, who would
>>> notice if patches affecting it broke packed commands?
>>> No one.
>>
>> This is a very good indication that it's not really used.
>> It would be useful to also check out the Android AOSP
>> kernel tree to see if it's in there, if anything important
>> supports packed commands, it's probably in there.
>>
>>> - There is no Device Tree binding or code to mark a host as
>>> supporting packed read or write commands, just this flag
>>> in caps2, so for sure there are not any DT systems using
>>> it either.
>>>
>>> It has other problems as well: mmc_blk_prep_packed_list() is
>>> speculatively picking requests out of the request queue with
>>> blk_fetch_request() making the MMC/SD stack harder to convert
>>> to the multiqueue block layer. By this we get rid of an
>>> obstacle.
>
> SDHCIv4 has a feature (ADMA3) which is slightly similar to packed
> commands but it does not require card support.
>
> Why is it a problem for blk-mq to allow some extra requests to
> pick from for packing?
>
In blk-mq, requests don't get picked from the queue, but instead those
gets "pushed" to the block device driver.
First, to support packing, it seems like we would need to specify a
queue-depth > 1, more or less lie to blk-mq layer about the device's
capability. Okay, we can do that. But..
I also believe, the implementation would become really complex, as you
would need to "hold" the first write request, while waiting for a
second to arrive. Then for how long shall you hold it? And then what
if you are unlucky so the next is read request, thus you can't pack
them. The solution will be suboptimal, won't it?
Perhaps Linus can add something, or confirm?
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Adrian Hunter @ 2016-11-21 14:17 UTC (permalink / raw)
To: Ulf Hansson
Cc: Linus Walleij, Arnd Bergmann, linux-mmc, Chunyan Zhang,
Baolin Wang, Jaehoon Chung, Namjae Jeon, Maya Erez, Luca Porzio,
Alex Lemberg
In-Reply-To: <CAPDyKFrW+Jge-Kd=PhRJvHG1Y==MMYinXxgHM1ySSG8gOx3tyA@mail.gmail.com>
On 21/11/16 16:02, Ulf Hansson wrote:
> On 21 November 2016 at 13:44, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 21/11/16 13:11, Arnd Bergmann wrote:
>>> On Monday, November 21, 2016 11:08:57 AM CET Linus Walleij wrote:
>>>> I've had it with this code now.
>>>>
>>>> The packed command support is a complex hurdle in the MMC/SD block
>>>> layer, around 500+ lines of code which was introduced in 2013 in
>>>> commits
>>>>
>>>> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
>>>> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>>>>
>>>> ...and since then it has been rotting. The original author of the
>>>> code has disappeared from the community and the mail address is
>>>> bouncing.
>>>>
>>>> For the code to be exercised the host must flag that it supports
>>>> packed commands, so in mmc_blk_prep_packed_list() which is called for
>>>> every single request, the following construction appears:
>>>>
>>>> u8 max_packed_rw = 0;
>>>>
>>>> if ((rq_data_dir(cur) == WRITE) &&
>>>> mmc_host_packed_wr(card->host))
>>>> max_packed_rw = card->ext_csd.max_packed_writes;
>>>>
>>>> if (max_packed_rw == 0)
>>>> goto no_packed;
>>>>
>>>> This has the following logical deductions:
>>>>
>>>> - Only WRITE commands can really be packed, so the solution is
>>>> only half-done: we support packed WRITE but not packed READ.
>>>> The packed command support has not been finalized by supporting
>>>> reads in three years!
>>>
>>> Packed reads don't make a lot of sense, there is very little
>>> for an MMC to optimize in reads that it can't already do without
>>> the packing. For writes, packing makes could be an important
>>> performance optimization, if the eMMC supports it.
>>>
>>> I've added Luca Porzio and Alex Lemberg to Cc. I think they
>>> are subscribed to the list already, but it would be good to
>>> get some estimate from them too about how common the packed
>>> write support is on existing hardware from their respective
>>> employers before we kill it off.
>>>
>>> If none of Samsung/Micron/Sandisk are currently shipping
>>> devices that support eMMC-4.5 packed commands but don't
>>> support the eMMC-5.1 command queuing (which IIRC is a more
>>> general way to achieve the same), we probably don't need to
>>> worry about it too much.
>>>
>>>> - mmc_host_packed_wr() is just a static inline that checks
>>>> host->caps2 & MMC_CAP2_PACKED_WR. The problem with this is
>>>> that NO upstream host sets this capability flag! No driver
>>>> in the kernel is using it, and we can't test it. Packed
>>>> command may be supported in out-of-tree code, but I doubt
>>>> it. I doubt that the code is even working anymore due to
>>>> other refactorings in the MMC block layer, who would
>>>> notice if patches affecting it broke packed commands?
>>>> No one.
>>>
>>> This is a very good indication that it's not really used.
>>> It would be useful to also check out the Android AOSP
>>> kernel tree to see if it's in there, if anything important
>>> supports packed commands, it's probably in there.
>>>
>>>> - There is no Device Tree binding or code to mark a host as
>>>> supporting packed read or write commands, just this flag
>>>> in caps2, so for sure there are not any DT systems using
>>>> it either.
>>>>
>>>> It has other problems as well: mmc_blk_prep_packed_list() is
>>>> speculatively picking requests out of the request queue with
>>>> blk_fetch_request() making the MMC/SD stack harder to convert
>>>> to the multiqueue block layer. By this we get rid of an
>>>> obstacle.
>>
>> SDHCIv4 has a feature (ADMA3) which is slightly similar to packed
>> commands but it does not require card support.
>>
>> Why is it a problem for blk-mq to allow some extra requests to
>> pick from for packing?
>>
>
> In blk-mq, requests don't get picked from the queue, but instead those
> gets "pushed" to the block device driver.
>
> First, to support packing, it seems like we would need to specify a
> queue-depth > 1, more or less lie to blk-mq layer about the device's
> capability. Okay, we can do that. But..
>
> I also believe, the implementation would become really complex, as you
> would need to "hold" the first write request, while waiting for a
> second to arrive. Then for how long shall you hold it? And then what
> if you are unlucky so the next is read request, thus you can't pack
> them. The solution will be suboptimal, won't it?
It doesn't hold and wait now. So why would it in the blk-mq case?
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Alex Lemberg @ 2016-11-21 14:23 UTC (permalink / raw)
To: Arnd Bergmann, Linus Walleij
Cc: linux-mmc@vger.kernel.org, Ulf Hansson, Chunyan Zhang,
Baolin Wang, Jaehoon Chung, Namjae Jeon, Maya Erez, Luca Porzio
In-Reply-To: <2697103.lML87IM8sj@wuerfel>
Hi,
On 11/21/16, 1:11 PM, "Arnd Bergmann" <arnd@arndb.de> wrote:
>On Monday, November 21, 2016 11:08:57 AM CET Linus Walleij wrote:
>> I've had it with this code now.
>>
>> The packed command support is a complex hurdle in the MMC/SD block
>> layer, around 500+ lines of code which was introduced in 2013 in
>> commits
>>
>> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
>> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>>
>> ...and since then it has been rotting. The original author of the
>> code has disappeared from the community and the mail address is
>> bouncing.
>>
>> For the code to be exercised the host must flag that it supports
>> packed commands, so in mmc_blk_prep_packed_list() which is called for
>> every single request, the following construction appears:
>>
>> u8 max_packed_rw = 0;
>>
>> if ((rq_data_dir(cur) == WRITE) &&
>> mmc_host_packed_wr(card->host))
>> max_packed_rw = card->ext_csd.max_packed_writes;
>>
>> if (max_packed_rw == 0)
>> goto no_packed;
>>
>> This has the following logical deductions:
>>
>> - Only WRITE commands can really be packed, so the solution is
>> only half-done: we support packed WRITE but not packed READ.
>> The packed command support has not been finalized by supporting
>> reads in three years!
>
>Packed reads don't make a lot of sense, there is very little
>for an MMC to optimize in reads that it can't already do without
>the packing. For writes, packing makes could be an important
>performance optimization, if the eMMC supports it.
>
>I've added Luca Porzio and Alex Lemberg to Cc. I think they
>are subscribed to the list already, but it would be good to
>get some estimate from them too about how common the packed
>write support is on existing hardware from their respective
>employers before we kill it off.
Correct, in general there is no value in using packed for Read.
But I can’t say this for all existing flash management solution.
The eMMC spec allows to use it for Read as well.
>
>If none of Samsung/Micron/Sandisk are currently shipping
>devices that support eMMC-4.5 packed commands but don't
>support the eMMC-5.1 command queuing (which IIRC is a more
>general way to achieve the same), we probably don't need to
>worry about it too much.
Please note that even by having eMMC-5.1 device,
not all chipset vendors are having HW/SW CMDQ support.
So they might be using packed commands instead.
>
>> - mmc_host_packed_wr() is just a static inline that checks
>> host->caps2 & MMC_CAP2_PACKED_WR. The problem with this is
>> that NO upstream host sets this capability flag! No driver
>> in the kernel is using it, and we can't test it. Packed
>> command may be supported in out-of-tree code, but I doubt
>> it. I doubt that the code is even working anymore due to
>> other refactorings in the MMC block layer, who would
>> notice if patches affecting it broke packed commands?
>> No one.
>
>This is a very good indication that it's not really used.
>It would be useful to also check out the Android AOSP
>kernel tree to see if it's in there, if anything important
>supports packed commands, it's probably in there.
As far as I can say from reviewing the mobile (Android)
platforms kernel source (from different vendors),
many of them are enabling Packed Commands.
>
> Arnd
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Ulf Hansson @ 2016-11-21 15:17 UTC (permalink / raw)
To: Adrian Hunter
Cc: Linus Walleij, Arnd Bergmann, linux-mmc, Chunyan Zhang,
Baolin Wang, Jaehoon Chung, Namjae Jeon, Maya Erez, Luca Porzio,
Alex Lemberg
In-Reply-To: <135cb4af-fbbe-d6a7-182b-73543f1c551a@intel.com>
[...]
>>>
>>> SDHCIv4 has a feature (ADMA3) which is slightly similar to packed
>>> commands but it does not require card support.
>>>
>>> Why is it a problem for blk-mq to allow some extra requests to
>>> pick from for packing?
>>>
>>
>> In blk-mq, requests don't get picked from the queue, but instead those
>> gets "pushed" to the block device driver.
>>
>> First, to support packing, it seems like we would need to specify a
>> queue-depth > 1, more or less lie to blk-mq layer about the device's
>> capability. Okay, we can do that. But..
>>
>> I also believe, the implementation would become really complex, as you
>> would need to "hold" the first write request, while waiting for a
>> second to arrive. Then for how long shall you hold it? And then what
>> if you are unlucky so the next is read request, thus you can't pack
>> them. The solution will be suboptimal, won't it?
>
> It doesn't hold and wait now. So why would it in the blk-mq case?
Because earlier the mmc block device driver *fetches* requests from
the queue and when finding more than one request, we could try to pack
them. If there are only one request, we just move on with it.
With blk-mq, we don't *fetch* request from the queue, but instead
those would get pushed to the mmc block device driver via the
implemented block-mq ops.
Please note, I am not an blk-mq expert, but this is my understanding.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Linus Walleij @ 2016-11-21 15:27 UTC (permalink / raw)
To: Adrian Hunter, Jens Axboe, Christoph Hellwig, linux-block
Cc: Ulf Hansson, Arnd Bergmann, linux-mmc, Chunyan Zhang, Baolin Wang,
Jaehoon Chung, Namjae Jeon, Maya Erez, Luca Porzio, Alex Lemberg,
Paolo Valente
In-Reply-To: <135cb4af-fbbe-d6a7-182b-73543f1c551a@intel.com>
[CC the block layer maintainers so they can smack my fingers
if I misunderstood any of how the block layer semantics work...]
On Mon, Nov 21, 2016 at 3:17 PM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 21/11/16 16:02, Ulf Hansson wrote:
>> I also believe, the implementation would become really complex, as you
>> would need to "hold" the first write request, while waiting for a
>> second to arrive. Then for how long shall you hold it? And then what
>> if you are unlucky so the next is read request, thus you can't pack
>> them. The solution will be suboptimal, won't it?
>
> It doesn't hold and wait now. So why would it in the blk-mq case?
The current kthread in drivers/mmc/card/queue.c looks like this
in essence:
struct request *r;
while (1)
r = blk_fetch_request(q);
issue();
}
It is pulling out as much as it can to asynchronously issue
two requests in parallel and also the packed command (as
mentioned in the commitlog to $SUBJECT) pulls out even more
stuff of the queue to speculatively issue things in a packed
command.
The block layer isn't supposed to be used like this. It is
supposed to be used like so:
1. You get notified by the request_fn that is passed with
blk_init_queue()
2. The request function fires a work.
3. The work pick ONE request with blk_fetch_request()
and handles it.
4. Repeat from (1)
Instead of doing this the MMC layer kthread is speculatively
pulling out stuff of the queue whenever it can, including
pulling out a few NULL at the end before it stops. The
mechanism is similar to a person running along a queue and
picking a segment of passengers into a bus to send off,
batching them. Which is clever, but the block layer is not
supposed to be used like that. It just happens to work.
In blk-mq this speculative fetching is no longer possible.
Instead you register a notification function in struct blk_mq_ops
vtable .queue_rq() callback: this will be called by the block
layer core whenever there is a request available on "our"
queue. It further approves a .init_request() callback that is
called overhead to allocate a per-request context, such as
the current struct mmc_queue_req - just not quite because
the current mmc_queue_req is not mapped 1-to-1 onto a
request from the block layer because of packed command;
but it is after this patch, hehe ;)
Any speculative batching needs to happen *after* this, i.e.
the MMC layer would have to report a certain larger queue
depth (if you set it to 1 you only ever get one request at the time
and have to finish it before you get a new one), group the
requests itself with packed command or command queueing,
then signal them back as they are confirmed completed by
the device, or, if they cannot be grouped, handle as far as you
can and put the remaining requests back on the queue
(creating a "bubble" in the pipeline).
Relying on iterating and inspecting the block layer queue is
*not* possible with blk-mq, sure blk_fetch_request() is still
there, but if you call it on an mq-registered queue, it will
crash the kernel. (At least it did for me.) Clearly it is not
intended to be used with MQ: none of the MQ-converted
subsystems use this. (drivers/mtd/ubi/block.c is a good
simple example)
I liken these mechanisms to a pipeline:
- The two-levels deep speculation buffer in struct mmc_queue
field .mqrq[2] is a "software pipeline" in the MMC layer (so we
can prepare and handle requests in parallel)
- The packed command and command queue is a
hardware-supported pipeline on the device side.
Both try to overcome the hardware limitations of the MMC/SD
logical interface. This batching-style pipelining isn't really
solving the problem the way real multiqueue hardware does,
so it is a poor man's patchwork to the problem.
In either case, as Ulf notes, you need to get a few requests
off the queue and group them using packed command or
command queueing if possible, but that grouping needs to
happen in the MMC/SD layer, after picking the requests from
the queue. I think it is OK to do so and just put any requests
you cannot pack into the pipeline back on the queue. But I
am not sure (still learning....)
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH RFCv2 01/10] mmc: core: Add support to read command queue parameters
From: Linus Walleij @ 2016-11-21 15:34 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Ulf Hansson, linux-mmc@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Adrian Hunter, Alex Lemberg,
mateusz.nowak, Yuliy.Izrailov, Jaehoon Chung, Dong Aisheng,
Asutosh Das, Zhangfei Gao, Sujit Reddy Thumma, kdorfman,
David Griego, Sahitya Tummala, venkatg, Shawn Lin,
Subhash Jadavani
In-Reply-To: <1467033757-32498-2-git-send-email-riteshh@codeaurora.org>
On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> From: Asutosh Das <asutoshd@codeaurora.org>
>
> eMMC cards with EXT_CSD version >= 8, optionally support command
> queuing feature as defined by JEDEC eMMC5.1. Add support for probing
> command queue feature for such type of cards.
>
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> [subhashj@codeaurora.org: fixed trivial merge conflicts]
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Even if we don't merge the specific mechanism provided by the
rest of the patches, this patch just make us know more about
the capabilities of the hardware we're running on, which is good.
WIll it be reported properly by the lsmmc command too?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH RFCv2 00/10] mmc: Add HW Command Queuing Support
From: Linus Walleij @ 2016-11-21 15:52 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Ulf Hansson, linux-mmc@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Adrian Hunter, Alex Lemberg,
mateusz.nowak, Yuliy.Izrailov, Jaehoon Chung, Dong Aisheng,
Asutosh Das, Zhangfei Gao, Sujit Reddy Thumma, kdorfman,
David Griego, Sahitya Tummala, venkatg, Shawn Lin
In-Reply-To: <1467033757-32498-1-git-send-email-riteshh@codeaurora.org>
On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> This patch series refreshes the older patch series sent a
> while ago by Asutosh Das[1].
>
> Current set of patch series is only introducing the basic CMDQ
> driver to get more review comments. Based on the reviews, will
> add other features as well to the patch list.
kthread is deprecated since the talks of the last kernel
summit. You should prefer to use workqueues.
The queueing mechanism looks like a duplicate copy/paste
solution built on the "old" processing queue.
Overall this is the problem with the whole patch set: instead
of a "deep" integration by improving the existing request
processing, a separate code path is bolted on the side.
What you should do is improve the existing infrastructure to
accomodate for the new mechanism.
For example: a new processing thread is introduced instead
of augmenting the existing one to handle this case, resulting
in a big slew of duplicated code.
For example: this is not at all integrated with the other feature,
packed command, which is *also* speculatively pulling out ever more
stuff from the block layer, albeit in a totally different manner,
actually integrated with the existing queue mechanism.
Needless to say: these two features (packed command and
command queue) should use the *same codepath* just that the
command queuing is more powerful, so that packed command is
merely a subset of the command queueing.
Now: note that I sent a patch do delete packed command. I do
not know if that will be accepted, but something deeply
integrated is for sure needed, we need a elegant and compact
core of code to maintain, not more copies of the already existing
messy code.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH RFCv2 00/10] mmc: Add HW Command Queuing Support
From: Arnd Bergmann @ 2016-11-21 16:05 UTC (permalink / raw)
To: Linus Walleij
Cc: Ritesh Harjani, Ulf Hansson, linux-mmc@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Adrian Hunter, Alex Lemberg,
mateusz.nowak, Yuliy.Izrailov, Jaehoon Chung, Dong Aisheng,
Asutosh Das, Zhangfei Gao, Sujit Reddy Thumma, kdorfman,
David Griego, Sahitya Tummala, venkatg, Shawn Lin
In-Reply-To: <CACRpkdbTkAzTGiKFLFgxFTQA0wa8egCAwZf8WzJNTa-nOifqOA@mail.gmail.com>
On Monday, November 21, 2016 4:52:37 PM CET Linus Walleij wrote:
> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>
> > This patch series refreshes the older patch series sent a
> > while ago by Asutosh Das[1].
> >
> > Current set of patch series is only introducing the basic CMDQ
> > driver to get more review comments. Based on the reviews, will
> > add other features as well to the patch list.
>
> kthread is deprecated since the talks of the last kernel
> summit. You should prefer to use workqueues.
It's the kthread freezer that is deprecated, not kthreads. It with
blk_mq, we probably won't want to have this kthread, but that's
a differennt issue, and using workqueues instead won't help.
Arnd
^ permalink raw reply
* [PATCH 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI
From: Zach Brown @ 2016-11-21 23:04 UTC (permalink / raw)
To: ulf.hansson; +Cc: adrian.hunter, linux-mmc, linux-kernel, zach.brown
In-Reply-To: <1479769446-8490-1-git-send-email-zach.brown@ni.com>
On NI 9037 boards the max SDIO frequency is limited by trace lengths
and other layout choices. The max SDIO frequency is stored in an ACPI
table.
The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
f_max field of the host.
Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
Reviewed-by: Josh Cartwright <joshc@ni.com>
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
drivers/mmc/host/sdhci-pci-core.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 9741505..a855c97 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -27,6 +27,7 @@
#include <linux/pm_runtime.h>
#include <linux/mmc/slot-gpio.h>
#include <linux/mmc/sdhci-pci-data.h>
+#include <linux/acpi.h>
#include "sdhci.h"
#include "sdhci-pci.h"
@@ -377,6 +378,20 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
{
+#ifdef CONFIG_ACPI
+ acpi_status status;
+ unsigned long long max_freq;
+
+ status = acpi_evaluate_integer(ACPI_HANDLE(&slot->chip->pdev->dev),
+ "MXFQ", NULL, &max_freq);
+ if (ACPI_FAILURE(status)) {
+ dev_info(&slot->chip->pdev->dev,
+ "MXFQ not found in acpi table\n");
+ return -EINVAL;
+ }
+
+ slot->host->mmc->f_max = max_freq * 1000000;
+#endif
slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
MMC_CAP_WAIT_WHILE_BUSY;
return 0;
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] mmc: sdhci-pci: Use ACPI to set max frequency of sdio host
From: Zach Brown @ 2016-11-21 23:04 UTC (permalink / raw)
To: ulf.hansson; +Cc: adrian.hunter, linux-mmc, linux-kernel, zach.brown
On some boards, max SDIO frequency is limited by trace lengths and other layout
choices. We would like a way to specify this limitation so the driver can
behave accordingly.
This patch set assumes that the limitation has been reported in an ACPI table
which the driver can check to get the max frequency.
The first patch creates a PCI ID and support for the Intel byt sdio where NI is
the subvendor.
The second patch uses the ACPI table to set f_max during the new
ni_byt_sdio_probe_slot.
Zach Brown (2):
mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller
sub-vended by NI
mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio
controller sub-vended by NI
drivers/mmc/host/sdhci-pci-core.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
--
2.7.4
^ permalink raw reply
* [PATCH 1/2] mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller sub-vended by NI
From: Zach Brown @ 2016-11-21 23:04 UTC (permalink / raw)
To: ulf.hansson; +Cc: adrian.hunter, linux-mmc, linux-kernel, zach.brown
In-Reply-To: <1479769446-8490-1-git-send-email-zach.brown@ni.com>
Add PCI ID for Intel byt sdio host controller sub-vended by NI.
The controller has different behavior because of the board layout NI
puts it on.
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
drivers/mmc/host/sdhci-pci-core.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 1d9e00a..9741505 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -375,6 +375,13 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
return 0;
}
+static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
+{
+ slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
+ MMC_CAP_WAIT_WHILE_BUSY;
+ return 0;
+}
+
static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
{
slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
@@ -447,6 +454,15 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_emmc = {
.ops = &sdhci_intel_byt_ops,
};
+static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
+ .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
+ .quirks2 = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
+ SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+ .allow_runtime_pm = true,
+ .probe_slot = ni_byt_sdio_probe_slot,
+ .ops = &sdhci_intel_byt_ops,
+};
+
static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
.quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
.quirks2 = SDHCI_QUIRK2_HOST_OFF_CARD_ON |
@@ -1079,6 +1095,14 @@ static const struct pci_device_id pci_ids[] = {
{
.vendor = PCI_VENDOR_ID_INTEL,
.device = PCI_DEVICE_ID_INTEL_BYT_SDIO,
+ .subvendor = PCI_VENDOR_ID_NI,
+ .subdevice = 0x7884,
+ .driver_data = (kernel_ulong_t)&sdhci_ni_byt_sdio,
+ },
+
+ {
+ .vendor = PCI_VENDOR_ID_INTEL,
+ .device = PCI_DEVICE_ID_INTEL_BYT_SDIO,
.subvendor = PCI_ANY_ID,
.subdevice = PCI_ANY_ID,
.driver_data = (kernel_ulong_t)&sdhci_intel_byt_sdio,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v9 00/16] mmc: sdhci-msm: Add clk-rates, DDR, HS400 support
From: Stephen Boyd @ 2016-11-21 23:11 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Ulf Hansson, Andy Gross, linux-mmc, Adrian Hunter, Shawn Lin,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-clk,
David Brown,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Georgi Djakov, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Asutosh Das, David Griego, Sahitya Tummala, Venkat Gopalakrishnan,
Rajendra Nayak
In-Reply-To: <d4d05fb9-8a9e-6cf2-dc63-0edbd27a9e55-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 11/21, Ritesh Harjani wrote:
>
>
> On 11/21/2016 3:36 PM, Ulf Hansson wrote:
> >On 21 November 2016 at 07:37, Ritesh Harjani <riteshh-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> >>Hi,
> >>
> >>This is v9 version of the patch series which adds support for MSM8996.
> >>Adds HS400 driver support as well.
> >>These are tested on internal msm8996 & db410c HW.
> >>
> >>The patch series is ready. Do we think we can apply these
> >>patches for next now?
> >
> >I guess the DTS changes can be picked up by Andy, so they can go via arm-soc?
> Yes.
>
> >
> >Then, does the mmc changes depend on the clock changes? If so, I can
> >pick them as well, but then I need an ack from Stephen....
> Ideal and preferable, would be that clk & mmc changes go in
> together. But either ways should be fine.
>
There's only a runtime dependency where the clk rates will be
wrong if clk tree isn't merged. I'd rather just apply the clk
ones directly to clk tree and let all three trees come together
in linux-next and work.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Jaehoon Chung @ 2016-11-22 3:53 UTC (permalink / raw)
To: Alex Lemberg, Arnd Bergmann, Linus Walleij
Cc: linux-mmc@vger.kernel.org, Ulf Hansson, Chunyan Zhang,
Baolin Wang, Namjae Jeon, Maya Erez, Luca Porzio
In-Reply-To: <0420FC2A-518F-49D5-90A0-556A0BA5395A@sandisk.com>
On 11/21/2016 11:23 PM, Alex Lemberg wrote:
> Hi,
>
>
> On 11/21/16, 1:11 PM, "Arnd Bergmann" <arnd@arndb.de> wrote:
>
>> On Monday, November 21, 2016 11:08:57 AM CET Linus Walleij wrote:
>>> I've had it with this code now.
>>>
>>> The packed command support is a complex hurdle in the MMC/SD block
>>> layer, around 500+ lines of code which was introduced in 2013 in
>>> commits
>>>
>>> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
>>> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>>>
>>> ...and since then it has been rotting. The original author of the
>>> code has disappeared from the community and the mail address is
>>> bouncing.
>>>
>>> For the code to be exercised the host must flag that it supports
>>> packed commands, so in mmc_blk_prep_packed_list() which is called for
>>> every single request, the following construction appears:
>>>
>>> u8 max_packed_rw = 0;
>>>
>>> if ((rq_data_dir(cur) == WRITE) &&
>>> mmc_host_packed_wr(card->host))
>>> max_packed_rw = card->ext_csd.max_packed_writes;
>>>
>>> if (max_packed_rw == 0)
>>> goto no_packed;
>>>
>>> This has the following logical deductions:
>>>
>>> - Only WRITE commands can really be packed, so the solution is
>>> only half-done: we support packed WRITE but not packed READ.
>>> The packed command support has not been finalized by supporting
>>> reads in three years!
>>
>> Packed reads don't make a lot of sense, there is very little
>> for an MMC to optimize in reads that it can't already do without
>> the packing. For writes, packing makes could be an important
>> performance optimization, if the eMMC supports it.
>>
>> I've added Luca Porzio and Alex Lemberg to Cc. I think they
>> are subscribed to the list already, but it would be good to
>> get some estimate from them too about how common the packed
>> write support is on existing hardware from their respective
>> employers before we kill it off.
>
> Correct, in general there is no value in using packed for Read.
> But I can’t say this for all existing flash management solution.
> The eMMC spec allows to use it for Read as well.
As i know, when packed command had implemented, early eMMC had the firmware problem
for Packed Read operation. but so I can't say Packed Read doesn't have the benefit for performance.
But Packed Write command can see the benefit for performance.
>
>>
>> If none of Samsung/Micron/Sandisk are currently shipping
>> devices that support eMMC-4.5 packed commands but don't
>> support the eMMC-5.1 command queuing (which IIRC is a more
>> general way to achieve the same), we probably don't need to
>> worry about it too much.
>
> Please note that even by having eMMC-5.1 device,
> not all chipset vendors are having HW/SW CMDQ support.
> So they might be using packed commands instead.
>
>>
>>> - mmc_host_packed_wr() is just a static inline that checks
>>> host->caps2 & MMC_CAP2_PACKED_WR. The problem with this is
>>> that NO upstream host sets this capability flag! No driver
>>> in the kernel is using it, and we can't test it. Packed
>>> command may be supported in out-of-tree code, but I doubt
>>> it. I doubt that the code is even working anymore due to
>>> other refactorings in the MMC block layer, who would
>>> notice if patches affecting it broke packed commands?
>>> No one.
>>
>> This is a very good indication that it's not really used.
>> It would be useful to also check out the Android AOSP
>> kernel tree to see if it's in there, if anything important
>> supports packed commands, it's probably in there.
>
> As far as I can say from reviewing the mobile (Android)
> platforms kernel source (from different vendors),
> many of them are enabling Packed Commands.
Actually, some shipping Samsung devices with eMMC4.5 might be used packed command.
(For Android/Tizen OS and ARTIK boards)
Best Regards,
Jaehoon Chung
>
>>
>> Arnd
>
^ permalink raw reply
* Re: [PATCH 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI
From: Adrian Hunter @ 2016-11-22 6:46 UTC (permalink / raw)
To: Zach Brown, ulf.hansson; +Cc: linux-mmc, linux-kernel
In-Reply-To: <1479769446-8490-3-git-send-email-zach.brown@ni.com>
On 22/11/16 01:04, Zach Brown wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table.
>
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host.
>
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
> Reviewed-by: Josh Cartwright <joshc@ni.com>
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
> drivers/mmc/host/sdhci-pci-core.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 9741505..a855c97 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/mmc/slot-gpio.h>
> #include <linux/mmc/sdhci-pci-data.h>
> +#include <linux/acpi.h>
>
> #include "sdhci.h"
> #include "sdhci-pci.h"
> @@ -377,6 +378,20 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
>
> static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
> {
> +#ifdef CONFIG_ACPI
> + acpi_status status;
> + unsigned long long max_freq;
> +
> + status = acpi_evaluate_integer(ACPI_HANDLE(&slot->chip->pdev->dev),
> + "MXFQ", NULL, &max_freq);
> + if (ACPI_FAILURE(status)) {
> + dev_info(&slot->chip->pdev->dev,
Doesn't it error out the whole probe. So it should be dev_err()
> + "MXFQ not found in acpi table\n");
> + return -EINVAL;
> + }
> +
> + slot->host->mmc->f_max = max_freq * 1000000;
> +#endif
> slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> MMC_CAP_WAIT_WHILE_BUSY;
> return 0;
>
^ permalink raw reply
* Re: [PATCH RFCv2 01/10] mmc: core: Add support to read command queue parameters
From: Adrian Hunter @ 2016-11-22 7:58 UTC (permalink / raw)
To: Linus Walleij, Ritesh Harjani
Cc: Ulf Hansson, linux-mmc@vger.kernel.org,
linux-arm-msm@vger.kernel.org, Alex Lemberg, mateusz.nowak,
Yuliy.Izrailov, Jaehoon Chung, Dong Aisheng, Asutosh Das,
Zhangfei Gao, Sujit Reddy Thumma, kdorfman, David Griego,
Sahitya Tummala, venkatg, Shawn Lin, Subhash Jadavani
In-Reply-To: <CACRpkdYDfdCoZUOryjVX4byu2BFx9KZx7YKoFG9wESDpaKtirg@mail.gmail.com>
On 21/11/16 17:34, Linus Walleij wrote:
> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>
>> From: Asutosh Das <asutoshd@codeaurora.org>
>>
>> eMMC cards with EXT_CSD version >= 8, optionally support command
>> queuing feature as defined by JEDEC eMMC5.1. Add support for probing
>> command queue feature for such type of cards.
>>
>> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> [subhashj@codeaurora.org: fixed trivial merge conflicts]
>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> Even if we don't merge the specific mechanism provided by the
> rest of the patches, this patch just make us know more about
> the capabilities of the hardware we're running on, which is good.
I think SW CMDQ is a better starting point:
https://marc.info/?l=linux-mmc&m=147729857722285
It cleans up the queue thread a bit:
https://marc.info/?l=linux-mmc&m=147729857222281&w=2
And introduces queue semantics:
https://marc.info/?l=linux-mmc&m=147729863322314&w=2
^ permalink raw reply
* Re: [PATCH 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio controller sub-vended by NI
From: Ulf Hansson @ 2016-11-22 8:24 UTC (permalink / raw)
To: Zach Brown
Cc: Adrian Hunter, linux-mmc@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1479769446-8490-3-git-send-email-zach.brown@ni.com>
On 22 November 2016 at 00:04, Zach Brown <zach.brown@ni.com> wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table.
>
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host.
>
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
> Reviewed-by: Josh Cartwright <joshc@ni.com>
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
> drivers/mmc/host/sdhci-pci-core.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index 9741505..a855c97 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/mmc/slot-gpio.h>
> #include <linux/mmc/sdhci-pci-data.h>
> +#include <linux/acpi.h>
>
> #include "sdhci.h"
> #include "sdhci-pci.h"
> @@ -377,6 +378,20 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
>
> static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
> {
> +#ifdef CONFIG_ACPI
I am not a fan of these kind of "ifdefs" immediately in the code. I
rather see them around functions.
Either separate the code within the ifdefs here into it's own function
(and call it from here) and make a stub function in case when
CONFIG_ACPI is unset. Or, perhaps easier, have one version of
ni_byt_sdio_probe_slot() when CONFIG_ACPI is set and another one when
it's unset.
> + acpi_status status;
> + unsigned long long max_freq;
> +
> + status = acpi_evaluate_integer(ACPI_HANDLE(&slot->chip->pdev->dev),
> + "MXFQ", NULL, &max_freq);
> + if (ACPI_FAILURE(status)) {
> + dev_info(&slot->chip->pdev->dev,
> + "MXFQ not found in acpi table\n");
> + return -EINVAL;
> + }
> +
> + slot->host->mmc->f_max = max_freq * 1000000;
> +#endif
> slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
> MMC_CAP_WAIT_WHILE_BUSY;
> return 0;
> --
> 2.7.4
>
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 0/2] mmc: sdhci-pci: Use ACPI to set max frequency of sdio host
From: Ulf Hansson @ 2016-11-22 8:27 UTC (permalink / raw)
To: Zach Brown
Cc: Adrian Hunter, linux-mmc@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1479769446-8490-1-git-send-email-zach.brown@ni.com>
On 22 November 2016 at 00:04, Zach Brown <zach.brown@ni.com> wrote:
> On some boards, max SDIO frequency is limited by trace lengths and other layout
> choices. We would like a way to specify this limitation so the driver can
> behave accordingly.
>
> This patch set assumes that the limitation has been reported in an ACPI table
> which the driver can check to get the max frequency.
>
> The first patch creates a PCI ID and support for the Intel byt sdio where NI is
> the subvendor.
>
> The second patch uses the ACPI table to set f_max during the new
> ni_byt_sdio_probe_slot.
>
>
> Zach Brown (2):
> mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller
> sub-vended by NI
> mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio
> controller sub-vended by NI
>
> drivers/mmc/host/sdhci-pci-core.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> --
> 2.7.4
>
Please try to not forget to bump the version number and to provide a
history of the what changes between revisions. It makes life easier
when reviewing and when I am about to apply patches.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] mmc: block: delete packed command support
From: Ulf Hansson @ 2016-11-22 8:49 UTC (permalink / raw)
To: Jaehoon Chung
Cc: Alex Lemberg, Arnd Bergmann, Linus Walleij,
linux-mmc@vger.kernel.org, Chunyan Zhang, Baolin Wang,
Namjae Jeon, Maya Erez, Luca Porzio
In-Reply-To: <22ef57c2-4e09-afaa-8349-32e4ae687c8d@samsung.com>
On 22 November 2016 at 04:53, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 11/21/2016 11:23 PM, Alex Lemberg wrote:
>> Hi,
>>
>>
>> On 11/21/16, 1:11 PM, "Arnd Bergmann" <arnd@arndb.de> wrote:
>>
>>> On Monday, November 21, 2016 11:08:57 AM CET Linus Walleij wrote:
>>>> I've had it with this code now.
>>>>
>>>> The packed command support is a complex hurdle in the MMC/SD block
>>>> layer, around 500+ lines of code which was introduced in 2013 in
>>>> commits
>>>>
>>>> ce39f9d17c14 ("mmc: support packed write command for eMMC4.5 devices")
>>>> abd9ac144947 ("mmc: add packed command feature of eMMC4.5")
>>>>
>>>> ...and since then it has been rotting. The original author of the
>>>> code has disappeared from the community and the mail address is
>>>> bouncing.
>>>>
>>>> For the code to be exercised the host must flag that it supports
>>>> packed commands, so in mmc_blk_prep_packed_list() which is called for
>>>> every single request, the following construction appears:
>>>>
>>>> u8 max_packed_rw = 0;
>>>>
>>>> if ((rq_data_dir(cur) == WRITE) &&
>>>> mmc_host_packed_wr(card->host))
>>>> max_packed_rw = card->ext_csd.max_packed_writes;
>>>>
>>>> if (max_packed_rw == 0)
>>>> goto no_packed;
>>>>
>>>> This has the following logical deductions:
>>>>
>>>> - Only WRITE commands can really be packed, so the solution is
>>>> only half-done: we support packed WRITE but not packed READ.
>>>> The packed command support has not been finalized by supporting
>>>> reads in three years!
>>>
>>> Packed reads don't make a lot of sense, there is very little
>>> for an MMC to optimize in reads that it can't already do without
>>> the packing. For writes, packing makes could be an important
>>> performance optimization, if the eMMC supports it.
>>>
>>> I've added Luca Porzio and Alex Lemberg to Cc. I think they
>>> are subscribed to the list already, but it would be good to
>>> get some estimate from them too about how common the packed
>>> write support is on existing hardware from their respective
>>> employers before we kill it off.
>>
>> Correct, in general there is no value in using packed for Read.
>> But I can’t say this for all existing flash management solution.
>> The eMMC spec allows to use it for Read as well.
>
> As i know, when packed command had implemented, early eMMC had the firmware problem
> for Packed Read operation. but so I can't say Packed Read doesn't have the benefit for performance.
> But Packed Write command can see the benefit for performance.
Regarding "performance", are you merely thinking about increased
throughput? With packed command we decrease the communication overhead
with the card so less commands becomes sent/received.
Or, did you also observed an improved behaviour of the card from a
garbage collect point of view? In other words also a decreased latency
when the device is becoming more and more used?
Finally, did you compare the packed command, towards using the
asynchronous request mechanisms (using the ->pre|post_req() mmc host
ops)?
>
>>
>>>
>>> If none of Samsung/Micron/Sandisk are currently shipping
>>> devices that support eMMC-4.5 packed commands but don't
>>> support the eMMC-5.1 command queuing (which IIRC is a more
>>> general way to achieve the same), we probably don't need to
>>> worry about it too much.
>>
>> Please note that even by having eMMC-5.1 device,
>> not all chipset vendors are having HW/SW CMDQ support.
>> So they might be using packed commands instead.
>>
>>>
>>>> - mmc_host_packed_wr() is just a static inline that checks
>>>> host->caps2 & MMC_CAP2_PACKED_WR. The problem with this is
>>>> that NO upstream host sets this capability flag! No driver
>>>> in the kernel is using it, and we can't test it. Packed
>>>> command may be supported in out-of-tree code, but I doubt
>>>> it. I doubt that the code is even working anymore due to
>>>> other refactorings in the MMC block layer, who would
>>>> notice if patches affecting it broke packed commands?
>>>> No one.
>>>
>>> This is a very good indication that it's not really used.
>>> It would be useful to also check out the Android AOSP
>>> kernel tree to see if it's in there, if anything important
>>> supports packed commands, it's probably in there.
>>
>> As far as I can say from reviewing the mobile (Android)
>> platforms kernel source (from different vendors),
>> many of them are enabling Packed Commands.
>
> Actually, some shipping Samsung devices with eMMC4.5 might be used packed command.
> (For Android/Tizen OS and ARTIK boards)
Thanks for sharing this information!
It seems like we need to run another round of performance
measurements, as to get some fresh number of the benefit of packed
command.
I would really appreciate if you could help out with that.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] RFC: mmc: block: replace semaphore with freezing
From: Linus Walleij @ 2016-11-22 8:54 UTC (permalink / raw)
To: Arnd Bergmann, Binoy Jayan
Cc: Rafael J. Wysocki, linux-mmc@vger.kernel.org, Ulf Hansson,
Chunyan Zhang, Baolin Wang, Linux PM, Rafael J . Wysocki,
Russell King, Jiri Kosina
In-Reply-To: <17679227.zg8zl3jhbr@wuerfel>
On Wed, Nov 16, 2016 at 5:32 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday, November 16, 2016 4:20:47 PM CET Linus Walleij wrote:
>> On Wed, Nov 16, 2016 at 1:46 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>>
>> > Well, we had a session at the KS regarding usage of the freezer on
>> > kernel threads and the conclusion was to get rid of that (as opposed
>> > to freezing user space, which is necessary IMO). So this change would
>> > go in the opposite direction.
>>
>> Aha so I should not make this thread look like everyone else, instead
>> everyone else should look like this thread, haha
>>
>> Ah well, I'll just drop it.
>
> It would still be good to remove the semaphore and do something else,
> as we also want to remove all semaphores. ;-)
>
> We could check "mq->flags & MMC_QUEUE_SUSPENDED" in the kthread to see
> if the queue is currently suspended, and otherwise go to sleep there,
> and then call wake_up() in the resume function.
Hm... so simply:
if (mq->flags & MMC_QUEUE_SUSPENDED)
schedule();
?
This whole kthread business is pretty messy. I would prefer if I could
just convert it to a workqueue. Just that it's not very simple the way
it speculates around in the request queue from the block layer.
> While looking at that code, I just noticed that access to
> mq->flags is racy and should be fixed as well.
I guess we should take the queue lock &q->lock around accessing
the flags.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH 0/2] mmc: sdhci-pci: Add GLK support and misc
From: Adrian Hunter @ 2016-11-22 9:03 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, David E. Box
Hi
Here are a couple of small patches.
Adrian Hunter (1):
mmc: sdhci-pci: Add support for Intel GLK
David E. Box (1):
mmc: sdhci-pci: Allow deferred probe for sd card detect gpio
drivers/mmc/host/sdhci-pci-core.c | 42 +++++++++++++++++++++++++++++++++------
drivers/mmc/host/sdhci-pci.h | 3 +++
2 files changed, 39 insertions(+), 6 deletions(-)
Regards
Adrian
^ permalink raw reply
* [PATCH 1/2] mmc: sdhci-pci: Add support for Intel GLK
From: Adrian Hunter @ 2016-11-22 9:03 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, David E. Box
In-Reply-To: <1479805418-19603-1-git-send-email-adrian.hunter@intel.com>
Add support for eMMC/SD/SDIO Intel GLK host controllers.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/host/sdhci-pci-core.c | 27 ++++++++++++++++++++++++++-
drivers/mmc/host/sdhci-pci.h | 3 +++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 1d9e00a00e9f..501098e65b0e 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -390,7 +390,8 @@ static int byt_sd_probe_slot(struct sdhci_pci_slot *slot)
slot->cd_override_level = true;
if (slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_BXT_SD ||
slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_BXTM_SD ||
- slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_APL_SD) {
+ slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_APL_SD ||
+ slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_GLK_SD) {
slot->host->mmc_host_ops.get_cd = bxt_get_cd;
slot->host->mmc->caps |= MMC_CAP_AGGRESSIVE_PM;
}
@@ -1277,6 +1278,30 @@ static int amd_probe(struct sdhci_pci_chip *chip)
},
{
+ .vendor = PCI_VENDOR_ID_INTEL,
+ .device = PCI_DEVICE_ID_INTEL_GLK_EMMC,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
+ .driver_data = (kernel_ulong_t)&sdhci_intel_byt_emmc,
+ },
+
+ {
+ .vendor = PCI_VENDOR_ID_INTEL,
+ .device = PCI_DEVICE_ID_INTEL_GLK_SDIO,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
+ .driver_data = (kernel_ulong_t)&sdhci_intel_byt_sdio,
+ },
+
+ {
+ .vendor = PCI_VENDOR_ID_INTEL,
+ .device = PCI_DEVICE_ID_INTEL_GLK_SD,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
+ .driver_data = (kernel_ulong_t)&sdhci_intel_byt_sd,
+ },
+
+ {
.vendor = PCI_VENDOR_ID_O2,
.device = PCI_DEVICE_ID_O2_8120,
.subvendor = PCI_ANY_ID,
diff --git a/drivers/mmc/host/sdhci-pci.h b/drivers/mmc/host/sdhci-pci.h
index 6bccf56bc5ff..4abdaed72bd4 100644
--- a/drivers/mmc/host/sdhci-pci.h
+++ b/drivers/mmc/host/sdhci-pci.h
@@ -34,6 +34,9 @@
#define PCI_DEVICE_ID_INTEL_APL_SD 0x5aca
#define PCI_DEVICE_ID_INTEL_APL_EMMC 0x5acc
#define PCI_DEVICE_ID_INTEL_APL_SDIO 0x5ad0
+#define PCI_DEVICE_ID_INTEL_GLK_SD 0x31ca
+#define PCI_DEVICE_ID_INTEL_GLK_EMMC 0x31cc
+#define PCI_DEVICE_ID_INTEL_GLK_SDIO 0x31d0
/*
* PCI registers
--
1.9.1
^ permalink raw reply related
* [PATCH 2/2] mmc: sdhci-pci: Allow deferred probe for sd card detect gpio
From: Adrian Hunter @ 2016-11-22 9:03 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, David E. Box
In-Reply-To: <1479805418-19603-1-git-send-email-adrian.hunter@intel.com>
From: "David E. Box" <david.e.box@linux.intel.com>
With commit f35bbf61ab77 ("gpio / ACPI: Return -EPROBE_DEFER if the
gpiochip was not found"), a gpio descriptor request can now be deferred if
the providing gpio host controller driver hasn't been loaded yet. Allow use
in mmc slot probe in order to prevent card detect gpio setup from failing
in this case.
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/mmc/host/sdhci-pci-core.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 501098e65b0e..2d20fb60ce83 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -1760,11 +1760,16 @@ static struct sdhci_pci_slot *sdhci_pci_probe_slot(
host->mmc->slotno = slotno;
host->mmc->caps2 |= MMC_CAP2_NO_PRESCAN_POWERUP;
- if (slot->cd_idx >= 0 &&
- mmc_gpiod_request_cd(host->mmc, slot->cd_con_id, slot->cd_idx,
- slot->cd_override_level, 0, NULL)) {
- dev_warn(&pdev->dev, "failed to setup card detect gpio\n");
- slot->cd_idx = -1;
+ if (slot->cd_idx >= 0) {
+ ret = mmc_gpiod_request_cd(host->mmc, slot->cd_con_id, slot->cd_idx,
+ slot->cd_override_level, 0, NULL);
+ if (ret == -EPROBE_DEFER)
+ goto remove;
+
+ if (ret) {
+ dev_warn(&pdev->dev, "failed to setup card detect gpio\n");
+ slot->cd_idx = -1;
+ }
}
ret = sdhci_add_host(host);
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox