[RFC 0/2] mmc: sdhci-pci: Use ACPI to set max frequency of sdio host

[RFC 0/2] mmc: sdhci-pci: Use ACPI to set max frequency of sdio host

[Zach Brown]

On some boards, max SDIO frequency is limited by trace lengths and other layout
choices. We would like a way to specify this limitation so the driver can
behave accordingly.

This patch set assumes that the limitation has been reported in an ACPI table
which the driver can check to get the max frequency.

The first patch creates a PCI ID and support for the Intel byt sdio where NI is
the subvendor.

The second patch uses the ACPI table to set f_max during the new
ni_byt_sdio_probe_slot.


Zach Brown (2):
  mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller
    sub-vended by NI
  mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio
    host controller sub-vended by NI

 drivers/mmc/host/sdhci-pci-core.c | 50 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

--
2.7.4

[RFC 1/2] mmc: sdhci-pci: Add PCI ID for Intel byt sdio host controller sub-vended by NI

[Zach Brown]

Add PCI ID for Intel byt sdio host controller sub-vended by NI.

The controller has different behavior because of the board layout NI
puts it on.

Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/mmc/host/sdhci-pci-core.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 1d9e00a..c333ce2 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -375,6 +375,12 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
 	return 0;
 }
 
+static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
+{
+	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE;
+	return 0;
+}
+
 static int byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
 {
 	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE |
@@ -447,6 +453,12 @@ static const struct sdhci_pci_fixes sdhci_intel_byt_emmc = {
 	.ops		= &sdhci_intel_byt_ops,
 };
 
+static const struct sdhci_pci_fixes sdhci_ni_byt_sdio = {
+	.quirks2	= SDHCI_QUIRK2_HOST_OFF_CARD_ON,
+	.allow_runtime_pm = true,
+	.probe_slot	= ni_byt_sdio_probe_slot,
+};
+
 static const struct sdhci_pci_fixes sdhci_intel_byt_sdio = {
 	.quirks		= SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
 	.quirks2	= SDHCI_QUIRK2_HOST_OFF_CARD_ON |
@@ -1079,6 +1091,14 @@ static const struct pci_device_id pci_ids[] = {
 	{
 		.vendor		= PCI_VENDOR_ID_INTEL,
 		.device		= PCI_DEVICE_ID_INTEL_BYT_SDIO,
+		.subvendor	= PCI_VENDOR_ID_NI,
+		.subdevice	= 0x7884,
+		.driver_data	= (kernel_ulong_t)&sdhci_ni_byt_sdio,
+	},
+
+	{
+		.vendor		= PCI_VENDOR_ID_INTEL,
+		.device		= PCI_DEVICE_ID_INTEL_BYT_SDIO,
 		.subvendor	= PCI_ANY_ID,
 		.subdevice	= PCI_ANY_ID,
 		.driver_data	= (kernel_ulong_t)&sdhci_intel_byt_sdio,
-- 
2.7.4

[RFC 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio host controller sub-vended by NI

[Zach Brown]

On NI 9037 boards the max SDIO frequency is limited by trace lengths
and other layout choices. The max SDIO frequency is stored in an ACPI
table, as MXFQ.

The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
f_max field of the host with it.

Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
Reviewed-by: Josh Cartwright <joshc@ni.com>
Signed-off-by: Zach Brown <zach.brown@ni.com>
---
 drivers/mmc/host/sdhci-pci-core.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index c333ce2..4ac7f16 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -27,6 +27,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/mmc/slot-gpio.h>
 #include <linux/mmc/sdhci-pci-data.h>
+#include <linux/acpi.h>
 
 #include "sdhci.h"
 #include "sdhci-pci.h"
@@ -377,6 +378,35 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
 
 static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
 {
+#ifdef CONFIG_ACPI
+	/* Get max freq from ACPI for NI hardware */
+	acpi_handle acpi_hdl;
+	acpi_status status;
+	struct acpi_buffer acpi_result = {
+		ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *acpi_buffer;
+	int max_freq;
+
+	status = acpi_get_handle(ACPI_HANDLE(&slot->chip->pdev->dev), "MXFQ",
+				 &acpi_hdl);
+	if (ACPI_FAILURE(status))
+		return  -ENODEV;
+
+	status = acpi_evaluate_object(acpi_hdl, NULL,
+				      NULL, &acpi_result);
+	if (ACPI_FAILURE(status))
+		return -EINVAL;
+
+	acpi_buffer = (union acpi_object *)acpi_result.pointer;
+
+	if (acpi_buffer->type != ACPI_TYPE_INTEGER)
+		return -EINVAL;
+
+	max_freq = acpi_buffer->integer.value;
+
+	slot->host->mmc->f_max = max_freq * 1000000;
+#endif
+
 	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE;
 	return 0;
 }
-- 
2.7.4

Re: [RFC 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio host controller sub-vended by NI

[Adrian Hunter]

On 08/11/16 22:07, Zach Brown wrote:
> On NI 9037 boards the max SDIO frequency is limited by trace lengths
> and other layout choices. The max SDIO frequency is stored in an ACPI
> table, as MXFQ.
> 
> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> f_max field of the host with it.
> 
> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
> Reviewed-by: Josh Cartwright <joshc@ni.com>
> Signed-off-by: Zach Brown <zach.brown@ni.com>
> ---
>  drivers/mmc/host/sdhci-pci-core.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> index c333ce2..4ac7f16 100644
> --- a/drivers/mmc/host/sdhci-pci-core.c
> +++ b/drivers/mmc/host/sdhci-pci-core.c
> @@ -27,6 +27,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/mmc/slot-gpio.h>
>  #include <linux/mmc/sdhci-pci-data.h>
> +#include <linux/acpi.h>
>  
>  #include "sdhci.h"
>  #include "sdhci-pci.h"
> @@ -377,6 +378,35 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
>  
>  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>  {
> +#ifdef CONFIG_ACPI
> +	/* Get max freq from ACPI for NI hardware */
> +	acpi_handle acpi_hdl;
> +	acpi_status status;
> +	struct acpi_buffer acpi_result = {
> +		ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *acpi_buffer;
> +	int max_freq;
> +
> +	status = acpi_get_handle(ACPI_HANDLE(&slot->chip->pdev->dev), "MXFQ",
> +				 &acpi_hdl);

Is "MXFQ" an object that has already been deployed or are you inventing it
now?  In the latter case, did you consider device properties as an alternative?

> +	if (ACPI_FAILURE(status))
> +		return  -ENODEV;
> +
> +	status = acpi_evaluate_object(acpi_hdl, NULL,
> +				      NULL, &acpi_result);
> +	if (ACPI_FAILURE(status))
> +		return -EINVAL;
> +
> +	acpi_buffer = (union acpi_object *)acpi_result.pointer;
> +
> +	if (acpi_buffer->type != ACPI_TYPE_INTEGER)
> +		return -EINVAL;
> +
> +	max_freq = acpi_buffer->integer.value;
> +
> +	slot->host->mmc->f_max = max_freq * 1000000;
> +#endif
> +
>  	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE;
>  	return 0;
>  }
> 

Re: [RFC 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio host controller sub-vended by NI

[Zach Brown]

On Wed, Nov 09, 2016 at 03:24:24PM +0200, Adrian Hunter wrote:
> On 08/11/16 22:07, Zach Brown wrote:
> > On NI 9037 boards the max SDIO frequency is limited by trace lengths
> > and other layout choices. The max SDIO frequency is stored in an ACPI
> > table, as MXFQ.
> >
> > The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> > f_max field of the host with it.
> >
> > Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> > Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
> > Reviewed-by: Josh Cartwright <joshc@ni.com>
> > Signed-off-by: Zach Brown <zach.brown@ni.com>
> > ---
> >  drivers/mmc/host/sdhci-pci-core.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
> > index c333ce2..4ac7f16 100644
> > --- a/drivers/mmc/host/sdhci-pci-core.c
> > +++ b/drivers/mmc/host/sdhci-pci-core.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/pm_runtime.h>
> >  #include <linux/mmc/slot-gpio.h>
> >  #include <linux/mmc/sdhci-pci-data.h>
> > +#include <linux/acpi.h>
> >
> >  #include "sdhci.h"
> >  #include "sdhci-pci.h"
> > @@ -377,6 +378,35 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
> >
> >  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
> >  {
> > +#ifdef CONFIG_ACPI
> > +	/* Get max freq from ACPI for NI hardware */
> > +	acpi_handle acpi_hdl;
> > +	acpi_status status;
> > +	struct acpi_buffer acpi_result = {
> > +		ACPI_ALLOCATE_BUFFER, NULL };
> > +	union acpi_object *acpi_buffer;
> > +	int max_freq;
> > +
> > +	status = acpi_get_handle(ACPI_HANDLE(&slot->chip->pdev->dev), "MXFQ",
> > +				 &acpi_hdl);
>
> Is "MXFQ" an object that has already been deployed or are you inventing it
> now?  In the latter case, did you consider device properties as an alternative?
>
"MXFQ" is an object that we have already deployed on some of our devices.

> > +	if (ACPI_FAILURE(status))
> > +		return  -ENODEV;
> > +
> > +	status = acpi_evaluate_object(acpi_hdl, NULL,
> > +				      NULL, &acpi_result);
> > +	if (ACPI_FAILURE(status))
> > +		return -EINVAL;
> > +
> > +	acpi_buffer = (union acpi_object *)acpi_result.pointer;
> > +
> > +	if (acpi_buffer->type != ACPI_TYPE_INTEGER)
> > +		return -EINVAL;
> > +
> > +	max_freq = acpi_buffer->integer.value;
> > +
> > +	slot->host->mmc->f_max = max_freq * 1000000;
> > +#endif
> > +
> >  	slot->host->mmc->caps |= MMC_CAP_POWER_OFF_CARD | MMC_CAP_NONREMOVABLE;
> >  	return 0;
> >  }
> >
>

Re: [RFC 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio host controller sub-vended by NI

[Julia Cartwright]

On Wed, Nov 09, 2016 at 10:08:29AM -0600, Zach Brown wrote:
> On Wed, Nov 09, 2016 at 03:24:24PM +0200, Adrian Hunter wrote:
> > On 08/11/16 22:07, Zach Brown wrote:
> > > On NI 9037 boards the max SDIO frequency is limited by trace lengths
> > > and other layout choices. The max SDIO frequency is stored in an ACPI
> > > table, as MXFQ.
> > >
> > > The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
> > > f_max field of the host with it.
> > >
> > > Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
> > > Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
> > > Reviewed-by: Josh Cartwright <joshc@ni.com>
> > > Signed-off-by: Zach Brown <zach.brown@ni.com>
[..]
> > >  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
> > >  {
> > > +#ifdef CONFIG_ACPI
> > > +	/* Get max freq from ACPI for NI hardware */
> > > +	acpi_handle acpi_hdl;
> > > +	acpi_status status;
> > > +	struct acpi_buffer acpi_result = {
> > > +		ACPI_ALLOCATE_BUFFER, NULL };
> > > +	union acpi_object *acpi_buffer;
> > > +	int max_freq;
> > > +
> > > +	status = acpi_get_handle(ACPI_HANDLE(&slot->chip->pdev->dev), "MXFQ",
> > > +				 &acpi_hdl);
> >
> > Is "MXFQ" an object that has already been deployed or are you inventing it
> > now?  In the latter case, did you consider device properties as an alternative?
> >
> "MXFQ" is an object that we have already deployed on some of our devices.

Unfortunately, the whole ACPI device properties table discussion was
just starting at the point where we were putting the firmware together
for these devices :(.  Had we engineered the firmware today, we would
certainly have looked at using it.

Thanks,
   Julia

Re: [RFC 2/2] mmc: sdhci-pci: Use ACPI to get max frequency for Intel byt sdio host controller sub-vended by NI

[Adrian Hunter]

On 11/11/16 21:49, Julia Cartwright wrote:
> On Wed, Nov 09, 2016 at 10:08:29AM -0600, Zach Brown wrote:
>> On Wed, Nov 09, 2016 at 03:24:24PM +0200, Adrian Hunter wrote:
>>> On 08/11/16 22:07, Zach Brown wrote:
>>>> On NI 9037 boards the max SDIO frequency is limited by trace lengths
>>>> and other layout choices. The max SDIO frequency is stored in an ACPI
>>>> table, as MXFQ.
>>>>
>>>> The driver reads the ACPI entry MXFQ during sdio_probe_slot and sets the
>>>> f_max field of the host with it.
>>>>
>>>> Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
>>>> Reviewed-by: Jaeden Amero <jaeden.amero@ni.com>
>>>> Reviewed-by: Josh Cartwright <joshc@ni.com>
>>>> Signed-off-by: Zach Brown <zach.brown@ni.com>
> [..]
>>>>  static int ni_byt_sdio_probe_slot(struct sdhci_pci_slot *slot)
>>>>  {
>>>> +#ifdef CONFIG_ACPI
>>>> +	/* Get max freq from ACPI for NI hardware */
>>>> +	acpi_handle acpi_hdl;
>>>> +	acpi_status status;
>>>> +	struct acpi_buffer acpi_result = {
>>>> +		ACPI_ALLOCATE_BUFFER, NULL };
>>>> +	union acpi_object *acpi_buffer;
>>>> +	int max_freq;
>>>> +
>>>> +	status = acpi_get_handle(ACPI_HANDLE(&slot->chip->pdev->dev), "MXFQ",
>>>> +				 &acpi_hdl);
>>>
>>> Is "MXFQ" an object that has already been deployed or are you inventing it
>>> now?  In the latter case, did you consider device properties as an alternative?
>>>
>> "MXFQ" is an object that we have already deployed on some of our devices.
> 
> Unfortunately, the whole ACPI device properties table discussion was
> just starting at the point where we were putting the firmware together
> for these devices :(.  Had we engineered the firmware today, we would
> certainly have looked at using it.

No problem.

WRT the code, could acpi_evaluate_integer() be used instead of
acpi_get_handle()/acpi_evaluate_object()?