| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7cfc1f2a-cb4a-4ddc-af0a-df7793ad4487@linux.intel.com>
Date: Mon, 4 Nov 2024 16:06:29 +0200
From: Jarkko Nikula <jarkko.nikula@...ux.intel.com>
To: Shyam Sundar S K <Shyam-sundar.S-k@....com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc: Sanket.Goswami@....com, linux-i3c@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org
Subject: Re: [PATCH v2 3/6] i3c: master: Add ACPI support to i3c subsystem
Hi
+ linux-acpi
On 10/23/24 8:51 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.
>
I think it would be good to have a DSDT example here?
> Co-developed-by: Sanket Goswami <Sanket.Goswami@....com>
> Signed-off-by: Sanket Goswami <Sanket.Goswami@....com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@....com>
> ---
> drivers/i3c/internals.h | 2 +
> drivers/i3c/master.c | 84 ++++++++++++++++++++++++++++++
> drivers/i3c/master/dw-i3c-master.c | 7 +++
> include/linux/i3c/master.h | 1 +
> 4 files changed, 94 insertions(+)
>
> diff --git a/drivers/i3c/internals.h b/drivers/i3c/internals.h
> index 433f6088b7ce..d2d6c69b19dd 100644
> --- a/drivers/i3c/internals.h
> +++ b/drivers/i3c/internals.h
> @@ -10,6 +10,8 @@
>
> #include <linux/i3c/master.h>
>
> +#define AMD_I3C_GET_SLAVE_ADDR 0x30
> +
> 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..7d23c32e1c0f 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 >> AMD_I3C_GET_SLAVE_ADDR;
This doesn't seem to match with ACPI 6.5 spec [1] chapter 6.1.1 _ADR
(Address)? Address encoding for I3C says:
Bits [63:52] - Reserved
Bits [51:48] - Master Instance
Bits [47:0] - I3C Device Provisional ID, following encoding defined in
the MIPI
Specification for I3C.
If an I3C device supports a static address instead of a Provisional ID,
then bits
[47:7] are Reserved (zero), and bits [6:0] are the 7-bit static address.
1. https://uefi.org/sites/default/files/resources/ACPI_Spec_6_5_Aug29.pdf
> +
> + 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;
> +
> + addrstatus = i3c_bus_get_addr_slot_status(&master->bus, boardinfo->static_addr);
> + if (addrstatus != I3C_ADDR_SLOT_FREE)
> + return -EINVAL;
> +
> + boardinfo->pid = (val & GENMASK(47, 0));
> + if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> + I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
> + return -EINVAL;
> +
> + /*
> + * According to the specification, SETDASA is not supported for DIMM slaves
> + * during device discovery. Therefore, AMD BIOS will populate same initial
> + * dynamic address as the static address.
> + */
> + boardinfo->init_dyn_addr = boardinfo->static_addr;
> + list_add_tail(&boardinfo->node, &master->boardinfo.i3c);
> + }
> +
> + return 0;
> +}
> +#else
> +static int i3c_acpi_configure_master(struct i3c_master_controller *master) { return 0; }
> +#endif
> +
> static int of_populate_i3c_bus(struct i3c_master_controller *master)
> {
> struct device *dev = &master->dev;
> @@ -2771,6 +2849,12 @@ int i3c_master_register(struct i3c_master_controller *master,
> master->dev.coherent_dma_mask = parent->coherent_dma_mask;
> master->dev.dma_parms = parent->dma_parms;
>
> + if (has_acpi_companion(master->dev.parent)) {
> + ret = i3c_acpi_configure_master(master);
> + if (ret < 0)
> + return ret;
> + }
> +
> ret = of_populate_i3c_bus(master);
> if (ret)
> goto err_put_dev;
> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index fd58a95ae1c3..f683e2a398ad 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -1602,6 +1602,13 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
> master->maxdevs = ret >> 16;
> master->free_pos = GENMASK(master->maxdevs - 1, 0);
>
> +#if IS_ENABLED(CONFIG_ACPI)
> + ACPI_COMPANION_SET(&master->base.dev, ACPI_COMPANION(&pdev->dev));
> + master->base.ahandle = acpi_device_handle(ACPI_COMPANION(&pdev->dev));
> + if (!master->base.ahandle)
> + dev_err(&pdev->dev, "Failed to get acpi device handle\n");
> +#endif
> +
> INIT_WORK(&master->hj_work, dw_i3c_hj_work);
> ret = i3c_master_register(&master->base, &pdev->dev,
> &dw_mipi_i3c_ops, false);
> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
> index 2a1ed05d5782..367faf7c4bf3 100644
> --- a/include/linux/i3c/master.h
> +++ b/include/linux/i3c/master.h
> @@ -523,6 +523,7 @@ struct i3c_master_controller {
> } boardinfo;
> struct i3c_bus bus;
> struct workqueue_struct *wq;
> + acpi_handle ahandle;
> };
>
> /**
Powered by blists - more mailing lists