| 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: <9d8fdd74-5e82-47b2-ba7b-c33f791065fa@amd.com>
Date: Wed, 6 Nov 2024 19:47:19 +0530
From: Shyam Sundar S K <Shyam-sundar.S-k@....com>
To: Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc: Sanket.Goswami@....com, linux-i3c@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/6] i3c: master: Add support for SETAASA CCC
On 11/4/2024 20:30, Jarkko Nikula wrote:
> On 10/23/24 8:51 AM, Shyam Sundar S K wrote:
>> I3C devices like DIMMs over SPD use SETAASA for bus discovery
>> instead of
>> SETDASA. Add a new routine for I3C host controller drivers to use.
>> If the
>> I3C slave on the bus is an SPD device, skip the regular DAA process.
>>
>> According to the SPD spec[1], use SETAASA for bus discovery, and avoid
>> sending RSTDAA and DISEC, as they are considered illegal. Skip this
>> entire
>> process if the slave is SPD-compliant, as indicated by the
>> "jdec_spd" flag
>> from the BIOS.
>>
>> [1] https://www.jedec.org/system/files/docs/JESD300-5B.01.pdf
>> (section 2.4 and 2.6.3)
>>
> SETAASA seems to come in MIPI v1.1.1 specification. I think worth to
> mention in commit log.
>
OK. I will do it in the next version.
>> 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/master.c | 32
>> +++++++++++++++++++++++++++++-
>> drivers/i3c/master/dw-i3c-master.c | 1 +
>> include/linux/i3c/ccc.h | 1 +
>> include/linux/i3c/master.h | 1 +
>> 4 files changed, 34 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
>> index ba6f17cb8aa6..1596efd6d82a 100644
>> --- a/drivers/i3c/master.c
>> +++ b/drivers/i3c/master.c
>> @@ -1657,6 +1657,21 @@ i3c_master_register_new_i3c_devs(struct
>> i3c_master_controller *master)
>> }
>> }
>> +static int i3c_master_setaasa_locked(struct i3c_master_controller
>> *master)
>> +{
>> + struct i3c_ccc_cmd_dest dest;
>> + struct i3c_ccc_cmd cmd;
>> + int ret;
>> +
>> + i3c_ccc_cmd_dest_init(&dest, I3C_BROADCAST_ADDR, 0);
>> + i3c_ccc_cmd_init(&cmd, false, I3C_CCC_SETAASA, &dest, 1);
>> +
>> + ret = i3c_master_send_ccc_cmd_locked(master, &cmd);
>> + i3c_ccc_cmd_dest_cleanup(&dest);
>> +
>> + return ret;
>> +}
>> +
>> static int i3c_master_add_spd_dev(struct i3c_master_controller
>> *master,
>> struct i3c_dev_boardinfo *boardinfo)
>> {
>> @@ -1684,6 +1699,10 @@ static int i3c_master_add_spd_dev(struct
>> i3c_master_controller *master,
>> i3cdev->info.pid = i3cdev->boardinfo->pid;
>> i3cdev->info.dyn_addr = i3cdev->boardinfo->init_dyn_addr;
>> + ret = i3c_master_setaasa_locked(master);
>> + if (ret)
>> + goto err_free_dev;
>> +
>> i3c_bus_normaluse_lock(&master->bus);
>> i3c_master_register_new_i3c_devs(master);
>> i3c_bus_normaluse_unlock(&master->bus);
>> @@ -1907,7 +1926,14 @@ static int i3c_master_bus_init(struct
>> i3c_master_controller *master)
>> goto err_bus_cleanup;
>> }
>> - i3c_master_add_spd_dev(master, i3cboardinfo);
>> + /*
>> + * If the I3C slave on the bus is SPD device, then do not
>> follow the regular
>> + * DAA process. Also, as per SPD spec SETAASA is required for
>> the bus discovery
>> + * and sending RSTDAA and DISEC is considered as illegal. So
>> skip the entire process
>> + * if the jdec_spd flag has been identified from the BIOS.
>> + */
>> + if (master->jdec_spd)
>> + return i3c_master_add_spd_dev(master, i3cboardinfo);
>> if (master->ops->set_speed) {
>> ret = master->ops->set_speed(master,
>> I3C_OPEN_DRAIN_SLOW_SPEED);
>> @@ -2311,6 +2337,10 @@ static int i3c_acpi_configure_master(struct
>> i3c_master_controller *master)
>> return -ENODEV;
>> }
>> + status = acpi_evaluate_object(master->ahandle, "_STR", NULL,
>> NULL);
>> + if (ACPI_SUCCESS(status))
>> + master->jdec_spd = true;
>> +
>
> Am I right "_STR" object should carry a string "jdec_spd"? But this
> code is not actually checking it, only the existence of _STR?
>
I don't think so.. its just the string SPD/DIMM.
Scope (\_SB.I3CA)
{
Name (_STR, Unicode ("SPD/DIMM")) // _STR: Description String
Name (_DSD, Package (0x04) // _DSD: Device-Specific Data
{
...
})
Name (MST0, Package (0x02)
{
...
})
Device (SPD0)
{
...
}
Device (PMI0)
{
...
}
Device (RCD0)
{
...
}
}
Thanks,
Shyam
>> num_dev = device_get_child_node_count(dev);
>> if (!num_dev) {
>> dev_err(&master->dev, "Error: no child node present\n");
>> diff --git a/drivers/i3c/master/dw-i3c-master.c
>> b/drivers/i3c/master/dw-i3c-master.c
>> index f683e2a398ad..90a43209e55e 100644
>> --- a/drivers/i3c/master/dw-i3c-master.c
>> +++ b/drivers/i3c/master/dw-i3c-master.c
>> @@ -282,6 +282,7 @@ static bool
>> dw_i3c_master_supports_ccc_cmd(struct i3c_master_controller *m,
>> case I3C_CCC_GETSTATUS:
>> case I3C_CCC_GETMXDS:
>> case I3C_CCC_GETHDRCAP:
>> + case I3C_CCC_SETAASA:
>> return true;
>> default:
>> return false;
>> diff --git a/include/linux/i3c/ccc.h b/include/linux/i3c/ccc.h
>> index ad59a4ae60d1..a145d766ab6f 100644
>> --- a/include/linux/i3c/ccc.h
>> +++ b/include/linux/i3c/ccc.h
>> @@ -32,6 +32,7 @@
>> #define I3C_CCC_DEFSLVS I3C_CCC_ID(0x8, true)
>> #define I3C_CCC_ENTTM I3C_CCC_ID(0xb, true)
>> #define I3C_CCC_ENTHDR(x) I3C_CCC_ID(0x20 + (x), true)
>> +#define I3C_CCC_SETAASA I3C_CCC_ID(0x29, true)
>> /* Unicast-only commands */
>> #define I3C_CCC_SETDASA I3C_CCC_ID(0x7, false)
>> diff --git a/include/linux/i3c/master.h b/include/linux/i3c/master.h
>> index 367faf7c4bf3..cd8390d8b469 100644
>> --- a/include/linux/i3c/master.h
>> +++ b/include/linux/i3c/master.h
>> @@ -516,6 +516,7 @@ struct i3c_master_controller {
>> const struct i3c_master_controller_ops *ops;
>> unsigned int secondary : 1;
>> unsigned int init_done : 1;
>> + unsigned int jdec_spd : 1;
>> unsigned int hotjoin: 1;
>> struct {
>> struct list_head i3c;
>
Powered by blists - more mailing lists