linux-kernel - Re: [PATCH v3 4/5] i3c: master: Add support for SETAASA CCC

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9a78959a-391b-4a77-a33e-e230fbf2e0c0@linux.intel.com>
Date: Wed, 13 Nov 2024 12:00:06 +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
Subject: Re: [PATCH v3 4/5] i3c: master: Add support for SETAASA CCC

Hi

On 11/8/24 9:33 AM, Shyam Sundar S K wrote:
> @@ -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);
>   
This looks wrong the previous patch adds unconditional call to 
i3c_master_add_spd_dev() and this patch makes it conditional. Can 
previous patch then cause a regression if applied without this one?

>   	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;
> +

I'm still suspicious about this one when existence of _STR for the host 
controller causes normal bus initialization to be skipped. I.e. like below.

Device (I3C0)
{
	_STR ("My I3C Host Controller")
...