All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Nikula <jarkko.nikula@linux.intel.com>
To: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Sanket.Goswami@amd.com, linux-i3c@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/5] i3c: master: Add support for SETAASA CCC
Date: Wed, 13 Nov 2024 12:00:06 +0200	[thread overview]
Message-ID: <9a78959a-391b-4a77-a33e-e230fbf2e0c0@linux.intel.com> (raw)
In-Reply-To: <20241108073323.523805-5-Shyam-sundar.S-k@amd.com>

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")
...

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

WARNING: multiple messages have this Message-ID (diff)
From: Jarkko Nikula <jarkko.nikula@linux.intel.com>
To: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Sanket.Goswami@amd.com, linux-i3c@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 4/5] i3c: master: Add support for SETAASA CCC
Date: Wed, 13 Nov 2024 12:00:06 +0200	[thread overview]
Message-ID: <9a78959a-391b-4a77-a33e-e230fbf2e0c0@linux.intel.com> (raw)
In-Reply-To: <20241108073323.523805-5-Shyam-sundar.S-k@amd.com>

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")
...

  reply	other threads:[~2024-11-13 11:01 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-08  7:33 [PATCH v3 0/5] Introduce initial support for the AMD I3C (non-HCI) to DW driver Shyam Sundar S K
2024-11-08  7:33 ` Shyam Sundar S K
2024-11-08  7:33 ` [PATCH v3 1/5] i3c: dw: Add support for AMDI0015 ACPI ID Shyam Sundar S K
2024-11-08  7:33   ` Shyam Sundar S K
2024-11-12  8:16   ` Jarkko Nikula
2024-11-12  8:16     ` Jarkko Nikula
2024-11-12  8:48     ` Shyam Sundar S K
2024-11-12  8:48       ` Shyam Sundar S K
2024-11-12  9:48       ` Jarkko Nikula
2024-11-12  9:48         ` Jarkko Nikula
2024-11-12 13:07         ` Shyam Sundar S K
2024-11-12 13:07           ` Shyam Sundar S K
2024-11-14  8:00   ` Jarkko Nikula
2024-11-14  8:00     ` Jarkko Nikula
2024-11-08  7:33 ` [PATCH v3 2/5] i3c: master: Add ACPI support to i3c subsystem Shyam Sundar S K
2024-11-08  7:33   ` Shyam Sundar S K
2024-11-12 15:17   ` Jarkko Nikula
2024-11-12 15:17     ` Jarkko Nikula
2024-11-13  9:42   ` Jarkko Nikula
2024-11-13  9:42     ` Jarkko Nikula
2024-11-13 14:21   ` Heikki Krogerus
2024-11-13 14:21     ` Heikki Krogerus
2024-11-14  4:33     ` Shyam Sundar S K
2024-11-14  4:33       ` Shyam Sundar S K
2024-11-14  7:56       ` Alexandre Belloni
2024-11-14  7:56         ` Alexandre Belloni
2024-11-14 11:04         ` Shyam Sundar S K
2024-11-14 11:04           ` Shyam Sundar S K
2024-11-14  8:00       ` Jarkko Nikula
2024-11-14  8:00         ` Jarkko Nikula
2024-11-08  7:33 ` [PATCH v3 3/5] i3c: master: Add a routine to include the I3C SPD device Shyam Sundar S K
2024-11-08  7:33   ` Shyam Sundar S K
2024-11-08  7:33 ` [PATCH v3 4/5] i3c: master: Add support for SETAASA CCC Shyam Sundar S K
2024-11-08  7:33   ` Shyam Sundar S K
2024-11-13 10:00   ` Jarkko Nikula [this message]
2024-11-13 10:00     ` Jarkko Nikula
2024-11-08  7:33 ` [PATCH v3 5/5] i3c: dw: Add quirk to address OD/PP timing issue on AMD platform Shyam Sundar S K
2024-11-08  7:33   ` Shyam Sundar S K
2024-11-14  8:01   ` Jarkko Nikula
2024-11-14  8:01     ` Jarkko Nikula

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9a78959a-391b-4a77-a33e-e230fbf2e0c0@linux.intel.com \
    --to=jarkko.nikula@linux.intel.com \
    --cc=Sanket.Goswami@amd.com \
    --cc=Shyam-sundar.S-k@amd.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=linux-i3c@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.