[PATCH 0/4] mmc: sdhci-acpi: Fix voltage switch for some Intel host controllers

[PATCH 0/4] mmc: sdhci-acpi: Fix voltage switch for some Intel host controllers

[Adrian Hunter]

Hi

Here are a few patches to fix voltage switching on some Intel host
controllers.


Adrian Hunter (4):
      mmc: sdhci-acpi: Use helper function acpi_device_uid()
      mmc: sdhci-acpi: Tidy Intel slot probe functions into one
      mmc: sdhci-acpi: Let devices define their own private data
      mmc: sdhci-acpi: Fix voltage switch for some Intel host controllers

 drivers/mmc/host/sdhci-acpi.c | 174 +++++++++++++++++++++++++++++++-----------
 1 file changed, 129 insertions(+), 45 deletions(-)


Regards
Adrian

[PATCH 1/4] mmc: sdhci-acpi: Use helper function acpi_device_uid()

[Adrian Hunter]

Make use of acpi_device_uid() instead of open coding.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 08ae0ff13513..e4b7fac5efb2 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -443,7 +443,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	hid = acpi_device_hid(device);
-	uid = device->pnp.unique_id;
+	uid = acpi_device_uid(device);
 
 	/* Power on the SDHCI controller and its children */
 	acpi_device_fix_up_power(device);
-- 
1.9.1

Re: [PATCH 1/4] mmc: sdhci-acpi: Use helper function acpi_device_uid()

[Ulf Hansson]

On 19 October 2017 at 12:41, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Make use of acpi_device_uid() instead of open coding.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index 08ae0ff13513..e4b7fac5efb2 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -443,7 +443,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>                 return -ENODEV;
>
>         hid = acpi_device_hid(device);
> -       uid = device->pnp.unique_id;
> +       uid = acpi_device_uid(device);
>
>         /* Power on the SDHCI controller and its children */
>         acpi_device_fix_up_power(device);
> --
> 1.9.1
>

[PATCH 2/4] mmc: sdhci-acpi: Tidy Intel slot probe functions into one

[Adrian Hunter]

Tidy Intel slot probe functions into one. A single function can be used
because the logic uses hid / uid as necessary to identify devices anyway.
This gets rid of some pointless comments and checks for variables that
cannot possibly be NULL, as well as giving the function a name that
identifies it as specific to Intel controllers.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-acpi.c | 48 ++++++-------------------------------------
 1 file changed, 6 insertions(+), 42 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index e4b7fac5efb2..640be5b618fc 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -269,53 +269,17 @@ static int bxt_get_cd(struct mmc_host *mmc)
 	return ret;
 }
 
-static int sdhci_acpi_emmc_probe_slot(struct platform_device *pdev,
-				      const char *hid, const char *uid)
+static int intel_probe_slot(struct platform_device *pdev, const char *hid,
+			    const char *uid)
 {
 	struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
-	struct sdhci_host *host;
-
-	if (!c || !c->host)
-		return 0;
-
-	host = c->host;
-
-	/* Platform specific code during emmc probe slot goes here */
+	struct sdhci_host *host = c->host;
 
 	if (hid && uid && !strcmp(hid, "80860F14") && !strcmp(uid, "1") &&
 	    sdhci_readl(host, SDHCI_CAPABILITIES) == 0x446cc8b2 &&
 	    sdhci_readl(host, SDHCI_CAPABILITIES_1) == 0x00000807)
 		host->timeout_clk = 1000; /* 1000 kHz i.e. 1 MHz */
 
-	return 0;
-}
-
-static int sdhci_acpi_sdio_probe_slot(struct platform_device *pdev,
-				      const char *hid, const char *uid)
-{
-	struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
-
-	if (!c || !c->host)
-		return 0;
-
-	/* Platform specific code during sdio probe slot goes here */
-
-	return 0;
-}
-
-static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
-				    const char *hid, const char *uid)
-{
-	struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
-	struct sdhci_host *host;
-
-	if (!c || !c->host || !c->slot)
-		return 0;
-
-	host = c->host;
-
-	/* Platform specific code during sd probe slot goes here */
-
 	if (hid && !strcmp(hid, "80865ACA"))
 		host->mmc_host_ops.get_cd = bxt_get_cd;
 
@@ -332,7 +296,7 @@ static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
 	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
 		   SDHCI_QUIRK2_STOP_WITH_TC |
 		   SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
-	.probe_slot	= sdhci_acpi_emmc_probe_slot,
+	.probe_slot	= intel_probe_slot,
 };
 
 static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sdio = {
@@ -343,7 +307,7 @@ static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
 		   MMC_CAP_WAIT_WHILE_BUSY,
 	.flags   = SDHCI_ACPI_RUNTIME_PM,
 	.pm_caps = MMC_PM_KEEP_POWER,
-	.probe_slot	= sdhci_acpi_sdio_probe_slot,
+	.probe_slot	= intel_probe_slot,
 };
 
 static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sd = {
@@ -353,7 +317,7 @@ static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
 	.quirks2 = SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON |
 		   SDHCI_QUIRK2_STOP_WITH_TC,
 	.caps    = MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_AGGRESSIVE_PM,
-	.probe_slot	= sdhci_acpi_sd_probe_slot,
+	.probe_slot	= intel_probe_slot,
 };
 
 static const struct sdhci_acpi_slot sdhci_acpi_slot_qcom_sd_3v = {
-- 
1.9.1

Re: [PATCH 2/4] mmc: sdhci-acpi: Tidy Intel slot probe functions into one

[Ulf Hansson]

On 19 October 2017 at 12:41, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Tidy Intel slot probe functions into one. A single function can be used
> because the logic uses hid / uid as necessary to identify devices anyway.
> This gets rid of some pointless comments and checks for variables that
> cannot possibly be NULL, as well as giving the function a name that
> identifies it as specific to Intel controllers.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>


Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-acpi.c | 48 ++++++-------------------------------------
>  1 file changed, 6 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index e4b7fac5efb2..640be5b618fc 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -269,53 +269,17 @@ static int bxt_get_cd(struct mmc_host *mmc)
>         return ret;
>  }
>
> -static int sdhci_acpi_emmc_probe_slot(struct platform_device *pdev,
> -                                     const char *hid, const char *uid)
> +static int intel_probe_slot(struct platform_device *pdev, const char *hid,
> +                           const char *uid)
>  {
>         struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
> -       struct sdhci_host *host;
> -
> -       if (!c || !c->host)
> -               return 0;
> -
> -       host = c->host;
> -
> -       /* Platform specific code during emmc probe slot goes here */
> +       struct sdhci_host *host = c->host;
>
>         if (hid && uid && !strcmp(hid, "80860F14") && !strcmp(uid, "1") &&
>             sdhci_readl(host, SDHCI_CAPABILITIES) == 0x446cc8b2 &&
>             sdhci_readl(host, SDHCI_CAPABILITIES_1) == 0x00000807)
>                 host->timeout_clk = 1000; /* 1000 kHz i.e. 1 MHz */
>
> -       return 0;
> -}
> -
> -static int sdhci_acpi_sdio_probe_slot(struct platform_device *pdev,
> -                                     const char *hid, const char *uid)
> -{
> -       struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
> -
> -       if (!c || !c->host)
> -               return 0;
> -
> -       /* Platform specific code during sdio probe slot goes here */
> -
> -       return 0;
> -}
> -
> -static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
> -                                   const char *hid, const char *uid)
> -{
> -       struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
> -       struct sdhci_host *host;
> -
> -       if (!c || !c->host || !c->slot)
> -               return 0;
> -
> -       host = c->host;
> -
> -       /* Platform specific code during sd probe slot goes here */
> -
>         if (hid && !strcmp(hid, "80865ACA"))
>                 host->mmc_host_ops.get_cd = bxt_get_cd;
>
> @@ -332,7 +296,7 @@ static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
>         .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
>                    SDHCI_QUIRK2_STOP_WITH_TC |
>                    SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
> -       .probe_slot     = sdhci_acpi_emmc_probe_slot,
> +       .probe_slot     = intel_probe_slot,
>  };
>
>  static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sdio = {
> @@ -343,7 +307,7 @@ static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
>                    MMC_CAP_WAIT_WHILE_BUSY,
>         .flags   = SDHCI_ACPI_RUNTIME_PM,
>         .pm_caps = MMC_PM_KEEP_POWER,
> -       .probe_slot     = sdhci_acpi_sdio_probe_slot,
> +       .probe_slot     = intel_probe_slot,
>  };
>
>  static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sd = {
> @@ -353,7 +317,7 @@ static int sdhci_acpi_sd_probe_slot(struct platform_device *pdev,
>         .quirks2 = SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON |
>                    SDHCI_QUIRK2_STOP_WITH_TC,
>         .caps    = MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_AGGRESSIVE_PM,
> -       .probe_slot     = sdhci_acpi_sd_probe_slot,
> +       .probe_slot     = intel_probe_slot,
>  };
>
>  static const struct sdhci_acpi_slot sdhci_acpi_slot_qcom_sd_3v = {
> --
> 1.9.1
>

[PATCH 3/4] mmc: sdhci-acpi: Let devices define their own private data

[Adrian Hunter]

Let devices define their own private data to facilitate device-specific
operations. The size of the private structure is specified in the
sdhci_acpi_slot structure, then sdhci_acpi_probe() will allocate extra
space for it, and sdhci_acpi_priv() can be used to get a reference to it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-acpi.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 640be5b618fc..5bb5880403b2 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -73,6 +73,7 @@ struct sdhci_acpi_slot {
 	unsigned int	caps2;
 	mmc_pm_flag_t	pm_caps;
 	unsigned int	flags;
+	size_t		priv_size;
 	int (*probe_slot)(struct platform_device *, const char *, const char *);
 	int (*remove_slot)(struct platform_device *);
 };
@@ -82,8 +83,14 @@ struct sdhci_acpi_host {
 	const struct sdhci_acpi_slot	*slot;
 	struct platform_device		*pdev;
 	bool				use_runtime_pm;
+	unsigned long			private[0] ____cacheline_aligned;
 };
 
+static inline void *sdhci_acpi_priv(struct sdhci_acpi_host *c)
+{
+	return (void *)c->private;
+}
+
 static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)
 {
 	return c->slot && (c->slot->flags & flag);
@@ -393,11 +400,13 @@ static const struct sdhci_acpi_slot *sdhci_acpi_get_slot(const char *hid,
 static int sdhci_acpi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	const struct sdhci_acpi_slot *slot;
 	struct acpi_device *device, *child;
 	struct sdhci_acpi_host *c;
 	struct sdhci_host *host;
 	struct resource *iomem;
 	resource_size_t len;
+	size_t priv_size;
 	const char *hid;
 	const char *uid;
 	int err;
@@ -409,6 +418,8 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 	hid = acpi_device_hid(device);
 	uid = acpi_device_uid(device);
 
+	slot = sdhci_acpi_get_slot(hid, uid);
+
 	/* Power on the SDHCI controller and its children */
 	acpi_device_fix_up_power(device);
 	if (!sdhci_acpi_no_fixup_child_power(hid, uid)) {
@@ -431,13 +442,14 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 	if (!devm_request_mem_region(dev, iomem->start, len, dev_name(dev)))
 		return -ENOMEM;
 
-	host = sdhci_alloc_host(dev, sizeof(struct sdhci_acpi_host));
+	priv_size = slot ? slot->priv_size : 0;
+	host = sdhci_alloc_host(dev, sizeof(struct sdhci_acpi_host) + priv_size);
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
 	c = sdhci_priv(host);
 	c->host = host;
-	c->slot = sdhci_acpi_get_slot(hid, uid);
+	c->slot = slot;
 	c->pdev = pdev;
 	c->use_runtime_pm = sdhci_acpi_flag(c, SDHCI_ACPI_RUNTIME_PM);
 
-- 
1.9.1

Re: [PATCH 3/4] mmc: sdhci-acpi: Let devices define their own private data

[Ulf Hansson]

On 19 October 2017 at 12:41, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Let devices define their own private data to facilitate device-specific
> operations. The size of the private structure is specified in the
> sdhci_acpi_slot structure, then sdhci_acpi_probe() will allocate extra
> space for it, and sdhci_acpi_priv() can be used to get a reference to it.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-acpi.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index 640be5b618fc..5bb5880403b2 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -73,6 +73,7 @@ struct sdhci_acpi_slot {
>         unsigned int    caps2;
>         mmc_pm_flag_t   pm_caps;
>         unsigned int    flags;
> +       size_t          priv_size;
>         int (*probe_slot)(struct platform_device *, const char *, const char *);
>         int (*remove_slot)(struct platform_device *);
>  };
> @@ -82,8 +83,14 @@ struct sdhci_acpi_host {
>         const struct sdhci_acpi_slot    *slot;
>         struct platform_device          *pdev;
>         bool                            use_runtime_pm;
> +       unsigned long                   private[0] ____cacheline_aligned;
>  };
>
> +static inline void *sdhci_acpi_priv(struct sdhci_acpi_host *c)
> +{
> +       return (void *)c->private;
> +}
> +
>  static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)
>  {
>         return c->slot && (c->slot->flags & flag);
> @@ -393,11 +400,13 @@ static const struct sdhci_acpi_slot *sdhci_acpi_get_slot(const char *hid,
>  static int sdhci_acpi_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
> +       const struct sdhci_acpi_slot *slot;
>         struct acpi_device *device, *child;
>         struct sdhci_acpi_host *c;
>         struct sdhci_host *host;
>         struct resource *iomem;
>         resource_size_t len;
> +       size_t priv_size;
>         const char *hid;
>         const char *uid;
>         int err;
> @@ -409,6 +418,8 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>         hid = acpi_device_hid(device);
>         uid = acpi_device_uid(device);
>
> +       slot = sdhci_acpi_get_slot(hid, uid);
> +
>         /* Power on the SDHCI controller and its children */
>         acpi_device_fix_up_power(device);
>         if (!sdhci_acpi_no_fixup_child_power(hid, uid)) {
> @@ -431,13 +442,14 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>         if (!devm_request_mem_region(dev, iomem->start, len, dev_name(dev)))
>                 return -ENOMEM;
>
> -       host = sdhci_alloc_host(dev, sizeof(struct sdhci_acpi_host));
> +       priv_size = slot ? slot->priv_size : 0;
> +       host = sdhci_alloc_host(dev, sizeof(struct sdhci_acpi_host) + priv_size);
>         if (IS_ERR(host))
>                 return PTR_ERR(host);
>
>         c = sdhci_priv(host);
>         c->host = host;
> -       c->slot = sdhci_acpi_get_slot(hid, uid);
> +       c->slot = slot;
>         c->pdev = pdev;
>         c->use_runtime_pm = sdhci_acpi_flag(c, SDHCI_ACPI_RUNTIME_PM);
>
> --
> 1.9.1
>

[PATCH 4/4] mmc: sdhci-acpi: Fix voltage switch for some Intel host controllers

[Adrian Hunter]

Some Intel host controllers use an ACPI device-specific method to ensure
correct voltage switching. Fix voltage switch for those, by adding a call
to the DSM.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 drivers/mmc/host/sdhci-acpi.c | 108 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 5bb5880403b2..b988997a1e80 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -96,6 +96,105 @@ static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)
 	return c->slot && (c->slot->flags & flag);
 }
 
+enum {
+	INTEL_DSM_FNS		=  0,
+	INTEL_DSM_V18_SWITCH	=  3,
+	INTEL_DSM_V33_SWITCH	=  4,
+};
+
+struct intel_host {
+	u32	dsm_fns;
+};
+
+static const guid_t intel_dsm_guid =
+	GUID_INIT(0xF6C13EA5, 0x65CD, 0x461F,
+		  0xAB, 0x7A, 0x29, 0xF7, 0xE8, 0xD5, 0xBD, 0x61);
+
+static int __intel_dsm(struct intel_host *intel_host, struct device *dev,
+		       unsigned int fn, u32 *result)
+{
+	union acpi_object *obj;
+	int err = 0;
+
+	obj = acpi_evaluate_dsm(ACPI_HANDLE(dev), &intel_dsm_guid, 0, fn, NULL);
+	if (!obj)
+		return -EOPNOTSUPP;
+
+	if (obj->type == ACPI_TYPE_INTEGER) {
+		*result = obj->integer.value;
+	} else if (obj->type == ACPI_TYPE_BUFFER && obj->buffer.length > 0) {
+		size_t len = min_t(size_t, obj->buffer.length, 4);
+
+		*result = 0;
+		memcpy(result, obj->buffer.pointer, len);
+	} else {
+		dev_err(dev, "%s DSM fn %u obj->type %d obj->buffer.length %d\n",
+			__func__, fn, obj->type, obj->buffer.length);
+		err = -EINVAL;
+	}
+
+	ACPI_FREE(obj);
+
+	return err;
+}
+
+static int intel_dsm(struct intel_host *intel_host, struct device *dev,
+		     unsigned int fn, u32 *result)
+{
+	if (fn > 31 || !(intel_host->dsm_fns & (1 << fn)))
+		return -EOPNOTSUPP;
+
+	return __intel_dsm(intel_host, dev, fn, result);
+}
+
+static void intel_dsm_init(struct intel_host *intel_host, struct device *dev,
+			   struct mmc_host *mmc)
+{
+	int err;
+
+	err = __intel_dsm(intel_host, dev, INTEL_DSM_FNS, &intel_host->dsm_fns);
+	if (err) {
+		pr_debug("%s: DSM not supported, error %d\n",
+			 mmc_hostname(mmc), err);
+		return;
+	}
+
+	pr_debug("%s: DSM function mask %#x\n",
+		 mmc_hostname(mmc), intel_host->dsm_fns);
+}
+
+static int intel_start_signal_voltage_switch(struct mmc_host *mmc,
+					     struct mmc_ios *ios)
+{
+	struct device *dev = mmc_dev(mmc);
+	struct sdhci_acpi_host *c = dev_get_drvdata(dev);
+	struct intel_host *intel_host = sdhci_acpi_priv(c);
+	unsigned int fn;
+	u32 result = 0;
+	int err;
+
+	err = sdhci_start_signal_voltage_switch(mmc, ios);
+	if (err)
+		return err;
+
+	switch (ios->signal_voltage) {
+	case MMC_SIGNAL_VOLTAGE_330:
+		fn = INTEL_DSM_V33_SWITCH;
+		break;
+	case MMC_SIGNAL_VOLTAGE_180:
+		fn = INTEL_DSM_V18_SWITCH;
+		break;
+	default:
+		return 0;
+	}
+
+	err = intel_dsm(intel_host, dev, fn, &result);
+	pr_debug("%s: %s DSM fn %u error %d result %u\n",
+		 mmc_hostname(mmc), __func__, fn, err, result);
+
+	return 0;
+}
+
 static void sdhci_acpi_int_hw_reset(struct sdhci_host *host)
 {
 	u8 reg;
@@ -280,6 +379,7 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
 			    const char *uid)
 {
 	struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
+	struct intel_host *intel_host = sdhci_acpi_priv(c);
 	struct sdhci_host *host = c->host;
 
 	if (hid && uid && !strcmp(hid, "80860F14") && !strcmp(uid, "1") &&
@@ -290,6 +390,11 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
 	if (hid && !strcmp(hid, "80865ACA"))
 		host->mmc_host_ops.get_cd = bxt_get_cd;
 
+	intel_dsm_init(intel_host, &pdev->dev, host->mmc);
+
+	host->mmc_host_ops.start_signal_voltage_switch =
+					intel_start_signal_voltage_switch;
+
 	return 0;
 }
 
@@ -304,6 +409,7 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
 		   SDHCI_QUIRK2_STOP_WITH_TC |
 		   SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
 	.probe_slot	= intel_probe_slot,
+	.priv_size	= sizeof(struct intel_host),
 };
 
 static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sdio = {
@@ -315,6 +421,7 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
 	.flags   = SDHCI_ACPI_RUNTIME_PM,
 	.pm_caps = MMC_PM_KEEP_POWER,
 	.probe_slot	= intel_probe_slot,
+	.priv_size	= sizeof(struct intel_host),
 };
 
 static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sd = {
@@ -325,6 +432,7 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
 		   SDHCI_QUIRK2_STOP_WITH_TC,
 	.caps    = MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_AGGRESSIVE_PM,
 	.probe_slot	= intel_probe_slot,
+	.priv_size	= sizeof(struct intel_host),
 };
 
 static const struct sdhci_acpi_slot sdhci_acpi_slot_qcom_sd_3v = {
-- 
1.9.1

Re: [PATCH 4/4] mmc: sdhci-acpi: Fix voltage switch for some Intel host controllers

[Ulf Hansson]

On 19 October 2017 at 12:41, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Some Intel host controllers use an ACPI device-specific method to ensure
> correct voltage switching. Fix voltage switch for those, by adding a call
> to the DSM.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  drivers/mmc/host/sdhci-acpi.c | 108 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index 5bb5880403b2..b988997a1e80 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -96,6 +96,105 @@ static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)
>         return c->slot && (c->slot->flags & flag);
>  }
>
> +enum {
> +       INTEL_DSM_FNS           =  0,
> +       INTEL_DSM_V18_SWITCH    =  3,
> +       INTEL_DSM_V33_SWITCH    =  4,
> +};
> +
> +struct intel_host {
> +       u32     dsm_fns;
> +};
> +
> +static const guid_t intel_dsm_guid =
> +       GUID_INIT(0xF6C13EA5, 0x65CD, 0x461F,
> +                 0xAB, 0x7A, 0x29, 0xF7, 0xE8, 0xD5, 0xBD, 0x61);
> +
> +static int __intel_dsm(struct intel_host *intel_host, struct device *dev,
> +                      unsigned int fn, u32 *result)
> +{
> +       union acpi_object *obj;
> +       int err = 0;
> +
> +       obj = acpi_evaluate_dsm(ACPI_HANDLE(dev), &intel_dsm_guid, 0, fn, NULL);
> +       if (!obj)
> +               return -EOPNOTSUPP;
> +
> +       if (obj->type == ACPI_TYPE_INTEGER) {
> +               *result = obj->integer.value;
> +       } else if (obj->type == ACPI_TYPE_BUFFER && obj->buffer.length > 0) {
> +               size_t len = min_t(size_t, obj->buffer.length, 4);
> +
> +               *result = 0;
> +               memcpy(result, obj->buffer.pointer, len);
> +       } else {
> +               dev_err(dev, "%s DSM fn %u obj->type %d obj->buffer.length %d\n",
> +                       __func__, fn, obj->type, obj->buffer.length);
> +               err = -EINVAL;
> +       }
> +

This hole ACPI thing looks weird. Could perhaps the ACPI core, model
this a vqmmc regulator instead?

That would keep the ACPI specific part closer to ACPI and we can treat
this similar to other mmc host drivers.

> +       ACPI_FREE(obj);
> +
> +       return err;
> +}
> +

[...]

Kind regards
Uffe

Re: [PATCH 4/4] mmc: sdhci-acpi: Fix voltage switch for some Intel host controllers

[Adrian Hunter]

On 20/10/17 12:16, Ulf Hansson wrote:
> On 19 October 2017 at 12:41, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> Some Intel host controllers use an ACPI device-specific method to ensure
>> correct voltage switching. Fix voltage switch for those, by adding a call
>> to the DSM.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  drivers/mmc/host/sdhci-acpi.c | 108 ++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 108 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
>> index 5bb5880403b2..b988997a1e80 100644
>> --- a/drivers/mmc/host/sdhci-acpi.c
>> +++ b/drivers/mmc/host/sdhci-acpi.c
>> @@ -96,6 +96,105 @@ static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)
>>         return c->slot && (c->slot->flags & flag);
>>  }
>>
>> +enum {
>> +       INTEL_DSM_FNS           =  0,
>> +       INTEL_DSM_V18_SWITCH    =  3,
>> +       INTEL_DSM_V33_SWITCH    =  4,
>> +};
>> +
>> +struct intel_host {
>> +       u32     dsm_fns;
>> +};
>> +
>> +static const guid_t intel_dsm_guid =
>> +       GUID_INIT(0xF6C13EA5, 0x65CD, 0x461F,
>> +                 0xAB, 0x7A, 0x29, 0xF7, 0xE8, 0xD5, 0xBD, 0x61);
>> +
>> +static int __intel_dsm(struct intel_host *intel_host, struct device *dev,
>> +                      unsigned int fn, u32 *result)
>> +{
>> +       union acpi_object *obj;
>> +       int err = 0;
>> +
>> +       obj = acpi_evaluate_dsm(ACPI_HANDLE(dev), &intel_dsm_guid, 0, fn, NULL);
>> +       if (!obj)
>> +               return -EOPNOTSUPP;
>> +
>> +       if (obj->type == ACPI_TYPE_INTEGER) {
>> +               *result = obj->integer.value;
>> +       } else if (obj->type == ACPI_TYPE_BUFFER && obj->buffer.length > 0) {
>> +               size_t len = min_t(size_t, obj->buffer.length, 4);
>> +
>> +               *result = 0;
>> +               memcpy(result, obj->buffer.pointer, len);
>> +       } else {
>> +               dev_err(dev, "%s DSM fn %u obj->type %d obj->buffer.length %d\n",
>> +                       __func__, fn, obj->type, obj->buffer.length);
>> +               err = -EINVAL;
>> +       }
>> +
> 
> This hole ACPI thing looks weird.

DSMs are normal.

>                                   Could perhaps the ACPI core, model
> this a vqmmc regulator instead?

The regulator framework does not support that.

> 
> That would keep the ACPI specific part closer to ACPI and we can treat
> this similar to other mmc host drivers.

Device-specific methods (DSM) are device specific so they belong in the
driver not ACPI core.  So the code would all end up here anyway but be more
complicated and more obfuscated.

Re: [PATCH 4/4] mmc: sdhci-acpi: Fix voltage switch for some Intel host controllers

[Ulf Hansson]

On 19 October 2017 at 12:41, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Some Intel host controllers use an ACPI device-specific method to ensure
> correct voltage switching. Fix voltage switch for those, by adding a call
> to the DSM.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-acpi.c | 108 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 108 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index 5bb5880403b2..b988997a1e80 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -96,6 +96,105 @@ static inline bool sdhci_acpi_flag(struct sdhci_acpi_host *c, unsigned int flag)
>         return c->slot && (c->slot->flags & flag);
>  }
>
> +enum {
> +       INTEL_DSM_FNS           =  0,
> +       INTEL_DSM_V18_SWITCH    =  3,
> +       INTEL_DSM_V33_SWITCH    =  4,
> +};
> +
> +struct intel_host {
> +       u32     dsm_fns;
> +};
> +
> +static const guid_t intel_dsm_guid =
> +       GUID_INIT(0xF6C13EA5, 0x65CD, 0x461F,
> +                 0xAB, 0x7A, 0x29, 0xF7, 0xE8, 0xD5, 0xBD, 0x61);
> +
> +static int __intel_dsm(struct intel_host *intel_host, struct device *dev,
> +                      unsigned int fn, u32 *result)
> +{
> +       union acpi_object *obj;
> +       int err = 0;
> +
> +       obj = acpi_evaluate_dsm(ACPI_HANDLE(dev), &intel_dsm_guid, 0, fn, NULL);
> +       if (!obj)
> +               return -EOPNOTSUPP;
> +
> +       if (obj->type == ACPI_TYPE_INTEGER) {
> +               *result = obj->integer.value;
> +       } else if (obj->type == ACPI_TYPE_BUFFER && obj->buffer.length > 0) {
> +               size_t len = min_t(size_t, obj->buffer.length, 4);
> +
> +               *result = 0;
> +               memcpy(result, obj->buffer.pointer, len);
> +       } else {
> +               dev_err(dev, "%s DSM fn %u obj->type %d obj->buffer.length %d\n",
> +                       __func__, fn, obj->type, obj->buffer.length);
> +               err = -EINVAL;
> +       }
> +
> +       ACPI_FREE(obj);
> +
> +       return err;
> +}
> +
> +static int intel_dsm(struct intel_host *intel_host, struct device *dev,
> +                    unsigned int fn, u32 *result)
> +{
> +       if (fn > 31 || !(intel_host->dsm_fns & (1 << fn)))
> +               return -EOPNOTSUPP;
> +
> +       return __intel_dsm(intel_host, dev, fn, result);
> +}
> +
> +static void intel_dsm_init(struct intel_host *intel_host, struct device *dev,
> +                          struct mmc_host *mmc)
> +{
> +       int err;
> +
> +       err = __intel_dsm(intel_host, dev, INTEL_DSM_FNS, &intel_host->dsm_fns);
> +       if (err) {
> +               pr_debug("%s: DSM not supported, error %d\n",
> +                        mmc_hostname(mmc), err);
> +               return;
> +       }
> +
> +       pr_debug("%s: DSM function mask %#x\n",
> +                mmc_hostname(mmc), intel_host->dsm_fns);
> +}
> +
> +static int intel_start_signal_voltage_switch(struct mmc_host *mmc,
> +                                            struct mmc_ios *ios)
> +{
> +       struct device *dev = mmc_dev(mmc);
> +       struct sdhci_acpi_host *c = dev_get_drvdata(dev);
> +       struct intel_host *intel_host = sdhci_acpi_priv(c);
> +       unsigned int fn;
> +       u32 result = 0;
> +       int err;
> +
> +       err = sdhci_start_signal_voltage_switch(mmc, ios);
> +       if (err)
> +               return err;
> +
> +       switch (ios->signal_voltage) {
> +       case MMC_SIGNAL_VOLTAGE_330:
> +               fn = INTEL_DSM_V33_SWITCH;
> +               break;
> +       case MMC_SIGNAL_VOLTAGE_180:
> +               fn = INTEL_DSM_V18_SWITCH;
> +               break;
> +       default:
> +               return 0;
> +       }
> +
> +       err = intel_dsm(intel_host, dev, fn, &result);
> +       pr_debug("%s: %s DSM fn %u error %d result %u\n",
> +                mmc_hostname(mmc), __func__, fn, err, result);
> +
> +       return 0;
> +}
> +
>  static void sdhci_acpi_int_hw_reset(struct sdhci_host *host)
>  {
>         u8 reg;
> @@ -280,6 +379,7 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
>                             const char *uid)
>  {
>         struct sdhci_acpi_host *c = platform_get_drvdata(pdev);
> +       struct intel_host *intel_host = sdhci_acpi_priv(c);
>         struct sdhci_host *host = c->host;
>
>         if (hid && uid && !strcmp(hid, "80860F14") && !strcmp(uid, "1") &&
> @@ -290,6 +390,11 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
>         if (hid && !strcmp(hid, "80865ACA"))
>                 host->mmc_host_ops.get_cd = bxt_get_cd;
>
> +       intel_dsm_init(intel_host, &pdev->dev, host->mmc);
> +
> +       host->mmc_host_ops.start_signal_voltage_switch =
> +                                       intel_start_signal_voltage_switch;
> +
>         return 0;
>  }
>
> @@ -304,6 +409,7 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
>                    SDHCI_QUIRK2_STOP_WITH_TC |
>                    SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400,
>         .probe_slot     = intel_probe_slot,
> +       .priv_size      = sizeof(struct intel_host),
>  };
>
>  static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sdio = {
> @@ -315,6 +421,7 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
>         .flags   = SDHCI_ACPI_RUNTIME_PM,
>         .pm_caps = MMC_PM_KEEP_POWER,
>         .probe_slot     = intel_probe_slot,
> +       .priv_size      = sizeof(struct intel_host),
>  };
>
>  static const struct sdhci_acpi_slot sdhci_acpi_slot_int_sd = {
> @@ -325,6 +432,7 @@ static int intel_probe_slot(struct platform_device *pdev, const char *hid,
>                    SDHCI_QUIRK2_STOP_WITH_TC,
>         .caps    = MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_AGGRESSIVE_PM,
>         .probe_slot     = intel_probe_slot,
> +       .priv_size      = sizeof(struct intel_host),
>  };
>
>  static const struct sdhci_acpi_slot sdhci_acpi_slot_qcom_sd_3v = {
> --
> 1.9.1
>