[V8 PATCH 0/3] Introduce ACPI support for ahci_platform driver

[V8 PATCH 0/3] Introduce ACPI support for ahci_platform driver

[Suravee Suthikulpanit]

This patch series introduce ACPI support for AHCI platform driver.
Existing ACPI support for AHCI assumes the device controller is a PCI device.
Since there is no ACPI _CID for generic AHCI controller, the driver
could not use it for matching devices. Therefore, this patch introduces
a mechanism for drivers to match devices using ACPI _CLS method.
_CLS contains PCI-defined class-code.

This patch series also modifies ACPI modalias to add class-code to the
exisiting format, which currently only uses _HID and _CIDs. This is required
to support loadable modules w/ _CLS.

This is rebased from and tested with:

    http://git.linaro.org/leg/acpi/acpi.git acpi-5.1-v11

This topic was discussed earlier here (as part of introducing support for
AMD Seattle SATA controller):

    http://marc.info/?l=linux-arm-kernel&m=141083492521584&w=2

Changes from V7 (https://lkml.org/lkml/2015/3/26/592)
	* [1/3] Return AE_AML_OPERAND_TYPE when _CLS package containing
	  invalid type (per Robert Moore suggestion). 
	* [2/3] Fixed build error due missing ACPI_DEVICE_CLASS definition
	  when disabling ACPI.

Changes from V6 (https://lkml.org/lkml/2015/3/25/797)
	* Adding Acked-by Mika, and Reviewed-by Hanjun
	* Minor clen up to use lower case 0xffffff for cls_msk
	  (per Mika suggestions).
	* Modify the ACPI_DEVICE_CLASS macro to use designated initializer
	  (per Mika suggestions).

Changes from V5 (https://lkml.org/lkml/2015/3/6/24)
	* Rebased and tested with acpi-5.1-v11
	* Splitting up the ACPICA changes into a separate patch [1/3].
	  (per Mika suggestion)
	* Adding class-code mask support (per Mika suggestion)
	* Use macro to define struct acpi_device_id entry (per Mika suggestion)
	* Note: Mika also recommend reordering the member of struct acpi_device_id
	  and define a macro to be used for declaring each table entry. This is a
	  large amount of changes, and will be done separtely from this patch series.

Changes from V4 (https://lkml.org/lkml/2015/3/2/56)
	* [1/2] Bug fixed: Reorder the declaration of
	  struct acpi_pnp_device_id cls in the struct acpi_device_info
	  (include/acpi/actypes.h) since compatible_id_list must be last one.
	* [2/2] Added Acked-by: Tejun Heo <tj@kernel.org>

Changes from V3 (https://lkml.org/lkml/2015/2/8/106)
	* Instead of introducing new structure acpi_device_cls, add cls into
	  the acpi_device_id, and modify the __acpi_match_device
	  to also match for cls. (per Mika suggestion.)
	* Add loadable module support, which requires changes in ACPI
	  modalias. (per Mika suggestion.)
	* Rebased and tested with acpi-5.1-v9

Changes from V2 (https://lkml.org/lkml/2015/1/5/662)
	* Update with review comment from Rafael in patch 1/2
	* Rebased and tested with acpi-5.1-v8

Changes from V1 (https://lkml.org/lkml/2014/12/19/345)
	* Rebased to 3.19.0-rc2
	* Change from acpi_cls in device_driver to acpi_match_cls (Hanjun comment)
	* Change the matching logic in acpi_driver_match_device() due to the new
	  special PRP0001 _HID.
	* Simplify the return type of acpi_match_device_cls() to boolean.

Changes from RFC (https://lkml.org/lkml/2014/12/17/446)
	* Remove #ifdef and make non-ACPI version of the acpi_match_device_cls
	  as inline. (per Arnd)
	* Simplify logic to retrieve and evaluate _CLS handle. (per Hanjun)

Suravee Suthikulpanit (3):
  ACPICA: Add ACPI _CLS processing
  ACPI / scan: Add support for ACPI _CLS device matching
  ata: ahci_platform: Add ACPI _CLS matching

 drivers/acpi/acpica/acutils.h     |  3 ++
 drivers/acpi/acpica/nsxfname.c    | 21 +++++++++--
 drivers/acpi/acpica/utids.c       | 73 +++++++++++++++++++++++++++++++++++++++
 drivers/acpi/scan.c               | 36 ++++++++++++++++---
 drivers/ata/Kconfig               |  2 +-
 drivers/ata/ahci_platform.c       |  9 +++++
 include/acpi/acnames.h            |  1 +
 include/acpi/actypes.h            |  4 ++-
 include/linux/acpi.h              | 14 ++++++++
 include/linux/mod_devicetable.h   |  2 ++
 scripts/mod/devicetable-offsets.c |  2 ++
 scripts/mod/file2alias.c          | 32 +++++++++++++++--
 12 files changed, 189 insertions(+), 10 deletions(-)

-- 
2.1.0

[V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing

[Suravee Suthikulpanit]

ACPI Device configuration often contain _CLS object to suppy PCI-defined
class code for the device. This patch introduces logic to process the _CLS
object.

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/acpi/acpica/acutils.h  |  3 ++
 drivers/acpi/acpica/nsxfname.c | 21 ++++++++++--
 drivers/acpi/acpica/utids.c    | 73 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/acpica/acutils.h b/drivers/acpi/acpica/acutils.h
index c2f03e8..2aef850 100644
--- a/drivers/acpi/acpica/acutils.h
+++ b/drivers/acpi/acpica/acutils.h
@@ -430,6 +430,9 @@ acpi_status
 acpi_ut_execute_CID(struct acpi_namespace_node *device_node,
 		    struct acpi_pnp_device_id_list ** return_cid_list);
 
+acpi_status
+acpi_ut_execute_CLS(struct acpi_namespace_node *device_node,
+		    struct acpi_pnp_device_id **return_id);
 /*
  * utlock - reader/writer locks
  */
diff --git a/drivers/acpi/acpica/nsxfname.c b/drivers/acpi/acpica/nsxfname.c
index d66c326..590ef06 100644
--- a/drivers/acpi/acpica/nsxfname.c
+++ b/drivers/acpi/acpica/nsxfname.c
@@ -276,11 +276,12 @@ acpi_get_object_info(acpi_handle handle,
 	struct acpi_pnp_device_id *hid = NULL;
 	struct acpi_pnp_device_id *uid = NULL;
 	struct acpi_pnp_device_id *sub = NULL;
+	struct acpi_pnp_device_id *cls = NULL;
 	char *next_id_string;
 	acpi_object_type type;
 	acpi_name name;
 	u8 param_count = 0;
-	u8 valid = 0;
+	u16 valid = 0;
 	u32 info_size;
 	u32 i;
 	acpi_status status;
@@ -320,7 +321,7 @@ acpi_get_object_info(acpi_handle handle,
 	if ((type == ACPI_TYPE_DEVICE) || (type == ACPI_TYPE_PROCESSOR)) {
 		/*
 		 * Get extra info for ACPI Device/Processor objects only:
-		 * Run the Device _HID, _UID, _SUB, and _CID methods.
+		 * Run the Device _HID, _UID, _SUB, _CID and _CLS methods.
 		 *
 		 * Note: none of these methods are required, so they may or may
 		 * not be present for this device. The Info->Valid bitfield is used
@@ -351,6 +352,14 @@ acpi_get_object_info(acpi_handle handle,
 			valid |= ACPI_VALID_SUB;
 		}
 
+		/* Execute the Device._CLS method */
+
+		status = acpi_ut_execute_CLS(node, &cls);
+		if (ACPI_SUCCESS(status)) {
+			info_size += cls->length;
+			valid |= ACPI_VALID_CLS;
+		}
+
 		/* Execute the Device._CID method */
 
 		status = acpi_ut_execute_CID(node, &cid_list);
@@ -468,6 +477,11 @@ acpi_get_object_info(acpi_handle handle,
 							sub, next_id_string);
 	}
 
+	if (cls) {
+		next_id_string = acpi_ns_copy_device_id(&info->cls,
+							cls, next_id_string);
+	}
+
 	if (cid_list) {
 		info->compatible_id_list.count = cid_list->count;
 		info->compatible_id_list.list_size = cid_list->list_size;
@@ -507,6 +521,9 @@ cleanup:
 	if (sub) {
 		ACPI_FREE(sub);
 	}
+	if (cls) {
+		ACPI_FREE(cls);
+	}
 	if (cid_list) {
 		ACPI_FREE(cid_list);
 	}
diff --git a/drivers/acpi/acpica/utids.c b/drivers/acpi/acpica/utids.c
index 27431cf..9745065 100644
--- a/drivers/acpi/acpica/utids.c
+++ b/drivers/acpi/acpica/utids.c
@@ -416,3 +416,76 @@ cleanup:
 	acpi_ut_remove_reference(obj_desc);
 	return_ACPI_STATUS(status);
 }
+
+/*******************************************************************************
+ *
+ * FUNCTION:    acpi_ut_execute_CLS
+ *
+ * PARAMETERS:  device_node         - Node for the device
+ *              return_id           - Where the string UID is returned
+ *
+ * RETURN:      Status
+ *
+ * DESCRIPTION: Executes the _CLS control method that returns PCI-defined
+ *              class code of the device. The ACPI spec define _CLS as a
+ *              package with three integers. The returned string has format:
+ *
+ *                      "bbsspp"
+ *              where:
+ *                  bb = Base-class code
+ *                  ss = Sub-class code
+ *                  pp = Programming Interface code
+ *
+ ******************************************************************************/
+
+acpi_status
+acpi_ut_execute_CLS(struct acpi_namespace_node *device_node,
+		    struct acpi_pnp_device_id **return_id)
+{
+	struct acpi_pnp_device_id *cls;
+	union acpi_operand_object *obj_desc;
+	union acpi_operand_object **cls_objects;
+	acpi_status status;
+
+	ACPI_FUNCTION_TRACE(ut_execute_CLS);
+	status = acpi_ut_evaluate_object(device_node, METHOD_NAME__CLS,
+					 ACPI_BTYPE_PACKAGE, &obj_desc);
+	if (ACPI_FAILURE(status))
+		return_ACPI_STATUS(status);
+
+	cls_objects = obj_desc->package.elements;
+
+	if (obj_desc->common.type == ACPI_TYPE_PACKAGE &&
+	    obj_desc->package.count == 3 &&
+	    cls_objects[0]->common.type == ACPI_TYPE_INTEGER &&
+	    cls_objects[1]->common.type == ACPI_TYPE_INTEGER &&
+	    cls_objects[2]->common.type == ACPI_TYPE_INTEGER) {
+
+		/* Allocate a buffer for the CLS */
+		cls = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_pnp_device_id) +
+					 (acpi_size) 7);
+		if (!cls) {
+			status = AE_NO_MEMORY;
+			goto cleanup;
+		}
+
+		cls->string =
+		    ACPI_ADD_PTR(char, cls, sizeof(struct acpi_pnp_device_id));
+
+		sprintf(cls->string, "%02x%02x%02x",
+			(u8)ACPI_TO_INTEGER(cls_objects[0]->integer.value),
+			(u8)ACPI_TO_INTEGER(cls_objects[1]->integer.value),
+			(u8)ACPI_TO_INTEGER(cls_objects[2]->integer.value));
+		cls->length = 7;
+		*return_id = cls;
+	} else {
+		status = AE_AML_OPERAND_TYPE;
+	}
+
+cleanup:
+
+	/* On exit, we must delete the return object */
+
+	acpi_ut_remove_reference(obj_desc);
+	return_ACPI_STATUS(status);
+}
-- 
2.1.0

[V8 PATCH 2/3] ACPI / scan: Add support for ACPI _CLS device matching

[Suravee Suthikulpanit]

Device drivers typically use ACPI _HIDs/_CIDs listed in struct device_driver
acpi_match_table to match devices. However, for generic drivers, we do not
want to list _HID for all supported devices. Also, certain classes of devices
do not have _CID (e.g. SATA, USB). Instead, we can leverage ACPI _CLS,
which specifies PCI-defined class code (i.e. base-class, subclass and
programming interface). This patch adds support for matching ACPI devices using
the _CLS method.

To support loadable module, current design uses _HID or _CID to match device's
modalias. With the new way of matching with _CLS this would requires modification
to the current ACPI modalias key to include _CLS. This patch appends PCI-defined
class-code to the existing ACPI modalias as following.

    acpi:<HID>:<CID1>:<CID2>:..:<CIDn>:<bbsspp>:
E.g:
    # cat /sys/devices/platform/AMDI0600:00/modalias
    acpi:AMDI0600:010601:

where bb is th base-class code, ss is te sub-class code, and pp is the
programming interface code

Since there would not be _HID/_CID in the ACPI matching table of the driver,
this patch adds a field to acpi_device_id to specify the matching _CLS.

    static const struct acpi_device_id ahci_acpi_match[] = {
        { ACPI_DEVICE_CLASS(PCI_CLASS_STORAGE_SATA_AHCI, 0xffffff) },
        {},
    };

In this case, the corresponded entry in modules.alias file would be:

    alias acpi*:010601:* ahci_platform

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/acpi/scan.c               | 36 ++++++++++++++++++++++++++++++++----
 include/acpi/acnames.h            |  1 +
 include/acpi/actypes.h            |  4 +++-
 include/linux/acpi.h              | 14 ++++++++++++++
 include/linux/mod_devicetable.h   |  2 ++
 scripts/mod/devicetable-offsets.c |  2 ++
 scripts/mod/file2alias.c          | 32 ++++++++++++++++++++++++++++++--
 7 files changed, 84 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index c03cadb..d8551cd 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -900,6 +900,29 @@ static void acpi_device_remove_files(struct acpi_device *dev)
 			ACPI Bus operations
    -------------------------------------------------------------------------- */
 
+static bool __acpi_match_device_cls(const struct acpi_device_id *id,
+				    struct acpi_hardware_id *hwid)
+{
+	int i, msk, byte_shift;
+	char buf[3];
+
+	if (!id->cls)
+		return false;
+
+	/* Apply class-code bitmask, before checking each class-code byte */
+	for (i = 1; i <= 3; i++) {
+		byte_shift = 8 * (3 - i);
+		msk = (id->cls_msk >> byte_shift) & 0xFF;
+		if (!msk)
+			continue;
+
+		sprintf(buf, "%02x", (id->cls >> byte_shift) & msk);
+		if (strncmp(buf, &hwid->id[(i - 1) * 2], 2))
+			return false;
+	}
+	return true;
+}
+
 static const struct acpi_device_id *__acpi_match_device(
 	struct acpi_device *device, const struct acpi_device_id *ids)
 {
@@ -913,11 +936,14 @@ static const struct acpi_device_id *__acpi_match_device(
 	if (!device->status.present)
 		return NULL;
 
-	for (id = ids; id->id[0]; id++)
-		list_for_each_entry(hwid, &device->pnp.ids, list)
-			if (!strcmp((char *) id->id, hwid->id))
+	for (id = ids; id->id[0] || id->cls; id++) {
+		list_for_each_entry(hwid, &device->pnp.ids, list) {
+			if (id->id[0] && !strcmp((char *) id->id, hwid->id))
 				return id;
-
+			else if (__acpi_match_device_cls(id, hwid))
+				return id;
+		}
+	}
 	return NULL;
 }
 
@@ -2032,6 +2058,8 @@ static void acpi_set_pnp_ids(acpi_handle handle, struct acpi_device_pnp *pnp,
 		if (info->valid & ACPI_VALID_UID)
 			pnp->unique_id = kstrdup(info->unique_id.string,
 							GFP_KERNEL);
+		if (info->valid & ACPI_VALID_CLS)
+			acpi_add_id(pnp, info->cls.string);
 
 		kfree(info);
 
diff --git a/include/acpi/acnames.h b/include/acpi/acnames.h
index 273de70..b52c0dc 100644
--- a/include/acpi/acnames.h
+++ b/include/acpi/acnames.h
@@ -51,6 +51,7 @@
 #define METHOD_NAME__BBN        "_BBN"
 #define METHOD_NAME__CBA        "_CBA"
 #define METHOD_NAME__CID        "_CID"
+#define METHOD_NAME__CLS        "_CLS"
 #define METHOD_NAME__CRS        "_CRS"
 #define METHOD_NAME__DDN        "_DDN"
 #define METHOD_NAME__HID        "_HID"
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index b034f10..ab3dac8 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -1148,7 +1148,7 @@ struct acpi_device_info {
 	u32 name;		/* ACPI object Name */
 	acpi_object_type type;	/* ACPI object Type */
 	u8 param_count;		/* If a method, required parameter count */
-	u8 valid;		/* Indicates which optional fields are valid */
+	u16 valid;		/* Indicates which optional fields are valid */
 	u8 flags;		/* Miscellaneous info */
 	u8 highest_dstates[4];	/* _sx_d values: 0xFF indicates not valid */
 	u8 lowest_dstates[5];	/* _sx_w values: 0xFF indicates not valid */
@@ -1157,6 +1157,7 @@ struct acpi_device_info {
 	struct acpi_pnp_device_id hardware_id;	/* _HID value */
 	struct acpi_pnp_device_id unique_id;	/* _UID value */
 	struct acpi_pnp_device_id subsystem_id;	/* _SUB value */
+	struct acpi_pnp_device_id cls;		/* _CLS value */
 	struct acpi_pnp_device_id_list compatible_id_list;	/* _CID list <must be last> */
 };
 
@@ -1174,6 +1175,7 @@ struct acpi_device_info {
 #define ACPI_VALID_CID                  0x20
 #define ACPI_VALID_SXDS                 0x40
 #define ACPI_VALID_SXWS                 0x80
+#define ACPI_VALID_CLS                  0x100
 
 /* Flags for _STA return value (current_status above) */
 
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index de4e86f..3d1e5dd 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -57,6 +57,19 @@ static inline acpi_handle acpi_device_handle(struct acpi_device *adev)
 #define ACPI_COMPANION_SET(dev, adev)	ACPI_COMPANION(dev) = (adev)
 #define ACPI_HANDLE(dev)		acpi_device_handle(ACPI_COMPANION(dev))
 
+/**
+ * ACPI_DEVICE_CLASS - macro used to describe an ACPI device with
+ * the PCI-defined class-code information
+ *
+ * @_cls : the class, subclass, prog-if triple for this device
+ * @_msk : the class mask for this device
+ *
+ * This macro is used to create a struct acpi_device_id that matches a
+ * specific PCI class. The .id and .driver_data fields will be left
+ * initialized with the default value.
+ */
+#define ACPI_DEVICE_CLASS(_cls, _msk)	.cls = (_cls), .cls_msk = (_msk),
+
 static inline void acpi_preset_companion(struct device *dev,
 					 struct acpi_device *parent, u64 addr)
 {
@@ -459,6 +472,7 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *);
 #define ACPI_COMPANION(dev)		(NULL)
 #define ACPI_COMPANION_SET(dev, adev)	do { } while (0)
 #define ACPI_HANDLE(dev)		(NULL)
+#define ACPI_DEVICE_CLASS(_cls, _msk)	.cls = (0), .cls_msk = (0),
 
 struct fwnode_handle;
 
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index e530533..2ed31b8 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -189,6 +189,8 @@ struct css_device_id {
 struct acpi_device_id {
 	__u8 id[ACPI_ID_LEN];
 	kernel_ulong_t driver_data;
+	__u32 cls;
+	__u32 cls_msk;
 };
 
 #define PNP_ID_LEN	8
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index f282516..8987160 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -63,6 +63,8 @@ int main(void)
 
 	DEVID(acpi_device_id);
 	DEVID_FIELD(acpi_device_id, id);
+	DEVID_FIELD(acpi_device_id, cls);
+	DEVID_FIELD(acpi_device_id, cls_msk);
 
 	DEVID(pnp_device_id);
 	DEVID_FIELD(pnp_device_id, id);
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index e614ef6..704d597 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -511,12 +511,40 @@ static int do_serio_entry(const char *filename,
 }
 ADD_TO_DEVTABLE("serio", serio_device_id, do_serio_entry);
 
-/* looks like: "acpi:ACPI0003 or acpi:PNP0C0B" or "acpi:LNXVIDEO" */
+/* looks like: "acpi:ACPI0003" or "acpi:PNP0C0B" or "acpi:LNXVIDEO" or
+ *             "acpi:bbsspp" (bb=base-class, ss=sub-class, pp=prog-if)
+ *
+ * NOTE: Each driver should use one of the following : _HID, _CIDs
+ *       or _CLS. Also, bb, ss, and pp can be substituted with ??
+ *       as don't care byte.
+ */
 static int do_acpi_entry(const char *filename,
 			void *symval, char *alias)
 {
 	DEF_FIELD_ADDR(symval, acpi_device_id, id);
-	sprintf(alias, "acpi*:%s:*", *id);
+	DEF_FIELD_ADDR(symval, acpi_device_id, cls);
+	DEF_FIELD_ADDR(symval, acpi_device_id, cls_msk);
+
+	if (id && strlen((const char *)*id))
+		sprintf(alias, "acpi*:%s:*", *id);
+	else if (cls) {
+		int i, byte_shift, cnt = 0;
+		unsigned int msk;
+
+		sprintf(&alias[cnt], "acpi*:");
+		cnt = 6;
+		for (i = 1; i <= 3; i++) {
+			byte_shift = 8 * (3-i);
+			msk = (*cls_msk >> byte_shift) & 0xFF;
+			if (msk)
+				sprintf(&alias[cnt], "%02x",
+					(*cls >> byte_shift) & 0xFF);
+			else
+				sprintf(&alias[cnt], "??");
+			cnt += 2;
+		}
+		sprintf(&alias[cnt], ":*");
+	}
 	return 1;
 }
 ADD_TO_DEVTABLE("acpi", acpi_device_id, do_acpi_entry);
-- 
2.1.0

[V8 PATCH 3/3] ata: ahci_platform: Add ACPI _CLS matching

[Suravee Suthikulpanit]

This patch adds ACPI supports for AHCI platform driver, which uses _CLS
method to match the device.

The following is an example of ASL structure in DSDT for a SATA controller,
which contains _CLS package to be matched by the ahci_platform driver:

  Device (AHC0) // AHCI Controller
  {
    Name(_HID, "AMDI0600")
    Name (_CCA, 1)
    Name (_CLS, Package (3)
    {
      0x01, // Base Class: Mass Storage
      0x06, // Sub-Class: serial ATA
      0x01, // Interface: AHCI
    })
    Name (_CRS, ResourceTemplate ()
    {
      Memory32Fixed (ReadWrite, 0xE0300000, 0x00010000)
      Interrupt (ResourceConsumer, Level, ActiveHigh, Exclusive,,,) { 387 }
    })
  }

Also, since ATA driver should not require PCI support for ATA_ACPI,
this patch removes dependency in the driver/ata/Kconfig.

Acked-by: Tejun Heo <tj@kernel.org>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/ata/Kconfig         | 2 +-
 drivers/ata/ahci_platform.c | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 5f60155..50305e3 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -48,7 +48,7 @@ config ATA_VERBOSE_ERROR
 
 config ATA_ACPI
 	bool "ATA ACPI Support"
-	depends on ACPI && PCI
+	depends on ACPI
 	default y
 	help
 	  This option adds support for ATA-related ACPI objects.
diff --git a/drivers/ata/ahci_platform.c b/drivers/ata/ahci_platform.c
index 78d6ae0..04975b8 100644
--- a/drivers/ata/ahci_platform.c
+++ b/drivers/ata/ahci_platform.c
@@ -20,6 +20,8 @@
 #include <linux/platform_device.h>
 #include <linux/libata.h>
 #include <linux/ahci_platform.h>
+#include <linux/acpi.h>
+#include <linux/pci_ids.h>
 #include "ahci.h"
 
 #define DRV_NAME "ahci"
@@ -78,12 +80,19 @@ static const struct of_device_id ahci_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, ahci_of_match);
 
+static const struct acpi_device_id ahci_acpi_match[] = {
+	{ ACPI_DEVICE_CLASS(PCI_CLASS_STORAGE_SATA_AHCI, 0xffffff) },
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, ahci_acpi_match);
+
 static struct platform_driver ahci_driver = {
 	.probe = ahci_probe,
 	.remove = ata_platform_remove_one,
 	.driver = {
 		.name = DRV_NAME,
 		.of_match_table = ahci_of_match,
+		.acpi_match_table = ahci_acpi_match,
 		.pm = &ahci_pm_ops,
 	},
 };
-- 
2.1.0

Re: [V8 PATCH 0/3] Introduce ACPI support for ahci_platform driver

[Suravee Suthikulanit]

Ping.

Are there any other concerns for this patch series?

Thanks,

Suravee

On 3/30/2015 4:56 PM, Suravee Suthikulpanit wrote:
> This patch series introduce ACPI support for AHCI platform driver.
> Existing ACPI support for AHCI assumes the device controller is a PCI device.
> Since there is no ACPI _CID for generic AHCI controller, the driver
> could not use it for matching devices. Therefore, this patch introduces
> a mechanism for drivers to match devices using ACPI _CLS method.
> _CLS contains PCI-defined class-code.
>
> This patch series also modifies ACPI modalias to add class-code to the
> exisiting format, which currently only uses _HID and _CIDs. This is required
> to support loadable modules w/ _CLS.
>
> This is rebased from and tested with:
>
>      http://git.linaro.org/leg/acpi/acpi.git acpi-5.1-v11
>
> This topic was discussed earlier here (as part of introducing support for
> AMD Seattle SATA controller):
>
>      http://marc.info/?l=linux-arm-kernel&m=141083492521584&w=2
>
> Changes from V7 (https://lkml.org/lkml/2015/3/26/592)
> 	* [1/3] Return AE_AML_OPERAND_TYPE when _CLS package containing
> 	  invalid type (per Robert Moore suggestion).
> 	* [2/3] Fixed build error due missing ACPI_DEVICE_CLASS definition
> 	  when disabling ACPI.
>
> Changes from V6 (https://lkml.org/lkml/2015/3/25/797)
> 	* Adding Acked-by Mika, and Reviewed-by Hanjun
> 	* Minor clen up to use lower case 0xffffff for cls_msk
> 	  (per Mika suggestions).
> 	* Modify the ACPI_DEVICE_CLASS macro to use designated initializer
> 	  (per Mika suggestions).
>
> Changes from V5 (https://lkml.org/lkml/2015/3/6/24)
> 	* Rebased and tested with acpi-5.1-v11
> 	* Splitting up the ACPICA changes into a separate patch [1/3].
> 	  (per Mika suggestion)
> 	* Adding class-code mask support (per Mika suggestion)
> 	* Use macro to define struct acpi_device_id entry (per Mika suggestion)
> 	* Note: Mika also recommend reordering the member of struct acpi_device_id
> 	  and define a macro to be used for declaring each table entry. This is a
> 	  large amount of changes, and will be done separtely from this patch series.
>
> Changes from V4 (https://lkml.org/lkml/2015/3/2/56)
> 	* [1/2] Bug fixed: Reorder the declaration of
> 	  struct acpi_pnp_device_id cls in the struct acpi_device_info
> 	  (include/acpi/actypes.h) since compatible_id_list must be last one.
> 	* [2/2] Added Acked-by: Tejun Heo <tj@kernel.org>
>
> Changes from V3 (https://lkml.org/lkml/2015/2/8/106)
> 	* Instead of introducing new structure acpi_device_cls, add cls into
> 	  the acpi_device_id, and modify the __acpi_match_device
> 	  to also match for cls. (per Mika suggestion.)
> 	* Add loadable module support, which requires changes in ACPI
> 	  modalias. (per Mika suggestion.)
> 	* Rebased and tested with acpi-5.1-v9
>
> Changes from V2 (https://lkml.org/lkml/2015/1/5/662)
> 	* Update with review comment from Rafael in patch 1/2
> 	* Rebased and tested with acpi-5.1-v8
>
> Changes from V1 (https://lkml.org/lkml/2014/12/19/345)
> 	* Rebased to 3.19.0-rc2
> 	* Change from acpi_cls in device_driver to acpi_match_cls (Hanjun comment)
> 	* Change the matching logic in acpi_driver_match_device() due to the new
> 	  special PRP0001 _HID.
> 	* Simplify the return type of acpi_match_device_cls() to boolean.
>
> Changes from RFC (https://lkml.org/lkml/2014/12/17/446)
> 	* Remove #ifdef and make non-ACPI version of the acpi_match_device_cls
> 	  as inline. (per Arnd)
> 	* Simplify logic to retrieve and evaluate _CLS handle. (per Hanjun)
>
> Suravee Suthikulpanit (3):
>    ACPICA: Add ACPI _CLS processing
>    ACPI / scan: Add support for ACPI _CLS device matching
>    ata: ahci_platform: Add ACPI _CLS matching
>
>   drivers/acpi/acpica/acutils.h     |  3 ++
>   drivers/acpi/acpica/nsxfname.c    | 21 +++++++++--
>   drivers/acpi/acpica/utids.c       | 73 +++++++++++++++++++++++++++++++++++++++
>   drivers/acpi/scan.c               | 36 ++++++++++++++++---
>   drivers/ata/Kconfig               |  2 +-
>   drivers/ata/ahci_platform.c       |  9 +++++
>   include/acpi/acnames.h            |  1 +
>   include/acpi/actypes.h            |  4 ++-
>   include/linux/acpi.h              | 14 ++++++++
>   include/linux/mod_devicetable.h   |  2 ++
>   scripts/mod/devicetable-offsets.c |  2 ++
>   scripts/mod/file2alias.c          | 32 +++++++++++++++--
>   12 files changed, 189 insertions(+), 10 deletions(-)
>

Re: [V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing

[Rafael J. Wysocki]

On Monday, March 30, 2015 04:56:17 PM Suravee Suthikulpanit wrote:
> ACPI Device configuration often contain _CLS object to suppy PCI-defined
> class code for the device. This patch introduces logic to process the _CLS
> object.
> 
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

As you know, this patch is for ACPICA, so I'd prefer it to go into
upstream ACPICA first and then to Linux from there.

I can apply the remaining two patches on top of it when that happens.

Would that work or is it a really big deal?

> ---
>  drivers/acpi/acpica/acutils.h  |  3 ++
>  drivers/acpi/acpica/nsxfname.c | 21 ++++++++++--
>  drivers/acpi/acpica/utids.c    | 73 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 95 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/acutils.h b/drivers/acpi/acpica/acutils.h
> index c2f03e8..2aef850 100644
> --- a/drivers/acpi/acpica/acutils.h
> +++ b/drivers/acpi/acpica/acutils.h
> @@ -430,6 +430,9 @@ acpi_status
>  acpi_ut_execute_CID(struct acpi_namespace_node *device_node,
>  		    struct acpi_pnp_device_id_list ** return_cid_list);
>  
> +acpi_status
> +acpi_ut_execute_CLS(struct acpi_namespace_node *device_node,
> +		    struct acpi_pnp_device_id **return_id);
>  /*
>   * utlock - reader/writer locks
>   */
> diff --git a/drivers/acpi/acpica/nsxfname.c b/drivers/acpi/acpica/nsxfname.c
> index d66c326..590ef06 100644
> --- a/drivers/acpi/acpica/nsxfname.c
> +++ b/drivers/acpi/acpica/nsxfname.c
> @@ -276,11 +276,12 @@ acpi_get_object_info(acpi_handle handle,
>  	struct acpi_pnp_device_id *hid = NULL;
>  	struct acpi_pnp_device_id *uid = NULL;
>  	struct acpi_pnp_device_id *sub = NULL;
> +	struct acpi_pnp_device_id *cls = NULL;
>  	char *next_id_string;
>  	acpi_object_type type;
>  	acpi_name name;
>  	u8 param_count = 0;
> -	u8 valid = 0;
> +	u16 valid = 0;
>  	u32 info_size;
>  	u32 i;
>  	acpi_status status;
> @@ -320,7 +321,7 @@ acpi_get_object_info(acpi_handle handle,
>  	if ((type == ACPI_TYPE_DEVICE) || (type == ACPI_TYPE_PROCESSOR)) {
>  		/*
>  		 * Get extra info for ACPI Device/Processor objects only:
> -		 * Run the Device _HID, _UID, _SUB, and _CID methods.
> +		 * Run the Device _HID, _UID, _SUB, _CID and _CLS methods.
>  		 *
>  		 * Note: none of these methods are required, so they may or may
>  		 * not be present for this device. The Info->Valid bitfield is used
> @@ -351,6 +352,14 @@ acpi_get_object_info(acpi_handle handle,
>  			valid |= ACPI_VALID_SUB;
>  		}
>  
> +		/* Execute the Device._CLS method */
> +
> +		status = acpi_ut_execute_CLS(node, &cls);
> +		if (ACPI_SUCCESS(status)) {
> +			info_size += cls->length;
> +			valid |= ACPI_VALID_CLS;
> +		}
> +
>  		/* Execute the Device._CID method */
>  
>  		status = acpi_ut_execute_CID(node, &cid_list);
> @@ -468,6 +477,11 @@ acpi_get_object_info(acpi_handle handle,
>  							sub, next_id_string);
>  	}
>  
> +	if (cls) {
> +		next_id_string = acpi_ns_copy_device_id(&info->cls,
> +							cls, next_id_string);
> +	}
> +
>  	if (cid_list) {
>  		info->compatible_id_list.count = cid_list->count;
>  		info->compatible_id_list.list_size = cid_list->list_size;
> @@ -507,6 +521,9 @@ cleanup:
>  	if (sub) {
>  		ACPI_FREE(sub);
>  	}
> +	if (cls) {
> +		ACPI_FREE(cls);
> +	}
>  	if (cid_list) {
>  		ACPI_FREE(cid_list);
>  	}
> diff --git a/drivers/acpi/acpica/utids.c b/drivers/acpi/acpica/utids.c
> index 27431cf..9745065 100644
> --- a/drivers/acpi/acpica/utids.c
> +++ b/drivers/acpi/acpica/utids.c
> @@ -416,3 +416,76 @@ cleanup:
>  	acpi_ut_remove_reference(obj_desc);
>  	return_ACPI_STATUS(status);
>  }
> +
> +/*******************************************************************************
> + *
> + * FUNCTION:    acpi_ut_execute_CLS
> + *
> + * PARAMETERS:  device_node         - Node for the device
> + *              return_id           - Where the string UID is returned
> + *
> + * RETURN:      Status
> + *
> + * DESCRIPTION: Executes the _CLS control method that returns PCI-defined
> + *              class code of the device. The ACPI spec define _CLS as a
> + *              package with three integers. The returned string has format:
> + *
> + *                      "bbsspp"
> + *              where:
> + *                  bb = Base-class code
> + *                  ss = Sub-class code
> + *                  pp = Programming Interface code
> + *
> + ******************************************************************************/
> +
> +acpi_status
> +acpi_ut_execute_CLS(struct acpi_namespace_node *device_node,
> +		    struct acpi_pnp_device_id **return_id)
> +{
> +	struct acpi_pnp_device_id *cls;
> +	union acpi_operand_object *obj_desc;
> +	union acpi_operand_object **cls_objects;
> +	acpi_status status;
> +
> +	ACPI_FUNCTION_TRACE(ut_execute_CLS);
> +	status = acpi_ut_evaluate_object(device_node, METHOD_NAME__CLS,
> +					 ACPI_BTYPE_PACKAGE, &obj_desc);
> +	if (ACPI_FAILURE(status))
> +		return_ACPI_STATUS(status);
> +
> +	cls_objects = obj_desc->package.elements;
> +
> +	if (obj_desc->common.type == ACPI_TYPE_PACKAGE &&
> +	    obj_desc->package.count == 3 &&
> +	    cls_objects[0]->common.type == ACPI_TYPE_INTEGER &&
> +	    cls_objects[1]->common.type == ACPI_TYPE_INTEGER &&
> +	    cls_objects[2]->common.type == ACPI_TYPE_INTEGER) {
> +
> +		/* Allocate a buffer for the CLS */
> +		cls = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_pnp_device_id) +
> +					 (acpi_size) 7);
> +		if (!cls) {
> +			status = AE_NO_MEMORY;
> +			goto cleanup;
> +		}
> +
> +		cls->string =
> +		    ACPI_ADD_PTR(char, cls, sizeof(struct acpi_pnp_device_id));
> +
> +		sprintf(cls->string, "%02x%02x%02x",
> +			(u8)ACPI_TO_INTEGER(cls_objects[0]->integer.value),
> +			(u8)ACPI_TO_INTEGER(cls_objects[1]->integer.value),
> +			(u8)ACPI_TO_INTEGER(cls_objects[2]->integer.value));
> +		cls->length = 7;
> +		*return_id = cls;
> +	} else {
> +		status = AE_AML_OPERAND_TYPE;
> +	}
> +
> +cleanup:
> +
> +	/* On exit, we must delete the return object */
> +
> +	acpi_ut_remove_reference(obj_desc);
> +	return_ACPI_STATUS(status);
> +}
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

RE: [V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing

[Zheng, Lv]

Before back porting this to ACPICA, let me ask one simple question.
According to the spec, the _CLS is optional and PCI specific.
So why should we implement it in ACPICA core not OSPM specific modules?
If this need to be implemented in ACPICA, then what about the following device identification objects?
_DDN, _HRV, _MLS, _PLD, _STR, _SUN

Thanks and best regards
-Lv

> From: Suravee Suthikulpanit [mailto:Suravee.Suthikulpanit@amd.com]
> Sent: Tuesday, March 31, 2015 5:56 AM
> 
> ACPI Device configuration often contain _CLS object to suppy PCI-defined
> class code for the device. This patch introduces logic to process the _CLS
> object.
> 
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> ---
>  drivers/acpi/acpica/acutils.h  |  3 ++
>  drivers/acpi/acpica/nsxfname.c | 21 ++++++++++--
>  drivers/acpi/acpica/utids.c    | 73 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 95 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/acutils.h b/drivers/acpi/acpica/acutils.h
> index c2f03e8..2aef850 100644
> --- a/drivers/acpi/acpica/acutils.h
> +++ b/drivers/acpi/acpica/acutils.h
> @@ -430,6 +430,9 @@ acpi_status
>  acpi_ut_execute_CID(struct acpi_namespace_node *device_node,
>  		    struct acpi_pnp_device_id_list ** return_cid_list);
> 
> +acpi_status
> +acpi_ut_execute_CLS(struct acpi_namespace_node *device_node,
> +		    struct acpi_pnp_device_id **return_id);
>  /*
>   * utlock - reader/writer locks
>   */
> diff --git a/drivers/acpi/acpica/nsxfname.c b/drivers/acpi/acpica/nsxfname.c
> index d66c326..590ef06 100644
> --- a/drivers/acpi/acpica/nsxfname.c
> +++ b/drivers/acpi/acpica/nsxfname.c
> @@ -276,11 +276,12 @@ acpi_get_object_info(acpi_handle handle,
>  	struct acpi_pnp_device_id *hid = NULL;
>  	struct acpi_pnp_device_id *uid = NULL;
>  	struct acpi_pnp_device_id *sub = NULL;
> +	struct acpi_pnp_device_id *cls = NULL;
>  	char *next_id_string;
>  	acpi_object_type type;
>  	acpi_name name;
>  	u8 param_count = 0;
> -	u8 valid = 0;
> +	u16 valid = 0;
>  	u32 info_size;
>  	u32 i;
>  	acpi_status status;
> @@ -320,7 +321,7 @@ acpi_get_object_info(acpi_handle handle,
>  	if ((type == ACPI_TYPE_DEVICE) || (type == ACPI_TYPE_PROCESSOR)) {
>  		/*
>  		 * Get extra info for ACPI Device/Processor objects only:
> -		 * Run the Device _HID, _UID, _SUB, and _CID methods.
> +		 * Run the Device _HID, _UID, _SUB, _CID and _CLS methods.
>  		 *
>  		 * Note: none of these methods are required, so they may or may
>  		 * not be present for this device. The Info->Valid bitfield is used
> @@ -351,6 +352,14 @@ acpi_get_object_info(acpi_handle handle,
>  			valid |= ACPI_VALID_SUB;
>  		}
> 
> +		/* Execute the Device._CLS method */
> +
> +		status = acpi_ut_execute_CLS(node, &cls);
> +		if (ACPI_SUCCESS(status)) {
> +			info_size += cls->length;
> +			valid |= ACPI_VALID_CLS;
> +		}
> +
>  		/* Execute the Device._CID method */
> 
>  		status = acpi_ut_execute_CID(node, &cid_list);
> @@ -468,6 +477,11 @@ acpi_get_object_info(acpi_handle handle,
>  							sub, next_id_string);
>  	}
> 
> +	if (cls) {
> +		next_id_string = acpi_ns_copy_device_id(&info->cls,
> +							cls, next_id_string);
> +	}
> +
>  	if (cid_list) {
>  		info->compatible_id_list.count = cid_list->count;
>  		info->compatible_id_list.list_size = cid_list->list_size;
> @@ -507,6 +521,9 @@ cleanup:
>  	if (sub) {
>  		ACPI_FREE(sub);
>  	}
> +	if (cls) {
> +		ACPI_FREE(cls);
> +	}
>  	if (cid_list) {
>  		ACPI_FREE(cid_list);
>  	}
> diff --git a/drivers/acpi/acpica/utids.c b/drivers/acpi/acpica/utids.c
> index 27431cf..9745065 100644
> --- a/drivers/acpi/acpica/utids.c
> +++ b/drivers/acpi/acpica/utids.c
> @@ -416,3 +416,76 @@ cleanup:
>  	acpi_ut_remove_reference(obj_desc);
>  	return_ACPI_STATUS(status);
>  }
> +
> +/*******************************************************************************
> + *
> + * FUNCTION:    acpi_ut_execute_CLS
> + *
> + * PARAMETERS:  device_node         - Node for the device
> + *              return_id           - Where the string UID is returned
> + *
> + * RETURN:      Status
> + *
> + * DESCRIPTION: Executes the _CLS control method that returns PCI-defined
> + *              class code of the device. The ACPI spec define _CLS as a
> + *              package with three integers. The returned string has format:
> + *
> + *                      "bbsspp"
> + *              where:
> + *                  bb = Base-class code
> + *                  ss = Sub-class code
> + *                  pp = Programming Interface code
> + *
> + ******************************************************************************/
> +
> +acpi_status
> +acpi_ut_execute_CLS(struct acpi_namespace_node *device_node,
> +		    struct acpi_pnp_device_id **return_id)
> +{
> +	struct acpi_pnp_device_id *cls;
> +	union acpi_operand_object *obj_desc;
> +	union acpi_operand_object **cls_objects;
> +	acpi_status status;
> +
> +	ACPI_FUNCTION_TRACE(ut_execute_CLS);
> +	status = acpi_ut_evaluate_object(device_node, METHOD_NAME__CLS,
> +					 ACPI_BTYPE_PACKAGE, &obj_desc);
> +	if (ACPI_FAILURE(status))
> +		return_ACPI_STATUS(status);
> +
> +	cls_objects = obj_desc->package.elements;
> +
> +	if (obj_desc->common.type == ACPI_TYPE_PACKAGE &&
> +	    obj_desc->package.count == 3 &&
> +	    cls_objects[0]->common.type == ACPI_TYPE_INTEGER &&
> +	    cls_objects[1]->common.type == ACPI_TYPE_INTEGER &&
> +	    cls_objects[2]->common.type == ACPI_TYPE_INTEGER) {
> +
> +		/* Allocate a buffer for the CLS */
> +		cls = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_pnp_device_id) +
> +					 (acpi_size) 7);
> +		if (!cls) {
> +			status = AE_NO_MEMORY;
> +			goto cleanup;
> +		}
> +
> +		cls->string =
> +		    ACPI_ADD_PTR(char, cls, sizeof(struct acpi_pnp_device_id));
> +
> +		sprintf(cls->string, "%02x%02x%02x",
> +			(u8)ACPI_TO_INTEGER(cls_objects[0]->integer.value),
> +			(u8)ACPI_TO_INTEGER(cls_objects[1]->integer.value),
> +			(u8)ACPI_TO_INTEGER(cls_objects[2]->integer.value));
> +		cls->length = 7;
> +		*return_id = cls;
> +	} else {
> +		status = AE_AML_OPERAND_TYPE;
> +	}
> +
> +cleanup:
> +
> +	/* On exit, we must delete the return object */
> +
> +	acpi_ut_remove_reference(obj_desc);
> +	return_ACPI_STATUS(status);
> +}
> --
> 2.1.0

RE: [V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing

[Moore, Robert]

Yes, this is a good question that we should think about.
Bob


> -----Original Message-----
> From: Zheng, Lv
> Sent: Thursday, April 16, 2015 6:46 PM
> To: Suravee Suthikulpanit; rjw@rjwysocki.net;
> mika.westerberg@linux.intel.com; Moore, Robert; hanjun.guo@linaro.org
> Cc: lenb@kernel.org; hdegoede@redhat.com; tj@kernel.org;
> mjg59@srcf.ucam.org; gregkh@linuxfoundation.org; al.stone@linaro.org;
> graeme.gregory@linaro.org; leo.duran@amd.com; linux-ide@vger.kernel.org;
> linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linaro-
> acpi@lists.linaro.org
> Subject: RE: [V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing
> 
> Before back porting this to ACPICA, let me ask one simple question.
> According to the spec, the _CLS is optional and PCI specific.
> So why should we implement it in ACPICA core not OSPM specific modules?
> If this need to be implemented in ACPICA, then what about the following
> device identification objects?
> _DDN, _HRV, _MLS, _PLD, _STR, _SUN
> 
> Thanks and best regards
> -Lv
> 
> > From: Suravee Suthikulpanit [mailto:Suravee.Suthikulpanit@amd.com]
> > Sent: Tuesday, March 31, 2015 5:56 AM
> >
> > ACPI Device configuration often contain _CLS object to suppy
> > PCI-defined class code for the device. This patch introduces logic to
> > process the _CLS object.
> >
> > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Reviewed-by: Hanjun Guo <hanjun.guo@linaro.org>
> > Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> > ---
> >  drivers/acpi/acpica/acutils.h  |  3 ++
> > drivers/acpi/acpica/nsxfname.c | 21 ++++++++++--
> >  drivers/acpi/acpica/utids.c    | 73
> ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 95 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/acutils.h
> > b/drivers/acpi/acpica/acutils.h index c2f03e8..2aef850 100644
> > --- a/drivers/acpi/acpica/acutils.h
> > +++ b/drivers/acpi/acpica/acutils.h
> > @@ -430,6 +430,9 @@ acpi_status
> >  acpi_ut_execute_CID(struct acpi_namespace_node *device_node,
> >  		    struct acpi_pnp_device_id_list ** return_cid_list);
> >
> > +acpi_status
> > +acpi_ut_execute_CLS(struct acpi_namespace_node *device_node,
> > +		    struct acpi_pnp_device_id **return_id);
> >  /*
> >   * utlock - reader/writer locks
> >   */
> > diff --git a/drivers/acpi/acpica/nsxfname.c
> > b/drivers/acpi/acpica/nsxfname.c index d66c326..590ef06 100644
> > --- a/drivers/acpi/acpica/nsxfname.c
> > +++ b/drivers/acpi/acpica/nsxfname.c
> > @@ -276,11 +276,12 @@ acpi_get_object_info(acpi_handle handle,
> >  	struct acpi_pnp_device_id *hid = NULL;
> >  	struct acpi_pnp_device_id *uid = NULL;
> >  	struct acpi_pnp_device_id *sub = NULL;
> > +	struct acpi_pnp_device_id *cls = NULL;
> >  	char *next_id_string;
> >  	acpi_object_type type;
> >  	acpi_name name;
> >  	u8 param_count = 0;
> > -	u8 valid = 0;
> > +	u16 valid = 0;
> >  	u32 info_size;
> >  	u32 i;
> >  	acpi_status status;
> > @@ -320,7 +321,7 @@ acpi_get_object_info(acpi_handle handle,
> >  	if ((type == ACPI_TYPE_DEVICE) || (type == ACPI_TYPE_PROCESSOR)) {
> >  		/*
> >  		 * Get extra info for ACPI Device/Processor objects only:
> > -		 * Run the Device _HID, _UID, _SUB, and _CID methods.
> > +		 * Run the Device _HID, _UID, _SUB, _CID and _CLS methods.
> >  		 *
> >  		 * Note: none of these methods are required, so they may or
> may
> >  		 * not be present for this device. The Info->Valid bitfield is
> used
> > @@ -351,6 +352,14 @@ acpi_get_object_info(acpi_handle handle,
> >  			valid |= ACPI_VALID_SUB;
> >  		}
> >
> > +		/* Execute the Device._CLS method */
> > +
> > +		status = acpi_ut_execute_CLS(node, &cls);
> > +		if (ACPI_SUCCESS(status)) {
> > +			info_size += cls->length;
> > +			valid |= ACPI_VALID_CLS;
> > +		}
> > +
> >  		/* Execute the Device._CID method */
> >
> >  		status = acpi_ut_execute_CID(node, &cid_list); @@ -468,6
> +477,11 @@
> > acpi_get_object_info(acpi_handle handle,
> >  							sub, next_id_string);
> >  	}
> >
> > +	if (cls) {
> > +		next_id_string = acpi_ns_copy_device_id(&info->cls,
> > +							cls, next_id_string);
> > +	}
> > +
> >  	if (cid_list) {
> >  		info->compatible_id_list.count = cid_list->count;
> >  		info->compatible_id_list.list_size = cid_list->list_size; @@ -
> 507,6
> > +521,9 @@ cleanup:
> >  	if (sub) {
> >  		ACPI_FREE(sub);
> >  	}
> > +	if (cls) {
> > +		ACPI_FREE(cls);
> > +	}
> >  	if (cid_list) {
> >  		ACPI_FREE(cid_list);
> >  	}
> > diff --git a/drivers/acpi/acpica/utids.c b/drivers/acpi/acpica/utids.c
> > index 27431cf..9745065 100644
> > --- a/drivers/acpi/acpica/utids.c
> > +++ b/drivers/acpi/acpica/utids.c
> > @@ -416,3 +416,76 @@ cleanup:
> >  	acpi_ut_remove_reference(obj_desc);
> >  	return_ACPI_STATUS(status);
> >  }
> > +
> > +/********************************************************************
> > +***********
> > + *
> > + * FUNCTION:    acpi_ut_execute_CLS
> > + *
> > + * PARAMETERS:  device_node         - Node for the device
> > + *              return_id           - Where the string UID is returned
> > + *
> > + * RETURN:      Status
> > + *
> > + * DESCRIPTION: Executes the _CLS control method that returns PCI-
> defined
> > + *              class code of the device. The ACPI spec define _CLS as
> a
> > + *              package with three integers. The returned string has
> format:
> > + *
> > + *                      "bbsspp"
> > + *              where:
> > + *                  bb = Base-class code
> > + *                  ss = Sub-class code
> > + *                  pp = Programming Interface code
> > + *
> > +
> > +*********************************************************************
> > +*********/
> > +
> > +acpi_status
> > +acpi_ut_execute_CLS(struct acpi_namespace_node *device_node,
> > +		    struct acpi_pnp_device_id **return_id) {
> > +	struct acpi_pnp_device_id *cls;
> > +	union acpi_operand_object *obj_desc;
> > +	union acpi_operand_object **cls_objects;
> > +	acpi_status status;
> > +
> > +	ACPI_FUNCTION_TRACE(ut_execute_CLS);
> > +	status = acpi_ut_evaluate_object(device_node, METHOD_NAME__CLS,
> > +					 ACPI_BTYPE_PACKAGE, &obj_desc);
> > +	if (ACPI_FAILURE(status))
> > +		return_ACPI_STATUS(status);
> > +
> > +	cls_objects = obj_desc->package.elements;
> > +
> > +	if (obj_desc->common.type == ACPI_TYPE_PACKAGE &&
> > +	    obj_desc->package.count == 3 &&
> > +	    cls_objects[0]->common.type == ACPI_TYPE_INTEGER &&
> > +	    cls_objects[1]->common.type == ACPI_TYPE_INTEGER &&
> > +	    cls_objects[2]->common.type == ACPI_TYPE_INTEGER) {
> > +
> > +		/* Allocate a buffer for the CLS */
> > +		cls = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_pnp_device_id) +
> > +					 (acpi_size) 7);
> > +		if (!cls) {
> > +			status = AE_NO_MEMORY;
> > +			goto cleanup;
> > +		}
> > +
> > +		cls->string =
> > +		    ACPI_ADD_PTR(char, cls, sizeof(struct
> acpi_pnp_device_id));
> > +
> > +		sprintf(cls->string, "%02x%02x%02x",
> > +			(u8)ACPI_TO_INTEGER(cls_objects[0]->integer.value),
> > +			(u8)ACPI_TO_INTEGER(cls_objects[1]->integer.value),
> > +			(u8)ACPI_TO_INTEGER(cls_objects[2]->integer.value));
> > +		cls->length = 7;
> > +		*return_id = cls;
> > +	} else {
> > +		status = AE_AML_OPERAND_TYPE;
> > +	}
> > +
> > +cleanup:
> > +
> > +	/* On exit, we must delete the return object */
> > +
> > +	acpi_ut_remove_reference(obj_desc);
> > +	return_ACPI_STATUS(status);
> > +}
> > --
> > 2.1.0

Re: [V8 PATCH 0/3] Introduce ACPI support for ahci_platform driver

[Suthikulpanit, Suravee]

Mika/Rafael, 

If there are no other concerns about this patch series, do you think this
ready to be pushed to linux-next tree along with the rest of the ARM64
ACPI patch series from Linaro?

Thanks,

Suravee

On 4/6/15, 10:45, "Suravee Suthikulanit" <suravee.suthikulpanit@amd.com>
wrote:

>Ping.
>
>Are there any other concerns for this patch series?
>
>Thanks,
>
>Suravee
>
>On 3/30/2015 4:56 PM, Suravee Suthikulpanit wrote:
>> This patch series introduce ACPI support for AHCI platform driver.
>> Existing ACPI support for AHCI assumes the device controller is a PCI
>>device.
>> Since there is no ACPI _CID for generic AHCI controller, the driver
>> could not use it for matching devices. Therefore, this patch introduces
>> a mechanism for drivers to match devices using ACPI _CLS method.
>> _CLS contains PCI-defined class-code.
>>
>> This patch series also modifies ACPI modalias to add class-code to the
>> exisiting format, which currently only uses _HID and _CIDs. This is
>>required
>> to support loadable modules w/ _CLS.
>>
>> This is rebased from and tested with:
>>
>>      http://git.linaro.org/leg/acpi/acpi.git acpi-5.1-v11
>>
>> This topic was discussed earlier here (as part of introducing support
>>for
>> AMD Seattle SATA controller):
>>
>>      http://marc.info/?l=linux-arm-kernel&m=141083492521584&w=2
>>
>> Changes from V7 (https://lkml.org/lkml/2015/3/26/592)
>> 	* [1/3] Return AE_AML_OPERAND_TYPE when _CLS package containing
>> 	  invalid type (per Robert Moore suggestion).
>> 	* [2/3] Fixed build error due missing ACPI_DEVICE_CLASS definition
>> 	  when disabling ACPI.
>>
>> Changes from V6 (https://lkml.org/lkml/2015/3/25/797)
>> 	* Adding Acked-by Mika, and Reviewed-by Hanjun
>> 	* Minor clen up to use lower case 0xffffff for cls_msk
>> 	  (per Mika suggestions).
>> 	* Modify the ACPI_DEVICE_CLASS macro to use designated initializer
>> 	  (per Mika suggestions).
>>
>> Changes from V5 (https://lkml.org/lkml/2015/3/6/24)
>> 	* Rebased and tested with acpi-5.1-v11
>> 	* Splitting up the ACPICA changes into a separate patch [1/3].
>> 	  (per Mika suggestion)
>> 	* Adding class-code mask support (per Mika suggestion)
>> 	* Use macro to define struct acpi_device_id entry (per Mika suggestion)
>> 	* Note: Mika also recommend reordering the member of struct
>>acpi_device_id
>> 	  and define a macro to be used for declaring each table entry. This
>>is a
>> 	  large amount of changes, and will be done separtely from this patch
>>series.
>>
>> Changes from V4 (https://lkml.org/lkml/2015/3/2/56)
>> 	* [1/2] Bug fixed: Reorder the declaration of
>> 	  struct acpi_pnp_device_id cls in the struct acpi_device_info
>> 	  (include/acpi/actypes.h) since compatible_id_list must be last one.
>> 	* [2/2] Added Acked-by: Tejun Heo <tj@kernel.org>
>>
>> Changes from V3 (https://lkml.org/lkml/2015/2/8/106)
>> 	* Instead of introducing new structure acpi_device_cls, add cls into
>> 	  the acpi_device_id, and modify the __acpi_match_device
>> 	  to also match for cls. (per Mika suggestion.)
>> 	* Add loadable module support, which requires changes in ACPI
>> 	  modalias. (per Mika suggestion.)
>> 	* Rebased and tested with acpi-5.1-v9
>>
>> Changes from V2 (https://lkml.org/lkml/2015/1/5/662)
>> 	* Update with review comment from Rafael in patch 1/2
>> 	* Rebased and tested with acpi-5.1-v8
>>
>> Changes from V1 (https://lkml.org/lkml/2014/12/19/345)
>> 	* Rebased to 3.19.0-rc2
>> 	* Change from acpi_cls in device_driver to acpi_match_cls (Hanjun
>>comment)
>> 	* Change the matching logic in acpi_driver_match_device() due to the
>>new
>> 	  special PRP0001 _HID.
>> 	* Simplify the return type of acpi_match_device_cls() to boolean.
>>
>> Changes from RFC (https://lkml.org/lkml/2014/12/17/446)
>> 	* Remove #ifdef and make non-ACPI version of the acpi_match_device_cls
>> 	  as inline. (per Arnd)
>> 	* Simplify logic to retrieve and evaluate _CLS handle. (per Hanjun)
>>
>> Suravee Suthikulpanit (3):
>>    ACPICA: Add ACPI _CLS processing
>>    ACPI / scan: Add support for ACPI _CLS device matching
>>    ata: ahci_platform: Add ACPI _CLS matching
>>
>>   drivers/acpi/acpica/acutils.h     |  3 ++
>>   drivers/acpi/acpica/nsxfname.c    | 21 +++++++++--
>>   drivers/acpi/acpica/utids.c       | 73
>>+++++++++++++++++++++++++++++++++++++++
>>   drivers/acpi/scan.c               | 36 ++++++++++++++++---
>>   drivers/ata/Kconfig               |  2 +-
>>   drivers/ata/ahci_platform.c       |  9 +++++
>>   include/acpi/acnames.h            |  1 +
>>   include/acpi/actypes.h            |  4 ++-
>>   include/linux/acpi.h              | 14 ++++++++
>>   include/linux/mod_devicetable.h   |  2 ++
>>   scripts/mod/devicetable-offsets.c |  2 ++
>>   scripts/mod/file2alias.c          | 32 +++++++++++++++--
>>   12 files changed, 189 insertions(+), 10 deletions(-)
>>
>

Re: [V8 PATCH 0/3] Introduce ACPI support for ahci_platform driver

[Rafael J. Wysocki]

On Thursday, April 23, 2015 04:32:25 PM Suthikulpanit, Suravee wrote:
> Mika/Rafael, 
> 
> If there are no other concerns about this patch series, do you think this
> ready to be pushed to linux-next tree along with the rest of the ARM64
> ACPI patch series from Linaro?

There are concerns regarding patch [1/3] from Bob and Lv (please look at the
last message from Bob in this thread for details).


> >On 3/30/2015 4:56 PM, Suravee Suthikulpanit wrote:
> >> This patch series introduce ACPI support for AHCI platform driver.
> >> Existing ACPI support for AHCI assumes the device controller is a PCI
> >>device.
> >> Since there is no ACPI _CID for generic AHCI controller, the driver
> >> could not use it for matching devices. Therefore, this patch introduces
> >> a mechanism for drivers to match devices using ACPI _CLS method.
> >> _CLS contains PCI-defined class-code.
> >>
> >> This patch series also modifies ACPI modalias to add class-code to the
> >> exisiting format, which currently only uses _HID and _CIDs. This is
> >>required
> >> to support loadable modules w/ _CLS.
> >>
> >> This is rebased from and tested with:
> >>
> >>      http://git.linaro.org/leg/acpi/acpi.git acpi-5.1-v11
> >>
> >> This topic was discussed earlier here (as part of introducing support
> >>for
> >> AMD Seattle SATA controller):
> >>
> >>      http://marc.info/?l=linux-arm-kernel&m=141083492521584&w=2
> >>
> >> Changes from V7 (https://lkml.org/lkml/2015/3/26/592)
> >> 	* [1/3] Return AE_AML_OPERAND_TYPE when _CLS package containing
> >> 	  invalid type (per Robert Moore suggestion).
> >> 	* [2/3] Fixed build error due missing ACPI_DEVICE_CLASS definition
> >> 	  when disabling ACPI.
> >>
> >> Changes from V6 (https://lkml.org/lkml/2015/3/25/797)
> >> 	* Adding Acked-by Mika, and Reviewed-by Hanjun
> >> 	* Minor clen up to use lower case 0xffffff for cls_msk
> >> 	  (per Mika suggestions).
> >> 	* Modify the ACPI_DEVICE_CLASS macro to use designated initializer
> >> 	  (per Mika suggestions).
> >>
> >> Changes from V5 (https://lkml.org/lkml/2015/3/6/24)
> >> 	* Rebased and tested with acpi-5.1-v11
> >> 	* Splitting up the ACPICA changes into a separate patch [1/3].
> >> 	  (per Mika suggestion)
> >> 	* Adding class-code mask support (per Mika suggestion)
> >> 	* Use macro to define struct acpi_device_id entry (per Mika suggestion)
> >> 	* Note: Mika also recommend reordering the member of struct
> >>acpi_device_id
> >> 	  and define a macro to be used for declaring each table entry. This
> >>is a
> >> 	  large amount of changes, and will be done separtely from this patch
> >>series.
> >>
> >> Changes from V4 (https://lkml.org/lkml/2015/3/2/56)
> >> 	* [1/2] Bug fixed: Reorder the declaration of
> >> 	  struct acpi_pnp_device_id cls in the struct acpi_device_info
> >> 	  (include/acpi/actypes.h) since compatible_id_list must be last one.
> >> 	* [2/2] Added Acked-by: Tejun Heo <tj@kernel.org>
> >>
> >> Changes from V3 (https://lkml.org/lkml/2015/2/8/106)
> >> 	* Instead of introducing new structure acpi_device_cls, add cls into
> >> 	  the acpi_device_id, and modify the __acpi_match_device
> >> 	  to also match for cls. (per Mika suggestion.)
> >> 	* Add loadable module support, which requires changes in ACPI
> >> 	  modalias. (per Mika suggestion.)
> >> 	* Rebased and tested with acpi-5.1-v9
> >>
> >> Changes from V2 (https://lkml.org/lkml/2015/1/5/662)
> >> 	* Update with review comment from Rafael in patch 1/2
> >> 	* Rebased and tested with acpi-5.1-v8
> >>
> >> Changes from V1 (https://lkml.org/lkml/2014/12/19/345)
> >> 	* Rebased to 3.19.0-rc2
> >> 	* Change from acpi_cls in device_driver to acpi_match_cls (Hanjun
> >>comment)
> >> 	* Change the matching logic in acpi_driver_match_device() due to the
> >>new
> >> 	  special PRP0001 _HID.
> >> 	* Simplify the return type of acpi_match_device_cls() to boolean.
> >>
> >> Changes from RFC (https://lkml.org/lkml/2014/12/17/446)
> >> 	* Remove #ifdef and make non-ACPI version of the acpi_match_device_cls
> >> 	  as inline. (per Arnd)
> >> 	* Simplify logic to retrieve and evaluate _CLS handle. (per Hanjun)
> >>
> >> Suravee Suthikulpanit (3):
> >>    ACPICA: Add ACPI _CLS processing
> >>    ACPI / scan: Add support for ACPI _CLS device matching
> >>    ata: ahci_platform: Add ACPI _CLS matching
> >>
> >>   drivers/acpi/acpica/acutils.h     |  3 ++
> >>   drivers/acpi/acpica/nsxfname.c    | 21 +++++++++--
> >>   drivers/acpi/acpica/utids.c       | 73
> >>+++++++++++++++++++++++++++++++++++++++
> >>   drivers/acpi/scan.c               | 36 ++++++++++++++++---
> >>   drivers/ata/Kconfig               |  2 +-
> >>   drivers/ata/ahci_platform.c       |  9 +++++
> >>   include/acpi/acnames.h            |  1 +
> >>   include/acpi/actypes.h            |  4 ++-
> >>   include/linux/acpi.h              | 14 ++++++++
> >>   include/linux/mod_devicetable.h   |  2 ++
> >>   scripts/mod/devicetable-offsets.c |  2 ++
> >>   scripts/mod/file2alias.c          | 32 +++++++++++++++--
> >>   12 files changed, 189 insertions(+), 10 deletions(-)
> >>
> >
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [V8 PATCH 0/3] Introduce ACPI support for ahci_platform driver

[Suravee Suthikulpanit]

On 4/23/15 12:02, Rafael J. Wysocki wrote:
> On Thursday, April 23, 2015 04:32:25 PM Suthikulpanit, Suravee wrote:
>> >Mika/Rafael,
>> >
>> >If there are no other concerns about this patch series, do you think this
>> >ready to be pushed to linux-next tree along with the rest of the ARM64
>> >ACPI patch series from Linaro?
> There are concerns regarding patch [1/3] from Bob and Lv (please look at the
> last message from Bob in this thread for details).
>

Sorry, I missed that email. I'll take a look and reply to that now.

Thanks,

Suravee

Re: [V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing

[Suravee Suthikulpanit]

On 4/16/15 20:45, Zheng, Lv wrote:
> Before back porting this to ACPICA, let me ask one simple question.
> According to the spec, the _CLS is optional and PCI specific.
> So why should we implement it in ACPICA core not OSPM specific modules?
> If this need to be implemented in ACPICA, then what about the following device identification objects?
> _DDN, _HRV, _MLS, _PLD, _STR, _SUN
>
> Thanks and best regards
> -Lv

Hi,

Sorry for late reply. As for the justification for introducing the _CLS 
support in the ACPICA, this is mainly because ACPI does not currently 
define _CID for certain device classes, which used to mostly be PCI 
devices. Instead, ACPI spec mentioned that _CLS can be used for loading 
generic drivers on hardware that is compatible with PCI-defined device 
classes, but that is not implemented on the PCI bus (and is therefore 
enumerated by ACPI.)

The code introduced for supporting _CLS is also similar in the way 
ACPICA is currently parsing the _CID or _SUB (which are also optional), 
and using it for the same purpose of identifying devices for loading 
drivers.

Also, since this method for identifying devices is OS-independent, I 
believe this should not be done in the OSPM specific modules.

Thanks,

Suravee

Re: [V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing

[Rafael J. Wysocki]

On Friday, April 24, 2015 04:08:31 PM Suravee Suthikulpanit wrote:
> On 4/16/15 20:45, Zheng, Lv wrote:
> > Before back porting this to ACPICA, let me ask one simple question.
> > According to the spec, the _CLS is optional and PCI specific.
> > So why should we implement it in ACPICA core not OSPM specific modules?
> > If this need to be implemented in ACPICA, then what about the following device identification objects?
> > _DDN, _HRV, _MLS, _PLD, _STR, _SUN
> >
> > Thanks and best regards
> > -Lv
> 
> Hi,
> 
> Sorry for late reply. As for the justification for introducing the _CLS 
> support in the ACPICA, this is mainly because ACPI does not currently 
> define _CID for certain device classes, which used to mostly be PCI 
> devices. Instead, ACPI spec mentioned that _CLS can be used for loading 
> generic drivers on hardware that is compatible with PCI-defined device 
> classes, but that is not implemented on the PCI bus (and is therefore 
> enumerated by ACPI.)

I think it would be good to point to the particular part of the spec
making that provision.  In what section is that mentioned, exactly?


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing

[Suthikulpanit, Suravee]

On 4/24/15, 21:28, "Rafael J. Wysocki" <rjw@rjwysocki.net> wrote:

>On Friday, April 24, 2015 04:08:31 PM Suravee Suthikulpanit wrote:
>> On 4/16/15 20:45, Zheng, Lv wrote:
>> > Before back porting this to ACPICA, let me ask one simple question.
>> > According to the spec, the _CLS is optional and PCI specific.
>> > So why should we implement it in ACPICA core not OSPM specific
>>modules?
>> > If this need to be implemented in ACPICA, then what about the
>>following device identification objects?
>> > _DDN, _HRV, _MLS, _PLD, _STR, _SUN
>> >
>> > Thanks and best regards
>> > -Lv
>> 
>> Hi,
>> 
>> Sorry for late reply. As for the justification for introducing the _CLS
>> support in the ACPICA, this is mainly because ACPI does not currently
>> define _CID for certain device classes, which used to mostly be PCI
>> devices. Instead, ACPI spec mentioned that _CLS can be used for loading
>> generic drivers on hardware that is compatible with PCI-defined device
>> classes, but that is not implemented on the PCI bus (and is therefore
>> enumerated by ACPI.)
>
>I think it would be good to point to the particular part of the spec
>making that provision.  In what section is that mentioned, exactly?

Here is the copied from section 6.1.3 _CLS (Class Code) from ACPI 5.1 spec:
"This object is used to supply OSPM with the PCI-defined class, subclass
and programming interface for a device. This object is optional but may be
useful for generic drivers written for PCI devices that move off of PCI
and are enumerated by ACPI.²

Otherwise, if the community think it¹s better to not putting the _CLS the
_CLS parsing code in ACPICA since, I can try looking into pulling the code
out of ACPICA. I also noticed a little issue in the patch series where the
ACPI_VALID_CLS is used in the patch 1 but defined in patch 2. I can send
out v9 with the fix once we agree on the _CLS parsing.

Thanks,

Suravee

Re: [V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing

[Rafael J. Wysocki]

On Sunday, April 26, 2015 10:45:29 PM Suthikulpanit, Suravee wrote:
> On 4/24/15, 21:28, "Rafael J. Wysocki" <rjw@rjwysocki.net> wrote:
> 
> >On Friday, April 24, 2015 04:08:31 PM Suravee Suthikulpanit wrote:
> >> On 4/16/15 20:45, Zheng, Lv wrote:
> >> > Before back porting this to ACPICA, let me ask one simple question.
> >> > According to the spec, the _CLS is optional and PCI specific.
> >> > So why should we implement it in ACPICA core not OSPM specific
> >>modules?
> >> > If this need to be implemented in ACPICA, then what about the
> >>following device identification objects?
> >> > _DDN, _HRV, _MLS, _PLD, _STR, _SUN
> >> >
> >> > Thanks and best regards
> >> > -Lv
> >> 
> >> Hi,
> >> 
> >> Sorry for late reply. As for the justification for introducing the _CLS
> >> support in the ACPICA, this is mainly because ACPI does not currently
> >> define _CID for certain device classes, which used to mostly be PCI
> >> devices. Instead, ACPI spec mentioned that _CLS can be used for loading
> >> generic drivers on hardware that is compatible with PCI-defined device
> >> classes, but that is not implemented on the PCI bus (and is therefore
> >> enumerated by ACPI.)
> >
> >I think it would be good to point to the particular part of the spec
> >making that provision.  In what section is that mentioned, exactly?
> 
> Here is the copied from section 6.1.3 _CLS (Class Code) from ACPI 5.1 spec:
> "This object is used to supply OSPM with the PCI-defined class, subclass
> and programming interface for a device. This object is optional but may be
> useful for generic drivers written for PCI devices that move off of PCI
> and are enumerated by ACPI."

OK, so the "move off of PCI" part should be understood as "something that
used to be on the PCI bus, but now may be included into an SoC directly
in which case it won't be a PCI device any more", right?

> Otherwise, if the community think it¹s better to not putting the _CLS the
> _CLS parsing code in ACPICA since, I can try looking into pulling the code
> out of ACPICA. I also noticed a little issue in the patch series where the
> ACPI_VALID_CLS is used in the patch 1 but defined in patch 2. I can send
> out v9 with the fix once we agree on the _CLS parsing.

To me that pretty much depends on the answer to the above question.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [V8 PATCH 1/3] ACPICA: Add ACPI _CLS processing

[Suravee Suthikulpanit]

On 04/27/2015 03:38 PM, Rafael J. Wysocki wrote:
> On Sunday, April 26, 2015 10:45:29 PM Suthikulpanit, Suravee wrote:
>> On 4/24/15, 21:28, "Rafael J. Wysocki" <rjw@rjwysocki.net> wrote:
>>
>>> On Friday, April 24, 2015 04:08:31 PM Suravee Suthikulpanit wrote:
>>>> On 4/16/15 20:45, Zheng, Lv wrote:
>>>>> Before back porting this to ACPICA, let me ask one simple question.
>>>>> According to the spec, the _CLS is optional and PCI specific.
>>>>> So why should we implement it in ACPICA core not OSPM specific
>>>> modules?
>>>>> If this need to be implemented in ACPICA, then what about the
>>>> following device identification objects?
>>>>> _DDN, _HRV, _MLS, _PLD, _STR, _SUN
>>>>>
>>>>> Thanks and best regards
>>>>> -Lv
>>>>
>>>> Hi,
>>>>
>>>> Sorry for late reply. As for the justification for introducing the _CLS
>>>> support in the ACPICA, this is mainly because ACPI does not currently
>>>> define _CID for certain device classes, which used to mostly be PCI
>>>> devices. Instead, ACPI spec mentioned that _CLS can be used for loading
>>>> generic drivers on hardware that is compatible with PCI-defined device
>>>> classes, but that is not implemented on the PCI bus (and is therefore
>>>> enumerated by ACPI.)
>>>
>>> I think it would be good to point to the particular part of the spec
>>> making that provision.  In what section is that mentioned, exactly?
>>
>> Here is the copied from section 6.1.3 _CLS (Class Code) from ACPI 5.1 spec:
>> "This object is used to supply OSPM with the PCI-defined class, subclass
>> and programming interface for a device. This object is optional but may be
>> useful for generic drivers written for PCI devices that move off of PCI
>> and are enumerated by ACPI."
>
> OK, so the "move off of PCI" part should be understood as "something that
> used to be on the PCI bus, but now may be included into an SoC directly
> in which case it won't be a PCI device any more", right?

That's right. For example, the SATA controller is a good example for 
this case. On most x86 platforms, they are often enumerated as PCI 
devices. However, in the ARM64 SOC (e.g. on AMD Seattle), it could be 
enumerated as non-PCI device.

Thanks,

Suravee

>
>> Otherwise, if the community think it¹s better to not putting the _CLS the
>> _CLS parsing code in ACPICA since, I can try looking into pulling the code
>> out of ACPICA. I also noticed a little issue in the patch series where the
>> ACPI_VALID_CLS is used in the patch 1 but defined in patch 2. I can send
>> out v9 with the fix once we agree on the _CLS parsing.
>
> To me that pretty much depends on the answer to the above question.
>
>