[PATCH v2] media: cec: meson: ao-cec: enable the controller clock at probe

[PATCH v2] media: cec: meson: ao-cec: enable the controller clock at probe

[christian.koever-draxl]

From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

The CEC core calls .adap_log_addr() before .adap_enable(), so the first
register access can happen while the controller is still in power-off
mode (CEC_GEN_CNTL clock field 0). The probe only pulses the reset bit,
leaves the clock field at zero, and does so only after the adapter has
been registered, when the core may already be using it.

On S4 an access to the powered-off controller never completes: it
stalls the AO bus and hard-hangs the SoC. This triggers on every HDMI
unplug, because that invalidates the physical address and makes the
core rewrite the logical address.

Reset the controller and leave its clock running before the adapter is
registered, as .adap_enable() would.

Fixes: 7ec2c0f72cb1 ("media: platform: Add Amlogic Meson AO CEC Controller driver")
Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
---
Changes in v2:
- Set up the controller before cec_notifier_cec_adap_register() and
  cec_register_adapter(), not after: once registered, the core may call
  .adap_log_addr() straight away (reported by the Sashiko AI review).
- Link to v1: https://lore.kernel.org/r/20261025192046.208710-1-christian.koever-draxl@student.uibk.ac.at

Tool use (Documentation/process/generated-content.rst):
This work was done with the help of an AI coding assistant, Claude
(Anthropic), used through Claude Code over many sessions. It did a large
part of the hands-on work: it read the vendor kernel and the firmware
interfaces, probed registers on the board, drafted the code and the
changelogs, and ran the builds, checkpatch, dt_binding_check and the
tests on the hardware. I directed the work, reviewed every change and
the final patches, and take responsibility for them. There is no single
prompt to quote; the sessions were of the form "find out why X fails on
this board, fix it, test it on the box".

Tested on an Amediatech X98Q TV box (Amlogic S905W2, S4 family, 2 GiB),
vendor U-Boot and BL31/OP-TEE, Linux 7.3 plus this work.

 drivers/media/cec/platform/meson/ao-cec.c | 26 +++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/media/cec/platform/meson/ao-cec.c b/drivers/media/cec/platform/meson/ao-cec.c
index 2df0d3743077..4c03c829fe86 100644
--- a/drivers/media/cec/platform/meson/ao-cec.c
+++ b/drivers/media/cec/platform/meson/ao-cec.c
@@ -660,6 +660,28 @@ static int meson_ao_cec_probe(struct platform_device *pdev)
 
 	device_reset_optional(&pdev->dev);
 
+	/*
+	 * Setup Hardware, before the adapter is registered: from then on the
+	 * CEC core may call .adap_log_addr() at any time.
+	 *
+	 * Leave the controller with its gated clock running. The CEC core
+	 * calls .adap_log_addr() before it ever calls .adap_enable(), so the
+	 * first register access happens while the block would otherwise still
+	 * be in power-off mode (CEC_GEN_CNTL clk field == 0). On Amlogic S4
+	 * that access never completes: it stalls the AO bus and hard-hangs the
+	 * SoC - every CPU stops, IRQs included - which happens on every HDMI
+	 * hot-unplug, because that is what invalidates the CEC physical
+	 * address.
+	 */
+	writel_relaxed(CEC_GEN_CNTL_RESET |
+		       FIELD_PREP(CEC_GEN_CNTL_CLK_CTRL_MASK,
+				  CEC_GEN_CNTL_CLK_ENABLE),
+		       ao_cec->base + CEC_GEN_CNTL_REG);
+	fsleep(100);
+	writel_relaxed(FIELD_PREP(CEC_GEN_CNTL_CLK_CTRL_MASK,
+				  CEC_GEN_CNTL_CLK_ENABLE),
+		       ao_cec->base + CEC_GEN_CNTL_REG);
+
 	ao_cec->pdev = pdev;
 	platform_set_drvdata(pdev, ao_cec);
 
@@ -674,10 +696,6 @@ static int meson_ao_cec_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto out_probe_notify;
 
-	/* Setup Hardware */
-	writel_relaxed(CEC_GEN_CNTL_RESET,
-		       ao_cec->base + CEC_GEN_CNTL_REG);
-
 	return 0;
 
 out_probe_notify:

base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
-- 
2.56.0

Re: [PATCH v2] media: cec: meson: ao-cec: enable the controller clock at probe

[Hans Verkuil]

Hi Christian,

On 06/11/2026 11:21, christian.koever-draxl@student.uibk.ac.at wrote:
> From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
> 
> The CEC core calls .adap_log_addr() before .adap_enable(), so the first
> register access can happen while the controller is still in power-off
> mode (CEC_GEN_CNTL clock field 0). The probe only pulses the reset bit,
> leaves the clock field at zero, and does so only after the adapter has
> been registered, when the core may already be using it.
> 
> On S4 an access to the powered-off controller never completes: it
> stalls the AO bus and hard-hangs the SoC. This triggers on every HDMI
> unplug, because that invalidates the physical address and makes the
> core rewrite the logical address.
> 
> Reset the controller and leave its clock running before the adapter is
> registered, as .adap_enable() would.

Hmm, I have a few issues with this.

The commit log does not really match what is happening: the problem is
that the CEC adapter is registered while the controller is still
powered off. That's clearly not right, since once it is registered, it
can be used by userspace, so in that respect this patch seems sane.

Regarding the changed code for the RESET pin: Neil, can you review this?

Christian, can you also point out where in the CEC core adap_log_addr is called
before adap_enable? Because I am not sure that should be done at all, so
that might be a core framework bug. In any case, that is unrelated to the
actual patch.

Regards,

	Hans

> 
> Fixes: 7ec2c0f72cb1 ("media: platform: Add Amlogic Meson AO CEC Controller driver")
> Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
> ---
> Changes in v2:
> - Set up the controller before cec_notifier_cec_adap_register() and
>   cec_register_adapter(), not after: once registered, the core may call
>   .adap_log_addr() straight away (reported by the Sashiko AI review).
> - Link to v1: https://lore.kernel.org/r/20261025192046.208710-1-christian.koever-draxl@student.uibk.ac.at
> 
> Tool use (Documentation/process/generated-content.rst):
> This work was done with the help of an AI coding assistant, Claude
> (Anthropic), used through Claude Code over many sessions. It did a large
> part of the hands-on work: it read the vendor kernel and the firmware
> interfaces, probed registers on the board, drafted the code and the
> changelogs, and ran the builds, checkpatch, dt_binding_check and the
> tests on the hardware. I directed the work, reviewed every change and
> the final patches, and take responsibility for them. There is no single
> prompt to quote; the sessions were of the form "find out why X fails on
> this board, fix it, test it on the box".
> 
> Tested on an Amediatech X98Q TV box (Amlogic S905W2, S4 family, 2 GiB),
> vendor U-Boot and BL31/OP-TEE, Linux 7.3 plus this work.
> 
>  drivers/media/cec/platform/meson/ao-cec.c | 26 +++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/cec/platform/meson/ao-cec.c b/drivers/media/cec/platform/meson/ao-cec.c
> index 2df0d3743077..4c03c829fe86 100644
> --- a/drivers/media/cec/platform/meson/ao-cec.c
> +++ b/drivers/media/cec/platform/meson/ao-cec.c
> @@ -660,6 +660,28 @@ static int meson_ao_cec_probe(struct platform_device *pdev)
>  
>  	device_reset_optional(&pdev->dev);
>  
> +	/*
> +	 * Setup Hardware, before the adapter is registered: from then on the
> +	 * CEC core may call .adap_log_addr() at any time.
> +	 *
> +	 * Leave the controller with its gated clock running. The CEC core
> +	 * calls .adap_log_addr() before it ever calls .adap_enable(), so the
> +	 * first register access happens while the block would otherwise still
> +	 * be in power-off mode (CEC_GEN_CNTL clk field == 0). On Amlogic S4
> +	 * that access never completes: it stalls the AO bus and hard-hangs the
> +	 * SoC - every CPU stops, IRQs included - which happens on every HDMI
> +	 * hot-unplug, because that is what invalidates the CEC physical
> +	 * address.
> +	 */
> +	writel_relaxed(CEC_GEN_CNTL_RESET |
> +		       FIELD_PREP(CEC_GEN_CNTL_CLK_CTRL_MASK,
> +				  CEC_GEN_CNTL_CLK_ENABLE),
> +		       ao_cec->base + CEC_GEN_CNTL_REG);
> +	fsleep(100);
> +	writel_relaxed(FIELD_PREP(CEC_GEN_CNTL_CLK_CTRL_MASK,
> +				  CEC_GEN_CNTL_CLK_ENABLE),
> +		       ao_cec->base + CEC_GEN_CNTL_REG);
> +
>  	ao_cec->pdev = pdev;
>  	platform_set_drvdata(pdev, ao_cec);
>  
> @@ -674,10 +696,6 @@ static int meson_ao_cec_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto out_probe_notify;
>  
> -	/* Setup Hardware */
> -	writel_relaxed(CEC_GEN_CNTL_RESET,
> -		       ao_cec->base + CEC_GEN_CNTL_REG);
> -
>  	return 0;
>  
>  out_probe_notify:
> 
> base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614