* Re: [PATCH v11 1/2] dt-bindings: hwmon: temperature: add support for EMC1812
From: sashiko-bot @ 2026-06-10 15:24 UTC (permalink / raw)
To: Marius Cristea; +Cc: robh, linux-hwmon, devicetree, conor+dt
In-Reply-To: <20260610-hw_mon-emc1812-v11-1-cef809af5c19@microchip.com>
> This is the devicetree schema for Microchip EMC1812/13/14/15/33
> Multichannel Low-Voltage Remote Diode Sensor Family. It also
> updates the MAINTAINERS file to include the new driver.
>
> EMC1812 has one external remote temperature monitoring channel.
> EMC1813 has two external remote temperature monitoring channels.
> EMC1814 has three external remote temperature monitoring channels and
> channels 2 and 3 support anti parallel diode.
> EMC1815 has four external remote temperature monitoring channels and
> channels 1/2 and 3/4 support anti parallel diode.
> EMC1833 has two external remote temperature monitoring channels and
> channels 1 and 2 support anti parallel diode.
> Resistance Error Correction is supported on channels 1/2 and 3/4.
>
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-hw_mon-emc1812-v11-0-cef809af5c19@microchip.com?part=1
^ permalink raw reply
* [PATCH v2] dt-bindings: interrupt-controller: renesas,r9a09g077-icu: Fix reg size in example
From: Geert Uytterhoeven @ 2026-06-10 15:24 UTC (permalink / raw)
To: Cosmin Tanislav, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm
Cc: linux-kernel, devicetree, linux-renesas-soc, Geert Uytterhoeven,
Krzysztof Kozlowski
According to Figure 5.1 ("Unified memory map"), the safety register
block is 64 KiB large, just like the non-safety register block.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
v2:
- Drop Fixes tag,
- Add Acked-by.
---
.../bindings/interrupt-controller/renesas,r9a09g077-icu.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,r9a09g077-icu.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,r9a09g077-icu.yaml
index 78c01d14e765721b..a809bc4f75159c60 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/renesas,r9a09g077-icu.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,r9a09g077-icu.yaml
@@ -165,7 +165,7 @@ examples:
icu: interrupt-controller@802a0000 {
compatible = "renesas,r9a09g077-icu";
reg = <0x802a0000 0x10000>,
- <0x812a0000 0x50>;
+ <0x812a0000 0x10000>;
#interrupt-cells = <2>;
#address-cells = <0>;
interrupt-controller;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH V3 0/8] PCI: imx6: Integrate pwrctrl API and update device trees
From: Manivannan Sadhasivam @ 2026-06-10 15:25 UTC (permalink / raw)
To: Sherry Sun
Cc: Hongxing Zhu (OSS), Sherry Sun (OSS), robh@kernel.org,
krzk+dt@kernel.org, conor+dt@kernel.org, Frank Li,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
lpieralisi@kernel.org, kwilczynski@kernel.org,
bhelgaas@google.com, l.stach@pengutronix.de, imx@lists.linux.dev,
linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <VI0PR04MB12114B321EB4DFD68B7030F76920E2@VI0PR04MB12114.eurprd04.prod.outlook.com>
On Thu, May 21, 2026 at 04:40:35AM +0000, Sherry Sun wrote:
>
> > > -----Original Message-----
> > > From: Sherry Sun (OSS) <sherry.sun@oss.nxp.com>
> > > Sent: Wednesday, May 20, 2026 4:49 PM
> > > To: robh@kernel.org; krzk+dt@kernel.org; conor+dt@kernel.org; Frank Li
> > > <frank.li@nxp.com>; s.hauer@pengutronix.de; kernel@pengutronix.de;
> > > festevam@gmail.com; lpieralisi@kernel.org; kwilczynski@kernel.org;
> > > mani@kernel.org; bhelgaas@google.com; Hongxing Zhu
> > > <hongxing.zhu@nxp.com>; l.stach@pengutronix.de
> > > Cc: imx@lists.linux.dev; linux-pci@vger.kernel.org; linux-arm-
> > > kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> > > kernel@vger.kernel.org; Sherry Sun <sherry.sun@nxp.com>
> > > Subject: [PATCH V3 0/8] PCI: imx6: Integrate pwrctrl API and update
> > > device trees
> > >
> > > From: Sherry Sun <sherry.sun@nxp.com>
> > >
> > > This series integrates the PCI pwrctrl framework into the pci-imx6
> > > driver and updates i.MX EVK board device trees to support it.
> > >
> > > Patches 2-8 update device trees for i.MX EVK boards which maintained
> > > by NXP to move power supply properties from the PCIe controller node
> > > to the Root Port child node, which is required for pwrctrl framework.
> > > Affected boards:
> > > - i.MX6Q/DL SABRESD
> > > - i.MX6SX SDB
> > > - i.MX8MM EVK
> > > - i.MX8MP EVK
> > > - i.MX8MQ EVK
> > > - i.MX8DXL/QM/QXP EVK
> > > - i.MX95 15x15/19x19 EVK
> > >
> > > The driver maintains legacy regulator handling for device trees that
> > > haven't been updated yet. Both old and new device tree structures are
> > supported.
> > >
> > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > Hi Sherry:
> > Since the vpcie3v3aux is used to power up the WAKE#, it is always on in this
> > pwrctrl framework whatever the system is in suspend or not, right?
> >
>
> Hi Richard,
> Currently the new pwrctrl framework doesn't support vpcie3v3aux, it handles all
> regulators with of_regulator_bulk_get_all() and regulator_bulk_enable/disable().
> The vpcie3v3aux now only works with pci-imx6 driver.
>
PWRCTRL_GENERIC driver can handle both vpcie3v3 and vpcie3v3aux, but not
POWER_SEQUENCING_PCIE_M2 driver, as there is no 3.3Vaux defined in M.2 spec.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v11 2/2] hwmon: temperature: add support for EMC1812
From: sashiko-bot @ 2026-06-10 15:26 UTC (permalink / raw)
To: Marius Cristea; +Cc: devicetree, linux-hwmon, robh, conor+dt
In-Reply-To: <20260610-hw_mon-emc1812-v11-2-cef809af5c19@microchip.com>
> This is the hwmon driver for Microchip EMC1812/13/14/15/33
> Multichannel Low-Voltage Remote Diode Sensor Family.
>
> EMC1812 has one external remote temperature monitoring channel.
> EMC1813 has two external remote temperature monitoring channels.
> EMC1814 has three external remote temperature monitoring channels,
> channels 2 and 3 support anti parallel diode.
> EMC1815 has four external remote temperature monitoring channels and
> channels 1/2 and 3/4 support anti parallel diode.
> EMC1833 has two external remote temperature monitoring channels and
> channels 1 and 2 support anti parallel diode.
> Resistance Error Correction is supported on channels 1/2 and 3/4.
>
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260610-hw_mon-emc1812-v11-0-cef809af5c19@microchip.com?part=2
^ permalink raw reply
* Re: [PATCH net-next v4 03/13] net: ethernet: oa_tc6: add OA_TC6_BROKEN_PHY quirk flag
From: sashiko-bot @ 2026-06-10 15:28 UTC (permalink / raw)
To: Ciprian Regus via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260609-adin1140-driver-v4-3-0753e28ee004@analog.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Heap buffer overflow in SPI TX/RX buffer preparation due to unvalidated chunk counts.
- [High] Unbounded `skb_put` calls in `oa_tc6_update_rx_skb` can cause a kernel panic due to missing frame length limits.
- [High] NULL pointer dereference in `oa_tc6_update_rx_skb` when receiving out-of-order chunks.
- [High] Use-After-Free of kthread task_struct due to premature exit on SPI error.
- [High] Use-After-Free in `lan865x_multicast_work_handler` due to incorrect driver teardown sequence.
- [High] Lost wakeup and device deadlock due to TOCTOU race on `tc6->int_flag`.
--
commit 3db563b67a9fa59e1785134f46c9effcdf6cb05c
Author: Ciprian Regus <ciprian.regus@analog.com>
net: ethernet: oa_tc6: add OA_TC6_BROKEN_PHY quirk flag
Some MAC-PHY devices need custom MDIO bus access functions to work
around hardware issues. Add the OA_TC6_BROKEN_PHY quirk flag so drivers
can opt in to skip oa_tc6's internal PHY init and manage the PHY
themselves. When the flag is set, oa_tc6 skips MDIO bus registration,
PHY discovery and PHY connection, leaving these to the driver.
> diff --git a/drivers/net/ethernet/microchip/lan865x/lan865x.c b/drivers/net/ethernet/microchip/lan865x/lan865x.c
> index 0277d9737369a..26a2761332a5a 100644
> --- a/drivers/net/ethernet/microchip/lan865x/lan865x.c
> +++ b/drivers/net/ethernet/microchip/lan865x/lan865x.c
> @@ -346,7 +346,7 @@ static int lan865x_probe(struct spi_device *spi)
> spi_set_drvdata(spi, priv);
> INIT_WORK(&priv->multicast_work, lan865x_multicast_work_handler);
[Severity: High]
This isn't a bug introduced by this patch, but does the teardown sequence
in lan865x_remove() leave a window for a use-after-free?
In lan865x_remove():
cancel_work_sync(&priv->multicast_work);
unregister_netdev(priv->netdev);
If a concurrent userspace process calls ndo_set_rx_mode via the RTNL lock
after the work is cancelled but before the netdev is unregistered, could
it reschedule the multicast work? If the remove function then proceeds to
free priv, wouldn't the erroneously rescheduled work access the freed priv
pointer when it later executes?
> - priv->tc6 = oa_tc6_init(spi, netdev);
> + priv->tc6 = oa_tc6_init(spi, netdev, NULL);
> if (!priv->tc6) {
> ret = -ENODEV;
> goto free_netdev;
[ ... ]
> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index baba5aad84dfa..2a72f0c4b0095 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
> @@ -134,6 +134,7 @@ struct oa_tc6 {
> bool rx_buf_overflow;
> bool int_flag;
[Severity: High]
This isn't a bug introduced by this patch, but does the handling of
tc6->int_flag have a race condition that could lose wakeups?
In oa_tc6_try_spi_transfer(), the flag is checked and cleared non-atomically:
if (tc6->int_flag) {
tc6->int_flag = false;
If the hardware asserts the interrupt exactly between the check and the clear,
and the ISR sets tc6->int_flag = true, wouldn't the kthread immediately
overwrite it with false? Could this cause the kthread to miss the event flag
and hang indefinitely?
[Severity: High]
This isn't a bug introduced by this patch, but can an uninterrupted stream of
ongoing chunks with DATA_VALID=1 and END_VALID=0 continuously append up to 64
bytes to tc6->rx_skb without bounds checking?
Looking at oa_tc6_prcs_ongoing_rx_frame(), it calls:
oa_tc6_update_rx_skb(tc6, payload, OA_TC6_CHUNK_PAYLOAD_SIZE);
This unconditionally calls skb_put():
memcpy(skb_put(tc6->rx_skb, length), payload, length);
If a device sends an endless stream of ongoing chunks, could this exhaust
the SKB tailroom and cause the core network stack to trigger a BUG() via
skb_over_panic()?
[Severity: High]
This isn't a bug introduced by this patch, but what happens if the MAC-PHY
sends an ongoing chunk (DATA_VALID=1, START_VALID=0, END_VALID=0) without a
preceding start chunk?
In oa_tc6_prcs_rx_chunk_payload():
/* Process the chunk with ongoing rx frame data */
oa_tc6_prcs_ongoing_rx_frame(tc6, data, footer);
If tc6->rx_skb is NULL because no start chunk was processed, wouldn't
oa_tc6_update_rx_skb() dereference a NULL pointer when calling skb_put(),
leading to an immediate crash?
> bool prot_ctrl;
> + enum oa_tc6_quirk_flag quirk_flags;
> };
[ ... ]
> @@ -1279,11 +1286,13 @@ static int oa_tc6_check_ctrl_protection(struct oa_tc6 *tc6)
> * oa_tc6_init - allocates and initializes oa_tc6 structure.
> * @spi: device with which data will be exchanged.
> * @netdev: network device interface structure.
> + * @quirks: device specific modifiers for the OA TC6 protocol.
> *
> * Return: pointer reference to the oa_tc6 structure if the MAC-PHY
> * initialization is successful otherwise NULL.
> */
> -struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev)
> +struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev,
> + struct oa_tc6_quirks *quirks)
> {
[Severity: Critical]
This isn't a bug introduced by this patch, but is it possible for unvalidated
hardware registers to cause out-of-bounds writes in the SPI data buffers?
In oa_tc6_update_buffer_status_from_register():
tc6->tx_credits = FIELD_GET(OA_TC6_BUFFER_STATUS_TX_CREDITS_AVAILABLE,
value);
tc6->rx_chunks_available = FIELD_GET(OA_TC6_BUFFER_STATUS_RX_CHUNKS_AVAILABLE,
value);
These are read directly from the hardware as 8-bit values (up to 255), but
the SPI data buffers (spi_data_tx_buf and spi_data_rx_buf) are allocated for
a maximum of 48 chunks (OA_TC6_MAX_TX_CHUNKS).
Later, in oa_tc6_prepare_spi_tx_buf_for_rx_chunks():
needed_empty_chunks = tc6->rx_chunks_available - tx_chunks;
oa_tc6_add_empty_chunks_to_spi_buf(tc6, needed_empty_chunks);
If a device reports unusually high values, wouldn't these loops write past
the end of the ~14KB buffer allocations?
[Severity: High]
This isn't a bug introduced by this patch, but could an early return from
oa_tc6_spi_thread_handler() lead to a use-after-free during teardown?
In oa_tc6_spi_thread_handler():
ret = oa_tc6_try_spi_transfer(tc6);
if (ret)
return ret;
If an SPI error occurs, the kthread exits and its task_struct is freed.
Later, during device teardown, oa_tc6_exit() unconditionally calls:
kthread_stop(tc6->spi_thread);
If the thread already exited, wouldn't calling kthread_stop() on the freed
task_struct cause a kernel crash?
> struct oa_tc6 *tc6;
> int ret;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-adin1140-driver-v4-0-0753e28ee004@analog.com?part=3
^ permalink raw reply
* Re: [PATCH net-next v4 02/13] net: ethernet: oa_tc6: Handle the OA TC6 SPI protected mode
From: sashiko-bot @ 2026-06-10 15:28 UTC (permalink / raw)
To: Ciprian Regus via B4 Relay; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260609-adin1140-driver-v4-2-0753e28ee004@analog.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The `size` parameter in `oa_tc6_check_ctrl_write_reply` is typed as `u8`, but it receives a calculated `u16` SPI transaction size from `oa_tc6_perform_ctrl`. This size can be up to 1032 bytes in protected mode or 520 bytes in unprotected mode. The truncation to `u8` causes integer underflow in the `memcmp` length calculation `size - OA_TC6_CTRL_IGNORED_SIZE` (where ignored size is 4). For example, a size of 256 (which occurs when writing exactly 31 registers in protected mode or 62 in unprotected mode) is truncated to 0, causing `0 - 4 = -4`. This negative value is cast to a massive `size_t` value, resulting in a severe out-of-bounds memory read.
--
commit f635b76e9df199f1fc90b685521cd415a107c2a3
Author: Ciprian Regus <ciprian.regus@analog.com>
net: ethernet: oa_tc6: Handle the OA TC6 SPI protected mode
This commit implements the OA TC6 standard protected mode for control
transactions. It adds support for including 1's complement values of the
data field in SPI transfer frames, which acts as an integrity check.
> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
[ ... ]
> @@ -283,10 +320,10 @@ static int oa_tc6_perform_ctrl(struct oa_tc6 *tc6, u32 address, u32 value[],
> u16 size;
> int ret;
>
> - /* Prepare control command and copy to SPI control buffer */
> - oa_tc6_prepare_ctrl_spi_buf(tc6, address, value, length, reg_op);
> + size = oa_tc6_calculate_ctrl_buf_size(length, tc6->prot_ctrl);
[ ... ]
> /* Check echoed/received control write command reply for errors */
> if (reg_op == OA_TC6_CTRL_REG_WRITE)
> return oa_tc6_check_ctrl_write_reply(tc6, size);
[Severity: High]
This isn't a bug introduced by this patch, but could this lead to an out of
bounds memory access when the size gets truncated?
Looking at oa_tc6_check_ctrl_write_reply(), the size parameter is typed
as u8:
static int oa_tc6_check_ctrl_write_reply(struct oa_tc6 *tc6, u8 size)
{
...
if (memcmp(tx_buf, rx_buf, size - OA_TC6_CTRL_IGNORED_SIZE))
...
}
Since oa_tc6_perform_ctrl() calculates a u16 size (which can be up to 1032
bytes in protected mode or 520 bytes in unprotected mode), passing it to a
function expecting a u8 will truncate the value.
If the truncated size is less than OA_TC6_CTRL_IGNORED_SIZE (which is 4),
for example if the original size is exactly 256, wouldn't the subtraction
underflow?
This would pass a massive size_t value to memcmp() and cause an out of
bounds read on the spi_ctrl_tx_buf and spi_ctrl_rx_buf buffers.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-adin1140-driver-v4-0-0753e28ee004@analog.com?part=2
^ permalink raw reply
* Re: [PATCH net-next v4 04/13] net: ethernet: oa_tc6: Export the C45 access functions
From: sashiko-bot @ 2026-06-10 15:28 UTC (permalink / raw)
To: Ciprian Regus via B4 Relay; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260609-adin1140-driver-v4-4-0753e28ee004@analog.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] Missing forward declaration of `struct mii_bus` in `include/linux/oa_tc6.h`.
Pre-existing issues:
- [High] A Time-Of-Check to Time-Of-Use race in `oa_tc6_start_xmit` can result in a permanently stopped TX queue.
--
commit 8341f219f15c736a6e2b1b018d43e766db074905
Author: Ciprian Regus <ciprian.regus@analog.com>
net: ethernet: oa_tc6: Export the C45 access functions
The C45 access functions can still be used by some Ethernet drivers
which set the OA_TC6_BROKEN_PHY flag. Export them.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Ciprian Regus <ciprian.regus@analog.com>
> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index 2a72f0c4b0095..b37e398e30e37 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
[ ... ]
[Severity: High]
This is a pre-existing issue, but is there a race condition in
oa_tc6_start_xmit() that can result in a permanently stopped TX queue?
In drivers/net/ethernet/oa_tc6.c:oa_tc6_start_xmit(), tc6->waiting_tx_skb
is checked without holding tx_skb_lock:
if (tc6->waiting_tx_skb) {
netif_stop_queue(tc6->netdev);
return NETDEV_TX_BUSY;
}
Concurrently, the SPI thread can acquire the lock, move waiting_tx_skb
to ongoing_tx_skb, and set waiting_tx_skb to NULL.
After processing, the SPI thread checks if the queue is stopped to wake
it up in oa_tc6_try_spi_transfer():
if (!tc6->waiting_tx_skb && netif_queue_stopped(tc6->netdev))
netif_wake_queue(tc6->netdev);
If the SPI thread evaluates netif_queue_stopped() before oa_tc6_start_xmit()
executes netif_stop_queue(), it won't wake it. oa_tc6_start_xmit() will
then stop the queue and return NETDEV_TX_BUSY.
With waiting_tx_skb being NULL and the queue stopped, will the SPI thread
fail to wake up to process new TX frames, stalling the interface?
> diff --git a/include/linux/oa_tc6.h b/include/linux/oa_tc6.h
> index 62e3d89f80ed0..2660eefa35043 100644
> --- a/include/linux/oa_tc6.h
> +++ b/include/linux/oa_tc6.h
> @@ -31,3 +31,7 @@ int oa_tc6_read_registers(struct oa_tc6 *tc6, u32 address, u32 value[],
> u8 length);
> netdev_tx_t oa_tc6_start_xmit(struct oa_tc6 *tc6, struct sk_buff *skb);
> int oa_tc6_zero_align_receive_frame_enable(struct oa_tc6 *tc6);
> +int oa_tc6_mdiobus_read_c45(struct mii_bus *bus, int addr, int devnum,
> + int regnum);
> +int oa_tc6_mdiobus_write_c45(struct mii_bus *bus, int addr, int devnum,
> + int regnum, u16 val);
[Severity: Low]
Could this cause a compiler warning due to a missing forward declaration
of struct mii_bus?
Including this header without prior MDIO/PHY declarations being in scope
will generate a warning about the struct being declared inside a parameter
list.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-adin1140-driver-v4-0-0753e28ee004@analog.com?part=4
^ permalink raw reply
* Re: [PATCH net-next v4 08/13] net: ethernet: oa_tc6: Use the read_mms/write_mms functions for C45
From: sashiko-bot @ 2026-06-10 15:28 UTC (permalink / raw)
To: Ciprian Regus via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260609-adin1140-driver-v4-8-0753e28ee004@analog.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The MDIO bus read callback incorrectly transforms negative I/O error codes into a successful read of register value `1`, masking SPI communication or device errors.
--
commit 1a0bc4b5b89b86918a4916cb260bddd03f3d58c8
Author: Ciprian Regus <ciprian.regus@analog.com>
net: ethernet: oa_tc6: Use the read_mms/write_mms functions for C45
This commit replaces the formatting of register addresses in C45 reads and
writes with the new oa_tc6_read_register_mms() and
oa_tc6_write_register_mms() functions. It also renames the ret variable to
mms for storing the memory map returned by oa_tc6_get_phy_c45_mms().
> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index d32a1a323d488..4fe4e41136c17 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
> @@ -500,13 +500,14 @@ int oa_tc6_mdiobus_read_c45(struct mii_bus *bus, int addr, int devnum,
> {
> struct oa_tc6 *tc6 = bus->priv;
> u32 regval;
> + int mms;
> int ret;
>
> - ret = oa_tc6_get_phy_c45_mms(devnum);
> - if (ret < 0)
> - return ret;
> + mms = oa_tc6_get_phy_c45_mms(devnum);
> + if (mms < 0)
> + return mms;
>
> - ret = oa_tc6_read_register(tc6, (ret << 16) | regnum, ®val);
> + ret = oa_tc6_read_register_mms(tc6, mms, regnum, ®val);
> if (ret)
> return ret;
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the error
handling for MDIO reads here, I noticed an issue in the related standard read
function.
Does oa_tc6_mdiobus_read() incorrectly handle negative error codes?
In drivers/net/ethernet/oa_tc6.c:oa_tc6_mdiobus_read():
bool ret;
ret = oa_tc6_read_register(tc6, OA_TC6_PHY_STD_REG_ADDR_BASE |
(regnum & OA_TC6_PHY_STD_REG_ADDR_MASK),
®val);
if (ret)
return ret;
When oa_tc6_read_register() fails and returns a negative error code (like
-ENODEV or -EIO), the bool cast converts it to 1 (true).
Returning 1 instead of the negative error code causes the PHY framework to
interpret the I/O error as a successful read of the value 1.
Could this lead to silent failures, incorrect PHY ID detection, or
misconfiguration of the MAC-PHY device?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-adin1140-driver-v4-0-0753e28ee004@analog.com?part=8
^ permalink raw reply
* Re: [PATCH net-next v4 06/13] net: ethernet: oa_tc6: Add the OA_TC6_ prefix to standard registers
From: sashiko-bot @ 2026-06-10 15:28 UTC (permalink / raw)
To: Ciprian Regus via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260609-adin1140-driver-v4-6-0753e28ee004@analog.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing SKB check when receiving an end-valid or ongoing chunk without a prior start-valid chunk causes a NULL pointer dereference.
- [High] Unbounded SKB tail extension allows a malicious or faulty MAC-PHY to trigger `skb_over_panic()`.
--
commit f42a7b7b1c5ba6bcf876685d2831cb4138c5ac3a
Author: Ciprian Regus <ciprian.regus@analog.com>
net: ethernet: oa_tc6: Add the OA_TC6_ prefix to standard registers
The OA TC6 standard registers are currently exported in a header file.
Add the OA_TC6_ prefix to the register address and subfield mask macros
to avoid future naming conflicts.
> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index 97df38207827d..92da5bb74cc76 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
[ ... ]
> @@ -687,25 +687,25 @@ static int oa_tc6_process_extended_status(struct oa_tc6 *tc6)
> return ret;
> }
>
> - if (FIELD_GET(STATUS0_RX_BUFFER_OVERFLOW_ERROR, value)) {
> + if (FIELD_GET(OA_TC6_STATUS0_RX_BUFFER_OVERFLOW_ERROR, value)) {
> tc6->rx_buf_overflow = true;
> oa_tc6_cleanup_ongoing_rx_skb(tc6);
> net_err_ratelimited("%s: Receive buffer overflow error\n",
> tc6->netdev->name);
> return -EAGAIN;
> }
[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the driver's
handling of rx_skb, I noticed a potential NULL pointer dereference in
oa_tc6_prcs_rx_chunk_payload().
If the function processes a chunk with end_valid set but start_valid clear:
drivers/net/ethernet/oa_tc6.c:oa_tc6_prcs_rx_chunk_payload() {
...
/* Process the chunk with only rx frame end */
if (end_valid && !start_valid) {
size = end_byte_offset + 1;
oa_tc6_prcs_rx_frame_end(tc6, data, size);
return 0;
}
...
}
Both this branch and the fallback to oa_tc6_prcs_ongoing_rx_frame()
eventually call oa_tc6_update_rx_skb(), which executes:
memcpy(skb_put(tc6->rx_skb, length), payload, length);
If tc6->rx_skb is NULL (which happens before any start_valid chunk is
processed), will skb_put() dereference a NULL pointer and cause a panic?
[Severity: High]
This is also a pre-existing issue, but I noticed another concern with
skb_put() in oa_tc6_prcs_ongoing_rx_frame().
When chunks arrive with both start_valid and end_valid cleared,
oa_tc6_prcs_rx_chunk_payload() calls oa_tc6_prcs_ongoing_rx_frame(), which
appends data without any bounds checking on the total accumulated length:
drivers/net/ethernet/oa_tc6.c:oa_tc6_prcs_ongoing_rx_frame() {
...
oa_tc6_update_rx_skb(tc6, payload, OA_TC6_CHUNK_PAYLOAD_SIZE);
...
}
If a faulty MAC-PHY or compromised bus sends an unbounded sequence of ongoing
chunks without an end_valid signal, does this exhaust the SKB's tailroom and
trigger an skb_over_panic()?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-adin1140-driver-v4-0-0753e28ee004@analog.com?part=6
^ permalink raw reply
* Re: [PATCH net-next v4 05/13] net: ethernet: oa_tc6: Export standard defined registers
From: sashiko-bot @ 2026-06-10 15:28 UTC (permalink / raw)
To: Ciprian Regus via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260609-adin1140-driver-v4-5-0753e28ee004@analog.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing bounds check before extending the receive SKB allows a malformed or corrupted hardware response to crash the kernel via `skb_over_panic()` or NULL pointer dereference.
--
commit 0ab8a76f7ac8aad2293bc78260fe46bae8c49cec
Author: Ciprian Regus <ciprian.regus@analog.com>
net: ethernet: oa_tc6: Export standard defined registers
This commit moves standard Open Alliance TC6 register addresses and subfields
into the public oa_tc6 header so other ethernet drivers can use them directly.
> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index b37e398e30e37..97df38207827d 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
[Severity: High]
This is a pre-existing issue, but while reviewing the changes in this file,
I noticed a potential issue with SKB bounds checking in the receive path.
In oa_tc6_update_rx_skb(), the code unconditionally extends the SKB tail:
static void oa_tc6_update_rx_skb(struct oa_tc6 *tc6, u8 *payload, u8 length)
{
memcpy(skb_put(tc6->rx_skb, length), payload, length);
}
This is called repeatedly for continuous chunks lacking an end_valid
marker via oa_tc6_prcs_ongoing_rx_frame().
If the MAC-PHY misbehaves or the SPI data is corrupted such that the
end_valid bit is missing for consecutive chunks exceeding the MTU size,
will this continuously append data to the SKB and eventually exhaust the
tailroom, triggering skb_over_panic()?
Additionally, if an ongoing chunk is received before a start chunk
(e.g., start_valid=0), could tc6->rx_skb be NULL here, leading to an
immediate NULL pointer dereference in skb_put()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-adin1140-driver-v4-0-0753e28ee004@analog.com?part=5
^ permalink raw reply
* Re: [PATCH net-next v4 13/13] net: ethernet: adi: Add a driver for the ADIN1140 MACPHY
From: sashiko-bot @ 2026-06-10 15:28 UTC (permalink / raw)
To: Ciprian Regus via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260609-adin1140-driver-v4-13-0753e28ee004@analog.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Data race and potential use-after-free when iterating MAC address lists in workqueue.
- [High] Hardware statistics are overwritten rather than accumulated, causing data loss and stats reset on SPI errors.
- [High] Deadlock risk between process context and softirq due to unsafe spinlock usage.
- [High] Secondary unicast MAC addresses are silently dropped when `IFF_ALLMULTI` is active.
- [Medium] Clearing a MAC address filter creates an active forwarding rule for the 00:00:00:00:00:00 address.
- [Medium] The custom MDIO C22 write function returns an error code instead of acting as a graceful no-op.
--
commit a8bc37d287ad78517cf1f710ad53f270ab363716
Author: Ciprian Regus <ciprian.regus@analog.com>
net: ethernet: adi: Add a driver for the ADIN1140 MACPHY
Add a driver for ADIN1140. The device is a 10BASE-T1S MAC-PHY
that connects to a CPU over an SPI bus, and implements the Open
Alliance TC6 protocol for control and frame transfers.
> diff --git a/drivers/net/ethernet/adi/adin1140.c b/drivers/net/ethernet/adi/adin1140.c
> new file mode 100644
> index 0000000000000..306e5de6faa54
> --- /dev/null
> +++ b/drivers/net/ethernet/adi/adin1140.c
[ ... ]
> +static int adin1140_mac_filter_set(struct adin1140_priv *priv,
> + const u8 *addr, const u8 *mask,
> + u8 slot)
> +{
> + u32 reg_address;
> + u32 val;
> + int ret;
> +
> + if (slot >= ADIN1140_MAC_FILT_MAX_SLOT)
> + return -ENOSPC;
> +
> + reg_address = ADIN1140_MAC_ADDR_FILT_UPR_REG + 2 * slot;
> +
> + ret = oa_tc6_write_register_mms(priv->tc6, OA_TC6_MAC_MMS1,
> + reg_address,
> + get_unaligned_be16(&addr[0]) |
> + ADIN1140_MAC_ADDR_FILT_APPLY2PORT1 |
> + ADIN1140_MAC_ADDR_FILT_TO_HOST);
[Severity: Medium]
When clearing a MAC address filter, does this unconditionally force the filter
slot to an enabled state?
In adin1140_mac_filter_clear(), the driver attempts to disable a filter slot
by programming it with an all-zeros MAC address and an all-ones mask. However,
calling this function unconditionally sets the
ADIN1140_MAC_ADDR_FILT_APPLY2PORT1 and ADIN1140_MAC_ADDR_FILT_TO_HOST bits.
Instead of safely disabling the filter slot, does this activate an explicit
forwarding rule that forces the MAC to send any frames addressed to
00:00:00:00:00:00 to the host? Unused filter slots should likely have their
enable bits cleared rather than being reprogrammed with active dummy rules.
[ ... ]
> +static void adin1140_rx_mode_work(struct work_struct *work)
> +{
[ ... ]
> + mac_addrs = netdev_uc_count(priv->netdev) +
> + netdev_mc_count(priv->netdev);
> +
> + if (priv->netdev->flags & IFF_PROMISC) {
> + promisc = true;
> + all_multi = false;
> + } else if (priv->netdev->flags & IFF_ALLMULTI) {
> + promisc = false;
> + all_multi = true;
> + } else if (mac_addrs <= ADIN1140_MAC_FILT_AVAIL) {
[Severity: High]
Are secondary unicast MAC addresses silently dropped when IFF_ALLMULTI is
active?
This exclusive else-if chain skips the block responsible for programming both
the unicast (uc) and specific multicast (mc) address lists when IFF_ALLMULTI
is enabled. Since IFF_ALLMULTI does not place the hardware in full promiscuous
mode (which would implicitly accept all unicast traffic), it appears all newly
added secondary unicast addresses will fail to be programmed. Unicast address
programming should probably not be mutually exclusive with the IFF_ALLMULTI
toggle.
> + promisc = false;
> + all_multi = false;
> +
> + slot = ADIN1140_MAC_FILT_UC_SLOT + 1;
> + memset(mask, 0xFF, ETH_ALEN);
> +
> + netdev_for_each_uc_addr(ha, priv->netdev) {
[Severity: High]
Does iterating the network device's address lists in a workqueue without
holding netif_addr_lock_bh() risk a data race and potential use-after-free?
Because this runs asynchronously in a process context, it can race against
concurrent modifications to the lists (e.g., dev_uc_add or dev_mc_add invoked
via socket options or core network events). Since the workqueue needs to sleep
to perform SPI transfers, should the driver acquire the lock, safely copy the
addresses to a local buffer, release the lock, and then perform the hardware
configuration?
> + ret = adin1140_mac_filter_set(priv, ha->addr, mask,
> + slot);
> + if (ret)
> + return;
> +
> + slot++;
> + }
[ ... ]
> +static void adin1140_stats_work(struct work_struct *work)
> +{
[ ... ]
> + for (i = 0; i < ARRAY_SIZE(adin1140_stat_regs); i++) {
> + ret = oa_tc6_read_register_mms(priv->tc6, OA_TC6_MAC_MMS1,
> + adin1140_stat_regs[i],
> + ®_val);
> + if (ret)
> + break;
> +
> + stat_buff[i] = reg_val;
> + }
> +
> + scoped_guard(spinlock, &priv->stat_lock)
[Severity: High]
Is there a deadlock risk here between process context and softirq?
The stat_lock is acquired in process context using scoped_guard(), which
translates to spin_lock() and does not disable bottom halves. However,
stat_lock is also acquired by the ndo_get_stats64 callback
(adin1140_ndo_get_stats64()), which can be executed from softirq context.
If a softirq fires on the same CPU while the workqueue holds the lock, the
softirq will spin indefinitely waiting for it. The driver must likely use
spin_lock_bh() for stat_lock.
[Severity: High]
Also, are hardware statistics overwritten rather than accumulated here?
If an SPI read fails, the loop breaks early, leaving the remaining elements in
the zero-initialized stat_buff as 0. The memcpy then unconditionally overwrites
the 64-bit software stats with these zeros, destroying previously accumulated
statistics.
Additionally, since hardware statistics counters are 32-bit and either wrap
or clear on read, overwriting the 64-bit priv->stats instead of accumulating
the deltas defeats the purpose of maintaining 64-bit software counters, as the
values will wrap around alongside the hardware.
> + memcpy(&priv->stats, stat_buff, sizeof(priv->stats));
> +
> + schedule_delayed_work(dwork, ADIN1140_STATS_CHECK_DELAY);
> +}
[ ... ]
> +static int adin1140_mdiobus_write(struct mii_bus *bus, int addr, int regnum,
> + u16 val)
> +{
> + return -EIO;
[Severity: Medium]
The commit message states that the C22 write function is a 'no-op' designed
to protect against ioctl MDIO access path hardware corruption. However, this
unconditionally returns -EIO.
Returning an error is functionally distinct from a no-op, as it signals an
I/O failure up to the MDIO core, potentially causing userspace tools or standard
PHY setup functions to loudly fail instead of gracefully bypassing the write.
If the hardware workaround necessitates silently ignoring C22 writes, should
this function return 0 instead?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-adin1140-driver-v4-0-0753e28ee004@analog.com?part=13
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add TI ADS1220
From: sashiko-bot @ 2026-06-10 15:29 UTC (permalink / raw)
To: Nguyen Minh Tien; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260610151342.44274-2-zizuzacker@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `diff-channels` property constraint uses invalid YAML flow sequence syntax (`items: [const: 0, const: 1]`) that will cause a fatal parser error or evaluate to a single-element list with a collapsed duplicate dictionary key.
- [Medium] The child node regex and `reg` maximum artificially limit the device tree to defining at most 7 channels, despite the hardware supporting 12 unique multiplexer configurations.
--
commit 2b3665a0096105a4bb2db6266e47dec0a0802cff
Author: Nguyen Minh Tien <zizuzacker@gmail.com>
dt-bindings: iio: adc: Add TI ADS1220
This commit adds a device tree binding for the TI ADS1220 24-bit ADC.
It describes the SPI device, its per-input channel child nodes,
external reference selection, and the DRDY interrupt.
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
> new file mode 100644
> index 0000000000000..1fedffc2a1475
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
[ ... ]
> +patternProperties:
> + "^channel@[0-6]$":
> + $ref: adc.yaml
> + type: object
> + description: Represents one ADC input configuration (channel).
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 6
[Severity: Medium]
The ADS1220 hardware supports 12 unique multiplexer combinations
(8 differential pairs and 4 single-ended inputs). Does this regex and
reg maximum artificially limit the device tree to defining at most 7
channels?
System integrators might not be able to represent all available
hardware configurations if this limit is enforced.
[ ... ]
> + diff-channels:
> + description:
> + Differential input pair routable by the ADS1220 multiplexer.
> + oneOf:
> + - items: [const: 0, const: 1]
> + - items: [const: 0, const: 2]
> + - items: [const: 0, const: 3]
> + - items: [const: 1, const: 2]
> + - items: [const: 1, const: 3]
> + - items: [const: 2, const: 3]
> + - items: [const: 1, const: 0]
> + - items: [const: 3, const: 2]
[Severity: High]
Will this syntax pass the dt_binding_check validation?
In YAML, a flow sequence like [const: 0, const: 1] evaluates to a list
containing a single implicit mapping with duplicate keys. The dt-schema
toolchain utilizes a strict parser that typically fails on duplicate
dictionary keys.
If it did parse, it would likely collapse the duplicate keys into a
single element, forcing the diff-channels array to have exactly one
element instead of the expected two.
Would it be better to use a valid flow mapping sequence, such as
- items: [{const: 0}, {const: 1}]?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260610151342.44274-1-zizuzacker@gmail.com?part=1
^ permalink raw reply
* [PATCH] dt-bindings: cache: l2c2x0: Add missing power-domains
From: Geert Uytterhoeven @ 2026-06-10 15:29 UTC (permalink / raw)
To: Conor Dooley, Jonathan Cameron, Rob Herring, Krzysztof Kozlowski
Cc: devicetree, linux-renesas-soc, Geert Uytterhoeven
On Renesas SH-Mobile and R-Mobile SoCs, the ARM PL310 L2 Cache
Controller is located in a controllable power area.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
This fixes "make dtbs_check":
arch/arm/boot/dts/renesas/r8a7740-armadillo800eva.dtb: cache-controller@f0100000 (arm,pl310-cache): 'power-domains' does not match any of the regexes: '^pinctrl-[0-9]+$'
from schema $id: http://devicetree.org/schemas/cache/l2c2x0.yaml
arch/arm/boot/dts/renesas/sh73a0-kzm9g.dtb: cache-controller@f0100000 (arm,pl310-cache): 'power-domains' does not match any of the regexes: '^pinctrl-[0-9]+$'
from schema $id: http://devicetree.org/schemas/cache/l2c2x0.yaml
---
Documentation/devicetree/bindings/cache/l2c2x0.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/cache/l2c2x0.yaml b/Documentation/devicetree/bindings/cache/l2c2x0.yaml
index 10c1a900202fc2ed..ee604117ffb3fe74 100644
--- a/Documentation/devicetree/bindings/cache/l2c2x0.yaml
+++ b/Documentation/devicetree/bindings/cache/l2c2x0.yaml
@@ -66,6 +66,9 @@ properties:
reg:
maxItems: 1
+ power-domains:
+ maxItems: 1
+
arm,data-latency:
description: Cycles of latency for Data RAM accesses. Specifies 3 cells of
read, write and setup latencies. Minimum valid values are 1. Controllers
--
2.43.0
^ permalink raw reply related
* Re: [PATCH V3 0/8] PCI: imx6: Integrate pwrctrl API and update device trees
From: Manivannan Sadhasivam @ 2026-06-10 15:29 UTC (permalink / raw)
To: Sherry Sun (OSS)
Cc: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
lpieralisi, kwilczynski, bhelgaas, hongxing.zhu, l.stach, imx,
linux-pci, linux-arm-kernel, devicetree, linux-kernel, sherry.sun
In-Reply-To: <20260520084904.2424253-1-sherry.sun@oss.nxp.com>
On Wed, May 20, 2026 at 04:48:56PM +0800, Sherry Sun (OSS) wrote:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> This series integrates the PCI pwrctrl framework into the pci-imx6
> driver and updates i.MX EVK board device trees to support it.
>
> Patches 2-8 update device trees for i.MX EVK boards which maintained
> by NXP to move power supply properties from the PCIe controller node
> to the Root Port child node, which is required for pwrctrl framework.
> Affected boards:
> - i.MX6Q/DL SABRESD
> - i.MX6SX SDB
> - i.MX8MM EVK
> - i.MX8MP EVK
> - i.MX8MQ EVK
> - i.MX8DXL/QM/QXP EVK
> - i.MX95 15x15/19x19 EVK
>
> The driver maintains legacy regulator handling for device trees that
> haven't been updated yet. Both old and new device tree structures are
> supported.
>
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
> Changes in V3:
> 1. Rebased on top of latest 7.1.0-rc4
>
> Changes in V2:
> 1. After commit 2d8c5098b847 ("PCI/pwrctrl: Do not power off on pwrctrl
> device removal"), the pwrctrl drivers no longer power off devices
> during removal. Update pci-imx6 driver's shutdown callback in patch#1
> to explicitly call pci_pwrctrl_power_off_devices() before
> pci_pwrctrl_destroy_devices() to ensure devices are properly powered
> off.
> ---
>
> Sherry Sun (8):
> PCI: imx6: Integrate new pwrctrl API for pci-imx6
> arm: dts: imx6qdl-sabresd: Move power supply property to Root Port
> node
> arm: dts: imx6sx-sdb: Move power supply property to Root Port node
> arm64: dts: imx8mm-evk: Move power supply property to Root Port node
> arm64: dts: imx8mp-evk: Move power supply properties to Root Port node
> arm64: dts: imx8mq-evk: Move power supply properties to Root Port node
> arm64: dts: imx8dxl/qm/qxp: Move power supply properties to Root Port
> node
> arm64: dts: imx95: Move power supply properties to Root Port node
Acked-by: Manivannan Sadhasivam <mani@kernel.org>
- Mani
>
> .../arm/boot/dts/nxp/imx/imx6qdl-sabresd.dtsi | 2 +-
> arch/arm/boot/dts/nxp/imx/imx6sx-sdb.dtsi | 2 +-
> arch/arm64/boot/dts/freescale/imx8dxl-evk.dts | 4 ++--
> arch/arm64/boot/dts/freescale/imx8mm-evk.dtsi | 2 +-
> arch/arm64/boot/dts/freescale/imx8mp-evk.dts | 4 ++--
> arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 4 ++--
> arch/arm64/boot/dts/freescale/imx8qm-mek.dts | 4 ++--
> arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 4 ++--
> .../boot/dts/freescale/imx95-15x15-evk.dts | 4 ++--
> .../boot/dts/freescale/imx95-19x19-evk.dts | 8 +++----
> drivers/pci/controller/dwc/Kconfig | 1 +
> drivers/pci/controller/dwc/pci-imx6.c | 24 ++++++++++++++++++-
> 12 files changed, 43 insertions(+), 20 deletions(-)
>
> --
> 2.37.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH net-next v6 3/5] net: dsa: tag_ks8995: Add the KS8995 tag handling
From: Jakub Kicinski @ 2026-06-10 15:39 UTC (permalink / raw)
To: linusw
Cc: Jakub Kicinski, woojung.huh, UNGLinuxDriver, andrew, olteanv,
davem, edumazet, pabeni, robh, krzk+dt, conor+dt, marex, horms,
linux, netdev, Woojung.Huh, devicetree, nb
In-Reply-To: <20260608-ks8995-to-ksz8-v6-3-d91eb43edcc5@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: tag_ks8995: Add the KS8995 tag handling
Adds a new DSA tagger for the Micrel/Microchip KS8995XA switch using a
modified 0x8100 ethertype where the lower nybble encodes a port bitmap.
The patch wires the new tagger into the DSA core, Kconfig, Makefile and
MAINTAINERS.
A few questions inline below.
> diff --git a/net/dsa/tag_ks8995.c b/net/dsa/tag_ks8995.c
> new file mode 100644
> index 000000000000..5bdb18271582
> --- /dev/null
> +++ b/net/dsa/tag_ks8995.c
> @@ -0,0 +1,132 @@
[ ... ]
> +static struct sk_buff *ks8995_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct vlan_ethhdr *hdr = vlan_eth_hdr(skb);
> + bool have_hwaccel_tag = false;
> + u16 tci = 0, portmask;
> +
> + /* Prepare the special KS8995 tags */
> + portmask = dsa_xmit_port_mask(skb, dev);
> +
> + if (skb_vlan_tag_present(skb) && skb->vlan_proto == htons(ETH_P_8021Q)) {
> + tci = skb_vlan_tag_get(skb);
> + __vlan_hwaccel_clear_tag(skb);
> + have_hwaccel_tag = true;
> + }
[Low]
What is intended to happen if the skb arrives with both a hwaccel VLAN
tag and an in-head 0x8100 tag, or with a hwaccel tag whose vlan_proto
is something other than 0x8100 (for example 0x88a8 service tag)?
In those cases this branch leaves the hwaccel tag alone or only consumes
it conditionally, and the second branch below either inserts a fresh
in-head tag while keeping a pre-existing in-head 0x8100, or modifies the
in-head 0x8100 while a non-8021Q hwaccel tag is still attached. Should
this path either reject such frames or normalise them first?
> +
> + if (have_hwaccel_tag || hdr->h_vlan_proto != htons(ETH_P_8021Q)) {
> + skb = vlan_insert_tag(skb, KS8995M_STPID(portmask), tci);
> + if (!skb)
> + return NULL;
[High]
Is returning NULL here safe?
include/linux/if_vlan.h documents that vlan_insert_tag() calls
dev_kfree_skb_any(skb) on failure (e.g. when skb_cow_head() fails under
memory pressure) and returns NULL. The caller in net/dsa/user.c does:
nskb = p->xmit(skb, dev);
if (!nskb) {
kfree_skb(skb);
return NETDEV_TX_OK;
}
so on the NULL return dsa_user_xmit() runs kfree_skb() on its local
pointer that vlan_insert_tag() has already freed. Doesn't that produce a
use-after-free on skb->users plus a double free?
Should this path swallow the free done by vlan_insert_tag() (e.g. by
returning a sentinel handled by the caller) so dsa_user_xmit() does not
re-free the skb?
> + hdr = vlan_eth_hdr(skb);
> + netdev_dbg(dev, "%s: inserted VLAN TAG %04x TCI %04x\n",
> + __func__, hdr->h_vlan_proto, hdr->h_vlan_TCI);
> + } else {
> + /* VLAN tag already exists in skb head, modify it in place */
> + hdr = vlan_eth_hdr(skb);
> + hdr->h_vlan_proto = KS8995M_STPID(portmask);
> + netdev_dbg(dev, "%s: modified VLAN TAG %04x\n",
> + __func__, hdr->h_vlan_proto);
> + }
> +
> + return skb;
> +}
> +
> +static struct sk_buff *ks8995_rcv(struct sk_buff *skb, struct net_device *dev)
> +{
> + int portmask;
> + u16 etype;
> +
> + /* We are expecting all received packets to have a mangled VLAN
> + * TPID, so drop anything else. Because of the non-standard TPID,
> + * don't even bother looking for a tag in the hwaccel area.
> + *
> + * We have to inspect the ethertype directly because skb->protocol
> + * will contain garbage.
> + */
> + etype = ntohs(*(__be16 *)dsa_etype_header_pos_rx(skb));
> + if ((etype & KS8995M_STPID_STD) != ETH_P_8021Q) {
> + netdev_dbg(dev, "%s: dropped ethertype 0x%04x\n",
> + __func__, etype);
> + return NULL;
> + }
> + netdev_dbg(dev, "%s: received ethertype %04x\n",
> + __func__, etype);
> +
> + /* Move the custom DSA+VLAN tag into the hwaccel area and strip
> + * it from the skb head
> + */
> + skb = skb_vlan_untag(skb);
> + if (!skb) {
> + netdev_err(dev, "%s: unable to untag skb\n", __func__);
> + return NULL;
> + }
[High]
Is the NULL return here compatible with the cpu_dp->rcv contract?
skb_vlan_untag() can take its err_free path (e.g. when the
pskb_may_pull(VLAN_HLEN + sizeof(unsigned short)) inside it fails, or
skb_reorder_vlan_header() fails) which calls kfree_skb(skb) and returns
NULL.
The caller in net/dsa/tag.c is:
skb = skb_unshare(skb, GFP_ATOMIC);
...
nskb = cpu_dp->rcv(skb, dev);
...
if (!nskb) {
kfree_skb(skb);
return 0;
}
so when ks8995_rcv() returns NULL after skb_vlan_untag() has already
freed its argument, dsa_switch_rcv() ends up calling kfree_skb() on a
freed skb. Wouldn't that be a use-after-free on skb->users plus a
double free?
Other cpu_dp->rcv handlers appear to assume that returning NULL means
the skb was not freed. Should this handler do something equivalent so
the contract still holds?
> +
> + portmask = FIELD_GET(KS8995M_STPID_PORTMASK, etype);
> + netdev_dbg(dev, "%s: etype %04x portmask %04x (%d)\n",
> + __func__, etype, portmask, ilog2(portmask));
> + skb->dev = dsa_conduit_find_user(dev, 0, ilog2(portmask));
> + if (!skb->dev)
> + return NULL;
[Low, Low]
A literal 0x8100 frame from the wire passes the
(etype & GENMASK(15,4)) != ETH_P_8021Q gate above, in which case
FIELD_GET(GENMASK(3,0), 0x8100) is 0 and ilog2(0) is computed (and also
passed as a netdev_dbg() argument). The frame is only dropped because
dsa_conduit_find_user() happens not to match port -1.
Would an explicit check be clearer:
portmask = FIELD_GET(KS8995M_STPID_PORTMASK, etype);
if (!portmask)
return NULL;
The commit message says "a bit in the last nybble is set for each
target port", suggesting bitmap semantics, but ilog2(portmask) on
ingress only handles a single bit and silently picks the highest set
bit if more than one is set. Would an is_power_of_2(portmask) sanity
check be appropriate here?
> +
> + /* Preserve the VLAN tag if it contains a non-zero VID which is not
> + * identical to 0x001, or PCP, and restore its TPID to the standard
> + * value.
> + *
> + * If this is just an ordinary inbound package the datasheet claims
> + * it will "replace null VID with ingress port VID", which means
> + * VID set to 1: 0x8101 0001 for port 0 or 0x8102 0001 for port 1.
> + * So in the DSA driver we will set the default port VID to 0 so
> + * we can properly detect non-VLAN frames.
> + */
> + if (!skb->vlan_tci) {
> + netdev_dbg(dev, "%s: clear VLAN tag from frame\n", __func__);
> + __vlan_hwaccel_clear_tag(skb);
> + } else {
> + skb->vlan_proto = htons(ETH_P_8021Q);
> + netdev_dbg(dev, "%s: vlan_tci = 0x%04x VLAN frame\n",
> + __func__, skb->vlan_tci);
> + }
[Low]
The comment says
Preserve the VLAN tag if it contains a non-zero VID which is not
identical to 0x001, or PCP
but the code only tests:
if (!skb->vlan_tci)
That matches VID == 0 and PCP == 0 and DEI == 0 only, so frames with
VID == 1 (the chip's "replace null VID with ingress port VID" default)
fall through into the "preserve as a VLAN frame" branch instead of
being stripped. Should the comment and the code be reconciled, or
should the test also strip VID == 1 frames so the behaviour does not
depend on the switch driver also forcing the default port VID to 0?
> +
> + dsa_default_offload_fwd_mark(skb);
> +
> + return skb;
> +}
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net-next v6 4/5] net: dsa: microchip: Support Microchip KSZ8995XA / KS8995XA
From: Jakub Kicinski @ 2026-06-10 15:39 UTC (permalink / raw)
To: linusw
Cc: Jakub Kicinski, woojung.huh, UNGLinuxDriver, andrew, olteanv,
davem, edumazet, pabeni, robh, krzk+dt, conor+dt, marex, horms,
linux, netdev, Woojung.Huh, devicetree, nb
In-Reply-To: <20260608-ks8995-to-ksz8-v6-4-d91eb43edcc5@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: microchip: Support Microchip KSZ8995XA / KS8995XA
This patch adds support for the Microchip KSZ8995XA (a.k.a. Micrel
KS8995XA) switch to the KSZ DSA driver, introducing a new chip entry, a
restricted dev_ops/switch_ops set, and a ksz_is_ksz8995xa() helper for
future KSZ8995E/MA differentiation.
> diff --git a/drivers/net/dsa/microchip/ksz8.c b/drivers/net/dsa/microchip/ksz8.c
> index 0f84b2e7d25d..0b02588e22fc 100644
> --- a/drivers/net/dsa/microchip/ksz8.c
> +++ b/drivers/net/dsa/microchip/ksz8.c
> @@ -137,7 +138,18 @@ static int ksz8_pme_pwrite8(struct ksz_device *dev, int port, int offset, u8 dat
>
> static int ksz8_reset_switch(struct ksz_device *dev)
> {
> - if (ksz_is_ksz88x3(dev)) {
> + if (ksz_is_ksz8995xa(dev)) {
> + unsigned int port;
> +
> + /* KSZ8995XA is "reset" by stopping the switch and starting it */
> + ksz_cfg(dev, REG_SW_ID_1, SW_START_SWITCH, false);
> + usleep_range(10, 100);
> + ksz_cfg(dev, REG_SW_ID_1, SW_START_SWITCH, true);
[Medium]
For KSZ8995XA, ksz8_reset_switch() asserts SW_START_SWITCH before
ksz8_setup() programs broadcast storm rate, REG_SW_CTRL_2,
REG_SW_CTRL_9 SW_SPECIAL_TAG, port-membership and the
PORT_INSERT_TAG/PORT_REMOVE_TAG bits. Other variants in this same file
(for example ksz88x3) keep the forwarding engine quiescent during
configuration and only assert SW_START at the end of ksz8_setup() via:
regmap_update_bits(ksz_regmap_8(dev), regs[S_START_CTRL],
SW_START, SW_START);
For KSZ8995XA, ksz8995xa_regs has [S_START_CTRL] = 0x01, the same
register written here, so by the time ksz8_setup() reaches its final
"start switch" step the engine has been forwarding throughout the
reconfiguration window. Is that intentional? Could the switch be left
stopped here and only started at the end of ksz8_setup() like the
other variants?
> +
> + /* Often boot loaders have fun with ports, so reset them */
> + for (port = 0; port < dev->info->port_cnt; port++)
> + ksz_port_cfg(dev, port, 0x0f, BIT(4), true);
[Medium]
This writes BIT(4) at port-relative offset 0x0F once per port and
never clears it, with no readback or settling delay before the rest of
the setup proceeds to read/write port registers. If that bit is a
level-triggered reset, every port is left held in reset; if it is
self-clearing or a feature bit, the code happens to work.
The KSZ8863 path in this same function explicitly clears its reset
bit:
ksz_cfg(dev, KSZ8863_REG_SW_RESET,
KSZ8863_GLOBAL_SOFTWARE_RESET | KSZ8863_PCS_RESET, true);
ksz_cfg(dev, KSZ8863_REG_SW_RESET,
KSZ8863_GLOBAL_SOFTWARE_RESET | KSZ8863_PCS_RESET, false);
and the KSZ87xx path uses an explicit power-down/power-up pair. Could
this code use named macros in ksz8_reg.h (matching the other named
constants added in this patch like SW_SPECIAL_TAG, SW_START_SWITCH,
PORT_INSERT_TAG, PORT_REMOVE_TAG) and document whether the bit is
self-clearing or whether a paired clear is needed?
> + } else if (ksz_is_ksz88x3(dev)) {
> /* reset switch */
> ksz_cfg(dev, KSZ8863_REG_SW_RESET,
> KSZ8863_GLOBAL_SOFTWARE_RESET | KSZ8863_PCS_RESET, true);
[ ... ]
> @@ -1693,14 +1729,16 @@ static void ksz8_port_setup(struct ksz_device *dev, int port, bool cpu_port)
> offset = P1CR1;
> ksz_port_cfg(dev, port, offset, PORT_BROADCAST_STORM, true);
>
> - ksz8_port_queue_split(dev, port, dev->info->num_tx_queues);
> + if (!ksz_is_ksz8995xa(dev)) {
> + ksz8_port_queue_split(dev, port, dev->info->num_tx_queues);
[Medium]
ksz_chip_data for KSZ8995XA sets .num_tx_queues = 4, and ksz8_setup()
unconditionally does:
ds->num_tx_queues = dev->info->num_tx_queues;
But ksz8_port_setup() now skips ksz8_port_queue_split() for
KSZ8995XA, so the chip's queue-split hardware registers are left at
their power-on default while DSA is told there are 4 TX queues. If the
default is not a 4-queue split, classification decisions made by the
stack (priority/PCP/DSCP-to-queue mapping) will not correspond to the
queues the hardware actually uses.
Should .num_tx_queues reflect the actual default for this chip, or
should the queue-split registers be programmed?
[ ... ]
> diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
> index 57c8abfe0147..ac27beb3054b 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -1485,6 +1499,21 @@ const struct ksz_chip_data ksz_switch_chips[] = {
> .internal_phy = {true, true, true, true, false},
> },
>
> + [KSZ8995XA] = {
> + .chip_id = KSZ8995XA_CHIP_ID, /* Also known as KS8995XA */
> + .dev_name = "KSZ8995XA",
> + .cpu_ports = 0x10, /* can be configured as cpu port */
> + .port_cnt = 5, /* total cpu and user ports */
> + .num_tx_queues = 4,
> + .num_ipms = 4,
> + .ops = &ksz8995xa_dev_ops,
> + .switch_ops = &ksz8995xa_switch_ops,
> + .phylink_mac_ops = &ksz88x3_phylink_mac_ops,
> + .regs = ksz8995xa_regs,
> + .supports_mii = {true, true, true, true, true},
> + .internal_phy = {true, true, true, true, false},
> + },
[Low]
The KSZ8995XA chip data omits .num_vlans, .num_alus, and .num_statics,
so they default to 0. ksz8_setup() then unconditionally calls:
dev->vlan_cache = devm_kcalloc(dev->dev, sizeof(struct vlan_table),
dev->info->num_vlans, GFP_KERNEL);
if (!dev->vlan_cache)
return -ENOMEM;
devm_kcalloc with n = 0 returns ZERO_SIZE_PTR, which is non-NULL and
therefore not caught by the !dev->vlan_cache check. Today every
dereference of vlan_cache is gated by !ksz_is_ksz8995xa(dev) or by the
absence of VLAN ops in ksz8995xa_switch_ops, so there is no crash, but
any future shared helper that touches vlan_cache without a chip-id
gate would dereference ZERO_SIZE_PTR.
Could the allocation be skipped for KSZ8995XA, or could .num_vlans be
set to a value that makes the !NULL check meaningful?
[ ... ]
> @@ -2826,6 +2855,10 @@ void ksz_init_mib_timer(struct ksz_device *dev)
> {
> int i;
>
> + /* KSZ8995XA lacks MiB features */
> + if (ksz_is_ksz8995xa(dev))
> + return;
> +
> INIT_DELAYED_WORK(&dev->mib_read, ksz_mib_read_work);
[Low]
This early return skips INIT_DELAYED_WORK(&dev->mib_read, ...). The
patch then relies on the undocumented invariant that mib_read_interval
== 0 means dev->mib_read must not be touched.
ksz_suspend()/ksz_resume() in ksz_common.c call
cancel_delayed_work_sync(&dev->mib_read) and
schedule_delayed_work(&dev->mib_read, ...) without any chip-id gate.
ksz8995xa_switch_ops happens to omit .suspend/.resume today, so there
is no current crash, but any future addition of those ops or any other
unguarded user of dev->mib_read on this chip will operate on
uninitialised work_struct memory.
Would it be safer to call INIT_DELAYED_WORK unconditionally and only
skip the schedule for KSZ8995XA?
[ ... ]
> @@ -4662,6 +4700,10 @@ int ksz_switch_register(struct ksz_device *dev)
> if (ret)
> return ret;
>
> + /* Override ops with something simpler for this legacy chip */
> + if (ksz_is_ksz8995xa(dev))
> + dev->ds->ops = &ksz8995xa_switch_ops;
> +
[Low]
Is this assignment a no-op in the supported flow? ksz_switch_alloc()
has already set ds->ops = chip->switch_ops, and
ksz_switch_chips[KSZ8995XA].switch_ops is &ksz8995xa_switch_ops, so
the same pointer is being written back.
The comment "Override ops with something simpler for this legacy chip"
implies a transformation that does not actually occur, and no other
chip in the family does this. Could this hunk be dropped, or the
comment reworded to match what the code does?
> diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
> index b67038cf1bd0..661a82026e47 100644
> --- a/drivers/net/dsa/microchip/ksz_common.h
> +++ b/drivers/net/dsa/microchip/ksz_common.h
> @@ -751,7 +752,13 @@ static inline bool ksz_is_ksz88x3(struct ksz_device *dev)
> static inline bool ksz_is_8895_family(struct ksz_device *dev)
> {
> return dev->chip_id == KSZ8895_CHIP_ID ||
> - dev->chip_id == KSZ8864_CHIP_ID;
> + dev->chip_id == KSZ8864_CHIP_ID ||
> + dev->chip_id == KSZ8995XA_CHIP_ID;
> }
[Low]
This silently expands ksz_is_8895_family() to include KSZ8995XA, which
through is_ksz8()/is_ksz88xx() classifies the new chip as an 8895
family member across ksz_dcb.c, ksz8.c and ksz_common.c.
Today every potentially-divergent path is explicitly gated by
ksz_is_ksz8995xa() opt-outs or by the dev->ds->ops override, but this
"opt in via family helper, then opt out via per-chip checks" pattern
is brittle: future helper-driven logic touching the 8895 family will
silently include KSZ8995XA, and comments such as "/* KSZ8864, KSZ8895
*/" in ksz88xx_get_tag_protocol/ksz88xx_connect_tag_protocol become
inaccurate.
The commit message also does not acknowledge this expansion or
explain why a feature-bit approach in ksz_chip_data was not used,
especially given that the KSZ8995E/MA variants are mentioned as
future work. Could the commit message cover this design choice, or
could the family classification be reconsidered?
^ permalink raw reply
* Re: [PATCH 2/2] iio: adc: Add TI ADS1220 driver
From: sashiko-bot @ 2026-06-10 15:43 UTC (permalink / raw)
To: Nguyen Minh Tien; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260610151342.44274-3-zizuzacker@gmail.com>
Thank you for your contribution! Sashiko AI review found 13 potential issue(s) to consider:
- [Critical] An integer overflow when calculating scale gain leads to a divide-by-zero kernel panic on 32-bit platforms.
- [High] Inconsistent formatting for IIO_CHAN_INFO_SCALE breaks IIO UAPI contracts. read_raw returns the true scale multiplier, but write_raw expects a gain reciprocal, and read_avail exposes gain reciprocals.
- [High] Using iio_validate_scan_mask_onehot breaks timestamp functionality and allows an out-of-bounds array read in buffer_preenable.
- [High] Missing `.validate_trigger` allows external triggers to read asynchronously without the RDATA command, causing data corruption.
- [High] The `rx` buffer is not DMA-aligned, violating DMA API requirements and risking memory corruption during SPI transfers.
- [High] Disabling the triggered buffer leaves the ADC in continuous conversion mode, causing DRDY interrupt storms.
- [High] Missing mutex lock protection for the shared `st->tx` and `st->rx` buffers.
- [High] Using IRQF_NO_THREAD for a potentially level-triggered interrupt without clearing the interrupt in hardware causes an interrupt storm.
- [Medium] ads1220_write_raw modifies channel configuration without claiming direct mode, causing races with active buffers and single-shot conversions.
- [Medium] IIO_CHAN_INFO_OFFSET returns the positive uncalibrated raw value instead of its negated value, causing the offset error to be doubled in user-space.
- [Medium] Concurrent SPI writes can occur during module removal due to incorrect `devm` cleanup order.
- [Medium] ads1220_debugfs_reg_access communicates with the device without resuming it from power-down.
- [Medium] The driver unconditionally fails probe if the AVDD regulator voltage cannot be read, breaking support for dummy regulators.
--
commit 1ae738a6247e14166f63bc753a84967fb3d54773
Author: Nguyen Minh Tien <zizuzacker@gmail.com>
iio: adc: Add TI ADS1220 driver
Add an IIO driver for the Texas Instruments ADS1220 24-bit delta-sigma
SPI ADC. The driver supports single-ended and differential voltage
channels described as device-tree child nodes, per-channel programmable
gain (exposed through scale) and data rate, internal and external reference
options, single-shot conversions and a DRDY-interrupt-driven triggered buffer.
diff --git a/drivers/iio/adc/ti-ads1220.c b/drivers/iio/adc/ti-ads1220.c
new file mode 100644
index 0000000000000..06bcc88418b4c
--- /dev/null
+++ b/drivers/iio/adc/ti-ads1220.c
[ ... ]
> +struct ads1220_state {
> + struct spi_device *spi;
> + struct completion completion;
> + struct iio_trigger *trig;
> + struct ads1220_channel_config *channels_cfg;
> + unsigned int num_channels_cfg;
> + int vref_uV;
> + unsigned int vref_source;
> +
> + /*
> + * DMA-safe buffers. tx is used for command/register writes, rx for
> + * register and conversion-result reads. scan holds one sample plus a
> + * timestamp for the triggered buffer.
> + */
> + u8 tx[2] __aligned(IIO_DMA_MINALIGN);
> + u8 rx[ADS1220_DATA_BYTES];
[Severity: High]
Does placing the rx array immediately after the 2-byte tx array without
alignment cause it to share the same cacheline? If st->rx is passed to
spi_read(), mapping this unaligned sub-cacheline buffer for DMA might
corrupt adjacent fields on architectures without hardware cache coherence.
> + struct {
> + s32 sample;
> + aligned_s64 timestamp;
> + } scan;
> +};
[ ... ]
> +static int ads1220_read_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int *val, int *val2, long mask)
> +{
> + struct ads1220_state *st = iio_priv(indio_dev);
> + struct ads1220_channel_config *cfg = &st->channels_cfg[chan->address];
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> + ret = ads1220_single_conversion(st, chan, val, false);
> + iio_device_release_direct(indio_dev);
> + return ret;
> + case IIO_CHAN_INFO_OFFSET:
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> + ret = ads1220_single_conversion(st, chan, val, true);
[Severity: Medium]
Should the hardware offset be negated before returning it here?
ads1220_single_conversion() returns the raw measurement of the shorted inputs.
Since the standard IIO calibration formula applied by user-space is
`value = (raw + offset) * scale`, returning a positive offset when the hardware
reads a positive value will apply the offset in the wrong direction and double
the error.
> + iio_device_release_direct(indio_dev);
> + return ret;
> + case IIO_CHAN_INFO_SCALE:
> + /* scale [mV] = vref / (gain * 2^23); gain is a power of two. */
> + *val = st->vref_uV / MILLI;
> + *val2 = (chan->scan_type.realbits - 1) + ilog2(cfg->gain);
> + return IIO_VAL_FRACTIONAL_LOG2;
[Severity: High]
Are the IIO_CHAN_INFO_SCALE interfaces handling values symmetrically?
ads1220_read_raw() returns the actual physical scale multiplier here, but
ads1220_write_raw() treats the user input as a gain reciprocal, and
ads1220_read_avail() exposes gain reciprocals instead of voltage scales.
If a user reads the scale and writes the exact same value back, could this
evaluate to an invalid gain and return -EINVAL?
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *val = cfg->datarate;
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
[ ... ]
> +static int ads1220_write_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int val, int val2, long mask)
> +{
> + struct ads1220_state *st = iio_priv(indio_dev);
> + struct ads1220_channel_config *cfg = &st->channels_cfg[chan->address];
> + unsigned int gain;
> + int i;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + /* The available scales are the gain reciprocals (e.g. 1/4). */
> + if (val == 0 && val2 == 0)
> + return -EINVAL;
> +
> + gain = MICRO / (val * MICRO + val2);
[Severity: Critical]
Can the expression `(val * MICRO + val2)` overflow on 32-bit platforms if a
user writes a crafted string (e.g., "4294.967296") to the sysfs scale
attribute? If the multiplication wraps to exactly zero, does the subsequent
`gain = MICRO / ...` division result in a divide-by-zero kernel panic?
> + if (!is_power_of_2(gain) || gain > BIT(ADS1220_NUM_GAINS - 1))
> + return -EINVAL;
> + if (cfg->single_ended && gain > ADS1220_MAX_SE_GAIN)
> + return -EINVAL;
> +
> + cfg->gain = gain;
> + return 0;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + for (i = 0; i < ARRAY_SIZE(ads1220_datarates); i++) {
> + if (ads1220_datarates[i] == val) {
> + cfg->datarate = val;
[Severity: Medium]
Does modifying cfg->gain and cfg->datarate in ads1220_write_raw() require
serialization via iio_device_claim_direct()? Modifying these without the lock
might race with an active triggered buffer or single-shot conversions,
potentially desynchronizing driver state and hardware or corrupting timeout
calculations.
> + return 0;
> + }
> + }
> + return -EINVAL;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int ads1220_debugfs_reg_access(struct iio_dev *indio_dev,
> + unsigned int reg, unsigned int writeval,
> + unsigned int *readval)
> +{
> + struct ads1220_state *st = iio_priv(indio_dev);
> + u8 val;
> + int ret;
> +
> + if (reg > ADS1220_MAX_REG)
> + return -EINVAL;
> +
> + if (readval) {
> + ret = ads1220_read_reg(st, reg, &val);
[Severity: Medium]
Does ads1220_debugfs_reg_access() need to call pm_runtime_resume_and_get()
before initiating SPI transfers? If the device is in the autosuspended
POWERDOWN state, these transactions might fail or return garbage data.
> + if (ret)
> + return ret;
> + *readval = val;
> + return 0;
> + }
> +
> + return ads1220_write_reg(st, reg, writeval);
[Severity: High]
Are the shared st->tx and st->rx buffers protected from concurrent access
here? Writing to st->tx directly via debugfs or pm_runtime_suspend without
locking could race with an active triggered buffer reading in
ads1220_trigger_handler(), potentially overwriting the tx buffer mid-transfer
and sending corrupted SPI commands.
> +}
> +
> +static const struct iio_info ads1220_info = {
> + .read_raw = ads1220_read_raw,
> + .read_avail = ads1220_read_avail,
> + .write_raw = ads1220_write_raw,
> + .debugfs_reg_access = ads1220_debugfs_reg_access,
> +};
[Severity: High]
Does this struct need a .validate_trigger callback to prevent arbitrary
external triggers from being attached? Without it, external triggers could
execute asynchronously to the ADC's data-ready signal. The trigger handler
uses spi_read() directly without an RDATA command, which according to the
datasheet could return corrupted data if a register update overlaps.
> +
> +static int ads1220_buffer_preenable(struct iio_dev *indio_dev)
> +{
> + struct ads1220_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> + struct ads1220_channel_config *cfg;
> + unsigned int index;
> + int ret;
> +
> + index = find_first_bit(indio_dev->active_scan_mask,
> + iio_get_masklength(indio_dev));
> + cfg = &st->channels_cfg[index];
[Severity: High]
What happens if a user enables ONLY the timestamp channel? The active scan
mask would have exactly one bit set at index = st->num_channels_cfg. Will
find_first_bit() return this index and cause an out-of-bounds array read
on st->channels_cfg[index]?
> +
> + ret = pm_runtime_resume_and_get(dev);
> + if (ret)
> + return ret;
> +
> + ret = ads1220_configure(st, cfg->mux, cfg->gain, cfg->datarate,
> + cfg->single_ended, true);
> + if (ret)
> + goto err;
> +
> + ret = ads1220_command(st, ADS1220_CMD_START);
> + if (ret)
> + goto err;
> +
> + return 0;
> +err:
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
> + return ret;
> +}
> +
> +static int ads1220_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> + struct ads1220_state *st = iio_priv(indio_dev);
> + struct device *dev = &st->spi->dev;
> +
> + pm_runtime_mark_last_busy(dev);
> + pm_runtime_put_autosuspend(dev);
[Severity: High]
Does the device need to be explicitly stopped or powered down here?
Dropping the PM runtime reference with a 2000ms autosuspend delay leaves
the ADC running in continuous conversion mode. The hardware might keep
asserting the DRDY interrupt, but since the buffer is disabled, the IRQ
handler will only complete the completion without reading data to clear
the interrupt, potentially causing an immediate IRQ storm.
> +
> + return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops ads1220_buffer_setup_ops = {
> + .preenable = ads1220_buffer_preenable,
> + .postdisable = ads1220_buffer_postdisable,
> + .validate_scan_mask = &iio_validate_scan_mask_onehot,
> +};
[Severity: High]
Is iio_validate_scan_mask_onehot the correct validator for this device?
This validator requires exactly one bit in the mask to be set. Since the
IIO core includes the timestamp channel in the active_scan_mask, enabling
one data channel plus the timestamp results in 2 bits being set, which
will cause the validator to reject valid configurations.
[ ... ]
> +static int ads1220_probe(struct spi_device *spi)
> +{
> + struct device *dev = &spi->dev;
> + struct iio_dev *indio_dev;
> + struct ads1220_state *st;
> + int avdd_uV;
> + int ret;
[ ... ]
> + ret = devm_regulator_get_enable(dev, "dvdd");
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to enable dvdd\n");
> +
> + avdd_uV = devm_regulator_get_enable_read_voltage(dev, "avdd");
> + if (avdd_uV < 0)
> + return dev_err_probe(dev, avdd_uV, "Failed to get avdd\n");
[Severity: Medium]
Should the driver fail probe unconditionally here? If the devicetree does
not explicitly model the AVDD supply, the core passes a dummy regulator.
Reading the voltage from a dummy regulator returns -EINVAL, which might
prevent the driver from probing entirely on such systems, even when
ti,vref-avdd is not specified.
[ ... ]
> + if (spi->irq > 0) {
> + ret = devm_request_irq(dev, spi->irq, ads1220_irq_handler,
> + IRQF_NO_THREAD, "ads1220", indio_dev);
[Severity: High]
Can using IRQF_NO_THREAD without clearing the physical DRDY line cause an
interrupt storm? The hardirq handler immediately returns IRQ_HANDLED
without issuing an SPI transaction to read the data. If the interrupt is
configured as level-triggered, the interrupt controller might repeatedly
re-fire the interrupt upon return. Does this need IRQF_ONESHOT with a
threaded handler, or an explicit enforcement of IRQF_TRIGGER_FALLING?
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to request irq\n");
> +
> + st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> + indio_dev->name,
> + iio_device_id(indio_dev));
> + if (!st->trig)
> + return -ENOMEM;
> +
> + st->trig->ops = &ads1220_trigger_ops;
> + iio_trigger_set_drvdata(st->trig, indio_dev);
> +
> + ret = devm_iio_trigger_register(dev, st->trig);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to register trigger\n");
> + }
> +
> + ret = ads1220_init(st);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to initialize device\n");
> +
> + pm_runtime_set_autosuspend_delay(dev, ADS1220_SUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_set_active(dev);
> +
> + ret = devm_pm_runtime_enable(dev);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to enable pm runtime\n");
> +
> + ret = devm_add_action_or_reset(dev, ads1220_powerdown, st);
[Severity: Medium]
Will registering ads1220_powerdown after pm_runtime_enable cause a teardown
ordering issue? Since devm executes cleanups in reverse order, powerdown
would execute before PM runtime is disabled. Could the PM autosuspend
workqueue trigger ads1220_runtime_suspend at the same moment powerdown
is executing, causing a data race on the unprotected st->tx buffer?
> + if (ret)
> + return ret;
> +
> + return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static int ads1220_runtime_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct ads1220_state *st = iio_priv(indio_dev);
> +
> + return ads1220_command(st, ADS1220_CMD_POWERDOWN);
> +}
[Severity: High]
Does ads1220_runtime_suspend() overwrite the shared st->tx buffer without
locking? This asynchronous PM runtime suspend could execute concurrently
with an active triggered buffer reading or other SPI transactions, corrupting
the sent SPI commands.
> +
> +static int ads1220_runtime_resume(struct device *dev)
> +{
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260610151342.44274-1-zizuzacker@gmail.com?part=2
^ permalink raw reply
* [PATCH v1 2/6] ASoC: qcom: q6apm-lpass-dais: add TDM DAI operations
From: Prasad Kumpatla @ 2026-06-10 15:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla
In-Reply-To: <20260610154517.134570-1-prasad.kumpatla@oss.qualcomm.com>
Add TDM DAI operations to q6apm-lpass-dais so AudioReach TDM
backends can be configured through the normal ASoC hw_params and DAI
setup flow.
The TDM set_tdm_slot() callback validates the supported slot width and
slot count, stores the active slot mask in the AudioReach module
configuration, and leaves existing DMA, I2S and HDMI paths unchanged.
Reuse the existing LPASS child-clock handling for TDM nodes as well as
MI2S nodes, since TDM backends also request optional backend clocks
through the machine driver set_sysclk() path.
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 64 ++++++++++++++++++++++++-
1 file changed, 63 insertions(+), 1 deletion(-)
diff --git a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
index 143750afb..d07b2d751 100644
--- a/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
+++ b/sound/soc/qcom/qdsp6/q6apm-lpass-dais.c
@@ -336,6 +336,55 @@ static int q6i2s_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
return 0;
}
+static int q6tdm_set_tdm_slot(struct snd_soc_dai *dai,
+ unsigned int tx_mask,
+ unsigned int rx_mask,
+ int slots, int slot_width)
+{
+ struct q6apm_lpass_dai_data *dai_data = dev_get_drvdata(dai->dev);
+ struct audioreach_module_config *cfg = &dai_data->module_config[dai->id];
+ unsigned int cap_mask;
+
+ if (slot_width != 16 && slot_width != 32) {
+ dev_err(dai->dev, "%s: invalid slot_width %d\n",
+ __func__, slot_width);
+ return -EINVAL;
+ }
+
+ switch (slots) {
+ case 2:
+ cap_mask = 0x03;
+ break;
+ case 4:
+ cap_mask = 0x0f;
+ break;
+ case 8:
+ cap_mask = 0xff;
+ break;
+ case 16:
+ cap_mask = 0xffff;
+ break;
+ default:
+ dev_err(dai->dev, "%s: invalid slots %d\n",
+ __func__, slots);
+ return -EINVAL;
+ }
+
+ switch (dai->id) {
+ case PRIMARY_TDM_RX_0 ... QUINARY_TDM_TX_7:
+ cfg->nslots_per_frame = slots;
+ cfg->slot_width = slot_width;
+ cfg->slot_mask = ((dai->id & 0x1) ? tx_mask : rx_mask) & cap_mask;
+ break;
+ default:
+ dev_err(dai->dev, "%s: invalid dai id 0x%x\n",
+ __func__, dai->id);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static const struct snd_soc_dai_ops q6dma_ops = {
.prepare = q6apm_lpass_dai_prepare,
.startup = q6apm_lpass_dai_startup,
@@ -365,6 +414,17 @@ static const struct snd_soc_dai_ops q6hdmi_ops = {
.trigger = q6apm_lpass_dai_trigger,
};
+static const struct snd_soc_dai_ops q6tdm_ops = {
+ .prepare = q6apm_lpass_dai_prepare,
+ .startup = q6apm_lpass_dai_startup,
+ .shutdown = q6i2s_lpass_dai_shutdown,
+ .set_tdm_slot = q6tdm_set_tdm_slot,
+ .hw_params = q6dma_hw_params,
+ .set_fmt = q6i2s_set_fmt,
+ .set_sysclk = q6i2s_set_sysclk,
+ .trigger = q6apm_lpass_dai_trigger,
+};
+
static const struct snd_soc_component_driver q6apm_lpass_dai_component = {
.name = "q6apm-be-dai-component",
.of_xlate_dai_name = q6dsp_audio_ports_of_xlate_dai_name,
@@ -390,9 +450,10 @@ static int of_q6apm_parse_dai_data(struct device *dev,
}
switch (id) {
- /* MI2S specific properties */
+ /* MI2S/TDM child clocks */
case PRIMARY_MI2S_RX ... QUATERNARY_MI2S_TX:
case QUINARY_MI2S_RX ... QUINARY_MI2S_TX:
+ case PRIMARY_TDM_RX_0 ... QUINARY_TDM_TX_7:
priv = &data->priv[id];
priv->mclk = of_clk_get_by_name(node, "mclk");
if (IS_ERR(priv->mclk)) {
@@ -448,6 +509,7 @@ static int q6apm_lpass_dai_dev_probe(struct platform_device *pdev)
cfg.q6i2s_ops = &q6i2s_ops;
cfg.q6dma_ops = &q6dma_ops;
cfg.q6hdmi_ops = &q6hdmi_ops;
+ cfg.q6tdm_ops = &q6tdm_ops;
dais = q6dsp_audio_ports_set_config(dev, &cfg, &num_dais);
return devm_snd_soc_register_component(dev, &q6apm_lpass_dai_component, dais, num_dais);
--
2.34.1
^ permalink raw reply related
* [PATCH v1 0/6] ASoC: qcom: add AudioReach TDM backend support
From: Prasad Kumpatla @ 2026-06-10 15:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla
Add AudioReach support for LPASS Audio IF based TDM backends and wire it
up for the sc8280xp machine driver.
The series first adds topology-driven Audio IF source/sink handling so
the DSP interface parameters can be described by topology while runtime
media format and slot configuration still come from the machine driver.
It then adds TDM DAI operations for q6apm-lpass-dais, exposes the Audio
IF clock IDs through the q6dsp-lpass-ports binding and q6prm clock
tables, and introduces common QCOM helpers for parsing standard
dai-tdm-slot-* properties from backend CPU and codec endpoints.
Finally, sc8280xp uses the common helpers during hw_params to program
CPU and codec TDM slots, derive the backend bit clock from the active
PCM parameters, and request the CPU and codec clocks before the stream is
started.
This series depends on:
- https://lore.kernel.org/all/20260607-rubikpi-next-20260605-v1-3-7f334e16fea6@thundersoft.com/
- https://lore.kernel.org/linux-arm-msm/20260608023011.942228-1-mohammad.rafi.shaik@oss.qualcomm.com/
Prasad Kumpatla (6):
ASoC: qcom: qdsp6: add topology-driven Audio IF support
ASoC: qcom: q6apm-lpass-dais: add TDM DAI operations
dt-bindings: sound: qcom,q6dsp-lpass-ports: add Audio IF clocks
ASoC: qcom: q6prm: add Audio IF clock IDs
ASoC: qcom: common: add DAI-node TDM slot helpers
ASoC: qcom: sc8280xp: add TDM hw_params support
.../sound/qcom,q6dsp-lpass-ports.h | 57 ++++++
include/uapi/sound/snd_ar_tokens.h | 58 +++++++
sound/soc/qcom/common.c | 164 ++++++++++++++++++
sound/soc/qcom/common.h | 14 ++
sound/soc/qcom/qdsp6/audioreach.c | 97 +++++++++++
sound/soc/qcom/qdsp6/audioreach.h | 62 +++++++
sound/soc/qcom/qdsp6/q6apm-lpass-dais.c | 64 ++++++-
sound/soc/qcom/qdsp6/q6prm-clocks.c | 28 +++
sound/soc/qcom/qdsp6/q6prm.h | 29 ++++
sound/soc/qcom/qdsp6/topology.c | 108 ++++++++++++
sound/soc/qcom/sc8280xp.c | 59 +++++++
11 files changed, 739 insertions(+), 1 deletion(-)
base-commit: 49e02880ec0a8c378e811bc9d85da188d7c6204c
prerequisite-patch-id: 2f1bd3efac328030dd8efe28fb95f84603868043
prerequisite-patch-id: 047b4fb1894b92109aa7afcebd7d5c7988ec5379
prerequisite-patch-id: 222630a15afc952683d954a3c66617a223546de0
prerequisite-patch-id: 3c55edb41f1e25920a350ce1c6f31fde67fee45a
--
2.34.1
^ permalink raw reply
* [PATCH v1 3/6] dt-bindings: sound: qcom,q6dsp-lpass-ports: add Audio IF clocks
From: Prasad Kumpatla @ 2026-06-10 15:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla
In-Reply-To: <20260610154517.134570-1-prasad.kumpatla@oss.qualcomm.com>
Add the LPASS Audio IF clock IDs used by newer backend interfaces.
Platforms using Audio IF module backends request the interface bit
clocks through q6prm. Add the Audio IF IBIT and EBIT IDs to the binding
header so these clocks can be referenced from device trees.
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
.../sound/qcom,q6dsp-lpass-ports.h | 57 +++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
index 45850f2d4..bc860fcbf 100644
--- a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
+++ b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h
@@ -233,6 +233,63 @@
/* Clock ID for RX CORE MCLK2 2X MCLK */
#define LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK 70
+/** Clock ID of the Audio Intf 0 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF0_IBIT 71
+/** Clock ID of the Audio Intf 0 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF0_EBIT 72
+/** Clock ID of the Audio Intf 1 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF1_IBIT 73
+/** Clock ID of the Audio Intf 1 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF1_EBIT 74
+/** Clock ID of the Audio Intf 2 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF2_IBIT 75
+/** Clock ID of the Audio Intf 2 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF2_EBIT 76
+/** Clock ID of the Audio Intf 3 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF3_IBIT 77
+/** Clock ID of the Audio Intf 3 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF3_EBIT 78
+/** Clock ID of the Audio Intf 4 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF4_IBIT 79
+/** Clock ID of the Audio Intf 4 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF4_EBIT 80
+/** Clock ID of the Audio Intf 5 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF5_IBIT 81
+/** Clock ID of the Audio Intf 5 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF5_EBIT 82
+/** Clock ID of the Audio Intf 6 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF6_IBIT 83
+/** Clock ID of the Audio Intf 6 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF6_EBIT 84
+/** Clock ID of the Audio Intf 7 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF7_IBIT 85
+/** Clock ID of the Audio Intf 7 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF7_EBIT 86
+/** Clock ID of the Audio Intf 8 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF8_IBIT 87
+/** Clock ID of the Audio Intf 8 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF8_EBIT 88
+/** Clock ID of the Audio Intf 9 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF9_IBIT 89
+/** Clock ID of the Audio Intf 9 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF9_EBIT 90
+/** Clock ID of the Audio Intf 10 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF10_IBIT 91
+/** Clock ID of the Audio Intf 10 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF10_EBIT 92
+/** Clock ID of the Audio Intf 11 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF11_IBIT 93
+/** Clock ID of the Audio Intf 11 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF11_EBIT 94
+/** Clock ID of the Audio Intf 12 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_INTF12_IBIT 95
+/** Clock ID of the Audio Intf 12 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_INTF12_EBIT 96
+/** Clock ID of the Audio VA Intf 0 internal bit clock (IBIT). */
+#define LPASS_CLK_ID_AUD_VA_INTF0_IBIT 97
+/** Clock ID of the Audio VA Intf 0 external bit clock (EBIT). */
+#define LPASS_CLK_ID_AUD_VA_INTF0_EBIT 98
+
#define LPASS_HW_AVTIMER_VOTE 101
#define LPASS_HW_MACRO_VOTE 102
#define LPASS_HW_DCODEC_VOTE 103
--
2.34.1
^ permalink raw reply related
* [PATCH v1 6/6] ASoC: qcom: sc8280xp: add TDM hw_params support
From: Prasad Kumpatla @ 2026-06-10 15:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla
In-Reply-To: <20260610154517.134570-1-prasad.kumpatla@oss.qualcomm.com>
Add TDM backend handling to the sc8280xp machine driver.
Use the common QCOM DAI-node TDM helper to parse the standard
DAI TDM slot properties from backend CPU and codec endpoints. Reuse the
parsed configuration when programming DAIs so hw_params does not need a
second DT traversal.
Derive the LPASS backend bit clock from the runtime TDM parameters and
request it through the backend child-clock path using LPAIF_MI2S_BCLK.
Program codec sysclk in hw_params so codec PLL setup happens before the
stream is triggered.
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
sound/soc/qcom/sc8280xp.c | 59 +++++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/sound/soc/qcom/sc8280xp.c b/sound/soc/qcom/sc8280xp.c
index 1f3afc6d0..e5d23e244 100644
--- a/sound/soc/qcom/sc8280xp.c
+++ b/sound/soc/qcom/sc8280xp.c
@@ -83,6 +83,63 @@ static inline int sc8280xp_get_bclk_freq(struct snd_pcm_hw_params *params)
snd_pcm_format_width(params_format(params)));
}
+static int sc8280xp_tdm_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
+ struct sc8280xp_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
+ struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+ struct snd_soc_dai *codec_dai;
+ struct qcom_snd_tdm_slot_cfg cpu_cfg;
+ struct qcom_snd_tdm_slot_cfg codec_cfg;
+ unsigned int bclk_freq;
+ int ret;
+ int i;
+
+ ret = qcom_snd_get_dai_tdm_slots(rtd, &cpu_cfg, &codec_cfg);
+ if (ret)
+ return ret == -EINVAL ? 0 : ret;
+
+ if (!cpu_cfg.slots)
+ return 0;
+
+ ret = snd_soc_dai_set_fmt(cpu_dai, SND_SOC_DAIFMT_BP_FP);
+ if (ret)
+ return ret;
+
+ ret = qcom_snd_apply_dai_tdm_slots_cfg(rtd, &cpu_cfg, &codec_cfg);
+ if (ret)
+ return ret;
+
+ bclk_freq = snd_soc_tdm_params_to_bclk(params, cpu_cfg.slot_width, cpu_cfg.slots, 1);
+ if (!bclk_freq)
+ return -EINVAL;
+
+ if (data->snd_soc_common_priv->mi2s_bclk_enable) {
+ ret = snd_soc_dai_set_sysclk(cpu_dai, LPAIF_MI2S_BCLK, bclk_freq,
+ SND_SOC_CLOCK_IN);
+ if (ret) {
+ dev_err(rtd->dev, "%s: failed to set cpu sysclk: %d\n",
+ __func__, ret);
+ return ret;
+ }
+ }
+
+ if (data->snd_soc_common_priv->codec_sysclk_set) {
+ for_each_rtd_codec_dais(rtd, i, codec_dai) {
+ ret = snd_soc_dai_set_sysclk(codec_dai, 0, bclk_freq,
+ SND_SOC_CLOCK_IN);
+ if (ret) {
+ dev_err(rtd->dev, "%s: failed to set codec sysclk on %s: %d\n",
+ __func__, codec_dai->name, ret);
+ return ret;
+ }
+ }
+ }
+
+ return 0;
+}
+
static int sc8280xp_snd_init(struct snd_soc_pcm_runtime *rtd)
{
struct sc8280xp_snd_data *data = snd_soc_card_get_drvdata(rtd->card);
@@ -186,6 +243,8 @@ static int sc8280xp_snd_hw_params(struct snd_pcm_substream *substream,
0, mclk_freq,
SND_SOC_CLOCK_IN);
break;
+ case PRIMARY_TDM_RX_0 ... QUINARY_TDM_TX_7:
+ return sc8280xp_tdm_hw_params(substream, params);
default:
break;
}
--
2.34.1
^ permalink raw reply related
* [PATCH v1 1/6] ASoC: qcom: qdsp6: add topology-driven Audio IF support
From: Prasad Kumpatla @ 2026-06-10 15:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla
In-Reply-To: <20260610154517.134570-1-prasad.kumpatla@oss.qualcomm.com>
Add topology parsing and media-format programming for Audio IF
source and sink modules.
Add the Audio IF module IDs, the required topology tokens, and a
dedicated topology loader that stores the parsed interface
configuration in the AudioReach module state. Also add the Audio IF
media-format path that sends the interface configuration, hardware
endpoint media format, and frame-duration parameters for Audio IF
modules.
This keeps the serial-interface configuration topology-driven while
still allowing the machine driver to provide runtime slot and media
format settings. The same Audio IF path can then be reused for TDM,
PCM, and I2S style backends.
The new UAPI tokens (AR_TKN_U32_MODULE_SYNC_SRC=262 through
AR_TKN_U32_MODULE_INV_EXT_BIT_CLK=276) are added.
MODULE_ID_AUDIO_IF_SINK (0x0700117C) and MODULE_ID_AUDIO_IF_SOURCE
(0x0700117D) are introduced in this patch.
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
include/uapi/sound/snd_ar_tokens.h | 58 ++++++++++++++++
sound/soc/qcom/qdsp6/audioreach.c | 97 ++++++++++++++++++++++++++
sound/soc/qcom/qdsp6/audioreach.h | 62 +++++++++++++++++
sound/soc/qcom/qdsp6/topology.c | 108 +++++++++++++++++++++++++++++
4 files changed, 325 insertions(+)
diff --git a/include/uapi/sound/snd_ar_tokens.h b/include/uapi/sound/snd_ar_tokens.h
index 6b8102eaa..355a1e629 100644
--- a/include/uapi/sound/snd_ar_tokens.h
+++ b/include/uapi/sound/snd_ar_tokens.h
@@ -168,6 +168,48 @@ enum ar_event_types {
* LOG_WAIT = 0,
* LOG_IMMEDIATELY = 1
*
+ * %AR_TKN_U32_MODULE_SYNC_SRC: Frame sync source
+ * 0 = external, 1 = internal
+ *
+ * %AR_TKN_U32_MODULE_CTRL_DATA_OUT_ENABLE: Enable data-out tri-state control
+ * 0 = disable, 1 = enable
+ *
+ * %AR_TKN_U32_MODULE_SLOT_MASK: Active TDM slot bitmask
+ *
+ * %AR_TKN_U32_MODULE_NSLOTS_PER_FRAME: Number of slots per TDM frame
+ *
+ * %AR_TKN_U32_MODULE_SLOT_WIDTH: Slot width in bits (16 or 32)
+ *
+ * %AR_TKN_U32_MODULE_SYNC_MODE: Frame sync mode
+ * 0 = short pulse, 1 = long pulse
+ *
+ * %AR_TKN_U32_MODULE_CTRL_INVERT_SYNC_PULSE: Invert frame sync pulse polarity
+ * 0 = normal, 1 = inverted
+ *
+ * %AR_TKN_U32_MODULE_CTRL_SYNC_DATA_DELAY: Data delay relative to frame sync
+ * 0 = no delay, 1 = one cycle delay
+ *
+ * %AR_TKN_U32_MODULE_INTF_MODE: Audio IF interface mode
+ * AUDIO_IF_INTF_MODE_TDM = 0,
+ * AUDIO_IF_INTF_MODE_PCM = 1,
+ * AUDIO_IF_INTF_MODE_I2S = 2
+ *
+ * %AR_TKN_U32_MODULE_QAIF_TYPE: QAIF hardware port type index
+ *
+ * %AR_TKN_U32_MODULE_ACTIVE_LANE_MASK: Active lane bitmask for multi-lane
+ *
+ * %AR_TKN_U32_MODULE_FRAME_SYNC_RATE: Frame sync rate in Hz
+ *
+ * %AR_TKN_U32_MODULE_BIT_CLK_TYPE: Bit clock type
+ * 0 = internal, 1 = external,
+ * 2 = skip (bypass bit clock enable)
+ *
+ * %AR_TKN_U32_MODULE_INV_INT_BIT_CLK: Invert internal bit clock
+ * 0 = normal, 1 = inverted
+ *
+ * %AR_TKN_U32_MODULE_INV_EXT_BIT_CLK: Invert external bit clock
+ * 0 = normal, 1 = inverted
+ *
* %AR_TKN_DAI_INDEX: dai index
*
*/
@@ -240,6 +282,22 @@ enum ar_event_types {
#define AR_TKN_U32_MODULE_LOG_TAP_POINT_ID 260
#define AR_TKN_U32_MODULE_LOG_MODE 261
+#define AR_TKN_U32_MODULE_SYNC_SRC 262
+#define AR_TKN_U32_MODULE_CTRL_DATA_OUT_ENABLE 263
+#define AR_TKN_U32_MODULE_SLOT_MASK 264
+#define AR_TKN_U32_MODULE_NSLOTS_PER_FRAME 265
+#define AR_TKN_U32_MODULE_SLOT_WIDTH 266
+#define AR_TKN_U32_MODULE_SYNC_MODE 267
+#define AR_TKN_U32_MODULE_CTRL_INVERT_SYNC_PULSE 268
+#define AR_TKN_U32_MODULE_CTRL_SYNC_DATA_DELAY 269
+#define AR_TKN_U32_MODULE_INTF_MODE 270
+#define AR_TKN_U32_MODULE_QAIF_TYPE 271
+#define AR_TKN_U32_MODULE_ACTIVE_LANE_MASK 272
+#define AR_TKN_U32_MODULE_FRAME_SYNC_RATE 273
+#define AR_TKN_U32_MODULE_BIT_CLK_TYPE 274
+#define AR_TKN_U32_MODULE_INV_INT_BIT_CLK 275
+#define AR_TKN_U32_MODULE_INV_EXT_BIT_CLK 276
+
#define SND_SOC_AR_TPLG_MODULE_CFG_TYPE 0x01001006
struct audioreach_module_priv_data {
__le32 size; /* size in bytes of the array, including all elements */
diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
index e6e9eb2e8..62140ce8e 100644
--- a/sound/soc/qcom/qdsp6/audioreach.c
+++ b/sound/soc/qcom/qdsp6/audioreach.c
@@ -152,6 +152,13 @@ struct apm_i2s_module_intf_cfg {
#define APM_I2S_INTF_CFG_PSIZE ALIGN(sizeof(struct apm_i2s_module_intf_cfg), 8)
+struct apm_audio_if_module_intf_cfg {
+ struct apm_module_param_data param_data;
+ struct param_id_audio_if_intf_cfg cfg;
+} __packed;
+
+#define APM_AUDIO_IF_INTF_CFG_PSIZE ALIGN(sizeof(struct apm_audio_if_module_intf_cfg), 8)
+
struct apm_module_hw_ep_mf_cfg {
struct apm_module_param_data param_data;
struct param_id_hw_ep_mf mf;
@@ -168,6 +175,13 @@ struct apm_module_frame_size_factor_cfg {
#define APM_FS_CFG_PSIZE ALIGN(sizeof(struct apm_module_frame_size_factor_cfg), 8)
+struct apm_module_hw_ep_frame_duration_cfg {
+ struct apm_module_param_data param_data;
+ struct param_id_hw_ep_frame_duration frame_duration;
+} __packed;
+
+#define APM_HW_EP_FRAME_DURATION_PSIZE ALIGN(sizeof(struct apm_module_hw_ep_frame_duration_cfg), 8)
+
struct apm_module_hw_ep_power_mode_cfg {
struct apm_module_param_data param_data;
struct param_id_hw_ep_power_mode_cfg power_mode;
@@ -1042,6 +1056,85 @@ static int audioreach_i2s_set_media_format(struct q6apm_graph *graph,
return q6apm_send_cmd_sync(graph->apm, pkt, 0);
}
+static int audioreach_audio_if_set_media_format(struct q6apm_graph *graph,
+ const struct audioreach_module *module,
+ const struct audioreach_module_config *cfg)
+{
+ struct apm_module_hw_ep_frame_duration_cfg *fd_cfg;
+ struct apm_module_param_data *param_data;
+ struct apm_audio_if_module_intf_cfg *intf_cfg;
+ struct apm_module_hw_ep_mf_cfg *hw_cfg;
+ int ic_sz = APM_AUDIO_IF_INTF_CFG_PSIZE;
+ int ep_sz = APM_HW_EP_CFG_PSIZE;
+ int fd_sz = APM_HW_EP_FRAME_DURATION_PSIZE;
+ int size = ic_sz + ep_sz + fd_sz;
+ /*
+ * A zero machine override means use the topology default; zero is not
+ * a valid override for these TDM slot fields.
+ */
+ u32 slot_mask = cfg->slot_mask ? cfg->slot_mask : module->slot_mask;
+ u16 nslots_per_frame = cfg->nslots_per_frame ?
+ (u16)cfg->nslots_per_frame : module->nslots_per_frame;
+ u16 slot_width = cfg->slot_width ? (u16)cfg->slot_width : module->slot_width;
+ void *p;
+
+ struct gpr_pkt *pkt __free(kfree) = audioreach_alloc_apm_cmd_pkt(size, APM_CMD_SET_CFG, 0);
+ if (IS_ERR(pkt))
+ return PTR_ERR(pkt);
+
+ p = (void *)pkt + GPR_HDR_SIZE + APM_CMD_HDR_SIZE;
+ intf_cfg = p;
+
+ param_data = &intf_cfg->param_data;
+ param_data->module_instance_id = module->instance_id;
+ param_data->error_code = 0;
+ param_data->param_id = PARAM_ID_AUDIO_IF_INTF_CFG;
+ param_data->param_size = ic_sz - APM_MODULE_PARAM_DATA_SIZE;
+ intf_cfg->cfg.qaif_type = module->qaif_type;
+ intf_cfg->cfg.intf_idx = (u16)module->hw_interface_idx;
+ intf_cfg->cfg.intf_mode = module->intf_mode;
+ intf_cfg->cfg.ctrl_data_out_enable = module->ctrl_data_out_enable;
+ intf_cfg->cfg.active_slot_mask = slot_mask;
+ intf_cfg->cfg.nslots_per_frame = nslots_per_frame;
+ intf_cfg->cfg.slot_width = slot_width;
+ intf_cfg->cfg.active_lane_mask = module->active_lane_mask;
+ intf_cfg->cfg.frame_sync_rate = module->frame_sync_rate;
+ intf_cfg->cfg.frame_sync_src = module->sync_src;
+ intf_cfg->cfg.frame_sync_mode = module->sync_mode;
+ intf_cfg->cfg.invert_frame_sync_pulse = module->ctrl_invert_sync_pulse;
+ intf_cfg->cfg.frame_sync_data_delay = module->ctrl_sync_data_delay;
+ intf_cfg->cfg.bit_clk_type = module->bit_clk_type;
+ intf_cfg->cfg.inv_int_bit_clk = module->inv_int_bit_clk;
+ intf_cfg->cfg.inv_ext_bit_clk = module->inv_ext_bit_clk;
+
+ p += ic_sz;
+ hw_cfg = p;
+ param_data = &hw_cfg->param_data;
+ param_data->module_instance_id = module->instance_id;
+ param_data->error_code = 0;
+ param_data->param_id = PARAM_ID_HW_EP_MF_CFG;
+ param_data->param_size = ep_sz - APM_MODULE_PARAM_DATA_SIZE;
+
+ hw_cfg->mf.sample_rate = cfg->sample_rate;
+ hw_cfg->mf.bit_width = cfg->bit_width;
+ hw_cfg->mf.num_channels = cfg->num_channels;
+ hw_cfg->mf.data_format = module->data_format;
+
+ p += ep_sz;
+ fd_cfg = p;
+ param_data = &fd_cfg->param_data;
+ param_data->module_instance_id = module->instance_id;
+ param_data->error_code = 0;
+ param_data->param_id = PARAM_ID_HW_EP_FRAME_DURATION;
+ param_data->param_size = fd_sz - APM_MODULE_PARAM_DATA_SIZE;
+ fd_cfg->frame_duration.frame_duration_in_us = AUDIO_IF_FRAME_DURATION_US;
+ fd_cfg->frame_duration.allow_frame_duration_normalization = 1;
+ fd_cfg->frame_duration.min_normalized_frame_dur_us = 1;
+ fd_cfg->frame_duration.max_normalized_frame_dur_us = 100000;
+
+ return q6apm_send_cmd_sync(graph->apm, pkt, 0);
+}
+
static int audioreach_logging_set_media_format(struct q6apm_graph *graph,
const struct audioreach_module *module)
{
@@ -1411,6 +1504,10 @@ int audioreach_set_media_format(struct q6apm_graph *graph,
if (!rc)
rc = audioreach_module_enable(graph, module, true);
break;
+ case MODULE_ID_AUDIO_IF_SOURCE:
+ case MODULE_ID_AUDIO_IF_SINK:
+ rc = audioreach_audio_if_set_media_format(graph, module, cfg);
+ break;
default:
rc = 0;
diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
index 62a2fd79b..1dc29ddfd 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -22,6 +22,8 @@ struct q6apm_graph;
#define MODULE_ID_PLACEHOLDER_DECODER 0x07001009
#define MODULE_ID_I2S_SINK 0x0700100A
#define MODULE_ID_I2S_SOURCE 0x0700100B
+#define MODULE_ID_AUDIO_IF_SINK 0x0700117C
+#define MODULE_ID_AUDIO_IF_SOURCE 0x0700117D
#define MODULE_ID_SAL 0x07001010
#define MODULE_ID_MFC 0x07001015
#define MODULE_ID_DATA_LOGGING 0x0700101A
@@ -544,6 +546,41 @@ struct param_id_i2s_intf_cfg {
#define PORT_ID_I2S_OUPUT 1
#define I2S_STACK_SIZE 2048
+#define PARAM_ID_AUDIO_IF_INTF_CFG 0x08001B11
+
+#define AUDIO_IF_INTF_MODE_TDM 0x0
+#define AUDIO_IF_INTF_MODE_PCM 0x1
+#define AUDIO_IF_INTF_MODE_I2S 0x2
+
+struct param_id_audio_if_intf_cfg {
+ uint16_t qaif_type;
+ uint16_t intf_idx;
+ uint16_t intf_mode;
+ uint16_t ctrl_data_out_enable;
+ uint32_t active_slot_mask;
+ uint16_t nslots_per_frame;
+ uint16_t slot_width;
+ uint32_t active_lane_mask;
+ uint32_t frame_sync_rate;
+ uint16_t frame_sync_src;
+ uint16_t frame_sync_mode;
+ uint16_t invert_frame_sync_pulse;
+ uint16_t frame_sync_data_delay;
+ uint16_t bit_clk_type;
+ uint8_t inv_int_bit_clk;
+ uint8_t inv_ext_bit_clk;
+} __packed;
+
+#define PARAM_ID_HW_EP_FRAME_DURATION 0x08001B2F
+#define AUDIO_IF_FRAME_DURATION_US 1000
+
+struct param_id_hw_ep_frame_duration {
+ uint32_t frame_duration_in_us;
+ uint32_t allow_frame_duration_normalization;
+ uint32_t min_normalized_frame_dur_us;
+ uint32_t max_normalized_frame_dur_us;
+} __packed;
+
#define PARAM_ID_DISPLAY_PORT_INTF_CFG 0x08001154
struct param_id_display_port_intf_cfg {
@@ -877,6 +914,28 @@ struct audioreach_module {
uint32_t data_format;
uint32_t hw_interface_type;
+ /* Audio IF module (TDM/PCM/I2S) */
+ /*
+ * uint32_t fields first to minimise intra-block padding;
+ * 2 bytes of trailing padding remain after inv_ext_bit_clk
+ * before the next uint32_t field (interleave_type).
+ */
+ uint32_t slot_mask;
+ uint32_t active_lane_mask;
+ uint32_t frame_sync_rate;
+ uint16_t qaif_type;
+ uint16_t sync_src;
+ uint16_t ctrl_data_out_enable;
+ uint16_t nslots_per_frame;
+ uint16_t slot_width;
+ uint16_t intf_mode;
+ uint16_t sync_mode;
+ uint16_t ctrl_invert_sync_pulse;
+ uint16_t ctrl_sync_data_delay;
+ uint16_t bit_clk_type;
+ uint8_t inv_int_bit_clk;
+ uint8_t inv_ext_bit_clk;
+
/* PCM module specific */
uint32_t interleave_type;
@@ -907,6 +966,9 @@ struct audioreach_module_config {
u32 channel_allocation;
u32 sd_line_mask;
int fmt;
+ u32 slot_mask;
+ u16 nslots_per_frame;
+ u16 slot_width;
struct snd_codec codec;
u8 channel_map[AR_PCM_MAX_NUM_CHANNEL];
};
diff --git a/sound/soc/qcom/qdsp6/topology.c b/sound/soc/qcom/qdsp6/topology.c
index 1f69fba6d..2ae7ac3d2 100644
--- a/sound/soc/qcom/qdsp6/topology.c
+++ b/sound/soc/qcom/qdsp6/topology.c
@@ -753,6 +753,108 @@ static int audioreach_widget_i2s_module_load(struct audioreach_module *mod,
return 0;
}
+static int audioreach_widget_audio_if_module_load(struct audioreach_module *mod,
+ const struct snd_soc_tplg_vendor_array *mod_array)
+{
+ const struct snd_soc_tplg_vendor_value_elem *mod_elem;
+ int tkn_count = 0;
+ u32 val;
+
+ mod_elem = mod_array->value;
+
+ while (tkn_count < le32_to_cpu(mod_array->num_elems)) {
+ val = le32_to_cpu(mod_elem->value);
+ switch (le32_to_cpu(mod_elem->token)) {
+ case AR_TKN_U32_MODULE_HW_IF_IDX:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->hw_interface_idx = val;
+ break;
+ case AR_TKN_U32_MODULE_FMT_DATA:
+ mod->data_format = val;
+ break;
+ case AR_TKN_U32_MODULE_HW_IF_TYPE:
+ mod->hw_interface_type = val;
+ break;
+ case AR_TKN_U32_MODULE_SYNC_SRC:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->sync_src = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_CTRL_DATA_OUT_ENABLE:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->ctrl_data_out_enable = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_SLOT_MASK:
+ mod->slot_mask = val;
+ break;
+ case AR_TKN_U32_MODULE_NSLOTS_PER_FRAME:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->nslots_per_frame = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_SLOT_WIDTH:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->slot_width = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_INTF_MODE:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->intf_mode = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_SYNC_MODE:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->sync_mode = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_CTRL_INVERT_SYNC_PULSE:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->ctrl_invert_sync_pulse = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_CTRL_SYNC_DATA_DELAY:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->ctrl_sync_data_delay = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_QAIF_TYPE:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->qaif_type = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_ACTIVE_LANE_MASK:
+ mod->active_lane_mask = val;
+ break;
+ case AR_TKN_U32_MODULE_FRAME_SYNC_RATE:
+ mod->frame_sync_rate = val;
+ break;
+ case AR_TKN_U32_MODULE_BIT_CLK_TYPE:
+ if (val > U16_MAX)
+ return -EINVAL;
+ mod->bit_clk_type = (u16)val;
+ break;
+ case AR_TKN_U32_MODULE_INV_INT_BIT_CLK:
+ if (val > U8_MAX)
+ return -EINVAL;
+ mod->inv_int_bit_clk = (u8)val;
+ break;
+ case AR_TKN_U32_MODULE_INV_EXT_BIT_CLK:
+ if (val > U8_MAX)
+ return -EINVAL;
+ mod->inv_ext_bit_clk = (u8)val;
+ break;
+ default:
+ break;
+ }
+ tkn_count++;
+ mod_elem++;
+ }
+
+ return 0;
+}
+
static int audioreach_widget_dp_module_load(struct audioreach_module *mod,
const struct snd_soc_tplg_vendor_array *mod_array)
{
@@ -806,6 +908,12 @@ static int audioreach_widget_load_buffer(struct snd_soc_component *component,
case MODULE_ID_I2S_SOURCE:
audioreach_widget_i2s_module_load(mod, mod_array);
break;
+ case MODULE_ID_AUDIO_IF_SINK:
+ case MODULE_ID_AUDIO_IF_SOURCE:
+ ret = audioreach_widget_audio_if_module_load(mod, mod_array);
+ if (ret)
+ return ret;
+ break;
case MODULE_ID_DISPLAY_PORT_SINK:
audioreach_widget_dp_module_load(mod, mod_array);
break;
--
2.34.1
^ permalink raw reply related
* [PATCH v1 4/6] ASoC: qcom: q6prm: add Audio IF clock IDs
From: Prasad Kumpatla @ 2026-06-10 15:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla
In-Reply-To: <20260610154517.134570-1-prasad.kumpatla@oss.qualcomm.com>
Add the q6prm clock table entries and internal DSP clock IDs for LPASS
Audio IF backend clocks.
The public binding IDs map to q6prm DSP clock IDs starting at 0x500 for
Audio IF0 IBIT/EBIT. Add the internal definitions and register all Audio
IF IBIT and EBIT clocks so machine drivers can request them through the
APM clock controller.
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
sound/soc/qcom/qdsp6/q6prm-clocks.c | 28 ++++++++++++++++++++++++++++
sound/soc/qcom/qdsp6/q6prm.h | 29 +++++++++++++++++++++++++++++
2 files changed, 57 insertions(+)
diff --git a/sound/soc/qcom/qdsp6/q6prm-clocks.c b/sound/soc/qcom/qdsp6/q6prm-clocks.c
index 51b131fa9..b6755da6a 100644
--- a/sound/soc/qcom/qdsp6/q6prm-clocks.c
+++ b/sound/soc/qcom/qdsp6/q6prm-clocks.c
@@ -64,6 +64,34 @@ static const struct q6dsp_clk_init q6prm_clks[] = {
Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_MCLK),
Q6PRM_CLK(LPASS_CLK_ID_WSA2_CORE_TX_2X_MCLK),
Q6PRM_CLK(LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF0_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF0_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF1_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF1_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF2_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF2_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF3_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF3_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF4_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF4_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF5_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF5_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF6_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF6_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF7_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF7_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF8_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF8_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF9_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF9_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF10_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF10_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF11_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF11_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF12_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_INTF12_EBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_VA_INTF0_IBIT),
+ Q6PRM_CLK(LPASS_CLK_ID_AUD_VA_INTF0_EBIT),
Q6DSP_VOTE_CLK(LPASS_HW_MACRO_VOTE, Q6PRM_HW_CORE_ID_LPASS,
"LPASS_HW_MACRO"),
Q6DSP_VOTE_CLK(LPASS_HW_DCODEC_VOTE, Q6PRM_HW_CORE_ID_DCODEC,
diff --git a/sound/soc/qcom/qdsp6/q6prm.h b/sound/soc/qcom/qdsp6/q6prm.h
index 7b751486c..c1838d80c 100644
--- a/sound/soc/qcom/qdsp6/q6prm.h
+++ b/sound/soc/qcom/qdsp6/q6prm.h
@@ -98,6 +98,35 @@
/* Clock ID for RX CORE MCLK2 2X MCLK */
#define Q6PRM_LPASS_CLK_ID_RX_CORE_MCLK2_2X_MCLK 0x318
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF0_IBIT 0x500
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF0_EBIT 0x501
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF1_IBIT 0x502
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF1_EBIT 0x503
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF2_IBIT 0x504
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF2_EBIT 0x505
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF3_IBIT 0x506
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF3_EBIT 0x507
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF4_IBIT 0x508
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF4_EBIT 0x509
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF5_IBIT 0x50A
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF5_EBIT 0x50B
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF6_IBIT 0x50C
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF6_EBIT 0x50D
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF7_IBIT 0x50E
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF7_EBIT 0x50F
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF8_IBIT 0x510
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF8_EBIT 0x511
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF9_IBIT 0x512
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF9_EBIT 0x513
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF10_IBIT 0x514
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF10_EBIT 0x515
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF11_IBIT 0x516
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF11_EBIT 0x517
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF12_IBIT 0x518
+#define Q6PRM_LPASS_CLK_ID_AUD_INTF12_EBIT 0x519
+#define Q6PRM_LPASS_CLK_ID_AUD_VA_INTF0_IBIT 0x550
+#define Q6PRM_LPASS_CLK_ID_AUD_VA_INTF0_EBIT 0x551
+
#define Q6PRM_LPASS_CLK_SRC_INTERNAL 1
#define Q6PRM_LPASS_CLK_ROOT_DEFAULT 0
#define Q6PRM_HW_CORE_ID_LPASS 1
--
2.34.1
^ permalink raw reply related
* [PATCH v1 5/6] ASoC: qcom: common: add DAI-node TDM slot helpers
From: Prasad Kumpatla @ 2026-06-10 15:45 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla
Cc: linux-arm-msm, linux-sound, devicetree, linux-kernel,
Prasad Kumpatla
In-Reply-To: <20260610154517.134570-1-prasad.kumpatla@oss.qualcomm.com>
Add common helpers to parse standard dai-tdm-slot-* properties from the
CPU and codec child nodes of a backend DAI link and apply the result to
the active DAIs.
QCOM machine drivers already use qcom_snd_parse_of() to build links from
DT, but they lacked a shared helper to translate endpoint TDM properties
into snd_soc_dai_set_tdm_slot() calls. Boards therefore had to carry ad
hoc parsing or rely on non-standard DT properties.
The helpers parse endpoint masks, validate the shared slot count and
slot width, and program CPU and codec DAIs with the resulting slot
configuration. A cfg-based apply helper is provided for callers that
already parsed the DT data and want to avoid a second DT traversal.
Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
---
sound/soc/qcom/common.c | 164 ++++++++++++++++++++++++++++++++++++++++
sound/soc/qcom/common.h | 14 ++++
2 files changed, 178 insertions(+)
diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c
index cf1f3a767..5ca720ecf 100644
--- a/sound/soc/qcom/common.c
+++ b/sound/soc/qcom/common.c
@@ -23,6 +23,170 @@ static const struct snd_soc_dapm_widget qcom_jack_snd_widgets[] = {
SND_SOC_DAPM_SPK("DP7 Jack", NULL),
};
+static struct device_node *qcom_snd_get_link_node(struct snd_soc_pcm_runtime *rtd)
+{
+ struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+ struct snd_soc_card *card = rtd->card;
+ struct device_node *np;
+ struct device_node *cpu_np;
+ struct of_phandle_args args;
+ int ret;
+
+ if (!card->dev || !card->dev->of_node)
+ return NULL;
+
+ for_each_available_child_of_node(card->dev->of_node, np) {
+ cpu_np = of_get_child_by_name(np, "cpu");
+ if (!cpu_np)
+ continue;
+
+ ret = of_parse_phandle_with_args(cpu_np, "sound-dai", "#sound-dai-cells", 0, &args);
+ of_node_put(cpu_np);
+ if (ret)
+ continue;
+
+ if (args.np == rtd->dai_link->cpus[0].of_node &&
+ args.args_count == 1 && args.args[0] == cpu_dai->id) {
+ of_node_put(args.np);
+ return np;
+ }
+
+ of_node_put(args.np);
+ }
+
+ return NULL;
+}
+
+static int qcom_snd_parse_tdm_slot(struct device_node *np,
+ struct qcom_snd_tdm_slot_cfg *cfg)
+{
+ memset(cfg, 0, sizeof(*cfg));
+
+ return snd_soc_of_parse_tdm_slot(np, &cfg->tx_mask, &cfg->rx_mask,
+ &cfg->slots, &cfg->slot_width);
+}
+
+static int qcom_snd_normalize_tdm_slots(struct qcom_snd_tdm_slot_cfg *cpu_cfg,
+ struct qcom_snd_tdm_slot_cfg *codec_cfg)
+{
+ unsigned int slots;
+ unsigned int slot_width;
+
+ if (cpu_cfg->slots && codec_cfg->slots && cpu_cfg->slots != codec_cfg->slots)
+ return -EINVAL;
+
+ if (cpu_cfg->slot_width && codec_cfg->slot_width &&
+ cpu_cfg->slot_width != codec_cfg->slot_width)
+ return -EINVAL;
+
+ slots = cpu_cfg->slots ?: codec_cfg->slots;
+ if (!slots)
+ return 0;
+
+ slot_width = cpu_cfg->slot_width ?: codec_cfg->slot_width;
+ if (!slot_width)
+ return -EINVAL;
+
+ cpu_cfg->slots = slots;
+ codec_cfg->slots = slots;
+ cpu_cfg->slot_width = slot_width;
+ codec_cfg->slot_width = slot_width;
+
+ return 0;
+}
+
+static int qcom_snd_parse_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd,
+ struct qcom_snd_tdm_slot_cfg *cpu_cfg,
+ struct qcom_snd_tdm_slot_cfg *codec_cfg)
+{
+ struct device_node *link_np;
+ struct device_node *cpu_np = NULL;
+ struct device_node *codec_np = NULL;
+ int ret;
+
+ link_np = qcom_snd_get_link_node(rtd);
+ if (!link_np)
+ return -EINVAL;
+
+ cpu_np = of_get_child_by_name(link_np, "cpu");
+ codec_np = of_get_child_by_name(link_np, "codec");
+ if (!cpu_np || !codec_np) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = qcom_snd_parse_tdm_slot(cpu_np, cpu_cfg);
+ if (ret)
+ goto out;
+
+ ret = qcom_snd_parse_tdm_slot(codec_np, codec_cfg);
+out:
+ of_node_put(codec_np);
+ of_node_put(cpu_np);
+ of_node_put(link_np);
+
+ return ret;
+}
+
+int qcom_snd_get_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd,
+ struct qcom_snd_tdm_slot_cfg *cpu_cfg,
+ struct qcom_snd_tdm_slot_cfg *codec_cfg)
+{
+ int ret;
+
+ ret = qcom_snd_parse_dai_tdm_slots(rtd, cpu_cfg, codec_cfg);
+ if (ret)
+ return ret;
+
+ return qcom_snd_normalize_tdm_slots(cpu_cfg, codec_cfg);
+}
+EXPORT_SYMBOL_GPL(qcom_snd_get_dai_tdm_slots);
+
+int qcom_snd_apply_dai_tdm_slots_cfg(struct snd_soc_pcm_runtime *rtd,
+ const struct qcom_snd_tdm_slot_cfg *cpu_cfg,
+ const struct qcom_snd_tdm_slot_cfg *codec_cfg)
+{
+ struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+ struct snd_soc_dai *codec_dai;
+ int i;
+ int ret;
+
+ if (!cpu_cfg->slots)
+ return 0;
+
+ ret = snd_soc_dai_set_tdm_slot(cpu_dai, cpu_cfg->tx_mask, cpu_cfg->rx_mask,
+ cpu_cfg->slots, cpu_cfg->slot_width);
+ if (ret)
+ return ret;
+
+ for_each_rtd_codec_dais(rtd, i, codec_dai) {
+ ret = snd_soc_dai_set_tdm_slot(codec_dai,
+ codec_cfg->tx_mask,
+ codec_cfg->rx_mask,
+ codec_cfg->slots,
+ codec_cfg->slot_width);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_snd_apply_dai_tdm_slots_cfg);
+
+int qcom_snd_apply_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd)
+{
+ struct qcom_snd_tdm_slot_cfg cpu_cfg;
+ struct qcom_snd_tdm_slot_cfg codec_cfg;
+ int ret;
+
+ ret = qcom_snd_get_dai_tdm_slots(rtd, &cpu_cfg, &codec_cfg);
+ if (ret)
+ return ret == -EINVAL ? 0 : ret;
+
+ return qcom_snd_apply_dai_tdm_slots_cfg(rtd, &cpu_cfg, &codec_cfg);
+}
+EXPORT_SYMBOL_GPL(qcom_snd_apply_dai_tdm_slots);
+
int qcom_snd_parse_of(struct snd_soc_card *card)
{
struct device_node *np;
diff --git a/sound/soc/qcom/common.h b/sound/soc/qcom/common.h
index ee6662885..02b24caec 100644
--- a/sound/soc/qcom/common.h
+++ b/sound/soc/qcom/common.h
@@ -9,7 +9,21 @@
#define LPASS_MAX_PORT (SENARY_MI2S_TX + 1)
+struct qcom_snd_tdm_slot_cfg {
+ unsigned int tx_mask;
+ unsigned int rx_mask;
+ unsigned int slots;
+ unsigned int slot_width;
+};
+
int qcom_snd_parse_of(struct snd_soc_card *card);
+int qcom_snd_get_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd,
+ struct qcom_snd_tdm_slot_cfg *cpu_cfg,
+ struct qcom_snd_tdm_slot_cfg *codec_cfg);
+int qcom_snd_apply_dai_tdm_slots_cfg(struct snd_soc_pcm_runtime *rtd,
+ const struct qcom_snd_tdm_slot_cfg *cpu_cfg,
+ const struct qcom_snd_tdm_slot_cfg *codec_cfg);
+int qcom_snd_apply_dai_tdm_slots(struct snd_soc_pcm_runtime *rtd);
int qcom_snd_wcd_jack_setup(struct snd_soc_pcm_runtime *rtd,
struct snd_soc_jack *jack, bool *jack_setup);
int qcom_snd_dp_jack_setup(struct snd_soc_pcm_runtime *rtd,
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v6 00/11] ASoC: renesas: fsi: Fix system hang by adding SPU clock
From: Bui Duc Phuc @ 2026-06-10 15:49 UTC (permalink / raw)
To: Mark Brown
Cc: Kuninori Morimoto, Geert Uytterhoeven, Liam Girdwood, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Jaroslav Kysela,
Takashi Iwai, linux-sound, linux-renesas-soc, devicetree,
linux-kernel
In-Reply-To: <178104804456.232889.7203992010119048881.b4-ty@b4>
Hi Mark,
> Applied to
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.2
>
Thank you for your review and for applying the patch.
Best regards,
Phuc
^ 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