* Re: [PATCH 07/12] rtc: rzn1: fix alarm range check truncation on 32-bit systems
From: Wolfram Sang @ 2026-06-23 21:08 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <CA+V-a8u2wt6623mYjhipOvJPo4va+bXs3qirQewocFr2QmUFhA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 423 bytes --]
> Can you please share the commands you tried, I'll try and replicate it
> on my side.
Sorry, can't give you the commands, just from my head: I tried to set an
alarm more than a week in the future, and the alarm was set to the next
day. But I was in a hurry, maybe I overlooked something, because that
handling used to work in the past IIRC. I can return to this topic on
Friday earliest, sadly. Maybe next week only...
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2 0/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: David Lechner @ 2026-06-23 21:04 UTC (permalink / raw)
To: Md Shofiqul Islam, linux-iio
Cc: jic23, nuno.sa, robh, krzk+dt, conor+dt, devicetree,
joshua.crofts1
In-Reply-To: <20260623155556.13701-1-shofiqtest@gmail.com>
On 6/23/26 10:55 AM, Md Shofiqul Islam wrote:
> The MAX86150 (Maxim/Analog Devices) combines two PPG optical channels
> (Red/IR LED) and one ECG biopotential channel in a single I2C device.
> Data is produced at up to 3200 Hz and buffered in a 32-entry hardware
> FIFO with a configurable almost-full interrupt.
>
> This series adds:
> 1. DT binding YAML schema for maxim,max86150
> 2. IIO driver with triggered buffer support
> 3. MAINTAINERS entry
>
> Changes in v2 (addressing Sashiko AI review, Joshua Crofts, Krzysztof Kozlowski):
Please don't send revisions in reply to previous series. It breaks
many workflows and tools that expect each series to be it's own
message thread.
Also, when submitting a series for a new driver, please wait at
least one week to gather feedback before sending the next revision.
^ permalink raw reply
* Re: [PATCH 8/9] drm/rockchip: dw_hdmi_qp: Enable YUV420 output format
From: sashiko-bot @ 2026-06-23 21:03 UTC (permalink / raw)
To: Cristian Ciocaltea; +Cc: dri-devel, robh, conor+dt, devicetree
In-Reply-To: <20260617-dw-hdmi-qp-yuv-v1-8-a665cfd06d7d@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Hardware State Mutation in Atomic Check
--
commit 88f697e7fc21ab15ca1c009f0f830ab90e6ec4e7
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
drm/rockchip: dw_hdmi_qp: Enable YUV420 output format
Both RK3576 and RK3588 SoCs are capable of driving the YUV420 output
color format, and the required bus-format handling and VOP2 support are
already in place. Advertise it via the platform supported formats so
the HDMI core can select it.
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> index 3a1c027aa90bb..e29522afc6f05 100644
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> @@ -612,7 +612,8 @@ static int dw_hdmi_qp_rockchip_bind(struct device *dev, struct device *master,
>
> plat_data.supported_formats = BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) |
> BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) |
> - BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422);
> + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) |
> + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420);
[Severity: High]
This is a pre-existing issue, but I noticed that adding new formats might
exercise the atomic check path more frequently.
Looking at dw_hdmi_qp_rockchip_encoder_atomic_check() in this same file,
it appears to mutate hardware state during the atomic validation phase:
drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c:dw_hdmi_qp_rockchip_encoder_atomic_check() {
...
ret = phy_configure(hdmi->phy, &phy_cfg);
if (!ret) {
hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
s->output_type = DRM_MODE_CONNECTOR_HDMIA;
s->output_bpc = conn_state->hdmi.output_bpc;
s->bus_format = ingest_fmt;
} else {
...
}
And in the PHY driver, phy_configure() mutates global state:
drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c:rk_hdptx_phy_configure() {
...
ret = rk_hdptx_phy_verify_hdmi_config(hdptx, &opts->hdmi, &hdptx->hdmi_cfg);
if (ret) {
dev_err(hdptx->dev, "invalid hdmi params for phy configure\n");
} else {
hdptx->restrict_rate_change = true;
...
}
Does this sequence cause problems when a userspace application (like a display
compositor) submits a test-only commit using the DRM_MODE_ATOMIC_TEST_ONLY
flag?
Since the atomic check runs for test-only commits, won't this corrupt the
active driver state and potentially break subsequent real commits if they skip
necessary hardware configurations due to the cached state already matching?
>
> encoder = &hdmi->encoder.encoder;
> encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=8
^ permalink raw reply
* RE: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
From: Selvamani Rajagopal @ 2026-06-23 21:01 UTC (permalink / raw)
To: Conor Dooley
Cc: Guenter Roeck, Jonathan Corbet, Shuah Khan, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-hwmon@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <20260623-anybody-gutter-e6ca04f53bdb@spud>
> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Subject: Re: [PATCH 2/3] dt-bindings: hwmon: pmbus: Support for onsemi's FD5121
>
>
> > +
> > +title: onsemi's multi-phase digital controllers
>
> Can someone explain to me what a "digital controller" actually is?
> Seems very generi and that a word may have been left out, were it not
> for the fact that this wording is used several times in the patch.
>
Thanks for reviewing.
According to me, "digital controller" means the controller uses digital circuits to implement
the features and functionality. We can remove "digital" and keep only controller. It won't make any
difference for Linux documentation.
> > +
> > + enum:
> > + - onnn,fd5121
> > + - onnn,fd5123
> > + - onnn,fd5125
>
> Your /OF/ match data in your driver suggests that you intended to permit
> fallback compatibles here?
Agree. Sorry about the discrepancy. Will fix it.
>
> |+#ifdef CONFIG_OF
> |+static const struct of_device_id fd5121_of_match[] = {
> |+ { .compatible = "onnn,fd5121" },
> |+ { }
> |+};
> |+MODULE_DEVICE_TABLE(of, fd5121_of_match);
> |+#endif
>
^ permalink raw reply
* Re: [PATCH v3 10/10] drm/bridge: synopsys: dw-dp: Add audio support
From: sashiko-bot @ 2026-06-23 21:00 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: dri-devel, robh, devicetree, conor+dt
In-Reply-To: <20260612-synopsys-dw-dp-improvements-v3-10-dc61e6352508@collabora.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] The return value of `pm_runtime_get_sync()` is ignored in `dw_dp_audio_startup()`, violating PM runtime API contracts and risking hardware access while suspended.
- [High] Missing error checks on `clk_prepare_enable()` in `dw_dp_audio_prepare()` lead to hardware access failures and clock reference count underflows.
- [High] SDP register bank slots are permanently leaked because `dw_dp_audio_unprepare()` does not free them.
- [Medium] Prematurely updating `dp->audio_interface` before hardware validation causes an unbalanced clock disable in the teardown path on error.
- [Medium] `dw_dp_audio_prepare()` explicitly rejects valid intermediate channel counts, breaking multi-channel playback.
Pre-existing issues:
- [High] Concurrent accesses to the `dp->sdp_reg_bank` bitmap between DRM and ALSA threads lack locking, leading to SDP register corruption.
--
commit 91c310c87d4ddad349da89d89896e7fa9319465d
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
drm/bridge: synopsys: dw-dp: Add audio support
Implement audio support for the Synopsys DesignWare DisplayPort
controller.
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
index 7f4f36c614844..f1946f2c945da 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
[ ... ]
> +static int dw_dp_audio_startup(struct drm_bridge *bridge,
> + struct drm_connector *connector)
> +{
> + struct dw_dp *dp = bridge_to_dp(bridge);
> +
> + dev_dbg(dp->dev, "audio startup\n");
> + pm_runtime_get_sync(dp->dev);
[Severity: High]
Does this code need to check the return value of pm_runtime_get_sync()?
If the power domain or clock fails to resume, it returns a negative error
code. Ignoring this might lead to accessing suspended hardware registers when
ALSA triggers startup.
> +
> + return 0;
> +}
> +
> +static void dw_dp_audio_unprepare(struct drm_bridge *bridge,
> + struct drm_connector *connector)
> +{
> + struct dw_dp *dp = bridge_to_dp(bridge);
> +
> + /* Disable all audio streams */
> + regmap_update_bits(dp->regmap, DW_DP_AUD_CONFIG1, AUDIO_DATA_IN_EN,
> + FIELD_PREP(AUDIO_DATA_IN_EN, 0));
> +
> + if (dp->audio_interface == DW_DP_AUDIO_SPDIF)
> + clk_disable_unprepare(dp->spdif_clk);
> + else if (dp->audio_interface == DW_DP_AUDIO_I2S)
> + clk_disable_unprepare(dp->i2s_clk);
> +
> + dp->audio_interface = DW_DP_AUDIO_UNUSED;
> +}
[Severity: High]
Does dw_dp_audio_unprepare() leak SDP register bank slots?
dw_dp_audio_prepare() allocates a new SDP slot by calling dw_dp_send_sdp(),
which sets a bit in dp->sdp_reg_bank. However, dw_dp_audio_unprepare() never
clears the allocated bit.
Could this exhaust the register bank slots if userspace starts and stops audio
multiple times?
[ ... ]
> +static int dw_dp_audio_prepare(struct drm_bridge *bridge,
> + struct drm_connector *connector,
> + struct hdmi_codec_daifmt *daifmt,
> + struct hdmi_codec_params *params)
> +{
> + struct dw_dp *dp = bridge_to_dp(bridge);
> + u8 audio_data_in_en, supported_audio_interfaces;
> + u32 cfg1;
> + int ret;
> +
> + /*
> + * prepare might be called multiple times, so release the clocks
> + * from previous calls to keep the calls in balance.
> + */
> + if (dp->audio_interface != DW_DP_AUDIO_UNUSED)
> + dw_dp_audio_unprepare(bridge, connector);
> +
> + dp->audio_channels = params->cea.channels;
> + switch (params->cea.channels) {
> + case 1:
> + case 2:
> + audio_data_in_en = AUDIO_DATA_IN_EN_CHANNEL12;
> + break;
> + case 8:
> + audio_data_in_en = AUDIO_DATA_IN_EN_CHANNEL12 |
> + AUDIO_DATA_IN_EN_CHANNEL34 |
> + AUDIO_DATA_IN_EN_CHANNEL56 |
> + AUDIO_DATA_IN_EN_CHANNEL78;
> + break;
> + default:
> + dev_err(dp->dev, "invalid audio channels %d\n", dp->audio_channels);
> + return -EINVAL;
> + }
[Severity: Medium]
Will this explicit rejection break valid multi-channel playback?
The driver sets bridge->hdmi_audio_max_i2s_playback_channels = 8 indicating
hardware support, but this switch statement returns -EINVAL for intermediate
channel counts like 4 or 6. Is it intended to reject valid multi-channel
audio setups?
> +
> + switch (daifmt->fmt) {
> + case HDMI_SPDIF:
> + dp->audio_interface = DW_DP_AUDIO_SPDIF;
> + break;
> + case HDMI_I2S:
> + /*
> + * It is recommended to use SPDIF instead of I2S, since I2S mode requires
> + * manually inserting PCUV control bits from userspace and this is done
> + * automatically in hardware for SPDIF mode.
> + */
> + dp->audio_interface = DW_DP_AUDIO_I2S;
> + break;
> + default:
> + dev_err(dp->dev, "invalid DAI format %d\n", daifmt->fmt);
> + return -EINVAL;
> + }
> +
> + regmap_read(dp->regmap, DW_DP_CONFIG_REG1, &cfg1);
> + supported_audio_interfaces = FIELD_GET(AUDIO_SELECT, cfg1);
> +
> + if (supported_audio_interfaces != DW_DP_AUDIO_I2S_AND_SPDIF &&
> + supported_audio_interfaces != dp->audio_interface) {
> + dev_err(dp->dev, "unsupported DAI %d\n", daifmt->fmt);
> + return -EINVAL;
> + }
[Severity: Medium]
Could this leave dp->audio_interface in a corrupted state on failure?
If an unsupported DAI format is provided, dp->audio_interface is updated prior
to the validation check. If this function then returns -EINVAL, the ALSA core
later calls dw_dp_audio_shutdown() -> dw_dp_audio_unprepare(), which acts on
the corrupted state and attempts to disable a clock that was never enabled.
> +
> + clk_prepare_enable(dp->spdif_clk);
> + clk_prepare_enable(dp->i2s_clk);
[Severity: High]
Does this code need to check the return values of clk_prepare_enable()?
If clk_prepare_enable() fails, the driver continues to program hardware
registers, which could lead to hardware lockups. Furthermore, the teardown
path unconditionally disables the failed clock, causing reference count
underflows.
[ ... ]
> + ret = dw_dp_audio_infoframe_send(dp);
> + if (ret < 0)
> + dev_err(dp->dev, "failed to send audio infoframe\n");
[Severity: High]
This isn't a bug introduced by this patch, but adding audio support introduces
a concurrent access path that makes a pre-existing race possible.
dw_dp_audio_infoframe_send() calls dw_dp_send_sdp(), which allocates a slot
using non-atomic test and set:
nr = find_first_zero_bit(dp->sdp_reg_bank, SDP_REG_BANK_SIZE);
if (nr < SDP_REG_BANK_SIZE)
set_bit(nr, dp->sdp_reg_bank);
Can ALSA audio preparation running concurrently with DRM display updates lead
to slot allocation collisions and SDP hardware register corruption?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-synopsys-dw-dp-improvements-v3-0-dc61e6352508@collabora.com?part=10
^ permalink raw reply
* Re: [PATCH v4 2/3] iio: health: add MAX86150 ECG and PPG biosensor driver
From: Andy Shevchenko @ 2026-06-23 20:52 UTC (permalink / raw)
To: Md Shofiqul Islam
Cc: linux-iio, jic23, dlechner, nuno.sa, andy, robh, krzk+dt,
conor+dt, lars, devicetree, linux-kernel
In-Reply-To: <20260623174600.17100-3-shofiqtest@gmail.com>
On Tue, Jun 23, 2026 at 08:45:59PM +0300, Md Shofiqul Islam wrote:
> The MAX86150 (Maxim/Analog Devices) combines two PPG optical channels
> (Red/IR LED) and one ECG biopotential channel in a single I2C device with
> a 32-entry hardware FIFO.
>
> Driver features:
> - Three IIO channels: in_intensityred_raw, in_intensityir_raw,
> in_voltage0_raw
> - Triggered buffer path driven by the FIFO almost-full interrupt;
> set_trigger_state enables/disables the interrupt only while the buffer
> is active and FIFO is flushed before capture starts
> - validate_trigger = iio_trigger_validate_own_device prevents
> incompatible external triggers from being attached
> - IRQ trigger type taken from irq_get_trigger_type() to honour DT
> configuration; falls back to falling-edge if unspecified
> - fifo_raw burst-read buffer is heap-allocated in struct max86150_data
> and aligned to ARCH_DMA_MINALIGN to satisfy DMA mapping requirements
> of I2C host controllers that use DMA for burst reads
> - SYS_SHDN asserted at end of chip_init so LED drivers draw no current
> when capture is inactive; set_trigger_state() and read_raw() wake and
> sleep the device on demand
> - Per-sample timestamps anchored to the A_FULL IRQ capture time: the
> sample at index (A_FULL_SAMPLES - 1) maps to pf->timestamp, samples
> accumulated after the IRQ receive future timestamps, eliminating
> load-dependent jitter
> - FIFO empty vs exactly-full disambiguation: when wr_ptr == rd_ptr with
> OVF_COUNTER == 0, the A_FULL status bit distinguishes a pointer
> wrap-around (full) from a genuinely empty FIFO
> - devm_regulator_get_enable_optional() for the two optional supplies;
> -ENODEV is treated as success (supply absent, not an error)
> - Powerdown devm action disables interrupts and asserts SYS_SHDN
...
> endmenu
>
> +
Single blank line is enough.
> +config MAX86150
...
IWYU, please! (The list of missing headers below may not be comprehensive.)
+ array_size.h
> +#include <linux/bitfield.h>
+ bitops.h // GENMASK(), sign_extend32()
> +#include <linux/delay.h>
+ dev_printk.h // dev_err_probe()
+ device/devres.h // devm_add_action_or_reset()
+ err.h // IS_ERR(), -ENOMEM, ...
> +#include <linux/i2c.h>
> +#include <linux/irq.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
Can we move this group out...
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
+ types.h // uXX, sXX
> +
...to be here?
...
> +/* Scan element indices */
> +enum max86150_scan_idx {
> + MAX86150_IDX_PPG_RED = 0,
> + MAX86150_IDX_PPG_IR = 1,
> + MAX86150_IDX_ECG = 2,
> + MAX86150_IDX_TS,
Why no value for TS? Or why values for the rest? I assume it's HW/SW cases?
> +};
> +/**
> + * struct max86150_data - driver private state
> + * @regmap: register map for this device
> + * @dev: parent device (for dev_err logging)
> + * @trig: IIO hardware trigger backed by the device interrupt line
> + * @sample_period_ns: sample period in nanoseconds (set from configured rate)
> + * @fifo_raw: DMA-safe buffer for regmap_noinc_read() FIFO bursts;
> + * must be in struct (heap) not on the stack to satisfy
> + * DMA mapping requirements of some I2C host controllers
> + * @buf: IIO push buffer sized for worst-case (all 3 channels
> + * active): 3 x s32 (12 bytes) + 4-byte pad + s64
> + * timestamp = 24 bytes. __aligned(8) satisfies
> + * iio_push_to_buffers_with_timestamp().
> + */
> +struct max86150_data {
> + struct regmap *regmap;
> + struct device *dev;
Is regmap device and dev is the same? If so, drop one and derive the other.
> + struct iio_trigger *trig;
> + u32 sample_period_ns;
> + u8 fifo_raw[MAX86150_SAMPLE_BYTES] __aligned(ARCH_DMA_MINALIGN);
> + s32 buf[6] __aligned(8);
We have a macro for these. (For alignment.)
> +};
...
> +static int max86150_read_one_sample(struct max86150_data *data,
> + u32 *ppg_red, u32 *ppg_ir, s32 *ecg)
> +{
> + int ret;
> +
> + /*
> + * Use data->fifo_raw (heap memory) not a local array so the buffer is
> + * DMA-mappable for I2C host controllers that use DMA for burst reads.
> + */
> + ret = regmap_noinc_read(data->regmap, MAX86150_REG_FIFO_DATA,
> + data->fifo_raw, sizeof(data->fifo_raw));
> + if (ret)
> + return ret;
> +
> + /* Bytes [0..2]: PPG Red - 19-bit value in bits [18:0] */
> + *ppg_red = (u32)(data->fifo_raw[0] & 0x07) << 16 |
> + (u32)data->fifo_raw[1] << 8 | data->fifo_raw[2];
Casting to u32 makes a little sense. Why?
> + /* Bytes [3..5]: PPG IR - same format */
> + *ppg_ir = (u32)(data->fifo_raw[3] & 0x07) << 16 |
> + (u32)data->fifo_raw[4] << 8 | data->fifo_raw[5];
> +
> + /* Bytes [6..8]: ECG - 18-bit signed, sign-extend to s32 */
> + *ecg = sign_extend32((u32)(data->fifo_raw[6] & 0x03) << 16 |
> + (u32)data->fifo_raw[7] << 8 | data->fifo_raw[8], 17);
Ditto for all these...
> + return 0;
> +}
...
> +static int max86150_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct max86150_data *data = iio_priv(indio_dev);
> + u32 ppg_red, ppg_ir;
> + s32 ecg;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + /*
> + * Claim direct mode to prevent concurrent sysfs reads from
> + * corrupting the FIFO pointers while a triggered buffer
> + * capture is active.
> + */
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> +
> + /*
> + * Single-shot path: wake the device, flush stale FIFO data,
> + * wait one sample period, read, then return to shutdown so
> + * the LEDs are not drawing current when idle.
> + */
> + ret = regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap,
> + MAX86150_REG_FIFO_WR_PTR, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap,
> + MAX86150_REG_OVF_COUNTER, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap,
> + MAX86150_REG_FIFO_RD_PTR, 0);
> + if (ret) {
> + regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
> + iio_device_release_direct(indio_dev);
> + return ret;
No, use regular pattern, id est
if (ret)
return ret;
Ditto for other cases like this.
> + }
> +
> + /* Wait for one complete sample period at 100 Hz (<= 10 ms) */
> + usleep_range(11000, 13000);
> +
> + ret = max86150_read_one_sample(data, &ppg_red, &ppg_ir, &ecg);
> + regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
> + iio_device_release_direct(indio_dev);
> + if (ret)
> + return ret;
> +
> + switch (chan->scan_index) {
> + case MAX86150_IDX_PPG_RED:
> + *val = ppg_red;
> + break;
> + case MAX86150_IDX_PPG_IR:
> + *val = ppg_ir;
> + break;
> + case MAX86150_IDX_ECG:
> + *val = ecg;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return IIO_VAL_INT;
> +
> + default:
> + return -EINVAL;
> + }
> +}
...
> +/*
> + * Control device power and the FIFO almost-full interrupt when the IIO
> + * triggered buffer is started (state=true) or stopped (state=false).
> + *
> + * On start: wake from shutdown, flush stale FIFO data so the first
> + * samples pushed to userspace are from after buffer enable, then
> + * unmask the A_FULL interrupt.
> + *
> + * On stop: mask the interrupt, then return to shutdown so the LED
> + * drivers do not draw current while capture is inactive.
> + */
> +static int max86150_set_trigger_state(struct iio_trigger *trig, bool state)
> +{
> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
> + struct max86150_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + if (!state) {
> + ret = regmap_write(data->regmap, MAX86150_REG_INT_ENABLE1, 0);
> + regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
_set_bits(), but why no check?
> + return ret;
> + }
> +
> + ret = regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_WR_PTR, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap, MAX86150_REG_OVF_COUNTER, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_RD_PTR, 0);
> + if (!ret)
> + ret = regmap_write(data->regmap, MAX86150_REG_INT_ENABLE1,
> + MAX86150_INT_A_FULL);
> + if (ret)
> + regmap_update_bits(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN, MAX86150_SYS_SHDN);
> + return ret;
Ditto.
...
Also why not to split to trigger_enable and trigger_disable?
_trigger_enable(max86150_data *data)
_trigger_disable(...)
_set_trigger_state()
if (enable)
return _trigger_enable()
return trigger_disable();
> +}
...
> +/**
> + * max86150_trigger_handler - threaded IRQ handler for FIFO almost-full
> + *
> + * Called by the IIO buffer infrastructure when the hardware trigger fires.
> + * Reads INT_STATUS1 to de-assert the interrupt, then drains all available
> + * FIFO samples into the IIO push buffer, packing only the channels that
> + * are currently enabled in active_scan_mask.
> + */
> +static irqreturn_t max86150_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *idev = pf->indio_dev;
> + struct max86150_data *data = iio_priv(idev);
> + unsigned int status, wr_ptr, rd_ptr, ovf;
> + u32 ppg_red, ppg_ir;
> + s32 ecg;
> + int ret, n_avail, i, j;
Usually it's not a good idea to mix ret with other variables that not
semantically related. Also, why those are signed?
> + /*
> + * Reading INT_STATUS1 clears the interrupt. Do this before touching
> + * the FIFO so the pin is de-asserted while we drain samples.
> + */
> + ret = regmap_read(data->regmap, MAX86150_REG_INT_STATUS1, &status);
> + if (ret)
> + goto done;
> +
> + ret = regmap_read(data->regmap, MAX86150_REG_FIFO_WR_PTR, &wr_ptr);
> + if (ret)
> + goto done;
> + ret = regmap_read(data->regmap, MAX86150_REG_FIFO_RD_PTR, &rd_ptr);
> + if (ret)
> + goto done;
> +
> + /*
> + * OVF_COUNTER: if non-zero the FIFO overflowed; drain all 32 slots.
> + * When wr_ptr == rd_ptr with no overflow the FIFO could be empty OR
> + * hold exactly MAX86150_FIFO_DEPTH entries (pointer wrap-around).
> + * Use the A_FULL status bit to disambiguate: if the IRQ fired for
> + * A_FULL but the computed count is zero, the FIFO wrapped to full.
> + */
> + ret = regmap_read(data->regmap, MAX86150_REG_OVF_COUNTER, &ovf);
> + if (ret)
> + goto done;
> +
> + if (ovf > 0) {
> + n_avail = MAX86150_FIFO_DEPTH;
> + } else {
> + n_avail = (wr_ptr - rd_ptr) & (MAX86150_FIFO_DEPTH - 1);
> + if (n_avail == 0 && (status & MAX86150_INT_A_FULL))
> + n_avail = MAX86150_FIFO_DEPTH;
> + }
> +
> + /*
> + * Anchor timestamps to the A_FULL IRQ capture time: sample index
> + * (MAX86150_A_FULL_SAMPLES - 1) corresponds to pf->timestamp.
> + * Samples that accumulated between the IRQ and handler execution
> + * receive timestamps in the future relative to the IRQ, eliminating
> + * load-dependent jitter in multi-sample drains.
> + */
> + for (i = 0; i < n_avail; i++) {
for (unsigned int i = 0; i < n_avail; i++) {
> + s64 ts = pf->timestamp +
> + (s64)(i - (MAX86150_A_FULL_SAMPLES - 1)) *
> + data->sample_period_ns;
This is unmaintainable. Split definition and assignment. Also, do you really
need a casting? I.o.w. isn't pf->timestamp already an s64?
> + ret = max86150_read_one_sample(data, &ppg_red, &ppg_ir, &ecg);
> + if (ret)
> + break;
> +
> + /*
> + * Zero the entire buffer before packing so padding bytes
> + * between enabled channels do not leak previous sample data
> + * to userspace when fewer than 3 channels are active.
> + */
> + memset(data->buf, 0, sizeof(data->buf));
> +
> + /*
> + * Pack only active channels at consecutive positions [0..j-1].
> + * iio_push_to_buffers_with_timestamp() uses scan_bytes (which
> + * accounts for the active channel count) to place the timestamp,
> + * so static indexing would misplace it when fewer than 3
> + * channels are enabled.
> + */
> + j = 0;
> + if (test_bit(MAX86150_IDX_PPG_RED, idev->active_scan_mask))
> + data->buf[j++] = ppg_red;
> + if (test_bit(MAX86150_IDX_PPG_IR, idev->active_scan_mask))
> + data->buf[j++] = ppg_ir;
> + if (test_bit(MAX86150_IDX_ECG, idev->active_scan_mask))
> + data->buf[j++] = ecg;
> +
> + iio_push_to_buffers_with_timestamp(idev, data->buf, ts);
> + }
> +
> +done:
> + iio_trigger_notify_done(idev->trig);
> + return IRQ_HANDLED;
> +}
> +
> +/* Chip initialisation / teardown */
> +
> +static void max86150_powerdown(void *arg)
> +{
> + struct max86150_data *data = arg;
> +
> + regmap_write(data->regmap, MAX86150_REG_INT_ENABLE1, 0);
> + regmap_write(data->regmap, MAX86150_REG_SYS_CTRL, MAX86150_SYS_SHDN);
> +}
...
> +static int max86150_chip_init(struct max86150_data *data)
> +{
> + int ret;
> +
> + /* Software reset; the bit self-clears within 1 ms */
> + ret = regmap_write(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_RESET);
> + if (ret)
> + return ret;
> + usleep_range(1000, 2000);
fsleep(). Also need a reference to datasheet section, table, et cetera.
> + /*
> + * FIFO: no sample averaging, rollover enabled, assert A_FULL when
> + * 17 samples are in the FIFO (32 - 15 = 17 available to read).
> + */
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_CONFIG,
> + MAX86150_FIFO_ROLLOVER_EN |
> + FIELD_PREP(MAX86150_FIFO_A_FULL,
> + MAX86150_FIFO_A_FULL_VAL));
> + if (ret)
> + return ret;
> +
> + /* Slot 1 = PPG Red (LED1), Slot 2 = PPG IR (LED2) */
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_DCTRL1,
> + FIELD_PREP(MAX86150_FIFO_FD1, MAX86150_FD_LED1) |
> + FIELD_PREP(MAX86150_FIFO_FD2, MAX86150_FD_LED2));
> + if (ret)
> + return ret;
> +
> + /* Slot 3 = ECG, Slot 4 = disabled */
> + ret = regmap_write(data->regmap, MAX86150_REG_FIFO_DCTRL2,
> + FIELD_PREP(MAX86150_FIFO_FD3, MAX86150_FD_ECG) |
> + FIELD_PREP(MAX86150_FIFO_FD4, MAX86150_FD_NONE));
> + if (ret)
> + return ret;
> +
> + /* PPG: 100 Hz sample rate, 16384 nA ADC full-scale range */
> + ret = regmap_write(data->regmap, MAX86150_REG_PPG_CONFIG1,
> + FIELD_PREP(MAX86150_PPG_ADC_RGE,
> + MAX86150_PPG_ADC_RGE_16384) |
> + FIELD_PREP(MAX86150_PPG_SR,
> + MAX86150_PPG_SR_100HZ));
> + if (ret)
> + return ret;
> +
> + /* LED pulse amplitudes (~50 mA) */
> + ret = regmap_write(data->regmap, MAX86150_REG_LED1_PA,
> + MAX86150_LED_PA_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, MAX86150_REG_LED2_PA,
> + MAX86150_LED_PA_DEFAULT);
> + if (ret)
> + return ret;
> +
> + /*
> + * Record sample period for timestamp reconstruction in the trigger
> + * handler. The PPG_SR field is fixed to 100 Hz in this driver.
> + */
> + data->sample_period_ns = 10000000; /* 100 Hz = 10 ms */
10 * NSEC_PER_MSEC
from time.h.
> + /*
> + * Assert SYS_SHDN so the LED drivers do not draw current while
> + * the driver is bound but no capture is active.
> + * set_trigger_state() clears SHDN when the IIO buffer is enabled
> + * and re-asserts it when disabled. read_raw() wakes and sleeps
> + * the device around each single-shot read.
> + */
> + return regmap_write(data->regmap, MAX86150_REG_SYS_CTRL,
> + MAX86150_SYS_SHDN);
> +}
...
> +/* Probe */
These comments bring no value, please, drop all of a such.
...
> +static int max86150_probe(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev;
> + struct max86150_data *data;
> + unsigned int part_id;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + data->dev = &client->dev;
A duplication. You have regmap with the same device.
> + /*
> + * Enable power supplies before any I2C access. Both supplies are
> + * optional in the device tree; use _optional variant so probing
> + * succeeds on boards that power the device from fixed rails with no
> + * DT regulator node.
> + */
> + ret = devm_regulator_get_enable_optional(&client->dev, "vdd");
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(&client->dev, ret,
Add
struct device *dev = &client->dev;
to the top of the function and use everywhere.
> + "Failed to get/enable vdd supply\n");
> +
> + ret = devm_regulator_get_enable_optional(&client->dev, "leds");
> + if (ret && ret != -ENODEV)
> + return dev_err_probe(&client->dev, ret,
> + "Failed to get/enable leds supply\n");
> +
> + data->regmap = devm_regmap_init_i2c(client, &max86150_regmap_config);
> + if (IS_ERR(data->regmap))
> + return dev_err_probe(&client->dev, PTR_ERR(data->regmap),
> + "Failed to initialise regmap\n");
> +
> + ret = regmap_read(data->regmap, MAX86150_REG_PART_ID, &part_id);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot read part ID\n");
> +
> + if (part_id != MAX86150_PART_ID_VAL)
> + return dev_err_probe(&client->dev, -ENODEV,
> + "Unexpected part ID 0x%02x (expected 0x%02x)\n",
> + part_id, MAX86150_PART_ID_VAL);
> +
> + ret = max86150_chip_init(data);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Chip initialisation failed\n");
> +
> + ret = devm_add_action_or_reset(&client->dev, max86150_powerdown, data);
> + if (ret)
> + return ret;
> +
> + indio_dev->name = "max86150";
> + indio_dev->channels = max86150_channels;
> + indio_dev->num_channels = ARRAY_SIZE(max86150_channels);
> + indio_dev->info = &max86150_iio_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + /*
> + * If the device tree provides an interrupt, set up a hardware
> + * trigger so userspace can use the FIFO almost-full signal to
> + * drive capture without polling.
> + */
> + if (client->irq > 0) {
> + unsigned long irq_trig;
> +
> + data->trig = devm_iio_trigger_alloc(&client->dev,
> + "%s-dev%d",
> + indio_dev->name,
> + iio_device_id(indio_dev));
> + if (!data->trig)
> + return -ENOMEM;
> +
> + data->trig->ops = &max86150_trigger_ops;
> + iio_trigger_set_drvdata(data->trig, indio_dev);
> +
> + /*
> + * Honour the interrupt trigger type from the device tree.
> + * Fall back to falling-edge if the DT does not specify one.
Why? Do we need to support broken DT?
> + */
> + irq_trig = irq_get_trigger_type(client->irq);
> + if (!irq_trig)
> + irq_trig = IRQF_TRIGGER_FALLING;
Simply drop this.
> + ret = devm_request_irq(&client->dev, client->irq,
> + iio_trigger_generic_data_rdy_poll,
> + irq_trig,
> + "max86150", data->trig);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot request IRQ %d\n",
> + client->irq);
No dup messages.
> +
> + ret = devm_iio_trigger_register(&client->dev, data->trig);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot register trigger\n");
> + }
> +
> + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev,
> + iio_pollfunc_store_time,
> + max86150_trigger_handler,
> + NULL);
> + if (ret)
> + return dev_err_probe(&client->dev, ret,
> + "Cannot setup triggered buffer\n");
So, it seems this message won't ever be printed. Drop it.
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v3 09/10] dt-bindings: display: rockchip: dw-dp: fix sound DAI cells
From: sashiko-bot @ 2026-06-23 20:51 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: conor+dt, devicetree, robh, dri-devel
In-Reply-To: <20260612-synopsys-dw-dp-improvements-v3-9-dc61e6352508@collabora.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Unconditionally changing `#sound-dai-cells` from `const: 0` to `const: 1` breaks backward compatibility with existing device tree binaries and causes `make dtbs_check` failures.
- [High] dw_dp_runtime_resume() fails to check the return value of clk_prepare_enable() for apb_clk and aux_clk, leading to unclocked hardware accesses.
- [High] The driver ignores the return value of pm_runtime_get_sync() and its scoped guard wrappers, leading to potential unclocked register accesses.
- [Medium] The `#sound-dai-cells = <1>` design is mathematically insufficient to support the RK3576's 6 DAI interfaces, ensuring future ABI breakage when MST is fully implemented.
--
commit dcfc80f29db8ade71eb0e49550ea25781716e785
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
dt-bindings: display: rockchip: dw-dp: fix sound DAI cells
This commit updates the #sound-dai-cells property to support the two
possible DAI interfaces (I2S and S/PDIF) on RK3588 and RK3576 DesignWare
DisplayPort controllers. It changes the const value to 1 and discusses
options for future Multi Stream Transport (MST) support.
> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml
> index 2b0d9e23e9432..1303d0e2145a9 100644
> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml
> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,dw-dp.yaml
> @@ -83,7 +83,8 @@ properties:
> maxItems: 1
>
> "#sound-dai-cells":
> - const: 0
> + const: 1
> + description: 0 for I2S, 1 for SPDIF
[Severity: Critical]
Does unconditionally changing #sound-dai-cells from const: 0 to const: 1
break backward compatibility with existing device tree binaries?
Existing device trees supplying 0 cells will now fail validation during
make dtbs_check, and older DTBs will no longer conform to the schema.
[Severity: Medium]
Is setting #sound-dai-cells = <1> mathematically sufficient to support the
RK3576 hardware?
The commit message mentions the RK3576 features MST with 6 DAIs, so a single
cell constraint only supports 2 states (I2S and S/PDIF). Will this necessitate
another breaking change when MST is implemented?
>
> required:
> - compatible
[ ... ]
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 36ee6e027af52..7f4f36c614844 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
[ ... ]
> @@ -1651,6 +1657,8 @@ static void dw_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> struct drm_connector_state *conn_state;
> int ret;
>
> + pm_runtime_get_sync(dp->dev);
> +
[Severity: High]
Does this code discard the return value of pm_runtime_get_sync()?
If the PM core fails to resume the device, the code proceeds with the enable
sequence, which could risk unclocked register accesses later on.
> connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> if (!connector) {
> dev_err(dp->dev, "failed to get connector\n");
[ ... ]
> @@ -1725,6 +1734,8 @@ static enum drm_connector_status dw_dp_bridge_detect(struct drm_bridge *bridge,
> {
> struct dw_dp *dp = bridge_to_dp(bridge);
>
> + ACQUIRE(pm_runtime_active_auto, pm)(dp->dev);
> +
> if (!dw_dp_hpd_detect(dp))
[Severity: High]
Does using this unconditional PM scope guard discard the return value?
If the device failed to resume, the code will proceed to access hardware
registers in dw_dp_hpd_detect(), which could trigger a bus fault.
> return connector_status_disconnected;
>
> if (!dw_dp_hpd_detect_link(dp, connector))
[ ... ]
> @@ -2099,6 +2160,26 @@ void dw_dp_unbind(struct dw_dp *dp)
> }
> EXPORT_SYMBOL_GPL(dw_dp_unbind);
>
> +int dw_dp_runtime_suspend(struct dw_dp *dp)
> +{
> + clk_disable_unprepare(dp->aux_clk);
> + clk_disable_unprepare(dp->apb_clk);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_dp_runtime_suspend);
> +
> +int dw_dp_runtime_resume(struct dw_dp *dp)
> +{
> + clk_prepare_enable(dp->apb_clk);
> + clk_prepare_enable(dp->aux_clk);
> +
[Severity: High]
Are we ignoring the return values of clk_prepare_enable() here?
If the clocks fail to enable, unconditionally calling dw_dp_init_hw() will
access hardware registers while unclocked, which could lead to a bus hang.
> + dw_dp_init_hw(dp);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_dp_runtime_resume);
[ ... ]
> diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> index 0de822360c8db..1f53228e56d9f 100644
> --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
[ ... ]
> +static void dw_dp_rockchip_hpd_sw_sel(void *data, bool force_hpd_from_sw)
> +{
> + struct rockchip_dw_dp *dp = data;
> + u32 hpd_reg = dp->pdata->hpd_reg[dp->id];
> +
> + dp->hpd_sel = force_hpd_from_sw;
> +
> + ACQUIRE(pm_runtime_active_auto, pm)(dp->dev);
> +
> + regmap_write(dp->vo_grf, hpd_reg,
> + FIELD_PREP_WM16(ROCKCHIP_VO_GRF_DP_SINK_HPD_SEL, dp->hpd_sel));
[Severity: High]
Does this ignore the possibility that the power domain failed to resume?
If the unconditional PM guard fails, writing to the vo_grf syscon will
trigger a bus fault.
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-synopsys-dw-dp-improvements-v3-0-dc61e6352508@collabora.com?part=9
^ permalink raw reply
* Re: [PATCH 7/9] drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format
From: sashiko-bot @ 2026-06-23 20:51 UTC (permalink / raw)
To: Cristian Ciocaltea; +Cc: robh, dri-devel, devicetree, conor+dt
In-Reply-To: <20260617-dw-hdmi-qp-yuv-v1-7-a665cfd06d7d@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Mutating hardware state and global device tracking variables inside an atomic_check callback.
--
commit ee5f0003f61d5c3a78b5bc65346f39307471c74c
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
drm/rockchip: dw_hdmi_qp: Support 10-bit YUV422 output format
This commit adds support for the 10-bit YUV 4:2:2 output format
(MEDIA_BUS_FMT_UYVY10_1X20) to the Rockchip DW HDMI QP encoder. It also
forces 8 bpc when YUV 4:2:2 is in use to ensure the PHY PLL output remains
aligned with the TMDS character rate.
> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> --- a/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c
[ ... ]
> @@ -227,7 +228,14 @@ dw_hdmi_qp_rockchip_encoder_atomic_check(struct drm_encoder *encoder,
> }
>
> phy_cfg.hdmi.tmds_char_rate = conn_state->hdmi.tmds_char_rate;
> - phy_cfg.hdmi.bpc = conn_state->hdmi.output_bpc;
> + /*
> + * YUV422 always transmits two 12-bit components per clock cycle,
> + * regardless of the color depth, which from a rate perspective is
> + * equivalent to three 8-bit RGB components. Force 8 bpc here to
> + * keep the PHY PLL output aligned with the TMDS character rate.
> + */
> + phy_cfg.hdmi.bpc = (s->output_mode == ROCKCHIP_OUT_MODE_YUV422 ?
> + 8 : conn_state->hdmi.output_bpc);
>
> ret = phy_configure(hdmi->phy, &phy_cfg);
> if (!ret) {
> hdmi->tmds_char_rate = conn_state->hdmi.tmds_char_rate;
> s->output_type = DRM_MODE_CONNECTOR_HDMIA;
> s->output_bpc = conn_state->hdmi.output_bpc;
> s->bus_format = ingest_fmt;
> } else {
[Severity: High]
This is a pre-existing issue, but does
dw_hdmi_qp_rockchip_encoder_atomic_check() mutate hardware state and global
tracking variables during an atomic check?
In the DRM atomic framework, the atomic_check phase is expected to be
side-effect free. However, this code calls phy_configure(), which writes
directly to the PHY hardware registers, and then modifies the global
hdmi->tmds_char_rate variable.
Could this corrupt the active display configuration if userspace performs a
DRM_MODE_ATOMIC_TEST_ONLY commit or if an atomic commit fails a later
validation stage?
Furthermore, if the device is runtime-suspended, could accessing unclocked
hardware registers here trigger a synchronous abort and hang the system?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=7
^ permalink raw reply
* [PATCH v5 6/6] arm64: dts: allwinner: A133: add support for Baijie Helper A133 board
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
Baijie Helper A133 board is a development board around Baijie A133 Core
SBC. Features:
- 1/2/4GiB LPDDR4 DRAM
- 8/16/32GiB eMMC
- AXP707 PMIC
- USB-C OTG port in peripheral mode (via onboard hub)
- 2 USB 2.0 ports
- MicroSD slot and on-board eMMC module
- Gigabit Ethernet
- Bluetooth
- WiFi
Add initial support for both the Helper and Core boards, including UART,
PMU, eMMC, USB, Ethernet, LRADC-connected buttons.
UART1 can only be used for Bluetooth module, but BT-WiFi combo Allwinner
AW869A chip has no mainline driver currently.
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v5:
- no changes
v4:
- renamed "sun50i-a133-baijie-helper.dtb" -> "sun50i-a133-helperboard.dtb"
- added "model" property into root of sun50i-a133-helperboard-core.dtsi
- added "cap-mmc-highspeed" and "max-frequency" into &mmc2
- added "x-powers,drive-vbus-en" and "*-supply" into &axp803
- dropped all "regulator-enable-ramp-delay" properties
- replaced ®_dcdc3 with a "polyphased" comment
- exact DRAM voltage in ®_dcdc5
- disabled ®_dcdc6 to avoid "[ 31.710641] dcdc6: disabling"
- added ®_vdd5v "root" regulator
- added "disable-wp" into &mmc0
- commented &usb_otg
- assigned usb1_vbus-supply in &usbphy
v3:
- added my copyrights into the newly introduced DTs
- all DT nodes sorted alphabetically
- all always-on regulators commented/propetly named
- all regulators got proper voltages (not default ranges)
- ADC-sensed buttons K1..K5 added
- re-labelled "eth_phy" -> "rgmii_phy"
- usbphy 0 switched from host into peripheral mode (downstream from an
onboard hub)
- typo sun50i-a133-baije-core.dtsi -> sun50i-a133-baijie-core.dtsi
v2:
- introduced baijie,helper-a133-core compatible for the Core (SoM) board
arch/arm64/boot/dts/allwinner/Makefile | 1 +
.../sun50i-a133-helperboard-core.dtsi | 197 ++++++++++++++++++
.../dts/allwinner/sun50i-a133-helperboard.dts | 148 +++++++++++++
3 files changed, 346 insertions(+)
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard-core.dtsi
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard.dts
diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
index 53e6b701e7d3..aa21f58a4be1 100644
--- a/arch/arm64/boot/dts/allwinner/Makefile
+++ b/arch/arm64/boot/dts/allwinner/Makefile
@@ -24,6 +24,7 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-sopine-baseboard.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a64-teres-i.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h64-remix-mini-pc.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a100-allwinner-perf1.dtb
+dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a133-helperboard.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-a133-liontron-h-a133l.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-bananapi-m2-plus.dtb
dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h5-bananapi-m2-plus-v1.2.dtb
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard-core.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard-core.dtsi
new file mode 100644
index 000000000000..545972d2324a
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard-core.dtsi
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2025 Arm Ltd.
+ * Copyright (c) 2026 Alexander Sverdlin <alexander.sverdlin@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "sun50i-a100.dtsi"
+#include "sun50i-a100-cpu-opp.dtsi"
+
+/{
+ model = "Baijie A133 HelperBoard Core";
+ compatible = "baijie,helperboard-a133-core",
+ "allwinner,sun50i-a100";
+
+ aliases {
+ serial1 = &uart1; /* BT module */
+ };
+};
+
+&cpu0 {
+ cpu-supply = <®_dcdc2>;
+};
+
+&lradc {
+ vref-supply = <®_aldo1>;
+};
+
+&mmc2 {
+ vmmc-supply = <®_dcdc1>;
+ vqmmc-supply = <®_eldo1>;
+ cap-mmc-highspeed;
+ cap-mmc-hw-reset;
+ max-frequency = <100000000>;
+ non-removable;
+ bus-width = <8>;
+ mmc-ddr-1_8v;
+ mmc-hs200-1_8v;
+ status = "okay";
+};
+
+&pio {
+ vcc-pb-supply = <®_dcdc1>;
+ vcc-pc-supply = <®_eldo1>;
+ vcc-pd-supply = <®_dcdc1>;
+ vcc-pe-supply = <®_dldo2>;
+ vcc-pf-supply = <®_dcdc1>;
+ vcc-pg-supply = <®_dldo1>;
+ vcc-ph-supply = <®_dcdc1>;
+ /*
+ * PL0/PL1 are the I2C connection to PMIC, but it would create a
+ * circular dependency:
+ * vcc-pl-supply = <®_aldo3>;
+ */
+};
+
+&r_i2c0 {
+ status = "okay";
+
+ axp803: pmic@34 {
+ compatible = "x-powers,axp803";
+ reg = <0x34>;
+ interrupt-parent = <&r_intc>;
+ interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+ x-powers,drive-vbus-en; /* set N_VBUSEN as output pin */
+ aldoin-supply = <®_vdd5v>;
+ dldoin-supply = <®_vdd5v>;
+ eldoin-supply = <®_vdd5v>;
+ fldoin-supply = <®_dcdc5>;
+ vin1-supply = <®_vdd5v>;
+ vin2-supply = <®_vdd5v>;
+ vin3-supply = <®_vdd5v>;
+ vin4-supply = <®_vdd5v>;
+ vin5-supply = <®_vdd5v>;
+ vin6-supply = <®_vdd5v>;
+ drivevbus-supply = <®_vdd5v>;
+ };
+};
+
+#include "axp803.dtsi"
+
+&ac_power_supply {
+ status = "okay";
+};
+
+®_aldo1 {
+ /* PLL + LRADC analog reference */
+ regulator-always-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pll";
+};
+
+®_aldo2 {
+ /* LPDDR */
+ regulator-always-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vdd18-lpddr";
+};
+
+®_aldo3 {
+ /*
+ * Port L, but linking it to &pio node would create a circular
+ * dependency because of PL0/PL1 I2C connection to PMIC
+ */
+ regulator-always-on;
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pl";
+};
+
+®_dcdc1 {
+ /* Besides Port D it also powers analog part of USB IP and SoC I/O */
+ regulator-always-on;
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-name = "vcc-3v3";
+};
+
+®_dcdc2 {
+ regulator-always-on;
+ regulator-min-microvolt = <810000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "vdd-cpu";
+};
+
+/* DCDC3 is polyphased with DCDC2 */
+
+®_dcdc4 {
+ /* Digital part of USB IP, "System" SoC power rail */
+ regulator-always-on;
+ regulator-min-microvolt = <950000>;
+ regulator-max-microvolt = <950000>;
+ regulator-name = "vdd-sys";
+};
+
+®_dcdc5 {
+ regulator-always-on;
+ regulator-min-microvolt = <1100000>;
+ regulator-max-microvolt = <1100000>;
+ regulator-name = "vcc-dram";
+};
+
+/* DCDC6 unused */
+®_dcdc6 {
+ status = "disabled";
+};
+
+®_dldo1 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pg";
+};
+
+®_dldo2 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pe";
+};
+
+®_dldo3 {
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-name = "avdd-csi";
+};
+
+®_dldo4 {
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ regulator-name = "afvcc-csi";
+};
+
+®_eldo1 {
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ regulator-name = "vcc-pc";
+};
+
+®_eldo2 {
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ regulator-name = "dvdd-csi";
+};
+
+/* ELDO3 unused */
+
+®_fldo1 {
+ /* CPUS power rail */
+ regulator-always-on;
+ regulator-min-microvolt = <900000>;
+ regulator-max-microvolt = <900000>;
+ regulator-name = "vdd-cpus";
+};
+
+/* reg_drivevbus unused */
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard.dts
new file mode 100644
index 000000000000..694c0cacf906
--- /dev/null
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard.dts
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2025 Arm Ltd.
+ * Copyright (c) 2026 Alexander Sverdlin <alexander.sverdlin@gmail.com>
+ */
+
+/dts-v1/;
+
+#include "sun50i-a133-helperboard-core.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/linux-event-codes.h>
+#include <dt-bindings/leds/common.h>
+
+/{
+ model = "Baijie HelperBoard A133";
+ compatible = "baijie,helperboard-a133",
+ "baijie,helperboard-a133-core",
+ "allwinner,sun50i-a100";
+
+ aliases {
+ serial0 = &uart0;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ leds {
+ compatible = "gpio-leds";
+
+ led {
+ function = LED_FUNCTION_INDICATOR;
+ color = <LED_COLOR_ID_GREEN>;
+ gpios = <&pio 7 13 GPIO_ACTIVE_LOW>; /* PH13 */
+ };
+ };
+
+ reg_vdd5v: vdd5v {
+ /* board wide 5V supply from a 12V->5V regulator */
+ compatible = "regulator-fixed";
+ regulator-name = "vdd-5v";
+ regulator-min-microvolt = <5000000>;
+ regulator-max-microvolt = <5000000>;
+ regulator-always-on;
+ };
+};
+
+&ehci1 {
+ status = "okay";
+};
+
+&emac0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&rgmii0_pins>;
+ phy-handle = <&rgmii_phy>;
+ phy-mode = "rgmii-id";
+ allwinner,rx-delay-ps = <200>;
+ allwinner,tx-delay-ps = <200>;
+ status = "okay";
+};
+
+&lradc {
+ wakeup-source;
+ status = "okay";
+
+ button-115 {
+ label = "K1";
+ linux,code = <KEY_1>;
+ channel = <0>;
+ voltage = <114607>;
+ };
+
+ button-235 {
+ label = "K2";
+ linux,code = <KEY_2>;
+ channel = <0>;
+ voltage = <234783>;
+ };
+
+ button-360 {
+ label = "K3";
+ linux,code = <KEY_3>;
+ channel = <0>;
+ voltage = <360000>;
+ };
+
+ button-476 {
+ label = "K4";
+ linux,code = <KEY_4>;
+ channel = <0>;
+ voltage = <476471>;
+ };
+
+ button-592 {
+ label = "K5";
+ linux,code = <KEY_5>;
+ channel = <0>;
+ voltage = <591946>;
+ };
+};
+
+&mdio0 {
+ reset-gpios = <&pio 7 11 GPIO_ACTIVE_LOW>; /* PH11 */
+ reset-delay-us = <10000>;
+ reset-post-delay-us = <150000>;
+
+ rgmii_phy: ethernet-phy@1 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <1>;
+ };
+};
+
+&mmc0 {
+ vmmc-supply = <®_dcdc1>;
+ cd-gpios = <&pio 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
+ bus-width = <4>;
+ disable-wp;
+ status = "okay";
+};
+
+&ohci1 {
+ status = "okay";
+};
+
+&rgmii0_pins {
+ drive-strength = <30>;
+};
+
+&uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&uart0_pb_pins>;
+ status = "okay";
+};
+
+&usb_otg {
+ /*
+ * Connected to a downstream port of an onboard hub, therefore only
+ * "peripheral" mode will work here.
+ */
+ dr_mode = "peripheral";
+ status = "okay";
+};
+
+&usbphy {
+ usb1_vbus-supply = <®_vdd5v>;
+ status = "okay";
+};
--
2.54.0
^ permalink raw reply related
* [PATCH v5 5/6] arm64: dts: allwinner: a100: Add LRADC node
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
A100/A133 SoCs feature a Low Rate ADC (LRADC) for Key application.
Specs:
- Power supply voltage: 1.8 V
- Reference voltage: 1.35 V
- Interrupt support
- Support Hold Key and General Key
- Support normal, continue and single work mode
- 6-bits resolution, sample rate up to 2 kHz
- Voltage input range between 0 and 1.35 V
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v5:
- reflowed "compatible" property of lradc node
v4:
- added allwinner,sun50i-a100-lradc compatible
v3:
- new patch
arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
index b3fb1e0ee796..ba6020989ce9 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a100.dtsi
@@ -466,6 +466,16 @@ ths: thermal-sensor@5070400 {
#thermal-sensor-cells = <1>;
};
+ lradc: lradc@5070800 {
+ compatible = "allwinner,sun50i-a100-lradc",
+ "allwinner,sun50i-r329-lradc";
+ reg = <0x05070800 0x400>;
+ interrupts = <GIC_SPI 22 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&ccu CLK_BUS_LRADC>;
+ resets = <&ccu RST_BUS_LRADC>;
+ status = "disabled";
+ };
+
usb_otg: usb@5100000 {
compatible = "allwinner,sun50i-a100-musb",
"allwinner,sun8i-a33-musb";
--
2.54.0
^ permalink raw reply related
* [PATCH v5 4/6] dt-bindings: input: sun4i-lradc-keys: Add A100/A133 compatible
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
The Allwinner A100/A133 SoCs have an LRADC which is compatible with the
versions in existing SoCs. Add a compatible string for A100, with the R329
fallback.
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v5:
- no changes
v4:
- new patch
.../bindings/input/allwinner,sun4i-a10-lradc-keys.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/input/allwinner,sun4i-a10-lradc-keys.yaml b/Documentation/devicetree/bindings/input/allwinner,sun4i-a10-lradc-keys.yaml
index 6bdb8040be65..524c8b51f53f 100644
--- a/Documentation/devicetree/bindings/input/allwinner,sun4i-a10-lradc-keys.yaml
+++ b/Documentation/devicetree/bindings/input/allwinner,sun4i-a10-lradc-keys.yaml
@@ -23,6 +23,7 @@ properties:
- const: allwinner,sun50i-r329-lradc
- items:
- enum:
+ - allwinner,sun50i-a100-lradc
- allwinner,sun50i-h616-lradc
- allwinner,sun20i-d1-lradc
- const: allwinner,sun50i-r329-lradc
--
2.54.0
^ permalink raw reply related
* [PATCH v5 3/6] dt-bindings: arm: sunxi: Add Baijie HelperBoard A133 compatible
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input, Conor Dooley
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
Baijie HelperBoard A133 is a development board around their A133 Core
board. Introduce a compatible for both the Core and the development
boards.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v5:
- no changes
v4:
- renamed "Baijie Helper A133" -> "Baijie A133 HelperBoard"
- renamed "baijie,helper-a133" -> "baijie,helperboard-a133"
v3:
- no separate section for "core" .dtsi
v2:
- introduced baijie,helper-a133-core compatible for the Core (SoM) board
Documentation/devicetree/bindings/arm/sunxi.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/sunxi.yaml b/Documentation/devicetree/bindings/arm/sunxi.yaml
index e6443c266fa1..82dd58b95f8a 100644
--- a/Documentation/devicetree/bindings/arm/sunxi.yaml
+++ b/Documentation/devicetree/bindings/arm/sunxi.yaml
@@ -96,6 +96,12 @@ properties:
- const: allwinner,ba10-tvbox
- const: allwinner,sun4i-a10
+ - description: Baijie A133 HelperBoard
+ items:
+ - const: baijie,helperboard-a133
+ - const: baijie,helperboard-a133-core
+ - const: allwinner,sun50i-a100
+
- description: BananaPi
items:
- const: lemaker,bananapi
--
2.54.0
^ permalink raw reply related
* [PATCH v5 2/6] dt-bindings: vendor-prefixes: Add Shenzhen Baijie Technology Co., Ltd.
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input, Conor Dooley, Paul Kocialkowski
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
Shenzhen Baijie Technology Co., Ltd. focuses on R&D and production of
embedded products as well as customization of embedded solutions.
Link: https://szbaijie.com/
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Paul Kocialkowski <paulk@sys-base.io>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v2-v5:
- no changes
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 6b9fb6a6bf0b..88225786e216 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -229,6 +229,8 @@ patternProperties:
description: Azoteq (Pty) Ltd
"^azw,.*":
description: Shenzhen AZW Technology Co., Ltd.
+ "^baijie,.*":
+ description: Shenzhen Baijie Technology Co., Ltd.
"^baikal,.*":
description: BAIKAL ELECTRONICS, JSC
"^bananapi,.*":
--
2.54.0
^ permalink raw reply related
* [PATCH v5 1/6] arm64: defconfig: Enable Allwinner LRADC input driver
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
In-Reply-To: <20260623204824.691832-1-alexander.sverdlin@gmail.com>
Enable Allwinner LRADC input driver as module to support buttons on Baijie
HelperBoard A133.
Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
---
Changelog:
v4-v5:
- no changes
v3:
- new patch
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 654a102cb5bc..c267f0906460 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -527,6 +527,7 @@ CONFIG_KEYBOARD_GPIO=y
CONFIG_KEYBOARD_GPIO_POLLED=m
CONFIG_KEYBOARD_SNVS_PWRKEY=m
CONFIG_KEYBOARD_IMX_SC_KEY=m
+CONFIG_KEYBOARD_SUN4I_LRADC=m
CONFIG_KEYBOARD_CROS_EC=y
CONFIG_KEYBOARD_MTK_PMIC=m
CONFIG_MOUSE_ELAN_I2C=m
--
2.54.0
^ permalink raw reply related
* [PATCH v5 0/6] Add support for Baijie Helper A133 board
From: Alexander Sverdlin @ 2026-06-23 20:48 UTC (permalink / raw)
To: linux-arm-kernel, linux-sunxi
Cc: Alexander Sverdlin, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
Baijie Helper A133 board is a development board around Baijie A133 Core
SBC. Features:
- 1/2/4GiB LPDDR4 DRAM
- 8/16/32GiB eMMC
- AXP707 PMIC
- USB-C OTG port in peripheral mode (via onboard hub)
- 2 USB 2.0 ports
- MicroSD slot and on-board eMMC module
- Gigabit Ethernet
- Bluetooth
- WiFi
Add initial support for both the Helper and Core boards, including UART,
PMU, eMMC, USB, Ethernet, LRADC-connected buttons.
UART1 can only be used for Bluetooth module, but BT-WiFi combo Allwinner
AW869A chip has not mainline driver currently.
Link: https://szbaijie.com/index/product/product_detail.html?product_id=23&language=en
Changelog:
v5:
- sun50i-a100.dtsi: reflowed "compatible" property of lradc node
- dropped "reserve RAM for ATF" patch [v4 6/7]
v4:
- reserve RAM for ATF
- sun4i-lradc-keys: Add A100/A133 compatible
- dt-bindings: renamed "Baijie Helper A133" -> "Baijie A133 HelperBoard"
- dt-bindings: renamed "baijie,helper-a133" -> "baijie,helperboard-a133"
- dt-bindings: introduced allwinner,sun50i-a100-lradc
- reserve RAM for ATF
- renamed "sun50i-a133-baijie-helper.dtb" -> "sun50i-a133-helperboard.dtb"
- added "model" property into root of sun50i-a133-helperboard-core.dtsi
- added "cap-mmc-highspeed" and "max-frequency" into &mmc2
- added "x-powers,drive-vbus-en" and "*-supply" into &axp803
- dropped all "regulator-enable-ramp-delay" properties
- replaced ®_dcdc3 with a "polyphased" comment
- exact DRAM voltage in ®_dcdc5
- disabled ®_dcdc6 to avoid "[ 31.710641] dcdc6: disabling"
- added ®_vdd5v "root" regulator
- added "disable-wp" into &mmc0
- commented &usb_otg
- assigned usb1_vbus-supply in &usbphy
- https://lore.kernel.org/all/20260605070923.3045073-1-alexander.sverdlin@gmail.com/
v3:
- added lradc node to sun50i-a100.dtsi
- enabled LRADC driver in arm64 defconfig
- added my copyrights into the newly introduced DTs
- all DT nodes sorted alphabetically
- all always-on regulators commented/propetly named
- all regulators got proper voltages (not default ranges)
- ADC-sensed buttons K1..K5 added
- re-labelled "eth_phy" -> "rgmii_phy"
- usbphy 0 switched from host into peripheral mode (downstream from an
onboard hub)
- typo sun50i-a133-baije-core.dtsi -> sun50i-a133-baijie-core.dtsi
- https://lore.kernel.org/all/20260517234134.2737320-1-alexander.sverdlin@gmail.com/
v2:
- introduced baijie,helper-a133-core compatible for the Core (SoM) board
- https://lore.kernel.org/all/20260510201644.4143710-1-alexander.sverdlin@gmail.com/
v1:
- https://lore.kernel.org/all/20260503191842.2736130-1-alexander.sverdlin@gmail.com/
Alexander Sverdlin (6):
arm64: defconfig: Enable Allwinner LRADC input driver
dt-bindings: vendor-prefixes: Add Shenzhen Baijie Technology Co., Ltd.
dt-bindings: arm: sunxi: Add Baijie HelperBoard A133 compatible
dt-bindings: input: sun4i-lradc-keys: Add A100/A133 compatible
arm64: dts: allwinner: a100: Add LRADC node
arm64: dts: allwinner: A133: add support for Baijie Helper A133 board
.../devicetree/bindings/arm/sunxi.yaml | 6 +
.../input/allwinner,sun4i-a10-lradc-keys.yaml | 1 +
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
arch/arm64/boot/dts/allwinner/Makefile | 1 +
.../arm64/boot/dts/allwinner/sun50i-a100.dtsi | 10 +
.../sun50i-a133-helperboard-core.dtsi | 197 ++++++++++++++++++
.../dts/allwinner/sun50i-a133-helperboard.dts | 148 +++++++++++++
arch/arm64/configs/defconfig | 1 +
8 files changed, 366 insertions(+)
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard-core.dtsi
create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-a133-helperboard.dts
--
2.54.0
^ permalink raw reply
* [PATCH v2 5/5] riscv: dts: spacemit: improve RTL8211F PHY configuration on K3 Pico-ITX board
From: Aurelien Jarno @ 2026-06-23 20:43 UTC (permalink / raw)
To: linux-kernel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Yixun Lan
Cc: Aurelien Jarno,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:RISC-V ARCHITECTURE,
open list:RISC-V SPACEMIT SoC Support
In-Reply-To: <20260623204431.498700-1-aurelien@aurel32.net>
Vendor kernel enabled ALDPS (Advanced Link Down Power Saving) on the
RTL8211F PHY to save power when link down.
Vendor kernel also disabled the 125MHz clkout clock signal, and indeed
the schematics confirms that it only goes to a test point (TP14), so
let's do the same.
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
arch/riscv/boot/dts/spacemit/k3-pico-itx.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
index bf64b6c6f867f..e12d798b48a49 100644
--- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
+++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
@@ -234,6 +234,8 @@ phy0: phy@1 {
reset-gpios = <&gpio 0 15 GPIO_ACTIVE_LOW>;
reset-assert-us = <10000>;
reset-deassert-us = <10000>;
+ realtek,aldps-enable;
+ realtek,clkout-disable;
};
};
};
--
2.53.0
^ permalink raw reply related
* [PATCH v2 4/5] riscv: dts: spacemit: add i2c aliases on K3 Pico-ITX board
From: Aurelien Jarno @ 2026-06-23 20:43 UTC (permalink / raw)
To: linux-kernel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Yixun Lan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti
Cc: Aurelien Jarno,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:RISC-V ARCHITECTURE,
open list:RISC-V SPACEMIT SoC Support
In-Reply-To: <20260623204431.498700-1-aurelien@aurel32.net>
Add i2c aliases for i2c2 and i2c8 on K3 Pico-ITX board. This is useful
to keep a stable number for the /dev entries after loading the i2c-dev
module.
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
arch/riscv/boot/dts/spacemit/k3-pico-itx.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
index 0a0c221c99e74..bf64b6c6f867f 100644
--- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
+++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
@@ -14,6 +14,8 @@ / {
aliases {
ethernet0 = ð0;
+ i2c2 = &i2c2;
+ i2c8 = &i2c8;
serial0 = &uart0;
};
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/5] riscv: dts: spacemit: add 24c02 eeprom on K3 Pico-ITX board
From: Aurelien Jarno @ 2026-06-23 20:43 UTC (permalink / raw)
To: linux-kernel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Yixun Lan
Cc: Aurelien Jarno,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
In-Reply-To: <20260623204431.498700-1-aurelien@aurel32.net>
The K3 Pico-ITX board includes a 24c02 eeprom, that stores the product
name, the part name, the MAC address of the network interfaces and the
board's serial number. These values are also exposed via an
onie,tlv-layout nvmem layout.
The eeprom is marked as read-only since its contents are not supposed to
be modified.
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
arch/riscv/boot/dts/spacemit/k3-pico-itx.dts | 30 ++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
index 5fe9041ec5702..0a0c221c99e74 100644
--- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
+++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
@@ -47,6 +47,36 @@ &i2c2 {
pinctrl-names = "default";
pinctrl-0 = <&i2c2_1_cfg>;
status = "okay";
+
+ eeprom@50 {
+ compatible = "atmel,24c02";
+ reg = <0x50>;
+ pagesize = <8>;
+ read-only;
+ size = <256>;
+ status = "okay";
+
+ nvmem-layout {
+ compatible = "onie,tlv-layout";
+
+ mac-address {
+ #nvmem-cell-cells = <1>;
+ };
+
+ num-macs {
+ };
+
+ part-number {
+ };
+
+ product-name {
+ };
+
+ serial-number {
+ };
+
+ };
+ };
};
&i2c8 {
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/5] riscv: dts: spacemit: enable the i2c2 adapter on K3 Pico-ITX board
From: Aurelien Jarno @ 2026-06-23 20:43 UTC (permalink / raw)
To: linux-kernel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Yixun Lan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti
Cc: Aurelien Jarno,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
In-Reply-To: <20260623204431.498700-1-aurelien@aurel32.net>
Define I2C2 pinctrl properties, and enable it on the K3 Pico-ITX board.
It will be used by the 24c02 eeprom.
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
arch/riscv/boot/dts/spacemit/k3-pico-itx.dts | 6 ++++++
arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi | 11 +++++++++++
2 files changed, 17 insertions(+)
v2:
- Rename pinctrl subnode from i2c2-0-pins to i2c2-1-pins
- Add a missing space after bias-pull-up
diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
index 509cebc0c9568..5fe9041ec5702 100644
--- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
+++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
@@ -43,6 +43,12 @@ reg_aux_vcc5v: regulator-aux-vcc5v {
};
};
+&i2c2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c2_1_cfg>;
+ status = "okay";
+};
+
&i2c8 {
pinctrl-names = "default";
pinctrl-0 = <&i2c8_cfg>;
diff --git a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
index 3ee1471f37985..50dc124c097f9 100644
--- a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
@@ -78,6 +78,17 @@ gmac1-phy-0-pins {
};
};
+ /omit-if-no-ref/
+ i2c2_1_cfg: i2c2-1-cfg {
+ i2c2-1-pins {
+ pinmux = <K3_PADCONF(46, 5)>, /* i2c2 scl */
+ <K3_PADCONF(47, 5)>; /* i2c2 sda */
+
+ bias-pull-up = <0>;
+ drive-strength = <25>;
+ };
+ };
+
/omit-if-no-ref/
i2c8_cfg: i2c8-cfg {
i2c8-pins {
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/5] riscv: dts: spacemit: set console baud rate on K3 Pico-ITX board
From: Aurelien Jarno @ 2026-06-23 20:43 UTC (permalink / raw)
To: linux-kernel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Yixun Lan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Guodong Xu
Cc: Aurelien Jarno, Guodong Xu,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:RISC-V SPACEMIT SoC Support,
open list:RISC-V SPACEMIT SoC Support
In-Reply-To: <20260623204431.498700-1-aurelien@aurel32.net>
Because the default console's baud rate is not set, defconfig kernels do
not have any serial output on this platform. Set the baud rate to
115200, matching what is used by U-Boot etc on this platform.
Fixes: 7a6131804986 ("riscv: dts: spacemit: add K3 Pico-ITX board support")
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Reviewed-by: Guodong Xu <docular.xu@gmail.com>
---
arch/riscv/boot/dts/spacemit/k3-pico-itx.dts | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
v2:
- Collect Reviewed-by tag
diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
index b89c1521e6649..509cebc0c9568 100644
--- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
+++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
@@ -18,7 +18,7 @@ aliases {
};
chosen {
- stdout-path = "serial0";
+ stdout-path = "serial0:115200n8";
};
memory@100000000 {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v3 08/10] drm/rockchip: dw_dp: Add runtime PM support
From: sashiko-bot @ 2026-06-23 20:40 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: dri-devel, conor+dt, robh, devicetree
In-Reply-To: <20260612-synopsys-dw-dp-improvements-v3-8-dc61e6352508@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Runtime PM initialized without matching active hardware state, causing clocks to remain permanently enabled.
- [High] Using devm_pm_runtime_enable() in a component bind callback ties PM lifecycle incorrectly, causing Use-After-Free and devres leaks.
--
commit 77c03b774daaa80f329daaa791a8229e9e24ad9e
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
drm/rockchip: dw_dp: Add runtime PM support
Add support for runtime PM to the Rockchip RK3576/3588 Synopsys
DesignWare DisplayPort driver.
> diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> index 9c53f1d2c29ab..1f53228e56d9f 100644
> --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
[ ... ]
> @@ -213,6 +218,12 @@ static int dw_dp_rockchip_bind(struct device *dev, struct device *master, void *
> if (IS_ERR(dp->base))
> return PTR_ERR(dp->base);
>
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_set_autosuspend_delay(dev, 500);
[Severity: High]
Since dw_dp_bind() enables the clocks (via devm_clk_get_enabled() when
setting up the base dw_dp struct), shouldn't we call
pm_runtime_set_active() before enabling runtime PM here?
Without it, the PM core assumes the device starts in RPM_SUSPENDED state.
When the device resumes, dw_dp_runtime_resume() will enable the clocks
again (incrementing the enable count to 2). When autosuspend fires,
dw_dp_runtime_suspend() will disable them (decrementing the count back to
1), meaning the hardware clocks are never physically disabled.
> + ret = devm_pm_runtime_enable(dev);
[Severity: High]
Does using devm_pm_runtime_enable() here tie the PM lifecycle to the
component platform device instead of the DRM master?
The rockchip_dw_dp structure is allocated using drmm_kzalloc() earlier in
this function, which binds its lifetime to the DRM master device. However,
devm_pm_runtime_enable() attaches the devres action to the component device,
whose lifetime spans the full probe/remove cycle.
If the DRM master is unbound, dw_dp_rockchip_unbind() will execute but it
does not disable PM runtime or flush the autosuspend timer. The DRM core
will then free the drmm-managed memory.
Could an autosuspend timer firing after unbind cause a use-after-free
regression in dw_dp_rockchip_runtime_suspend() when it tries to access the
freed dp pointer? Repeated bind/unbind cycles might also endlessly leak
devres actions on the platform device.
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to enable runtime PM\n");
> +
> connector = drm_bridge_connector_init(drm_dev, encoder);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-synopsys-dw-dp-improvements-v3-0-dc61e6352508@collabora.com?part=8
^ permalink raw reply
* Re: [PATCH 4/9] drm/rockchip: vop2: Consolidate HDMI PHY PLL clock parent switch
From: sashiko-bot @ 2026-06-23 20:40 UTC (permalink / raw)
To: Cristian Ciocaltea; +Cc: dri-devel, conor+dt, devicetree, robh
In-Reply-To: <20260617-dw-hdmi-qp-yuv-v1-4-a665cfd06d7d@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Calling sleepable functions (`mutex_lock` and CCF APIs) inside an atomic context.
--
commit daeb5d187e2f25bcf364f65898a9cf55a75558aa
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
drm/rockchip: vop2: Consolidate HDMI PHY PLL clock parent switch
This commit refactors the DCLK parent switch logic for HDMI0 and HDMI1
PHY PLLs. It consolidates duplicated clk_get_parent() and clk_set_parent()
calls into a single shared code path based on the active HDMI endpoint.
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
[ ... ]
> @@ -1932,42 +1932,34 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
[ ... ]
> + if (pll_hdmiphy) {
> + if (!vp->dclk_src)
> + vp->dclk_src = clk_get_parent(vp->dclk);
> +
> + ret = clk_set_parent(vp->dclk, pll_hdmiphy);
[Severity: High]
This is a pre-existing issue, but does calling clk_set_parent() here
result in a sleep inside an atomic context?
The vop2_crtc_atomic_enable() function executes as a CRTC atomic enable
callback, which runs in an atomic context where sleeping is forbidden.
Both clk_set_parent() and clk_prepare_enable() (called earlier in this
function) acquire sleepable mutexes, and vop2_lock() internally calls
mutex_lock(). Can this lead to kernel warnings or potential deadlocks
during the atomic commit phase?
> + if (ret < 0)
> + drm_warn(vop2->drm,
> + "Failed to switch DCLK to HDMI PHY PLL: %d\n",
> + ret);
> + }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=4
^ permalink raw reply
* Re: [PATCH v5 6/7] arm64: dts: renesas: sparrow-hawk: Enable DisplayPort by adding DSC
From: sashiko-bot @ 2026-06-23 20:37 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: devicetree, dri-devel, wsa+renesas, robh, conor+dt
In-Reply-To: <20260615-rcar-du-dsc-v5-6-aed1a28610e4@ideasonboard.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The rcar_dsc driver ignores the `resets` property from the DT binding, failing to deassert the DSC hardware module's reset state.
--
commit a8616c71b764dc9fa4ae244e47795371e7fead44
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>
drm/rcar-du: dsc: Add rudimentary Renesas R-Car V4H DSC driver
This commit introduces a rudimentary DSC bridge driver for the Renesas R-Car
V4H SoC. The driver functions as a pass-through to enable the DSI1 pipeline
and performs VESA DSC encoding.
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c b/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c
> new file mode 100644
> index 0000000000000..362e683289d69
> --- /dev/null
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_dsc.c
[ ... ]
> +static int rcar_dsc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct rcar_dsc *dsc;
> + int ret;
> +
> + dsc = devm_drm_bridge_alloc(dev, struct rcar_dsc, bridge,
> + &rcar_dsc_bridge_ops);
> + if (IS_ERR(dsc))
> + return PTR_ERR(dsc);
> +
> + platform_set_drvdata(pdev, dsc);
> +
> + dsc->dev = &pdev->dev;
> +
> + dsc->bridge.next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node,
> + 1, 0);
[Severity: High]
Does this driver fail to retrieve and deassert the DSC module's hardware reset
state?
If the reset control isn't retrieved and deasserted, the DSC hardware module
will remain in a reset state, breaking the display pipeline functionality. It
might coincidentally work if the bootloader has already deasserted the reset,
but could fail on suspend/resume or when the bootloader does not initialize
the display.
Should the driver use devm_reset_control_get_exclusive() here and deassert
the reset state during probe?
> + if (IS_ERR(dsc->bridge.next_bridge))
> + return PTR_ERR(dsc->bridge.next_bridge);
> +
> + dsc->bridge.of_node = dev->of_node;
> +
> + pm_runtime_enable(&pdev->dev);
> +
> + ret = devm_drm_bridge_add(dev, &dsc->bridge);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-rcar-du-dsc-v5-0-aed1a28610e4@ideasonboard.com?part=6
^ permalink raw reply
* Re: [PATCH 3/9] drm/rockchip: vop2: Avoid DCLK source switch for 10-bit YUV422 output
From: sashiko-bot @ 2026-06-23 20:33 UTC (permalink / raw)
To: Cristian Ciocaltea; +Cc: conor+dt, robh, dri-devel, devicetree
In-Reply-To: <20260617-dw-hdmi-qp-yuv-v1-3-a665cfd06d7d@collabora.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Division by zero in max_dclk calculation due to uninitialized output_bpc.
- [High] Integer overflow in max_dclk calculation on 32-bit architectures.
- [High] Atomic context violation due to calling sleeping functions.
--
commit cdb8cbbc3fd201db3157d6704b3856b8565e10e2
Author: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
drm/rockchip: vop2: Avoid DCLK source switch for 10-bit YUV422 output
The commit modifies the DCLK source decision logic for HDMI output in VOP2.
It prevents unnecessary switching of the DCLK source to the system CRU for
10-bit YUV422 output by forcing the bandwidth check to 8 bpc.
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 2833fb49ad81a..17d21e08ad977 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -1922,8 +1922,17 @@ static void vop2_crtc_atomic_enable(struct drm_crtc *crtc,
[Severity: High]
This is a pre-existing issue, but does this function violate atomic context
rules?
According to the DRM subsystem guidelines, CRTC atomic enable callbacks execute
in atomic context where sleeping is forbidden. However, vop2_crtc_atomic_enable()
acquires a mutex via vop2_lock() and later calls clk_prepare_enable(), both of
which can sleep.
Can this cause scheduling while atomic warnings or deadlocks during atomic
commits?
> * to 4K@60Hz, if available, otherwise keep using the system CRU.
> */
> if (vop2->pll_hdmiphy0 || vop2->pll_hdmiphy1) {
> - unsigned long max_dclk = DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8,
> - vcstate->output_bpc);
> + /*
> + * YUV422 always transmits two 12-bit components per clock
> + * cycle, regardless of the color depth, which from a rate
> + * perspective is equivalent to three 8-bit RGB components.
> + * Force 8 bpc here so the bandwidth check reflects the actual
> + * TMDS rate and avoids an unnecessary DCLK source switch.
> + */
> + unsigned int bpc = vcstate->output_mode == ROCKCHIP_OUT_MODE_YUV422 ?
> + 8 : vcstate->output_bpc;
> + unsigned long max_dclk = DIV_ROUND_CLOSEST_ULL(VOP2_MAX_DCLK_RATE * 8, bpc);
[Severity: High]
This isn't a bug introduced by this patch, but can bpc be 0 here and cause a
division by zero?
If the VOP2 driver is used with a non-HDMI output like DSI or LVDS on a board
that also has an HDMI PHY, vop2->pll_hdmiphy0 or vop2->pll_hdmiphy1 will be
non-NULL. Since DSI/LVDS encoders do not set vcstate->output_bpc, it defaults
to 0.
This would lead to DIV_ROUND_CLOSEST_ULL(..., 0) before the loop checks if
the endpoint is HDMI.
[Severity: High]
This is also a pre-existing issue, but does VOP2_MAX_DCLK_RATE * 8 overflow on
32-bit architectures?
VOP2_MAX_DCLK_RATE is defined as 600000000UL. On architectures where
unsigned long is 32 bits, 600000000UL * 8 equals 4,800,000,000, which exceeds
ULONG_MAX.
This truncates to 505,032,704 before being promoted to unsigned long long inside
DIV_ROUND_CLOSEST_ULL, improperly forcing higher resolutions to fall back to the
system CRU.
Should this be cast to a 64-bit type first, such as VOP2_MAX_DCLK_RATE * 8ULL?
> if (clock <= max_dclk) {
> drm_for_each_encoder_mask(encoder, crtc->dev, crtc_state->encoder_mask) {
> struct rockchip_encoder *rkencoder = to_rockchip_encoder(encoder);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-dw-hdmi-qp-yuv-v1-0-a665cfd06d7d@collabora.com?part=3
^ permalink raw reply
* Re: [PATCH v3 07/10] drm/bridge: synopsys: dw-dp: Add Runtime PM support
From: sashiko-bot @ 2026-06-23 20:31 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: robh, conor+dt, dri-devel, devicetree
In-Reply-To: <20260612-synopsys-dw-dp-improvements-v3-7-dc61e6352508@collabora.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] Unchecked return value of clk_prepare_enable() in runtime resume.
- [High] Unchecked return value of pm_runtime_get_sync() during atomic enable.
- [High] Interrupt handler accesses hardware registers while device may be suspended.
Pre-existing issues:
- [High] Memory/refcount leak of the next_bridge acquired via of_drm_get_bridge_by_endpoint().
- [High] PHY power_count underflow causing permanent PHY failure after early return in atomic_enable.
- [High] Uninitialized stack memory copied to buffer in dw_dp_aux_read_data().
--
commit 25b9778935f0029deadc6a4ac4d19728b07017ee
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: drm/bridge: synopsys: dw-dp: Add Runtime PM support
The patch adds runtime PM stubs to the Synopsys DesignWare DisplayPort bridge
driver and hooks them up to various hardware access paths.
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 7fa38145e35ce..7f4f36c614844 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> @@ -1465,6 +1465,8 @@ static ssize_t dw_dp_aux_transfer(struct drm_dp_aux *aux,
> if (WARN_ON(msg->size > 16))
> return -E2BIG;
>
> + ACQUIRE(pm_runtime_active_auto, pm)(dp->dev);
> +
[Severity: High]
This is a pre-existing issue, but I noticed a potential information leak in
dw_dp_aux_read_data() which is called during these transfers:
drivers/gpu/drm/bridge/synopsys/dw-dp.c:dw_dp_aux_read_data() {
...
u32 value;
regmap_read(dp->regmap, DW_DP_AUX_DATA0 + i * 4, &value);
for (j = 0; j < num; j++)
buffer[i * 4 + j] = value >> (j * 8);
...
}
If regmap_read() fails, the local variable "value" remains uninitialized.
Could this copy uninitialized kernel stack memory into the buffer, which is
then returned to userspace via the DRM DP AUX chardev?
> switch (msg->request & ~DP_AUX_I2C_MOT) {
> case DP_AUX_NATIVE_WRITE:
> case DP_AUX_I2C_WRITE:
[ ... ]
> @@ -1655,6 +1657,8 @@ static void dw_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> struct drm_connector_state *conn_state;
> int ret;
>
> + pm_runtime_get_sync(dp->dev);
> +
[Severity: High]
Does this code need to check the return value of pm_runtime_get_sync()?
If the device fails to resume (for example, if the clocks fail to enable),
the driver will continue with hardware initialization routines like
dw_dp_link_enable(). Can this lead to a crash by accessing a suspended
device?
> connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> if (!connector) {
> dev_err(dp->dev, "failed to get connector\n");
> return;
> }
[Severity: High]
This is a pre-existing issue, but does returning early here cause a
phy->power_count underflow later?
If we return early because connector is NULL, or if dw_dp_link_enable() fails,
we skip powering on the PHY. However, because atomic_enable returns void,
the DRM framework will still consider the bridge enabled and later call
dw_dp_bridge_atomic_disable(), which unconditionally executes:
phy_power_off(dp->phy);
Could this drop the PHY's power_count below zero and permanently prevent it
from powering on during subsequent modesets?
[ ... ]
> @@ -2153,6 +2160,26 @@ void dw_dp_unbind(struct dw_dp *dp)
> }
> EXPORT_SYMBOL_GPL(dw_dp_unbind);
>
> +int dw_dp_runtime_suspend(struct dw_dp *dp)
> +{
> + clk_disable_unprepare(dp->aux_clk);
> + clk_disable_unprepare(dp->apb_clk);
> +
> + return 0;
> +}
[Severity: High]
Is it safe to leave the IRQ enabled while the device is suspended?
If a spurious interrupt or a hot-plug event fires while the device is in
the RPM_SUSPENDED state, dw_dp_irq() will execute and unconditionally call:
regmap_read(dp->regmap, DW_DP_GENERAL_INTERRUPT, &value);
Since the APB clock is disabled here, might this MMIO read hang the bus
and cause a kernel panic?
> +EXPORT_SYMBOL_GPL(dw_dp_runtime_suspend);
> +
> +int dw_dp_runtime_resume(struct dw_dp *dp)
> +{
> + clk_prepare_enable(dp->apb_clk);
> + clk_prepare_enable(dp->aux_clk);
> +
[Severity: High]
Should the return values of clk_prepare_enable() be checked?
If the clocks fail to enable, the function proceeds to call dw_dp_init_hw()
which accesses hardware registers via regmap. Could this lead to a bus fault?
> + dw_dp_init_hw(dp);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(dw_dp_runtime_resume);
There is also one additional pre-existing issue in a part of the file not
modified by this patch:
[Severity: High]
This is a pre-existing issue, but I noticed a potential leak in dw_dp_bind():
drivers/gpu/drm/bridge/synopsys/dw-dp.c:dw_dp_bind() {
...
next_bridge = of_drm_get_bridge_by_endpoint(dev->of_node, 1, 0);
if (IS_ERR(next_bridge)) {
...
ret = drm_bridge_attach(encoder, next_bridge, bridge,
DRM_BRIDGE_ATTACH_NO_CONNECTOR);
...
}
Does this code leak the reference count for next_bridge?
of_drm_get_bridge_by_endpoint() returns a bridge with an incremented refcount,
and drm_bridge_attach() takes its own reference. Should there be a call to
drm_bridge_put() to drop the initial reference so the next bridge module can
be unloaded?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-synopsys-dw-dp-improvements-v3-0-dc61e6352508@collabora.com?part=7
^ 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