[PATCH 1/2] mmc: mmci: fix an ages old detection error
commit 4956e10903fd3459306dd9438c1e714ba3068a2a
"ARM: 6244/1: mmci: add variant data and default MCICLOCK support"
added variant data for ARM, U300 and Ux500 variants. The
Nomadik NHK8815/8820 variant was erroneously labeled as a
U300 variant, and when the proper Nomadik variant was
later introduced in commit
34fd421349ffc6a4280b71276bf7c6d48f92156f
"ARM: 7378/1: mmci: add support for the Nomadik MMCI variant"
this was not fixes. Let's say this fixes the latter commit
as there was no proper Nomadik support until then.
Cc: sta...@vger.kernel.org
Fixes: 34fd421349ff "ARM: 7378/1: mmci: add support for the Nomadik MMCI
variant"
Signed-off-by: Linus Walleij <linus.wall...@linaro.org>
---
drivers/mmc/host/mmci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index fb266745f824..acece3299756 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1886,7 +1886,7 @@ static struct amba_id mmci_ids[] = {
{
.id = 0x00280180,
.mask = 0x00ff,
- .data = _u300,
+ .data = _nomadik,
},
{
.id = 0x00480180,
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] mmc: mvsdio: delete platform data code path
On Wed, Nov 25, 2015 at 4:02 PM, Andrew Lunn <and...@lunn.ch> wrote: > On Wed, Nov 25, 2015 at 02:57:57PM +0100, Linus Walleij wrote: >> There are no in-kernel users of the MVSDIO platform data method >> (instantiating from a board file) so just delete this code and >> make this a DT-only driver. We depend on OF and check that we have >> an OF node in probe(). >> >> Cc: Nicolas Pitre <n...@fluxnic.net> >> Cc: Andrew Lunn <and...@lunn.ch> >> Cc: Sebastian Hesselbarth <sebastian.hesselba...@gmail.com> >> Signed-off-by: Linus Walleij <linus.wall...@linaro.org> > > Looks good. I guess i would of reversed the order of these two patches > so you don't first add struct mvsdio_platform_data to the driver and > then remove it, but that is a minor nit pick. > > Acked-by: Andrew Lunn <and...@lunn.ch> Ulf can just squash them if he likes, that'd do it. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] mmc: mvsdio: delete platform data header
This platform data struct is only used inside the MVSDIO driver,
nowhere else in the entire kernel. Move the struct into the
driver and delete the external header.
Cc: Nicolas Pitre <n...@fluxnic.net>
Cc: Andrew Lunn <and...@lunn.ch>
Cc: Sebastian Hesselbarth <sebastian.hesselba...@gmail.com>
Signed-off-by: Linus Walleij <linus.wall...@linaro.org>
---
drivers/mmc/host/mvsdio.c| 7 ++-
include/linux/platform_data/mmc-mvsdio.h | 18 --
2 files changed, 6 insertions(+), 19 deletions(-)
delete mode 100644 include/linux/platform_data/mmc-mvsdio.h
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index a448498e3af2..18c70380ea93 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -28,7 +28,6 @@
#include
#include
-#include
#include "mvsdio.h"
@@ -37,6 +36,12 @@
static int maxfreq;
static int nodma;
+struct mvsdio_platform_data {
+ unsigned int clock;
+ int gpio_card_detect;
+ int gpio_write_protect;
+};
+
struct mvsd_host {
void __iomem *base;
struct mmc_request *mrq;
diff --git a/include/linux/platform_data/mmc-mvsdio.h
b/include/linux/platform_data/mmc-mvsdio.h
deleted file mode 100644
index d02704cd3695..
--- a/include/linux/platform_data/mmc-mvsdio.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/*
- * This file is licensed under the terms of the GNU General Public
- * License version 2. This program is licensed "as is" without any
- * warranty of any kind, whether express or implied.
- */
-
-#ifndef __MMC_MVSDIO_H
-#define __MMC_MVSDIO_H
-
-#include
-
-struct mvsdio_platform_data {
- unsigned int clock;
- int gpio_card_detect;
- int gpio_write_protect;
-};
-
-#endif
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] mmc: mvsdio: delete platform data code path
There are no in-kernel users of the MVSDIO platform data method
(instantiating from a board file) so just delete this code and
make this a DT-only driver. We depend on OF and check that we have
an OF node in probe().
Cc: Nicolas Pitre <n...@fluxnic.net>
Cc: Andrew Lunn <and...@lunn.ch>
Cc: Sebastian Hesselbarth <sebastian.hesselba...@gmail.com>
Signed-off-by: Linus Walleij <linus.wall...@linaro.org>
---
drivers/mmc/host/Kconfig | 1 +
drivers/mmc/host/mvsdio.c | 63 +++
2 files changed, 15 insertions(+), 49 deletions(-)
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 1dee533634c9..1526b8a10b09 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -455,6 +455,7 @@ config MMC_TIFM_SD
config MMC_MVSDIO
tristate "Marvell MMC/SD/SDIO host driver"
depends on PLAT_ORION
+ depends on OF
---help---
This selects the Marvell SDIO host driver.
SDIO may currently be found on the Kirkwood 88F6281 and 88F6192
diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 18c70380ea93..42296e55b9de 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -20,8 +20,6 @@
#include
#include
#include
-#include
-#include
#include
#include
#include
@@ -36,12 +34,6 @@
static int maxfreq;
static int nodma;
-struct mvsdio_platform_data {
- unsigned int clock;
- int gpio_card_detect;
- int gpio_write_protect;
-};
-
struct mvsd_host {
void __iomem *base;
struct mmc_request *mrq;
@@ -709,6 +701,10 @@ static int mvsd_probe(struct platform_device *pdev)
struct resource *r;
int ret, irq;
+ if (!np) {
+ dev_err(>dev, "no DT node\n");
+ return -ENODEV;
+ }
r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
irq = platform_get_irq(pdev, 0);
if (!r || irq < 0)
@@ -732,8 +728,12 @@ static int mvsd_probe(struct platform_device *pdev)
* fixed rate clock).
*/
host->clk = devm_clk_get(>dev, NULL);
- if (!IS_ERR(host->clk))
- clk_prepare_enable(host->clk);
+ if (IS_ERR(host->clk)) {
+ dev_err(>dev, "no clock associated\n");
+ ret = -EINVAL;
+ goto out;
+ }
+ clk_prepare_enable(host->clk);
mmc->ops = _ops;
@@ -749,45 +749,10 @@ static int mvsd_probe(struct platform_device *pdev)
mmc->max_seg_size = mmc->max_blk_size * mmc->max_blk_count;
mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
- if (np) {
- if (IS_ERR(host->clk)) {
- dev_err(>dev, "DT platforms must have a clock
associated\n");
- ret = -EINVAL;
- goto out;
- }
-
- host->base_clock = clk_get_rate(host->clk) / 2;
- ret = mmc_of_parse(mmc);
- if (ret < 0)
- goto out;
- } else {
- const struct mvsdio_platform_data *mvsd_data;
-
- mvsd_data = pdev->dev.platform_data;
- if (!mvsd_data) {
- ret = -ENXIO;
- goto out;
- }
- mmc->caps = MMC_CAP_4_BIT_DATA | MMC_CAP_SDIO_IRQ |
- MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
- host->base_clock = mvsd_data->clock / 2;
- /* GPIO 0 regarded as invalid for backward compatibility */
- if (mvsd_data->gpio_card_detect &&
- gpio_is_valid(mvsd_data->gpio_card_detect)) {
- ret = mmc_gpio_request_cd(mmc,
- mvsd_data->gpio_card_detect,
- 0);
- if (ret)
- goto out;
- } else {
- mmc->caps |= MMC_CAP_NEEDS_POLL;
- }
-
- if (mvsd_data->gpio_write_protect &&
- gpio_is_valid(mvsd_data->gpio_write_protect))
- mmc_gpio_request_ro(mmc, mvsd_data->gpio_write_protect);
- }
-
+ host->base_clock = clk_get_rate(host->clk) / 2;
+ ret = mmc_of_parse(mmc);
+ if (ret < 0)
+ goto out;
if (maxfreq)
mmc->f_max = maxfreq;
--
2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: core: Remove MMC_CLKGATE
On Fri, Oct 2, 2015 at 1:56 AM, Ulf Hansson <ulf.hans...@linaro.org> wrote: > MMC_CLKGATE was once invented to save power by gating the bus clock at > request inactivity. At that time it served its purpose. The modern way to > deal with power saving for these scenarios, is by using runtime PM. > > Nowadays, several host drivers have deployed runtime PM, but for those > that haven't and which still cares power saving at request inactivity, > it's certainly time to deploy runtime PM as it has been around for several > years now. > > To simplify code to mmc core and thus decrease maintenance efforts, this > patch removes all code related to MMC_CLKGATE. > > Signed-off-by: Ulf Hansson <ulf.hans...@linaro.org> Runtime PM is the generic solution to the specific problem, good feat. Reviewed-by: Linus Walleij <linus.wall...@linaro.org> Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 2/3] mmc: sdhci: add host_ops->voltage_switch callback for all other voltages
On Wed, Sep 2, 2015 at 1:19 AM, Vaibhav Hiremath <vaibhav.hirem...@linaro.org> wrote: > Not quite sure whether regulator would be right fit for this. > > Initially I was thinking of making use of pinconf framework, using > PIN_CONFIG_POWER_SOURCE, but that too I am not sure is the right way of > doing it. > > Probably, question for pinctrl maintainer. Looping Linus Walleji. We have another pinctrl driver exposing the occasional fixed regulator for exactly this purpose, see: sh-pfc/pfc-sh73a0.c So use the same mechanism for this driver. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 4/7] mmc: sdhci-pxav3: Add pinctl setting according to bus clock
On Tue, Sep 8, 2015 at 5:07 PM, Vaibhav Hiremath <vaibhav.hirem...@linaro.org> wrote: > But I still have one small doubt on expectation from > devm_pinctrl_get() function. > > If pinctrl properties are not populated in Devicetree node, > then, shouldn't devm_pinctrl_get() return error ? > I followed the code flow, and it seems even if pinctrl properties are > not populated in DT node, the devm_pinctrl_get() returns valid > pointer to "struct pinctrl", isn't this against the expectation of the > call? > > > Code flow - > > devm_pinctrl_get() > ... > --> creat_pinctrl() That is the spelling mistake Dennis Ritchie and Ken Thompson wish they had avoided in the first syscall interface :D > --> pinctrl_dt_to_map() > ... > > pinctrl_dt_to_map() iterates for pinctrl-x (x = 0,1,...) and if it > founds the entry then it parses the node. If it doesn't find any > pinctrl property then also it returns 0. and subsequently rreturns > handle to "struct pinctrl" for the device. Why is so? So pinctrl_dt_to_map() returns 0 if there are no maps in the device tree. This is correct: there may be systems that have a mixture of device tree and platform data, and in that case the code needs to continue after calling pinctrl_dt_to_map() because else it bails out before coming to the loop in create_pinctrl() where we iterate over the static maps. So after the loop in create_pinctrl() it should be possible to return something like -ENOENT if we didn't find anything. The device core pinctrl handling in drivers/base/pinctrl.c seems to cope with it. It's a semantic change so we would need to test to toss it in and see what happens on some different systems but in principe I think you're right. What we get if there is no state is basically a dummy pinctrl that does nothing. Do you wanna make a patch for this? (Looping in Stephen Warren so he can tell if I miss something obviously evident in the design that require stubs to be present.) Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH-v2 4/7] mmc: sdhci-pxav3: Add pinctl setting according to bus clock
On Mon, Sep 7, 2015 at 1:18 PM, Vaibhav Hiremath
<vaibhav.hirem...@linaro.org> wrote:
> Different bus clock may need different pin setting.
> For example, fast bus clock like 208Mhz need pin drive fast
> while slow bus clock prefer pin drive slow to guarantee
> signal quality.
>
> So this patch creates two states,
> - Default (slow/normal) pin state
> - And fast pin state for higher freq bus speed.
>
> And selection of pin state is done based on timing mode.
>
> Signed-off-by: Vaibhav Hiremath <vaibhav.hirem...@linaro.org>
> Signed-off-by: Kevin Liu <kl...@marvell.com>
(...)
> + pxa->pinctrl = devm_pinctrl_get(dev);
> + if (!IS_ERR(pxa->pinctrl)) {
> + pxa->pins_default = pinctrl_lookup_state(pxa->pinctrl,
> "default");
> + if (IS_ERR(pxa->pins_default))
> + dev_err(dev, "could not get default pinstate\n");
> + pxa->pins_fast = pinctrl_lookup_state(pxa->pinctrl, "fast");
> + if (IS_ERR(pxa->pins_fast))
> + dev_info(dev, "could not get fast pinstate\n");
> + }
This is exactly how I think it should be used from a pin control
point of view.
If you depended on CONFIG_PM you could use
pinctrl_pm_select_default_state() but for this simple scenario
this is fine.
Reviewed-by: Linus Walleij <linus.wall...@linaro.org>
>From a pinctrl point of view.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/8] pinctrl: sh-pfc: Implement pinconf power-source param for voltage switching
On Tue, Jun 30, 2015 at 6:53 PM, Ben Hutchings ben.hutchi...@codethink.co.uk wrote: The pfc in the R8A7790 (and probably others in the R-Car gen 2 family) supports switching SDHI signals between 3.3V and 1.8V nominal voltage, and the SD driver should do that when switching to and from UHS modes. Add a flag for pins that have configurable I/O voltage and SoC operations to get and set the nominal voltage. Implement the pinconf power-source parameter using these operations. Signed-off-by: Ben Hutchings ben.hutchi...@codethink.co.uk Patch applied with Laurent's ACK. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 3/6] pinctrl: sh-pfc: r8a7790: Add separate functions for SDHI 1.8V operation
On Mon, Jun 15, 2015 at 4:02 AM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: On a side note, I have a patch to support the standard groups, functions and pins properties instead of the Renesas-specific versions, I'll try to revive it. Yes please :) Makes things a little less confusing in this confusing boundary between electronics and code... Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: pcf857x: restore the initial line state of all pcf lines
On Wed, Mar 18, 2015 at 2:21 PM, Kishon Vijay Abraham I kis...@ti.com wrote: [Me] Specify exactly what stuff may happen after the reboot notifier. okay, it's assumed the device may be used (active) till the shutdown handler of that particular device is called. In this particular case we are restoring the initial line state of all pcf lines even though we don't know if the devices connected to it are still being active, which might cause a contention as explained by NM [1] OK I understand this, just that I want to know if it is a practical problem, i.e. do you run into it? Does the system lock up? If the reboot notifier gets invoked after the shutdown handler then we could be sure that restoring the initial line state of the pcf lines wouldn't affect the devices connected to it. OK so does it? ftrace is your friend. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: pcf857x: restore the initial line state of all pcf lines
On Mon, Mar 16, 2015 at 9:46 AM, Kishon Vijay Abraham I kis...@ti.com wrote:
On Wednesday 14 January 2015 05:28 PM, Linus Walleij wrote:
#include linux/reboot.h
static int foo_reboot_handler(struct notifier_block *this,
unsigned long code,
void *unused)
{
pr_crit(do some last minute stuff\n);
return NOTIFY_OK;
}
static struct notifier_block foo_reboot_notifier = {
.notifier_call = foo_reboot_handler,
};
register_reboot_notifier(foo_reboot_notifier);
Added debug prints and found the reboot notifier gets invoked before the
shutdown handler which means some stuff can be done after this reboot
notifier:-(
Specify exactly what stuff may happen after the reboot notifier.
Of course stuff happens both before and after the reboot
notifier, the question is exactly what, that may conflict with this.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 4/4] mmc: pwrseq_simple: Add support for a reset GPIO pin
On Wed, Feb 11, 2015 at 5:34 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 10 February 2015 at 10:12, Alexandre Courbot gnu...@gmail.com wrote: This code would be a great candidate to use this GPIO array API, but since it is not in -next yet (should happen soon though) you might want to consider doing it later. This is now in my git and -next, I can take patches to pwrseq if Ulf ACKs them to go through my GPIO tree. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] pinctrl: mediatek: emulate GPIO interrupt on both-edges
On Tue, Jan 27, 2015 at 2:15 PM, Chaotian Jing chaotian.j...@mediatek.com wrote: From: Yingjoe Chen yingjoe.c...@mediatek.com MTK EINT does not support generating interrupt on both edges. Emulate this by changing edge polarity while enable irq, set types and interrupt handling. This follows an example of drivers/gpio/gpio-mxc.c. Signed-off-by: Yingjoe Chen yingjoe.c...@mediatek.com Signed-off-by: Chaotian Jing chaotian.j...@mediatek.com Patch applied on top of the rest of the mtk pinctrl support. Adding Hongzhou's ACK on top. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/7] pinctrl: mediatek: emulate GPIO interrupt on both-edges
On Tue, Jan 27, 2015 at 7:15 AM, Chaotian Jing chaotian.j...@mediatek.com wrote: From: Yingjoe Chen yingjoe.c...@mediatek.com MTK EINT does not support generating interrupt on both edges. Emulate this by changing edge polarity while enable irq, set types and interrupt handling. This follows an example of drivers/gpio/gpio-mxc.c. Signed-off-by: Yingjoe Chen yingjoe.c...@mediatek.com Signed-off-by: Chaotian Jing chaotian.j...@mediatek.com Hongzhu, if you're fine with this patch can you ACK it as maintainer of the pinctrl driver so I can merge it immediately on top of your drivern whenever we consider it finished (and if it applies still...) Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gpio: pcf857x: restore the initial line state of all pcf lines
On Mon, Jan 5, 2015 at 7:26 AM, Kishon Vijay Abraham I kis...@ti.com wrote:
On Thursday 18 December 2014 07:41 PM, Nishanth Menon wrote:
On 12/18/2014 12:18 AM, Kishon Vijay Abraham I wrote:
On Tuesday 16 December 2014 02:20 AM, Nishanth Menon wrote:
On 12/12/2014 02:06 AM, Kishon Vijay Abraham I wrote:
The reset values for all the PCF lines are high and hence on shutdown
we should drive all the lines high in order to bring it to the reset
state.
This is actually required since pcf doesn't have a reset line and even
after
warm reset (by invoking reboot in prompt) the pcf lines maintains it's
previous programmed state. This becomes a problem if the boards are
designed
to work with the default initial state.
DRA7XX_evm uses PCF8575 and one of the PCF output lines feeds to MMC/SD
and
this line should be driven high in order for the MMC/SD to be detected.
This line is modelled as regulator and the hsmmc driver takes care of
enabling
and disabling it. In the case of 'reboot', during shutdown path as part
of it's
cleanup process the hsmmc driver disables this regulator. This makes MMC
boot
not functional.
Fixed it by driving high all the pcf lines.
Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
---
drivers/gpio/gpio-pcf857x.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpio/gpio-pcf857x.c b/drivers/gpio/gpio-pcf857x.c
index 236708a..00b15b2 100644
--- a/drivers/gpio/gpio-pcf857x.c
+++ b/drivers/gpio/gpio-pcf857x.c
@@ -448,6 +448,14 @@ static int pcf857x_remove(struct i2c_client *client)
return status;
}
+static void pcf857x_shutdown(struct i2c_client *client)
+{
+ struct pcf857x *gpio = i2c_get_clientdata(client);
+
+ /* Drive all the I/O lines high */
+ gpio-write(gpio-client, BIT(gpio-chip.ngpio) - 1);
you might force a contention here - depending on System configuration.
example:
+---+
| |
| U1 | +--+ +---+
| +- | | |
+---+ | | | |
| Switch-+SoC|
+---+ | | | |
| | | | | |
| U2-+--^---+ +---+
| ||
| ||
+---+|
+--+--+
| |
| PCF |
| |
+-+
At low, SoC pin is connected to U2 as drive. when reset to high, you
now have U1 driving to the same pin that SoC has, potentially
resulting in contention.
Unfortunately, at this level, you do not know what the state of the
system is, blindly forcing a pin level will potentially cause
contention risk depending on pin configuration.
Assume we are doing a reset when the system is powered on, irrespective of
the
state of the system, we'll be forcing the pin level to the default state.
Yes, I dont deny that system will be fine *after* reset sequence is
started or completed. However there is a duration between the pcf
shutdown handler is called and the final reset handler is invoked -
that is the duration when the contention might cause device behavior.
Essentially ignoring the state various drivers have asked PCF to setup
the pins and doing a hands down configuration may have side effects we
cant properly expect.
The solution might be to invoke the shutdown handler of the various drivers
using the PCF before the shutdown handler of the PCF driver itself gets
invoked? But I'm not sure how can that be achieved in linux kernel :-(
#include linux/reboot.h
static int foo_reboot_handler(struct notifier_block *this,
unsigned long code,
void *unused)
{
pr_crit(do some last minute stuff\n);
return NOTIFY_OK;
}
static struct notifier_block foo_reboot_notifier = {
.notifier_call = foo_reboot_handler,
};
register_reboot_notifier(foo_reboot_notifier);
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] mmc: core: restore detect line inversion semantics
commit 98e90de99a0c43bd434da814c882c4332441871e
mmc: host: switch OF parser to use gpio descriptors
switched the semantic behaviour of card detect and read
only flags such that the inversion capability flag would
only be set if inversion was explicitly specified in the
device tree, in the hopes that no-one was using double
inversion.
It turns out that the XOR:ing between the explicit
inversion was indeed in use, so we need to restore the
old semantics where both ways of inversion are checked
and the end result XOR:ed.
Reported-by: Javier Martinez Canillas jav...@dowhile0.org
Tested-by: Javier Martinez Canillas jav...@dowhile0.org
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
ChangeLog v3-v4:
- Fix breakage in sdhci-acpi.c due to changed function
prototype.
ChangeLog v2-v3:
- Make it possible to pass NULL as pointer for the gpio_invert
variable, so we don't need dummy variables in callers
ChangeLog v1-v2:
- Always set override_active_level when getting CD GPIO
- Invert the return value from gpiod_is_active_low() to
follow the old semantics
---
drivers/mmc/core/host.c | 32
drivers/mmc/core/slot-gpio.c | 14 --
drivers/mmc/host/mmci.c | 4 ++--
drivers/mmc/host/sdhci-acpi.c | 2 +-
include/linux/mmc/slot-gpio.h | 4 ++--
5 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 31969436d77c..03c53b72a2d6 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -311,6 +311,7 @@ int mmc_of_parse(struct mmc_host *host)
struct device_node *np;
u32 bus_width;
int len, ret;
+ bool cap_invert, gpio_invert;
if (!host-parent || !host-parent-of_node)
return 0;
@@ -359,12 +360,15 @@ int mmc_of_parse(struct mmc_host *host)
host-caps |= MMC_CAP_NONREMOVABLE;
} else {
if (of_property_read_bool(np, cd-inverted))
- host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+ cap_invert = true;
+ else
+ cap_invert = false;
if (of_find_property(np, broken-cd, len))
host-caps |= MMC_CAP_NEEDS_POLL;
- ret = mmc_gpiod_request_cd(host, cd, 0, false, 0);
+ ret = mmc_gpiod_request_cd(host, cd, 0, true,
+ 0, gpio_invert);
if (ret) {
if (ret == -EPROBE_DEFER)
return ret;
@@ -375,13 +379,29 @@ int mmc_of_parse(struct mmc_host *host)
}
} else
dev_info(host-parent, Got CD GPIO\n);
+
+ /*
+* There are two ways to flag that the CD line is inverted:
+* through the cd-inverted flag and by the GPIO line itself
+* being inverted from the GPIO subsystem. This is a leftover
+* from the times when the GPIO subsystem did not make it
+* possible to flag a line as inverted.
+*
+* If the capability on the host AND the GPIO line are
+* both inverted, the end result is that the CD line is
+* not inverted.
+*/
+ if (cap_invert ^ gpio_invert)
+ host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
}
/* Parse Write Protection */
if (of_property_read_bool(np, wp-inverted))
- host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+ cap_invert = true;
+ else
+ cap_invert = false;
- ret = mmc_gpiod_request_ro(host, wp, 0, false, 0);
+ ret = mmc_gpiod_request_ro(host, wp, 0, false, 0, gpio_invert);
if (ret) {
if (ret == -EPROBE_DEFER)
goto out;
@@ -393,6 +413,10 @@ int mmc_of_parse(struct mmc_host *host)
} else
dev_info(host-parent, Got WP GPIO\n);
+ /* See the comment on CD inversion above */
+ if (cap_invert ^ gpio_invert)
+ host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+
if (of_find_property(np, cap-sd-highspeed, len))
host-caps |= MMC_CAP_SD_HIGHSPEED;
if (of_find_property(np, cap-mmc-highspeed, len))
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 38f76555d4bf..69bbf2adb329 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -281,6 +281,8 @@ EXPORT_SYMBOL(mmc_gpio_free_cd);
* @idx: index of the GPIO to obtain in the consumer
* @override_active_level: ignore %GPIO_ACTIVE_LOW flag
* @debounce: debounce time in microseconds
+ * @gpio_invert: will return whether the GPIO line is inverted or not, set
+ * to NULL to ignore
*
* Use this function in place of mmc_gpio_request_cd() to use the GPIO
* descriptor API. Note that it is paired
[PATCH v2] mmc: core: restore detect line inversion semantics
commit 98e90de99a0c43bd434da814c882c4332441871e
mmc: host: switch OF parser to use gpio descriptors
switched the semantic behaviour of card detect and read
only flags such that the inversion capability flag would
only be set if inversion was explicitly specified in the
device tree, in the hopes that no-one was using double
inversion.
It turns out that the XOR:ing between the explicit
inversion was indeed in use, so we need to restore the
old semantics where both ways of inversion are checked
and the end result XOR:ed.
Reported-by: Javier Martinez Canillas jav...@dowhile0.org
Tested-by: Javier Martinez Canillas jav...@dowhile0.org
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
ChangeLog v1-v2:
- Always set override_active_level when getting CD GPIO
- Invert the return value from gpiod_is_active_low() to
follow the old semantics
---
drivers/mmc/core/host.c | 32
drivers/mmc/core/slot-gpio.c | 10 --
drivers/mmc/host/mmci.c | 5 +++--
include/linux/mmc/slot-gpio.h | 4 ++--
4 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 31969436d77c..03c53b72a2d6 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -311,6 +311,7 @@ int mmc_of_parse(struct mmc_host *host)
struct device_node *np;
u32 bus_width;
int len, ret;
+ bool cap_invert, gpio_invert;
if (!host-parent || !host-parent-of_node)
return 0;
@@ -359,12 +360,15 @@ int mmc_of_parse(struct mmc_host *host)
host-caps |= MMC_CAP_NONREMOVABLE;
} else {
if (of_property_read_bool(np, cd-inverted))
- host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+ cap_invert = true;
+ else
+ cap_invert = false;
if (of_find_property(np, broken-cd, len))
host-caps |= MMC_CAP_NEEDS_POLL;
- ret = mmc_gpiod_request_cd(host, cd, 0, false, 0);
+ ret = mmc_gpiod_request_cd(host, cd, 0, true,
+ 0, gpio_invert);
if (ret) {
if (ret == -EPROBE_DEFER)
return ret;
@@ -375,13 +379,29 @@ int mmc_of_parse(struct mmc_host *host)
}
} else
dev_info(host-parent, Got CD GPIO\n);
+
+ /*
+* There are two ways to flag that the CD line is inverted:
+* through the cd-inverted flag and by the GPIO line itself
+* being inverted from the GPIO subsystem. This is a leftover
+* from the times when the GPIO subsystem did not make it
+* possible to flag a line as inverted.
+*
+* If the capability on the host AND the GPIO line are
+* both inverted, the end result is that the CD line is
+* not inverted.
+*/
+ if (cap_invert ^ gpio_invert)
+ host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
}
/* Parse Write Protection */
if (of_property_read_bool(np, wp-inverted))
- host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+ cap_invert = true;
+ else
+ cap_invert = false;
- ret = mmc_gpiod_request_ro(host, wp, 0, false, 0);
+ ret = mmc_gpiod_request_ro(host, wp, 0, false, 0, gpio_invert);
if (ret) {
if (ret == -EPROBE_DEFER)
goto out;
@@ -393,6 +413,10 @@ int mmc_of_parse(struct mmc_host *host)
} else
dev_info(host-parent, Got WP GPIO\n);
+ /* See the comment on CD inversion above */
+ if (cap_invert ^ gpio_invert)
+ host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+
if (of_find_property(np, cap-sd-highspeed, len))
host-caps |= MMC_CAP_SD_HIGHSPEED;
if (of_find_property(np, cap-mmc-highspeed, len))
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 38f76555d4bf..411f643fa369 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -281,6 +281,7 @@ EXPORT_SYMBOL(mmc_gpio_free_cd);
* @idx: index of the GPIO to obtain in the consumer
* @override_active_level: ignore %GPIO_ACTIVE_LOW flag
* @debounce: debounce time in microseconds
+ * @gpio_invert: will return whether the GPIO line is inverted or not
*
* Use this function in place of mmc_gpio_request_cd() to use the GPIO
* descriptor API. Note that it is paired with mmc_gpiod_free_cd() not
@@ -291,7 +292,7 @@ EXPORT_SYMBOL(mmc_gpio_free_cd);
*/
int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
unsigned int idx, bool override_active_level,
-unsigned int debounce)
+unsigned
Re: [PATCH] mmc: core: restore detect line inversion semantics
On Tue, Sep 30, 2014 at 5:07 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: On Tue, Sep 30, 2014 at 4:05 PM, Linus Walleij linus.wall...@linaro.org wrote: @@ -316,6 +317,8 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id, return ret; } + *gpio_invert = gpiod_is_active_low(desc); + The old code set gpio_inv_cd if (!(flags OF_GPIO_ACTIVE_LOW)) so the above should be: *gpio_invert = !gpiod_is_active_low(desc); Argh, done it like this in v2, but isn't that variable name a lie then? If it's active low then it's inverted, but if it's not active low it's not inverted right... Sigh, atleast we have restored the semantics, but maybe we need to comb over this again. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] mmc: core: restore detect line inversion semantics
commit 98e90de99a0c43bd434da814c882c4332441871e
mmc: host: switch OF parser to use gpio descriptors
switched the semantic behaviour of card detect and read
only flags such that the inversion capability flag would
only be set if inversion was explicitly specified in the
device tree, in the hopes that no-one was using double
inversion.
It turns out that the XOR:ing between the explicit
inversion was indeed in use, so we need to restore the
old semantics where both ways of inversion are checked
and the end result XOR:ed.
Reported-by: Javier Martinez Canillas jav...@dowhile0.org
Tested-by: Javier Martinez Canillas jav...@dowhile0.org
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
ChangeLog v2-v3:
- Make it possible to pass NULL as pointer for the gpio_invert
variable, so we don't need dummy variables in callers
ChangeLog v1-v2:
- Always set override_active_level when getting CD GPIO
- Invert the return value from gpiod_is_active_low() to
follow the old semantics
---
drivers/mmc/core/host.c | 32
drivers/mmc/core/slot-gpio.c | 14 --
drivers/mmc/host/mmci.c | 4 ++--
include/linux/mmc/slot-gpio.h | 4 ++--
4 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 31969436d77c..03c53b72a2d6 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -311,6 +311,7 @@ int mmc_of_parse(struct mmc_host *host)
struct device_node *np;
u32 bus_width;
int len, ret;
+ bool cap_invert, gpio_invert;
if (!host-parent || !host-parent-of_node)
return 0;
@@ -359,12 +360,15 @@ int mmc_of_parse(struct mmc_host *host)
host-caps |= MMC_CAP_NONREMOVABLE;
} else {
if (of_property_read_bool(np, cd-inverted))
- host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+ cap_invert = true;
+ else
+ cap_invert = false;
if (of_find_property(np, broken-cd, len))
host-caps |= MMC_CAP_NEEDS_POLL;
- ret = mmc_gpiod_request_cd(host, cd, 0, false, 0);
+ ret = mmc_gpiod_request_cd(host, cd, 0, true,
+ 0, gpio_invert);
if (ret) {
if (ret == -EPROBE_DEFER)
return ret;
@@ -375,13 +379,29 @@ int mmc_of_parse(struct mmc_host *host)
}
} else
dev_info(host-parent, Got CD GPIO\n);
+
+ /*
+* There are two ways to flag that the CD line is inverted:
+* through the cd-inverted flag and by the GPIO line itself
+* being inverted from the GPIO subsystem. This is a leftover
+* from the times when the GPIO subsystem did not make it
+* possible to flag a line as inverted.
+*
+* If the capability on the host AND the GPIO line are
+* both inverted, the end result is that the CD line is
+* not inverted.
+*/
+ if (cap_invert ^ gpio_invert)
+ host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
}
/* Parse Write Protection */
if (of_property_read_bool(np, wp-inverted))
- host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+ cap_invert = true;
+ else
+ cap_invert = false;
- ret = mmc_gpiod_request_ro(host, wp, 0, false, 0);
+ ret = mmc_gpiod_request_ro(host, wp, 0, false, 0, gpio_invert);
if (ret) {
if (ret == -EPROBE_DEFER)
goto out;
@@ -393,6 +413,10 @@ int mmc_of_parse(struct mmc_host *host)
} else
dev_info(host-parent, Got WP GPIO\n);
+ /* See the comment on CD inversion above */
+ if (cap_invert ^ gpio_invert)
+ host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+
if (of_find_property(np, cap-sd-highspeed, len))
host-caps |= MMC_CAP_SD_HIGHSPEED;
if (of_find_property(np, cap-mmc-highspeed, len))
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 38f76555d4bf..69bbf2adb329 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -281,6 +281,8 @@ EXPORT_SYMBOL(mmc_gpio_free_cd);
* @idx: index of the GPIO to obtain in the consumer
* @override_active_level: ignore %GPIO_ACTIVE_LOW flag
* @debounce: debounce time in microseconds
+ * @gpio_invert: will return whether the GPIO line is inverted or not, set
+ * to NULL to ignore
*
* Use this function in place of mmc_gpio_request_cd() to use the GPIO
* descriptor API. Note that it is paired with mmc_gpiod_free_cd() not
@@ -291,7 +293,7 @@ EXPORT_SYMBOL(mmc_gpio_free_cd);
*/
int mmc_gpiod_request_cd(struct mmc_host *host, const
Re: [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors
On Tue, Sep 30, 2014 at 1:30 PM, Javier Martinez Canillas jav...@dowhile0.org wrote: On Wed, Aug 27, 2014 at 1:00 PM, Linus Walleij linus.wall...@linaro.org wrote: This switches the central MMC OF parser to use gpio descriptors instead of grabbing GPIOs explicitly from the device tree. This strips out an unecessary use of the integer-based GPIO API that we want to get rid of, cuts down on code as the gpio descriptor code will handle active low flags. Acked-by: Alexandre Courbot acour...@nvidia.com Signed-off-by: Linus Walleij linus.wall...@linaro.org Card detection is failing in some boards because this patch changes some semantics. I tried to come up with a fix but I didn't find an obvious way to do it (more on that below). I think some of this may be weird semantics: the CAP flags in the MMC subsystem, MMC_CAP2_CD_ACTIVE_HIGH and MMC_CAP2_RO_ACTIVE_HIGH are somewhat confusing since the GPIO subsystem nowadays has its own concept of a line active high or low. The MMC caps make a lot of sense if the host has dedicated lines to read CD and WP, and does not use a GPIO for this. When a GPIO descriptor is in use, the thing gets a bit convoluted: what if both the GPIO *and* the CAP flag is set to inversion? So the old code did this with XOR: if (explicit_inv_cd ^ gpio_inv_cd) host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH; Meaning double inversion - no inversion. I guess I was sort of hoping that nobody did this crazy thing. Turns out I was wrong, so have to restore the old semantics... I'm sending a patch for you to test. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: core: restore detect line inversion semantics
commit 98e90de99a0c43bd434da814c882c4332441871e
mmc: host: switch OF parser to use gpio descriptors
switched the semantic behaviour of card detect and read
only flags such that the inversion capability flag would
only be set if inversion was explicitly specified in the
device tree, in the hopes that no-one was using double
inversion.
It turns out that the XOR:ing between the explicit
inversion was indeed in use, so we need to restore the
old semantics where both ways of inversion are checked
and the end result XOR:ed.
Reported-by: Javier Martinez Canillas jav...@dowhile0.org
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/mmc/core/host.c | 32
drivers/mmc/core/slot-gpio.c | 10 --
drivers/mmc/host/mmci.c | 5 +++--
include/linux/mmc/slot-gpio.h | 4 ++--
4 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 31969436d77c..480100515067 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -311,6 +311,7 @@ int mmc_of_parse(struct mmc_host *host)
struct device_node *np;
u32 bus_width;
int len, ret;
+ bool cap_invert, gpio_invert;
if (!host-parent || !host-parent-of_node)
return 0;
@@ -359,12 +360,15 @@ int mmc_of_parse(struct mmc_host *host)
host-caps |= MMC_CAP_NONREMOVABLE;
} else {
if (of_property_read_bool(np, cd-inverted))
- host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+ cap_invert = true;
+ else
+ cap_invert = false;
if (of_find_property(np, broken-cd, len))
host-caps |= MMC_CAP_NEEDS_POLL;
- ret = mmc_gpiod_request_cd(host, cd, 0, false, 0);
+ ret = mmc_gpiod_request_cd(host, cd, 0, false,
+ 0, gpio_invert);
if (ret) {
if (ret == -EPROBE_DEFER)
return ret;
@@ -375,13 +379,29 @@ int mmc_of_parse(struct mmc_host *host)
}
} else
dev_info(host-parent, Got CD GPIO\n);
+
+ /*
+* There are two ways to flag that the CD line is inverted:
+* through the cd-inverted flag and by the GPIO line itself
+* being inverted from the GPIO subsystem. This is a leftover
+* from the times when the GPIO subsystem did not make it
+* possible to flag a line as inverted.
+*
+* If the capability on the host AND the GPIO line are
+* both inverted, the end result is that the CD line is
+* not inverted.
+*/
+ if (cap_invert ^ gpio_invert)
+ host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
}
/* Parse Write Protection */
if (of_property_read_bool(np, wp-inverted))
- host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+ cap_invert = true;
+ else
+ cap_invert = false;
- ret = mmc_gpiod_request_ro(host, wp, 0, false, 0);
+ ret = mmc_gpiod_request_ro(host, wp, 0, false, 0, gpio_invert);
if (ret) {
if (ret == -EPROBE_DEFER)
goto out;
@@ -393,6 +413,10 @@ int mmc_of_parse(struct mmc_host *host)
} else
dev_info(host-parent, Got WP GPIO\n);
+ /* See the comment on CD inversion above */
+ if (cap_invert ^ gpio_invert)
+ host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+
if (of_find_property(np, cap-sd-highspeed, len))
host-caps |= MMC_CAP_SD_HIGHSPEED;
if (of_find_property(np, cap-mmc-highspeed, len))
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 38f76555d4bf..e069acba93f5 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -281,6 +281,7 @@ EXPORT_SYMBOL(mmc_gpio_free_cd);
* @idx: index of the GPIO to obtain in the consumer
* @override_active_level: ignore %GPIO_ACTIVE_LOW flag
* @debounce: debounce time in microseconds
+ * @gpio_invert: will return whether the GPIO line is inverted or not
*
* Use this function in place of mmc_gpio_request_cd() to use the GPIO
* descriptor API. Note that it is paired with mmc_gpiod_free_cd() not
@@ -291,7 +292,7 @@ EXPORT_SYMBOL(mmc_gpio_free_cd);
*/
int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
unsigned int idx, bool override_active_level,
-unsigned int debounce)
+unsigned int debounce, bool *gpio_invert)
{
struct mmc_gpio *ctx;
struct gpio_desc *desc;
@@ -316,6 +317,8 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char
*con_id
Re: [PATCH v2 2/2] mmc, sdhci, bcm-kona, LLVMLinux: Remove use of __initconst
On Wed, Sep 24, 2014 at 8:21 PM, Behan Webster beh...@converseincode.com wrote: On 09/24/14 02:22, Arnd Bergmann wrote: but the first one seems easier to read for someone familiar with kernel code. No worries. Happy to post a v3. Linus Walleij: Would you like me to respin the gpio, bcm-kona, LLVMLinux: Remove use of __initconst patch as well? No it's already applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] gpio, bcm-kona, LLVMLinux: Remove use of __initconst
On Wed, Sep 24, 2014 at 12:55 AM, beh...@converseincode.com wrote: From: Behan Webster beh...@converseincode.com The __initconst is in the wrong place, and when moved to the correct place it uncovers an error where the variable is used by non-init data structures. Instead merely make them const and put the const in the right spot. Signed-off-by: Behan Webster beh...@converseincode.com Reviewed-by: Mark Charlebois charl...@gmail.com Acked-by: Arnd Bergmann a...@arndb.de Acked-by: Matt Porter mpor...@linaro.org Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO
On Mon, Sep 22, 2014 at 10:20 AM, Adrian Hunter adrian.hun...@intel.com wrote: Unfortunately it doesn't seem to work. I needed the patch below. From: Adrian Hunter adrian.hun...@intel.com Date: Mon, 22 Sep 2014 11:01:16 +0300 Subject: [PATCH] gpio: Fix gpio direction flags not getting set GPIO direction flags are not getting set because an 'if' statement is the wrong way around. Signed-off-by: Adrian Hunter adrian.hun...@intel.com Oopps that's a bug, patch applied for fixes, so it'll work when this hits upstream. Alex: confirm? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors
On Wed, Aug 27, 2014 at 1:00 PM, Linus Walleij linus.wall...@linaro.org wrote: This switches the central MMC OF parser to use gpio descriptors instead of grabbing GPIOs explicitly from the device tree. This strips out an unecessary use of the integer-based GPIO API that we want to get rid of, cuts down on code as the gpio descriptor code will handle active low flags. Acked-by: Alexandre Courbot acour...@nvidia.com Signed-off-by: Linus Walleij linus.wall...@linaro.org --- ChangeLog v1-v2: - Restore error reporting as done in previous stand-alone patch. Hi Ulf, the v2 version of this patch set should be working fine on v3.17-rc4, as I merged a patch setting up the correct prototypes for systems without gpiolib. Fenguang's build tests are working on it atleast, I promise... Tell me if you want me to resend the patch set or if you can take the same patches again, but on top of -rc4. There is no change in them. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ulf.hansson-mmc:next 58/67] drivers/mmc/core/slot-gpio.c:311:2: error: too many arguments to function 'devm_gpiod_get_index'
On Mon, Sep 1, 2014 at 3:05 PM, Ulf Hansson ulf.hans...@linaro.org wrote: On 1 September 2014 14:38, kbuild test robot fengguang...@intel.com wrote: tree: git://git.linaro.org/people/ulf.hansson/mmc next head: 2969aa939a41fa8e50d3e8aa46137bfc4a35e05c commit: 84e445bbda19b801c60d7414add19c1f3ebc4d26 [58/67] mmc: slot-gpio: switch to use flags when getting GPIO config: x86_64-rhel (attached as .config) All error/warnings: drivers/mmc/core/slot-gpio.c: In function 'mmc_gpiod_request_cd': drivers/mmc/core/slot-gpio.c:311:2: error: too many arguments to function 'devm_gpiod_get_index' desc = devm_gpiod_get_index(host-parent, con_id, idx, GPIOD_IN); ^ In file included from drivers/mmc/core/slot-gpio.c:13:0: include/linux/gpio/consumer.h:166:32: note: declared here struct gpio_desc *__must_check devm_gpiod_get_index(struct device *dev, ^ vim +/devm_gpiod_get_index +311 drivers/mmc/core/slot-gpio.c 305 306 ctx = host-slot.handler_priv; 307 308 if (!con_id) 309 con_id = ctx-cd_label; 310 311 desc = devm_gpiod_get_index(host-parent, con_id, idx, GPIOD_IN); 312 if (IS_ERR(desc)) 313 return PTR_ERR(desc); 314 --- 0-DAY kernel build testing backend Open Source Technology Center http://lists.01.org/mailman/listinfo/kbuild Intel Corporation Linus, I will drop the GPIO desc patches again from my mmc tree. We need another round to sort out the decencies to GPIOLIB. Yeah this is due to the way the vararg hacks for the new flags-enabled descriptor API is done only when GPIOLIB is compiled in. Oh well, I'll funnel another fix. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO
This makes it possible to get the write protect (read only)
GPIO line from a GPIO descriptor. Written to exactly mirror
the card detect function.
Acked-by: Alexandre Courbot acour...@nvidia.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/mmc/core/slot-gpio.c | 48 +++
include/linux/mmc/slot-gpio.h | 3 +++
2 files changed, 51 insertions(+)
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 908c2b29e79f..e3fce4493fab 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -326,6 +326,54 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char
*con_id,
EXPORT_SYMBOL(mmc_gpiod_request_cd);
/**
+ * mmc_gpiod_request_ro - request a gpio descriptor for write protection
+ * @host: mmc host
+ * @con_id: function within the GPIO consumer
+ * @idx: index of the GPIO to obtain in the consumer
+ * @override_active_level: ignore %GPIO_ACTIVE_LOW flag
+ * @debounce: debounce time in microseconds
+ *
+ * Use this function in place of mmc_gpio_request_ro() to use the GPIO
+ * descriptor API. Note that it is paired with mmc_gpiod_free_ro() not
+ * mmc_gpio_free_ro().
+ *
+ * Returns zero on success, else an error.
+ */
+int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
+unsigned int idx, bool override_active_level,
+unsigned int debounce)
+{
+ struct mmc_gpio *ctx;
+ struct gpio_desc *desc;
+ int ret;
+
+ ret = mmc_gpio_alloc(host);
+ if (ret 0)
+ return ret;
+
+ ctx = host-slot.handler_priv;
+
+ if (!con_id)
+ con_id = ctx-ro_label;
+
+ desc = devm_gpiod_get_index(host-parent, con_id, idx, GPIOD_IN);
+ if (IS_ERR(desc))
+ return PTR_ERR(desc);
+
+ if (debounce) {
+ ret = gpiod_set_debounce(desc, debounce);
+ if (ret 0)
+ return ret;
+ }
+
+ ctx-override_ro_active_level = override_active_level;
+ ctx-ro_gpio = desc;
+
+ return 0;
+}
+EXPORT_SYMBOL(mmc_gpiod_request_ro);
+
+/**
* mmc_gpiod_free_cd - free the card-detection gpio descriptor
* @host: mmc host
*
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index d2433381e828..a0d0442c15bf 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -25,6 +25,9 @@ void mmc_gpio_free_cd(struct mmc_host *host);
int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
unsigned int idx, bool override_active_level,
unsigned int debounce);
+int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
+unsigned int idx, bool override_active_level,
+unsigned int debounce);
void mmc_gpiod_free_cd(struct mmc_host *host);
void mmc_gpiod_request_cd_irq(struct mmc_host *host);
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] mmc: mmci: augment driver to handle gpio descriptors
Currently the MMCI driver will only handle GPIO descriptors
implicitly through the device tree probe glue in mmc_of_init(),
but devices instatiated other ways such as through board files
and passing descriptors using the GPIO descriptor table will
not be able to exploit descriptors.
Augment the driver to look for a GPIO descriptor if device
tree is not used for the device, and if that doesn't work,
fall back to platform data GPIO assignment using the old
API. The end goal is to get rid of the platform data integer
GPIO assingments from the kernel.
This enable the MMCI-embedding platforms to be converted to
GPIO descritor tables.
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Russell King li...@arm.linux.org.uk
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/mmc/host/mmci.c | 53 +++--
1 file changed, 43 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e4d470704150..37a6542f056c 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1658,16 +1658,49 @@ static int mmci_probe(struct amba_device *dev,
writel(0, host-base + MMCIMASK1);
writel(0xfff, host-base + MMCICLEAR);
- /* If DT, cd/wp gpios must be supplied through it. */
- if (!np gpio_is_valid(plat-gpio_cd)) {
- ret = mmc_gpio_request_cd(mmc, plat-gpio_cd, 0);
- if (ret)
- goto clk_disable;
- }
- if (!np gpio_is_valid(plat-gpio_wp)) {
- ret = mmc_gpio_request_ro(mmc, plat-gpio_wp);
- if (ret)
- goto clk_disable;
+ /*
+* If:
+* - not using DT but using a descriptor table, or
+* - using a table of descriptors ALONGSIDE DT, or
+* look up these descriptors named cd and wp right here, fail
+* silently of these do not exist and proceed to try platform data
+*/
+ if (!np) {
+ ret = mmc_gpiod_request_cd(mmc, cd, 0, false, 0);
+ if (!ret)
+ dev_info(mmc_dev(mmc), got CD GPIO (table)\n);
+ else {
+ if (ret == -EPROBE_DEFER)
+ goto clk_disable;
+ else if (gpio_is_valid(plat-gpio_cd)) {
+ ret = mmc_gpio_request_cd(mmc, plat-gpio_cd,
0);
+ if (ret)
+ goto clk_disable;
+ else
+ dev_info(mmc_dev(mmc), got CD GPIO
(pdata)\n);
+ } else {
+ dev_dbg(mmc_dev(mmc),
+ no CD GPIO in DT, table or pdata\n);
+ }
+ }
+
+ ret = mmc_gpiod_request_ro(mmc, wp, 0, false, 0);
+ if (!ret)
+ dev_info(mmc_dev(mmc), got WP GPIO (table)\n);
+ else {
+ if (ret == -EPROBE_DEFER)
+ goto clk_disable;
+ else if (gpio_is_valid(plat-gpio_wp)) {
+ ret = mmc_gpio_request_ro(mmc, plat-gpio_wp);
+ if (ret)
+ goto clk_disable;
+ else
+ dev_info(mmc_dev(mmc), got WP GPIO
(pdata)\n);
+ } else {
+ dev_dbg(mmc_dev(mmc),
+ no WP GPIO in DT, table or pdata\n);
+ }
+ }
}
ret = devm_request_irq(dev-dev, dev-irq[0], mmci_irq, IRQF_SHARED,
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO
When the slot GPIO driver gets the GPIO to be used for card
detect, it is now possible to specify a flag to have the line
set up as input. Get rid of the explicit setup call for input
and use the flag.
The extra argument works as there are transition varargs
macros in place in the linux/gpio/consumer.h header, in
the future we will make the flags argument compulsory.
Reviewed-by: Alexandre Courbot acour...@nvidia.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/mmc/core/slot-gpio.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 5f89cb83d5f0..908c2b29e79f 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -308,14 +308,10 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const
char *con_id,
if (!con_id)
con_id = ctx-cd_label;
- desc = devm_gpiod_get_index(host-parent, con_id, idx);
+ desc = devm_gpiod_get_index(host-parent, con_id, idx, GPIOD_IN);
if (IS_ERR(desc))
return PTR_ERR(desc);
- ret = gpiod_direction_input(desc);
- if (ret 0)
- return ret;
-
if (debounce) {
ret = gpiod_set_debounce(desc, debounce);
if (ret 0)
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4 v2] mmc: host: switch OF parser to use gpio descriptors
This switches the central MMC OF parser to use gpio descriptors
instead of grabbing GPIOs explicitly from the device tree.
This strips out an unecessary use of the integer-based GPIO
API that we want to get rid of, cuts down on code as the
gpio descriptor code will handle active low flags.
Acked-by: Alexandre Courbot acour...@nvidia.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
ChangeLog v1-v2:
- Restore error reporting as done in previous stand-alone
patch.
---
drivers/mmc/core/host.c | 68 +
1 file changed, 23 insertions(+), 45 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 95cceae96944..6f7ed9c50346 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -310,9 +310,7 @@ int mmc_of_parse(struct mmc_host *host)
{
struct device_node *np;
u32 bus_width;
- bool explicit_inv_wp, gpio_inv_wp = false;
- enum of_gpio_flags flags;
- int len, ret, gpio;
+ int len, ret;
if (!host-parent || !host-parent-of_node)
return 0;
@@ -360,60 +358,40 @@ int mmc_of_parse(struct mmc_host *host)
if (of_find_property(np, non-removable, len)) {
host-caps |= MMC_CAP_NONREMOVABLE;
} else {
- bool explicit_inv_cd, gpio_inv_cd = false;
-
- explicit_inv_cd = of_property_read_bool(np, cd-inverted);
+ if (of_property_read_bool(np, cd-inverted))
+ host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
if (of_find_property(np, broken-cd, len))
host-caps |= MMC_CAP_NEEDS_POLL;
- gpio = of_get_named_gpio_flags(np, cd-gpios, 0, flags);
- if (gpio == -EPROBE_DEFER)
- return gpio;
- if (gpio_is_valid(gpio)) {
- if (!(flags OF_GPIO_ACTIVE_LOW))
- gpio_inv_cd = true;
-
- ret = mmc_gpio_request_cd(host, gpio, 0);
- if (ret 0) {
- dev_err(host-parent,
- Failed to request CD GPIO #%d: %d!\n,
- gpio, ret);
+ ret = mmc_gpiod_request_cd(host, cd, 0, false, 0);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
return ret;
- } else {
- dev_info(host-parent, Got CD GPIO #%d.\n,
-gpio);
+ if (ret != -ENOENT) {
+ dev_err(host-parent,
+ Failed to request CD GPIO: %d\n,
+ ret);
}
- }
-
- if (explicit_inv_cd ^ gpio_inv_cd)
- host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+ } else
+ dev_info(host-parent, Got CD GPIO\n);
}
/* Parse Write Protection */
- explicit_inv_wp = of_property_read_bool(np, wp-inverted);
-
- gpio = of_get_named_gpio_flags(np, wp-gpios, 0, flags);
- if (gpio == -EPROBE_DEFER) {
- ret = -EPROBE_DEFER;
- goto out;
- }
- if (gpio_is_valid(gpio)) {
- if (!(flags OF_GPIO_ACTIVE_LOW))
- gpio_inv_wp = true;
+ if (of_property_read_bool(np, wp-inverted))
+ host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
- ret = mmc_gpio_request_ro(host, gpio);
- if (ret 0) {
- dev_err(host-parent,
- Failed to request WP GPIO: %d!\n, ret);
+ ret = mmc_gpiod_request_ro(host, wp, 0, false, 0);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
goto out;
- } else {
- dev_info(host-parent, Got WP GPIO #%d.\n,
-gpio);
+ if (ret != -ENOENT) {
+ dev_err(host-parent,
+ Failed to request WP GPIO: %d\n,
+ ret);
}
- }
- if (explicit_inv_wp ^ gpio_inv_wp)
- host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+ } else
+ dev_info(host-parent, Got WP GPIO\n);
if (of_find_property(np, cap-sd-highspeed, len))
host-caps |= MMC_CAP_SD_HIGHSPEED;
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4 v2] mmc: mmci: augment driver to handle gpio descriptors
Currently the MMCI driver will only handle GPIO descriptors
implicitly through the device tree probe glue in mmc_of_init(),
but devices instatiated other ways such as through board files
and passing descriptors using the GPIO descriptor table will
not be able to exploit descriptors.
Augment the driver to look for a GPIO descriptor if device
tree is not used for the device, and if that doesn't work,
fall back to platform data GPIO assignment using the old
API. The end goal is to get rid of the platform data integer
GPIO assingments from the kernel.
This enable the MMCI-embedding platforms to be converted to
GPIO descritor tables.
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Russell King li...@arm.linux.org.uk
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
ChangeLog v1-v2:
- Skip excess error/info/debug report prints.
---
drivers/mmc/host/mmci.c | 39 +--
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e4d470704150..4aec22439d0e 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1658,16 +1658,35 @@ static int mmci_probe(struct amba_device *dev,
writel(0, host-base + MMCIMASK1);
writel(0xfff, host-base + MMCICLEAR);
- /* If DT, cd/wp gpios must be supplied through it. */
- if (!np gpio_is_valid(plat-gpio_cd)) {
- ret = mmc_gpio_request_cd(mmc, plat-gpio_cd, 0);
- if (ret)
- goto clk_disable;
- }
- if (!np gpio_is_valid(plat-gpio_wp)) {
- ret = mmc_gpio_request_ro(mmc, plat-gpio_wp);
- if (ret)
- goto clk_disable;
+ /*
+* If:
+* - not using DT but using a descriptor table, or
+* - using a table of descriptors ALONGSIDE DT, or
+* look up these descriptors named cd and wp right here, fail
+* silently of these do not exist and proceed to try platform data
+*/
+ if (!np) {
+ ret = mmc_gpiod_request_cd(mmc, cd, 0, false, 0);
+ if (ret 0) {
+ if (ret == -EPROBE_DEFER)
+ goto clk_disable;
+ else if (gpio_is_valid(plat-gpio_cd)) {
+ ret = mmc_gpio_request_cd(mmc, plat-gpio_cd,
0);
+ if (ret)
+ goto clk_disable;
+ }
+ }
+
+ ret = mmc_gpiod_request_ro(mmc, wp, 0, false, 0);
+ if (ret 0) {
+ if (ret == -EPROBE_DEFER)
+ goto clk_disable;
+ else if (gpio_is_valid(plat-gpio_wp)) {
+ ret = mmc_gpio_request_ro(mmc, plat-gpio_wp);
+ if (ret)
+ goto clk_disable;
+ }
+ }
}
ret = devm_request_irq(dev-dev, dev-irq[0], mmci_irq, IRQF_SHARED,
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ulf.hansson-mmc:next 17/29] drivers/input/touchscreen/cyttsp4_core.c:1218:1: warning: 'cyttsp4_irq' uses dynamic stack allocation
On Wed, Aug 20, 2014 at 10:17 AM, Ulf Hansson ulf.hans...@linaro.org wrote: Implementation wise, I would prefer to not have stubs unless it's explicitly needed. Still one may argue that a built kernel binary should have the smallest possible footprint, which means stubs should be added. Do you know if there are common understanding among maintainers of how this should be done? During the kernel summit kernel tinyfication discussion I think the point was made that Josh (who initiated the discussion) did not worry so much about the size of subsystems, as these can always be hacked if need be. He was more worried about core kernel code growing wild. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ulf.hansson-mmc:next 17/29] drivers/input/touchscreen/cyttsp4_core.c:1218:1: warning: 'cyttsp4_irq' uses dynamic stack allocation
On Tue, Aug 19, 2014 at 2:19 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 18 August 2014 19:13, Linus Walleij linus.wall...@linaro.org wrote: Or maybe even do the second approach of putting stubs in slot-gpio.h for !GPIOLIB... Hmm, that doesn't sound the proper solution either. Currently there are already stubs for the gpiod API in gpio/consumer.h. I think the original error was that the GPIOD_IN flag couldn't be found for !GPIOLIB. So, how about actually move these flags and it's corresponding defines to be available for !GPIOLIB as well. That should solve this problem for other subsytems using gpiod API as well, right? Yeah you're right, I've sent such a patch now, I will queue this for fixes so we can rely on it in the development trees. But I still think it's kind of awkward that slot-gpio is compiled into the MMC core even if GPIOLIB is not available on the machine :-/ it simply doesn't make much sense, since it never will add anything. But this path should get us to the situation where everything just compiles nicely atleast. I will try it on zeroday builds. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: select GPIOLIB for MMC support
The slot-gpio support has been compiled into the core for some time, and it basically requires GPIOLIB to be present, very few systems are using the archaic GPIO API (just defined function names) and those should be migrated to GPIOLIB anyway. The rewrite to use GPIO descriptors require that we enable GPIOLIB, so make CONFIG_MMC select CONFIG_GPIOLIB. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/mmc/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index f2eeb38efa65..bf44002793c4 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -5,6 +5,7 @@ menuconfig MMC tristate MMC/SD/SDIO card support depends on HAS_IOMEM + select GPIOLIB help This selects MultiMediaCard, Secure Digital and Secure Digital I/O support. -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 4/5] mmc: mmci: Add sdio enable mask in variant data
On Tue, Aug 12, 2014 at 2:05 PM, Srinivas Kandagatla srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds sdio enable mask in variant data, SOCs like ST have special bits in datactrl register to enable sdio. Unconditionally setting this bit in this driver breaks other SOCs like Qualcomm which maps this bits to something else, so making this enable bit to come from variant data solves the issue. Originally the issue is detected while testing WLAN ath6kl on Qualcomm APQ8064. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC 3/5] mmc: mmci: relax blksz check for SDIO
On Wed, Aug 13, 2014 at 7:14 AM, Srinivas Kandagatla srinivas.kandaga...@linaro.org wrote: In the past Ulf Hansson and Stefan Nilsson have submitted a patch to fix this issue for ux500v2. This patch adds new flag non_power_of_2_blksize / any_blksize to variant_data. That's the right approach. It makes non-2^n support the exception rather than the rule and applies only to the variants relevant so the logic is right. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] mmc: mmci: augment driver to handle gpio descriptors
Currently the MMCI driver will only handle GPIO descriptors
implicitly through the device tree probe glue in mmc_of_init(),
but devices instatiated other ways such as through board files
and passing descriptors using the GPIO descriptor table will
not be able to exploit descriptors.
Augment the driver to look for a GPIO descriptor if device
tree is not used for the device, and if that doesn't work,
fall back to platform data GPIO assignment using the old
API. The end goal is to get rid of the platform data integer
GPIO assingments from the kernel.
This enable the MMCI-embedding platforms to be converted to
GPIO descritor tables.
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Russell King li...@arm.linux.org.uk
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/mmc/host/mmci.c | 53 +++--
1 file changed, 43 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 7ad463e9741c..24a83af6d153 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -1552,16 +1552,49 @@ static int mmci_probe(struct amba_device *dev,
writel(0, host-base + MMCIMASK1);
writel(0xfff, host-base + MMCICLEAR);
- /* If DT, cd/wp gpios must be supplied through it. */
- if (!np gpio_is_valid(plat-gpio_cd)) {
- ret = mmc_gpio_request_cd(mmc, plat-gpio_cd, 0);
- if (ret)
- goto clk_disable;
- }
- if (!np gpio_is_valid(plat-gpio_wp)) {
- ret = mmc_gpio_request_ro(mmc, plat-gpio_wp);
- if (ret)
- goto clk_disable;
+ /*
+* If:
+* - not using DT but using a descriptor table, or
+* - using a table of descriptors ALONGSIDE DT, or
+* look up these descriptors named cd and wp right here, fail
+* silently of these do not exist and proceed to try platform data
+*/
+ if (!np) {
+ ret = mmc_gpiod_request_cd(mmc, cd, 0, false, 0);
+ if (!ret)
+ dev_info(mmc_dev(mmc), got CD GPIO (table)\n);
+ else {
+ if (ret == -EPROBE_DEFER)
+ goto clk_disable;
+ else if (gpio_is_valid(plat-gpio_cd)) {
+ ret = mmc_gpio_request_cd(mmc, plat-gpio_cd,
0);
+ if (ret)
+ goto clk_disable;
+ else
+ dev_info(mmc_dev(mmc), got CD GPIO
(pdata)\n);
+ } else {
+ dev_dbg(mmc_dev(mmc),
+ no CD GPIO in DT, table or pdata\n);
+ }
+ }
+
+ ret = mmc_gpiod_request_ro(mmc, wp, 0, false, 0);
+ if (!ret)
+ dev_info(mmc_dev(mmc), got WP GPIO (table)\n);
+ else {
+ if (ret == -EPROBE_DEFER)
+ goto clk_disable;
+ else if (gpio_is_valid(plat-gpio_wp)) {
+ ret = mmc_gpio_request_ro(mmc, plat-gpio_wp);
+ if (ret)
+ goto clk_disable;
+ else
+ dev_info(mmc_dev(mmc), got WP GPIO
(pdata)\n);
+ } else {
+ dev_dbg(mmc_dev(mmc),
+ no WP GPIO in DT, table or pdata\n);
+ }
+ }
}
ret = devm_request_irq(dev-dev, dev-irq[0], mmci_irq, IRQF_SHARED,
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] mmc: slot-gpio: switch to use flags when getting GPIO
When the slot GPIO driver gets the GPIO to be used for card
detect, it is now possible to specify a flag to have the line
set up as input. Get rid of the explicit setup call for input
and use the flag.
The extra argument works as there are transition varargs
macros in place in the linux/gpio/consumer.h header, in
the future we will make the flags argument compulsory.
Cc: Alexandre Courbot gnu...@gmail.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/mmc/core/slot-gpio.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 5f89cb83d5f0..908c2b29e79f 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -308,14 +308,10 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const
char *con_id,
if (!con_id)
con_id = ctx-cd_label;
- desc = devm_gpiod_get_index(host-parent, con_id, idx);
+ desc = devm_gpiod_get_index(host-parent, con_id, idx, GPIOD_IN);
if (IS_ERR(desc))
return PTR_ERR(desc);
- ret = gpiod_direction_input(desc);
- if (ret 0)
- return ret;
-
if (debounce) {
ret = gpiod_set_debounce(desc, debounce);
if (ret 0)
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] mmc: host: switch OF parser to use gpio descriptors
This switches the central MMC OF parser to use gpio descriptors
instead of grabbing GPIOs explicitly from the device tree.
This strips out an unecessary use of the integer-based GPIO
API that we want to get rid of, cuts down on code as the
gpio descriptor code will handle active low flags.
As it is in no way a bug not to supply CD/WP GPIOs the messages
about them not being specified have been depromoted from
dev_err() to dev_dbg().
Cc: Alexandre Courbot gnu...@gmail.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/mmc/core/host.c | 68 +++--
1 file changed, 21 insertions(+), 47 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 95cceae96944..048c6d687cc9 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -310,9 +310,7 @@ int mmc_of_parse(struct mmc_host *host)
{
struct device_node *np;
u32 bus_width;
- bool explicit_inv_wp, gpio_inv_wp = false;
- enum of_gpio_flags flags;
- int len, ret, gpio;
+ int len, ret;
if (!host-parent || !host-parent-of_node)
return 0;
@@ -360,60 +358,36 @@ int mmc_of_parse(struct mmc_host *host)
if (of_find_property(np, non-removable, len)) {
host-caps |= MMC_CAP_NONREMOVABLE;
} else {
- bool explicit_inv_cd, gpio_inv_cd = false;
-
- explicit_inv_cd = of_property_read_bool(np, cd-inverted);
+ if (of_property_read_bool(np, cd-inverted))
+ host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
if (of_find_property(np, broken-cd, len))
host-caps |= MMC_CAP_NEEDS_POLL;
- gpio = of_get_named_gpio_flags(np, cd-gpios, 0, flags);
- if (gpio == -EPROBE_DEFER)
- return gpio;
- if (gpio_is_valid(gpio)) {
- if (!(flags OF_GPIO_ACTIVE_LOW))
- gpio_inv_cd = true;
-
- ret = mmc_gpio_request_cd(host, gpio, 0);
- if (ret 0) {
- dev_err(host-parent,
- Failed to request CD GPIO #%d: %d!\n,
- gpio, ret);
+ ret = mmc_gpiod_request_cd(host, cd, 0, false, 0);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
return ret;
- } else {
- dev_info(host-parent, Got CD GPIO #%d.\n,
-gpio);
- }
- }
-
- if (explicit_inv_cd ^ gpio_inv_cd)
- host-caps2 |= MMC_CAP2_CD_ACTIVE_HIGH;
+ dev_dbg(host-parent,
+ Failed to request CD GPIO: %d\n,
+ ret);
+ } else
+ dev_info(host-parent, Got CD GPIO\n);
}
/* Parse Write Protection */
- explicit_inv_wp = of_property_read_bool(np, wp-inverted);
+ if (of_property_read_bool(np, wp-inverted))
+ host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
- gpio = of_get_named_gpio_flags(np, wp-gpios, 0, flags);
- if (gpio == -EPROBE_DEFER) {
- ret = -EPROBE_DEFER;
- goto out;
- }
- if (gpio_is_valid(gpio)) {
- if (!(flags OF_GPIO_ACTIVE_LOW))
- gpio_inv_wp = true;
-
- ret = mmc_gpio_request_ro(host, gpio);
- if (ret 0) {
- dev_err(host-parent,
- Failed to request WP GPIO: %d!\n, ret);
+ ret = mmc_gpiod_request_ro(host, wp, 0, false, 0);
+ if (ret) {
+ if (ret == -EPROBE_DEFER)
goto out;
- } else {
- dev_info(host-parent, Got WP GPIO #%d.\n,
-gpio);
- }
- }
- if (explicit_inv_wp ^ gpio_inv_wp)
- host-caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
+ dev_dbg(host-parent,
+ Failed to request WP GPIO: %d\n,
+ ret);
+ } else
+ dev_info(host-parent, Got WP GPIO\n);
if (of_find_property(np, cap-sd-highspeed, len))
host-caps |= MMC_CAP_SD_HIGHSPEED;
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] mmc: slot-gpio: add gpiod variant to get wp GPIO
This makes it possible to get the write protect (read only)
GPIO line from a GPIO descriptor. Written to exactly mirror
the card detect function.
Cc: Alexandre Courbot gnu...@gmail.com
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
drivers/mmc/core/slot-gpio.c | 48 +++
include/linux/mmc/slot-gpio.h | 3 +++
2 files changed, 51 insertions(+)
diff --git a/drivers/mmc/core/slot-gpio.c b/drivers/mmc/core/slot-gpio.c
index 908c2b29e79f..e3fce4493fab 100644
--- a/drivers/mmc/core/slot-gpio.c
+++ b/drivers/mmc/core/slot-gpio.c
@@ -326,6 +326,54 @@ int mmc_gpiod_request_cd(struct mmc_host *host, const char
*con_id,
EXPORT_SYMBOL(mmc_gpiod_request_cd);
/**
+ * mmc_gpiod_request_ro - request a gpio descriptor for write protection
+ * @host: mmc host
+ * @con_id: function within the GPIO consumer
+ * @idx: index of the GPIO to obtain in the consumer
+ * @override_active_level: ignore %GPIO_ACTIVE_LOW flag
+ * @debounce: debounce time in microseconds
+ *
+ * Use this function in place of mmc_gpio_request_ro() to use the GPIO
+ * descriptor API. Note that it is paired with mmc_gpiod_free_ro() not
+ * mmc_gpio_free_ro().
+ *
+ * Returns zero on success, else an error.
+ */
+int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
+unsigned int idx, bool override_active_level,
+unsigned int debounce)
+{
+ struct mmc_gpio *ctx;
+ struct gpio_desc *desc;
+ int ret;
+
+ ret = mmc_gpio_alloc(host);
+ if (ret 0)
+ return ret;
+
+ ctx = host-slot.handler_priv;
+
+ if (!con_id)
+ con_id = ctx-ro_label;
+
+ desc = devm_gpiod_get_index(host-parent, con_id, idx, GPIOD_IN);
+ if (IS_ERR(desc))
+ return PTR_ERR(desc);
+
+ if (debounce) {
+ ret = gpiod_set_debounce(desc, debounce);
+ if (ret 0)
+ return ret;
+ }
+
+ ctx-override_ro_active_level = override_active_level;
+ ctx-ro_gpio = desc;
+
+ return 0;
+}
+EXPORT_SYMBOL(mmc_gpiod_request_ro);
+
+/**
* mmc_gpiod_free_cd - free the card-detection gpio descriptor
* @host: mmc host
*
diff --git a/include/linux/mmc/slot-gpio.h b/include/linux/mmc/slot-gpio.h
index d2433381e828..a0d0442c15bf 100644
--- a/include/linux/mmc/slot-gpio.h
+++ b/include/linux/mmc/slot-gpio.h
@@ -25,6 +25,9 @@ void mmc_gpio_free_cd(struct mmc_host *host);
int mmc_gpiod_request_cd(struct mmc_host *host, const char *con_id,
unsigned int idx, bool override_active_level,
unsigned int debounce);
+int mmc_gpiod_request_ro(struct mmc_host *host, const char *con_id,
+unsigned int idx, bool override_active_level,
+unsigned int debounce);
void mmc_gpiod_free_cd(struct mmc_host *host);
void mmc_gpiod_request_cd_irq(struct mmc_host *host);
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] mmc: mmci: Add qcom dml support to the driver.
On Fri, Jul 18, 2014 at 10:53 PM, Srinivas Kandagatla srinivas.kandaga...@linaro.org wrote: On Qualcomm APQ8064 SOCs, SD card controller has an additional glue called DML (Data Mover Local/Lite) to assist dma transfers. This hardware needs to be setup before any dma transfer is requested. DML itself is not a DMA engine, its just a gule between the SD card controller and dma controller. Most of this code has been ported from qualcomm's 3.4 kernel. This patch adds the code necessary to intialize the hardware and setup before doing any dma transfers. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- Changes since v1: - Added licence text as spotted by Stephen B. - Simplified dma channel lookup as suggested by Stephen B. - Removed unnecessary parens spotted by Stephen B. - in general some cleanup done. Changes since RFC: - Moved qcom_dml.* to mmci_qcom_dml.* as suggested by Linus W. - added BAM DMA dependency in Kconfig as suggested by Linus W I really like this v2 version. Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH] mmc: mmci: Add qcom dml support to the driver.
On Fri, Jul 11, 2014 at 1:48 PM, Srinivas Kandagatla
srinivas.kandaga...@linaro.org wrote:
On Qualcomm APQ8064 SOCs, SD card controller has an additional glue
called DML (Data Mover Local/Lite) to assist dma transfers.
This hardware needs to be setup before any dma transfer is requested.
DML itself is not a DMA engine, its just a gule between the SD card
controller and dma controller.
Most of this code has been ported from qualcomm's 3.4 kernel.
This patch adds the code necessary to intialize the hardware and setup
before doing any dma transfers.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
Pretty cool :-)
drivers/mmc/host/mmci.c | 19 -
drivers/mmc/host/qcom_dml.c | 171
drivers/mmc/host/qcom_dml.h | 17 +
Please call this file mmci-qcom-dml.c/h so we know that these files
have a strong
dependence.
+config MMC_QCOM_DML
+ tristate Qualcomm Data Mover for SD Card Controller
+ depends on MMC_ARMMMCI ARCH_QCOM
The way I get it is that this only makes sense when the qualcomm
DMA engine is used so should it not be
depends on MMC_ARMMMCI QCOM_BAM_DMA
?
Or maybe we should be less specific?
depends on MMC_ARMMMCI DMA_ENGINE
?
/*
@@ -569,6 +579,7 @@ static int __mmci_dma_prep_data(struct mmci_host *host,
struct mmc_data *data,
struct dma_async_tx_descriptor *desc;
enum dma_data_direction buffer_dirn;
int nr_sg;
+ u32 flags = DMA_CTRL_ACK;
if (data-flags MMC_DATA_READ) {
conf.direction = DMA_DEV_TO_MEM;
@@ -593,9 +604,12 @@ static int __mmci_dma_prep_data(struct mmci_host *host,
struct mmc_data *data,
if (nr_sg == 0)
return -EINVAL;
+ if (host-variant-qcom_dml)
+ flags |= DMA_PREP_INTERRUPT;
Is this correct? That augments every incoming DMA request to try to
do an interrupt callback. Why is that necessary? The interrupt
callback still isn't registered, so this isn't used for anything?
If it's still needed it would be a bug in the DMA engine I think.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: core: Add DT bindings for card detect debounce time
On Sat, Jun 21, 2014 at 9:22 AM, Alexandre Courbot gnu...@gmail.com wrote: I have added Linus Walleij and Alexandre Courbot, the maintainers of gpio. Let's see if they can point us in a direction. I agree it would be nice if the debounce value could be handled by the GPIO framework. I agree too. I just wonder what would be the correct way of doing it? Contrary to ACTIVE_LOW and other flags which can be specified with the GPIO phandle, debounce is a numeric value. Maybe using a different property, e.g.: cd-gpios = gpio 1 GPIO_ACTIVE_HIGH; cd-gpios-debounce = 10; When looking up a GPIO through gpiod_get(), the GPIO framework could then check for -debounce property and set the debounce time accordingly. At first sight I'd say that would work and could be used for MMC and all other subsystems that need something similar. Yes, but we also need to write generic debounce handling into the gpiolib, so it doesn't matter if the GPIO driver can or cannot handle this itself. Some hardware has the capability to debounce lines in *hardware*. Right now a call to gpiod_set_debounce() will fail unless the hardware has a debounce option. What we should really do is make gpiod_set_debounce() always work, put the debounce value into the gpio_desc, and move some logic similar to what exists in drivers/input/keyboard/gpio_keys.c into the gpiolib, then get rid of any local implementations like in gpio_keys. Then we can rely on the gpiolib handling this at all times, and also to get the debounce config from DT. Dmitry, opinions on this? Anyone up for implementing the timer in gpiolib and moving the keyboard driver over to using the gpiolib debounce set? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 02/12] mmc: mmci: Add Qualcomm specific register defines.
On Mon, Jun 2, 2014 at 11:08 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds a Qualcomm SD Card controller specific register variations to header file. Qualcomm SDCC controller is pl180, with slight changes in the register layout from standard pl180 register set. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 03/12] mmc: mmci: Add enough delay between writes to CMD register.
On Mon, Jun 2, 2014 at 11:08 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org On Qcom SD Card controller POWER, CLKCTRL, DATACTRL and COMMAND registers should be updated in MCLK domain, and writes to these registers must be separated by three MCLK cycles. This resitriction is not applicable for other registers. Any subsequent writes to these register will be ignored until 3 MCLK have passed. One usec delay between two CMD register writes is not sufficient in the card identification phase where the CCLK is very low. This patch replaces a static 1 usec delay to use mmci_reg_delay function which can provide correct delay depending on the cclk frequency. Without this patch the card is not detected. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 09/12] mmc: mmci: add f_max to variant structure
On Mon, Jun 2, 2014 at 11:09 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org Some of the controller have maximum supported frequency, This patch adds support in variant data structure to specify such restrictions. This gives more flexibility in calculating the f_max before passing it to mmc-core. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 10/12] mmc: mmci: add explicit clk control
On Mon, Jun 2, 2014 at 11:09 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org On Controllers like Qcom SD card controller where cclk is mclk and mclk should be directly controlled by the driver. This patch adds support to control mclk directly in the driver, and also adds explicit_mclk_control flag in variant structure giving more flexibility to the driver. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 11/12] mmc: mmci: Add Qcom specific rx_fifocnt logic.
On Mon, Jun 2, 2014 at 11:10 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org MCIFIFOCNT register behaviour on Qcom chips is very different than the other pl180 integrations. MCIFIFOCNT register contains the number of words that are still waiting to be transferred through the FIFO. It keeps decrementing once the host CPU reads the MCIFIFO. With the existing logic and the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT register will always return transfer size before reading the FIFO. Also the data sheet states that This register is only useful for debug purposes and should not be used for normal operation since it does not reflect data which may or may not be in the pipeline. This patch implements a qcom specific get_rx_fifocnt function which is implemented based on status register flags. Based on qcom_fifo flag in variant data structure, the corresponding get_rx_fifocnt function is selected. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 12/12] mmc: mmci: Add Qualcomm Id to amba id table
On Mon, Jun 2, 2014 at 11:10 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds a fake Qualcomm ID 0x00051180 to the amba_ids, as Qualcomm SDCC controller is pl180, but amba id registers read 0x0's. The plan is to remove SDCC driver totally and use mmci as the main SD controller driver for Qualcomm SOCs. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/13] mmc: mmci: Add Qcom specific pio_read function.
On Fri, May 30, 2014 at 3:30 AM, Stephen Boyd sb...@codeaurora.org wrote: Hm... that doesn't sound right. Please see this thread on lkml[1], and also this video from Ben H.[2] But Benji is talking about the *PCI BUS*, and this is raw register access. He just says that This is how a BE CPU is normally wired to a LE bus we have no idea whether that is actually the case on any existing system with this peripheral. Such things have to be tested. My understanding is that this is a fifo so I would think we want to use the ioread32_rep() function here (or other string io functionality). Otherwise we're going to byteswap the data that we're trying to interpret as a buffer and big endian CPUs won't work. You don't know that until you have tested on real BE ARM hardware with this PrimeCell. How do you know the SoC designers will have been smart enough to insert byte swap logic? You are right. readsl() or ioread32_rep() isn't used for this reason. We could always use a size of 1 right? What do you mean? Do you mean using: ioread32_rep(io, buf, 1)? Notice this is totally equivalent to readsl() in arm/include/asm/io.h: #define readsl(p,d,l) __raw_readsl(p,d,l) #define ioread32_rep(p,d,c) __raw_readsl(p,d,c) The semantic effect is switching from readl() - readsl() or conversely ioread32 - ioread32_rep() which will use __raw accessors in native endianness rather than LE access. Which is what you want if the bus will do byte swapping. Which we don't know if it does, because noone has such a system and can verify. If the native bus is not byte swapping this will just force you to use cpu_to_le32() on the result. That is not helpful at all. We need to experiment with real BE hardware with this PrimeCell to determine whether to do this, and until that is done just don't try to outsmart the hardware by thinking ahead and designing for the unknown, that will just fail. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 08/13] mmc: mmci: add 8bit bus support in variant data
On Wed, May 28, 2014 at 9:27 AM, Srinivas Kandagatla
srinivas.kandaga...@linaro.org wrote:
Hi Linus W,
On 26/05/14 11:07, Ulf Hansson wrote:
unsigned intfifosize;
unsigned intfifohalfsize;
@@ -116,6 +118,7 @@ static struct variant_data variant_u300 = {
.fifosize = 16 * 4,
.fifohalfsize = 8 * 4,
.clkreg_enable = MCI_ST_U300_HWFCEN,
+ .clkreg_8bit_bus_enable = MCI_ST_8BIT_BUS,
Linus, will have to confirm this. I don't know if the u300 variant
support 8-bit.
Do you know if u300 supports 8BIT bus?
Yes it does actually.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/13] mmc: mmci: Add Qcom specific pio_read function.
On Fri, May 23, 2014 at 2:53 PM, srinivas.kandaga...@linaro.org wrote:
+ if (unlikely(bytes)) {
+ unsigned char buf[4];
(...)
Please think twice about this.
http://lwn.net/Articles/70473/
http://lwn.net/Articles/420019/
http://lwn.net/Articles/182369/
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/11] pinctrl: sunxi: Move setting of mux to irq type from unmask to set_type
On Mon, May 26, 2014 at 9:47 AM, Hans de Goede hdego...@redhat.com wrote: With level triggered interrupt mask / unmask will get called for each interrupt, doing the somewhat expensive mux setting on each unmask thus is not a good idea. Instead move it to the set_type callback, which is typically done only once for each irq. Signed-off-by: Hans de Goede hdego...@redhat.com Yes move it out of mask/unmask but no, not into set_type(). Can you not use the irqchip startup()/shutdown() callbacks instead? And how come we have no clean separation between gpiochip and the pinmux parts here, why cant we just call pinctrl_request_gpio() and pinctrl_free_gpio() here instead? Or maybe pinctrl_gpio_direction_input() and have that set up the muxing in the pinctrl driver side? It looks slightly convoluted. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 05/14] mmc: mmci: Add register read/write wrappers.
On Thu, May 15, 2014 at 11:36 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds wrappers for readl/writel functions used in the driver. The reason for this wrappers is to accommodate SOCs like Qualcomm which has requirement for delaying the write for few cycles when writing to its SD Card Controller registers. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 08/14] mmc: mmci: add 8bit bus support in variant data
On Thu, May 15, 2014 at 11:37 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds 8bit bus enable to variant structure giving more flexibility to the driver to support more SOCs which have different clock register layout. Without this patch other new SOCs like Qcom will have to add more code to special case them. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 07/14] mmc: mmci: add ddrmode mask to variant data
On Thu, May 15, 2014 at 11:37 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds ddrmode mask to variant structure giving more flexibility to the driver to support more SOCs which have different datactrl register layout. Without this patch datactrl register is updated with wrong ddrmode mask on non ST SOCs, resulting in card detection failures. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 09/14] mmc: mmci: add edge support to data and command out in variant data.
On Thu, May 15, 2014 at 11:37 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds edge support for data and command out to variant structure giving more flexibility to the driver to support more SOCs which have different clock register layout. Without this patch other new SOCs like Qcom will have to add more code to special case them Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/14] mmc: mmci: add Qcom specifics of clk and datactrl registers.
On Thu, May 15, 2014 at 11:37 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds specifics of clk and datactrl register on Qualcomm SD Card controller. This patch also populates the Qcom variant data with these new values specific to Qualcomm SD Card Controller. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Now it looks real elegant. Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 12/14] mmc: mmci: add support for fbclk to latch data and cmd.
On Thu, May 15, 2014 at 11:37 AM, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla srinivas.kandaga...@linaro.org
This patch adds support to fbclk that is used to latch data and
cmd on some controllers like SD Card controller in Qcom SOC.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
(...)
Isn't this overkill?
@@ -189,6 +192,7 @@ static struct variant_data variant_qcom = {
.clkreg = MCI_CLK_ENABLE,
-.clkreg_enable = MCI_QCOM_CLK_FLOWENA,
+ .clkreg_enable = MCI_QCOM_CLK_FLOWENA |
MCI_QCOM_CLK_FEEDBACK_CLK,
.clkreg_8bit_bus_enable = MCI_QCOM_CLK_WIDEBUS_8,
Isn't this achieveing exactly the same thing without the extra fields?
You unconditionally do it at every enable anyway, don't you?
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 11/14] mmc: mmci: Add support to data commands via variant structure.
On Thu, May 15, 2014 at 11:37 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org On some SOCs like Qcom there are explicit bits in the command register to specify if its a data transfer command or not. So this patch adds support to such bits in variant data, giving more flexibility to the driver. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 14/14] mmc: mmci: Add Qcom specific pio_read function.
On Thu, May 15, 2014 at 11:38 AM, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla srinivas.kandaga...@linaro.org
MCIFIFOCNT register behaviour on Qcom chips is very different than the other
pl180 integrations. MCIFIFOCNT register contains the number of
words that are still waiting to be transferred through the FIFO. It keeps
decrementing once the host CPU reads the MCIFIFO. With the existing logic and
the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT
register will always return transfer size before reading the FIFO.
Also the data sheet states that This register is only useful for debug
purposes and should not be used for normal operation since it does not reflect
data which may or may not be in the pipeline.
This patch implements qcom_pio_read function so as existing mmci_pio_read is
not suitable for Qcom SOCs. qcom_pio_read function is only selected
based on qcom_fifo flag in variant data structure.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
(...)
+static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
+unsigned int remain)
+{
+ uint32_t*ptr = (uint32_t *) buffer;
Just use u32 like the rest of the driver does.
+ int fifo_size = variant-fifosize;
+
+ if (remain % 4)
+ remain = ((remain 2) + 1) 2;
Insert a comment here so we know what is happening.
It seems you are rounding up to the nearest divisible
by four reads. It looks complicated. Does this accomplish
the same thing?
if (remain % 4) {
remain = ~0x03;
remain += 4;
}
+ while (readl(host-base + MMCISTATUS) MCI_RXDATAAVLBL) {
+ *ptr = readl(host-base + MMCIFIFO + (count % fifo_size));
+ ptr++;
+ count += sizeof(uint32_t);
+
+ remain -= sizeof(uint32_t);
I already commented that this looks weird. It is a well known fact
that a u32 is 4 bytes.
count += 4;
remain -= 4;
works just fine!
+ if (remain == 0)
+ break;
+ }
+ return count;
+}
And another variant is to count the *number or words*
to read from the FIFO rather than the number of bytes! I would do it
like this:
static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
unsigned int remain)
{
u32 *ptr = (u32*) buffer;
unsigned int count = 0;
unsigned int words;
unsigned int fifo_size = host-variant-fifosize;
words = DIV_ROUND_UP(remain, 4);
while (readl(host-base + MMCISTATUS) MCI_RXDATAAVLBL) {
*ptr = readl(host-base + MMCIFIFO + (count % fifo_size));
ptr++;
count += 4;
remain--;
if (!remain)
break;
}
return count;
}
I guess you will run into additional problems when you come to doing
SDIO. This function can return *more* bytes than asked for, as it rounds
up. It won't happen with MMC/SD transfers since these are always
divisible by 8, but it *will* happen on SDIO!
If you look carefully at the comments in mmci_pio_read() you will
see how this is handled. Are you sure this function cannot be
augmented to handle the qcom variant as well so you don't get this
problem further down the road?
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.
On Wed, May 14, 2014 at 12:13 AM, Stephen Boyd sb...@codeaurora.org wrote: Please add a note that this id is fake in the commit text or in the code as well. You just need one of your hardware engineers to add is to one instance of some test chip round so it exists in any hardware whatsoever and it is not fake anymore... This is a bit like device tree, it's defined by the kernel community as much as by the hardware engineers and it's a little bit of back-and-forth. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 01/11] ARM: amba: Add Qualcomm vendor ID.
On Tue, Apr 29, 2014 at 10:19 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds Qualcomm amba vendor Id to the list. This ID is used in mmci driver. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org (...) + AMBA_VENDOR_QCOM = 0x51, Yeah it's a Q, like 0x41 is A for ARM. You could as well mention this in the commit message. And you can probably just put this patch into Russell's patch tracker already. Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 02/11] mmc: mmci: Add Qualcomm Id to amba id table
On Tue, Apr 29, 2014 at 10:19 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch adds a fake Qualcomm ID 0x00051180 to the amba_ids, as Qualcomm SDCC controller is pl180, but amba id registers read 0x0's. The plan is to remove SDCC driver totally and use mmci as the main SD controller driver for Qualcomm SOCs. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 03/11] mmc: mmci: Add Qcom datactrl register variant
On Tue, Apr 29, 2014 at 10:19 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org Instance of this IP on Qualcomm's SOCs has bit different layout for datactrl register. Bit postion datactrl[16:4] hold the true block size instead of power of 2. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org This is probably something I got wrong when I attempted to use this driver on the Dragon board. Now I see what I missed... Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 05/11] mmc: mmci: use NSEC_PER_SEC macro
On Tue, Apr 29, 2014 at 10:20 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org This patch replaces a constant used in calculating timeout with a proper macro. This is make code more readable. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org This can be merged out-of-order just as-is. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 06/11] mmc: mmci: Qcomm: Add 3 clock cycle delay after register write
On Tue, Apr 29, 2014 at 10:20 AM, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla srinivas.kandaga...@linaro.org
Most of the Qcomm SD card controller registers must be updated to the MCLK
domain so subsequent writes to registers will be ignored until 3 clock cycles
have passed.
This patch adds a 3 clock cycle delay required after writing to controller
registers on Qualcomm SOCs. Without this delay all the register writes are not
successfull, resulting in not detecting cards.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
Sounds like someone decided to clock the internal state machine
in the MMCI using MCLK instead of PCLK :-(
A bit nasty if this ends up in the fastpath (irq) though. Which it
invariably does, right?
+ /*
+* On QCom SD card controller, registers must be updated to the
+* MCLK domain so subsequent writes to this register will be ignored
+* for 3 clk cycles.
+*/
+ if (host-hw_designer == AMBA_VENDOR_QCOM)
+ udelay(1 + ((3 * USEC_PER_SEC)/host-mclk));
Add a new field in vendor data instead, and use DIV_ROUND_UP():
static struct variant_data variant_qcom = {
.mclk_delayed_writes = true,
(...)
if (host-vendor-mclk_delayed_writes)
udelay(DIV_ROUND_UP((3 * USEC_PER_SEC), host-mclk));
You get the idea.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 07/11] mmc: mmci: move ST specific register extensions access under condition.
On Tue, Apr 29, 2014 at 10:20 AM, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla srinivas.kandaga...@linaro.org
This patch moves some of the ST specific register extensions access under
condition, so that other SOCs like Qualcomm or ARM would not a side effect of
writing to those reserved/different purpose bits.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
(...)
/* Keep ST Micro busy mode if enabled */
- datactrl |= host-datactrl_reg MCI_ST_DPSM_BUSYMODE;
+ if (host-hw_designer == AMBA_VENDOR_ST)
+ datactrl |= host-datactrl_reg MCI_ST_DPSM_BUSYMODE;
Do not hard-check the hw_designer everywhere, follow the pattern
if storing special stuff in the variant data.
struct variant_data {
u32 datactrl_mask_busymode;
(...)
static struct variant_data variant_u300 = {
.datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_nomadik = {
.datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_ux500 = {
.datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
static struct variant_data variant_ux500v2 = {
.datactrl_mask_busymode = MCI_ST_DPSM_BUSYMODE,
(...)
Then end up like this:
/* Keep ST Micro busy mode if enabled */
- datactrl |= host-datactrl_reg MCI_ST_DPSM_BUSYMODE;
+ datactrl |= host-datactrl_reg host-vendor-datactrl_mask_busymode;
OK we know this should have been done like this from the beginning
but that is not an excuse not to fix it up now.
- if (host-mmc-ios.timing == MMC_TIMING_UHS_DDR50)
+ if (host-mmc-ios.timing == MMC_TIMING_UHS_DDR50
+ host-hw_designer == AMBA_VENDOR_ST)
datactrl |= MCI_ST_DPSM_DDRMODE;
Same pattern here.
Actually I think this is only available on Ux500v2 (Ulf? Can you verify
this) so it is probably plain wrong to do it for other variants.
struct variant_data {
u32 datactrl_ddrmode;
(...)
static struct variant_data variant_ux500v2 = {
.datactrl_ddrmode = MCI_ST_DPSM_DDRMODE,
(...)
Then end up like this:
if (host-mmc-ios.timing == MMC_TIMING_UHS_DDR50)
-datactrl |= MCI_ST_DPSM_DDRMODE;
+ datactrl |= host-vendor-datactrl_ddrmode;
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 08/11] mmc: mmci: Qcom fix MCICLK register settings.
On Tue, Apr 29, 2014 at 10:20 AM, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla srinivas.kandaga...@linaro.org
MCICLK register layout is bit different to the standard pl180 register layout.
Qcom SDCC controller some setup in MCICLK register to get it going. So this
patch adds new setup and makes it specific to Qcom hw designer.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
(...)
- if (host-mmc-ios.bus_width == MMC_BUS_WIDTH_4)
- clk |= MCI_4BIT_BUS;
- if (host-mmc-ios.bus_width == MMC_BUS_WIDTH_8)
- clk |= MCI_ST_8BIT_BUS;
+ if (host-hw_designer == AMBA_VENDOR_QCOM) {
+ clk |= MCI_CLK_QCOM_FLOWENA;
+ clk |= (MCI_CLK_QCOM_SEL_FEEDBACK_CLK
+ MCI_CLK_QCOM_SEL_IN_SHIFT); /* feedback clk */
+ if (host-mmc-ios.bus_width == MMC_BUS_WIDTH_8)
+ clk |= MCI_CLK_QCOM_WIDEBUS_8;
+ else if (host-mmc-ios.bus_width == MMC_BUS_WIDTH_4)
+ clk |= MCI_CLK_QCOM_WIDEBUS_4;
+ else
+ clk |= MCI_CLK_QCOM_WIDEBUS_1;
+
+ if (host-mmc-ios.timing == MMC_TIMING_UHS_DDR50) {
+ /* clear SELECT_IN field */
+ clk = ~(MCI_CLK_QCOM_SEL_MASK
+ MCI_CLK_QCOM_SEL_IN_SHIFT);
+ /* set DDR timing mode */
+ clk |= (MCI_CLK_QCOM_SEL_DDR_MODE
+ MCI_CLK_QCOM_SEL_IN_SHIFT);
+ }
+ clk |= (MCI_CLK_SDC4_MCLK_SEL_MCLK
+ MCI_CLK_SDC4_MCLK_SEL_SHIFT);
- if (host-mmc-ios.timing == MMC_TIMING_UHS_DDR50)
- clk |= MCI_ST_UX500_NEG_EDGE;
+ } else {
+ if (host-mmc-ios.bus_width == MMC_BUS_WIDTH_4)
+ clk |= MCI_4BIT_BUS;
+ if (host-mmc-ios.bus_width == MMC_BUS_WIDTH_8)
+ clk |= MCI_ST_8BIT_BUS;
+
+ if (host-mmc-ios.timing == MMC_TIMING_UHS_DDR50)
+ clk |= MCI_ST_UX500_NEG_EDGE;
+ }
Again follow the pattern of storing register templates in the vendor_data
struct. I think you will quickly realize how this can be cut down with
new fields like .clk_4bitmode etc.
/* Modified PL180 on Versatile Express platform */
#define MCI_ARM_HWFCEN (1 12)
+/* Modified on Qualcomm Integrations */
First: follow the convention set for the ST-specific registers that
look e.g. like this:
MCI_ST_8BIT_BUS i.e. MCI_vendor_SPECIFIER
So the below becomes MCI_QCOM_CLK_WIDEBUS... etc.
+#define MCI_CLK_QCOM_WIDEBUS_1 (0 10)
+#define MCI_CLK_QCOM_WIDEBUS_4 (2 10)
+#define MCI_CLK_QCOM_WIDEBUS_8 (3 10)
Compare to what we have:
#define MMCICLOCK 0x004
#define MCI_CLK_ENABLE (1 8)
#define MCI_CLK_PWRSAVE (1 9)
#define MCI_CLK_BYPASS (1 10)
#define MCI_4BIT_BUS(1 11)
MCI_CLK_QCOM_WIDEBUS_1 is clearly surplus since it is
not setting any bit.
MCI_CLK_QCOM_WIDEBUS_4 (2 10) ==
MCI_4BIT_BUS (1 11), this is the same thing just
expressed in two ways! No need to have some surplus
definition adding to the confusion. Just use MCI_4BIT_BUS
The only thing that is really different is
MCI_CLK_QCOM_WIDEBUS_8 which whould have
the name (following the sibling definitions for the ST block):
#define MCI_QCOM_8BIT_BUS (3 10)
+#define MCI_CLK_QCOM_FLOWENA (1 12)
Rename:
MCI_QCOM_CLK_HWFCEN (113)
to match sibling definitions. It's clear that this enabled
hardware flow control and that name is more helpful to
understand what is going on.
+#define MCI_CLK_QCOM_INVERTOUT (1 13)
Rename:
MCI_QCOM_CLK_INV
To match siblings.
+/* select in latch data and command */
+#define MCI_CLK_QCOM_SEL_IN_SHIFT (14)
+#define MCI_CLK_QCOM_SEL_MASK (0x3)
+#define MCI_CLK_QCOM_SEL_RISING_EDGE (1)
+#define MCI_CLK_QCOM_SEL_FEEDBACK_CLK (2)
+#define MCI_CLK_QCOM_SEL_DDR_MODE (3)
+
+/* mclk selection */
+#define MCI_CLK_SDC4_MCLK_SEL_SHIFT(23)
+#define MCI_CLK_SDC4_MCLK_SEL_MASK (0x3)
+#define MCI_CLK_SDC4_MCLK_SEL_FB_CLK (1)
+#define MCI_CLK_SDC4_MCLK_SEL_MCLK (2)
These seem to have no related siblings but should still be
renamed MCL_QCOM_*
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 09/11] mmc: mmci: Add clock support for Qualcomm.
On Tue, Apr 29, 2014 at 10:20 AM, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla srinivas.kandaga...@linaro.org
MCICLK going to card bus is directly driven by the clock controller, so the
driver has to set the required rates depending on the state of the card. This
bit of support is very much similar to bypass mode but there is no such thing
called bypass mode in MCICLK register of Qcom SD card controller. By default
the clock is directly driven by the clk controller.
This patch adds clock support for Qualcomm SDCC in the driver. This bit of
code is conditioned on hw designer.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
(...)
+ if (host-hw_designer == AMBA_VENDOR_QCOM) {
+ host-cclk = host-mclk;
+ } else if (desired = host-mclk) {
Again refrain from hard-checking the vendor everywhere.
struct variant_data {
boolqcom_cclk_is_mclk;
(...)
As per example from st_clkdiv...
Then
if (host-vendor-qcom_cclk_is_mclk) {
(...)
}
+ if (ios-clock != host-mclk
+ host-hw_designer == AMBA_VENDOR_QCOM) {
+ /* Qcom MCLKCLK register does not define bypass bits */
+ int rc = clk_set_rate(host-clk, ios-clock);
+ if (rc 0) {
+ dev_err(mmc_dev(host-mmc),
+ Error setting clock rate (%d)\n, rc);
+ } else {
+ host-mclk = clk_get_rate(host-clk);
+ host-cclk = host-mclk;
+ }
+ }
For this I would define a vendor data like:
struct variant_data {
boolexplicit_mclk_control;
(...)
Or something. It explains what is actually going on.
if (plat-f_max)
- mmc-f_max = min(host-mclk, plat-f_max);
+ mmc-f_max = (host-hw_designer == AMBA_VENDOR_QCOM) ?
+ plat-f_max : min(host-mclk, plat-f_max);
So rewrite like that:
if (host-vendor-explicit_mclk_control)
mmc-f_max = plat-f_max;
else
mmc-f_max = min(host-mclk, plat-f_max);
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 10/11] mmc: mmci: Add Qcom variations to MCICommand register.
On Tue, Apr 29, 2014 at 10:21 AM, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla srinivas.kandaga...@linaro.org Some bits which control Command Path State Machine (CPSM) are new in Qcom integration, so this patch adds support to those bits. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org (...) + if (host-hw_designer == AMBA_VENDOR_QCOM + mmc_cmd_type(cmd) == MMC_CMD_ADTC) + c |= MCI_CSPM_QCOM_DATCMD; + You know the drill. Use vendor data. +/* Modified on Qualcomm Integrations */ +#define MCI_CSPM_QCOM_DATCMD (1 12) +#define MCI_CSPM_QCOM_MCIABORT (1 13) +#define MCI_CSPM_QCOM_CCSENABLE(1 14) +#define MCI_CSPM_QCOM_CCSDISABLE (1 15) +#define MCI_CSPM_QCOM_AUTO_CMD19 (1 16) +#define MCI_CSPM_QCOM_AUTO_CMD21 (1 21) You know how to rename these defines too :-) Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 11/11] mmc: mmci: Add Qcom specific pio_read function.
On Tue, Apr 29, 2014 at 10:21 AM, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla srinivas.kandaga...@linaro.org
MCIFIFOCNT register behaviour on Qcom chips is very different than the other
pl180 integrations. MCIFIFOCNT register contains the number of
words that are still waiting to be transferred through the FIFO. It keeps
decrementing once the host CPU reads the MCIFIFO. With the existing logic and
the MCIFIFOCNT behaviour, mmci_pio_read will loop forever, as the FIFOCNT
register will always return transfer size before reading the FIFO.
Also the data sheet states that This register is only useful for debug
purposes and should not be used for normal operation since it does not reflect
data which may or may not be in the pipeline.
This patch implements qcom_pio_read function so as existing mmci_pio_read is
not suitable for Qcom SOCs.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
(...)
+static int mmci_qcom_pio_read(struct mmci_host *host, char *buffer,
+unsigned int remain)
+{
+ uint32_t*ptr = (uint32_t *) buffer;
Just use u32 for this.
+ int count = 0;
+ struct variant_data *variant = host-variant;
+ int fifo_size = variant-fifosize;
+
+ if (remain % 4)
+ remain = ((remain 2) + 1) 2;
Explain in a comment exactly what is happening here or noone will
understand the code.
+ while (readl(host-base + MMCISTATUS) MCI_RXDATAAVLBL) {
+ *ptr = readl(host-base + MMCIFIFO + (count % fifo_size));
+ ptr++;
+ count += sizeof(uint32_t);
+ remain -= sizeof(uint32_t);
sizeof(u32) or just 4 works for these...
count += 4;
remain -= 4;
Is easier to parse and understand I think.
+ if (remain == 0)
+ break;
if (!remain)
break;
+ }
+ return count;
+}
- if (status MCI_RXACTIVE)
- len = mmci_pio_read(host, buffer, remain);
+ if (status MCI_RXACTIVE) {
+ if (host-hw_designer == AMBA_VENDOR_QCOM)
+ len = mmci_qcom_pio_read(host, buffer,
remain);
+ else
+ len = mmci_pio_read(host, buffer, remain);
+ }
Use something like bool qcom_fifo; in vendor data instead.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: mmc: slot-gpio
On Sat, May 3, 2014 at 8:54 PM, Alexander Shiyan shc_w...@mail.ru wrote: Is this patch (http://www.spinics.net/lists/linux-mmc/msg25410.html) looks like a rip off my idea: http://permalink.gmane.org/gmane.linux.kernel.mmc/25506 I have no complaints but it is very strange that the same idea came immediately after me. Sometimes good ideas are just in the air, like with Edison and Sawyers fight over the lightbulb... Can't you folks just add the other's Signed-off-by to either patch. I guess Chris or Ulf will decide which one to take. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] mmc: mmci: switch the driver to using gpio descriptors
On Wed, Apr 16, 2014 at 9:31 AM, Ulf Hansson ulf.hans...@linaro.org wrote: On 15 April 2014 10:33, Linus Walleij linus.wall...@linaro.org wrote: The next step in modernization of GPIO is to let drivers handle descriptors rather than integer numbers representing GPIO pins, akin to how clocks or regulators are already handled today. This patch makes the MMCI driver use GPIO descriptos in the core code with fallback code using the platform data if that is not possible. After all platforms with MMCI have been migrated to use descriptors, the platform data entries for GPIO pins can be removed. Cc: Alexandre Courbot gnu...@gmail.com Cc: Ulf Hansson ulf.hans...@linaro.org Cc: Russell King li...@arm.linux.org.uk Signed-off-by: Linus Walleij linus.wall...@linaro.org --- Hi Russell,Ulf: there is no hurry to do these changes (only used for my very corner-case MMCI PL181 experiments) but it's the desired direction to use descriptors for GPIOs going forward. I can rebase this on top of Ulf's patch stack any time, no problem. If you rebase it on top of my patch stack - I can resend the pull request I sent a few days ago and include this one!? There is no hurry. I'd let Russell think about your recent patches first and I will keep this patch on my watchlist until things settle down. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: mmci: switch the driver to using gpio descriptors
The next step in modernization of GPIO is to let drivers handle
descriptors rather than integer numbers representing GPIO pins,
akin to how clocks or regulators are already handled today.
This patch makes the MMCI driver use GPIO descriptos in the
core code with fallback code using the platform data if that
is not possible. After all platforms with MMCI have been
migrated to use descriptors, the platform data entries for
GPIO pins can be removed.
Cc: Alexandre Courbot gnu...@gmail.com
Cc: Ulf Hansson ulf.hans...@linaro.org
Cc: Russell King li...@arm.linux.org.uk
Signed-off-by: Linus Walleij linus.wall...@linaro.org
---
Hi Russell,Ulf: there is no hurry to do these changes (only used
for my very corner-case MMCI PL181 experiments) but it's the
desired direction to use descriptors for GPIOs going forward.
I can rebase this on top of Ulf's patch stack any time, no
problem.
---
drivers/mmc/host/mmci.c | 76 -
drivers/mmc/host/mmci.h | 4 +--
2 files changed, 40 insertions(+), 40 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 771c60ab4a32..f84e39a5d592 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -26,7 +26,7 @@
#include linux/amba/bus.h
#include linux/clk.h
#include linux/scatterlist.h
-#include linux/gpio.h
+#include linux/gpio/consumer.h
#include linux/of_gpio.h
#include linux/regulator/consumer.h
#include linux/dmaengine.h
@@ -1330,10 +1330,10 @@ static int mmci_get_ro(struct mmc_host *mmc)
{
struct mmci_host *host = mmc_priv(mmc);
- if (host-gpio_wp == -ENOSYS)
+ if (IS_ERR(host-gpio_wp))
return -ENOSYS;
- return gpio_get_value_cansleep(host-gpio_wp);
+ return gpiod_get_value_cansleep(host-gpio_wp);
}
static int mmci_get_cd(struct mmc_host *mmc)
@@ -1342,13 +1342,13 @@ static int mmci_get_cd(struct mmc_host *mmc)
struct mmci_platform_data *plat = host-plat;
unsigned int status;
- if (host-gpio_cd == -ENOSYS) {
+ if (IS_ERR(host-gpio_cd)) {
if (!plat-status)
return 1; /* Assume always present */
status = plat-status(mmc_dev(host-mmc));
} else
- status = !!gpio_get_value_cansleep(host-gpio_cd)
+ status = !!gpiod_get_value_cansleep(host-gpio_cd)
^ plat-cd_invert;
/*
@@ -1412,13 +1412,10 @@ static struct mmc_host_ops mmci_ops = {
#ifdef CONFIG_OF
static void mmci_dt_populate_generic_pdata(struct device_node *np,
- struct mmci_platform_data *pdata)
+ struct mmci_platform_data *pdata)
{
int bus_width = 0;
- pdata-gpio_wp = of_get_named_gpio(np, wp-gpios, 0);
- pdata-gpio_cd = of_get_named_gpio(np, cd-gpios, 0);
-
if (of_get_property(np, cd-inverted, NULL))
pdata-cd_invert = true;
else
@@ -1494,9 +1491,20 @@ static int mmci_probe(struct amba_device *dev,
host = mmc_priv(mmc);
host-mmc = mmc;
- host-gpio_wp = -ENOSYS;
- host-gpio_cd = -ENOSYS;
+ host-gpio_wp = ERR_PTR(-ENOSYS);
+ host-gpio_cd = ERR_PTR(-ENOSYS);
host-gpio_cd_irq = -1;
+ /*
+* TODO: when we finally get rid of all platform data for all
+* platforms deploying the MMCI block, we can delete the
+* gpio_to_desc() calls.
+*/
+ host-gpio_wp = gpiod_get(dev-dev, wp);
+ if (IS_ERR(host-gpio_wp) gpio_is_valid(plat-gpio_wp))
+ host-gpio_wp = gpio_to_desc(plat-gpio_wp);
+ host-gpio_cd = gpiod_get(dev-dev, cd);
+ if (IS_ERR(host-gpio_cd) gpio_is_valid(plat-gpio_wp))
+ host-gpio_cd = gpio_to_desc(plat-gpio_cd);
host-hw_designer = amba_manf(dev);
host-hw_revision = amba_rev(dev);
@@ -1616,17 +1624,13 @@ static int mmci_probe(struct amba_device *dev,
writel(0, host-base + MMCIMASK1);
writel(0xfff, host-base + MMCICLEAR);
- if (plat-gpio_cd == -EPROBE_DEFER) {
+ if (PTR_ERR(host-gpio_cd) == -EPROBE_DEFER) {
ret = -EPROBE_DEFER;
goto err_gpio_cd;
}
- if (gpio_is_valid(plat-gpio_cd)) {
- ret = gpio_request(plat-gpio_cd, DRIVER_NAME (cd));
- if (ret == 0)
- ret = gpio_direction_input(plat-gpio_cd);
- if (ret == 0)
- host-gpio_cd = plat-gpio_cd;
- else if (ret != -ENOSYS)
+ if (!IS_ERR(host-gpio_cd)) {
+ ret = gpiod_direction_input(host-gpio_cd);
+ if (ret 0 ret != -ENOSYS)
goto err_gpio_cd;
/*
@@ -1636,28 +1640,24 @@ static int mmci_probe(struct amba_device *dev,
* for the inverted case) so we request triggers on both
* edges
Re: [PATCH 1/6] mmc: slot-gpio: Record GPIO descriptors instead of GPIO numbers
On Mon, Mar 10, 2014 at 2:02 PM, Adrian Hunter adrian.hun...@intel.com wrote: In preparation for adding a descriptor-based CD GPIO API, switch from recording GPIO numbers to recording GPIO descriptors. Signed-off-by: Adrian Hunter adrian.hun...@intel.com Certainly this makes things look better from a GPIO POV so Acked-by. However: (...) + ctx-ro_gpio = gpio_to_desc(gpio); (...) + ctx-cd_gpio = gpio_to_desc(gpio); How much harder would it be to switch the platforms using this MMC driver to pass GPIO descriptors using the gpiod_lookup_table in affected platforms (or device tree or ACPI) and then use devm_gpiod_get[_index]() in probe()? This can certainly be step 2, but that is where we want to go with the GPIO clean-up work in the long run. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] mmc: slot-gpio: Add GPIO descriptor based CD GPIO API
On Mon, Mar 10, 2014 at 2:02 PM, Adrian Hunter adrian.hun...@intel.com wrote: Add functions to request a CD GPIO using the GPIO descriptor API. Note that the new request function is paired with mmc_gpiod_free_cd() not mmc_gpio_free_cd(). Note also that it must be called prior to mmc_add_host() otherwise the caller must also call mmc_gpiod_request_cd_irq(). Signed-off-by: Adrian Hunter adrian.hun...@intel.com This is very helpful. Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/17] mmc: mmci: Mask IRQs for all variants during runtime suspend
On Tue, Feb 18, 2014 at 5:36 PM, Ulf Hansson ulf.hans...@linaro.org wrote: On 18 February 2014 17:05, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Tue, Feb 04, 2014 at 04:58:44PM +0100, Ulf Hansson wrote: In runtime suspended state, we are not expecting IRQs and thus we can safely mask them, not only for pwrreg_nopower variants but for all. Obviously we then also need to make sure we restore the IRQ mask while becoming runtime resumed. So, what happens when this patch is applied, and a SDIO card is attached which expects to receive interrupts at any moment? Currently, no variant implements SDIO irq. The SDIO irq polling mode from the sdio core will still be functional, as of today. So, this patch will not break SDIO. Given that I've run into this during the last week with a SDHCI controller, I'm not that thrilled with other interfaces doing the same broken thing. If we add SDIO irq support to mmci in future; parts of that implementation includes a re-route of DAT1 to a GPIO irq when entering runtime suspend state. The mmci HW will in runtime suspend state, not be responsible for handling irqs, which is the same as of today. [Just smalltalk] Switching DAT1 to gpio mode (which is something of a fallacy, see explanation in Documentation/pinctrl.txt) is not at all possible in all implementations of the PL18x, as it depends on exploiting properties on an assumed pin controller. Systems that don't have such wakeup features on their pins are unlikely to support deepsleep in any capacity, and if they do they are ill-designed from the top level as this needs to be taken into account when devising the hardware :-/ Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] mmc: mmci: rename some extended flags
These four (so far unused) flags are only found in the ST Micro versions of MMCI, so infix them properly with the _ST_ infix. Signed-off-by: Linus Walleij linus.wall...@linaro.org --- drivers/mmc/host/mmci.h | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 168bc72f7a94..84c0e59b792a 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -38,10 +38,11 @@ #define MCI_CPSM_INTERRUPT (1 8) #define MCI_CPSM_PENDING (1 9) #define MCI_CPSM_ENABLE(1 10) -#define MCI_SDIO_SUSP (1 11) -#define MCI_ENCMD_COMPL(1 12) -#define MCI_NIEN (1 13) -#define MCI_CE_ATACMD (1 14) +/* Argument flag extenstions in the ST Micro versions */ +#define MCI_ST_SDIO_SUSP (1 11) +#define MCI_ST_ENCMD_COMPL (1 12) +#define MCI_ST_NIEN(1 13) +#define MCI_ST_CE_ATACMD (1 14) #define MMCIRESPCMD0x010 #define MMCIRESPONSE0 0x014 -- 1.8.5.3 -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/17] mmc: mmci: Put the device into low power state at system suspend
On Tue, Feb 4, 2014 at 8:22 PM, Kevin Hilman khil...@linaro.org wrote: Ulf Hansson ulf.hans...@linaro.org writes: Due to the available runtime PM callbacks, we are now able to put our device into low power state at system suspend. (...) I'm trying to thing of a good reason to not make PM_SLEEP depend on PM_RUNTIME for platforms like this. Isn't the typical Android platform using PM_SLEEP without using PM_RUNTIME? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/17] spi: pl022: Remove redundant pinctrl to default state in probe
On Tue, Feb 4, 2014 at 4:58 PM, Ulf Hansson ulf.hans...@linaro.org wrote: The driver core is now taking care of putting our pins into default state at probe. Thus we can remove the redundant call for it in probe. Cc: Mark Brown broo...@kernel.org Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/17] i2c: nomadik: Convert to devm functions
On Tue, Feb 4, 2014 at 4:58 PM, Ulf Hansson ulf.hans...@linaro.org wrote: Use devm_* functions to simplify code and error handling. Cc: Alessandro Rubini rub...@unipv.it Cc: Linus Walleij linus.wall...@linaro.org Cc: Wolfram Sang w...@the-dreams.de Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Linus Walleij linus.wall...@linaro.org However make sure this (and the rest) applies on top of this patch: http://marc.info/?l=linux-i2cm=139142325809973w=2 Because I expect that to be applied first. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/17] i2c: nomadik: Fixup deployment of runtime PM
On Tue, Feb 4, 2014 at 4:58 PM, Ulf Hansson ulf.hans...@linaro.org wrote: Since the device is active while a successful probe has been completed, the reference counting for the clock will be screwed up and never reach zero. The issue is resolved by implementing runtime PM callbacks and let them handle the resources accordingly, including the clock. Cc: Alessandro Rubini rub...@unipv.it Cc: Linus Walleij linus.wall...@linaro.org Cc: Wolfram Sang w...@the-dreams.de Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Hm do I read it right as patch 13 breaks runtime PM by leaving the device active after probe() and this patch 14 fixes it again? Maybe these two patches should be squashed then. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/16] gpio: rcar: use postcore_init()
On Mon, Nov 18, 2013 at 3:00 PM, Laurent Pinchart laurent.pinch...@ideasonboard.com wrote: I'm not against moving the gpio-rcar initialization to postcore or subsys initcall, but in that case I believe we should standardize (or at least try to) this across the GPIO drivers. We currently have $ cat drivers/gpio/gpio-*.c | grep _initcall | grep '^[a-z]' | sed 's/(.*//' | sort | uniq -c 2 arch_initcall 1 core_initcall 1 device_initcall 1 late_initcall 11 postcore_initcall 2 pure_initcall 31 subsys_initcall $ cat drivers/gpio/gpio-*.c | grep 'module_.*_driver' | sed 's/(.*//' | sort | uniq -c 3 module_i2c_driver 4 module_pci_driver 23 module_platform_driver 1 module_spi_driver Linus, do you have any guidelines on this ? The general guideline, as everybody should be aware ;-) is that we should always use module_init(), i.e. device_initcall() and let deferred probe handle any dependencies. The only exception would be things like timers and interrupt controllers... Usually not relying on deferred probe is a sign of bugs in the deferral probe path. I know my drivers have this problem too, I would prefer that we try to fix the root issue instead of trying to shovel initcalls around. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 1/4] mmc: mmci: Adapt to new pinctrl handling
On Tue, Sep 3, 2013 at 11:29 AM, Ulf Hansson ulf.hans...@linaro.org wrote: There is no need for every driver to fetch a pinctrl handle and to select the default state. Instead this is handled by the device driver core, thus we can remove this piece of code from mmci. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Cc: Linus Walleij linus.wall...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3 2/4] mmc: mmci: Use optional sleep pinctrl state
On Tue, Sep 3, 2013 at 11:29 AM, Ulf Hansson ulf.hans...@linaro.org wrote: By optionally putting the pins into sleep state in the .runtime_suspend callback we can accomplish two things. One is to minimize current leakage from pins and thus save power, second we can prevent the IP from driving pins output in an uncontrolled manner, which may happen if the power domain drops the domain regulator. When returning from idle, entering .runtime_resume callback, the pins are restored to default state. Signed-off-by: Ulf Hansson ulf.hans...@linaro.org Acked-by: Rickard Andersson rickard.anders...@stericsson.com Cc: Linus Walleij linus.wall...@linaro.org Reviewed-by: Linus Walleij linus.wall...@linaro.org Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] mmc: omap_hsmmc: add support for pbias configuration in dt
On Thu, Jun 6, 2013 at 9:14 PM, Balaji T K balaj...@ti.com wrote:
PBIAS register configuration is based on the regulator voltage
which supplies these pbias cells, sd i/o pads.
With PBIAS register address and bit definitions different across
omap[3,4,5], Simplify PBIAS configuration under three different
regulator voltage levels - O V, 1.8 V, 3 V. Corresponding pinctrl states
are defined as pbias_off, pbias_1v8, pbias_3v.
pinctrl state mmc_init is used for configuring speed mode, loopback clock
(in devconf0/devconf1/prog_io1 register for omap3) and pull strength
configuration (in control_mmc1 for omap4)
Signed-off-by: Balaji T K balaj...@ti.com
You *need* Lee Jones and Mark Brown to review this.
Maybe Laurent has something to add too.
Ux500 had the very same thing, and there this was solved using
a GPIO regulator for vqmmc a level-shifter. I vaguely remember
Laurent doing something similar with the SH stuff.
+ /* 100ms delay required for PBIAS configuration */
+ msleep(100);
+ if (!vdd host-pinctrl host-pbias_off) {
+ ret = pinctrl_select_state(host-pinctrl, host-pbias_off);
+ if (ret 0)
+ dev_err(host-dev, pinctrl pbias_off select
error\n);
+ } else if (((1 vdd) = MMC_VDD_165_195) host-pinctrl
+ host-pbias_1v8) {
+ ret = pinctrl_select_state(host-pinctrl, host-pbias_1v8);
+ if (ret 0)
+ dev_err(host-dev, pinctrl pbias_1v8 select
error\n);
+ } else if (((1 vdd) MMC_VDD_165_195) host-pinctrl
+ host-pbias_3v) {
+ ret = pinctrl_select_state(host-pinctrl, host-pbias_3v);
+ if (ret 0)
+ dev_err(host-dev, pinctrl pbias_3v select error\n);
+ }
So why does the pin control API control bias voltage?
This seem so intuitively wrong as it can possibly get, clearly this
is regulator territory.
This also looks strange from an MMC point of view.
It just seems these bits in these registers should be poked at
by the regulator world, not the pinctrl world. That the bits are
in the middle of pinctrl things does not really matter.
+ usleep_range(350, 400);
And the regulator framework supports power-on delays.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] mmc: omap_hsmmc: add support for pbias configuration in dt
On Wed, Jun 12, 2013 at 4:37 PM, Tony Lindgren t...@atomide.com wrote: Linus W may have some comments on this, although this is not the standard muxing stuff. It's in the wrong subsystem and needs to be rewritten IMO. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] mmc: omap_hsmmc: add support for pbias configuration in dt
On Thu, Jun 13, 2013 at 11:53 AM, Tony Lindgren t...@atomide.com wrote:
* Linus Walleij linus.wall...@linaro.org [130613 02:42]:
On Thu, Jun 6, 2013 at 9:14 PM, Balaji T K balaj...@ti.com wrote:
+ /* 100ms delay required for PBIAS configuration */
+ msleep(100);
+ if (!vdd host-pinctrl host-pbias_off) {
+ ret = pinctrl_select_state(host-pinctrl, host-pbias_off);
+ if (ret 0)
+ dev_err(host-dev, pinctrl pbias_off select
error\n);
+ } else if (((1 vdd) = MMC_VDD_165_195) host-pinctrl
+ host-pbias_1v8) {
+ ret = pinctrl_select_state(host-pinctrl, host-pbias_1v8);
+ if (ret 0)
+ dev_err(host-dev, pinctrl pbias_1v8 select
error\n);
+ } else if (((1 vdd) MMC_VDD_165_195) host-pinctrl
+ host-pbias_3v) {
+ ret = pinctrl_select_state(host-pinctrl, host-pbias_3v);
+ if (ret 0)
+ dev_err(host-dev, pinctrl pbias_3v select
error\n);
+ }
So why does the pin control API control bias voltage?
I agree, it should be a regulator for the MMC driver and that's what I
already suggested earlier. Having it as a regulator allows us to get
rid of all the non-standard before and after calls in the omap_hsmmc.c.
This way the omap_hsmmc.c code can handle the internal and external
voltages the same way.
So someone is not taking the OMAP maintainers word for it and
instead trying to push this past the pinctrl maintainer?
Grrr OK I'll assume good faith and I therefore conclude that
this must have been just some kind of mistake in the act.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/10] mmc: omap_hsmmc: add support for pbias configuration in dt
On Thu, Jun 13, 2013 at 4:41 PM, Balaji T K balaj...@ti.com wrote: [Me] This seem so intuitively wrong as it can possibly get, clearly this is regulator territory. It is not really a regulator, CONTROL_PBIAS_LITE is just a register in control module which configures pad/pin on SOC. In this case PBIAS cells are powered down before any voltage changes and after the external voltage supplied to VDDS_MMC of OMAP stabilizes pbias cells is powered ON again with specific Voltage which is given to OMAP for MMC io pins So there is some external actual regulator supplying the voltage and then that is looped back in to the pads. (I understand this is quite a common construction.) Anyway since it is obviously removing and applying voltage to the pins, I think this is more of a cascaded regulator. We use regulators for simple light-switches and things you know. We have generic pinconf options for things like current but not voltage really. It's because in the case of pin current configuration it's basically about how many driver stages (totempoles) you connect to the output, which is very different from the voltages we're dealing with here. So I think these bits go into a regulator driver: BIT26 MMC1_PWRDNZ PWRDNZ control to MMC1 IO This bit is used to protect the MMC1 I/O cell when SDMMC1_VDDS is not stable. 0x0: Software must clear this bit when SDMMC1_VDDS changes. 0x1: Software must set this bit only when SDMMC1_VDDS is stable. BIT24 MMC1_PBIASLITE_SUPPLY_HI SUPPLY_HI_OUT from MMC1 PBIASLITE _OUT Read 0x0: SDMMC1_VDDS = 1.8V Read 0x1: SDMMC1_VDDS = 3V BIT23 MMC1_PBIASLITE_VMODE_ER VMODE ERROR from MMC1 PBIASLITE ROR Read 0x0: VMODE level is same as SUPPLY_HI_OUT Read 0x1: VMODE level is not same as SUPPLY_HI_OUT BIT22 MMC1_PBIASLITE_PWRDNZ PWRDNZ control to MMC1 PBIASLITE This bit is used to protect the MMC1_PBIAS cell (MMC1 I/O cell associated) when SDMMC1_VDDS is not stable. 0x0: Software must clear this bit when SDMMC1_VDDS changes. 0x1: Software must set this bit only when SDMMC1_VDDS is stable. BIT21 MMC1_PBIASLITE_VMODE VMODE control to MMC1 PBIASLITE 0x0: SDMMC1_VDDS = 1.8V 0x1: SDMMC1_VDDS = 3V This looks much more cascaded regulator control than anything else to me. For OMAP2430, OMAP3430 It additionally has a bit for speed mode control which are set always (static config) I guess you mean this: BIT2 PBIASSPEEDCTRL0 Speed Control for MMC I/O 0b0 = 26 MHz I/O max speed 0b1 = 52 MHz I/O max speed This seems to belong to the MMC host driver. It is common that registers contain indiviual bits that end up in different subsystems. Speed mode seems to belong in the MMC driver. The infrastructure used to spread out responsibility across different drivers from a common register range or even for certain bits in a certain register, is regmap. Check for example mfd/syscon.c. BIT25 MMC1_PBIASLITE_HIZ_MODE HIZ_MODE from MMC1 PBIASLITE 0x0: PBIAS in normal operation mode 0x1: PBIAS output is in high impedence state This is actually pin config. But if it makes sense it should also be part of the MMC regulator driver. You mean regulator via pinctrl APIs, I think It will just move the code from omap_hsmmc to a new regulator file with it own init data for pinctrl. No I'm not saying you should use pinctrl as a back-end for this. I mean you shall instantiate a regulator and let the callback ops vtable for that regulator poke these bits. Not sure if Regulator maintainer will agree to it. I will respect Marks judgement on this for sure. Moreover what I needs is three different states 0V, 0 to 1.8V, 3 V to 3.3V not 0, 1.8V, 3V. plus pbias register fields got moved around between omap3, omap4 and omap5, That was one of the reason for moving to pinctrl states. The regulator framework supports selector tables just like this. Where the register fields are located does not matter. That the bits are in the middle of pinctrl things does not really matter. It thought pinctrl-single,bits in pinctrl-single.c is introduced precisely for such misc control register which has bit configuration affecting different module i/o pads. No. If we go down that road *anything* that is connected to a pad becomes part of the pinctrl subsystem, then pinctrl-single becomes some kind of hardware abstraction or BIOS, and that is *not* the intent. It is only supposed to deal with the bits there that are 100% related to what pinctrl does, nothing else. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] mmc: omap_hsmmc: Remux pins to support SDIO interrupt and PM runtime
On Mon, Jun 10, 2013 at 6:23 PM, Tony Lindgren t...@atomide.com wrote: We only should remux the pins that need remuxing as that's done every time hitting idle. So I think we should have the following default groups: default Static pins that don't change, no need to remux configured in consumer driver probe like we already do active Optional dynamic pins remuxed for runtime, can be configured and selected in consumer driver probe. The consumer driver may also want to select this state in PM runtime resume. idleOptional dynamic pins remuxed for idle. The consumer driver may also want to select this state in PM runtime suspend depending on device_can_wakeup() and driver specific needs. The one thing I don't understand is why a driver would select the active state in probe(), unless it's a driver that does not support runtime PM. (But maybe that's what you mean.) Compare this to linus/pinctrl/pinctrl-state.h: /** * @PINCTRL_STATE_DEFAULT: the state the pinctrl handle shall be put * into as default, usually this means the pins are up and ready to * be used by the device driver. This state is commonly used by * hogs to configure muxing and pins at boot, and also as a state * to go into when returning from sleep and idle in * .pm_runtime_resume() or ordinary .resume() for example. * @PINCTRL_STATE_IDLE: the state the pinctrl handle shall be put into * when the pins are idle. This is a state where the system is relaxed * but not fully sleeping - some power may be on but clocks gated for * example. Could typically be set from a pm_runtime_suspend() or * pm_runtime_idle() operation. * @PINCTRL_STATE_SLEEP: the state the pinctrl handle shall be put into * when the pins are sleeping. This is a state where the system is in * its lowest sleep state. Could typically be set from an * ordinary .suspend() function. */ #define PINCTRL_STATE_DEFAULT default #define PINCTRL_STATE_IDLE idle #define PINCTRL_STATE_SLEEP sleep The way I currently use these in e.g. drivers/spi/spi-pl022 is: probe: - default runtime_suspend: - idle runtime_resume: - default suspend: - sleep resume: - default - idle Notice that we go to default then idle on probe and runtime resume. This is because the idle state is optional (as is the sleep state). So I guess if we should extend this terminology to match what you are using for the OMAP it would rather be like this: probe: - default runtime_suspend: - idle runtime_resume: - default - active suspend: - sleep resume: - default - idle Just one more optional active state in runtime resume. Correct? If we can agree on this I will add the active state to the state table and add a container in the core for this as well as pinctrl_pm_select_active_state() so we can skip all the pointless boilerplate also in the OMAP drivers, plus increase the readability and portability quite a bit. However in this case I *suspect* that what you really want to do it to rename the state called default to sleep (it appears the default state is sleepy) and then rename the active state to default (as this is the defined semantic meaning of default from linux/pinctrl/pinctrl-state.h. The idle state above could also be called sleep instead of idle if you prefer that. No I think we should reserve that name for the pin state associated with suspend(). Let's leave it like this. I think the confusion is caused by the fact that we need three mux groups, not just two :) The toggling between active and idle is the hotpath as that can potentially happen for multiple drivers every time we enter and exit idle. Actually we have the same thing, it's just that our default and active are the same thing. But it seems we need to add your granularity to this. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] mmc: omap_hsmmc: Remux pins to support SDIO interrupt and PM runtime
On Fri, Jun 7, 2013 at 11:49 PM, Tony Lindgren t...@atomide.com wrote:
On some omaps we need to remux MMC pins for PM, and for some omaps
we need to remux the SDIO IRQ pin.
Based on an earlier patch by Andreas Fenkart afenk...@gmail.com.
(...)
+ host-pinctrl = devm_pinctrl_get(host-dev);
+ if (IS_ERR(host-pinctrl)) {
+ dev_dbg(host-dev, no pinctrl handle\n);
+ ret = 0;
+ goto out;
+ }
+
+ host-fixed = pinctrl_lookup_state(host-pinctrl,
+ PINCTRL_STATE_DEFAULT);
+ if (IS_ERR(host-fixed)) {
+ dev_dbg(host-dev,
+pins are not configured from the driver\n);
+ host-fixed = NULL;
+ ret = 0;
+ goto out;
+ }
+
+ ret = pinctrl_select_state(host-pinctrl, host-fixed);
+ if (ret 0)
+ goto err;
+
+ /* For most cases we don't have wake-ups, and exit after this */
+ host-active = pinctrl_lookup_state(host-pinctrl, active);
+ if (IS_ERR(host-active)) {
+ ret = PTR_ERR(host-active);
+ host-active = NULL;
+ return 0;
+ }
+
+ host-idle = pinctrl_lookup_state(host-pinctrl,
+ PINCTRL_STATE_IDLE);
+ if (IS_ERR(host-idle)) {
+ ret = PTR_ERR(host-idle);
+ host-idle = NULL;
+ goto err;
+ }
You can use the new infrastructure to make the core select:
pinctrl_pm_select_default_state(host-dev);
pinctrl_pm_select_idle_state(host-dev);
What is the semantic difference between default and active?
If this is something very generic that a lot of platforms will want
to have, why not add it to include/linux/pinctrl/pinctrl-state.h
and augment the core to cache and handle this too?
However in this case I *suspect* that what you really want
to do it to rename the state called default to sleep
(it appears the default state is sleepy) and then rename
the active state to default (as this is the defined semantic
meaning of default from linux/pinctrl/pinctrl-state.h.
But maybe I'm not quite getting the subtle difference between
default and active here so enlighten me.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/8] clk: flag to use upper half of the register as change indicator
On Thu, Jun 6, 2013 at 9:08 PM, Heiko Stübner he...@sntech.de wrote:
There exist platforms, namely at least all Rockchip Cortex-A9 based ones,
that don't use the paradigm of reading-changing-writing the register contents,
but instead only write the changes to the register with a mask that indicates
the changed bits.
This patch adds flags and code to support the case where the lower 16 bit of
hold the information and the upper 16 bit are used as mask to indicate the
written changes.
As hardware-specific flags should not be added to the common clk flags, the
flags are added to gate, mux and divider clocks individually.
Signed-off-by: Heiko Stuebner he...@sntech.de
(...)
+ if ((clk_gate_flags CLK_GATE_MASK_UPPER_HALF) bit_idx 15) {
+ pr_err(%s: bit_idx %d invalid\n, __func__, bit_idx);
+ return ERR_PTR(-EINVAL);
+ }
Now this looks *EXTREMELY* familiar to a patch just sent by Haojian
for HiSilicon.
[PATCH v2 3/6] clk: divider: add CLK_DIVIDER_HIWORD_MASK flag
http://marc.info/?l=linux-arm-kernelm=137035873916777w=2
What kind of coincidence is this? Are Rockchip and HiSilicon using
the same silicon IP or are they of a common origin? (It is a small
world after all.)
I think you two guys need to read each others patch sets closely
here. I'd like Haojian to look at Heiko's patches and Heiko to look
at Haojian's patches, just to make sure you're not actually writing
two drivers for the same hardware in the end.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 5/8] pinctrl: add pinctrl driver for Rockchip SoCs
On Thu, Jun 6, 2013 at 9:11 PM, Heiko Stübner he...@sntech.de wrote:
This driver adds support the Cortex-A9 based SoCs from Rockchip,
so at least the RK2928, RK3066 (a and b) and RK3188.
Earlier Rockchip SoCs seem to use similar mechanics for gpio
handling so should be supportable with relative small changes.
Pull handling on the rk3188 is currently a stub, due to it being
a bit different to the earlier SoCs.
Pinmuxing as well as gpio (and interrupt-) handling tested on
a rk3066a based machine.
Signed-off-by: Heiko Stuebner he...@sntech.de
This basically looks fine.
(...)
+Required properties for pin configuration node:
+ - rockchip,pins: 4 integers array, represents a group of pins mux and
config
+setting. The format is rockchip,pins = PIN_BANK PIN_BANK_NUM MUX
CONFIG.
+The MUX 0 means gpio and MUX 1 to 3 mean the specific device function
+
+Bits used for CONFIG:
+PULL_AUTO (1 0): indicate this pin needs a pull setting for SoCs
+ that determine the pull up or down themselfs
+PULL_UP(1 1): indicate this pin needs a pull up
+PULL_DOWN (1 2): indicate this pin needs a pull down
So I would rather have these (as a separate patch) in
dt-bindings/pinctrl/pinconfig.h
The documentation in
Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
and the same bindings to be used for as many systems as
possible utilizing generic pin config.
I will be liberal in accepting patches creating this
infrastructure.
We need to realize that we will be setting example for everyone
else, and everyone else will be following that example.
+ for (i = 0, j = 0; i size; i += 4, j++) {
+ unsigned long pinconf;
+
+ num = be32_to_cpu(*list++);
+ bank = bank_num_to_bank(info, num);
+ if (IS_ERR(bank))
+ return PTR_ERR(bank);
+
+ pin = be32_to_cpu(*list++);
+ grp-pins[j] = bank-pin_base + pin;
+ grp-func[j] = be32_to_cpu(*list++);
+
+ pinconf = be32_to_cpu(*list++);
+ switch(pinconf) {
+ case RK_PINCTRL_NONE:
+ bias = PIN_CONFIG_BIAS_DISABLE;
+ break;
+ case RK_PINCTRL_PULL_PIN_DEFAULT:
+ bias = PIN_CONFIG_BIAS_PULL_PIN_DEFAULT;
+ break;
+ case RK_PINCTRL_PULL_UP:
+ bias = PIN_CONFIG_BIAS_PULL_UP;
+ break;
+ case RK_PINCTRL_PULL_DOWN:
+ bias = PIN_CONFIG_BIAS_PULL_DOWN;
+ break;
+ }
+
+ grp-configs[j] = pinconf_to_config_packed(bias, 0);
+ }
I would like this to be added to
drivers/pinctrl/pinconf-generic.c
as a utility function, with the header in
drivers/pinctrl/pinconf.h
Also in a separate patch.
+++ b/include/dt-bindings/pinctrl/rockchip.h
(...)
+#define RK_PINCTRL_NONE0
+#define RK_PINCTRL_PULL_PIN_DEFAULT(1 0)
+#define RK_PINCTRL_PULL_UP (1 1)
+#define RK_PINCTRL_PULL_DOWN (1 2)
So I'd like these moved to /include/dt-bindings/pinctrl/pinconfig.h
and used as a separete include. Drop the RK_* prefix as it
will be universal and use a PINCONFIG_* prefix instead
of PINCTRL_*.
I think that is the route we need to take.
Bonus if you implement more config options from
pinconf-generic.h but otherwise me and others will have
to do it.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/10] pinctrl: add pinctrl driver for Rockchip SoCs
On Tue, Jun 4, 2013 at 2:05 PM, Heiko Stübner he...@sntech.de wrote: +PULL_AUTO (1 0): indicate this pin needs a pull setting for SoCs + that determine the pull up or down themselfs Hm, never saw that before... Citing the original gpio driver: /* * Values written to this register independently * control Pullup/Pulldown or not for the * corresponding data bit in GPIO. * 0: pull up/down enable, PAD type will decide * to be up or down, not related with this value * 1: pull up/down disable */ So if it's a pull up or down is decided based on the mux of the pin. Calling everything a pull down (or up) when it isn't seemed somehow wrong to me. The rk3188 on the other hand supports both pull up and down separately. Or should this be selected as PULL_UP | PULL_DOWN in the config? The generic config is pretty much either/or so it'd be a new config for that approach. Basically it seems they have embedded knowledge into the silicon: there is no specific rule as to whether a pad should be pulled up or down depending on pad type as is stated, rather it's so that if you know a pad will be used for I2C SCL then you know it needs pull-up. Probably something like a 1 on some constant switch in VHDL/Verilog is hard-coded into the silicon turning on pull-up if I2C is selected and the autopull is set for example. +config PINCTRL_ROCKCHIP + bool + select PINMUX + select PINCONF + select GENERIC_IRQ_CHIP Why is this super-simple pin config thing not using GENERIC_PINCONF? I *know* it is simpler to implement your own thing, but think of the poor maintainers that have to wade through 50 identical implementations. Do this, it pays off. generic pinconf sounds interesting ... will give it a try. The only problem is the pull stuff mentioned above that is either pull up or down without the driver having knowledge about it. And generic_pinconf only knows about them separately right now. Create a separate patch adding PIN_CONFIG_BIAS_PULL_AUTO to include/linux/pinctrl/pinconf-generic.h, don't forget the kerneldoc, and patching drivers/pinctrl/pinconf-generic.c. I'll apply it right away. Nothing else you want to say about the pins here? (No big deal for sure, but) when using pinconfig_generic, its dump_pin function should be more talkative right? Yes, one of the things you get for free... soon also the pin config DT parser will be for free I hope. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/10] clocksource: dw_apb_timer_of: use the clocksource as sched clock if necessary
On Mon, Jun 3, 2013 at 12:56 AM, Heiko Stübner he...@sntech.de wrote: Currently the dw_apb_timer always expects a separate special timer to be availbable for the sched_clock. Some devices using dw_apb_timers do not have the sptimer but can use the clocksource as sched_clock. Therefore this patch adds using the clocksource timer as a fallback if no usable sched timer is found. Signed-off-by: Heiko Stuebner he...@sntech.de Is this really what the patch does? I mean ass the clocksourse as fallback, it seems more like that is controlled from the device tree, this looks more like some more careful handling of the device tree input making the sched_timer optional, it doesn't really add anything, does it? Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/10] pinctrl: add pinctrl driver for Rockchip SoCs
;
That's a bit hard to read, I'd just:
#include linux/bitops.h
return !(readl_relaxed(reg) BIT(bit));
And skip the data variable. The ! operator will
clamp this to a bool (0/1).
But we all have our habits.
+static int rockchip_set_pull(struct rockchip_pin_bank *bank,
+ int pin_num, int pull)
Similar comments for this function.
+static int rockchip_pmx_gpio_set_direction(struct pinctrl_dev *pctldev,
+ struct pinctrl_gpio_range
*range,
+ unsigned offset, bool input)
(...)
+ /* set bit to 1 for output, 0 for input */
+ if (!input)
+ data |= (1 pin);
+ else
+ data = ~(1 pin);
Here again I would use linux/bitops.h and:
data |= BIT(pin);
etc, but it's a matter of taste so if you prefer this, do keep it.
(...)
+static int rockchip_irq_set_type(struct irq_data *d, unsigned int type)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+ struct rockchip_pin_bank *bank = gc-private;
+ u32 bit = (1 d-hwirq);
Name that something like mask to match other naming in the
kernel. bit sort of implies a number between 0 and 31 and that
is not the case.
All I could think of right now...
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/11] mmc: omap_hsmmc: enhance pinctrl support
On Fri, May 31, 2013 at 12:13 PM, Hebbar Gururaja gururaja.heb...@ti.com wrote: Amend the hsmmc controller to optionally take a pin control handle and set the state of the pins to: - default on boot, resume and before performing a mmc transfer - idle after initial default, after resume default, and after each mmc/sd card access - sleep on suspend() By optionally putting the pins into sleep state in the suspend callback we can accomplish two things. - One is to minimize current leakage from pins and thus save power, - second, we can prevent the IP from driving pins output in an uncontrolled manner, which may happen if the power domain drops the domain regulator. If any of the above pin states are missing in dt, a warning message about the missing state is displayed. If certain pin-states are not available, to remove this warning message pass respective state name with null phandler. Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com Cc: Balaji T K balaj...@ti.com Cc: Chris Ball c...@laptop.org Cc: linux-mmc@vger.kernel.org Cc: linux-o...@vger.kernel.org This is perfectly correct. Acked-by: Linus Walleij linus.wall...@linaro.org As the PM code seems to be similar across platforms I have had loose plans to move this to the device core as well, but right now I'm too busy with other things, and it can surely be refactored later. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/11] mmc: omap_hsmmc: enhance pinctrl support
On Tue, Jun 4, 2013 at 9:11 AM, Linus Walleij linus.wall...@linaro.org wrote: On Fri, May 31, 2013 at 12:13 PM, Hebbar Gururaja gururaja.heb...@ti.com wrote: Amend the hsmmc controller to optionally take a pin control handle and set the state of the pins to: - default on boot, resume and before performing a mmc transfer - idle after initial default, after resume default, and after each mmc/sd card access - sleep on suspend() By optionally putting the pins into sleep state in the suspend callback we can accomplish two things. - One is to minimize current leakage from pins and thus save power, - second, we can prevent the IP from driving pins output in an uncontrolled manner, which may happen if the power domain drops the domain regulator. If any of the above pin states are missing in dt, a warning message about the missing state is displayed. If certain pin-states are not available, to remove this warning message pass respective state name with null phandler. Signed-off-by: Hebbar Gururaja gururaja.heb...@ti.com Cc: Balaji T K balaj...@ti.com Cc: Chris Ball c...@laptop.org Cc: linux-mmc@vger.kernel.org Cc: linux-o...@vger.kernel.org This is perfectly correct. Acked-by: Linus Walleij linus.wall...@linaro.org So please consider my other option given in patch 2 instead. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-mmc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
