| Message ID | 1430397136.5802.44.camel@xylophone.i.decadent.org.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hello. On 04/30/2015 03:32 PM, Ben Hutchings wrote: > Implement voltage switch, supporting modes up to SDR-50. > Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. > This uses two voltage regulators, one external and one on the pfc. > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk> > --- > drivers/mmc/host/sh_mobile_sdhi.c | 48 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c > index 92a58c6007fe..c8538a256e22 100644 > --- a/drivers/mmc/host/sh_mobile_sdhi.c > +++ b/drivers/mmc/host/sh_mobile_sdhi.c [...] > @@ -148,6 +150,41 @@ static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int 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); > + int min_uV, max_uV; > + int ret; > + > + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) { > + min_uV = 2700000; > + max_uV = 3600000; > + } else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) { > + min_uV = 1700000; > + max_uV = 1950000; > + } else { > + return -EINVAL; > + } The above is asking to be a *switch* statement. [...] WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > Implement voltage switch, supporting modes up to SDR-50. > > Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. > > This uses two voltage regulators, one external and one on the pfc. Why two? If there is a parent child relation ship, that should be handled through the regulator tree, right!? Please elaborate. Kind regards Uffe > > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk> > --- > drivers/mmc/host/sh_mobile_sdhi.c | 48 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c > index 92a58c6007fe..c8538a256e22 100644 > --- a/drivers/mmc/host/sh_mobile_sdhi.c > +++ b/drivers/mmc/host/sh_mobile_sdhi.c > @@ -30,6 +30,7 @@ > #include <linux/mfd/tmio.h> > #include <linux/sh_dma.h> > #include <linux/delay.h> > +#include <linux/regulator/consumer.h> > > #include "tmio_mmc.h" > > @@ -84,6 +85,7 @@ MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match); > > struct sh_mobile_sdhi { > struct clk *clk; > + struct regulator *vqmmc_ref; > struct tmio_mmc_data mmc_data; > struct tmio_mmc_dma dma_priv; > }; > @@ -148,6 +150,41 @@ static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int 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); > + int min_uV, max_uV; > + int ret; > + > + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) { > + min_uV = 2700000; > + max_uV = 3600000; > + } else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) { > + min_uV = 1700000; > + max_uV = 1950000; > + } else { > + return -EINVAL; > + } > + > + if (!IS_ERR(host->mmc->supply.vqmmc)) { > + ret = regulator_set_voltage(host->mmc->supply.vqmmc, > + min_uV, max_uV); > + if (ret) > + return ret; > + } > + > + if (!IS_ERR(priv->vqmmc_ref)) { > + ret = regulator_set_voltage(priv->vqmmc_ref, > + min_uV, max_uV); > + 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; > @@ -240,6 +277,13 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev) > goto eprobe; > } > > + priv->vqmmc_ref = devm_regulator_get_optional(&pdev->dev, "vqmmc-ref"); > + if (IS_ERR(priv->vqmmc_ref)) { > + if (PTR_ERR(priv->vqmmc_ref) == -EPROBE_DEFER) > + return -EPROBE_DEFER; > + dev_info(&pdev->dev, "No vqmmc reference regulator found\n"); > + } > + > host = tmio_mmc_host_alloc(pdev); > if (!host) { > ret = -ENOMEM; > @@ -253,6 +297,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev) > host->clk_enable = sh_mobile_sdhi_clk_enable; > 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; > + > /* SD control register space size is 0x100, 0x200 for bus_shift=1 */ > if (resource_size(res) > 0x100) > host->bus_shift = 1; > -- > 1.7.10.4 > > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/05/15 10:56, Ulf Hansson wrote: > On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: >> Implement voltage switch, supporting modes up to SDR-50. >> >> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. >> >> This uses two voltage regulators, one external and one on the pfc. > > Why two? If there is a parent child relation ship, that should be > handled through the regulator tree, right!? Please elaborate. The card main power is separate from the IO line voltages. To get to the high-speed, card power is left at 3.3V and the IO voltage is changed to 1.8V. In the systems we have the power gate is separate from the controls for the IO but not integrated into the MMC controller itself.
On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > On 05/05/15 10:56, Ulf Hansson wrote: >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: >>> Implement voltage switch, supporting modes up to SDR-50. >>> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. >>> >>> This uses two voltage regulators, one external and one on the pfc. >> >> Why two? If there is a parent child relation ship, that should be >> handled through the regulator tree, right!? Please elaborate. > > The card main power is separate from the IO line voltages. > > To get to the high-speed, card power is left at 3.3V and the IO > voltage is changed to 1.8V. > > In the systems we have the power gate is separate from the controls > for the IO but not integrated into the MMC controller itself. Okay, that's what I was expecting and hoping for :-) Then you need to rework $subject patch according to below. 1) Use mmc_regulator_get_supply() to fetch both the I/O voltage regulator (vqmmc) and the main power regulator (vmmc).Your "vqmmc_ref" regulator should thus be renamed to "vmmc". 2) The vmmc regulator should not be handled from the ->start_signal_voltage_switch() callback, since it's only vqmmc voltage levels that should be changed from there. 3) The voltage levels changes for vmmc shall be handled via the ->set_ios() callback. I suggest you go and have a look in drivers/mmc/host/mmci.c, that should provide you with a good inspiration. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote: > On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > > On 05/05/15 10:56, Ulf Hansson wrote: > >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > >>> Implement voltage switch, supporting modes up to SDR-50. > >>> > >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. > >>> > >>> This uses two voltage regulators, one external and one on the pfc. > >> > >> Why two? If there is a parent child relation ship, that should be > >> handled through the regulator tree, right!? Please elaborate. > > > > The card main power is separate from the IO line voltages. > > > > To get to the high-speed, card power is left at 3.3V and the IO > > voltage is changed to 1.8V. > > > > In the systems we have the power gate is separate from the controls > > for the IO but not integrated into the MMC controller itself. In this case, there are *three* regulators: 1. External regulator for card power (VDD pin): "vmmc" 2. External regulator for pull-up voltage for the data pins: "vqmmc" 3. Internal regulator for input(?) level on the data pins: "vqmmc_ref" (I'm open to suggestions of a better name) > Okay, that's what I was expecting and hoping for :-) > > Then you need to rework $subject patch according to below. > > 1) Use mmc_regulator_get_supply() to fetch both the I/O voltage > regulator (vqmmc) and the main power regulator (vmmc). We already do that. > Your "vqmmc_ref" regulator should thus be renamed to "vmmc". No, we have one of those already. > 2) The vmmc regulator should not be handled from the > ->start_signal_voltage_switch() callback, since it's only vqmmc > voltage levels that should be changed from there. It isn't. > 3) The voltage levels changes for vmmc shall be handled via the > ->set_ios() callback. We don't support UHS-II so we never change the voltage of this regulator. Ben. > I suggest you go and have a look in drivers/mmc/host/mmci.c, that > should provide you with a good inspiration. > > Kind regards > Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2015-04-30 at 19:04 +0300, Sergei Shtylyov wrote: > Hello. > > On 04/30/2015 03:32 PM, Ben Hutchings wrote: > > > Implement voltage switch, supporting modes up to SDR-50. > > > Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. > > > This uses two voltage regulators, one external and one on the pfc. > > > Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk> > > --- > > drivers/mmc/host/sh_mobile_sdhi.c | 48 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 48 insertions(+) > > > diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c > > index 92a58c6007fe..c8538a256e22 100644 > > --- a/drivers/mmc/host/sh_mobile_sdhi.c > > +++ b/drivers/mmc/host/sh_mobile_sdhi.c > [...] > > @@ -148,6 +150,41 @@ static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int 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); > > + int min_uV, max_uV; > > + int ret; > > + > > + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) { > > + min_uV = 2700000; > > + max_uV = 3600000; > > + } else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) { > > + min_uV = 1700000; > > + max_uV = 1950000; > > + } else { > > + return -EINVAL; > > + } > > The above is asking to be a *switch* statement. I suppose so, yes. Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote: >> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote: >> > On 05/05/15 10:56, Ulf Hansson wrote: >> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: >> >>> Implement voltage switch, supporting modes up to SDR-50. >> >>> >> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. >> >>> >> >>> This uses two voltage regulators, one external and one on the pfc. >> >> >> >> Why two? If there is a parent child relation ship, that should be >> >> handled through the regulator tree, right!? Please elaborate. >> > >> > The card main power is separate from the IO line voltages. >> > >> > To get to the high-speed, card power is left at 3.3V and the IO >> > voltage is changed to 1.8V. >> > >> > In the systems we have the power gate is separate from the controls >> > for the IO but not integrated into the MMC controller itself. > > In this case, there are *three* regulators: > > 1. External regulator for card power (VDD pin): "vmmc" > 2. External regulator for pull-up voltage for the data pins: "vqmmc" Is this really a regulator and not just about changing a pinctrl setting? The reason why I wonder, is because there are several other mmc host driver's the use a specific pinctrl state for this. > 3. Internal regulator for input(?) level on the data pins: > "vqmmc_ref" (I'm open to suggestions of a better name) > >> Okay, that's what I was expecting and hoping for :-) >> >> Then you need to rework $subject patch according to below. >> >> 1) Use mmc_regulator_get_supply() to fetch both the I/O voltage >> regulator (vqmmc) and the main power regulator (vmmc). > > We already do that. Okay, great! > >> Your "vqmmc_ref" regulator should thus be renamed to "vmmc". > > No, we have one of those already. > >> 2) The vmmc regulator should not be handled from the >> ->start_signal_voltage_switch() callback, since it's only vqmmc >> voltage levels that should be changed from there. > > It isn't. > >> 3) The voltage levels changes for vmmc shall be handled via the >> ->set_ios() callback. > > We don't support UHS-II so we never change the voltage of this > regulator. That's not related to UHS-II. Voltage level negotiation for vmmc is done even for legacy mode cards. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2015-05-06 at 10:44 +0200, Ulf Hansson wrote: > On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > > On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote: > >> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > >> > On 05/05/15 10:56, Ulf Hansson wrote: > >> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > >> >>> Implement voltage switch, supporting modes up to SDR-50. > >> >>> > >> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. > >> >>> > >> >>> This uses two voltage regulators, one external and one on the pfc. > >> >> > >> >> Why two? If there is a parent child relation ship, that should be > >> >> handled through the regulator tree, right!? Please elaborate. > >> > > >> > The card main power is separate from the IO line voltages. > >> > > >> > To get to the high-speed, card power is left at 3.3V and the IO > >> > voltage is changed to 1.8V. > >> > > >> > In the systems we have the power gate is separate from the controls > >> > for the IO but not integrated into the MMC controller itself. > > > > In this case, there are *three* regulators: > > > > 1. External regulator for card power (VDD pin): "vmmc" > > 2. External regulator for pull-up voltage for the data pins: "vqmmc" > > Is this really a regulator and not just about changing a pinctrl setting? Looking at the Lager board, the pull-up voltage appears to be controlled by the external PMIC (DA9063), which is signalled using GPIOs (I don't know why, when it's also connected to I2C). > The reason why I wonder, is because there are several other mmc host > driver's the use a specific pinctrl state for this. > > > 3. Internal regulator for input(?) level on the data pins: > > "vqmmc_ref" (I'm open to suggestions of a better name) This one is implemented in the pinctrl (pfc) block. [...] > >> 3) The voltage levels changes for vmmc shall be handled via the > >> ->set_ios() callback. > > > > We don't support UHS-II so we never change the voltage of this > > regulator. > > That's not related to UHS-II. Voltage level negotiation for vmmc is > done even for legacy mode cards. I'm looking at "SD Specifications, Part 1, Physical Layer Simplified Specifications, Version 4.10" and it seems clear from that, that only UHS-II cards have a variable supply voltage. Maybe that changed in a later version? Anyway, I don't have any board that can change the supply voltage. Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 6 May 2015 at 15:49, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > On Wed, 2015-05-06 at 10:44 +0200, Ulf Hansson wrote: >> On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: >> > On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote: >> >> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote: >> >> > On 05/05/15 10:56, Ulf Hansson wrote: >> >> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: >> >> >>> Implement voltage switch, supporting modes up to SDR-50. >> >> >>> >> >> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. >> >> >>> >> >> >>> This uses two voltage regulators, one external and one on the pfc. >> >> >> >> >> >> Why two? If there is a parent child relation ship, that should be >> >> >> handled through the regulator tree, right!? Please elaborate. >> >> > >> >> > The card main power is separate from the IO line voltages. >> >> > >> >> > To get to the high-speed, card power is left at 3.3V and the IO >> >> > voltage is changed to 1.8V. >> >> > >> >> > In the systems we have the power gate is separate from the controls >> >> > for the IO but not integrated into the MMC controller itself. >> > >> > In this case, there are *three* regulators: >> > >> > 1. External regulator for card power (VDD pin): "vmmc" >> > 2. External regulator for pull-up voltage for the data pins: "vqmmc" >> >> Is this really a regulator and not just about changing a pinctrl setting? > > Looking at the Lager board, the pull-up voltage appears to be controlled > by the external PMIC (DA9063), which is signalled using GPIOs (I don't > know why, when it's also connected to I2C). Okay, thus we should have a regulator for it. > >> The reason why I wonder, is because there are several other mmc host >> driver's the use a specific pinctrl state for this. >> >> > 3. Internal regulator for input(?) level on the data pins: >> > "vqmmc_ref" (I'm open to suggestions of a better name) > > This one is implemented in the pinctrl (pfc) block. I understand this as we should have a specific UHS pinctrl state, instead of a regulator. Right? [...] Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-05-11 at 10:54 +0200, Ulf Hansson wrote: > On 6 May 2015 at 15:49, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > > On Wed, 2015-05-06 at 10:44 +0200, Ulf Hansson wrote: > >> On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > >> > On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote: > >> >> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > >> >> > On 05/05/15 10:56, Ulf Hansson wrote: > >> >> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > >> >> >>> Implement voltage switch, supporting modes up to SDR-50. > >> >> >>> > >> >> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. > >> >> >>> > >> >> >>> This uses two voltage regulators, one external and one on the pfc. > >> >> >> > >> >> >> Why two? If there is a parent child relation ship, that should be > >> >> >> handled through the regulator tree, right!? Please elaborate. > >> >> > > >> >> > The card main power is separate from the IO line voltages. > >> >> > > >> >> > To get to the high-speed, card power is left at 3.3V and the IO > >> >> > voltage is changed to 1.8V. > >> >> > > >> >> > In the systems we have the power gate is separate from the controls > >> >> > for the IO but not integrated into the MMC controller itself. > >> > > >> > In this case, there are *three* regulators: > >> > > >> > 1. External regulator for card power (VDD pin): "vmmc" > >> > 2. External regulator for pull-up voltage for the data pins: "vqmmc" > >> > >> Is this really a regulator and not just about changing a pinctrl setting? > > > > Looking at the Lager board, the pull-up voltage appears to be controlled > > by the external PMIC (DA9063), which is signalled using GPIOs (I don't > > know why, when it's also connected to I2C). > > Okay, thus we should have a regulator for it. > > > > >> The reason why I wonder, is because there are several other mmc host > >> driver's the use a specific pinctrl state for this. > >> > >> > 3. Internal regulator for input(?) level on the data pins: > >> > "vqmmc_ref" (I'm open to suggestions of a better name) > > > > This one is implemented in the pinctrl (pfc) block. > > I understand this as we should have a specific UHS pinctrl state, > instead of a regulator. Right? That was another option I considered, but I didn't see anything in the pinctrl API that would allow changing the pin configuration dynamically. (And sh_mobile_sdhi doesn't explicitly do anything with pinctrl at the moment.) Now that you mention pinctrl states, I can see that might be a good way to handle this. Since the states are identified by strings, does that mean it's OK to add a driver-specific state such as "sh-pfc-1.8v" and not add it to the common pinctrl headers? Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 11 May 2015 at 16:01, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: > On Mon, 2015-05-11 at 10:54 +0200, Ulf Hansson wrote: >> On 6 May 2015 at 15:49, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: >> > On Wed, 2015-05-06 at 10:44 +0200, Ulf Hansson wrote: >> >> On 6 May 2015 at 03:38, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: >> >> > On Tue, 2015-05-05 at 10:47 +0200, Ulf Hansson wrote: >> >> >> On 5 May 2015 at 10:35, Ben Dooks <ben.dooks@codethink.co.uk> wrote: >> >> >> > On 05/05/15 10:56, Ulf Hansson wrote: >> >> >> >> On 30 April 2015 at 14:32, Ben Hutchings <ben.hutchings@codethink.co.uk> wrote: >> >> >> >>> Implement voltage switch, supporting modes up to SDR-50. >> >> >> >>> >> >> >> >>> Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. >> >> >> >>> >> >> >> >>> This uses two voltage regulators, one external and one on the pfc. >> >> >> >> >> >> >> >> Why two? If there is a parent child relation ship, that should be >> >> >> >> handled through the regulator tree, right!? Please elaborate. >> >> >> > >> >> >> > The card main power is separate from the IO line voltages. >> >> >> > >> >> >> > To get to the high-speed, card power is left at 3.3V and the IO >> >> >> > voltage is changed to 1.8V. >> >> >> > >> >> >> > In the systems we have the power gate is separate from the controls >> >> >> > for the IO but not integrated into the MMC controller itself. >> >> > >> >> > In this case, there are *three* regulators: >> >> > >> >> > 1. External regulator for card power (VDD pin): "vmmc" >> >> > 2. External regulator for pull-up voltage for the data pins: "vqmmc" >> >> >> >> Is this really a regulator and not just about changing a pinctrl setting? >> > >> > Looking at the Lager board, the pull-up voltage appears to be controlled >> > by the external PMIC (DA9063), which is signalled using GPIOs (I don't >> > know why, when it's also connected to I2C). >> >> Okay, thus we should have a regulator for it. >> >> > >> >> The reason why I wonder, is because there are several other mmc host >> >> driver's the use a specific pinctrl state for this. >> >> >> >> > 3. Internal regulator for input(?) level on the data pins: >> >> > "vqmmc_ref" (I'm open to suggestions of a better name) >> > >> > This one is implemented in the pinctrl (pfc) block. >> >> I understand this as we should have a specific UHS pinctrl state, >> instead of a regulator. Right? > > That was another option I considered, but I didn't see anything in the > pinctrl API that would allow changing the pin configuration dynamically. > (And sh_mobile_sdhi doesn't explicitly do anything with pinctrl at the > moment.) > > Now that you mention pinctrl states, I can see that might be a good way > to handle this. Since the states are identified by strings, does that > mean it's OK to add a driver-specific state such as "sh-pfc-1.8v" and > not add it to the common pinctrl headers? Yes, we already have other drivers doing it like that. Depending on what SoC this driver is used for, maybe you want this state to be optional though. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c index 92a58c6007fe..c8538a256e22 100644 --- a/drivers/mmc/host/sh_mobile_sdhi.c +++ b/drivers/mmc/host/sh_mobile_sdhi.c @@ -30,6 +30,7 @@ #include <linux/mfd/tmio.h> #include <linux/sh_dma.h> #include <linux/delay.h> +#include <linux/regulator/consumer.h> #include "tmio_mmc.h" @@ -84,6 +85,7 @@ MODULE_DEVICE_TABLE(of, sh_mobile_sdhi_of_match); struct sh_mobile_sdhi { struct clk *clk; + struct regulator *vqmmc_ref; struct tmio_mmc_data mmc_data; struct tmio_mmc_dma dma_priv; }; @@ -148,6 +150,41 @@ static void sh_mobile_sdhi_set_clk_div(struct platform_device *pdev, int 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); + int min_uV, max_uV; + int ret; + + if (signal_voltage == MMC_SIGNAL_VOLTAGE_330) { + min_uV = 2700000; + max_uV = 3600000; + } else if (signal_voltage == MMC_SIGNAL_VOLTAGE_180) { + min_uV = 1700000; + max_uV = 1950000; + } else { + return -EINVAL; + } + + if (!IS_ERR(host->mmc->supply.vqmmc)) { + ret = regulator_set_voltage(host->mmc->supply.vqmmc, + min_uV, max_uV); + if (ret) + return ret; + } + + if (!IS_ERR(priv->vqmmc_ref)) { + ret = regulator_set_voltage(priv->vqmmc_ref, + min_uV, max_uV); + 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; @@ -240,6 +277,13 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev) goto eprobe; } + priv->vqmmc_ref = devm_regulator_get_optional(&pdev->dev, "vqmmc-ref"); + if (IS_ERR(priv->vqmmc_ref)) { + if (PTR_ERR(priv->vqmmc_ref) == -EPROBE_DEFER) + return -EPROBE_DEFER; + dev_info(&pdev->dev, "No vqmmc reference regulator found\n"); + } + host = tmio_mmc_host_alloc(pdev); if (!host) { ret = -ENOMEM; @@ -253,6 +297,10 @@ static int sh_mobile_sdhi_probe(struct platform_device *pdev) host->clk_enable = sh_mobile_sdhi_clk_enable; 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; + /* SD control register space size is 0x100, 0x200 for bus_shift=1 */ if (resource_size(res) > 0x100) host->bus_shift = 1;
Implement voltage switch, supporting modes up to SDR-50. Based on work by Shinobu Uehara, Rob Taylor, William Towle and Ian Molton. This uses two voltage regulators, one external and one on the pfc. Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk> --- drivers/mmc/host/sh_mobile_sdhi.c | 48 +++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)