- 14 Feb, 2017 3 commits
-
-
Linus Walleij authored
This function is doing to many clever things at the same time under too many various conditions. Start to make things clearer by refactoring: break out the finalization of the previous asynchronous request to its own function mmc_finalize_areq(). We can get rid of the default assignment of status and let the call deal with this. Signed-off-by:
Linus Walleij <linus.walleij@linaro.org> Signed-off-by:
Ulf Hansson <ulf.hansson@linaro.org>
-
Linus Walleij authored
The return value from blk_end_request() is a bool but is treated like an int. This is generally safe, but the variable also has the opaque name "ret" and gets returned from the helper function mmc_blk_cmd_err(). - Switch the variable to a bool, applies everywhere. - Return a bool from mmc_blk_cmd_err() and rename the function mmc_blk_rw_cmd_err() to indicate through the namespace that this is a helper for mmc_blk_issue_rw_rq(). - Rename the variable from "ret" to "req_pending" inside the while() loop inside mmc_blk_issue_rq_rq(), which finally makes it very clear what this while loop is waiting for. - Augment the argument "ret" to mmc_blk_rq_cmd_err() to old_req_pending so it becomes evident that this is an older state, and it is returned only if we fail to get the number of written blocks from an SD card in the function mmc_sd_num_wr_blocks(). - Augment the while() loop in mmc_blk_rq_cmd_abort(): it is evident now that we know this is a bool variable, that the function is just spinning waiting for blk_end_request() to return false. Signed-off-by:
-
Linus Walleij authored
mmc_sd_num_wr_blocks() has an interesting construction that saves one return argument by casting (u32)-1 as error code if something goes wrong. This is however a bit confusing when the normal kernel pattern is to return an int error code on success. So instead pass a variable "blocks" that the function can fill in with the number of successfully transferred blocks and return an integer as error code. Signed-off-by:
-
- 13 Feb, 2017 37 commits
-
-
Linus Walleij authored
Instead of masking and setting two bits in the "flags" field for the mmc_queue, just use two bools named "suspended" and "new_request". The masking and setting would likely have race conditions anyways, it is better to use a simple member like this. Signed-off-by:
-
Linus Walleij authored
The mmc_active member of struct mmc_queue_req has a very confusing name: this is certainly not always "active", it is the asynchronous request associated by the mmc_queue_req but it is not guaranteed to be "active" in any sense, such as being running on the host. Simply rename this member to "areq". Signed-off-by:
-
Linus Walleij authored
The mmc_blk_rw_start_new() was named after the label inside mmc_blk_issue_rw_rq() but is really a confusing name for this function: what it does is to try to restart the latest issued command on the host and card of the current MMC queue. So rename it mmc_blk_rw_try_restart() that reflects what it is doing and at this point also refactore the function to treat the removed card as an exception and just exit if this happens and run on in the function if that is not happening. Signed-off-by:
-
Linus Walleij authored
With the coexisting __mmc_start_request(), mmc_start_request() and __mmc_start_req() it is a bit confusing that mmc_start_req() actually does not start a normal request, but an asynchronous request. Rename it to mmc_start_areq() to make it explicit what the function is doing, also fix the kerneldoc for this function while we're at it. Signed-off-by:
-
Linus Walleij authored
In the function mmc_blk_issue_rw_rq() the new request coming in from the block layer is called "rqc" and the old request that was potentially just returned back from the asynchronous mechanism is called "req". This is really confusing when trying to analyze and understand the code, it becomes a perceptual nightmare to me. Maybe others have better parserheads but it is not working for me. Rename "rqc" to "new_req" and "req" to "old_req" to reflect what is semantically going on into the syntax. Signed-off-by:
-
Linus Walleij authored
The goto statements sprinkled over the mmc_blk_issue_rw_rq() function has grown over the years and makes the code pretty hard to read. Inline the calls such that: goto cmd_abort; -> mmc_blk_rw_cmd_abort(card, req); mmc_blk_rw_start_new(mq, card, rqc); return; goto start_new_req; -> mmc_blk_rw_start_new(mq, card, rqc); return; After this it is more clear how we exit the do {} while loop in this function, and it gets possible to split the code apart. Signed-off-by: -
Heiner Kallweit authored
ida code in block.c can be significantly simplified by switching to the ida_simple_ functions. Signed-off-by:
Heiner Kallweit <hkallweit1@gmail.com> Signed-off-by:
-
Ravikumar Kattekola authored
Fixes: a45c6cb8 ("[ARM] 5369/1: omap mmc: Add new omap hsmmc controller for 2430 and 34xx, v3") when using really large timeout (up to 4*60*1000 ms for bkops) there is a possibility of data overflow using unsigned int so use 64 bit unsigned long long. Signed-off-by:
Ravikumar Kattekola <rk@ti.com> Signed-off-by:
Sekhar Nori <nsekhar@ti.com> Signed-off-by:
-
Kishon Vijay Abraham I authored
commit e2bf08d6 ("omap_hsmmc: set a large data timeout for commands with busy signal") sets an arbitrary timeout value (100ms) for commands like CMD6 (MMC SWITCH). However extended CSD register defined in the eMMC standard has a field for GENERIC_CMD6_TIME which indicates the default maximum timeout for a SWITCH command. Use busy_timeout of cmd structure (populated with GENERIC_CMD6_TIME in the case of SWITCH command) to program the data timeout value in omap_hsmmc driver. SWITCH command to turn the cache on took more than 100ms to complete with MICRON eMMC card present in AM572x IDK REV 1.3A resulting in timeout and failed enumeration. It is fixed here by programming the timeout with the value advertised in GENERIC_CMD6_TIME. Signed-off-by:
Kishon Vijay Abraham I <kishon@ti.com> Signed-off-by:
-
Ravikumar Kattekola authored
When CEB (command end bit error) occurs reset CMD line to avoid system ending up in erroneous state. While command line is reset for CTO and CCRC errors, it's not done for CEB error. Fix it here. Reviewed-by:
-
Maxime Ripard authored
We introduced recently a new compatible to deal with the A64 eMMC controller, let's document its binding. Signed-off-by:
Maxime Ripard <maxime.ripard@free-electrons.com> Acked-by:
Chen-Yu Tsai <wens@csie.org> Acked-by:
Rob Herring <robh@kernel.org> Signed-off-by:
-
Shawn Lin authored
When deploying runtime PM, it's quite verbose to print the log of ios setting. Also it's useless to print it from system PM as it should be the same with booting time. We also have sysfs to get all these information from ios attribute, so let's skip this print from PM context. Signed-off-by:
Shawn Lin <shawn.lin@rock-chips.com> Signed-off-by:
Jaehoon Chung <jh80.chung@samsung.com> Signed-off-by:
-
Linus Walleij authored
commit 64b12a68 "mmc: core: fix prepared requests while doing bkops" is fixing a bug in the wrong way. A bug in the MMCI device driver is fixed by amending the MMC core. Thinking about it: what the pre- and post-callbacks are doing is to essentially map and unmap SG lists for DMA transfers. Why would we not be able to do that just because a BKOPS command is sent inbetween? Having to unprepare/prepare the next asynchronous request for DMA seems wrong. Looking the backtrace in that commit we can see what the real problem actually is: mmci_data_irq() is calling mmci_dma_unmap() twice which is goung to call arm_dma_unmap_sg() twice and v7_dma_inv_range() twice for the same sglist and that will crash. This happens because a request is prepared, then a BKOPS is sent. The IRQ completing the BKOPS command goes through mmci_data_irq() and thinks that a DMA operation has just been completed because dma_inprogress() reports true. It then proceeds to unmap the sglist. But that was wrong! dma_inprogress() should NOT be true because no DMA was actually in progress! We had just prepared the sglist, and the DMA channel dma_current has been configured, but NOT started! Because of this, the sglist is already unmapped when we get our actual data completion IRQ, and we are unmapping the sglist once more, and we get this crash. Therefore, we need to revert this solution pushing the problem to the core and causing problems, and instead augment the implementation such that dma_inprogress() only reports true if some DMA has actually been started. After this we can keep the request prepared during the BKOPS and we need not unprepare/reprepare it. Fixes: 64b12a68 ("mmc: core: fix prepared requests while doing bkops") Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Signed-off-by:
Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Signed-off-by:
-
Maxime Ripard authored
Add a bit more debug messages that can be helpful when debugging the clock setup. Also fill the actual_clock field in struct mmc_host to report properly the current frequency in debugfs. Signed-off-by:
Florian Vaussard <florian.vaussard@heig-vd.ch> Acked-by:
-
Maxime Ripard authored
The MMC2 controller on the A64 is kind of a special beast. While the general controller design is the same than the other MMC controllers in the SoC, it also has a bunch of features and changes that prevent it to be driven in the same way. It has for example a different bus width limit, a different maximum frequency, and, for some reason, the maximum buffer size of a DMA descriptor. Add a new compatible specifically for this controller. Acked-by:
-
Maxime Ripard authored
The A64 MMC controllers need DATA0 to be masked while updating the clock, otherwise any subsequent command will result in a timeout. It's not really clear at this point what DATA0 is exactly, but this behaviour is present in Allwinner's tree, and has been suggested by Allwinner engineers as fixes for the timeout. Signed-off-by:
-
Maxime Ripard authored
The A64 MMC controllers need to set a "new timings" bit when a new rate is set. The actual meaning of that bit is not clear yet, but not setting it leads to some corner-case issues, like the CMD53 failing, which is used to implement SDIO packet aggregation. Signed-off-by:
-
Maxime Ripard authored
Experience have shown that the using the autocalibration could severely degrade the performances of the MMC bus. Allwinner is using in its BSP a delay set to 0 for all the modes but HS400. Remove the calibration code for now, and add comments to document our findings. Reviewed-by:
Andre Przywara <andre.przywara@arm.com> Signed-off-by:
-
Maxime Ripard authored
The MMC core assumes that the code will gate the clock when the bus frequency is set to 0, which we've been ignoring so far. Handle that. Signed-off-by:
-
Maxime Ripard authored
The SD specification documents that the clock frequency should only be changed once gated (Section 3.2.3 - SD Clock Frequency Change Sequence). The current code first modifies the parent clock, gates it and then modifies the internal divider. This means that since the parent clock rate might be changed, the bus clock might be changed as well before it is gated, which breaks the specification. Move the gating before the parent rate modification. Signed-off-by:
-
Heiner Kallweit authored
Properly reverse everything if mmc_gpio_alloc(host) fails. Signed-off-by:
-
Heiner Kallweit authored
ida handling can be simplified by switching to the ida_simple_ functions. Signed-off-by:
-
Ritesh Harjani authored
This removes CDC init sequence comments which are not useful anyway. Signed-off-by:
Ritesh Harjani <riteshh@codeaurora.org> Acked-by:
Adrian Hunter <adrian.hunter@intel.com> Signed-off-by:
-
Ritesh Harjani authored
platform_execute_tuning should not really exist as it does not do anything useful. So remove this ops and directly plug sdhci_msm_execute_tuning with mmc_host_ops. Also in case of HS400 tuning clear SDHCI_HS400_TUNING flag once HS400 related mode selection is done. Signed-off-by:
-
Ulf Hansson authored
When mmc_of_parse() finds the binding, it sets the mmc cap, MMC_CAP_3_3V_DDR, which informs the core whether eMMC DDR at 3.3V I/O is supported by the mmc host. Signed-off-by:
Jan Glauber <jglauber@cavium.com> Tested-by:
Stefan Wahren <stefan.wahren@i2se.com>
-
Ulf Hansson authored
Cc: <devicetree@vger.kernel.org> Cc: Rob Herring <robh@kernel.org> Signed-off-by:
-
Ulf Hansson authored
According the JEDEC specification an eMMC card supporting 1.8V vccq in DDR mode should also be capable of 3.3V. However, it's been reported that some mmc hosts supports 3.3V, but not 1.8V. Currently the mmc core implements an error handling when the host fails to set 1.8V for vccq, by falling back to 3.3V. Unfortunate, this seems to be insufficient for some mmc hosts. To enable these to use eMMC DDR mode let's invent a new mmc cap, MMC_CAP_3_3V_DDR, which tells whether they support the eMMC 3.3V DDR mode. In case MMC_CAP_3_3V_DDR is set, but not MMC_CAP_1_8V_DDR, let's change to remain on the 3.3V, as it's the default voltage level for vccq, set by the earlier power up sequence. As this change introduces MMC_CAP_3_3V_DDR, let's take the opportunity to do some re-formatting of the related defines in the header file. Signed-off-by:
-
Ulf Hansson authored
Earlier the mmc_set_signal_voltage() existed, but since it has been renamed to mmc_set_uhs_voltage(), we can now use that name instead. Signed-off-by:
-
Ulf Hansson authored
The mmc_set_signal_voltage() function is used for SD/SDIO when switching to 1.8V for UHS mode. To clarify this let's do the following changes. - We are always providing MMC_SIGNAL_VOLTAGE_180 as the signal_voltage parameter to the function. Then, let's just remove the parameter as it serves no purpose. - Rename the function to mmc_set_uhs_voltage(). Signed-off-by:
-
Ulf Hansson authored
The mmc_set_signal_voltage() function is used for SD/SDIO when switching to 1.8V for UHS mode. Therefore let's remove the redundant code dealing with MMC_SIGNAL_VOLTAGE_330. Signed-off-by:
-
Chris Brandt authored
In the case of a single clock source, you don't need names. However, if the controller has 2 clock sources, you need to name them correctly so the driver can find the 2nd one. The 2nd clock is for the internal card detect logic. Signed-off-by:
Chris Brandt <chris.brandt@renesas.com> Reviewed-by:
Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by:
Wolfram Sang <wsa+renesas@sang-engineering.com> Acked-by:
-
Chris Brandt authored
Some controllers have 2 clock sources instead of 1. The 2nd clock is for the internal card detect logic and must be enabled/disabled along with the main core clock for proper operation. Signed-off-by:
-
Kevin Hilman authored
Cleanup some debug prints that cause needless noise during normal usage. Signed-off-by:
Kevin Hilman <khilman@baylibre.com> Signed-off-by:
-
Linus Walleij authored
The mmc_blk_issue_rq() function is called in exactly one place in queue.c and there the return value is ignored. So the functions called from that function that also meticulously return 0/1 do so for no good reason. Error reporting on the asynchronous requests are done upward to the block layer when the requests are eventually completed or fail, which may happen during the flow of the mmc_blk_issue_* functions directly (for "special commands") or later, when an asynchronous read/write request is completed. The issuing functions do not give rise to errors on their own, and there is nothing to return back to the caller in queue.c. Drop all return values and make the function return void. Signed-off-by:
-
Linus Walleij authored
Recycling the same variable in an x=x+1 fashion may seem clever here but it makes the code terse and hard to follow for humans. Introduce a new_areq and old_areq variable so we see what is going on. Signed-off-by:
-
Linus Walleij authored
Setting rqc to NULL followed by a goto to cmd_abort is just a way to do unconditional abort without starting any new command. Inline the calls to mmc_blk_rw_cmd_abort() and return immediately in those cases. Add some comments to the code flow so it is clear that this is where the asynchronous requests come back in and the result of them gets handled. Signed-off-by:
-
Linus Walleij authored
The code in mmc_blk_issue_rq_rq() aborts a command if the request is not properly aligned on large sectors. As part of the path jumping out, it assigns the local variable mq_rq reflecting a MMC queue request to the current MMC queue request, which is confusing since the variable is not used after this jump. Signed-off-by:
-
