[PATCH v1 1/3] PCI / ACPI: Do not export pci_get_hp_params()

[PATCH v1 1/3] PCI / ACPI: Do not export pci_get_hp_params()

[Alexandru Gagniuc]

This is only used within drivers/pci, and there is no reason to make
it available outside of the PCI core.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 drivers/pci/pci-acpi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index e1949f7efd9c..b25e5fa9d1c9 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -366,7 +366,6 @@ int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp)
 	}
 	return -ENODEV;
 }
-EXPORT_SYMBOL_GPL(pci_get_hp_params);
 
 /**
  * pciehp_is_native - Check whether a hotplug port is handled by the OS
-- 
2.19.2

Re: [PATCH v1 1/3] PCI / ACPI: Do not export pci_get_hp_params()

[Bjorn Helgaas]

On Fri, Feb 08, 2019 at 10:24:11AM -0600, Alexandru Gagniuc wrote:
> This is only used within drivers/pci, and there is no reason to make
> it available outside of the PCI core.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

Applied the whole series to pci/hotplug for v5.2, thanks!

I dropped the "list" member from struct hpx_type3 because it didn't
seem to be used.

> ---
>  drivers/pci/pci-acpi.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index e1949f7efd9c..b25e5fa9d1c9 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -366,7 +366,6 @@ int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp)
>  	}
>  	return -ENODEV;
>  }
> -EXPORT_SYMBOL_GPL(pci_get_hp_params);
>  
>  /**
>   * pciehp_is_native - Check whether a hotplug port is handled by the OS
> -- 
> 2.19.2
> 

Re: [PATCH v1 1/3] PCI / ACPI: Do not export pci_get_hp_params()

[Alex G]

On 4/22/19 3:58 PM, Bjorn Helgaas wrote:
> On Fri, Feb 08, 2019 at 10:24:11AM -0600, Alexandru Gagniuc wrote:
>> This is only used within drivers/pci, and there is no reason to make
>> it available outside of the PCI core.
>>
>> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> 
> Applied the whole series to pci/hotplug for v5.2, thanks!
> 
> I dropped the "list" member from struct hpx_type3 because it didn't
> seem to be used.

That's a good call. That was a vestigial appendage from when I first 
intended to store a list of registers in memory. I'm glad we didn't end 
up needing a list.

Alex

[PATCH v1 2/3] PCI / ACPI: Remove the need for 'struct hotplug_params'

[Alexandru Gagniuc]

We used to first parse all the _HPP and _HPX tables before using the
information to program registers of PCIe devices. Up until HPX type 2,
there was only one structure of each type, so we could cheat and store
it on the stack.

With HPX type 3 we get an arbitrary number of entries, so the above
model doesn't scale that well. Instead of parsing all tables at once,
parse and program each entry separately. For _HPP and _HPX 0 thru 2,
this is functionally equivalent. The change enables the upcoming _HPX3
to integrate more easily.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 drivers/pci/pci-acpi.c      | 108 +++++++++++++++++++-----------------
 drivers/pci/probe.c         |  16 ++----
 include/linux/pci_hotplug.h |  18 +++---
 3 files changed, 72 insertions(+), 70 deletions(-)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index b25e5fa9d1c9..95f4f86d2f34 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -119,7 +119,7 @@ phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle)
 }
 
 static acpi_status decode_type0_hpx_record(union acpi_object *record,
-					   struct hotplug_params *hpx)
+					   struct hpp_type0 *hpx0)
 {
 	int i;
 	union acpi_object *fields = record->package.elements;
@@ -132,12 +132,11 @@ static acpi_status decode_type0_hpx_record(union acpi_object *record,
 		for (i = 2; i < 6; i++)
 			if (fields[i].type != ACPI_TYPE_INTEGER)
 				return AE_ERROR;
-		hpx->t0 = &hpx->type0_data;
-		hpx->t0->revision        = revision;
-		hpx->t0->cache_line_size = fields[2].integer.value;
-		hpx->t0->latency_timer   = fields[3].integer.value;
-		hpx->t0->enable_serr     = fields[4].integer.value;
-		hpx->t0->enable_perr     = fields[5].integer.value;
+		hpx0->revision        = revision;
+		hpx0->cache_line_size = fields[2].integer.value;
+		hpx0->latency_timer   = fields[3].integer.value;
+		hpx0->enable_serr     = fields[4].integer.value;
+		hpx0->enable_perr     = fields[5].integer.value;
 		break;
 	default:
 		printk(KERN_WARNING
@@ -149,7 +148,7 @@ static acpi_status decode_type0_hpx_record(union acpi_object *record,
 }
 
 static acpi_status decode_type1_hpx_record(union acpi_object *record,
-					   struct hotplug_params *hpx)
+					   struct hpp_type1 *hpx1)
 {
 	int i;
 	union acpi_object *fields = record->package.elements;
@@ -162,11 +161,10 @@ static acpi_status decode_type1_hpx_record(union acpi_object *record,
 		for (i = 2; i < 5; i++)
 			if (fields[i].type != ACPI_TYPE_INTEGER)
 				return AE_ERROR;
-		hpx->t1 = &hpx->type1_data;
-		hpx->t1->revision      = revision;
-		hpx->t1->max_mem_read  = fields[2].integer.value;
-		hpx->t1->avg_max_split = fields[3].integer.value;
-		hpx->t1->tot_max_split = fields[4].integer.value;
+		hpx1->revision      = revision;
+		hpx1->max_mem_read  = fields[2].integer.value;
+		hpx1->avg_max_split = fields[3].integer.value;
+		hpx1->tot_max_split = fields[4].integer.value;
 		break;
 	default:
 		printk(KERN_WARNING
@@ -178,7 +176,7 @@ static acpi_status decode_type1_hpx_record(union acpi_object *record,
 }
 
 static acpi_status decode_type2_hpx_record(union acpi_object *record,
-					   struct hotplug_params *hpx)
+					   struct hpp_type2 *hpx2)
 {
 	int i;
 	union acpi_object *fields = record->package.elements;
@@ -191,24 +189,23 @@ static acpi_status decode_type2_hpx_record(union acpi_object *record,
 		for (i = 2; i < 18; i++)
 			if (fields[i].type != ACPI_TYPE_INTEGER)
 				return AE_ERROR;
-		hpx->t2 = &hpx->type2_data;
-		hpx->t2->revision      = revision;
-		hpx->t2->unc_err_mask_and      = fields[2].integer.value;
-		hpx->t2->unc_err_mask_or       = fields[3].integer.value;
-		hpx->t2->unc_err_sever_and     = fields[4].integer.value;
-		hpx->t2->unc_err_sever_or      = fields[5].integer.value;
-		hpx->t2->cor_err_mask_and      = fields[6].integer.value;
-		hpx->t2->cor_err_mask_or       = fields[7].integer.value;
-		hpx->t2->adv_err_cap_and       = fields[8].integer.value;
-		hpx->t2->adv_err_cap_or        = fields[9].integer.value;
-		hpx->t2->pci_exp_devctl_and    = fields[10].integer.value;
-		hpx->t2->pci_exp_devctl_or     = fields[11].integer.value;
-		hpx->t2->pci_exp_lnkctl_and    = fields[12].integer.value;
-		hpx->t2->pci_exp_lnkctl_or     = fields[13].integer.value;
-		hpx->t2->sec_unc_err_sever_and = fields[14].integer.value;
-		hpx->t2->sec_unc_err_sever_or  = fields[15].integer.value;
-		hpx->t2->sec_unc_err_mask_and  = fields[16].integer.value;
-		hpx->t2->sec_unc_err_mask_or   = fields[17].integer.value;
+		hpx2->revision      = revision;
+		hpx2->unc_err_mask_and      = fields[2].integer.value;
+		hpx2->unc_err_mask_or       = fields[3].integer.value;
+		hpx2->unc_err_sever_and     = fields[4].integer.value;
+		hpx2->unc_err_sever_or      = fields[5].integer.value;
+		hpx2->cor_err_mask_and      = fields[6].integer.value;
+		hpx2->cor_err_mask_or       = fields[7].integer.value;
+		hpx2->adv_err_cap_and       = fields[8].integer.value;
+		hpx2->adv_err_cap_or        = fields[9].integer.value;
+		hpx2->pci_exp_devctl_and    = fields[10].integer.value;
+		hpx2->pci_exp_devctl_or     = fields[11].integer.value;
+		hpx2->pci_exp_lnkctl_and    = fields[12].integer.value;
+		hpx2->pci_exp_lnkctl_or     = fields[13].integer.value;
+		hpx2->sec_unc_err_sever_and = fields[14].integer.value;
+		hpx2->sec_unc_err_sever_or  = fields[15].integer.value;
+		hpx2->sec_unc_err_mask_and  = fields[16].integer.value;
+		hpx2->sec_unc_err_mask_or   = fields[17].integer.value;
 		break;
 	default:
 		printk(KERN_WARNING
@@ -219,17 +216,18 @@ static acpi_status decode_type2_hpx_record(union acpi_object *record,
 	return AE_OK;
 }
 
-static acpi_status acpi_run_hpx(acpi_handle handle, struct hotplug_params *hpx)
+static acpi_status acpi_run_hpx(struct pci_dev *dev, acpi_handle handle,
+				const struct hotplug_program_ops *hp_ops)
 {
 	acpi_status status;
 	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
 	union acpi_object *package, *record, *fields;
+	struct hpp_type0 hpx0;
+	struct hpp_type1 hpx1;
+	struct hpp_type2 hpx2;
 	u32 type;
 	int i;
 
-	/* Clear the return buffer with zeros */
-	memset(hpx, 0, sizeof(struct hotplug_params));
-
 	status = acpi_evaluate_object(handle, "_HPX", NULL, &buffer);
 	if (ACPI_FAILURE(status))
 		return status;
@@ -257,19 +255,25 @@ static acpi_status acpi_run_hpx(acpi_handle handle, struct hotplug_params *hpx)
 		type = fields[0].integer.value;
 		switch (type) {
 		case 0:
-			status = decode_type0_hpx_record(record, hpx);
+			memset(&hpx0, 0, sizeof(hpx0));
+			status = decode_type0_hpx_record(record, &hpx0);
 			if (ACPI_FAILURE(status))
 				goto exit;
+			hp_ops->program_type0(dev, &hpx0);
 			break;
 		case 1:
-			status = decode_type1_hpx_record(record, hpx);
+			memset(&hpx1, 0, sizeof(hpx1));
+			status = decode_type1_hpx_record(record, &hpx1);
 			if (ACPI_FAILURE(status))
 				goto exit;
+			hp_ops->program_type1(dev, &hpx1);
 			break;
 		case 2:
-			status = decode_type2_hpx_record(record, hpx);
+			memset(&hpx2, 0, sizeof(hpx2));
+			status = decode_type2_hpx_record(record, &hpx2);
 			if (ACPI_FAILURE(status))
 				goto exit;
+			hp_ops->program_type2(dev, &hpx2);
 			break;
 		default:
 			printk(KERN_ERR "%s: Type %d record not supported\n",
@@ -283,14 +287,16 @@ static acpi_status acpi_run_hpx(acpi_handle handle, struct hotplug_params *hpx)
 	return status;
 }
 
-static acpi_status acpi_run_hpp(acpi_handle handle, struct hotplug_params *hpp)
+static acpi_status acpi_run_hpp(struct pci_dev *dev, acpi_handle handle,
+				const struct hotplug_program_ops *hp_ops)
 {
 	acpi_status status;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *package, *fields;
+	struct hpp_type0 hpp0;
 	int i;
 
-	memset(hpp, 0, sizeof(struct hotplug_params));
+	memset(&hpp0, 0, sizeof(hpp0));
 
 	status = acpi_evaluate_object(handle, "_HPP", NULL, &buffer);
 	if (ACPI_FAILURE(status))
@@ -311,12 +317,13 @@ static acpi_status acpi_run_hpp(acpi_handle handle, struct hotplug_params *hpp)
 		}
 	}
 
-	hpp->t0 = &hpp->type0_data;
-	hpp->t0->revision        = 1;
-	hpp->t0->cache_line_size = fields[0].integer.value;
-	hpp->t0->latency_timer   = fields[1].integer.value;
-	hpp->t0->enable_serr     = fields[2].integer.value;
-	hpp->t0->enable_perr     = fields[3].integer.value;
+	hpp0.revision        = 1;
+	hpp0.cache_line_size = fields[0].integer.value;
+	hpp0.latency_timer   = fields[1].integer.value;
+	hpp0.enable_serr     = fields[2].integer.value;
+	hpp0.enable_perr     = fields[3].integer.value;
+
+	hp_ops->program_type0(dev, &hpp0);
 
 exit:
 	kfree(buffer.pointer);
@@ -328,7 +335,8 @@ static acpi_status acpi_run_hpp(acpi_handle handle, struct hotplug_params *hpp)
  * @dev - the pci_dev for which we want parameters
  * @hpp - allocated by the caller
  */
-int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp)
+int pci_acpi_program_hp_params(struct pci_dev *dev,
+			       const struct hotplug_program_ops *hp_ops)
 {
 	acpi_status status;
 	acpi_handle handle, phandle;
@@ -351,10 +359,10 @@ int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp)
 	 * this pci dev.
 	 */
 	while (handle) {
-		status = acpi_run_hpx(handle, hpp);
+		status = acpi_run_hpx(dev, handle, hp_ops);
 		if (ACPI_SUCCESS(status))
 			return 0;
-		status = acpi_run_hpp(handle, hpp);
+		status = acpi_run_hpp(dev, handle, hp_ops);
 		if (ACPI_SUCCESS(status))
 			return 0;
 		if (acpi_is_root_bridge(handle))
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 257b9f6f2ebb..527c209f0c94 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2131,8 +2131,11 @@ static void pci_configure_eetlp_prefix(struct pci_dev *dev)
 
 static void pci_configure_device(struct pci_dev *dev)
 {
-	struct hotplug_params hpp;
-	int ret;
+	static const struct hotplug_program_ops hp_ops = {
+		.program_type0 = program_hpp_type0,
+		.program_type1 = program_hpp_type1,
+		.program_type2 = program_hpp_type2,
+	};
 
 	pci_configure_mps(dev);
 	pci_configure_extended_tags(dev, NULL);
@@ -2140,14 +2143,7 @@ static void pci_configure_device(struct pci_dev *dev)
 	pci_configure_ltr(dev);
 	pci_configure_eetlp_prefix(dev);
 
-	memset(&hpp, 0, sizeof(hpp));
-	ret = pci_get_hp_params(dev, &hpp);
-	if (ret)
-		return;
-
-	program_hpp_type2(dev, hpp.t2);
-	program_hpp_type1(dev, hpp.t1);
-	program_hpp_type0(dev, hpp.t0);
+	pci_acpi_program_hp_params(dev, &hp_ops);
 }
 
 static void pci_release_capabilities(struct pci_dev *dev)
diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index 7acc9f91e72b..c85378edf235 100644
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -124,26 +124,24 @@ struct hpp_type2 {
 	u32 sec_unc_err_mask_or;
 };
 
-struct hotplug_params {
-	struct hpp_type0 *t0;		/* Type0: NULL if not available */
-	struct hpp_type1 *t1;		/* Type1: NULL if not available */
-	struct hpp_type2 *t2;		/* Type2: NULL if not available */
-	struct hpp_type0 type0_data;
-	struct hpp_type1 type1_data;
-	struct hpp_type2 type2_data;
+struct hotplug_program_ops {
+	void (*program_type0)(struct pci_dev *dev, struct hpp_type0 *hpp);
+	void (*program_type1)(struct pci_dev *dev, struct hpp_type1 *hpp);
+	void (*program_type2)(struct pci_dev *dev, struct hpp_type2 *hpp);
 };
 
 #ifdef CONFIG_ACPI
 #include <linux/acpi.h>
-int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp);
+int pci_acpi_program_hp_params(struct pci_dev *dev,
+			       const struct hotplug_program_ops *hp_ops);
 bool pciehp_is_native(struct pci_dev *bridge);
 int acpi_get_hp_hw_control_from_firmware(struct pci_dev *bridge);
 bool shpchp_is_native(struct pci_dev *bridge);
 int acpi_pci_check_ejectable(struct pci_bus *pbus, acpi_handle handle);
 int acpi_pci_detect_ejectable(acpi_handle handle);
 #else
-static inline int pci_get_hp_params(struct pci_dev *dev,
-				    struct hotplug_params *hpp)
+int pci_acpi_program_hp_params(struct pci_dev *dev,
+			       const struct hotplug_program_ops *hp_ops);
 {
 	return -ENODEV;
 }
-- 
2.19.2

Re: [PATCH v1 2/3] PCI / ACPI: Remove the need for 'struct hotplug_params'

[Bjorn Helgaas]

On Fri, Feb 08, 2019 at 10:24:12AM -0600, Alexandru Gagniuc wrote:
> We used to first parse all the _HPP and _HPX tables before using the
> information to program registers of PCIe devices. Up until HPX type 2,
> there was only one structure of each type, so we could cheat and store
> it on the stack.
> 
> With HPX type 3 we get an arbitrary number of entries, so the above
> model doesn't scale that well. Instead of parsing all tables at once,
> parse and program each entry separately. For _HPP and _HPX 0 thru 2,
> this is functionally equivalent. The change enables the upcoming _HPX3
> to integrate more easily.

I think this is tremendous!  It's going to simplify this code
dramatically.  Two comments below.

> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>  drivers/pci/pci-acpi.c      | 108 +++++++++++++++++++-----------------
>  drivers/pci/probe.c         |  16 ++----
>  include/linux/pci_hotplug.h |  18 +++---
>  3 files changed, 72 insertions(+), 70 deletions(-)
> 
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index b25e5fa9d1c9..95f4f86d2f34 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -119,7 +119,7 @@ phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle)
>  }
>  
>  static acpi_status decode_type0_hpx_record(union acpi_object *record,
> -					   struct hotplug_params *hpx)
> +					   struct hpp_type0 *hpx0)
>  {
>  	int i;
>  	union acpi_object *fields = record->package.elements;
> @@ -132,12 +132,11 @@ static acpi_status decode_type0_hpx_record(union acpi_object *record,
>  		for (i = 2; i < 6; i++)
>  			if (fields[i].type != ACPI_TYPE_INTEGER)
>  				return AE_ERROR;
> -		hpx->t0 = &hpx->type0_data;
> -		hpx->t0->revision        = revision;
> -		hpx->t0->cache_line_size = fields[2].integer.value;
> -		hpx->t0->latency_timer   = fields[3].integer.value;
> -		hpx->t0->enable_serr     = fields[4].integer.value;
> -		hpx->t0->enable_perr     = fields[5].integer.value;
> +		hpx0->revision        = revision;
> +		hpx0->cache_line_size = fields[2].integer.value;
> +		hpx0->latency_timer   = fields[3].integer.value;
> +		hpx0->enable_serr     = fields[4].integer.value;
> +		hpx0->enable_perr     = fields[5].integer.value;
>  		break;
>  	default:
>  		printk(KERN_WARNING
> @@ -149,7 +148,7 @@ static acpi_status decode_type0_hpx_record(union acpi_object *record,
>  }
>  
>  static acpi_status decode_type1_hpx_record(union acpi_object *record,
> -					   struct hotplug_params *hpx)
> +					   struct hpp_type1 *hpx1)
>  {
>  	int i;
>  	union acpi_object *fields = record->package.elements;
> @@ -162,11 +161,10 @@ static acpi_status decode_type1_hpx_record(union acpi_object *record,
>  		for (i = 2; i < 5; i++)
>  			if (fields[i].type != ACPI_TYPE_INTEGER)
>  				return AE_ERROR;
> -		hpx->t1 = &hpx->type1_data;
> -		hpx->t1->revision      = revision;
> -		hpx->t1->max_mem_read  = fields[2].integer.value;
> -		hpx->t1->avg_max_split = fields[3].integer.value;
> -		hpx->t1->tot_max_split = fields[4].integer.value;
> +		hpx1->revision      = revision;
> +		hpx1->max_mem_read  = fields[2].integer.value;
> +		hpx1->avg_max_split = fields[3].integer.value;
> +		hpx1->tot_max_split = fields[4].integer.value;
>  		break;
>  	default:
>  		printk(KERN_WARNING
> @@ -178,7 +176,7 @@ static acpi_status decode_type1_hpx_record(union acpi_object *record,
>  }
>  
>  static acpi_status decode_type2_hpx_record(union acpi_object *record,
> -					   struct hotplug_params *hpx)
> +					   struct hpp_type2 *hpx2)
>  {
>  	int i;
>  	union acpi_object *fields = record->package.elements;
> @@ -191,24 +189,23 @@ static acpi_status decode_type2_hpx_record(union acpi_object *record,
>  		for (i = 2; i < 18; i++)
>  			if (fields[i].type != ACPI_TYPE_INTEGER)
>  				return AE_ERROR;
> -		hpx->t2 = &hpx->type2_data;
> -		hpx->t2->revision      = revision;
> -		hpx->t2->unc_err_mask_and      = fields[2].integer.value;
> -		hpx->t2->unc_err_mask_or       = fields[3].integer.value;
> -		hpx->t2->unc_err_sever_and     = fields[4].integer.value;
> -		hpx->t2->unc_err_sever_or      = fields[5].integer.value;
> -		hpx->t2->cor_err_mask_and      = fields[6].integer.value;
> -		hpx->t2->cor_err_mask_or       = fields[7].integer.value;
> -		hpx->t2->adv_err_cap_and       = fields[8].integer.value;
> -		hpx->t2->adv_err_cap_or        = fields[9].integer.value;
> -		hpx->t2->pci_exp_devctl_and    = fields[10].integer.value;
> -		hpx->t2->pci_exp_devctl_or     = fields[11].integer.value;
> -		hpx->t2->pci_exp_lnkctl_and    = fields[12].integer.value;
> -		hpx->t2->pci_exp_lnkctl_or     = fields[13].integer.value;
> -		hpx->t2->sec_unc_err_sever_and = fields[14].integer.value;
> -		hpx->t2->sec_unc_err_sever_or  = fields[15].integer.value;
> -		hpx->t2->sec_unc_err_mask_and  = fields[16].integer.value;
> -		hpx->t2->sec_unc_err_mask_or   = fields[17].integer.value;
> +		hpx2->revision      = revision;
> +		hpx2->unc_err_mask_and      = fields[2].integer.value;
> +		hpx2->unc_err_mask_or       = fields[3].integer.value;
> +		hpx2->unc_err_sever_and     = fields[4].integer.value;
> +		hpx2->unc_err_sever_or      = fields[5].integer.value;
> +		hpx2->cor_err_mask_and      = fields[6].integer.value;
> +		hpx2->cor_err_mask_or       = fields[7].integer.value;
> +		hpx2->adv_err_cap_and       = fields[8].integer.value;
> +		hpx2->adv_err_cap_or        = fields[9].integer.value;
> +		hpx2->pci_exp_devctl_and    = fields[10].integer.value;
> +		hpx2->pci_exp_devctl_or     = fields[11].integer.value;
> +		hpx2->pci_exp_lnkctl_and    = fields[12].integer.value;
> +		hpx2->pci_exp_lnkctl_or     = fields[13].integer.value;
> +		hpx2->sec_unc_err_sever_and = fields[14].integer.value;
> +		hpx2->sec_unc_err_sever_or  = fields[15].integer.value;
> +		hpx2->sec_unc_err_mask_and  = fields[16].integer.value;
> +		hpx2->sec_unc_err_mask_or   = fields[17].integer.value;
>  		break;
>  	default:
>  		printk(KERN_WARNING
> @@ -219,17 +216,18 @@ static acpi_status decode_type2_hpx_record(union acpi_object *record,
>  	return AE_OK;
>  }
>  
> -static acpi_status acpi_run_hpx(acpi_handle handle, struct hotplug_params *hpx)
> +static acpi_status acpi_run_hpx(struct pci_dev *dev, acpi_handle handle,
> +				const struct hotplug_program_ops *hp_ops)
>  {
>  	acpi_status status;
>  	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
>  	union acpi_object *package, *record, *fields;
> +	struct hpp_type0 hpx0;
> +	struct hpp_type1 hpx1;
> +	struct hpp_type2 hpx2;
>  	u32 type;
>  	int i;
>  
> -	/* Clear the return buffer with zeros */
> -	memset(hpx, 0, sizeof(struct hotplug_params));
> -
>  	status = acpi_evaluate_object(handle, "_HPX", NULL, &buffer);
>  	if (ACPI_FAILURE(status))
>  		return status;
> @@ -257,19 +255,25 @@ static acpi_status acpi_run_hpx(acpi_handle handle, struct hotplug_params *hpx)
>  		type = fields[0].integer.value;
>  		switch (type) {
>  		case 0:
> -			status = decode_type0_hpx_record(record, hpx);
> +			memset(&hpx0, 0, sizeof(hpx0));
> +			status = decode_type0_hpx_record(record, &hpx0);
>  			if (ACPI_FAILURE(status))
>  				goto exit;
> +			hp_ops->program_type0(dev, &hpx0);
>  			break;
>  		case 1:
> -			status = decode_type1_hpx_record(record, hpx);
> +			memset(&hpx1, 0, sizeof(hpx1));
> +			status = decode_type1_hpx_record(record, &hpx1);
>  			if (ACPI_FAILURE(status))
>  				goto exit;
> +			hp_ops->program_type1(dev, &hpx1);
>  			break;
>  		case 2:
> -			status = decode_type2_hpx_record(record, hpx);
> +			memset(&hpx2, 0, sizeof(hpx2));
> +			status = decode_type2_hpx_record(record, &hpx2);
>  			if (ACPI_FAILURE(status))
>  				goto exit;
> +			hp_ops->program_type2(dev, &hpx2);
>  			break;
>  		default:
>  			printk(KERN_ERR "%s: Type %d record not supported\n",
> @@ -283,14 +287,16 @@ static acpi_status acpi_run_hpx(acpi_handle handle, struct hotplug_params *hpx)
>  	return status;
>  }
>  
> -static acpi_status acpi_run_hpp(acpi_handle handle, struct hotplug_params *hpp)
> +static acpi_status acpi_run_hpp(struct pci_dev *dev, acpi_handle handle,
> +				const struct hotplug_program_ops *hp_ops)
>  {
>  	acpi_status status;
>  	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>  	union acpi_object *package, *fields;
> +	struct hpp_type0 hpp0;
>  	int i;
>  
> -	memset(hpp, 0, sizeof(struct hotplug_params));
> +	memset(&hpp0, 0, sizeof(hpp0));
>  
>  	status = acpi_evaluate_object(handle, "_HPP", NULL, &buffer);
>  	if (ACPI_FAILURE(status))
> @@ -311,12 +317,13 @@ static acpi_status acpi_run_hpp(acpi_handle handle, struct hotplug_params *hpp)
>  		}
>  	}
>  
> -	hpp->t0 = &hpp->type0_data;
> -	hpp->t0->revision        = 1;
> -	hpp->t0->cache_line_size = fields[0].integer.value;
> -	hpp->t0->latency_timer   = fields[1].integer.value;
> -	hpp->t0->enable_serr     = fields[2].integer.value;
> -	hpp->t0->enable_perr     = fields[3].integer.value;
> +	hpp0.revision        = 1;
> +	hpp0.cache_line_size = fields[0].integer.value;
> +	hpp0.latency_timer   = fields[1].integer.value;
> +	hpp0.enable_serr     = fields[2].integer.value;
> +	hpp0.enable_perr     = fields[3].integer.value;
> +
> +	hp_ops->program_type0(dev, &hpp0);
>  
>  exit:
>  	kfree(buffer.pointer);
> @@ -328,7 +335,8 @@ static acpi_status acpi_run_hpp(acpi_handle handle, struct hotplug_params *hpp)
>   * @dev - the pci_dev for which we want parameters
>   * @hpp - allocated by the caller
>   */
> -int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp)
> +int pci_acpi_program_hp_params(struct pci_dev *dev,
> +			       const struct hotplug_program_ops *hp_ops)
>  {
>  	acpi_status status;
>  	acpi_handle handle, phandle;
> @@ -351,10 +359,10 @@ int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp)
>  	 * this pci dev.
>  	 */
>  	while (handle) {
> -		status = acpi_run_hpx(handle, hpp);
> +		status = acpi_run_hpx(dev, handle, hp_ops);
>  		if (ACPI_SUCCESS(status))
>  			return 0;
> -		status = acpi_run_hpp(handle, hpp);
> +		status = acpi_run_hpp(dev, handle, hp_ops);
>  		if (ACPI_SUCCESS(status))
>  			return 0;
>  		if (acpi_is_root_bridge(handle))
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 257b9f6f2ebb..527c209f0c94 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2131,8 +2131,11 @@ static void pci_configure_eetlp_prefix(struct pci_dev *dev)
>  
>  static void pci_configure_device(struct pci_dev *dev)
>  {
> -	struct hotplug_params hpp;
> -	int ret;
> +	static const struct hotplug_program_ops hp_ops = {
> +		.program_type0 = program_hpp_type0,
> +		.program_type1 = program_hpp_type1,
> +		.program_type2 = program_hpp_type2,
> +	};

What if we just moved program_hpp_type0(), etc from probe.c to
pci-acpi.c?  The only reason I see to have it in probe.c is for
pci_default_type0, and I think that is a pretty obtuse way of doing
default configuration.  I would have no problem at all just hardcoding
those defaults in probe.c and then potentially having them overwritten
by _HPP/_HPX.

If we moved program_hpp_type0(), etc to pci-acpi.c, we could get rid
of the function pointers and all the ACPI-related code would be in one
place.

>  	pci_configure_mps(dev);
>  	pci_configure_extended_tags(dev, NULL);
> @@ -2140,14 +2143,7 @@ static void pci_configure_device(struct pci_dev *dev)
>  	pci_configure_ltr(dev);
>  	pci_configure_eetlp_prefix(dev);
>  
> -	memset(&hpp, 0, sizeof(hpp));
> -	ret = pci_get_hp_params(dev, &hpp);
> -	if (ret)
> -		return;
> -
> -	program_hpp_type2(dev, hpp.t2);
> -	program_hpp_type1(dev, hpp.t1);
> -	program_hpp_type0(dev, hpp.t0);
> +	pci_acpi_program_hp_params(dev, &hp_ops);
>  }
>  
>  static void pci_release_capabilities(struct pci_dev *dev)
> diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
> index 7acc9f91e72b..c85378edf235 100644
> --- a/include/linux/pci_hotplug.h
> +++ b/include/linux/pci_hotplug.h
> @@ -124,26 +124,24 @@ struct hpp_type2 {
>  	u32 sec_unc_err_mask_or;
>  };
>  
> -struct hotplug_params {
> -	struct hpp_type0 *t0;		/* Type0: NULL if not available */
> -	struct hpp_type1 *t1;		/* Type1: NULL if not available */
> -	struct hpp_type2 *t2;		/* Type2: NULL if not available */
> -	struct hpp_type0 type0_data;
> -	struct hpp_type1 type1_data;
> -	struct hpp_type2 type2_data;
> +struct hotplug_program_ops {
> +	void (*program_type0)(struct pci_dev *dev, struct hpp_type0 *hpp);
> +	void (*program_type1)(struct pci_dev *dev, struct hpp_type1 *hpp);
> +	void (*program_type2)(struct pci_dev *dev, struct hpp_type2 *hpp);
>  };

I think a lot of this stuff is only used in drivers/pci, so it could
be moved from include/linux/pci_hotplug.h to drivers/pci/pci.h.

>  #ifdef CONFIG_ACPI
>  #include <linux/acpi.h>
> -int pci_get_hp_params(struct pci_dev *dev, struct hotplug_params *hpp);
> +int pci_acpi_program_hp_params(struct pci_dev *dev,
> +			       const struct hotplug_program_ops *hp_ops);
>  bool pciehp_is_native(struct pci_dev *bridge);
>  int acpi_get_hp_hw_control_from_firmware(struct pci_dev *bridge);
>  bool shpchp_is_native(struct pci_dev *bridge);
>  int acpi_pci_check_ejectable(struct pci_bus *pbus, acpi_handle handle);
>  int acpi_pci_detect_ejectable(acpi_handle handle);
>  #else
> -static inline int pci_get_hp_params(struct pci_dev *dev,
> -				    struct hotplug_params *hpp)
> +int pci_acpi_program_hp_params(struct pci_dev *dev,
> +			       const struct hotplug_program_ops *hp_ops);
>  {
>  	return -ENODEV;
>  }
> -- 
> 2.19.2
> 

Re: [PATCH v1 2/3] PCI / ACPI: Remove the need for 'struct hotplug_params'

[Bjorn Helgaas]

On Fri, Apr 19, 2019 at 03:07:45PM -0500, Bjorn Helgaas wrote:
> On Fri, Feb 08, 2019 at 10:24:12AM -0600, Alexandru Gagniuc wrote:
> > We used to first parse all the _HPP and _HPX tables before using the
> > information to program registers of PCIe devices. Up until HPX type 2,
> > there was only one structure of each type, so we could cheat and store
> > it on the stack.
> > 
> > With HPX type 3 we get an arbitrary number of entries, so the above
> > model doesn't scale that well. Instead of parsing all tables at once,
> > parse and program each entry separately. For _HPP and _HPX 0 thru 2,
> > this is functionally equivalent. The change enables the upcoming _HPX3
> > to integrate more easily.
> 
> I think this is tremendous!  It's going to simplify this code
> dramatically.  Two comments below.

> >  static void pci_configure_device(struct pci_dev *dev)
> >  {
> > -	struct hotplug_params hpp;
> > -	int ret;
> > +	static const struct hotplug_program_ops hp_ops = {
> > +		.program_type0 = program_hpp_type0,
> > +		.program_type1 = program_hpp_type1,
> > +		.program_type2 = program_hpp_type2,
> > +	};
> 
> What if we just moved program_hpp_type0(), etc from probe.c to
> pci-acpi.c?  The only reason I see to have it in probe.c is for
> pci_default_type0, and I think that is a pretty obtuse way of doing
> default configuration.  I would have no problem at all just hardcoding
> those defaults in probe.c and then potentially having them overwritten
> by _HPP/_HPX.

Actually, never mind about this.  This would be a perfect project for
mentoring a Linux newbie.

I'll merge this series as-is and any restructuring/cleanup can happen
later, since it's not related to this series anyway.

Bjorn

[PATCH v1 3/3] PCI / ACPI: Implement Type 3 _HPX record

[Alexandru Gagniuc]

_HPX Type 3 is intended to be more generic and allow configuration of
settings not possible with Type 2 tables. For example, FW could ensure
that the completion timeout value is set accordingly throughout the PCI
tree.

Implement support for _HPX3 tables.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
---
 drivers/pci/pci-acpi.c      |  63 ++++++++++++++++++++
 drivers/pci/probe.c         | 114 ++++++++++++++++++++++++++++++++++++
 include/linux/pci_hotplug.h |  56 ++++++++++++++++++
 3 files changed, 233 insertions(+)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 95f4f86d2f34..03e02dd6c1d9 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -216,6 +216,64 @@ static acpi_status decode_type2_hpx_record(union acpi_object *record,
 	return AE_OK;
 }
 
+static void parse_hpx3_register(struct hpx_type3 *hpx3_reg,
+				union acpi_object *reg_fields)
+{
+	hpx3_reg->device_type            = reg_fields[0].integer.value;
+	hpx3_reg->function_type          = reg_fields[1].integer.value;
+	hpx3_reg->config_space_location  = reg_fields[2].integer.value;
+	hpx3_reg->pci_exp_cap_id         = reg_fields[3].integer.value;
+	hpx3_reg->pci_exp_cap_ver        = reg_fields[4].integer.value;
+	hpx3_reg->pci_exp_vendor_id      = reg_fields[5].integer.value;
+	hpx3_reg->dvsec_id               = reg_fields[6].integer.value;
+	hpx3_reg->dvsec_rev              = reg_fields[7].integer.value;
+	hpx3_reg->match_offset           = reg_fields[8].integer.value;
+	hpx3_reg->match_mask_and         = reg_fields[9].integer.value;
+	hpx3_reg->match_value            = reg_fields[10].integer.value;
+	hpx3_reg->reg_offset             = reg_fields[11].integer.value;
+	hpx3_reg->reg_mask_and           = reg_fields[12].integer.value;
+	hpx3_reg->reg_mask_or            = reg_fields[13].integer.value;
+}
+
+static acpi_status program_type3_hpx_record(struct pci_dev *dev,
+					   union acpi_object *record,
+					   const struct hotplug_program_ops *hp_ops)
+{
+	union acpi_object *fields = record->package.elements;
+	u32 desc_count, expected_length, revision;
+	union acpi_object *reg_fields;
+	struct hpx_type3 hpx3;
+	int i;
+
+	revision = fields[1].integer.value;
+	switch (revision) {
+	case 1:
+		desc_count = fields[2].integer.value;
+		expected_length = 3 + desc_count * 14;
+
+		if (record->package.count != expected_length)
+			return AE_ERROR;
+
+		for (i = 2; i < expected_length; i++)
+			if (fields[i].type != ACPI_TYPE_INTEGER)
+				return AE_ERROR;
+
+		for (i = 0; i < desc_count; i++) {
+			reg_fields = fields + 3 + i * 14;
+			parse_hpx3_register(&hpx3, reg_fields);
+			hp_ops->program_type3(dev, &hpx3);
+		}
+
+		break;
+	default:
+		printk(KERN_WARNING
+			"%s: Type 3 Revision %d record not supported\n",
+			__func__, revision);
+		return AE_ERROR;
+	}
+	return AE_OK;
+}
+
 static acpi_status acpi_run_hpx(struct pci_dev *dev, acpi_handle handle,
 				const struct hotplug_program_ops *hp_ops)
 {
@@ -275,6 +333,11 @@ static acpi_status acpi_run_hpx(struct pci_dev *dev, acpi_handle handle,
 				goto exit;
 			hp_ops->program_type2(dev, &hpx2);
 			break;
+		case 3:
+			status = program_type3_hpx_record(dev, record, hp_ops);
+			if (ACPI_FAILURE(status))
+				goto exit;
+			break;
 		default:
 			printk(KERN_ERR "%s: Type %d record not supported\n",
 			       __func__, type);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 527c209f0c94..5f3b6b1cd762 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1979,6 +1979,119 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
+static u16 hpx3_device_type(struct pci_dev *dev)
+{
+	u16 pcie_type = pci_pcie_type(dev);
+	const int pcie_to_hpx3_type[] = {
+		[PCI_EXP_TYPE_ENDPOINT]    = HPX_TYPE_ENDPOINT,
+		[PCI_EXP_TYPE_LEG_END]     = HPX_TYPE_LEG_END,
+		[PCI_EXP_TYPE_RC_END]      = HPX_TYPE_RC_END,
+		[PCI_EXP_TYPE_RC_EC]       = HPX_TYPE_RC_EC,
+		[PCI_EXP_TYPE_ROOT_PORT]   = HPX_TYPE_ROOT_PORT,
+		[PCI_EXP_TYPE_UPSTREAM]    = HPX_TYPE_UPSTREAM,
+		[PCI_EXP_TYPE_DOWNSTREAM]  = HPX_TYPE_DOWNSTREAM,
+		[PCI_EXP_TYPE_PCI_BRIDGE]  = HPX_TYPE_PCI_BRIDGE,
+		[PCI_EXP_TYPE_PCIE_BRIDGE] = HPX_TYPE_PCIE_BRIDGE,
+	};
+
+	if (pcie_type >= ARRAY_SIZE(pcie_to_hpx3_type))
+		return 0;
+
+	return pcie_to_hpx3_type[pcie_type];
+}
+
+static u8 hpx3_function_type(struct pci_dev *dev)
+{
+	if (dev->is_virtfn)
+		return HPX_FN_SRIOV_VIRT;
+	else if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_SRIOV) > 0)
+		return HPX_FN_SRIOV_PHYS;
+	else
+		return HPX_FN_NORMAL;
+}
+
+static bool hpx3_cap_ver_matches(u8 pcie_cap_id, u8 hpx3_cap_id)
+{
+	u8 cap_ver = hpx3_cap_id & 0xf;
+
+	if ((hpx3_cap_id & BIT(4)) && cap_ver >= pcie_cap_id)
+		return true;
+	else if (cap_ver == pcie_cap_id)
+		return true;
+
+	return false;
+}
+
+static void program_hpx_type3_register(struct pci_dev *dev,
+				       const struct hpx_type3 *reg)
+{
+	u32 match_reg, write_reg, header, orig_value;
+	u16 pos;
+
+	if (!(hpx3_device_type(dev) & reg->device_type))
+		return;
+
+	if (!(hpx3_function_type(dev) & reg->function_type))
+		return;
+
+	switch (reg->config_space_location) {
+	case HPX_CFG_PCICFG:
+		pos = 0;
+		break;
+	case HPX_CFG_PCIE_CAP:
+		pos = pci_find_capability(dev, reg->pci_exp_cap_id);
+		if (pos == 0)
+			return;
+
+		break;
+	case HPX_CFG_PCIE_CAP_EXT:
+		pos = pci_find_ext_capability(dev, reg->pci_exp_cap_id);
+		if (pos == 0)
+			return;
+
+		pci_read_config_dword(dev, pos, &header);
+		if (!hpx3_cap_ver_matches(PCI_EXT_CAP_VER(header),
+					  reg->pci_exp_cap_ver))
+			return;
+
+		break;
+	case HPX_CFG_VEND_CAP:	/* Fall through */
+	case HPX_CFG_DVSEC:	/* Fall through */
+	default:
+		pci_warn(dev, "Encontered _HPX type 3 with unsupported config space location");
+		return;
+	}
+
+	pci_read_config_dword(dev, pos + reg->match_offset, &match_reg);
+
+	if ((match_reg & reg->match_mask_and) != reg->match_value)
+		return;
+
+	pci_read_config_dword(dev, pos + reg->reg_offset, &write_reg);
+	orig_value = write_reg;
+	write_reg &= reg->reg_mask_and;
+	write_reg |= reg->reg_mask_or;
+
+	if (orig_value == write_reg)
+		return;
+
+	pci_write_config_dword(dev, pos + reg->reg_offset, write_reg);
+
+	pci_dbg(dev, "Applied _HPX3 at [0x%x]: 0x%08x -> 0x%08x",
+		pos, orig_value, write_reg);
+}
+
+static void program_hpx_type3(struct pci_dev *dev, struct hpx_type3 *hpx3)
+{
+	if (!hpx3)
+		return;
+
+	if (!pci_is_pcie(dev))
+		return;
+
+	program_hpx_type3_register(dev, hpx3);
+}
+
 int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
 {
 	struct pci_host_bridge *host;
@@ -2135,6 +2248,7 @@ static void pci_configure_device(struct pci_dev *dev)
 		.program_type0 = program_hpp_type0,
 		.program_type1 = program_hpp_type1,
 		.program_type2 = program_hpp_type2,
+		.program_type3 = program_hpx_type3,
 	};
 
 	pci_configure_mps(dev);
diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h
index c85378edf235..24409dccb7cf 100644
--- a/include/linux/pci_hotplug.h
+++ b/include/linux/pci_hotplug.h
@@ -124,10 +124,66 @@ struct hpp_type2 {
 	u32 sec_unc_err_mask_or;
 };
 
+/*
+ * PCI Express Setting Record (Type 3)
+ * Should be good for another thirteen years until we need to handle some
+ * situation we didn't think of today.
+ */
+struct hpx_type3 {
+	u16 device_type;
+	u16 function_type;
+	u16 config_space_location;
+	u16 pci_exp_cap_id;
+	u16 pci_exp_cap_ver;
+	u16 pci_exp_vendor_id;
+	u16 dvsec_id;
+	u16 dvsec_rev;
+	u16 match_offset;
+	u32 match_mask_and;
+	u32 match_value;
+	u16 reg_offset;
+	u32 reg_mask_and;
+	u32 reg_mask_or;
+	struct list_head list;
+};
+
 struct hotplug_program_ops {
 	void (*program_type0)(struct pci_dev *dev, struct hpp_type0 *hpp);
 	void (*program_type1)(struct pci_dev *dev, struct hpp_type1 *hpp);
 	void (*program_type2)(struct pci_dev *dev, struct hpp_type2 *hpp);
+	void (*program_type3)(struct pci_dev *dev, struct hpx_type3 *hpp);
+};
+
+/*
+ * ACPI: The world leader in _almost_ getting bitfields right
+ * Would be nice if these matched the PCI_EXP_FLAGS_TYPE, but that would have
+ * made things too simple, and would actually have made sense. Can't have that!
+ */
+enum hpx_type3_dev_type {
+	HPX_TYPE_ENDPOINT	= BIT(0),
+	HPX_TYPE_LEG_END	= BIT(1),
+	HPX_TYPE_RC_END		= BIT(2),
+	HPX_TYPE_RC_EC		= BIT(3),
+	HPX_TYPE_ROOT_PORT	= BIT(4),
+	HPX_TYPE_UPSTREAM	= BIT(5),
+	HPX_TYPE_DOWNSTREAM	= BIT(6),
+	HPX_TYPE_PCI_BRIDGE	= BIT(7),
+	HPX_TYPE_PCIE_BRIDGE	= BIT(8),
+};
+
+enum hpx_type3_fn_type {
+	HPX_FN_NORMAL		= BIT(0),
+	HPX_FN_SRIOV_PHYS	= BIT(1),
+	HPX_FN_SRIOV_VIRT	= BIT(2),
+};
+
+enum hpx_type3_cfg_loc {
+	HPX_CFG_PCICFG		= 0,
+	HPX_CFG_PCIE_CAP	= 1,
+	HPX_CFG_PCIE_CAP_EXT	= 2,
+	HPX_CFG_VEND_CAP	= 3,
+	HPX_CFG_DVSEC		= 4,
+	HPX_CFG_MAX,
 };
 
 #ifdef CONFIG_ACPI
-- 
2.19.2