* Re: [PATCH v4 08/16] spi: cadence-quadspi: add PHY tuning support
From: sashiko-bot @ 2026-06-18 7:59 UTC (permalink / raw)
To: Santhosh Kumar K; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260618073725.84733-9-s-k6@ti.com>
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] Unchecked dereference of `cqspi->ddata` in `cqspi_mem_op_execute_tuning` when it can be NULL.
- [High] Passing `.rodata` buffer to `spi_mem_exec_op()` violates SPI API DMA constraints.
- [High] Dual-region PHY tuning arithmetic lacks bounds clamping, causing `u8` underflows.
- [High] The tuning loops sweep `read_delay`, but `cqspi_phy_apply_setting` fails to update the hardware register with the new delay.
- [High] PHY tuning modifies shared SPI controller registers without holding the SPI bus lock, creating a race condition with concurrent transfers.
- [High] The PHY tuning sequence performs direct MMIO to controller registers without acquiring a PM runtime reference.
- [Medium] Missing error check in SDR tuning causes invalid read_delay configuration.
- [Medium] The PHY tuning search loops ignore critical hardware timeouts and memory allocation errors, causing the system to needlessly stall.
--
commit a0e116638901b220157d7f4a14b288dc44e8d0fd
Author: Santhosh Kumar K <s-k6@ti.com>
Subject: spi: cadence-quadspi: add PHY tuning support
This commit adds an execute_tuning callback for delay-line calibration on
Cadence QSPI controllers. It implements tuning sweeps for both DDR and SDR
modes. The calibration relies on writing and reading a predefined pattern
to and from flash memory.
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index f4efb92c9446a..05e1b7a2f3433 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
[ ... ]
> @@ -1559,6 +1640,1678 @@ static bool cqspi_supports_mem_op(struct spi_mem *mem,
> +static int cqspi_write_pattern_to_cache(struct cqspi_flash_pdata *f_pdata,
> + struct spi_mem *mem,
> + struct spi_mem_op *write_op)
> +{
> + struct device *dev = &f_pdata->cqspi->pdev->dev;
> + int ret;
> +
> + write_op->data.nbytes = sizeof(phy_tuning_pattern);
> + write_op->data.buf.out = phy_tuning_pattern;
> +
> + ret = spi_mem_exec_op(mem, write_op);
[Severity: High]
Does passing phy_tuning_pattern directly to spi_mem_exec_op violate SPI DMA
constraints? The phy_tuning_pattern array is defined as static const u8 and
resides in the rodata section.
The SPI framework mandates that buffers passed to the SPI core must be
DMA-capable, as fallback paths will attempt to call dma_map_single on them.
Mapping a rodata buffer violates the DMA API and will trigger kernel panics
or warnings when CONFIG_DMA_API_DEBUG is enabled. Could this be copied to a
dynamically allocated bounce buffer first?
[ ... ]
> +static int cqspi_phy_apply_setting(struct cqspi_flash_pdata *f_pdata,
> + struct phy_setting *phy)
> +{
> + struct cqspi_st *cqspi = f_pdata->cqspi;
> + unsigned int reg;
> + int ret;
> +
> + reg = readl(cqspi->iobase + CQSPI_REG_READCAPTURE);
> + reg |= BIT(CQSPI_REG_READCAPTURE_EDGE_LSB);
> + writel(reg, cqspi->iobase + CQSPI_REG_READCAPTURE);
> +
> + cqspi_set_dll(cqspi->iobase, phy->rx, phy->tx);
> +
> + ret = cqspi_resync_dll(cqspi);
> + if (ret)
> + return ret;
> +
> + f_pdata->phy_setting.read_delay = phy->read_delay;
> + return 0;
> +}
[Severity: High]
Is the read_delay value actually being programmed into the hardware during
the tuning sweeps?
While cqspi_phy_apply_setting updates the DLL and edge registers, it appears
it never writes the phy->read_delay into the DELAY_LSB field of the
CQSPI_REG_READCAPTURE register.
Because the internal configuration updates are bypassed when the SPI clock
frequency has not changed, doesn't this mean the hardware read_delay remains
fixed at its initial value throughout the entire tuning sequence?
[ ... ]
> +static int cqspi_find_rx_low_ddr(struct cqspi_flash_pdata *f_pdata,
> + struct spi_mem *mem, struct phy_setting *phy)
> +{
> + struct device *dev = &f_pdata->cqspi->pdev->dev;
> + int ret;
> +
> + do {
> + phy->rx = CQSPI_PHY_RX_LOW_SEARCH_START;
> + do {
> + ret = cqspi_phy_apply_setting(f_pdata, phy);
> + if (!ret) {
> + ret = cqspi_phy_check_pattern(f_pdata, mem);
> + if (!ret)
> + return 0;
> + }
> +
> + phy->rx += CQSPI_PHY_DDR_SEARCH_STEP;
> + } while (phy->rx <= CQSPI_PHY_RX_LOW_SEARCH_END);
[Severity: Medium]
Will this loop stall the boot process needlessly if a fatal error occurs?
If cqspi_phy_apply_setting returns -ETIMEDOUT because the DLL failed to
lock, or cqspi_phy_check_pattern returns -ENOMEM due to an allocation error,
the loop checks if (!ret) and then blindly continues to the next coordinate.
Since these operations wait up to 600us on each iteration, ignoring these
environmental errors instead of failing fast seems to cause a significant
stall. Should we check for specific fatal error codes and abort?
[ ... ]
> + /* Compare Manhattan distances: choose corner furthest from gap */
> + if ((abs(gaplow.tx - bottomleft.tx) +
> + abs(gaplow.rx - bottomleft.rx)) <
> + (abs(gaphigh.tx - topright.tx) +
> + abs(gaphigh.rx - topright.rx))) {
> + /* Topright further: Use Region 2, 16 taps inward */
> + searchpoint = topright;
> + searchpoint.tx -= 16;
> + searchpoint.rx -= (16 * (topright.rx - bottomleft.rx)) /
> + (topright.tx - bottomleft.tx);
[Severity: High]
Can this proportional adjustment underflow the u8 searchpoint.rx variable?
When resolving the dual-region tuning target, if the calculated adjustment
exceeds topright.rx (e.g. if topright.rx is 24 and the adjustment evaluates
to 27), the u8 variable will severely underflow and wrap to 253.
This wrapped value is then passed into cqspi_set_dll where the 0x7F mask
would truncate it to an arbitrary out-of-bounds point, corrupting the
calibration. Should there be a clamp applied here?
[ ... ]
> +static int cqspi_phy_tuning_sdr(struct cqspi_flash_pdata *f_pdata,
> + struct spi_mem *mem)
> +{
[ ... ]
> + do {
> + ret = cqspi_find_rx_low_sdr(f_pdata, mem, &rxlow);
> +
> + if (ret)
> + rxlow.read_delay++;
> + } while (ret && rxlow.read_delay <= CQSPI_PHY_MAX_RD);
> +
> + /* Find rxhigh: Decrement from RX=127 at same read_delay */
> +
[ ... ]
> +
> + rxhigh.read_delay = rxlow.read_delay;
> + ret = cqspi_find_rx_high_sdr(f_pdata, mem, &rxhigh, rxlow.rx);
[Severity: Medium]
Is there a missing error check after the first search loop?
Unlike the DDR implementation, if the loop exceeds CQSPI_PHY_MAX_RD without
finding rxlow, it exits with a non-zero ret. However, the code proceeds
directly to searching rxhigh with an out-of-bounds rxlow.read_delay (5) and
an invalid rxlow.rx (128). This causes out-of-bounds values to be applied to
the hardware configuration. Could we add an if (ret) goto out; check here?
[ ... ]
> +static int cqspi_am654_ospi_execute_tuning(struct spi_mem *mem,
> + struct spi_mem_op *read_op,
> + struct spi_mem_op *write_op)
> +{
[ ... ]
> + if (read_op->cmd.dtr || read_op->addr.dtr || read_op->dummy.dtr ||
> + read_op->data.dtr) {
> + f_pdata->use_dqs = true;
> + cqspi_phy_pre_config(cqspi, f_pdata, false);
> + ret = cqspi_phy_tuning_ddr(f_pdata, mem);
> + } else {
> + f_pdata->use_dqs = false;
> + cqspi_phy_pre_config(cqspi, f_pdata, true);
> + ret = cqspi_phy_tuning_sdr(f_pdata, mem);
> + }
[Severity: High]
Are we adequately protecting concurrent transfers and power management state
during these tuning calls?
The spi_mem_execute_tuning framework API does not acquire the SPI
controller's bus_lock_mutex. Consequently, this callback runs lock-free. It
performs raw read-modify-write operations on shared controller registers
like CQSPI_REG_CONFIG and unconditionally disables the controller via
cqspi_resync_dll(). If another SPI device on the same bus initiates a
concurrent transfer, its transfer could be aborted or corrupted by these
unguarded register writes.
[Severity: High]
Additionally, the tuning sequence accesses the controller's MMIO registers
directly but does not acquire an active PM runtime reference with
pm_runtime_resume_and_get(). If the SPI controller is suspended by
auto-suspend immediately after an inner spi_mem_exec_op returns, subsequent
manual MMIO accesses might hit unclocked hardware and trigger synchronous
external aborts.
[ ... ]
> +static int cqspi_mem_op_execute_tuning(struct spi_mem *mem,
> + struct spi_mem_op *read_op,
> + struct spi_mem_op *write_op)
> +{
> + struct cqspi_st *cqspi =
> + spi_controller_get_devdata(mem->spi->controller);
> +
> + if (!cqspi->ddata->execute_tuning)
> + return -EOPNOTSUPP;
> +
> + return cqspi->ddata->execute_tuning(mem, read_op, write_op);
> +}
[Severity: High]
Can cqspi->ddata be NULL here?
The execute_tuning callback is unconditionally added to cqspi_mem_ops. In
this driver, cqspi->ddata is populated via of_device_get_match_data(), which
can return NULL for generic fallbacks or manual device instantiation. If
ddata is NULL, dereferencing cqspi->ddata->execute_tuning will trigger a
kernel panic when the SPI core attempts to execute tuning.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618073725.84733-1-s-k6@ti.com?part=8
^ permalink raw reply
* Re: [PATCH v4 16/16] mtd: spi-nor: run PHY tuning after init and update dirmap frequency
From: sashiko-bot @ 2026-06-18 8:01 UTC (permalink / raw)
To: Santhosh Kumar K; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260618073725.84733-17-s-k6@ti.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] If probe steps fail after device initialization, `spi_nor_probe()` returns without restoring hardware state, leaving the device in an inconsistent configuration across soft reboots.
--
commit 7ac575a84edff91451a5b41913132dbcedb404cd
Author: Santhosh Kumar K <s-k6@ti.com>
mtd: spi-nor: run PHY tuning after init and update dirmap frequency
Run PHY tuning in spi_nor_probe() before creating dirmaps so the validated
frequency is available at dirmap construction time, falling back to base
rate on failure.
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 2c9859fb07949..e9e66d28fa52a 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
[ ... ]
> @@ -3773,6 +3776,17 @@ static int spi_nor_probe(struct spi_mem *spimem)
> return -ENOMEM;
> }
>
> + /*
> + * Populate the persistent template and run PHY tuning before dirmap
> + * creation so the validated frequency feeds into the dirmap op.
> + * Tuning failure is non-fatal; the device operates at base speed.
> + */
> + nor->max_read_op = spi_nor_spimem_get_read_op(nor);
> +
> + ret = spi_mem_execute_tuning(spimem, &nor->max_read_op, NULL);
> + if (ret && ret != -EOPNOTSUPP)
> + dev_warn(dev, "Failed to execute PHY tuning: %d\n", ret);
> +
> ret = spi_nor_create_read_dirmap(nor);
> if (ret)
> return ret;
[Severity: High]
This isn't a bug introduced by this patch, but does spi_nor_probe() fail to
restore the hardware state on error paths after device initialization?
Looking at the error returns here for spi_nor_create_read_dirmap() and
subsequent calls like spi_nor_create_write_dirmap() or mtd_device_register(),
the function returns the error code directly without calling spi_nor_restore().
Because spi_nor_scan() executes spi_nor_init() earlier in the probe to
modify the physical flash hardware state (such as enabling 4-byte address
mode or Octal DTR mode), returning without unwinding these changes leaves
the flash in a non-default mode.
Unlike spi_nor_remove() and spi_nor_shutdown() which explicitly call
spi_nor_restore() to revert the flash to its default 3-byte/1-1-1 state,
this error path lacks teardown, and spi_nor_restore() is not registered as
a devm action.
If probe fails here, could this prevent the system's ROM bootloader from
reading the flash after a soft reboot, effectively bricking the device
until a hard power cycle?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618073725.84733-1-s-k6@ti.com?part=16
^ permalink raw reply
* Re: [PATCH v4 14/16] mtd: spinand: negotiate optimal PHY operating point before dirmap creation
From: sashiko-bot @ 2026-06-18 8:02 UTC (permalink / raw)
To: Santhosh Kumar K; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260618073725.84733-15-s-k6@ti.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Fallback to SSDR during PHY tuning does not invalidate ODTR templates, causing a mode mismatch and persistent flash failure upon system resume.
- [High] Fallback PHY tuning downgrades only the primary read operation, leaving write, update, and continuous read operations at their original unvalidated high-speed variants.
- [High] Missing reset of `max_freq` on PHY tuning error paths and across tuning loops violates the zero-frequency invariant, leading to operations at unvalidated frequencies.
--
commit 0f63572c37a35621b6ca824014ba943d4c511e93
Author: Santhosh Kumar K <s-k6@ti.com>
mtd: spinand: negotiate optimal PHY operating point before dirmap creation
This commit moves dirmap creation to spinand_probe() and introduces PHY
tuning negotiation before dirmaps are created. It iterates through DTR
and SDR variants in descending performance order to find the best PHY
operating point, and falls back to non-PHY modes on complete failure.
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index b678d05342979..5dcfaabaf2cc5 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -1294,9 +1300,11 @@ static int spinand_create_dirmap(struct spinand_device *spinand,
> /* Read descriptor */
> info.primary_op_tmpl = *spinand->op_templates->read_cache;
> info.primary_op_tmpl.data.ecc = enable_ecc;
> + info.primary_op_tmpl.max_freq = spinand->max_read_op.max_freq;
> if (secondary_op) {
> info.secondary_op_tmpl = *spinand->op_templates->cont_read_cache;
> info.secondary_op_tmpl.data.ecc = enable_ecc;
> + info.secondary_op_tmpl.max_freq = spinand->max_read_op.max_freq;
[Severity: High]
When the fallback tuning negotiates a slower read variant, the read_cache
is updated but the cont_read_cache, update_cache, and write_cache are not
downgraded.
Does this result in the uncalibrated high-speed cont_read_cache and
update_cache variants being embedded into the dirmaps using the max_freq
of the downgraded read variant?
Could this lead to I/O failures or data corruption when the controller
executes these uncalibrated high-speed variants?
> }
> desc = spinand_create_rdesc(spinand, &info);
[ ... ]
> +static bool spinand_try_phy_ranked(struct spinand_device *spinand,
> + struct spi_mem *mem, bool odtr,
> + u32 *tried_mask)
> +{
> + const struct spinand_op_variants *variants = spinand->phy_read_variants;
> + const struct spi_mem_op *best;
> + int ret;
> +
> + if (!variants)
> + return false;
> +
> + while ((best = spinand_op_find_best(spinand, variants, odtr,
> + *tried_mask))) {
> + *tried_mask |= BIT(best - variants->ops);
> + spinand->max_read_op = *best;
> + spinand->max_read_op.max_freq = 0;
> + ret = spi_mem_execute_tuning(mem, &spinand->max_read_op,
> + &spinand->max_write_op);
[Severity: High]
The loop zeroes out spinand->max_read_op.max_freq before each iteration,
but it does not reset spinand->max_write_op.max_freq.
If spi_mem_execute_tuning() mutates max_write_op.max_freq before failing,
does this leaked non-zero frequency propagate to subsequent iterations?
> + if (ret && ret != -EOPNOTSUPP)
> + dev_warn(&mem->spi->dev, "%s PHY tuning failed: %d\n",
> + odtr ? "ODTR" : "SSDR", ret);
> + if (!ret && spinand->max_read_op.max_freq) {
> + if (odtr)
> + spinand->odtr_op_templates.read_cache = best;
> + else
> + spinand->ssdr_op_templates.read_cache = best;
> + return true;
> + }
> + }
> + return false;
> +}
[ ... ]
> +static void spinand_configure_phy(struct spinand_device *spinand,
> + struct spi_mem *mem)
> +{
> + u32 tried_mask;
> + int ret;
> +
> + spinand_reset_max_ops(spinand, spinand->op_templates);
> +
> + ret = spi_mem_execute_tuning(mem, &spinand->max_read_op,
> + &spinand->max_write_op);
> + if (ret && ret != -EOPNOTSUPP)
> + dev_warn(&mem->spi->dev, "Failed to execute PHY tuning: %d\n",
> + ret);
> +
> + /*
> + * Any non-zero return or a set max_freq means we are done (error,
> + * unsupported, or success). Fallback only for the op-specific "skip"
> + * signal: ret == 0 with max_freq still 0.
> + */
> + if (ret || spinand->max_read_op.max_freq)
> + return;
[Severity: High]
If the initial tuning call fails and returns early here, does it leave the
potentially mutated max_write_op.max_freq without resetting it?
Could this leaked frequency be embedded into dirmap descriptors and cause
operations to run at unvalidated frequencies?
> +
> + if (!mem->spi->post_config_max_speed_hz || spinand->bus_iface == SSDR ||
> + !spinand->phy_read_variants)
> + return;
[ ... ]
> + /*
> + * Pass 2: switch to SSDR and try all SSDR variants in performance
> + * order.
> + *
> + * Only enter if we actually have SSDR support and a reconfigure
> + * callback. The hardware is still in ODTR mode here so no
> + * configure_chip call is needed to undo; just set up the ODTR non-PHY
> + * fallback and return.
> + */
> + if (!spinand->ssdr_op_templates.read_cache ||
> + !spinand->ssdr_op_templates.write_cache ||
> + !spinand->configure_chip)
> + goto use_odtr_non_phy;
> +
> + if (spinand->configure_chip(spinand, SSDR))
> + goto use_odtr_non_phy;
> +
> + spinand->op_templates = &spinand->ssdr_op_templates;
> + spinand->bus_iface = SSDR;
> + spinand->max_write_op = *spinand->ssdr_op_templates.write_cache;
> + spinand->max_write_op.max_freq = 0;
[Severity: Critical]
When dynamically falling back to SSDR mode, spinand->odtr_op_templates is
not cleared.
When the system resumes from suspend, spinand_configure_chip() uses the
uncleared ODTR templates to determine the mode:
if (spinand->odtr_op_templates.read_cache &&
spinand->odtr_op_templates.write_cache &&
spinand->odtr_op_templates.update_cache)
odtr = true;
Will this cause the flash chip to be incorrectly reconfigured back to ODTR
mode upon resume, while the statically created dirmaps still contain SSDR
commands?
Can this command desynchronization cause I/O failures after the device
resumes?
> +
> + /*
> + * Only ODTR variants were candidates in Pass 1; SSDR bit positions
> + * are clear
> + */
> + if (spinand_try_phy_ranked(spinand, mem, false, &tried_mask))
> + return;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618073725.84733-1-s-k6@ti.com?part=14
^ permalink raw reply
* Re: [PATCH v4 04/16] spi: spi-mem: teach spi_mem_adjust_op_freq() about post-config ops
From: sashiko-bot @ 2026-06-18 8:02 UTC (permalink / raw)
To: Santhosh Kumar K; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260618073725.84733-5-s-k6@ti.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Overloading `op->max_freq` as a magic exact-match token to identify post-configuration operations breaks frequency limit semantics, preventing valid operations from running at tuned speeds or erroneously allowing untuned operations to bypass safe limits.
--
commit c232673d530121d1e383d41f1db73523ed370bfb
Author: Santhosh Kumar K <s-k6@ti.com>
spi: spi-mem: teach spi_mem_adjust_op_freq() about post-config ops
This commit extends spi_mem_adjust_op_freq() to allow post-configuration
operations to run at higher speeds. It bypasses the max_speed_hz cap if the
operation's max_freq exactly matches the post_config_max_speed_hz value.
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index f64eda9bbd9f0..b0b111642a358 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
[ ... ]
> @@ -584,9 +584,18 @@ EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
> * Some chips have per-op frequency limitations and must adapt the maximum
> * speed. This function allows SPI mem drivers to set @op->max_freq to the
> * maximum supported value.
> + *
> + * When @mem->spi->post_config_max_speed_hz is set, ops with @op->max_freq
> + * equal to that value are treated as post-configuration ops (e.g. PHY-tuned)
> + * and are allowed to run at the full post-config rate. All other ops are
> + * capped to @mem->spi->max_speed_hz, the always-reachable base rate.
> */
> void spi_mem_adjust_op_freq(struct spi_mem *mem, struct spi_mem_op *op)
> {
> + if (mem->spi->post_config_max_speed_hz &&
> + op->max_freq == mem->spi->post_config_max_speed_hz)
> + return;
[Severity: High]
Does this overload op->max_freq as a magic exact-match token to identify
post-configuration operations?
If an untuned operation (e.g., a write or alternate read variant defined in a
flash driver with a static hardware limit of 104MHz) happens to exactly match
the configured post_config_max_speed_hz, won't it incorrectly bypass the safe
max_speed_hz cap?
Furthermore, controller drivers like Cadence QuadSPI appear to rely on this
exact same equality check to decide whether to route the incoming operation
through the hardware PHY:
drivers/spi/spi-cadence-quadspi.c:cqspi_use_tuned_phy() {
...
return f_pdata->use_tuned_phy &&
op->max_freq == post_config_max_speed_hz;
}
Could this cause uncalibrated operations to be erroneously executed via the
PHY at high speeds, leading to potential data corruption or hardware lockups?
Would it be safer to use an explicit boolean flag (e.g., op->is_tuned) rather
than hijacking the frequency limit field?
> +
> if (!op->max_freq || op->max_freq > mem->spi->max_speed_hz)
> op->max_freq = mem->spi->max_speed_hz;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618073725.84733-1-s-k6@ti.com?part=4
^ permalink raw reply
* Re: [PATCH v3 4/8] irqchip/qcom-pdc: Differentiate between direct SPI and GPIO as SPI
From: Konrad Dybcio @ 2026-06-18 8:02 UTC (permalink / raw)
To: Maulik Shah, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <20260616-hamoa_pdc_v3-v3-4-4d8e1504ea75@oss.qualcomm.com>
On 6/16/26 11:25 AM, Maulik Shah wrote:
> Before commit 4dc70713dc24 ("irqchip/qcom-pdc: Kill non-wakeup irqdomain")
> there are separate domains for direct SPIs and GPIO used as SPIs. Separate
> domains can be useful in case irqchip want to differentiate both of them.
> Since commit unified both the domains there is no way to differentiate.
>
> In preparation to add the second level interrupt controller support where
> GPIO interrupts get latched at PDC (but not direct SPIs) there is a need to
> differentiate between SPIs and GPIOs as SPIs. Reverting above commit do not
> seem a good option either which leads to waste of resources.
>
> PDC HW have the IRQ_PARAM register telling number of direct SPIs and number
> of GPIOs as SPIs. Further PDC allocates direct SPIs at the beginning and
> all GPIOs as SPIs are allocated at the end. This information can be used in
> driver to differentiate them.
>
> Add the support to read this register and keep this information in
> struct pdc_desc. Later change utilizes same.
>
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
[...]
> + irq_param = pdc_reg_read(pdc->regs->irq_param_reg, 0);
> + pdc->num_spis = FIELD_GET(GENMASK(7, 0), irq_param);
> + pdc->num_gpios = FIELD_GET(GENMASK(15, 8), irq_param);
num_gpios is not used in this series, please either drop it or
use it to limit the index in the following patches
Konrad
^ permalink raw reply
* Re: [PATCH v4 09/16] spi: cadence-quadspi: skip DDR PHY tuning for 2-byte-address ops (i2383)
From: sashiko-bot @ 2026-06-18 8:04 UTC (permalink / raw)
To: Santhosh Kumar K; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260618073725.84733-10-s-k6@ti.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unchecked dereference of `cqspi->ddata` in `cqspi_mem_op_execute_tuning()` leads to a potential kernel oops.
--
commit 40592f2a22efbd29a6272035f538dcdb01d9f776
Author: Santhosh Kumar K <s-k6@ti.com>
spi: cadence-quadspi: skip DDR PHY tuning for 2-byte-address ops (i2383)
This commit introduces a CQSPI_NO_2BYTE_ADDR_PHY_DDR quirk to skip DDR PHY
tuning for 2-byte column addresses. This is done to address erratum i2383 on
AM654, which locks the address phase in PHY DDR mode for such operations.
[0] https://www.ti.com/lit/er/sprz544c/sprz544c.pdf
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 05e1b7a2f3433..c94bb8ad5d1dd 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
[ ... ]
> @@ -3864,7 +3879,7 @@ static const struct cqspi_driver_platdata k2g_qspi = {
>
> static const struct cqspi_driver_platdata am654_ospi = {
> .hwcaps_mask = CQSPI_SUPPORTS_OCTAL | CQSPI_SUPPORTS_QUAD,
> - .quirks = CQSPI_NEEDS_WR_DELAY,
> + .quirks = CQSPI_NEEDS_WR_DELAY | CQSPI_NO_2BYTE_ADDR_PHY_DDR,
> .execute_tuning = cqspi_am654_ospi_execute_tuning,
> };
>
[Severity: Medium]
Looking at the tuning execution path that uses these callbacks, is there a
potential NULL pointer dereference in cqspi_mem_op_execute_tuning()?
drivers/spi/spi-cadence-quadspi.c:cqspi_mem_op_execute_tuning() {
...
struct cqspi_st *cqspi =
spi_controller_get_devdata(mem->spi->controller);
if (!cqspi->ddata->execute_tuning)
return -EOPNOTSUPP;
return cqspi->ddata->execute_tuning(mem, read_op, write_op);
}
The cqspi->ddata pointer is initialized via of_device_get_match_data(), which
can return NULL if the driver is bound via sysfs to a device node without a
matching compatible string.
Other uses of ddata in the driver (like in cqspi_probe or cqspi_setup_flash)
include a NULL check before dereferencing it, proving that ddata == NULL is a
supported and handled state.
Should this tuning wrapper also include a defensive check for ddata == NULL?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618073725.84733-1-s-k6@ti.com?part=9
^ permalink raw reply
* Re: [PATCH v3 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode
From: Konrad Dybcio @ 2026-06-18 8:18 UTC (permalink / raw)
To: Maulik Shah, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <20260616-hamoa_pdc_v3-v3-5-4d8e1504ea75@oss.qualcomm.com>
On 6/16/26 11:25 AM, Maulik Shah wrote:
> All PDC irqchip supports pass through mode in which both Direct SPIs and
> GPIO IRQs (as SPIs) are sent to GIC without latching at PDC.
>
> Newer PDCs (v3.0 onwards) also support additional secondary controller mode
> where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
> still works same as pass through mode without latching at PDC even in
> secondary controller mode.
>
> All the SoCs so far default uses pass through mode with the exception of
> x1e. x1e PDC may be set to secondary controller mode for builds on CRD
> boards whereas it may be set to pass through mode for IoT-EVK boards.
> The mode configuration is done in firmware and initially shipped windows
> firmware did not have SCM interface to read or modify the PDC mode.
> Later only write access is opened up for non secure world.
>
> Using the write access available add changes to modify the PDC mode to
> pass through mode via SCM write. When the write fails (on older firmware)
> assume to work in secondary mode.
>
> In secondary mode set the separate irqchip for the GPIOs to perform
> additional operations only for the GPIO irqs.
>
> Co-developed-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
[...]
> +static int qcom_pdc_gic_secondary_set_type(struct irq_data *d, unsigned int type)
> +{
> + enum pdc_irq_config_bits pdc_type;
> + enum pdc_irq_config_bits old_pdc_type;
> + int ret;
> +
> + switch (type) {
> + case IRQ_TYPE_EDGE_RISING:
> + pdc_type = PDC_EDGE_RISING;
> + break;
> + case IRQ_TYPE_EDGE_FALLING:
> + pdc_type = PDC_EDGE_FALLING;
> + break;
> + case IRQ_TYPE_EDGE_BOTH:
> + pdc_type = PDC_EDGE_DUAL;
> + break;
> + case IRQ_TYPE_LEVEL_HIGH:
> + pdc_type = PDC_LEVEL_HIGH;
> + break;
> + case IRQ_TYPE_LEVEL_LOW:
> + pdc_type = PDC_LEVEL_LOW;
> + break;
> + default:
> + WARN_ON(1);
> + return -EINVAL;
> + }
> +
> + old_pdc_type = pdc_reg_read(pdc->regs->irq_cfg_reg, d->hwirq);
> + pdc_type |= (old_pdc_type & ~pdc->cfg_fields->irq_type);
> + pdc_reg_write(pdc->regs->irq_cfg_reg, d->hwirq, pdc_type);
> +
> + type = IRQ_TYPE_LEVEL_HIGH;
Please carry your comment from the previous revision:
/*
* PDC forwards GPIOs as level high to GIC in secondary
* mode. Update the type and clear any previously latched
* phantom interrupt at PDC.
*/
> + pdc->clear_gpio(d->hwirq);
> +
> + ret = irq_chip_set_type_parent(d, type);
> + if (ret)
> + return ret;
> +
> + /*
> + * When we change types the PDC can give a phantom interrupt.
> + * Clear it. Specifically the phantom shows up when reconfiguring
> + * polarity of interrupt without changing the state of the signal
> + * but let's be consistent and clear it always.
> + *
> + * Doing this works because we have IRQCHIP_SET_TYPE_MASKED so the
> + * interrupt will be cleared before the rest of the system sees it.
> + */
> + if (old_pdc_type != pdc_type)
> + irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, false);
This bit and the switch statement above are common between the two paths..
I'm debating whether we should factor them out as static inline void, but
neither solution is perfect.. so, up to you
[...]
> @@ -385,20 +547,37 @@ static int qcom_pdc_alloc(struct irq_domain *domain, unsigned int virq,
> if (hwirq == GPIO_NO_WAKE_IRQ)
> return irq_domain_disconnect_hierarchy(domain, virq);
>
> - ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> - &qcom_pdc_gic_chip, NULL);
> - if (ret)
> - return ret;
> + /*
> + * PDC secondary chip is only set for the GPIO interrupts as SPIs.
> + * Direct SPI interrupts are still in pass through mode (no latching
> + * at PDC).
> + */
> + if (pdc->mode == PDC_PASS_THROUGH_MODE || !pdc_pin_is_gpio(hwirq)) {
> + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> + &qcom_pdc_gic_chip,
> + NULL);
> + if (ret)
> + return ret;
>
> - region = get_pin_region(hwirq);
> - if (!region)
> - return irq_domain_disconnect_hierarchy(domain->parent, virq);
> + if (type & IRQ_TYPE_EDGE_BOTH)
> + type = IRQ_TYPE_EDGE_RISING;
>
> - if (type & IRQ_TYPE_EDGE_BOTH)
> - type = IRQ_TYPE_EDGE_RISING;
> + if (type & IRQ_TYPE_LEVEL_MASK)
> + type = IRQ_TYPE_LEVEL_HIGH;
> + } else {
> + ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq,
> + &qcom_pdc_gic_secondary_chip,
> + NULL);
> + if (ret)
> + return ret;
>
> - if (type & IRQ_TYPE_LEVEL_MASK)
> + /* Secondary mode converts all interrupts to LEVEL HIGH type */
> type = IRQ_TYPE_LEVEL_HIGH;
> + }
nit: (pdc->mode == PDC_SECONDARY_MODE && pdc_pin_is_gpio(hwirq))
could be the primary case to better communicate intent
Konrad
^ permalink raw reply
* Re: [PATCH v3 5/8] irqchip/qcom-pdc: Configure PDC to pass through mode
From: Konrad Dybcio @ 2026-06-18 8:19 UTC (permalink / raw)
To: Maulik Shah, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <20260616-hamoa_pdc_v3-v3-5-4d8e1504ea75@oss.qualcomm.com>
On 6/16/26 11:25 AM, Maulik Shah wrote:
> All PDC irqchip supports pass through mode in which both Direct SPIs and
> GPIO IRQs (as SPIs) are sent to GIC without latching at PDC.
>
> Newer PDCs (v3.0 onwards) also support additional secondary controller mode
> where PDC latches GPIO IRQs and sends to GIC as level type IRQ. Direct SPIs
> still works same as pass through mode without latching at PDC even in
> secondary controller mode.
>
> All the SoCs so far default uses pass through mode with the exception of
> x1e. x1e PDC may be set to secondary controller mode for builds on CRD
> boards whereas it may be set to pass through mode for IoT-EVK boards.
> The mode configuration is done in firmware and initially shipped windows
> firmware did not have SCM interface to read or modify the PDC mode.
> Later only write access is opened up for non secure world.
>
> Using the write access available add changes to modify the PDC mode to
> pass through mode via SCM write. When the write fails (on older firmware)
> assume to work in secondary mode.
>
> In secondary mode set the separate irqchip for the GPIOs to perform
> additional operations only for the GPIO irqs.
>
> Co-developed-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Sneh Mankad <sneh.mankad@oss.qualcomm.com>
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
[...]
> +static void pdc_clear_gpio_cfg(int pin_out)
> +{
> + unsigned long gpio_sts;
> +
> + if (pdc->version < PDC_VERSION_3_0)
> + return;
> +
> + gpio_sts = pdc_reg_read(pdc->regs->irq_cfg_reg, pin_out);
> + __clear_bit(pdc->cfg_fields->gpio_irq_sts, &gpio_sts);
> + pdc_reg_write(pdc->regs->irq_cfg_reg, pin_out, gpio_sts);
> +}
> +
> +static void pdc_unmask_gpio_cfg(int pin_out, bool unmask)
> +{
> + unsigned long gpio_mask;
> +
> + if (pdc->version < PDC_VERSION_3_0)
> + return;
These version checks should be unnecessary now, given the function pointer
is only assigned for hamoa
Konrad
^ permalink raw reply
* Re: [PATCH v3 1/5] clk: renesas: rzv2h-cpg: Use per-SoC PLL reference frequency for calculations
From: Lad, Prabhakar @ 2026-06-18 8:20 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Michael Turquette, Stephen Boyd, Brian Masney, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, linux-kernel,
linux-renesas-soc, linux-clk, devicetree, Biju Das,
Fabrizio Castro, Lad Prabhakar
In-Reply-To: <CAMuHMdVx_SS0w=x4H3WUb4BdnOXQOxboeFZwmvhP31HLoQNNkw@mail.gmail.com>
Hi Geert,
On Thu, Jun 18, 2026 at 8:18 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Wed, 17 Jun 2026 at 22:17, Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > On Wed, Jun 17, 2026 at 10:48 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Mon, 15 Jun 2026 at 12:48, Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > Introduce a per-SoC PLL reference input frequency parameter to avoid
> > > > relying on a hardcoded 24MHz constant during PLL configuration math.
> > > >
> > > > Add an input_fref member to struct rzv2h_pll_limits. In the core
> > > > calculation helper rzv2h_get_pll_pars(), derive the base input clock
> > > > rate from limits->input_fref, utilizing the conditional ternary operator
> > > > to fall back to 24MHz if the struct field is left uninitialized (0), and
> > > > drop the obsolete macro RZ_V2H_OSC_CLK_IN_MEGA.
> > > >
> > > > This abstraction permits the reuse of the common PLL divider logic on
> > > > newer SoC platforms like the RZ/T2H, which feature a 48 MHz PLL reference
> > > > clock input instead of the 24 MHz signal used by RZ/V2H(P), without
> > > > disrupting existing platforms.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > Thanks for your patch!
> > >
> > > > --- a/include/linux/clk/renesas.h
> > > > +++ b/include/linux/clk/renesas.h
> > > > @@ -53,6 +53,9 @@ static inline void rzg2l_cpg_dsi_div_set_divider(u8 divider, int target) { }
> > > > * various parameters used to configure a PLL. These limits ensure
> > > > * the PLL operates within valid and stable ranges.
> > > > *
> > > > + * @input_fref: Reference input frequency to the PLL (in MHz). If set
> > >
> > > "in Hz", as pointed out by Sashiko.
> > >
> > Agreed.
> >
> > > > + * to 0, a default value of 24MHz is used.
> > > > + *
> > I'll also replace `24MHz` to `24000000 Hz`.
>
> Personally, I find "24 MHz" easier to read.
> Counting zeroes without thousands-grouping can be hard...
> https://lore.kernel.org/CAMuHMdUp3fOkAttk2FKJTh5svaiMwgu4JZ8utPeHghU4TQLE-w@mail.gmail.com
>
Agreed, I will leave it as is.
Cheers,
Prabhakar
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
^ permalink raw reply
* Re: [PATCH v3 8/8] arm64: dts: qcom: x1e80100: Add deepest idle state
From: Konrad Dybcio @ 2026-06-18 8:25 UTC (permalink / raw)
To: Maulik Shah, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Gleixner, Linus Walleij
Cc: linux-arm-msm, linux-kernel, devicetree, linux-gpio, Sneh Mankad
In-Reply-To: <20260616-hamoa_pdc_v3-v3-8-4d8e1504ea75@oss.qualcomm.com>
On 6/16/26 11:25 AM, Maulik Shah wrote:
> Add deepest idle state as GPIO IRQs can work as wakeup capable interrupts
> in deepest idle state.
>
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/hamoa.dtsi | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index 4ba751a65142..47e425003028 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -302,6 +302,14 @@ cluster_cl5: cluster-sleep-1 {
> exit-latency-us = <4000>;
> min-residency-us = <7000>;
> };
> +
> + domain_ss3: domain-sleep-0 {
> + compatible = "domain-idle-state";
> + arm,psci-suspend-param = <0x0200c354>;
> + entry-latency-us = <2800>;
> + exit-latency-us = <4400>;
The DSDT has "wake_latency" (presumably the same as exit latency) set
to 5000 us, should we follow?
FWIW, the 2800/4400 numbers here are the exact same as for sm8650..
which doesn't sound very reassuring
> + min-residency-us = <9000>;
This number matches the DSDT
Should the entry latency then be 9000 - 5000 = 4000?
On a separate note, the DSDT also defines:
SS1 (0x02000154, total=7500, exit=500)
SS2 (0x02000254, total=8000, exit=3000)
These are obviously shallower states, but perhaps they could still
be useful?
Konrad
^ permalink raw reply
* [PATCH v2 0/7] ASoC: codecs: ES8389: Modify the ES8389 driver
From: Zhang Yi @ 2026-06-18 8:28 UTC (permalink / raw)
To: linux-sound, broonie, devicetree; +Cc: tiwai, robh, krzk+dt, conor+dt, Zhang Yi
Modify the initialization configuration and routes for ES8389,
and add private members.
Describe the added private members in `devicetree/bindings`
v2 -> v1:
-Keep the old ABI around mclk_src
-Describe the member related to the HPF frequency in the DTS.
-Instead of accessing HPF-related register values from DTS,
set a specific frequency and then match the corresponding value in the code.
v1 -> v0:
-The binding patch has been placed before the corresponding code patch
-Provide a more detailed description of the binding patch
-Provide a more detailed description of the patch about HPF
Zhang Yi (7):
ASoC: dt-bindings: ES8389: Add members about HPF and clock
ASoC: codecs: ES8389: Modify volatile_register
ASoC: codecs: ES8389: Fix the issue about mclk_src
ASoC: codecs: ES8389: Modify the clock table
ASoC: codecs: ES8389: Modify the initial configuration
ASoC: codecs: ES8389: Add private members about HPF
ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX
.../bindings/sound/everest,es8389.yaml | 16 ++
sound/soc/codecs/es8389.c | 184 ++++++++++++++----
sound/soc/codecs/es8389.h | 12 +-
3 files changed, 174 insertions(+), 38 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v2 2/7] ASoC: codecs: ES8389: Modify volatile_register
From: Zhang Yi @ 2026-06-18 8:28 UTC (permalink / raw)
To: linux-sound, broonie, devicetree; +Cc: tiwai, robh, krzk+dt, conor+dt, Zhang Yi
In-Reply-To: <20260618082842.2778-1-zhangyi@everest-semi.com>
Mark some registers that are not volatile as false
And modified the logic for `cache_bypass` during `8389_resume`.
Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
sound/soc/codecs/es8389.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 449d9574b..be7a36f17 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -50,10 +50,29 @@ static const char * const es8389_core_supplies[] = {
static bool es8389_volatile_register(struct device *dev,
unsigned int reg)
{
- if ((reg <= 0xff))
- return true;
- else
+ switch (reg) {
+ case ES8389_ADCL_VOL:
+ case ES8389_ADCR_VOL:
+ case ES8389_MIC1_GAIN:
+ case ES8389_MIC2_GAIN:
+ case ES8389_DACL_VOL:
+ case ES8389_DACR_VOL:
+ case ES8389_ALC_ON:
+ case ES8389_ALC_CTL:
+ case ES8389_ALC_TARGET:
+ case ES8389_ALC_GAIN:
+ case ES8389_ADC_MUTE:
+ case ES8389_OSR_VOL:
+ case ES8389_DAC_INV:
+ case ES8389_MIX_VOL:
+ case ES8389_DAC_MIX:
+ case ES8389_ADC_RESET:
+ case ES8389_ADC_MODE:
+ case ES8389_DMIC_EN:
return false;
+ default:
+ return true;
+ }
}
static const DECLARE_TLV_DB_SCALE(dac_vol_tlv, -9550, 50, 0);
@@ -861,13 +880,13 @@ static int es8389_resume(struct snd_soc_component *component)
regcache_cache_only(es8389->regmap, false);
regcache_cache_bypass(es8389->regmap, true);
regmap_read(es8389->regmap, ES8389_RESET, ®v);
- regcache_cache_bypass(es8389->regmap, false);
if (regv == 0xff)
es8389_init(component);
else
es8389_set_bias_level(component, SND_SOC_BIAS_ON);
+ regcache_cache_bypass(es8389->regmap, false);
regcache_sync(es8389->regmap);
return 0;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 3/7] ASoC: codecs: ES8389: Fix the issue about mclk_src
From: Zhang Yi @ 2026-06-18 8:28 UTC (permalink / raw)
To: linux-sound, broonie, devicetree; +Cc: tiwai, robh, krzk+dt, conor+dt, Zhang Yi
In-Reply-To: <20260618082842.2778-1-zhangyi@everest-semi.com>
Fix the issue with incorrect modifications to mclk_src
When the system needs to be configured to use the MCLK from the SCLK pin,
the code still sets the relevant registers to use the MCLK from the MCLK pin
Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
sound/soc/codecs/es8389.c | 4 ++--
sound/soc/codecs/es8389.h | 8 +++++---
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index be7a36f17..7c1d61dd2 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -607,9 +607,9 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
regmap_update_bits(es8389->regmap, ES8389_DAC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
- if (es8389->mclk_src == ES8389_SCLK_PIN) {
+ if (es8389->mclk_src) {
regmap_update_bits(es8389->regmap, ES8389_MASTER_CLK,
- ES8389_MCLK_SOURCE, es8389->mclk_src);
+ ES8389_MCLK_MASK, ES8389_MCLK_FROM_SCLK);
es8389->sysclk = params_channels(params) * params_width(params) * params_rate(params);
}
diff --git a/sound/soc/codecs/es8389.h b/sound/soc/codecs/es8389.h
index d21e72f87..57bf7c5f8 100644
--- a/sound/soc/codecs/es8389.h
+++ b/sound/soc/codecs/es8389.h
@@ -116,9 +116,11 @@
#define ES8389_TDM_SLOT (0x70 << 0)
#define ES8389_TDM_SHIFT 4
-#define ES8389_MCLK_SOURCE (1 << 6)
-#define ES8389_MCLK_PIN (1 << 6)
-#define ES8389_SCLK_PIN (0 << 6)
+#define ES8389_MCLK_MASK (3 << 6)
+#define ES8389_MCLK_FROM_SCLK (1 << 6)
+#define ES8389_MCLK_SOURCE ES8389_MCLK_PIN
+#define ES8389_MCLK_PIN 0
+#define ES8389_SCLK_PIN 1
/* ES8389_FMT */
#define ES8389_S24_LE (0 << 5)
--
2.17.1
^ permalink raw reply related
* [PATCH v2 4/7] ASoC: codecs: ES8389: Modify the clock table
From: Zhang Yi @ 2026-06-18 8:28 UTC (permalink / raw)
To: linux-sound, broonie, devicetree; +Cc: tiwai, robh, krzk+dt, conor+dt, Zhang Yi
In-Reply-To: <20260618082842.2778-1-zhangyi@everest-semi.com>
Updated the configuration for certain frequencies
If get_coeff does not find a value that matches the condition, return -EINVAL
Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
sound/soc/codecs/es8389.c | 49 +++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 7c1d61dd2..501b3a16b 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -434,52 +434,54 @@ static const struct _coeff_div coeff_div[] = {
{36, 576000, 16000, 0x00, 0x55, 0x84, 0xD0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x23, 0x8F, 0xBF, 0xC0, 0x1F, 0x8F, 0x01, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{48, 768000, 16000, 0x02, 0x57, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{50, 800000, 16000, 0x00, 0x7E, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
- {64, 1024000, 16000, 0x00, 0x45, 0x24, 0xC0, 0x01, 0xD1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
- {72, 1152000, 16000, 0x00, 0x45, 0x24, 0xC0, 0x01, 0xD1, 0x90, 0x00, 0x00, 0x23, 0x8F, 0xBF, 0xC0, 0x1F, 0x8F, 0x01, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+ {64, 1024000, 16000, 0x00, 0x45, 0x24, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+ {72, 1152000, 16000, 0x00, 0x45, 0x24, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x23, 0x8F, 0xBF, 0xC0, 0x1F, 0x8F, 0x01, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{96, 1536000, 16000, 0x02, 0x55, 0x84, 0xD0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{128, 2048000, 16000, 0x00, 0x51, 0x04, 0xD0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{144, 2304000, 16000, 0x00, 0x51, 0x00, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x23, 0x8F, 0xBF, 0xC0, 0x1F, 0x8F, 0x01, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+ {150, 2400000, 16000, 0x02, 0x7E, 0x01, 0xC9, 0x00, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
{192, 3072000, 16000, 0x02, 0x65, 0x25, 0xE0, 0x00, 0xE1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
- {256, 4096000, 16000, 0x00, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+ {256, 4096000, 16000, 0x00, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{300, 4800000, 16000, 0x02, 0x66, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{384, 6144000, 16000, 0x02, 0x51, 0x04, 0xD0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
- {512, 8192000, 16000, 0x01, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+ {512, 8192000, 16000, 0x01, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{750, 12000000, 16000, 0x0E, 0x7E, 0x01, 0xC9, 0x00, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
- {768, 12288000, 16000, 0x02, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
- {1024, 16384000, 16000, 0x03, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+ {768, 12288000, 16000, 0x02, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+ {1024, 16384000, 16000, 0x03, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{1152, 18432000, 16000, 0x08, 0x51, 0x04, 0xD0, 0x01, 0xC1, 0x90, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{1200, 19200000, 16000, 0x0B, 0x66, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{1500, 24000000, 16000, 0x0E, 0x26, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0xC0, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
- {1536, 24576000, 16000, 0x05, 0x41, 0x04, 0xC0, 0x01, 0xD1, 0x90, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
+ {1536, 24576000, 16000, 0x05, 0x41, 0x04, 0xC0, 0x01, 0xC1, 0x90, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0xFF, 0x7F, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{1625, 26000000, 16000, 0x40, 0x6E, 0x05, 0xC8, 0x01, 0xC2, 0x90, 0xC0, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x12, 0x31, 0x0E, 2, 2},
{800, 19200000, 24000, 0x07, 0x66, 0x01, 0xD9, 0x00, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0xC7, 0x95, 0x00, 0x12, 0x00, 0x1A, 0x49, 0x14, 2, 2},
{375, 12000000, 32000, 0x0E, 0x2E, 0x05, 0xC8, 0x00, 0xC2, 0x80, 0x40, 0x01, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x23, 0x61, 0x1B, 2, 0},
- {600, 19200000, 32000, 0x05, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x23, 0x61, 0x1B, 2, 2},
- {32, 1411200, 44100, 0x00, 0x45, 0xA4, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {600, 19200000, 32000, 0x05, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x23, 0x61, 0x1B, 2, 2},
+ {32, 1411200, 44100, 0x00, 0x45, 0xA4, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
{64, 2822400, 44100, 0x00, 0x51, 0x00, 0xC0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {128, 5644800, 44100, 0x00, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {256, 11289600, 44100, 0x01, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {512, 22579200, 44100, 0x03, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {32, 1536000, 48000, 0x00, 0x45, 0xA4, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {128, 5644800, 44100, 0x00, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {256, 11289600, 44100, 0x01, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {512, 22579200, 44100, 0x03, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {32, 1536000, 48000, 0x00, 0x45, 0xA4, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
{48, 2304000, 48000, 0x02, 0x55, 0x04, 0xC0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
{50, 2400000, 48000, 0x00, 0x76, 0x01, 0xC8, 0x10, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
{64, 3072000, 48000, 0x00, 0x51, 0x04, 0xC0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {100, 4800000, 48000, 0x00, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {100, 4800000, 48000, 0x00, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
{125, 6000000, 48000, 0x04, 0x6E, 0x05, 0xC8, 0x10, 0xC2, 0x80, 0x00, 0x01, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {128, 6144000, 48000, 0x00, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {200, 9600000, 48000, 0x01, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {128, 6144000, 48000, 0x00, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {200, 9600000, 48000, 0x01, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0x00, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
{250, 12000000, 48000, 0x04, 0x76, 0x01, 0xC8, 0x10, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {256, 12288000, 48000, 0x01, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {384, 18432000, 48000, 0x02, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {400, 19200000, 48000, 0x03, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {500, 24000000, 48000, 0x04, 0x46, 0x01, 0xD8, 0x10, 0xD2, 0x80, 0xC0, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
- {512, 24576000, 48000, 0x03, 0x41, 0x04, 0xD0, 0x10, 0xD1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {256, 12288000, 48000, 0x01, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {384, 18432000, 48000, 0x02, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0x40, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {400, 19200000, 48000, 0x03, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0x40, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {500, 24000000, 48000, 0x04, 0x46, 0x01, 0xD8, 0x10, 0xC2, 0x80, 0xC0, 0x00, 0x18, 0x95, 0xD0, 0xC0, 0x63, 0x95, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
+ {512, 24576000, 48000, 0x03, 0x41, 0x04, 0xD0, 0x10, 0xC1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
{800, 38400000, 48000, 0x18, 0x45, 0x04, 0xC0, 0x10, 0xC1, 0x80, 0xC0, 0x00, 0x1F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x00, 0x12, 0x00, 0x35, 0x91, 0x28, 2, 2},
{128, 11289600, 88200, 0x00, 0x50, 0x00, 0xC0, 0x10, 0xC1, 0x80, 0x40, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x32, 0x89, 0x25, 2, 2},
- {64, 6144000, 96000, 0x00, 0x41, 0x00, 0xD0, 0x10, 0xD1, 0x80, 0x00, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x35, 0x91, 0x28, 2, 2},
+ {64, 6144000, 96000, 0x00, 0x41, 0x00, 0xD0, 0x10, 0xC1, 0x80, 0x00, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x35, 0x91, 0x28, 2, 2},
{96, 9216000, 96000, 0x02, 0x43, 0x00, 0xC0, 0x10, 0xC0, 0x80, 0x00, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x35, 0x91, 0x28, 2, 2},
{256, 24576000, 96000, 0x00, 0x40, 0x00, 0xC0, 0x10, 0xC1, 0x80, 0xC0, 0x00, 0x9F, 0x7F, 0xBF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x35, 0x91, 0x28, 2, 2},
{128, 24576000, 192000, 0x00, 0x50, 0x00, 0xC0, 0x18, 0xC1, 0x81, 0xC0, 0x00, 0x8F, 0x7F, 0xBF, 0xC0, 0x3F, 0x7F, 0x80, 0x12, 0xC0, 0x3F, 0xF9, 0x3F, 2, 2},
+ {64, 12288000, 192000, 0x00, 0x41, 0x00, 0xC0, 0x18, 0xC1, 0x80, 0x00, 0x00, 0x8F, 0x7F, 0xEF, 0xC0, 0x7F, 0x7F, 0x80, 0x12, 0xC0, 0x3F, 0xF9, 0x3F, 1, 0},
};
static inline int get_coeff(u8 vddd, u8 dmic, int mclk, int rate)
@@ -657,7 +659,8 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
regmap_write(es8389->regmap, ES8389_SYSTEM18, coeff_div[coeff].Reg0x18);
regmap_write(es8389->regmap, ES8389_SYSTEM19, coeff_div[coeff].Reg0x19);
} else {
- dev_warn(component->dev, "Clock coefficients do not match");
+ dev_err(component->dev, "Clock coefficients do not match");
+ return -EINVAL;
}
return 0;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 5/7] ASoC: codecs: ES8389: Modify the initial configuration
From: Zhang Yi @ 2026-06-18 8:28 UTC (permalink / raw)
To: linux-sound, broonie, devicetree; +Cc: tiwai, robh, krzk+dt, conor+dt, Zhang Yi
In-Reply-To: <20260618082842.2778-1-zhangyi@everest-semi.com>
Modify the initial configuration
Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
sound/soc/codecs/es8389.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 501b3a16b..04b6a48b0 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -793,7 +793,7 @@ static void es8389_init(struct snd_soc_component *component)
regmap_read(es8389->regmap, ES8389_MAX_REGISTER, ®);
es8389->version = reg;
- regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x00);
+ regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x56);
regmap_write(es8389->regmap, ES8389_RESET, 0x7E);
regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x38);
regmap_write(es8389->regmap, ES8389_ADC_HPF1, 0x64);
@@ -845,7 +845,7 @@ static void es8389_init(struct snd_soc_component *component)
regmap_write(es8389->regmap, ES8389_SCLK_DIV, 0x04);
regmap_write(es8389->regmap, ES8389_LRCK_DIV1, 0x01);
regmap_write(es8389->regmap, ES8389_LRCK_DIV2, 0x00);
- regmap_write(es8389->regmap, ES8389_OSC_CLK, 0x00);
+ regmap_write(es8389->regmap, ES8389_OSC_CLK, 0x10);
regmap_write(es8389->regmap, ES8389_ADC_OSR, 0x1F);
regmap_write(es8389->regmap, ES8389_ADC_DSP, 0x7F);
regmap_write(es8389->regmap, ES8389_ADC_MUTE, 0xC0);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 7/7] ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX
From: Zhang Yi @ 2026-06-18 8:28 UTC (permalink / raw)
To: linux-sound, broonie, devicetree; +Cc: tiwai, robh, krzk+dt, conor+dt, Zhang Yi
In-Reply-To: <20260618082842.2778-1-zhangyi@everest-semi.com>
Add INPUTL MUX and INPUTR MUX in route
Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
sound/soc/codecs/es8389.c | 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index c80cc9601..19f4ced77 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -165,6 +165,16 @@ static const struct soc_enum alc_ramprate =
static const struct soc_enum alc_winsize =
SOC_ENUM_SINGLE(ES8389_ALC_CTL, 0, 16, winsize);
+static const char *const es8389_adcl_mux_txt[] = {
+ "Normal",
+ "ADC2 channel to ADC1 channel",
+};
+
+static const char *const es8389_adcr_mux_txt[] = {
+ "Normal",
+ "ADC1 channel to ADC2 channel",
+};
+
static const char *const es8389_outl_mux_txt[] = {
"Normal",
"DAC2 channel to DAC1 channel",
@@ -192,6 +202,20 @@ static const unsigned int es8389_pga_values[] = {
1, 5, 6
};
+static const struct soc_enum es8389_adcl_mux_enum =
+ SOC_ENUM_SINGLE(ES8389_ADC_MODE, 5,
+ ARRAY_SIZE(es8389_adcl_mux_txt), es8389_adcl_mux_txt);
+
+static const struct snd_kcontrol_new es8389_adcl_mux_controls =
+ SOC_DAPM_ENUM("INPUTL MUX", es8389_adcl_mux_enum);
+
+static const struct soc_enum es8389_adcr_mux_enum =
+ SOC_ENUM_SINGLE(ES8389_ADC_MODE, 4,
+ ARRAY_SIZE(es8389_adcr_mux_txt), es8389_adcr_mux_txt);
+
+static const struct snd_kcontrol_new es8389_adcr_mux_controls =
+ SOC_DAPM_ENUM("INPUTR MUX", es8389_adcr_mux_enum);
+
static const struct soc_enum es8389_outl_mux_enum =
SOC_ENUM_SINGLE(ES8389_DAC_MIX, 5,
ARRAY_SIZE(es8389_outl_mux_txt), es8389_outl_mux_txt);
@@ -320,6 +344,8 @@ static const struct snd_soc_dapm_widget es8389_dapm_widgets[] = {
&es8389_adc_mixer_controls[0],
ARRAY_SIZE(es8389_adc_mixer_controls)),
SND_SOC_DAPM_MUX("ADC MUX", SND_SOC_NOPM, 0, 0, &es8389_dmic_mux_controls),
+ SND_SOC_DAPM_MUX("INPUTL MUX", SND_SOC_NOPM, 0, 0, &es8389_adcl_mux_controls),
+ SND_SOC_DAPM_MUX("INPUTR MUX", SND_SOC_NOPM, 0, 0, &es8389_adcr_mux_controls),
SND_SOC_DAPM_MUX("OUTL MUX", SND_SOC_NOPM, 0, 0, &es8389_outl_mux_controls),
SND_SOC_DAPM_MUX("OUTR MUX", SND_SOC_NOPM, 0, 0, &es8389_outr_mux_controls),
@@ -333,10 +359,15 @@ static const struct snd_soc_dapm_route es8389_dapm_routes[] = {
{"ADCL", NULL, "PGAL"},
{"ADCR", NULL, "PGAR"},
+ {"INPUTL MUX", "Normal", "ADCL"},
+ {"INPUTL MUX", "ADC2 channel to ADC1 channel", "ADCR"},
+ {"INPUTR MUX", "Normal", "ADCR"},
+ {"INPUTR MUX", "ADC1 channel to ADC2 channel", "ADCL"},
+
{"ADC Mixer", "DACL ADCL Mixer", "DACL"},
{"ADC Mixer", "DACR ADCR Mixer", "DACR"},
- {"ADC Mixer", NULL, "ADCL"},
- {"ADC Mixer", NULL, "ADCR"},
+ {"ADC Mixer", NULL, "INPUTL MUX"},
+ {"ADC Mixer", NULL, "INPUTR MUX"},
{"ADC MUX", "AMIC", "ADC Mixer"},
{"ADC MUX", "DMIC", "DMIC"},
--
2.17.1
^ permalink raw reply related
* [PATCH v2 1/7] ASoC: dt-bindings: ES8389: Add members about HPF and clock
From: Zhang Yi @ 2026-06-18 8:28 UTC (permalink / raw)
To: linux-sound, broonie, devicetree; +Cc: tiwai, robh, krzk+dt, conor+dt, Zhang Yi
In-Reply-To: <20260618082842.2778-1-zhangyi@everest-semi.com>
Add members related to HPF and mclk_source
Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
.../bindings/sound/everest,es8389.yaml | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/everest,es8389.yaml b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
index 7db84cf11..88fbce7b0 100644
--- a/Documentation/devicetree/bindings/sound/everest,es8389.yaml
+++ b/Documentation/devicetree/bindings/sound/everest,es8389.yaml
@@ -42,6 +42,21 @@ properties:
description:
Interface power supply.
+ everest,mclk-src:
+ $ref: /schemas/types.yaml#/definitions/uint8
+ description:
+ Indicates that SCLK is used as the internal clock.
+ minimum: 0
+ maximum: 0x01
+ default: 0x00
+
+ everest,hpf-frq:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ The frequency of HPF in Hz.
+ maximum: 1020
+ default: 16
+
required:
- compatible
- reg
@@ -62,5 +77,6 @@ examples:
#sound-dai-cells = <0>;
vddd-supply = <&vdd3v3>;
vdda-supply = <&vdd3v3>;
+ everest,hpf-frq = <16>;
};
};
--
2.17.1
^ permalink raw reply related
* [PATCH v2 6/7] ASoC: codecs: ES8389: Add private members about HPF
From: Zhang Yi @ 2026-06-18 8:28 UTC (permalink / raw)
To: linux-sound, broonie, devicetree; +Cc: tiwai, robh, krzk+dt, conor+dt, Zhang Yi
In-Reply-To: <20260618082842.2778-1-zhangyi@everest-semi.com>
Add private members related to HPF.
Add a helper function to match the set HPF frequency.
Access the HPF frequency set by the user from the DTS
Signed-off-by: Zhang Yi <zhangyi@everest-semi.com>
---
sound/soc/codecs/es8389.c | 65 +++++++++++++++++++++++++++++++++++++--
sound/soc/codecs/es8389.h | 4 +++
2 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
index 04b6a48b0..c80cc9601 100644
--- a/sound/soc/codecs/es8389.c
+++ b/sound/soc/codecs/es8389.c
@@ -36,6 +36,9 @@ struct es8389_private {
unsigned int sysclk;
int mastermode;
+ u8 hpfl;
+ u8 hpfr;
+ u32 target_freq;
u8 mclk_src;
u8 vddd;
int version;
@@ -504,6 +507,48 @@ static inline int get_coeff(u8 vddd, u8 dmic, int mclk, int rate)
return -EINVAL;
}
+static const u32 hpf_table[10][10] = {
+ {1020, 754, 624, 559, 527, 511, 502, 498, 497, 496},
+ {754, 495, 368, 306, 274, 259, 251, 247, 246, 244},
+ {624, 368, 243, 182, 151, 136, 128, 124, 123, 121},
+ {559, 306, 182, 120, 90, 75, 68, 63, 62, 60},
+ {527, 274, 151, 90, 60, 45, 38, 33, 32, 31},
+ {511, 259, 136, 75, 45, 30, 23, 19, 18, 17},
+ {502, 251, 128, 68, 38, 23, 16, 13, 11, 11},
+ {498, 247, 124, 63, 33, 19, 13, 10, 8, 8},
+ {497, 246, 123, 62, 32, 18, 11, 8, 8, 0},
+ {496, 244, 121, 60, 31, 17, 11, 8, 0, 0}
+};
+
+static bool find_best_hpf_freq(u32 target_hz, u8 *hpf1, u8 *hpf2)
+{
+ int best_row = -1, best_col = -1;
+ u32 min_diff = U32_MAX;
+ u32 f, diff;
+ int i, j;
+
+ if ((*hpf1 == ES8389_HPF_INVALID) | (*hpf2 == ES8389_HPF_INVALID))
+ return false;
+
+ for (i = 0; i < 10; i++) {
+ for (j = i; j < 10; j++) {
+ f = hpf_table[i][j];
+
+ diff = (target_hz > f) ? (target_hz - f) : (f - target_hz);
+ if (diff < min_diff) {
+ min_diff = diff;
+ best_row = i;
+ best_col = j;
+ }
+ }
+ }
+
+ *hpf1 = best_col + ES8389_HPF_OFFSET;
+ *hpf2 = best_row + ES8389_HPF_OFFSET;
+
+ return true;
+}
+
/*
* if PLL not be used, use internal clk1 for mclk,otherwise, use internal clk2 for PLL source.
*/
@@ -585,6 +630,8 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
int coeff, ret;
u8 dmic_enable, state = 0;
unsigned int regv;
+ u32 freq;
+ bool hpf;
switch (params_format(params)) {
case SNDRV_PCM_FORMAT_S16_LE:
@@ -663,6 +710,14 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
return -EINVAL;
}
+ freq = (es8389->target_freq * 48000) / params_rate(params);
+ hpf = find_best_hpf_freq(freq, &es8389->hpfl, &es8389->hpfr);
+ if (!hpf) {
+ dev_dbg(component->dev, "Can't find best freq. Use the default setting");
+ es8389->hpfl = ES8389_HPF_DEFAULT;
+ es8389->hpfr = ES8389_HPF_DEFAULT;
+ }
+
return 0;
}
@@ -743,8 +798,8 @@ static int es8389_mute(struct snd_soc_dai *dai, int mute, int direction)
regmap_update_bits(es8389->regmap, ES8389_DAC_FORMAT_MUTE,
0x03, 0x00);
} else {
- regmap_update_bits(es8389->regmap, ES8389_ADC_HPF1, 0x0f, 0x0a);
- regmap_update_bits(es8389->regmap, ES8389_ADC_HPF2, 0x0f, 0x0a);
+ regmap_update_bits(es8389->regmap, ES8389_ADC_HPF1, 0x0f, es8389->hpfl);
+ regmap_update_bits(es8389->regmap, ES8389_ADC_HPF2, 0x0f, es8389->hpfr);
regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE,
0x03, 0x00);
}
@@ -906,6 +961,12 @@ static int es8389_probe(struct snd_soc_component *component)
es8389->mclk_src = ES8389_MCLK_SOURCE;
}
+ ret = device_property_read_u32(component->dev, "everest,hpf-frq", &es8389->target_freq);
+ if (ret != 0) {
+ dev_dbg(component->dev, "hpf-freq return %d\n", ret);
+ es8389->target_freq = ES8389_HPF_INVALID;
+ }
+
for (i = 0; i < ARRAY_SIZE(es8389_core_supplies); i++)
es8389->core_supply[i].supply = es8389_core_supplies[i];
ret = devm_regulator_bulk_get(component->dev, ARRAY_SIZE(es8389_core_supplies), es8389->core_supply);
diff --git a/sound/soc/codecs/es8389.h b/sound/soc/codecs/es8389.h
index 57bf7c5f8..995a3eb58 100644
--- a/sound/soc/codecs/es8389.h
+++ b/sound/soc/codecs/es8389.h
@@ -106,6 +106,10 @@
#define ES8389_MIC_SEL_MASK (7 << 4)
#define ES8389_MIC_DEFAULT (1 << 4)
+#define ES8389_HPF_INVALID U32_MAX
+#define ES8389_HPF_DEFAULT (0x0A << 0)
+#define ES8389_HPF_OFFSET 4
+
#define ES8389_MASTER_MODE_EN (1 << 0)
#define ES8389_TDM_OFF (0 << 0)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v4 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss clock controller
From: Philipp Zabel @ 2026-06-18 8:30 UTC (permalink / raw)
To: Joakim Zhang, Conor Dooley
Cc: mturquette@baylibre.com, sboyd@kernel.org, bmasney@redhat.com,
robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
Gary Yang, cix-kernel-upstream, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <SEYPR06MB62262C0F7823337CA9496DE982E32@SEYPR06MB6226.apcprd06.prod.outlook.com>
On Do, 2026-06-18 at 01:43 +0000, Joakim Zhang wrote:
> Hello,
>
>
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Wednesday, June 17, 2026 11:56 PM
> > To: Joakim Zhang <joakim.zhang@cixtech.com>
> > Cc: mturquette@baylibre.com; sboyd@kernel.org; bmasney@redhat.com;
> > robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org;
> > p.zabel@pengutronix.de; Gary Yang <gary.yang@cixtech.com>; cix-kernel-
> > upstream <cix-kernel-upstream@cixtech.com>; linux-clk@vger.kernel.org;
> > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org
> > Subject: Re: [PATCH v4 3/5] dt-bindings: clock: cix,sky1-audss-clock: add audss
> > clock controller
> >
> > On Wed, Jun 17, 2026 at 02:04:35PM +0800, joakim.zhang@cixtech.com wrote:
> > > From: Joakim Zhang <joakim.zhang@cixtech.com>
> > >
> > > The AUDSS CRU contains an internal clock tree of muxes, dividers and
> > > gates for DSP, I2S, HDA, DMAC and related blocks. The clock provider
> > > is a child node of the cix,sky1-audss-system-control syscon and
> > > accesses registers through the parent MMIO region.
> >
> > Why can this not just be part of the parent syscon node?
>
> The clock and reset blocks are handled by different subsystems and maintainers (clk vs reset). Putting the clock provider on the parent syscon node would mean a single driver has to register both the reset controller and the clock provider on one device, which doesn't fit well.
There are many examples of clock and reset drivers sharing the same
node, by using platform_driver for one (usually clk) and
auxiliary_driver for the other (usually reset).
regards
Philipp
^ permalink raw reply
* Re: [PATCH 2/3] irqchip/gic-v3: Add Renesas R-Car Gen4 erratum workaround
From: Marc Zyngier @ 2026-06-18 8:38 UTC (permalink / raw)
To: Marek Vasut
Cc: Marek Vasut, linux-pci, Yoshihiro Shimoda,
Krzysztof Wilczyński, Bjorn Helgaas, Catalin Marinas,
Conor Dooley, Geert Uytterhoeven, Krzysztof Kozlowski,
Lorenzo Pieralisi, Manivannan Sadhasivam, Rob Herring, devicetree,
linux-arm-kernel, linux-doc, linux-kernel, linux-renesas-soc
In-Reply-To: <0935eb67-83d2-49ea-89ab-0d0aa51ead8a@mailbox.org>
On Thu, 18 Jun 2026 03:50:29 +0100,
Marek Vasut <marek.vasut@mailbox.org> wrote:
>
> On 6/17/26 9:24 AM, Marc Zyngier wrote:
>
> Hello Marc,
>
> >> Renesas R-Car S4/V4H/V4M GIC600 integration has address width for AXI
> >> or APB interface configured to 32 bit, it can therefore access only
> >> the first 4 GiB of physical address space. This information comes from
> >> R-Car V4H Interface Specification sheet, there is currently no technical
> >> update number assigned to this limitation. Further input from hardware
> >> engineer indicates that this limitation also applies to R-Car S4 and V4M.
> >> Name the limitation GEN4GICITS1, and add a driver quirk to mitigate this
> >> limitation.
>
> My concern is this ^ , I do not have an erratum number, because there
> isn't one. I am in touch with the hardware engineer and I did get a
> glimpse at internal details of the three SoC, which confirm the
> limitations. Is this sufficient ?
To be honest, this is between you and the SoC vendor. I'll take
whatever symbol you come up with at face value, and will assume that
the vendor agrees with it. After all, they are on Cc and have their
SoB on the patch.
>
> >> Note that the 0x0201743b GIC600 ID is not Renesas-specific, it is
> >> common for many ARM GICv3 implementations. Therefore, add an extra
> >
> > Not quite. It designates GIC600 unambiguously.
>
> What I am trying to communicate is, that the 0x0201743b ID is not ID
> of the Renesas GIC implementation, but it is a generic ARM GIC600
> ID. That is why we cannot match the quirk on the ID (it is generic ARM
> GIC600 ID), and instead we have to match the quirk on the [ ID
> combined with of_machine_is_compatible("renesas,...") ].
This is understood, and is no different from the other broken
platforms in the tree.
>
> > It is just that GIC600
> > is integrated in zillions of SoCs, most of which don't have this
> > problem (the machine I'm typing this from has a GIC600 *and* 96GB of
> > RAM).
>
> Right.
>
> Shall I reword this paragraph somehow to make it clearer ?
I'd simply say that the workaround is keyed on the combination of the
GIC implementation and the platform identification in the device tree.
>
> >> of_machine_is_compatible() check.
> >>
> >> The GIC600 implementation in R-Car S4/V4H/V4M is r1p6.
> >
> > Is this relevant?
>
> I included it for the sake of completeness and to provide all relevant
> information, based on previous discussions about similar limitations
> that I could find on lore.k.o
This information is already contained in the ID you quote (bits
[19:12]), and can be decoded using the public TRM [1].
Thanks,
M.
[1] https://documentation-service.arm.com/static/5e7ddddacbfe76649ba53034
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* Re: [PATCH 1/9] dt-bindings: display: vop2: Add missing reset properties
From: Cristian Ciocaltea @ 2026-06-18 8:39 UTC (permalink / raw)
To: Diederik de Haas, Sandy Huang, Heiko Stübner, Andy Yan,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli
Cc: kernel, Andy Yan, dri-devel, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel
In-Reply-To: <DJC0L3CRJ0WL.IZEYVLPROMM1@cknow-tech.com>
Hi Diederik,
On 6/18/26 10:58 AM, Diederik de Haas wrote:
> Hi Cristian,
>
> Thanks for this series :-) Just 1 nit (at the end) ...
>
> On Wed Jun 17, 2026 at 8:52 PM CEST, Cristian Ciocaltea wrote:
>> Document the VOP2 resets corresponding to the AXI, AHB and DCLK_VP0..2
>> clocks, which are common to all supported SoCs, plus DCLK_VP3 which is
>> provided only on RK3588.
>>
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>> .../bindings/display/rockchip/rockchip-vop2.yaml | 42 ++++++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
>> index 93da1fb9adc4..d3bc5380f910 100644
>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml
[...]
>> @@ -289,6 +321,16 @@ examples:
>> "dclk_vp0",
>> "dclk_vp1",
>> "dclk_vp2";
>> + resets = <&cru SRST_A_VOP>,
>> + <&cru SRST_H_VOP>,
>> + <&cru SRST_VOP0>,
>> + <&cru SRST_VOP1>,
>> + <&cru SRST_VOP2>;
>> + reset-names = "axi",
>> + "ahb",
>> + "dclk_vp0",
>> + "dclk_vp1",
>> + "dclk_vp2";
>> power-domains = <&power RK3568_PD_VO>;
>
> Place reset* props below power-domains (like in patch 9) ?
> So everyone who copies your example has the correct sorting order.
The example doesn't strictly follow that ordering either — see e.g. the iommus
property — so I placed the resets right after the clocks, which keeps the
related properties grouped together.
That said, I don't have a strong preference.
Heiko, is there a convention you'd like the Rockchip bindings to follow here?
Happy to reorder if so.
Regards,
Cristian
>
> Cheers,
> Diederik
>
>> rockchip,grf = <&grf>;
>> iommus = <&vop_mmu>;
>
^ permalink raw reply
* Re: [PATCH v2 6/7] ASoC: codecs: ES8389: Add private members about HPF
From: sashiko-bot @ 2026-06-18 8:39 UTC (permalink / raw)
To: Zhang Yi; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260618082842.2778-7-zhangyi@everest-semi.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The fallback logic for a missing `everest,hpf-frq` DT property is broken, causing an integer overflow and an impossible `u8` to `U32_MAX` comparison.
- [High] The ADC High-Pass Filter parameters `hpfl` and `hpfr` are used without being initialized during probe, leading to incorrect hardware configuration.
- [High] Playback stream operations silently corrupt the ADC (capture) High-Pass Filter state due to missing stream direction checks.
- [Medium] The newly introduced device tree property `everest,hpf-frq` contains a typo ('frq' instead of 'freq').
--
commit 5aa70910dd654fd778ac694838a2c5d06ae7cd64
Author: Zhang Yi <zhangyi@everest-semi.com>
ASoC: codecs: ES8389: Add private members about HPF
This commit adds HPF (High-Pass Filter) related private members to the
ES8389 codec. It includes a helper function to match the set HPF frequency
and accesses the HPF frequency set by the user from the Device Tree.
> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index 09257687bb84c..bcec9d9200dbe 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
[ ... ]
> @@ -504,6 +507,48 @@ static inline int get_coeff(u8 vddd, u8 dmic, int mclk, int rate)
[ ... ]
> +static bool find_best_hpf_freq(u32 target_hz, u8 *hpf1, u8 *hpf2)
> +{
> + int best_row = -1, best_col = -1;
> + u32 min_diff = U32_MAX;
> + u32 f, diff;
> + int i, j;
> +
> + if ((*hpf1 == ES8389_HPF_INVALID) | (*hpf2 == ES8389_HPF_INVALID))
[Severity: High]
Does this code correctly evaluate the fallback condition?
hpf1 and hpf2 are pointers to u8. Dereferencing them yields a maximum value of
255, so comparing them against ES8389_HPF_INVALID (which is U32_MAX) will
always evaluate to false. This prevents the fallback path from ever executing.
> + return false;
> +
> + for (i = 0; i < 10; i++) {
> + for (j = i; j < 10; j++) {
> + f = hpf_table[i][j];
[ ... ]
> @@ -663,6 +710,14 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
> return -EINVAL;
> }
>
> + freq = (es8389->target_freq * 48000) / params_rate(params);
[Severity: High]
If the Device Tree property is missing, target_freq is set to U32_MAX.
Could this cause an integer overflow here, resulting in a garbage freq value?
Additionally, since the High-Pass Filter is an ADC (Capture) specific feature,
does this code safely avoid overwriting the ADC configuration during playback
stream operations?
If es8389_pcm_hw_params() is called for a playback stream, it appears it
will unconditionally recalculate the capture stream's filter configuration
using the playback rate. Could this update logic be restricted by checking
the stream direction?
> + hpf = find_best_hpf_freq(freq, &es8389->hpfl, &es8389->hpfr);
> + if (!hpf) {
> + dev_dbg(component->dev, "Can't find best freq. Use the default setting");
> + es8389->hpfl = ES8389_HPF_DEFAULT;
> + es8389->hpfr = ES8389_HPF_DEFAULT;
> + }
> +
> return 0;
> }
>
> @@ -743,8 +798,8 @@ static int es8389_mute(struct snd_soc_dai *dai, int mute, int direction)
> regmap_update_bits(es8389->regmap, ES8389_DAC_FORMAT_MUTE,
> 0x03, 0x00);
> } else {
> - regmap_update_bits(es8389->regmap, ES8389_ADC_HPF1, 0x0f, 0x0a);
> - regmap_update_bits(es8389->regmap, ES8389_ADC_HPF2, 0x0f, 0x0a);
> + regmap_update_bits(es8389->regmap, ES8389_ADC_HPF1, 0x0f, es8389->hpfl);
> + regmap_update_bits(es8389->regmap, ES8389_ADC_HPF2, 0x0f, es8389->hpfr);
[Severity: High]
Are hpfl and hpfr properly initialized before they are used here?
Since they are dynamically allocated and zero-initialized, if es8389_mute()
is called prior to es8389_pcm_hw_params() (for instance during stream startup
or idle states), this will write uninitialized 0x00 values to the hardware
registers instead of the previously hardcoded 0x0A default.
Should they be explicitly initialized in es8389_probe()?
> regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE,
> 0x03, 0x00);
> }
> @@ -906,6 +961,12 @@ static int es8389_probe(struct snd_soc_component *component)
> es8389->mclk_src = ES8389_MCLK_SOURCE;
> }
>
> + ret = device_property_read_u32(component->dev, "everest,hpf-frq", &es8389->target_freq);
[Severity: Medium]
This isn't a bug, but there's a typo in the newly introduced device tree
property name ("frq" instead of "freq"). Since Device Tree bindings become
permanent ABI, could this be corrected before it becomes baked into the kernel?
> + if (ret != 0) {
> + dev_dbg(component->dev, "hpf-freq return %d\n", ret);
> + es8389->target_freq = ES8389_HPF_INVALID;
> + }
> +
> for (i = 0; i < ARRAY_SIZE(es8389_core_supplies); i++)
> es8389->core_supply[i].supply = es8389_core_supplies[i];
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618082842.2778-1-zhangyi@everest-semi.com?part=6
^ permalink raw reply
* Re: [PATCH v2 7/7] ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX
From: sashiko-bot @ 2026-06-18 8:41 UTC (permalink / raw)
To: Zhang Yi; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260618082842.2778-8-zhangyi@everest-semi.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Logic error in get_coeff incorrectly treats 0 as a wildcard for dmic_sel and dvdd_vol, shadowing specific coefficient configurations.
--
commit 6402e3af14ba28c3424e8a95b28fe5d44fa00f37
Author: Zhang Yi <zhangyi@everest-semi.com>
ASoC: codecs: ES8389: Add INPUTL MUX and INPUTR MUX
This commit adds INPUTL MUX and INPUTR MUX to the DAPM routing paths for
the ES8389 codec.
> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index bcec9d9200dbe..294951d49432c 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
[ ... ]
[Severity: High]
This isn't a bug introduced by this patch, but looking at get_coeff() in
sound/soc/codecs/es8389.c, is there a logic error with how dmic_sel and
dvdd_vol are checked?
sound/soc/codecs/es8389.c:get_coeff() {
...
for (i = 0; i < ARRAY_SIZE(coeff_div); i++) {
if (coeff_div[i].rate == rate && coeff_div[i].mclk == mclk) {
vddd_det = ~(coeff_div[i].dvdd_vol ^ vddd) & 0x01;
dmic_det = ~(coeff_div[i].dmic_sel ^ dmic) & 0x01;
vddd_det |= ~(coeff_div[i].dvdd_vol % 2) & 0x01;
dmic_det |= ~(coeff_div[i].dmic_sel % 2) & 0x01;
if (vddd_det && dmic_det)
return i;
}
}
...
}
Because ~(0 % 2) & 0x01 evaluates to 1, doesn't this calculation cause 0 to
act as a wildcard, evaluating to true regardless of the dmic or vddd
parameters?
If entries with dmic_sel = 0 appear before dmic_sel = 1 in the coeff_div
table, could get_coeff() prematurely return the wildcard match? This appears
to make the correct clock coefficient entries for dmic = 1 dead code,
potentially resulting in incorrect clock configurations.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618082842.2778-1-zhangyi@everest-semi.com?part=7
^ permalink raw reply
* Re: [PATCH v2 5/7] ASoC: codecs: ES8389: Modify the initial configuration
From: sashiko-bot @ 2026-06-18 8:41 UTC (permalink / raw)
To: Zhang Yi; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260618082842.2778-6-zhangyi@everest-semi.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Regulators enabled in component probe are never disabled, leading to a refcount leak on module removal.
- [High] Unbalanced clock prepare/enable calls in bias level handling cause clock refcount underflows.
- [High] The return value of `regmap_read` is ignored, leading to the use of uninitialized stack variables if the I2C read fails.
- [High] If the codec was reset during suspend, es8389_resume re-initializes it but fails to turn on the clocks and bias.
--
commit 15d74449bde5fe0d61c65b106c592477686df171
Author: Zhang Yi <zhangyi@everest-semi.com>
ASoC: codecs: ES8389: Modify the initial configuration
This commit modifies the initial configuration for the ES8389 codec by
updating the values written to ES8389_ISO_CTL and ES8389_OSC_CLK during
initialization.
> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> index dc3410459f39d..09257687bb84c 100644
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
> @@ -793,7 +793,7 @@ static void es8389_init(struct snd_soc_component *component)
>
> regmap_read(es8389->regmap, ES8389_MAX_REGISTER, ®);
> es8389->version = reg;
[Severity: High]
This is a pre-existing issue, but what happens if regmap_read() fails here?
If the underlying I2C transfer fails, regmap_read() returns an error and
leaves reg uninitialized. This uninitialized stack variable is then blindly
assigned to es8389->version.
This same pattern occurs in multiple places, such as es8389_resume():
sound/soc/codecs/es8389.c:es8389_resume() {
...
regmap_read(es8389->regmap, ES8389_RESET, ®v);
if (regv == 0xff)
...
}
and es8389_mute():
sound/soc/codecs/es8389.c:es8389_mute() {
...
regmap_read(es8389->regmap, ES8389_CSM_STATE1, ®v);
if (regv != ES8389_STATE_ON) {
...
}
Should the return value of regmap_read() be checked to prevent using
uninitialized stack variables and causing random device state
misconfigurations?
> - regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x00);
> + regmap_write(es8389->regmap, ES8389_ISO_CTL, 0x56);
[ ... ]
[Severity: High]
This is a pre-existing issue, but does this driver leak regulator references
on module removal?
In es8389_probe(), the core regulators are enabled:
sound/soc/codecs/es8389.c:es8389_probe() {
...
ret = regulator_bulk_enable(ARRAY_SIZE(es8389_core_supplies),
es8389->core_supply);
...
}
However, es8389_remove() fails to call regulator_bulk_disable():
sound/soc/codecs/es8389.c:es8389_remove() {
struct es8389_private *es8389 = snd_soc_component_get_drvdata(component);
regmap_write(es8389->regmap, ES8389_MASTER_MODE, 0x28);
...
}
When the component is unbound, the devres framework will free the regulator
structure while its enable count is unbalanced. Does this cause a regulator
core warning and a permanent resource leak?
[Severity: High]
This is a pre-existing issue, but do we have unbalanced clock prepare/enable
calls in the bias level handling?
es8389_set_bias_level() unconditionally disables mclk in the standby case:
sound/soc/codecs/es8389.c:es8389_set_bias_level() {
...
case SND_SOC_BIAS_STANDBY:
...
clk_disable_unprepare(es8389->mclk);
break;
...
}
Then in es8389_suspend(), the driver explicitly forces a standby transition:
sound/soc/codecs/es8389.c:es8389_suspend() {
...
es8389_set_bias_level(component, SND_SOC_BIAS_STANDBY);
...
}
If the ASoC core had already transitioned the bias to standby prior to
component suspend, does this force a double-disable of the clock and cause a
clock underflow?
[Severity: High]
This is a pre-existing issue, but does es8389_resume() correctly bring the
device out of standby?
If the codec was reset during suspend, it re-initializes the codec via
es8389_init():
sound/soc/codecs/es8389.c:es8389_resume() {
...
if (regv == 0xff)
es8389_init(component);
else
es8389_set_bias_level(component, SND_SOC_BIAS_ON);
...
}
However, es8389_init() only configures registers without transitioning the
bias state or enabling mclk. By skipping the es8389_set_bias_level(...,
SND_SOC_BIAS_ON) call, does the device remain unclocked and in a powered-off
bias state?
If so, a subsequent ASoC state transition to standby might try to unprepare
the already disabled clock.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618082842.2778-1-zhangyi@everest-semi.com?part=5
^ permalink raw reply
* Re: [PATCH v2 3/7] ASoC: codecs: ES8389: Fix the issue about mclk_src
From: sashiko-bot @ 2026-06-18 8:42 UTC (permalink / raw)
To: Zhang Yi; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260618082842.2778-4-zhangyi@everest-semi.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Critical] Modifying device tree property interpretation breaks existing configurations.
Pre-existing issues:
- [High] Incorrect BCLK (sysclk) calculation for mono streams.
--
commit 97d350d02faf890bb7862a3fb657b9cbe4613b33
Author: Zhang Yi <zhangyi@everest-semi.com>
ASoC: codecs: ES8389: Fix the issue about mclk_src
Fix the issue with incorrect modifications to mclk_src.
When the system needs to be configured to use the MCLK from the SCLK pin,
the code still sets the relevant registers to use the MCLK from the MCLK
pin.
> diff --git a/sound/soc/codecs/es8389.h b/sound/soc/codecs/es8389.h
> --- a/sound/soc/codecs/es8389.h
> +++ b/sound/soc/codecs/es8389.h
[ ... ]
> @@ -116,9 +116,11 @@
> #define ES8389_TDM_SLOT (0x70 << 0)
> #define ES8389_TDM_SHIFT 4
>
> -#define ES8389_MCLK_SOURCE (1 << 6)
> -#define ES8389_MCLK_PIN (1 << 6)
> -#define ES8389_SCLK_PIN (0 << 6)
> +#define ES8389_MCLK_MASK (3 << 6)
> +#define ES8389_MCLK_FROM_SCLK (1 << 6)
> +#define ES8389_MCLK_SOURCE ES8389_MCLK_PIN
> +#define ES8389_MCLK_PIN 0
> +#define ES8389_SCLK_PIN 1
[ ... ]
> diff --git a/sound/soc/codecs/es8389.c b/sound/soc/codecs/es8389.c
> --- a/sound/soc/codecs/es8389.c
> +++ b/sound/soc/codecs/es8389.c
[ ... ]
> @@ -607,9 +607,9 @@ static int es8389_pcm_hw_params(struct snd_pcm_substream *substream,
> regmap_update_bits(es8389->regmap, ES8389_ADC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
> regmap_update_bits(es8389->regmap, ES8389_DAC_FORMAT_MUTE, ES8389_DATA_LEN_MASK, state);
>
> - if (es8389->mclk_src == ES8389_SCLK_PIN) {
> + if (es8389->mclk_src) {
[Severity: Critical]
Does changing this condition and redefining the pin macros break existing
device tree configurations?
Previously in es8389.h, ES8389_SCLK_PIN was 0. If an existing device tree
explicitly selected SCLK using <0>, it would match the old condition.
Because ES8389_SCLK_PIN is now redefined as 1, an existing legacy DT passing
<0> will evaluate as false here and incorrectly fall back to MCLK.
Similarly, if a legacy configuration used <64> for MCLK, it would now
evaluate as true and incorrectly select SCLK. Is there a need for
compatibility fallback logic to prevent ABI breakage for deployed platforms?
> regmap_update_bits(es8389->regmap, ES8389_MASTER_CLK,
> - ES8389_MCLK_SOURCE, es8389->mclk_src);
> + ES8389_MCLK_MASK, ES8389_MCLK_FROM_SCLK);
> es8389->sysclk = params_channels(params) * params_width(params) * params_rate(params);
[Severity: High]
This is a pre-existing issue, but does this calculation result in an
incorrect sysclk for mono streams?
By using params_channels() in es8389_pcm_hw_params(), the physical clock
rate is derived from the logical channel count. For a mono stream, this
returns 1. However, on standard I2S buses, the physical frame always
contains 2 slots (left and right), meaning the actual physical bit clock on
the wire should be 2 * width * rate.
Could this cause the sysclk calculation to be halved, making it fail to
find a matching frequency and breaking mono playback?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260618082842.2778-1-zhangyi@everest-semi.com?part=3
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox