Re: [PATCH v3 2/5] i3c: master: Add ACPI support to i3c subsystem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On 11/8/24 9:33 AM, Shyam Sundar S K wrote:
As of now, the I3C subsystem only has ARM-specific initialization, and
there is no corresponding ACPI plumbing present. To address this, ACPI
support needs to be added to both the I3C core and DW driver.

Add support to get the ACPI handle from the _HID probed and parse the apci
object to retrieve the slave information from BIOS.

Based on the acpi object information propogated via BIOS, build the i3c
board information so that the same information can be used across the
driver to handle the slave requests.

Co-developed-by: Sanket Goswami <Sanket.Goswami@xxxxxxx>
Signed-off-by: Sanket Goswami <Sanket.Goswami@xxxxxxx>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
---
Cc: linux-acpi@xxxxxxxxxxxxxxx

  drivers/i3c/internals.h            |  3 ++
  drivers/i3c/master.c               | 84 ++++++++++++++++++++++++++++++
  drivers/i3c/master/dw-i3c-master.c |  7 +++
  include/linux/i3c/master.h         |  1 +
  4 files changed, 95 insertions(+)

To clarify: The DSDT example I mentioned in the previous version I meant it would be good to have in the patch/series itself. It helps both reviewing the patchset and future documentation.

First I was thinking commit log itself but I guess a full DSDT example showing SDP target devices becomes too large and e.g. Documentation/firmware-guide/acpi/enumeration.rst is better?

diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
index 433f6088b7ce..178bc0ebe6b6 100644
--- a/drivers/i3c/internals.h
+++ b/drivers/i3c/internals.h
@@ -10,6 +10,9 @@
#include <linux/i3c/master.h> +#define I3C_GET_PID 0x08
+#define I3C_GET_ADDR		0x7F
+

These are bit confusing. One could think these are commands while I3C_GET_PID is a shift and I3C_GET_ADDR is a mask in the code below.

Since these are actually defines for the ACPI _ADR I tried to find are there already similar defines in ACPI headers but could not find and e.g. drivers/ata/libata-acpi.c is interpreting itself so I guess it's ok to define here?

  void i3c_bus_normaluse_lock(struct i3c_bus *bus);
  void i3c_bus_normaluse_unlock(struct i3c_bus *bus);
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 6f3eb710a75d..0ceef2aa9161 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2251,6 +2251,84 @@ static int of_i3c_master_add_dev(struct i3c_master_controller *master,
  	return ret;
  }
+#if IS_ENABLED(CONFIG_ACPI)
+static int i3c_acpi_configure_master(struct i3c_master_controller *master)
+{
+	struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL};
+	enum i3c_addr_slot_status addrstatus;
+	struct i3c_dev_boardinfo *boardinfo;
+	struct device *dev = &master->dev;
+	struct fwnode_handle *fwnode;
+	struct acpi_device *adev;
+	u32 slv_addr, num_dev;
+	acpi_status status;
+	u64 val;
+
+	status = acpi_evaluate_object_typed(master->ahandle, "_DSD", NULL, &buf, ACPI_TYPE_PACKAGE);
+	if (ACPI_FAILURE(status)) {
+		dev_err(&master->dev, "Error reading _DSD:%s\n", acpi_format_exception(status));
+		return -ENODEV;
+	}
+
+	num_dev = device_get_child_node_count(dev);
+	if (!num_dev) {
+		dev_err(&master->dev, "Error: no child node present\n");
+		return -EINVAL;
+	}
+
+	device_for_each_child_node(dev, fwnode) {
+		adev = to_acpi_device_node(fwnode);
+		if (!adev)
+			return -ENODEV;
+
+		status = acpi_evaluate_integer(adev->handle, "_ADR", NULL, &val);
+		if (ACPI_FAILURE(status)) {
+			dev_err(&master->dev, "Error: eval _ADR failed\n");
+			return -EINVAL;
+		}
+		slv_addr = val & I3C_GET_ADDR;
+
+		boardinfo = devm_kzalloc(dev, sizeof(*boardinfo), GFP_KERNEL);
+		if (!boardinfo)
+			return -ENOMEM;
+
+		if (slv_addr) {
+			if (slv_addr > I3C_MAX_ADDR)
+				return -EINVAL;
+
+			addrstatus = i3c_bus_get_addr_slot_status(&master->bus, slv_addr);
+			if (addrstatus != I3C_ADDR_SLOT_FREE)
+				return -EINVAL;
+		}
+
+		boardinfo->static_addr = slv_addr;
+		if (boardinfo->static_addr > I3C_MAX_ADDR)
+			return -EINVAL;
+

slv_addr/static_addr > I3C_MAX_ADDR test is needless due masking above.

+		boardinfo->pid = val >> I3C_GET_PID;
+		if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
+		    I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
+			return -EINVAL;
+

Bit shifting makes PID and also test incorrect?




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux