| Message ID | 20241108073323.523805-3-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce initial support for the AMD I3C (non-HCI) to DW driver | expand |
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@amd.com> > Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > --- > Cc: linux-acpi@vger.kernel.org > > 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?
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 + 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; + + addrstatus = i3c_bus_get_addr_slot_status(&master->bus, boardinfo->static_addr); + if (addrstatus != I3C_ADDR_SLOT_FREE) + return -EINVAL; + + boardinfo->pid = val >> I3C_GET_PID; + 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, 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 1a7c300b6d45..d44b771d03e1 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; }; /**