* [RFC 1/7] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI
2016-02-19 20:16 [RFC 0/7] r8a7790: add UHS-I (SDR50) support to Lager Wolfram Sang
@ 2016-02-19 20:16 ` Wolfram Sang
2016-02-29 10:53 ` Geert Uytterhoeven
2016-02-19 20:16 ` [RFC 2/7] mmc: tmio, sh_mobile_sdhi: Pass tmio_mmc_host ptr to clk_{enable,disable} ops Wolfram Sang
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2016-02-19 20:16 UTC (permalink / raw)
To: linux-renesas-soc
Cc: linux-mmc, Ben Hutchings, Dirk Behme, Wolfram Sang, Ulrich Hecht
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
All the SHDIs can operate with either 3.3V or 1.8V signals, depending
on negotiation with the card.
Implement the {get,set}_io_voltage operations and set the related
capability flag for the associated pins.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Ulrich Hecht <ulrich.hecht@gmail.com>
---
The get/set-functions introduced here are the same for all Gen2, except for
the pin-sanity checks at the beginning. Somehow a generic function might be
nice for consistency reasons (I don't care so much about the code savings).
But not sure if it is worth the hazzle.
@Uli, Geert: Since bias support is probably similar, what do you think? Does
it make sense to add gen{2,3}.c?
drivers/pinctrl/sh-pfc/core.c | 2 +-
drivers/pinctrl/sh-pfc/core.h | 1 +
drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 60 +++++++++++++++++++++++++++++++++++-
3 files changed, 61 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 181ea98a63b7ab..315f9db9f5affe 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -88,7 +88,7 @@ static int sh_pfc_map_resources(struct sh_pfc *pfc,
return 0;
}
-static void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 reg)
+void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 reg)
{
struct sh_pfc_window *window;
phys_addr_t address = reg;
diff --git a/drivers/pinctrl/sh-pfc/core.h b/drivers/pinctrl/sh-pfc/core.h
index 62f53b22ae8500..ca041a753a2521 100644
--- a/drivers/pinctrl/sh-pfc/core.h
+++ b/drivers/pinctrl/sh-pfc/core.h
@@ -59,6 +59,7 @@ int sh_pfc_unregister_gpiochip(struct sh_pfc *pfc);
int sh_pfc_register_pinctrl(struct sh_pfc *pfc);
int sh_pfc_unregister_pinctrl(struct sh_pfc *pfc);
+void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 address);
u32 sh_pfc_read_raw_reg(void __iomem *mapped_reg, unsigned int reg_width);
void sh_pfc_write_raw_reg(void __iomem *mapped_reg, unsigned int reg_width,
u32 data);
diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
index 0f4d48f9400ba0..ed51d67609050d 100644
--- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
+++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
@@ -21,16 +21,21 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/
+#include <linux/io.h>
#include <linux/kernel.h>
#include "core.h"
#include "sh_pfc.h"
+/*
+ * All pins assigned to GPIO bank 3 can be used for SD interfaces in
+ * which case they support both 3.3V and 1.8V signalling.
+ */
#define CPU_ALL_PORT(fn, sfx) \
PORT_GP_32(0, fn, sfx), \
PORT_GP_30(1, fn, sfx), \
PORT_GP_30(2, fn, sfx), \
- PORT_GP_32(3, fn, sfx), \
+ PORT_GP_CFG_32(3, fn, sfx, SH_PFC_PIN_CFG_IO_VOLTAGE), \
PORT_GP_32(4, fn, sfx), \
PORT_GP_32(5, fn, sfx)
@@ -4691,6 +4696,53 @@ static const char * const vin3_groups[] = {
"vin3_clk",
};
+#define IOCTRL6 0xe606008c
+
+static int r8a7790_get_io_voltage(struct sh_pfc *pfc, unsigned int pin)
+{
+ void __iomem *reg;
+ u32 data, mask;
+
+ if (WARN(pin < RCAR_GP_PIN(3, 0) || pin > RCAR_GP_PIN(3, 31), "invalid pin %#x", pin))
+ return -EINVAL;
+
+ reg = sh_pfc_phys_to_virt(pfc, IOCTRL6);
+ data = ioread32(reg);
+
+ /* Bits in IOCTRL6 are numbered in opposite order to pins */
+ mask = 0x80000000 >> (pin & 0x1f);
+
+ return (data & mask) ? 3300 : 1800;
+}
+
+static int r8a7790_set_io_voltage(struct sh_pfc *pfc, unsigned int pin, u16 mV)
+{
+ void __iomem *reg;
+ u32 data, mask;
+
+ if (WARN(pin < RCAR_GP_PIN(3, 0) || pin > RCAR_GP_PIN(3, 31), "invalid pin %#x", pin))
+ return -EINVAL;
+
+ if (mV != 1800 && mV != 3300)
+ return -EINVAL;
+
+ reg = sh_pfc_phys_to_virt(pfc, IOCTRL6);
+ data = ioread32(reg);
+
+ /* Bits in IOCTRL6 are numbered in opposite order to pins */
+ mask = 0x80000000 >> (pin & 0x1f);
+
+ if (mV == 3300)
+ data |= mask;
+ else
+ data &= ~mask;
+
+ iowrite32(~data, sh_pfc_phys_to_virt(pfc, pfc->info->unlock_reg));
+ iowrite32(data, reg);
+
+ return 0;
+}
+
static const struct sh_pfc_function pinmux_functions[] = {
SH_PFC_FUNCTION(audio_clk),
SH_PFC_FUNCTION(avb),
@@ -5690,8 +5742,14 @@ static const struct pinmux_cfg_reg pinmux_config_regs[] = {
{ },
};
+static const struct sh_pfc_soc_operations pinmux_ops = {
+ .get_io_voltage = r8a7790_get_io_voltage,
+ .set_io_voltage = r8a7790_set_io_voltage,
+};
+
const struct sh_pfc_soc_info r8a7790_pinmux_info = {
.name = "r8a77900_pfc",
+ .ops = &pinmux_ops,
.unlock_reg = 0xe6060000, /* PMMR */
.function = { PINMUX_FUNCTION_BEGIN, PINMUX_FUNCTION_END },
--
2.7.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RFC 1/7] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI
2016-02-19 20:16 ` [RFC 1/7] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI Wolfram Sang
@ 2016-02-29 10:53 ` Geert Uytterhoeven
2016-02-29 11:37 ` Wolfram Sang
2016-03-01 13:42 ` Wolfram Sang
0 siblings, 2 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2016-02-29 10:53 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-renesas-soc, Linux MMC List, Ben Hutchings, Dirk Behme,
Wolfram Sang, Ulrich Hecht
Hi Wolfram,
On Fri, Feb 19, 2016 at 9:16 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> All the SHDIs can operate with either 3.3V or 1.8V signals, depending
> on negotiation with the card.
>
> Implement the {get,set}_io_voltage operations and set the related
> capability flag for the associated pins.
>
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Shouldn't the "From" line have Ben's authorship, too?
You can list your modifications in "[...]".
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Thanks!
> Cc: Ulrich Hecht <ulrich.hecht@gmail.com>
> ---
>
> The get/set-functions introduced here are the same for all Gen2, except for
> the pin-sanity checks at the beginning. Somehow a generic function might be
> nice for consistency reasons (I don't care so much about the code savings).
> But not sure if it is worth the hazzle.
>
> @Uli, Geert: Since bias support is probably similar, what do you think? Does
> it make sense to add gen{2,3}.c?
It's up to you.
We can always consolidate later.
> drivers/pinctrl/sh-pfc/core.c | 2 +-
> drivers/pinctrl/sh-pfc/core.h | 1 +
> drivers/pinctrl/sh-pfc/pfc-r8a7790.c | 60 +++++++++++++++++++++++++++++++++++-
> 3 files changed, 61 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
> index 181ea98a63b7ab..315f9db9f5affe 100644
> --- a/drivers/pinctrl/sh-pfc/core.c
> +++ b/drivers/pinctrl/sh-pfc/core.c
> @@ -88,7 +88,7 @@ static int sh_pfc_map_resources(struct sh_pfc *pfc,
> return 0;
> }
>
> -static void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 reg)
> +void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 reg)
You don't need to make sh_pfc_phys_to_virt() non-static...
> {
> struct sh_pfc_window *window;
> phys_addr_t address = reg;
> diff --git a/drivers/pinctrl/sh-pfc/core.h b/drivers/pinctrl/sh-pfc/core.h
> index 62f53b22ae8500..ca041a753a2521 100644
> --- a/drivers/pinctrl/sh-pfc/core.h
> +++ b/drivers/pinctrl/sh-pfc/core.h
> @@ -59,6 +59,7 @@ int sh_pfc_unregister_gpiochip(struct sh_pfc *pfc);
> int sh_pfc_register_pinctrl(struct sh_pfc *pfc);
> int sh_pfc_unregister_pinctrl(struct sh_pfc *pfc);
>
> +void __iomem *sh_pfc_phys_to_virt(struct sh_pfc *pfc, u32 address);
... and declare it...
> u32 sh_pfc_read_raw_reg(void __iomem *mapped_reg, unsigned int reg_width);
> void sh_pfc_write_raw_reg(void __iomem *mapped_reg, unsigned int reg_width,
> u32 data);
> diff --git a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> index 0f4d48f9400ba0..ed51d67609050d 100644
> --- a/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> +++ b/drivers/pinctrl/sh-pfc/pfc-r8a7790.c
> @@ -4691,6 +4696,53 @@ static const char * const vin3_groups[] = {
> "vin3_clk",
> };
>
> +#define IOCTRL6 0xe606008c
... if you use 0x8c here...
> +static int r8a7790_get_io_voltage(struct sh_pfc *pfc, unsigned int pin)
> +{
> + void __iomem *reg;
> + u32 data, mask;
> +
> + if (WARN(pin < RCAR_GP_PIN(3, 0) || pin > RCAR_GP_PIN(3, 31), "invalid pin %#x", pin))
> + return -EINVAL;
> +
> + reg = sh_pfc_phys_to_virt(pfc, IOCTRL6);
... and use "pfc->windows->virt + IOCTRL6" here and in
r8a7790_set_io_voltage(), cfr. r8a7778_pinmux_get_bias().
> + data = ioread32(reg);
> +
> + /* Bits in IOCTRL6 are numbered in opposite order to pins */
> + mask = 0x80000000 >> (pin & 0x1f);
> +
> + return (data & mask) ? 3300 : 1800;
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 1/7] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI
2016-02-29 10:53 ` Geert Uytterhoeven
@ 2016-02-29 11:37 ` Wolfram Sang
2016-03-01 13:42 ` Wolfram Sang
1 sibling, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2016-02-29 11:37 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-renesas-soc, Linux MMC List, Ben Hutchings, Dirk Behme,
Wolfram Sang, Ulrich Hecht
[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]
> Shouldn't the "From" line have Ben's authorship, too?
I considered the changes to this patch significant enough to take over
ownership. Even more so with the changes you suggested.
> > @Uli, Geert: Since bias support is probably similar, what do you think? Does
> > it make sense to add gen{2,3}.c?
>
> It's up to you.
> We can always consolidate later.
Then let's start like this and see later if consolidation is really needed.
> > @@ -4691,6 +4696,53 @@ static const char * const vin3_groups[] = {
> > "vin3_clk",
> > };
> >
> > +#define IOCTRL6 0xe606008c
>
> ... if you use 0x8c here...
>
> > +static int r8a7790_get_io_voltage(struct sh_pfc *pfc, unsigned int pin)
> > +{
> > + void __iomem *reg;
> > + u32 data, mask;
> > +
> > + if (WARN(pin < RCAR_GP_PIN(3, 0) || pin > RCAR_GP_PIN(3, 31), "invalid pin %#x", pin))
> > + return -EINVAL;
> > +
> > + reg = sh_pfc_phys_to_virt(pfc, IOCTRL6);
>
> ... and use "pfc->windows->virt + IOCTRL6" here and in
> r8a7790_set_io_voltage(), cfr. r8a7778_pinmux_get_bias().
I see. I thought sh_pfc_phys_to_virt was more correct because it
iterates over all possible windows. But yeah, we can probably live with
the simpler approach on Gen2/3. Will change.
Thanks for the review,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 1/7] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI
2016-02-29 10:53 ` Geert Uytterhoeven
2016-02-29 11:37 ` Wolfram Sang
@ 2016-03-01 13:42 ` Wolfram Sang
1 sibling, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2016-03-01 13:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-renesas-soc, Linux MMC List, Ben Hutchings, Dirk Behme,
Wolfram Sang, Ulrich Hecht
[-- Attachment #1: Type: text/plain, Size: 381 bytes --]
> ... and use "pfc->windows->virt + IOCTRL6" here and in
> r8a7790_set_io_voltage(), cfr. r8a7778_pinmux_get_bias().
This would mean I need to do something like this for unlocking:
iowrite32(~data, pfc->windows->virt); /* unlock reg */
I hope this is okay, too.
Note: The new fast SDXC card I got produces timeouts. I want to
investigate first before resending this series.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC 2/7] mmc: tmio, sh_mobile_sdhi: Pass tmio_mmc_host ptr to clk_{enable,disable} ops
2016-02-19 20:16 [RFC 0/7] r8a7790: add UHS-I (SDR50) support to Lager Wolfram Sang
2016-02-19 20:16 ` [RFC 1/7] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI Wolfram Sang
@ 2016-02-19 20:16 ` Wolfram Sang
2016-02-19 20:16 ` [RFC 3/7] mmc: tmio, sh_mobile_sdhi: Add support for variable input clock frequency Wolfram Sang
` (4 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2016-02-19 20:16 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: linux-mmc, Ben Hutchings, Dirk Behme, Wolfram Sang
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
Change the clk_enable operation to take a pointer to the struct
tmio_mmc_host and have it set f_max. For consistency, also change the
clk_disable operation to take a pointer to struct tmio_mmc_host.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/sh_mobile_sdhi.c | 12 +++++-------
drivers/mmc/host/tmio_mmc.h | 4 ++--
drivers/mmc/host/tmio_mmc_pio.c | 4 ++--
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index f7eff5f53e0013..675562653fdbe6 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -131,16 +131,15 @@ static void sh_mobile_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
sd_ctrl_write16(host, EXT_ACC, val);
}
-static int sh_mobile_sdhi_clk_enable(struct platform_device *pdev, unsigned int *f)
+static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host)
{
- struct mmc_host *mmc = platform_get_drvdata(pdev);
- struct tmio_mmc_host *host = mmc_priv(mmc);
+ struct mmc_host *mmc = host->mmc;
struct sh_mobile_sdhi *priv = host_to_priv(host);
int ret = clk_prepare_enable(priv->clk);
if (ret < 0)
return ret;
- *f = clk_get_rate(priv->clk);
+ mmc->f_max = clk_get_rate(priv->clk);
/* enable 16bit data access on SDBUF as default */
sh_mobile_sdhi_sdbuf_width(host, 16);
@@ -148,11 +147,10 @@ static int sh_mobile_sdhi_clk_enable(struct platform_device *pdev, unsigned int
return 0;
}
-static void sh_mobile_sdhi_clk_disable(struct platform_device *pdev)
+static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
{
- struct mmc_host *mmc = platform_get_drvdata(pdev);
- struct tmio_mmc_host *host = mmc_priv(mmc);
struct sh_mobile_sdhi *priv = host_to_priv(host);
+
clk_disable_unprepare(priv->clk);
}
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 4a597f5a53e20f..68fd8d791358c1 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -95,8 +95,8 @@ struct tmio_mmc_host {
bool sdio_irq_enabled;
int (*write16_hook)(struct tmio_mmc_host *host, int addr);
- int (*clk_enable)(struct platform_device *pdev, unsigned int *f);
- void (*clk_disable)(struct platform_device *pdev);
+ int (*clk_enable)(struct tmio_mmc_host *host);
+ void (*clk_disable)(struct tmio_mmc_host *host);
int (*multi_io_quirk)(struct mmc_card *card,
unsigned int direction, int blk_size);
};
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 03f6e74c190691..d1160156678ade 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -845,7 +845,7 @@ static int tmio_mmc_clk_update(struct tmio_mmc_host *host)
if (!host->clk_enable)
return -ENOTSUPP;
- ret = host->clk_enable(host->pdev, &mmc->f_max);
+ ret = host->clk_enable(host);
if (!ret)
mmc->f_min = mmc->f_max / 512;
@@ -1251,7 +1251,7 @@ int tmio_mmc_host_runtime_suspend(struct device *dev)
tmio_mmc_clk_stop(host);
if (host->clk_disable)
- host->clk_disable(host->pdev);
+ host->clk_disable(host);
return 0;
}
--
2.7.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [RFC 3/7] mmc: tmio, sh_mobile_sdhi: Add support for variable input clock frequency
2016-02-19 20:16 [RFC 0/7] r8a7790: add UHS-I (SDR50) support to Lager Wolfram Sang
2016-02-19 20:16 ` [RFC 1/7] pinctrl: sh-pfc: r8a7790: Implement voltage switching for SDHI Wolfram Sang
2016-02-19 20:16 ` [RFC 2/7] mmc: tmio, sh_mobile_sdhi: Pass tmio_mmc_host ptr to clk_{enable,disable} ops Wolfram Sang
@ 2016-02-19 20:16 ` Wolfram Sang
2016-02-19 20:16 ` [RFC 4/7] mmc: tmio: Add UHS-I mode support Wolfram Sang
` (3 subsequent siblings)
6 siblings, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2016-02-19 20:16 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: linux-mmc, Ben Hutchings, Dirk Behme, Wolfram Sang
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
Currently tmio_mmc assumes that the input clock frequency is fixed and
only its own clock divider can be changed. This is not true in the
case of sh_mobile_sdhi; we can use the clock API to change it.
In tmio_mmc:
- Delegate setting of f_min from tmio to the clk_enable operation (if
implemented), as it can be smaller than f_max / 512
- Add an optional clk_update operation called from tmio_mmc_set_clock()
that updates the input clock frequency
- Rename tmio_mmc_clk_update() to tmio_mmc_clk_enable(), to avoid
confusion with the clk_update operation
In sh_mobile_sdhi:
- Make the setting of f_max conditional; it should be set through the
max-frequency property in the device tree in future
- Set f_min based on the input clock's minimum frequency
- Implement the clk_update operation, selecting the best input clock
frequency for the bus frequency that's wanted
sh_mobile_sdhi_clk_update() is loosely based on Kuninori Morimoto's work
in sh_mmcif.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/sh_mobile_sdhi.c | 56 +++++++++++++++++++++++++++++++++++++--
drivers/mmc/host/tmio_mmc.h | 2 ++
drivers/mmc/host/tmio_mmc_pio.c | 24 +++++++----------
3 files changed, 66 insertions(+), 16 deletions(-)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 675562653fdbe6..4e2ac0e5e5d415 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -139,7 +139,20 @@ static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host)
if (ret < 0)
return ret;
- mmc->f_max = clk_get_rate(priv->clk);
+ /*
+ * The clock driver may not know what maximum frequency
+ * actually works, so it should be set with the max-frequency
+ * property which will already have been read to f_max. If it
+ * was missing, assume the current frequency is the maximum.
+ */
+ if (!mmc->f_max)
+ mmc->f_max = clk_get_rate(priv->clk);
+
+ /*
+ * Minimum frequency is the minimum input clock frequency
+ * divided by our maximum divider.
+ */
+ mmc->f_min = max(clk_round_rate(priv->clk, 1) / 512, 1L);
/* enable 16bit data access on SDBUF as default */
sh_mobile_sdhi_sdbuf_width(host, 16);
@@ -147,6 +160,44 @@ static int sh_mobile_sdhi_clk_enable(struct tmio_mmc_host *host)
return 0;
}
+static unsigned int sh_mobile_sdhi_clk_update(struct tmio_mmc_host *host,
+ unsigned int new_clock)
+{
+ struct sh_mobile_sdhi *priv = host_to_priv(host);
+ unsigned int freq, best_freq, diff_min, diff;
+ int i;
+
+ diff_min = ~0;
+ best_freq = 0;
+
+ /*
+ * We want the bus clock to be as close as possible to, but no
+ * greater than, new_clock. As we can divide by 1 << i for
+ * any i in [0, 9] we want the input clock to be as close as
+ * possible, but no greater than, new_clock << i.
+ */
+ for (i = min(9, ilog2(UINT_MAX / new_clock)); i >= 0; i--) {
+ freq = clk_round_rate(priv->clk, new_clock << i);
+ if (freq > (new_clock << i)) {
+ /* Too fast; look for a slightly slower option */
+ freq = clk_round_rate(priv->clk,
+ (new_clock << i) / 4 * 3);
+ if (freq > (new_clock << i))
+ continue;
+ }
+
+ diff = new_clock - (freq >> i);
+ if (diff <= diff_min) {
+ best_freq = freq;
+ diff_min = diff;
+ }
+ }
+
+ clk_set_rate(priv->clk, best_freq);
+
+ return best_freq;
+}
+
static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
{
struct sh_mobile_sdhi *priv = host_to_priv(host);
@@ -265,6 +316,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
host->dma = dma_priv;
host->write16_hook = sh_mobile_sdhi_write16_hook;
host->clk_enable = sh_mobile_sdhi_clk_enable;
+ host->clk_update = sh_mobile_sdhi_clk_update;
host->clk_disable = sh_mobile_sdhi_clk_disable;
host->multi_io_quirk = sh_mobile_sdhi_multi_io_quirk;
@@ -362,7 +414,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
}
}
- dev_info(&pdev->dev, "%s base at 0x%08lx clock rate %u MHz\n",
+ dev_info(&pdev->dev, "%s base at 0x%08lx max clock rate %u MHz\n",
mmc_hostname(host->mmc), (unsigned long)
(platform_get_resource(pdev, IORESOURCE_MEM, 0)->start),
host->mmc->f_max / 1000000);
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 68fd8d791358c1..b44b5890290622 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -96,6 +96,8 @@ struct tmio_mmc_host {
int (*write16_hook)(struct tmio_mmc_host *host, int addr);
int (*clk_enable)(struct tmio_mmc_host *host);
+ unsigned int (*clk_update)(struct tmio_mmc_host *host,
+ unsigned int new_clock);
void (*clk_disable)(struct tmio_mmc_host *host);
int (*multi_io_quirk)(struct mmc_card *card,
unsigned int direction, int blk_size);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index d1160156678ade..ae81b34f17a5a5 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -160,9 +160,12 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host,
u32 clk = 0, clock;
if (new_clock) {
- for (clock = host->mmc->f_min, clk = 0x80000080;
- new_clock >= (clock << 1);
- clk >>= 1)
+ if (host->clk_update)
+ clock = host->clk_update(host, new_clock) / 512;
+ else
+ clock = host->mmc->f_min;
+
+ for (clk = 0x80000080; new_clock >= (clock << 1); clk >>= 1)
clock <<= 1;
/* 1/1 clock is option */
@@ -837,19 +840,12 @@ fail:
pm_runtime_put_autosuspend(mmc_dev(mmc));
}
-static int tmio_mmc_clk_update(struct tmio_mmc_host *host)
+static int tmio_mmc_clk_enable(struct tmio_mmc_host *host)
{
- struct mmc_host *mmc = host->mmc;
- int ret;
-
if (!host->clk_enable)
return -ENOTSUPP;
- ret = host->clk_enable(host);
- if (!ret)
- mmc->f_min = mmc->f_max / 512;
-
- return ret;
+ return host->clk_enable(host);
}
static void tmio_mmc_power_on(struct tmio_mmc_host *host, unsigned short vdd)
@@ -1135,7 +1131,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
mmc->caps & MMC_CAP_NONREMOVABLE ||
mmc->slot.cd_irq >= 0);
- if (tmio_mmc_clk_update(_host) < 0) {
+ if (tmio_mmc_clk_enable(_host) < 0) {
mmc->f_max = pdata->hclk;
mmc->f_min = mmc->f_max / 512;
}
@@ -1263,7 +1259,7 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
struct tmio_mmc_host *host = mmc_priv(mmc);
tmio_mmc_reset(host);
- tmio_mmc_clk_update(host);
+ tmio_mmc_clk_enable(host);
if (host->clk_cache) {
tmio_mmc_set_clock(host, host->clk_cache);
--
2.7.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [RFC 4/7] mmc: tmio: Add UHS-I mode support
2016-02-19 20:16 [RFC 0/7] r8a7790: add UHS-I (SDR50) support to Lager Wolfram Sang
` (2 preceding siblings ...)
2016-02-19 20:16 ` [RFC 3/7] mmc: tmio, sh_mobile_sdhi: Add support for variable input clock frequency Wolfram Sang
@ 2016-02-19 20:16 ` Wolfram Sang
2016-03-03 14:41 ` Ulf Hansson
2016-02-19 20:16 ` [RFC 5/7] mmc: sh_mobile_sdhi: " Wolfram Sang
` (2 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2016-02-19 20:16 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: linux-mmc, Ben Hutchings, Dirk Behme, Wolfram Sang
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
Based on work by Shinobu Uehara and Ben Dooks. This adds the voltage
switch operation needed for all UHS-I modes, but not the tuning needed
for SDR-104 which will come later.
The card_busy implementation is a bit of a guess, but works for me on
an R8A7790 chip.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/tmio_mmc.h | 3 +++
drivers/mmc/host/tmio_mmc_pio.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index b44b5890290622..aabd36955e73fb 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -101,6 +101,9 @@ struct tmio_mmc_host {
void (*clk_disable)(struct tmio_mmc_host *host);
int (*multi_io_quirk)(struct mmc_card *card,
unsigned int direction, int blk_size);
+
+ int (*start_signal_voltage_switch)(struct tmio_mmc_host *host,
+ unsigned char signal_voltage);
};
struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index ae81b34f17a5a5..081a3def9305c1 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1012,12 +1012,43 @@ static int tmio_multi_io_quirk(struct mmc_card *card,
return blk_size;
}
+static int tmio_mmc_start_signal_voltage_switch(struct mmc_host *mmc,
+ struct mmc_ios *ios)
+{
+ struct tmio_mmc_host *host = mmc_priv(mmc);
+
+ if (!host->start_signal_voltage_switch)
+ return 0;
+
+ return host->start_signal_voltage_switch(host, ios->signal_voltage);
+}
+
+static int tmio_mmc_card_busy(struct mmc_host *mmc)
+{
+ struct tmio_mmc_host *host = mmc_priv(mmc);
+ u32 status;
+
+ pm_runtime_get_sync(mmc_dev(mmc));
+ status = sd_ctrl_read32(host, CTL_STATUS);
+ pm_runtime_mark_last_busy(mmc_dev(mmc));
+ pm_runtime_put_autosuspend(mmc_dev(mmc));
+
+ /*
+ * Card signals busy by pulling down all of DAT[3:0]. This status
+ * flag appears to reflect DAT3.
+ */
+ return !(status & TMIO_STAT_SIGSTATE_A);
+}
+
static const struct mmc_host_ops tmio_mmc_ops = {
.request = tmio_mmc_request,
.set_ios = tmio_mmc_set_ios,
.get_ro = tmio_mmc_get_ro,
.get_cd = mmc_gpio_get_cd,
.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
+ .start_signal_voltage_switch
+ = tmio_mmc_start_signal_voltage_switch,
+ .card_busy = tmio_mmc_card_busy,
.multi_io_quirk = tmio_multi_io_quirk,
};
--
2.7.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RFC 4/7] mmc: tmio: Add UHS-I mode support
2016-02-19 20:16 ` [RFC 4/7] mmc: tmio: Add UHS-I mode support Wolfram Sang
@ 2016-03-03 14:41 ` Ulf Hansson
2016-03-06 15:25 ` Wolfram Sang
2016-03-09 16:17 ` Wolfram Sang
0 siblings, 2 replies; 18+ messages in thread
From: Ulf Hansson @ 2016-03-03 14:41 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-renesas-soc, linux-mmc, Ben Hutchings, Dirk Behme,
Wolfram Sang
On 19 February 2016 at 21:16, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Ben Hutchings <ben.hutchings@codethink.co.uk>
>
> Based on work by Shinobu Uehara and Ben Dooks. This adds the voltage
> switch operation needed for all UHS-I modes, but not the tuning needed
> for SDR-104 which will come later.
>
> The card_busy implementation is a bit of a guess, but works for me on
> an R8A7790 chip.
>
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/mmc/host/tmio_mmc.h | 3 +++
> drivers/mmc/host/tmio_mmc_pio.c | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index b44b5890290622..aabd36955e73fb 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -101,6 +101,9 @@ struct tmio_mmc_host {
> void (*clk_disable)(struct tmio_mmc_host *host);
> int (*multi_io_quirk)(struct mmc_card *card,
> unsigned int direction, int blk_size);
> +
> + int (*start_signal_voltage_switch)(struct tmio_mmc_host *host,
> + unsigned char signal_voltage);
Do you really need to add a new tmio specific callback for this?
Why can't you instead have the tmio variant driver assign the
->start_signal_voltage_switch() callback into the struct mmc_host_ops
instead?
> };
>
> struct tmio_mmc_host *tmio_mmc_host_alloc(struct platform_device *pdev);
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index ae81b34f17a5a5..081a3def9305c1 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -1012,12 +1012,43 @@ static int tmio_multi_io_quirk(struct mmc_card *card,
> return blk_size;
> }
>
> +static int tmio_mmc_start_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + struct tmio_mmc_host *host = mmc_priv(mmc);
> +
> + if (!host->start_signal_voltage_switch)
> + return 0;
> +
> + return host->start_signal_voltage_switch(host, ios->signal_voltage);
> +}
> +
> +static int tmio_mmc_card_busy(struct mmc_host *mmc)
> +{
> + struct tmio_mmc_host *host = mmc_priv(mmc);
> + u32 status;
> +
> + pm_runtime_get_sync(mmc_dev(mmc));
This isn't needed as the mmc core already deal with runtime PM of the
host device via mmc_claim|release_host().
> + status = sd_ctrl_read32(host, CTL_STATUS);
> + pm_runtime_mark_last_busy(mmc_dev(mmc));
> + pm_runtime_put_autosuspend(mmc_dev(mmc));
Ditto.
> +
> + /*
> + * Card signals busy by pulling down all of DAT[3:0]. This status
> + * flag appears to reflect DAT3.
> + */
> + return !(status & TMIO_STAT_SIGSTATE_A);
> +}
> +
> static const struct mmc_host_ops tmio_mmc_ops = {
> .request = tmio_mmc_request,
> .set_ios = tmio_mmc_set_ios,
> .get_ro = tmio_mmc_get_ro,
> .get_cd = mmc_gpio_get_cd,
> .enable_sdio_irq = tmio_mmc_enable_sdio_irq,
> + .start_signal_voltage_switch
> + = tmio_mmc_start_signal_voltage_switch,
> + .card_busy = tmio_mmc_card_busy,
> .multi_io_quirk = tmio_multi_io_quirk,
> };
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 4/7] mmc: tmio: Add UHS-I mode support
2016-03-03 14:41 ` Ulf Hansson
@ 2016-03-06 15:25 ` Wolfram Sang
2016-03-09 16:17 ` Wolfram Sang
1 sibling, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2016-03-06 15:25 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-renesas-soc, linux-mmc, Ben Hutchings, Dirk Behme,
Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 1843 bytes --]
On Thu, Mar 03, 2016 at 03:41:13PM +0100, Ulf Hansson wrote:
> On 19 February 2016 at 21:16, Wolfram Sang <wsa@the-dreams.de> wrote:
> > From: Ben Hutchings <ben.hutchings@codethink.co.uk>
> >
> > Based on work by Shinobu Uehara and Ben Dooks. This adds the voltage
> > switch operation needed for all UHS-I modes, but not the tuning needed
> > for SDR-104 which will come later.
> >
> > The card_busy implementation is a bit of a guess, but works for me on
> > an R8A7790 chip.
> >
> > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> > drivers/mmc/host/tmio_mmc.h | 3 +++
> > drivers/mmc/host/tmio_mmc_pio.c | 31 +++++++++++++++++++++++++++++++
> > 2 files changed, 34 insertions(+)
> >
> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> > index b44b5890290622..aabd36955e73fb 100644
> > --- a/drivers/mmc/host/tmio_mmc.h
> > +++ b/drivers/mmc/host/tmio_mmc.h
> > @@ -101,6 +101,9 @@ struct tmio_mmc_host {
> > void (*clk_disable)(struct tmio_mmc_host *host);
> > int (*multi_io_quirk)(struct mmc_card *card,
> > unsigned int direction, int blk_size);
> > +
> > + int (*start_signal_voltage_switch)(struct tmio_mmc_host *host,
> > + unsigned char signal_voltage);
>
> Do you really need to add a new tmio specific callback for this?
>
> Why can't you instead have the tmio variant driver assign the
> ->start_signal_voltage_switch() callback into the struct mmc_host_ops
> instead?
Can do if you prefer that. I agree that it will save a bit of code; but
it also hides this feature, because all the other config is in struct
tmio_mmc_host. I like the better visibility a tad more.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 4/7] mmc: tmio: Add UHS-I mode support
2016-03-03 14:41 ` Ulf Hansson
2016-03-06 15:25 ` Wolfram Sang
@ 2016-03-09 16:17 ` Wolfram Sang
2016-03-16 8:58 ` Ulf Hansson
1 sibling, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2016-03-09 16:17 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-renesas-soc, linux-mmc, Ben Hutchings, Dirk Behme,
Wolfram Sang
[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]
> > +
> > + int (*start_signal_voltage_switch)(struct tmio_mmc_host *host,
> > + unsigned char signal_voltage);
>
> Do you really need to add a new tmio specific callback for this?
>
> Why can't you instead have the tmio variant driver assign the
> ->start_signal_voltage_switch() callback into the struct mmc_host_ops
> instead?
Just to see how it looks, I tried your suggestion. However, mmc_ops is
consted all the way into the core (rightfully, I'd say), so this makes
this approach quite clumsy. I kept the above callback now, just changed
the second paramater to pass the whole ios so I can use it with
mmc_regulator_set_vqmmc().
> > +static int tmio_mmc_card_busy(struct mmc_host *mmc)
> > +{
> > + struct tmio_mmc_host *host = mmc_priv(mmc);
> > + u32 status;
> > +
> > + pm_runtime_get_sync(mmc_dev(mmc));
>
> This isn't needed as the mmc core already deal with runtime PM of the
> host device via mmc_claim|release_host().
sdhci.c does it, too - missing cleanup?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 4/7] mmc: tmio: Add UHS-I mode support
2016-03-09 16:17 ` Wolfram Sang
@ 2016-03-16 8:58 ` Ulf Hansson
2016-03-22 17:21 ` Wolfram Sang
0 siblings, 1 reply; 18+ messages in thread
From: Ulf Hansson @ 2016-03-16 8:58 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-renesas-soc, linux-mmc, Ben Hutchings, Dirk Behme,
Wolfram Sang
On 9 March 2016 at 17:17, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> > +
>> > + int (*start_signal_voltage_switch)(struct tmio_mmc_host *host,
>> > + unsigned char signal_voltage);
>>
>> Do you really need to add a new tmio specific callback for this?
>>
>> Why can't you instead have the tmio variant driver assign the
>> ->start_signal_voltage_switch() callback into the struct mmc_host_ops
>> instead?
>
> Just to see how it looks, I tried your suggestion. However, mmc_ops is
> consted all the way into the core (rightfully, I'd say), so this makes
> this approach quite clumsy. I kept the above callback now, just changed
> the second paramater to pass the whole ios so I can use it with
> mmc_regulator_set_vqmmc().
Okay, let me elaborate a bit more on what I really meant.
In tmio_mmc_of_parse(), the mmc->ops gets assigned to the
&tmio_mmc_ops. Before doing that, you can conditionally assign
->start_signal_voltage_switch() callback within the &tmio_mmc_ops,
right!?
Why am I suggesting this? I want to avoid us evolving the code in the
wrong direction, similar what has happened to SDHCI. *One* way to
address this, is to minimize unnecessary variant specific callbacks.
>
>> > +static int tmio_mmc_card_busy(struct mmc_host *mmc)
>> > +{
>> > + struct tmio_mmc_host *host = mmc_priv(mmc);
>> > + u32 status;
>> > +
>> > + pm_runtime_get_sync(mmc_dev(mmc));
>>
>> This isn't needed as the mmc core already deal with runtime PM of the
>> host device via mmc_claim|release_host().
>
> sdhci.c does it, too - missing cleanup?
We quite recently added this into the mmc core, before it was the
controller driver itself who dealt with pm_runtime_get|put().
So, yes several drivers can be cleaned up.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [RFC 4/7] mmc: tmio: Add UHS-I mode support
2016-03-16 8:58 ` Ulf Hansson
@ 2016-03-22 17:21 ` Wolfram Sang
0 siblings, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2016-03-22 17:21 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-renesas-soc, linux-mmc, Ben Hutchings, Dirk Behme,
Wolfram Sang
Hi Ulf,
> Okay, let me elaborate a bit more on what I really meant.
>
> In tmio_mmc_of_parse(), the mmc->ops gets assigned to the
> &tmio_mmc_ops. Before doing that, you can conditionally assign
> ->start_signal_voltage_switch() callback within the &tmio_mmc_ops,
> right!?
So, lets talk with code. Do you mean something like this on top of my
series? (Compile tested only to see if I got you right):
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 5d2a852f17eebd..89e0c5e9fcc668 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -210,9 +210,10 @@ static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
clk_disable_unprepare(priv->clk);
}
-static int sh_mobile_sdhi_start_signal_voltage_switch(struct tmio_mmc_host *host,
+static int sh_mobile_sdhi_start_signal_voltage_switch(struct mmc_host *mmc,
struct mmc_ios *ios)
{
+ struct tmio_mmc_host *host = mmc_priv(mmc);
struct sh_mobile_sdhi *priv = host_to_priv(host);
struct pinctrl_state *pin_state;
int ret;
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 759bf015d09b84..b1819c74965b47 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -101,7 +101,7 @@ struct tmio_mmc_host {
void (*clk_disable)(struct tmio_mmc_host *host);
int (*multi_io_quirk)(struct mmc_card *card,
unsigned int direction, int blk_size);
- int (*start_signal_voltage_switch)(struct tmio_mmc_host *host,
+ int (*start_signal_voltage_switch)(struct mmc_host *mmc,
struct mmc_ios *ios);
};
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 8dee637638c31e..8e2fef7c5e481c 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1014,16 +1014,6 @@ static int tmio_multi_io_quirk(struct mmc_card *card,
return blk_size;
}
-static int tmio_mmc_start_signal_voltage_switch(struct mmc_host *mmc, struct mmc_ios *ios)
-{
- struct tmio_mmc_host *host = mmc_priv(mmc);
-
- if (!host->start_signal_voltage_switch)
- return 0;
-
- return host->start_signal_voltage_switch(host, ios);
-}
-
static int tmio_mmc_card_busy(struct mmc_host *mmc)
{
struct tmio_mmc_host *host = mmc_priv(mmc);
@@ -1031,14 +1021,12 @@ static int tmio_mmc_card_busy(struct mmc_host *mmc)
return !(sd_ctrl_read32(host, CTL_STATUS2) & TMIO_STATUS2_DAT0);
}
-static const struct mmc_host_ops tmio_mmc_ops = {
+static struct mmc_host_ops tmio_mmc_ops = {
.request = tmio_mmc_request,
.set_ios = tmio_mmc_set_ios,
.get_ro = tmio_mmc_get_ro,
.get_cd = mmc_gpio_get_cd,
.enable_sdio_irq = tmio_mmc_enable_sdio_irq,
- .start_signal_voltage_switch
- = tmio_mmc_start_signal_voltage_switch,
.card_busy = tmio_mmc_card_busy,
.multi_io_quirk = tmio_multi_io_quirk,
};
@@ -1138,7 +1126,9 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host,
goto host_free;
}
+ tmio_mmc_ops.start_signal_voltage_switch = _host->start_signal_voltage_switch;
mmc->ops = &tmio_mmc_ops;
+
mmc->caps |= MMC_CAP_4_BIT_DATA | pdata->capabilities;
mmc->caps2 |= pdata->capabilities2;
mmc->max_segs = 32;
> Why am I suggesting this? I want to avoid us evolving the code in the
> wrong direction, similar what has happened to SDHCI. *One* way to
> address this, is to minimize unnecessary variant specific callbacks.
Well, I can't test the original tmio_mmc implementation, only the sdhi
variant. So, we need a variant specific start_signal_voltage_switch().
As I see it, the above patch reduces one layer of abstraction. This is
nice, but the variant specific callback is still there. or?
Or i still got you wrong, then sorry!
Thanks,
Wolfram
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [RFC 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support
2016-02-19 20:16 [RFC 0/7] r8a7790: add UHS-I (SDR50) support to Lager Wolfram Sang
` (3 preceding siblings ...)
2016-02-19 20:16 ` [RFC 4/7] mmc: tmio: Add UHS-I mode support Wolfram Sang
@ 2016-02-19 20:16 ` Wolfram Sang
2016-03-03 14:47 ` Ulf Hansson
2016-02-19 20:16 ` [RFC 6/7] ARM: shmobile: r8a7790: Set maximum frequencies for SDHI clocks Wolfram Sang
2016-02-19 20:16 ` [RFC 7/7] ARM: shmobile: r8a7790: lager: Enable UHS-I SDR-50 Wolfram Sang
6 siblings, 1 reply; 18+ messages in thread
From: Wolfram Sang @ 2016-02-19 20:16 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: linux-mmc, Ben Hutchings, Dirk Behme, Wolfram Sang
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
Implement voltage switch, supporting modes up to SDR-50.
Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/mmc/host/sh_mobile_sdhi.c | 57 +++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 4e2ac0e5e5d415..d044fed2736a15 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -32,6 +32,9 @@
#include <linux/mfd/tmio.h>
#include <linux/sh_dma.h>
#include <linux/delay.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/pinctrl-state.h>
+#include <linux/regulator/consumer.h>
#include "tmio_mmc.h"
@@ -97,6 +100,8 @@ struct sh_mobile_sdhi {
struct clk *clk;
struct tmio_mmc_data mmc_data;
struct tmio_mmc_dma dma_priv;
+ struct pinctrl *pinctrl;
+ struct pinctrl_state *pinctrl_3v3, *pinctrl_1v8;
};
static void sh_mobile_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
@@ -205,6 +210,49 @@ static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
clk_disable_unprepare(priv->clk);
}
+static int sh_mobile_sdhi_start_signal_voltage_switch(
+ struct tmio_mmc_host *host, unsigned char signal_voltage)
+{
+ struct sh_mobile_sdhi *priv = host_to_priv(host);
+ struct pinctrl_state *state;
+ int min_uV, max_uV;
+ int ret;
+
+ switch (signal_voltage) {
+ case MMC_SIGNAL_VOLTAGE_330:
+ min_uV = 2700000;
+ max_uV = 3600000;
+ state = priv->pinctrl_3v3;
+ break;
+ case MMC_SIGNAL_VOLTAGE_180:
+ min_uV = 1700000;
+ max_uV = 1950000;
+ state = priv->pinctrl_1v8;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /*
+ * If anything is missing, assume signal voltage is fixed at
+ * 3.3V and succeed/fail accordingly.
+ */
+ if (IS_ERR(host->mmc->supply.vqmmc) ||
+ IS_ERR(priv->pinctrl) || IS_ERR(state))
+ return signal_voltage == MMC_SIGNAL_VOLTAGE_330 ? 0 : -EINVAL;
+
+ ret = regulator_set_voltage(host->mmc->supply.vqmmc, min_uV, max_uV);
+ if (ret)
+ return ret;
+
+ ret = pinctrl_select_state(priv->pinctrl, state);
+ if (ret)
+ return ret;
+
+ usleep_range(5000, 5500);
+ return 0;
+}
+
static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
{
int timeout = 1000;
@@ -296,6 +344,14 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
goto eprobe;
}
+ priv->pinctrl = devm_pinctrl_get(&pdev->dev);
+ if (!IS_ERR(priv->pinctrl)) {
+ priv->pinctrl_3v3 = pinctrl_lookup_state(priv->pinctrl,
+ PINCTRL_STATE_DEFAULT);
+ priv->pinctrl_1v8 = pinctrl_lookup_state(priv->pinctrl,
+ "1v8");
+ }
+
host = tmio_mmc_host_alloc(pdev);
if (!host) {
ret = -ENOMEM;
@@ -319,6 +375,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
host->clk_update = sh_mobile_sdhi_clk_update;
host->clk_disable = sh_mobile_sdhi_clk_disable;
host->multi_io_quirk = sh_mobile_sdhi_multi_io_quirk;
+ host->start_signal_voltage_switch = sh_mobile_sdhi_start_signal_voltage_switch;
/* Orginally registers were 16 bit apart, could be 32 or 64 nowadays */
if (!host->bus_shift && resource_size(res) > 0x100) /* old way to determine the shift */
--
2.7.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [RFC 5/7] mmc: sh_mobile_sdhi: Add UHS-I mode support
2016-02-19 20:16 ` [RFC 5/7] mmc: sh_mobile_sdhi: " Wolfram Sang
@ 2016-03-03 14:47 ` Ulf Hansson
2016-03-09 16:20 ` Wolfram Sang
0 siblings, 1 reply; 18+ messages in thread
From: Ulf Hansson @ 2016-03-03 14:47 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-renesas-soc, linux-mmc, Ben Hutchings, Dirk Behme,
Wolfram Sang
On 19 February 2016 at 21:16, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Ben Hutchings <ben.hutchings@codethink.co.uk>
>
> Implement voltage switch, supporting modes up to SDR-50.
>
> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton.
>
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/mmc/host/sh_mobile_sdhi.c | 57 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
> index 4e2ac0e5e5d415..d044fed2736a15 100644
> --- a/drivers/mmc/host/sh_mobile_sdhi.c
> +++ b/drivers/mmc/host/sh_mobile_sdhi.c
> @@ -32,6 +32,9 @@
> #include <linux/mfd/tmio.h>
> #include <linux/sh_dma.h>
> #include <linux/delay.h>
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/pinctrl/pinctrl-state.h>
> +#include <linux/regulator/consumer.h>
>
> #include "tmio_mmc.h"
>
> @@ -97,6 +100,8 @@ struct sh_mobile_sdhi {
> struct clk *clk;
> struct tmio_mmc_data mmc_data;
> struct tmio_mmc_dma dma_priv;
> + struct pinctrl *pinctrl;
> + struct pinctrl_state *pinctrl_3v3, *pinctrl_1v8;
May I suggest the following names for the pin states instead?
pins_default
pins_uhs
The corresponding pinctrl state-names I prefer are these:
PINCTRL_STATE_DEFAULT and "state_uhs". These names are currently used by mtk-sd.
> };
>
> static void sh_mobile_sdhi_sdbuf_width(struct tmio_mmc_host *host, int width)
> @@ -205,6 +210,49 @@ static void sh_mobile_sdhi_clk_disable(struct tmio_mmc_host *host)
> clk_disable_unprepare(priv->clk);
> }
>
> +static int sh_mobile_sdhi_start_signal_voltage_switch(
> + struct tmio_mmc_host *host, unsigned char signal_voltage)
> +{
> + struct sh_mobile_sdhi *priv = host_to_priv(host);
> + struct pinctrl_state *state;
> + int min_uV, max_uV;
> + int ret;
> +
> + switch (signal_voltage) {
> + case MMC_SIGNAL_VOLTAGE_330:
> + min_uV = 2700000;
> + max_uV = 3600000;
> + state = priv->pinctrl_3v3;
> + break;
> + case MMC_SIGNAL_VOLTAGE_180:
> + min_uV = 1700000;
> + max_uV = 1950000;
> + state = priv->pinctrl_1v8;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + /*
> + * If anything is missing, assume signal voltage is fixed at
> + * 3.3V and succeed/fail accordingly.
> + */
> + if (IS_ERR(host->mmc->supply.vqmmc) ||
> + IS_ERR(priv->pinctrl) || IS_ERR(state))
> + return signal_voltage == MMC_SIGNAL_VOLTAGE_330 ? 0 : -EINVAL;
> +
> + ret = regulator_set_voltage(host->mmc->supply.vqmmc, min_uV, max_uV);
> + if (ret)
> + return ret;
You should make use of the mmc_regulator_set_vqmmc() API here instead.
> +
> + ret = pinctrl_select_state(priv->pinctrl, state);
> + if (ret)
> + return ret;
> +
> + usleep_range(5000, 5500);
> + return 0;
> +}
> +
> static int sh_mobile_sdhi_wait_idle(struct tmio_mmc_host *host)
> {
> int timeout = 1000;
> @@ -296,6 +344,14 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
> goto eprobe;
> }
>
> + priv->pinctrl = devm_pinctrl_get(&pdev->dev);
> + if (!IS_ERR(priv->pinctrl)) {
> + priv->pinctrl_3v3 = pinctrl_lookup_state(priv->pinctrl,
> + PINCTRL_STATE_DEFAULT);
> + priv->pinctrl_1v8 = pinctrl_lookup_state(priv->pinctrl,
> + "1v8");
I am missing a separate DT binding documentation patch related to the
above pinctrls.
> + }
> +
> host = tmio_mmc_host_alloc(pdev);
> if (!host) {
> ret = -ENOMEM;
> @@ -319,6 +375,7 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev)
> host->clk_update = sh_mobile_sdhi_clk_update;
> host->clk_disable = sh_mobile_sdhi_clk_disable;
> host->multi_io_quirk = sh_mobile_sdhi_multi_io_quirk;
> + host->start_signal_voltage_switch = sh_mobile_sdhi_start_signal_voltage_switch;
>
> /* Orginally registers were 16 bit apart, could be 32 or 64 nowadays */
> if (!host->bus_shift && resource_size(res) > 0x100) /* old way to determine the shift */
> --
> 2.7.0
>
Kind regards
Uffe
^ permalink raw reply [flat|nested] 18+ messages in thread
* [RFC 6/7] ARM: shmobile: r8a7790: Set maximum frequencies for SDHI clocks
2016-02-19 20:16 [RFC 0/7] r8a7790: add UHS-I (SDR50) support to Lager Wolfram Sang
` (4 preceding siblings ...)
2016-02-19 20:16 ` [RFC 5/7] mmc: sh_mobile_sdhi: " Wolfram Sang
@ 2016-02-19 20:16 ` Wolfram Sang
2016-02-19 20:16 ` [RFC 7/7] ARM: shmobile: r8a7790: lager: Enable UHS-I SDR-50 Wolfram Sang
6 siblings, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2016-02-19 20:16 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: linux-mmc, Ben Hutchings, Dirk Behme, Wolfram Sang
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
Taken from the datasheet.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
arch/arm/boot/dts/r8a7790.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index c9583fa6cae713..16cee3f02efb94 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -545,6 +545,7 @@
reg = <0 0xee100000 0 0x328>;
interrupts = <GIC_SPI 165 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
+ max-frequency = <156000000>;
dmas = <&dmac1 0xcd>, <&dmac1 0xce>;
dma-names = "tx", "rx";
power-domains = <&cpg_clocks>;
@@ -556,6 +557,7 @@
reg = <0 0xee120000 0 0x328>;
interrupts = <GIC_SPI 166 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&mstp3_clks R8A7790_CLK_SDHI1>;
+ max-frequency = <156000000>;
dmas = <&dmac1 0xc9>, <&dmac1 0xca>;
dma-names = "tx", "rx";
power-domains = <&cpg_clocks>;
@@ -567,6 +569,7 @@
reg = <0 0xee140000 0 0x100>;
interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
+ max-frequency = <97500000>;
dmas = <&dmac1 0xc1>, <&dmac1 0xc2>;
dma-names = "tx", "rx";
power-domains = <&cpg_clocks>;
@@ -578,6 +581,7 @@
reg = <0 0xee160000 0 0x100>;
interrupts = <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&mstp3_clks R8A7790_CLK_SDHI3>;
+ max-frequency = <97500000>;
dmas = <&dmac1 0xd3>, <&dmac1 0xd4>;
dma-names = "tx", "rx";
power-domains = <&cpg_clocks>;
--
2.7.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* [RFC 7/7] ARM: shmobile: r8a7790: lager: Enable UHS-I SDR-50
2016-02-19 20:16 [RFC 0/7] r8a7790: add UHS-I (SDR50) support to Lager Wolfram Sang
` (5 preceding siblings ...)
2016-02-19 20:16 ` [RFC 6/7] ARM: shmobile: r8a7790: Set maximum frequencies for SDHI clocks Wolfram Sang
@ 2016-02-19 20:16 ` Wolfram Sang
6 siblings, 0 replies; 18+ messages in thread
From: Wolfram Sang @ 2016-02-19 20:16 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: linux-mmc, Ben Hutchings, Dirk Behme, Wolfram Sang
From: Ben Hutchings <ben.hutchings@codethink.co.uk>
Add the "1v8" pinctrl state and sd-uhs-sdr50 property to SDHI{0,2}.
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
arch/arm/boot/dts/r8a7790-lager.dts | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index cdc0414f5f0716..08eef61ffad1af 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -327,11 +327,25 @@
sdhi0_pins: sd0 {
renesas,groups = "sdhi0_data4", "sdhi0_ctrl";
renesas,function = "sdhi0";
+ power-source = <3300>;
+ };
+
+ sdhi0_pins_1v8: sd0_1v8 {
+ renesas,groups = "sdhi0_data4", "sdhi0_ctrl";
+ renesas,function = "sdhi0";
+ power-source = <1800>;
};
sdhi2_pins: sd2 {
renesas,groups = "sdhi2_data4", "sdhi2_ctrl";
renesas,function = "sdhi2";
+ power-source = <3300>;
+ };
+
+ sdhi2_pins_1v8: sd2_1v8 {
+ renesas,groups = "sdhi2_data4", "sdhi2_ctrl";
+ renesas,function = "sdhi2";
+ power-source = <1800>;
};
mmc1_pins: mmc1 {
@@ -515,21 +529,25 @@
&sdhi0 {
pinctrl-0 = <&sdhi0_pins>;
- pinctrl-names = "default";
+ pinctrl-1 = <&sdhi0_pins_1v8>;
+ pinctrl-names = "default", "1v8";
vmmc-supply = <&vcc_sdhi0>;
vqmmc-supply = <&vccq_sdhi0>;
cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
+ sd-uhs-sdr50;
status = "okay";
};
&sdhi2 {
pinctrl-0 = <&sdhi2_pins>;
- pinctrl-names = "default";
+ pinctrl-1 = <&sdhi2_pins_1v8>;
+ pinctrl-names = "default", "1v8";
vmmc-supply = <&vcc_sdhi2>;
vqmmc-supply = <&vccq_sdhi2>;
cd-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;
+ sd-uhs-sdr50;
status = "okay";
};
--
2.7.0
^ permalink raw reply related [flat|nested] 18+ messages in thread