* Re: [PATCH v5 1/5] dt-bindings: arm: qcom: Document Shikra and its EVK boards
From: Komal Bajaj @ 2026-06-12 17:18 UTC (permalink / raw)
To: Rob Herring
Cc: Bjorn Andersson, Konrad Dybcio, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Neil Armstrong, Wesley Cheng, Ulf Hansson,
linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
monish.chunara
In-Reply-To: <20260611181841.GA3163205-robh@kernel.org>
On 6/11/2026 11:48 PM, Rob Herring wrote:
> On Thu, Jun 11, 2026 at 03:40:08PM +0530, Komal Bajaj wrote:
>> Shikra is a Qualcomm IoT SoC available in a System-on-Module (SoM)
>> form factor. The SoM integrates the Shikra SoC, PMICs, and essential
>> passives, and is designed to be mounted on carrier boards.
>>
>> Three eSoM variant are introduced:
>> - CQM: retail variant with integrated modem (PM4125 and PM8005 PMIC)
>> - CQS: retail variant without modem (PM4125 and PM8005 PMIC)
>> - IQS: industrial-grade variant without modem (PM8150 PMIC)
>>
>> Each SoM variant pairs with a common EVK carrier board provides debug
>> UART, USB, and other peripheral interfaces.
>>
>> Add compatible strings for the CQ2390M, CQ2390S, IQ2390S SoM variant and
>> its corresponding EVK boards.
>>
>> Signed-off-by: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
> Missing Krzysztof's reviewed-by.
Missed that. Will add his reviewed-by in next revision.
Thanks
Komal
^ permalink raw reply
* Re: [PATCH v5 17/18] phy: rockchip: usbdp: Support going from DP-only mode to USB mode
From: sashiko-bot @ 2026-06-12 17:08 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, devicetree, olteanv, conor+dt, robh, vkoul,
neil.armstrong
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-17-efc83069869f@collabora.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Medium] The return value of `rk_udphy_power_on()` is silently ignored during Type-C mux state transitions.
- [Medium] The `usb_in_use` flag is permanently left as `true` if `rk_udphy_power_on()` fails during USB PHY initialization.
Pre-existing issues:
- [High] The `UDPHY_MODE_USB` bit in `udphy->status` is permanently leaked if `rk_udphy_usb3_phy_exit()` is called while the Type-C mux is currently in DP-only mode.
- [High] Failing to reinitialize the PHY after a transient error masks subsequent failures and reports false success.
--
commit 3ea1c2acafe2e85bbef04dfbb4e06950b88402b0
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Support going from DP-only mode to USB mode
When a USB-C adapter, which maps all Superspeed lanes to DP is plugged
in, the USB support is disabled in the PHY. When the adapter is
unplugged and a different adapter with USB functionality is plugged in
afterwards, USB functionality is not restored as the USB controller
keeps the PHY enabled for the entire time.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1015,6 +1016,9 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> ret = rk_udphy_init(udphy);
> if (ret)
> return ret;
[Severity: High]
This is a pre-existing issue, but if rk_udphy_init() fails here,
udphy->phy_needs_reinit was already set to false earlier in the block.
Will subsequent calls to rk_udphy_power_on() skip initialization and
falsely return success while the hardware remains broken?
Should phy_needs_reinit be restored to true on the error path?
> +
> + if (udphy->mode & UDPHY_MODE_USB)
> + rk_udphy_u3_port_disable(udphy, false);
> }
>
> udphy->status |= mode;
[ ... ]
> @@ -1278,6 +1282,15 @@ static int rk_udphy_usb3_phy_init(struct phy *phy)
>
> guard(mutex)(&udphy->mutex);
>
> + udphy->usb_in_use = true;
> +
> /* DP only or high-speed, disable U3 port */
> if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> rk_udphy_u3_port_disable(udphy, true);
> return 0;
> }
>
> ret = rk_udphy_power_on(udphy, UDPHY_MODE_USB);
>
> return ret;
> }
[Severity: Medium]
If rk_udphy_power_on() fails and returns an error, usb_in_use remains true.
Will this cause rk_udphy_typec_mux_set() to incorrectly attempt to power on
the USB PHY later, despite the USB controller not actively using it?
Should the flag be reverted to false on the error path?
> @@ -1295,6 +1301,13 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
>
> guard(mutex)(&udphy->mutex);
>
> + udphy->usb_in_use = false;
> +
> /* DP only or high-speed */
> if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
> return 0;
[Severity: High]
This isn't a bug introduced by this patch, but if the Type-C mode
transitions to DP-only, udphy->mode loses the UDPHY_MODE_USB bit.
When rk_udphy_usb3_phy_exit() is subsequently called, it checks
!(udphy->mode & UDPHY_MODE_USB) and returns 0 early, skipping
rk_udphy_power_off().
Does this result in a permanent leak of the PHY's clocks and resets, since
udphy->status never clears the UDPHY_MODE_USB bit?
>
> rk_udphy_power_off(udphy, UDPHY_MODE_USB);
>
> return 0;
> }
[ ... ]
> @@ -1325,8 +1334,20 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>
> guard(mutex)(&udphy->mutex);
>
> + old_mode = udphy->mode;
> +
> rk_udphy_set_typec_state(udphy, state->mode);
>
> + /*
> + * If the new mode includes USB but the old one didn't (e.g. leaving
> + * DP-only), and the USB PHY was already initialized by the USB
> + * controller, we need to power on the USB side now since no
> + * subsequent phy_init call will come from the controller.
> + */
> + if ((udphy->mode & UDPHY_MODE_USB) && !(old_mode & UDPHY_MODE_USB) &&
> + udphy->usb_in_use && !udphy->hs)
> + rk_udphy_power_on(udphy, UDPHY_MODE_USB);
[Severity: Medium]
Does this code discard the return value of rk_udphy_power_on()?
If initialization fails, the error is ignored and the driver reports success
for the Type-C mux state transition, which could leave the USB functionality
silently broken.
Should this check the return value and at least log or propagate the error?
> +
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=17
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add TI ADS1220
From: David Lechner @ 2026-06-12 17:07 UTC (permalink / raw)
To: Nguyen Minh Tien, Jonathan Cameron
Cc: linux-iio, devicetree, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Nuno Sá, Andy Shevchenko, linux-kernel
In-Reply-To: <788b7c6f-fae8-47cd-b78a-33f2803d8c88@baylibre.com>
On 6/12/26 11:10 AM, David Lechner wrote:
> On 6/10/26 10:13 AM, Nguyen Minh Tien wrote:
>> The ADS1220 is a 24-bit, 2-kSPS, 4-channel delta-sigma ADC from Texas
>> Instruments with an SPI (mode 1) interface, a programmable gain amplifier,
>> an internal 2.048V reference and a dedicated DRDY data-ready output.
>>
...
>> +patternProperties:
>> + "^channel@[0-6]$":
>> + $ref: adc.yaml
>> + type: object
>> + description: Represents one ADC input configuration (channel).
>> +
>> + properties:
>> + reg:
>> + minimum: 0
>> + maximum: 6
>
> I would add a comment explaining that the limit here is arbitrary. Also wouldn't
> hurt to make it a bit bigger. Technically, someone could have every possible
> combination of all inputs (16 differential + 4 single-ended).
From the dataheet, it looks like there are 12 possible valid
channel settings rather than 7.
>
>> +
>> + 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]
>
> Sometimes, the same channel may be used as both positive and negative
> for a diagnostic, so I would just allow any combination instead of
> listing specific combinations.
>
And now that I read the datasheet better, I guess this is fine.
^ permalink raw reply
* Re: [PATCH v5 18/18] phy: rockchip: usbdp: Add some extra debug messages
From: sashiko-bot @ 2026-06-12 17:06 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, conor+dt, neil.armstrong, olteanv, vkoul, linux-phy,
robh
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-18-efc83069869f@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing explicit inclusion of `<linux/string_choices.h>` for string choice helpers.
--
commit 1df9f844b4fa19da9efe1fba10ad8a1d451bea55
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Add some extra debug messages
This commit adds additional debug logging to the Rockchip USBDP PHY
driver. The new messages report port status and track PHY initialization
to help debug USB-C hotplugging behavior.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 236331cc0d132..4042e2dd81219 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -491,6 +491,8 @@ static void rk_udphy_u3_port_disable(struct rk_udphy *udphy, u8 disable)
> const struct rk_udphy_cfg *cfg = udphy->cfgs;
> const struct rk_udphy_grf_reg *preg;
>
> + dev_dbg(udphy->dev, "USB3 port %s\n", str_on_off(!disable));
[Severity: Low]
Does this file explicitly include <linux/string_choices.h>? Relying on
implicit inclusion for str_on_off() might cause build failures in the future
if header dependencies are reorganized across architectures.
[ ... ]
> @@ -784,6 +786,10 @@ static int rk_udphy_init(struct rk_udphy *udphy)
> const struct rk_udphy_cfg *cfg = udphy->cfgs;
> int ret;
>
> + dev_dbg(udphy->dev, "(re-)init PHY with USB=%s and DP=%s\n",
> + str_enabled_disabled(udphy->mode & UDPHY_MODE_USB),
> + str_enabled_disabled(udphy->mode & UDPHY_MODE_DP));
[Severity: Low]
Similarly, should we add <linux/string_choices.h> here to ensure
str_enabled_disabled() is safely resolved without relying on transitive
includes?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=18
^ permalink raw reply
* Re: [PATCH v5 14/18] phy: rockchip: usbdp: Re-init the PHY on orientation change
From: sashiko-bot @ 2026-06-12 17:03 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, devicetree, vkoul, olteanv, linux-phy, robh,
conor+dt
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-14-efc83069869f@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] rk_udphy_dp_phy_configure accesses shared state and modifies PHY registers without holding udphy->mutex, racing with orientation switches and PHY resets.
- [High] udphy->phy_needs_reinit is cleared before initialization but is not restored if rk_udphy_init fails, permanently breaking the PHY.
--
commit c7d9d38e9a690c6a3592fb9c0e4736966a879674
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Re-init the PHY on orientation change
Changing the cable orientation reconfigures the lane muxing, which requires
re-initializing the PHY. This patch sets the phy_needs_reinit flag when the
cable orientation is swapped to ensure the PHY is reset and reconfigured
appropriately.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index a3b4e2e0f5786..89a08267611c5 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -633,7 +634,10 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> goto unlock_ret;
> }
>
> - udphy->flip = orien == TYPEC_ORIENTATION_REVERSE;
> + if (udphy->flip != flipped)
> + udphy->phy_needs_reinit = true;
[Severity: High]
This isn't a bug introduced by this patch, but setting
udphy->phy_needs_reinit here interacts with an error handling issue.
In drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_on(), the flag
is eagerly cleared before initialization:
rk_udphy_power_on() {
...
} else if (udphy->phy_needs_reinit) {
udphy->phy_needs_reinit = false;
...
ret = rk_udphy_init(udphy);
if (ret)
return ret;
...
}
If rk_udphy_init() fails (for example, due to a retryable PLL lock timeout
returning -EPROBE_DEFER), the flag is not restored to true. Since the
hardware is held in reset on failure, will subsequent calls to
rk_udphy_power_on() skip reinitialization entirely and falsely report
success while leaving both USB and DP functionally dead?
> +
> + udphy->flip = flipped;
> rk_udphy_set_typec_default_mapping(udphy);
[Severity: High]
This is a pre-existing issue, but can this state update race with DP PHY
configuration?
This function holds udphy->mutex while updating state like udphy->flip and
lane selections in rk_udphy_set_typec_default_mapping(). However,
rk_udphy_dp_phy_configure() is called by the generic PHY framework holding
only the DP-specific phy->mutex:
rk_udphy_dp_phy_configure() {
struct rk_udphy *udphy = phy_get_drvdata(phy);
// ... accesses udphy->dp_lane_sel without taking udphy->mutex
...
}
If a concurrent Type-C mode switch or USB PHY initialization holds
udphy->mutex and triggers rk_udphy_init(), it will assert all hardware
resets:
rk_udphy_init() {
...
rk_udphy_reset_assert_all(udphy);
...
}
Because rk_udphy_dp_phy_configure() does not acquire udphy->mutex, can its
register writes interleave with a full hardware reset, leaving the DP PHY
unconfigured and causing link training failures?
> rk_udphy_usb_bvalid_enable(udphy, true);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=14
^ permalink raw reply
* Re: [PATCH 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor
From: Jonathan Cameron @ 2026-06-12 17:02 UTC (permalink / raw)
To: Siratul Islam
Cc: robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy, linux-iio,
devicetree, linux-kernel
In-Reply-To: <20260612124557.13750-4-email@sirat.me>
On Fri, 12 Jun 2026 18:45:27 +0600
Siratul Islam <email@sirat.me> wrote:
> Add driver for the QST QMC5883L 3-Axis Magnetic Sensor
> connected via i2c.
>
> Signed-off-by: Siratul Islam <email@sirat.me>
Trying to avoid repeating stuff Joshua already covered.
Various comments inline.
Thanks,
Jonathan
> diff --git a/drivers/iio/magnetometer/qmc5883l.c b/drivers/iio/magnetometer/qmc5883l.c
> new file mode 100644
> index 000000000000..055e51570635
> --- /dev/null
> +++ b/drivers/iio/magnetometer/qmc5883l.c
> +
> +#define QMC5883L_REG_X_LSB 0x00
> +#define QMC5883L_REG_STATUS1 0x06
> +#define QMC5883L_REG_CTRL1 0x09
> +#define QMC5883L_REG_CTRL2 0x0A
> +#define QMC5883L_REG_SET_RESET 0x0B
> +#define QMC5883L_REG_ID 0x0D
> +
> +#define QMC5883L_CHIP_ID 0xFF
> +
> +#define QMC5883L_MODE_MASK GENMASK(1, 0)
> +#define QMC5883L_ODR_MASK GENMASK(3, 2)
> +#define QMC5883L_RNG_MASK GENMASK(5, 4)
> +#define QMC5883L_OSR_MASK GENMASK(7, 6)
> +
> +#define QMC5883L_MODE_STANDBY FIELD_PREP_CONST(QMC5883L_MODE_MASK, 0x00)
> +#define QMC5883L_MODE_CONT FIELD_PREP_CONST(QMC5883L_MODE_MASK, 0x01)
> +
> +#define QMC5883L_ODR_10HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x00)
> +#define QMC5883L_ODR_50HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x01)
> +#define QMC5883L_ODR_100HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x02)
> +#define QMC5883L_ODR_200HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x03)
> +
> +#define QMC5883L_RNG_2G FIELD_PREP_CONST(QMC5883L_RNG_MASK, 0x00)
> +#define QMC5883L_RNG_8G FIELD_PREP_CONST(QMC5883L_RNG_MASK, 0x01)
> +
> +#define QMC5883L_OSR_512 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x00)
> +#define QMC5883L_OSR_256 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x01)
> +#define QMC5883L_OSR_128 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x02)
> +#define QMC5883L_OSR_64 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x03)
These are used for matching - normally we'd add defines for the filed value and
then use FIELD_GET() to extract it for matching.
e.g.
#define QMC5883L_OSR_512 0x0
#define QMC5883L_OSR_256 0x1
rather these.
> +
> +#define QMC5883L_STATUS_DRDY BIT(0)
> +#define QMC5883L_STATUS_OVL BIT(1)
> +
> +#define QMC5883L_SET_RESET_VAL BIT(0)
> +#define QMC5883L_INT_DISABLE BIT(0)
> +#define QMC5883L_SOFT_RESET BIT(7)
> +
> +#define QMC5883L_SCALE_2G 83333
> +#define QMC5883L_SCALE_8G 333333
> +
> +/* POR completion time max per datasheet */
> +#define QMC5883L_PORT_US 350
> +
> +struct qmc5883l_data {
> + struct regmap *regmap;
> + struct mutex mutex; /* update and read regmap data */
Need more than that. regmap has its own locks that do this bit.
Be sure to describe exactly what data you are protecting.
Usually it is something like read / modify / write cycles or
need to serialize groupd of actions.
> + u8 range;
> + u8 odr;
> + u8 osr;
> +};
> +
> +enum qmc5883l_chan {
> + QMC5883L_AXIS_X,
> + QMC5883L_AXIS_Y,
> + QMC5883L_AXIS_Z,
> +};
> +
> +static const int qmc5883l_odr_avail[] = { 10, 50, 100, 200 };
> +
> +static const int qmc5883l_osr_avail[] = { 512, 256, 128, 64 };
> +
> +static const int qmc5883l_rng_avail[] = {
> + 0, QMC5883L_SCALE_2G, /* 2G */
I'm not sure the defines really help. Perhaps push the value down here
and then look it up from this array when matching.
> + 0, QMC5883L_SCALE_8G, /* 8G */
> +};
> +
> +static int qmc5883l_read_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int *val,
> + int *val2, long mask)
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (!iio_device_claim_direct(indio_dev))
What is this serializing? In a driver that only supports direct mode it should
never be necessary to claim it. So this code should only be added in a patch
adding buffered modes. If you need to serialize, most likely you should be using
a local lock as it has nothing to do with state changes from direct to buffered.
> + return -EBUSY;
> + ret = qmc5883l_take_measurement(indio_dev, chan->address, val);
> + iio_device_release_direct(indio_dev);
> + if (ret)
> + return ret;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
> + scoped_guard(mutex, &data->mutex)
Joshua already mentioned this, but prefer guard() and defined scope
for each of these case blocks. That just ends up easier to read.
> + {
> + *val = 0;
> + *val2 = data->range == QMC5883L_RNG_2G ?
> + QMC5883L_SCALE_2G :
> + QMC5883L_SCALE_8G;
> + }
> + return IIO_VAL_INT_PLUS_NANO;
...
> +
> +static int qmc5883l_write_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int val,
> + int val2, long mask)
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + u8 rng;
> + u8 osr;
> + u8 odr;
Can combine as
u8 rng, osr, odr;
without loosing readability so do that to save a few lines of scrolling!
> + int ret;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
Add scope by doing
case IIO_CHAN_INFO_OVERSAMPLING_RATIO: {
Same applies for other cases.
> + switch (val) {
> + case 64:
> + osr = QMC5883L_OSR_64;
> + break;
> + case 128:
> + osr = QMC5883L_OSR_128;
> + break;
> + case 256:
> + osr = QMC5883L_OSR_256;
> + break;
> + case 512:
> + osr = QMC5883L_OSR_512;
> + break;
> + default:
> + return -EINVAL;
> + }
> + scoped_guard(mutex, &data->mutex)
then this can be a guard.
Note that if it were a scoped_guard() that should be treated like an if ()
so formatting wise it would be
scoped_guard(mutex, &data->mutex) {
However with scope added as suggested this can be
guard(mutex)(&data->mutex);
and avoid the need for greater indent.
> + {
> + ret = regmap_update_bits(data->regmap,
> + QMC5883L_REG_CTRL1,
> + QMC5883L_OSR_MASK, osr);
See above,
QMC5883L_OSR_MASK,
FIELD_PREP(QMC5883L_OSR_MASK, osr));
> + if (ret)
> + return ret;
> + data->osr = osr;
Here store the field value not the shifted version.
> + }
> + break;
return 0; (see below)
Close scope with
}
here.
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
Probably better to return in the good paths above and save us having
to go see if there is anything else to be done.
> +}
> +static int qmc5883l_init(struct qmc5883l_data *data)
> +{
> + unsigned int reg;
> + int ret;
> +
> + ret = regmap_read(data->regmap, QMC5883L_REG_ID, ®);
> + if (ret)
> + return ret;
> +
> + /* Not failing because rev 1.0 had this register reserved */
> + if (reg != QMC5883L_CHIP_ID)
> + dev_warn(regmap_get_device(data->regmap),
> + "unknown chip id: 0x%02x, continuing\n", reg);
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL2,
> + QMC5883L_SOFT_RESET);
> + if (ret)
> + return ret;
> +
> + fsleep(QMC5883L_PORT_US);
> +
> + /* DRDY pin no used in this version of the driver */
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL2,
> + QMC5883L_INT_DISABLE);
I don't mind if these sorts of cases go a little over 80 chars as sometimes
it helps readability.
Does it really reset with interrupts on? That's odd. Mind you the
INT_ENB sounds like it would be an enable but as you have named it here
it is actually a disable so all bets are off when it comes to sensible ;)
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_SET_RESET,
> + QMC5883L_SET_RESET_VAL);
> + if (ret)
> + return ret;
> +
> + data->odr = QMC5883L_ODR_50HZ;
> + data->range = QMC5883L_RNG_2G;
> + data->osr = QMC5883L_OSR_64;
Generally should only set these after the write succeeds otherwise on error
these are out of sync with the device. Not that important though in an init
function as any error leads to us bailing out anyway.
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL1,
> + (QMC5883L_MODE_CONT | data->odr | data->range |
> + data->osr));
return regmap_write();
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +static const struct of_device_id qmc5883l_match[] = {
> + { .compatible = "qstcorp,qmc5883l" },
> + { },
As below, no comma.
> +};
> +MODULE_DEVICE_TABLE(of, qmc5883l_match);
> +
> +static const struct i2c_device_id qmc5883l_id[] = {
> + { "qmc5883l" },
Named initializers for these please. Uwe is doing some work
to ensure consistency on this across there kernel and queued
up for this kernel cycle is a patch that does that for all the
i2c_device_id tables. Uwe mentioned that a checkpatch change
to check for this was on the todo list.
> + { },
No trailing comma on 'terminating' entries like this.
Whatever entries are added in the future they must not come
after this so comma here is never appropriate.
> +};
> +MODULE_DEVICE_TABLE(i2c, qmc5883l_id);
> +
> +static struct i2c_driver qmc5883l_driver = {
> + .driver = {
> + .name = "qmc5883l",
> + .of_match_table = qmc5883l_match,
> + },
> + .id_table = qmc5883l_id,
> + .probe = qmc5883l_probe,
> +};
> +
Trivial but a common convention (that I'm trying to encourage in IIO)
is no blank line here. It is good to keep the tighter coupling between
the structure and the macro that is its only user.
> +module_i2c_driver(qmc5883l_driver);
^ permalink raw reply
* Re: [PATCH v2 2/5] dt-bindings: display: bridge: Document Renesas R-Car V4H DSC bindings
From: Laurent Pinchart @ 2026-06-12 17:01 UTC (permalink / raw)
To: Conor Dooley
Cc: Tomi Valkeinen, Geert Uytterhoeven, Michael Turquette,
Stephen Boyd, Andrzej Hajda, Neil Armstrong, Robert Foss,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Marek Vasut,
Kieran Bingham, Philipp Zabel, linux-renesas-soc, linux-clk,
linux-kernel, dri-devel, devicetree
In-Reply-To: <20260612-landed-remedial-79582e900699@spud>
On Fri, Jun 12, 2026 at 05:09:48PM +0100, Conor Dooley wrote:
> On Fri, Jun 12, 2026 at 01:43:44PM +0300, Tomi Valkeinen wrote:
> > On 15/05/2026 20:32, Conor Dooley wrote:
> > > On Fri, May 15, 2026 at 10:56:15AM +0300, Tomi Valkeinen wrote:
> > > > From: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > >
> > > > The Renesas DSC Display Stream Compression is a bridge embedded in the
> > > > Renesas R-Car V4H SoC. The bridge performs VESA DSC encoding of up to
> > > > 8k or 400 Mpixel/s .
> > > >
> > > > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > > > [tomi.valkeinen: fix the example]
> > > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@ideasonboard.com>
> > > > ---
> > > > .../bindings/display/bridge/renesas,dsc.yaml | 96 ++++++++++++++++++++++
> > > > 1 file changed, 96 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsc.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsc.yaml
> > > > new file mode 100644
> > > > index 000000000000..2918d592732b
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsc.yaml
> > >
> > > Filename matching the compatible please.
> >
> > All the other Documentation/devicetree/bindings/display/bridge/renesas,*
> > files follow the same style, where the file name is in a generic format, but
> > the actual compat strings are per SoC (and no generic compat string).
>
> No idea why it's like that currently, but filename matching compatible
> is the policy.
I wonder if we should use
compatible:
items:
- enum:
- renesas,r8a779g0-dsc
- const: renesas,rcar-dsc
to prepare for the other SoCs that include a DSC. It's of course hard to
predict how compatible all new silicon will be, but we know there will
be some compatible ones.
This being said, looking for instance at renesas,du.yaml in the same
directory, we have
compatible:
enum:
- renesas,du-r8a7742 # for RZ/G1H compatible DU
- renesas,du-r8a7743 # for RZ/G1M compatible DU
- renesas,du-r8a7744 # for RZ/G1N compatible DU
- renesas,du-r8a7745 # for RZ/G1E compatible DU
- renesas,du-r8a77470 # for RZ/G1C compatible DU
- renesas,du-r8a774a1 # for RZ/G2M compatible DU
- renesas,du-r8a774b1 # for RZ/G2N compatible DU
- renesas,du-r8a774c0 # for RZ/G2E compatible DU
- renesas,du-r8a774e1 # for RZ/G2H compatible DU
- renesas,du-r8a7779 # for R-Car H1 compatible DU
- renesas,du-r8a7790 # for R-Car H2 compatible DU
- renesas,du-r8a7791 # for R-Car M2-W compatible DU
- renesas,du-r8a7792 # for R-Car V2H compatible DU
- renesas,du-r8a7793 # for R-Car M2-N compatible DU
- renesas,du-r8a7794 # for R-Car E2 compatible DU
- renesas,du-r8a7795 # for R-Car H3 compatible DU
- renesas,du-r8a7796 # for R-Car M3-W compatible DU
- renesas,du-r8a77961 # for R-Car M3-W+ compatible DU
- renesas,du-r8a77965 # for R-Car M3-N compatible DU
- renesas,du-r8a77970 # for R-Car V3M compatible DU
- renesas,du-r8a77980 # for R-Car V3H compatible DU
- renesas,du-r8a77990 # for R-Car E3 compatible DU
- renesas,du-r8a77995 # for R-Car D3 compatible DU
- renesas,du-r8a779a0 # for R-Car V3U compatible DU
- renesas,du-r8a779g0 # for R-Car V4H compatible DU
- renesas,du-r8a779h0 # for R-Car V4M compatible DU
Could the file have been named renesas,du-r8a7790.yaml ? Sure. Would it
be better for users than renesas,du.yaml ? I'm less sure.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v5 07/18] phy: rockchip: usbdp: Support single-lane DP
From: sashiko-bot @ 2026-06-12 16:55 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, robh, conor+dt, neil.armstrong, olteanv, devicetree,
vkoul
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-7-efc83069869f@collabora.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Medium] Incorrect USB orientation (`flip`) configuration when 1-lane DP is assigned to lane 1.
Pre-existing issues:
- [High] Writing to PMA registers while PHY clocks are disabled when DP mode is not supported.
- [High] Clock resources enabled during probe are leaked on probe error paths and upon driver unbind.
- [High] The DP PHY configuration lacks boundary checks against the physical lane count, leading to USB register corruption.
--
commit c46d73844b37e1f374324a737da2265a3fef0813
Author: Zhang Yubing <yubing.zhang@rock-chips.com>
Subject: phy: rockchip: usbdp: Support single-lane DP
This commit implements support for using a single DisplayPort line in the
Rockchip USBDP PHY driver.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index febc148a754e0..bf8394174294e 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -923,7 +910,8 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
> }
>
> udphy->mode = UDPHY_MODE_DP;
> - if (num_lanes == 2) {
> + udphy->dp_lanes = num_lanes;
> + if (num_lanes == 1 || num_lanes == 2) {
> udphy->mode |= UDPHY_MODE_USB;
> udphy->flip = (udphy->lane_mux_sel[0] == PHY_LANE_MUX_DP);
> }
[Severity: Medium]
When configuring a single DP lane, does this correctly set the flip state if
the DP lane is assigned to lane 1 instead of lane 0?
For a 1-lane DP setup where rockchip,dp-lane-mux = <1>, lane 1 is occupied
by DP, so USB must be mapped to the lane 2 and 3 pair. This requires the
flip state to be true so the driver correctly polls lane 2 for USB RX CDR
lock.
However, in this scenario lane_mux_sel[0] will not be PHY_LANE_MUX_DP, making
flip evaluate to false. This might cause the driver to poll the unconnected
lane 0 for USB lock and time out. Should this condition check if either lane
0 or lane 1 is occupied by DP?
[ ... ]
> @@ -1074,18 +1062,17 @@ static int rk_udphy_dp_phy_exit(struct phy *phy)
> static int rk_udphy_dp_phy_power_on(struct phy *phy)
> {
> struct rk_udphy *udphy = phy_get_drvdata(phy);
> - int ret, dp_lanes;
> + int ret;
>
> mutex_lock(&udphy->mutex);
>
> - dp_lanes = rk_udphy_dplane_get(udphy);
> - phy_set_bus_width(phy, dp_lanes);
> + phy_set_bus_width(phy, udphy->dp_lanes);
>
> ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
> if (ret)
> goto unlock;
>
> - rk_udphy_dplane_enable(udphy, dp_lanes);
> + rk_udphy_dplane_enable(udphy, udphy->dp_lanes);
[Severity: High]
This is a pre-existing issue, but can this sequence crash the system if the
DP mode is not supported?
In rk_udphy_power_on(), if the mode does not include UDPHY_MODE_DP, it
prints an error message and returns 0 instead of an error code:
rk_udphy_power_on() {
if (!(udphy->mode & mode)) {
dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
return 0;
}
...
}
Because it returns success, the execution falls through in
rk_udphy_dp_phy_power_on() and proceeds to call rk_udphy_dplane_enable().
This performs register operations on memory-mapped PHY registers. Accessing
these registers while the corresponding PHY clocks remain disabled can
result in a synchronous external abort and crash the kernel. Should
rk_udphy_power_on() return an error like -ENODEV to properly abort this
sequence?
[ ... ]
> @@ -1529,7 +1518,7 @@ static int rk_udphy_probe(struct platform_device *pdev)
> ret = PTR_ERR(udphy->phy_dp);
> return dev_err_probe(dev, ret, "failed to create DP phy\n");
> }
> - phy_set_bus_width(udphy->phy_dp, rk_udphy_dplane_get(udphy));
> + phy_set_bus_width(udphy->phy_dp, udphy->dp_lanes);
[Severity: High]
This is a pre-existing issue, but does the probe sequence leak clock
resources on error paths?
In rk_udphy_get_initial_status(), if the hardware is already in DP mode,
the clocks are enabled and intentionally left enabled:
rk_udphy_get_initial_status() {
...
if (FIELD_GET(CMN_DP_LANE_MUX_ALL, value) &&
FIELD_GET(CMN_DP_LANE_EN_ALL, value))
udphy->status = UDPHY_MODE_DP;
else
rk_udphy_disable(udphy);
}
However, if subsequent operations in rk_udphy_probe() fail, the probe
function returns directly without calling clk_bulk_disable_unprepare():
rk_udphy_probe() {
...
udphy->phy_u3 = devm_phy_create(dev, dev->of_node,
&rk_udphy_usb3_phy_ops);
if (IS_ERR(udphy->phy_u3)) {
ret = PTR_ERR(udphy->phy_u3);
return dev_err_probe(dev, ret, "failed to create USB3 phy\n");
}
...
}
Additionally, there appears to be no remove callback or devm action
registered to clean up these manually enabled clocks when the driver
unbinds.
[Severity: High]
This is also a pre-existing issue, but could a consumer request more lanes
than are physically configured in udphy->dp_lanes?
In rk_udphy_dp_phy_verify_lanes(), the driver validates that the requested
lane count is 1, 2, or 4, but it does not check if the requested lanes
exceed the physically configured capacity:
rk_udphy_dp_phy_verify_lanes() {
...
switch (dp->lanes) {
case 1:
case 2:
case 4:
udphy->lanes = dp->lanes;
break;
...
}
If a consumer requests 4 lanes on a 1-lane or 2-lane configuration,
rk_udphy_dp_phy_configure() will iterate up to the requested lane count,
reading from unconfigured indices:
rk_udphy_dp_phy_configure() {
...
for (i = 0; i < udphy->lanes; i++) {
lane = udphy->dp_lane_sel[i];
...
rk_udphy_dp_set_voltage(udphy, udphy->bw, dp->voltage[i],
dp->pre[i], lane);
}
}
When writing voltage settings, this will overwrite the transceiver
registers for those unintended lanes. If one of those lanes is actively
assigned to USB, this could silently corrupt the active USB connection.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=7
^ permalink raw reply
* Re: [PATCH v3 3/8] media: Add meta formats supported by NXP neoisp driver
From: Frank Li @ 2026-06-12 16:55 UTC (permalink / raw)
To: Antoine Bouyer
Cc: julien.vuillaumier, alexi.birlinger, daniel.baluta, peng.fan,
frank.li, jacopo.mondi, laurent.pinchart, mchehab, robh, krzk+dt,
conor+dt, michael.riesch, anthony.mcgivern, linux-media,
linux-kernel, devicetree, imx, ai.luthra, paul.elder, geert,
sakari.ailus, hverkuil+cisco
In-Reply-To: <20260612132039.2089051-4-antoine.bouyer@nxp.com>
On Fri, Jun 12, 2026 at 03:20:34PM +0200, Antoine Bouyer wrote:
> This patch adds new v4l2 meta formats definitions and descriptions used by
Avoid use words "This patch"\ "This commit") in commit message.
Add new v4l2 ....
> neoisp driver for the parameters and statistics buffers:
> - `V4L2_META_FMT_NEO_ISP_EXT_PARAMS` used for the generic v4l2-isp
> extensible parameters structure, supporting a non-fixed-size buffer and
> changeable ISP configuration blocks.
> - `V4L2_META_FMT_NEO_ISP_EXT_STATS` used for the generic v4l2-isp
> extensible statistics structure, supporting a non-fixed-size buffer
> and changeable ISP statistics blocks.
>
> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 2 ++
> include/uapi/linux/videodev2.h | 4 ++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index a2b650f4ec3c..acc60dc69d31 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1471,6 +1471,8 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> case V4L2_META_FMT_C3ISP_STATS: descr = "Amlogic C3 ISP Statistics"; break;
> case V4L2_META_FMT_MALI_C55_PARAMS: descr = "ARM Mali-C55 ISP Parameters"; break;
> case V4L2_META_FMT_MALI_C55_STATS: descr = "ARM Mali-C55 ISP 3A Statistics"; break;
> + case V4L2_META_FMT_NEO_ISP_EXT_PARAMS: descr = "NXP Neo ISP ext 3A Parameters"; break;
> + case V4L2_META_FMT_NEO_ISP_EXT_STATS: descr = "NXP Neo ISP ext 3A Statistics"; break;
> case V4L2_PIX_FMT_NV12_8L128: descr = "NV12 (8x128 Linear)"; break;
> case V4L2_PIX_FMT_NV12M_8L128: descr = "NV12M (8x128 Linear)"; break;
> case V4L2_PIX_FMT_NV12_10BE_8L128: descr = "10-bit NV12 (8x128 Linear, BE)"; break;
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index eda4492e40dc..e1656520b312 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -889,6 +889,10 @@ struct v4l2_pix_format {
> #define V4L2_META_FMT_MALI_C55_PARAMS v4l2_fourcc('C', '5', '5', 'P') /* ARM Mali-C55 Parameters */
> #define V4L2_META_FMT_MALI_C55_STATS v4l2_fourcc('C', '5', '5', 'S') /* ARM Mali-C55 3A Statistics */
>
> +/* Vendor specific - used for NXP NEOISP sub-system */
> +#define V4L2_META_FMT_NEO_ISP_EXT_PARAMS v4l2_fourcc('N', 'N', 'E', 'P') /* NXP NEOISP Extensible Parameters */
> +#define V4L2_META_FMT_NEO_ISP_EXT_STATS v4l2_fourcc('N', 'N', 'E', 'S') /* NXP NEOISP Extensible Statistics */
Does checkpatch report extend 100 chas warning?
Frank
> +
> #ifdef __KERNEL__
> /*
> * Line-based metadata formats. Remember to update v4l_fill_fmtdesc() when
> --
> 2.53.0
>
>
^ permalink raw reply
* Re: [PATCH v5 12/18] phy: rockchip: usbdp: Drop DP HPD handling
From: sashiko-bot @ 2026-06-12 16:53 UTC (permalink / raw)
To: Sebastian Reichel
Cc: conor+dt, robh, devicetree, vkoul, neil.armstrong, linux-phy,
olteanv
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-12-efc83069869f@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Lockless access to shared Type-C lane mappings in `rk_udphy_dp_phy_configure` causes data race
--
commit 5858d851fd3d6f0beb8a1b3b702ddc25ce1b25b1
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Drop DP HPD handling
This commit drops the HPD handling logic from the USBDP PHY. The registers
involved require the display controller power domain being enabled, so the HPD
signal should be handled by the displayport controller itself.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> static int rk_udphy_dp_phy_configure(struct phy *phy,
> union phy_configure_opts *opts)
> {
[ ... ]
> if (dp->set_voltages) {
> for (i = 0; i < udphy->lanes; i++) {
> lane = udphy->dp_lane_sel[i];
> switch (udphy->link_rate) {
> case 1620:
> case 2700:
[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential data
race here. Does this loop access udphy->dp_lane_sel and
udphy->lane_mux_sel safely?
These arrays define the active Type-C DP lane mapping and are updated by
asynchronous Type-C subsystem events in rk_udphy_orien_sw_set() and
rk_udphy_typec_mux_set() under the protection of udphy->mutex.
Since rk_udphy_dp_phy_configure() does not hold udphy->mutex (only the
framework-level phy->mutex), could a concurrent Type-C cable orientation
or mode switch cause this code to read a partially updated lane mapping?
This might lead to incorrect DP PHY register programming, configuring the
wrong hardware lanes and causing link training or display failures.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=12
^ permalink raw reply
* Re: [PATCH] arm64: dts: rockchip: fix eMMC reset polarity on PP-1516
From: sashiko-bot @ 2026-06-12 16:52 UTC (permalink / raw)
To: Quentin Schulz; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260612-pp1516-emmc-polarity-v1-1-4816c1c909f7@cherry.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Incomplete fix: identical incorrect eMMC reset polarity (GPIO_ACTIVE_HIGH) remains in multiple PX30 boards.
--
commit 764233a2abb692d41607851c93d3fed45fcab940
Author: Quentin Schulz <quentin.schulz@cherry.de>
arm64: dts: rockchip: fix eMMC reset polarity on PP-1516
This commit fixes the eMMC reset polarity for the px30-pp1516 board by
changing it to GPIO_ACTIVE_LOW to comply with the Jedec 5.1 specification.
The previous GPIO_ACTIVE_HIGH polarity caused eMMC devices with specific
RST_n_FUNCTION bitfield settings to be held in reset forever.
> diff --git a/arch/arm64/boot/dts/rockchip/px30-pp1516.dtsi b/arch/arm64/boot/dts/rockchip/px30-pp1516.dtsi
> index 192791993f059..02200de695d31 100644
> --- a/arch/arm64/boot/dts/rockchip/px30-pp1516.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/px30-pp1516.dtsi
> @@ -33,7 +33,7 @@ emmc_pwrseq: emmc-pwrseq {
> compatible = "mmc-pwrseq-emmc";
> pinctrl-0 = <&emmc_reset>;
> pinctrl-names = "default";
> - reset-gpios = <&gpio1 RK_PB3 GPIO_ACTIVE_HIGH>;
> + reset-gpios = <&gpio1 RK_PB3 GPIO_ACTIVE_LOW>;
[Severity: High]
This isn't a bug introduced by this patch, but does this same fix need to be
applied to other PX30 boards?
Looking at the tree, several other PX30 device trees still use the exact same
incorrect GPIO_ACTIVE_HIGH configuration for their emmc_pwrseq nodes:
arch/arm64/boot/dts/rockchip/px30-cobra.dtsi
arch/arm64/boot/dts/rockchip/px30-evb.dts
arch/arm64/boot/dts/rockchip/px30-firefly-jd4-core.dtsi
arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
Will these boards also fail to boot if used with eMMC devices where the
RST_n_FUNCTION[162] bitfield is set to 0x1, or do they guarantee different
hardware behavior?
> };
>
> gpio-leds {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-pp1516-emmc-polarity-v1-1-4816c1c909f7@cherry.de?part=1
^ permalink raw reply
* Re: [PATCH v3 2/8] media: v4l2-ctrls: Add user control base for NXP neoisp controls
From: Frank Li @ 2026-06-12 16:52 UTC (permalink / raw)
To: Antoine Bouyer
Cc: julien.vuillaumier, alexi.birlinger, daniel.baluta, peng.fan,
frank.li, jacopo.mondi, laurent.pinchart, mchehab, robh, krzk+dt,
conor+dt, michael.riesch, anthony.mcgivern, linux-media,
linux-kernel, devicetree, imx, ai.luthra, paul.elder, geert,
sakari.ailus, hverkuil+cisco
In-Reply-To: <20260612132039.2089051-3-antoine.bouyer@nxp.com>
On Fri, Jun 12, 2026 at 03:20:33PM +0200, Antoine Bouyer wrote:
> [You don't often get email from antoine.bouyer@nxp.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Add a control base for the NXP neoisp driver controls, and reserve up to
> 16 controls.
>
> Signed-off-by: Antoine Bouyer <antoine.bouyer@nxp.com>
> ---
> include/uapi/linux/v4l2-controls.h | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index affec0ab4781..1c44229d349d 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -234,6 +234,12 @@ enum v4l2_colorfx {
> */
> #define V4L2_CID_USER_MALI_C55_BASE (V4L2_CID_USER_BASE + 0x1230)
>
> +/*
> + * The base for NEOISP driver controls.
> + * We reserve 16 controls for this driver.
Generally, don't use "we" \ "you", just
Reserve 16 controls for NXP NeoISP
Frank
> + */
> +#define V4L2_CID_USER_NEOISP_BASE (V4L2_CID_USER_BASE + 0x1240)
> +
> /* MPEG-class control IDs */
> /* The MPEG controls are applicable to all codec controls
> * and the 'MPEG' part of the define is historical */
> --
> 2.53.0
>
>
^ permalink raw reply
* Re: [PATCH v5 13/18] phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit
From: sashiko-bot @ 2026-06-12 16:52 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, vkoul, conor+dt, devicetree, neil.armstrong, olteanv,
linux-phy
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-13-efc83069869f@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] If `rk_udphy_init()` fails during a mode change or orientation switch, the PHY is left in a reset state but `udphy->status` is not updated and `phy_needs_reinit` is permanently lost. Subsequent operations will silently fail to reinitialize the PHY.
--
commit c4618d1b0b4a42d4505fb255a32ef513dc21e759
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Rename mode_change to phy_needs_reinit
This patch renames the mode_change property to phy_needs_reinit. The rename
prepares the driver to also trigger a PHY re-initialization when the cable
orientation changes, not just when the mode changes.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 7255c80e0fe2f..a3b4e2e0f5786 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -968,15 +968,15 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> }
>
> if (udphy->status == UDPHY_MODE_NONE) {
> - udphy->mode_change = false;
> + udphy->phy_needs_reinit = false;
> ret = rk_udphy_setup(udphy);
> if (ret)
> return ret;
>
> if (udphy->mode & UDPHY_MODE_USB)
> rk_udphy_u3_port_disable(udphy, false);
> - } else if (udphy->mode_change) {
> - udphy->mode_change = false;
> + } else if (udphy->phy_needs_reinit) {
> + udphy->phy_needs_reinit = false;
> if (udphy->mode == UDPHY_MODE_DP)
> rk_udphy_u3_port_disable(udphy, true);
>
[Severity: High]
This isn't a bug introduced by this patch, but if rk_udphy_init() fails
immediately after this block, the PHY is left in a reset state while the
udphy->phy_needs_reinit flag is permanently lost.
When changing modes on an already active PHY, rk_udphy_power_on() calls
rk_udphy_init() which immediately asserts all PHY resets. If rk_udphy_init()
subsequently fails (e.g., due to a PLL lock timeout), it leaves the resets
asserted and returns an error.
However, udphy->phy_needs_reinit is cleared right before this, and
udphy->status is not rolled back. As a result, the driver's state tracker
still believes the previously active modes are operational, but the hardware is
completely in reset.
Will future calls to rk_udphy_power_on() see phy_needs_reinit == false and
skip initialization, permanently breaking the active interfaces until a full
system reboot or module reload?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=13
^ permalink raw reply
* Re: [PATCH v3 7/8] media: platform: neoisp: Add debugfs support
From: Frank Li @ 2026-06-12 16:49 UTC (permalink / raw)
To: Antoine Bouyer
Cc: julien.vuillaumier, alexi.birlinger, daniel.baluta, peng.fan,
frank.li, jacopo.mondi, laurent.pinchart, mchehab, robh, krzk+dt,
conor+dt, michael.riesch, anthony.mcgivern, linux-media,
linux-kernel, devicetree, imx, ai.luthra, paul.elder, geert,
sakari.ailus, hverkuil+cisco
In-Reply-To: <20260612132039.2089051-8-antoine.bouyer@nxp.com>
On Fri, Jun 12, 2026 at 03:20:38PM +0200, Antoine Bouyer wrote:
> Add debugfs entries to dump ISP registers, and some internal memory
> regions used to store Vignetting, DRC global and DRC local coefficients.
>
> Debug mode is activated with the `enable_debugfs` module's parameter, to
> avoid runtime suspend which blocks register access when IP is not active,
> so we can capture an ISP snapshot after a frame is decoded.
Look like you needn't this option, see below implementaiton. If you correct
set regset->dev, it will call pm_runtime_get_sync() for you
static int debugfs_regset32_show(struct seq_file *s, void *data)
{
struct debugfs_regset32 *regset = s->private;
if (regset->dev)
pm_runtime_get_sync(regset->dev);
debugfs_print_regs32(s, regset->regs, regset->nregs, regset->base, "");
if (regset->dev)
pm_runtime_put(regset->dev);
return 0;
}
Frank
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: add entry for qstcorp
From: Conor Dooley @ 2026-06-12 16:49 UTC (permalink / raw)
To: Siratul Islam
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20260612124557.13750-2-email@sirat.me>
[-- Attachment #1: Type: text/plain, Size: 1046 bytes --]
On Fri, Jun 12, 2026 at 06:45:25PM +0600, Siratul Islam wrote:
> Add an entry for QST Corporation Limited
>
> Signed-off-by: Siratul Islam <email@sirat.me>
Link: https://www.qstcorp.com/
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
Cheers,
Conor.
\x02
> ---
> 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 28784d66ae7b..11aac47f90ce 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1355,6 +1355,8 @@ patternProperties:
> description: Shenzhen QiShenglong Industrialist Co., Ltd.
> "^qnap,.*":
> description: QNAP Systems, Inc.
> + "^qstcorp,.*":
> + description: QST Corporation Limited
> "^quanta,.*":
> description: Quanta Computer Inc.
> "^radxa,.*":
> --
> 2.54.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH] arm64: dts: rockchip: fix eMMC reset polarity on PP-1516
From: Quentin Schulz @ 2026-06-12 16:47 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: Heiko Stuebner, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Quentin Schulz, stable
From: Quentin Schulz <quentin.schulz@cherry.de>
According to the Jedec 5.1 specification, the device is held in reset
when RST_n is low, therefore the polarity of the line must be that, as
specified in the Device Tree binding (mmc/mmc-pwrseq-emmc.yaml).
Due to the wrong polarity, eMMC devices with RST_n_FUNCTION[162]
bitfield [1:0] set to 0x1 (the default is 0x0) will be held in reset
forever.
Cc: stable@vger.kernel.org
Fixes: 56198acdbf0d ("arm64: dts: rockchip: add px30-pp1516 base dtsi and board variants")
Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
PP-1516 is affected by the same issue that Cobra has and for which a
patch[1] has already been sent.
[1] https://lore.kernel.org/linux-rockchip/20260609081728.30616-2-jakobunt@gmail.com/
---
arch/arm64/boot/dts/rockchip/px30-pp1516.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/rockchip/px30-pp1516.dtsi b/arch/arm64/boot/dts/rockchip/px30-pp1516.dtsi
index 192791993f059..02200de695d31 100644
--- a/arch/arm64/boot/dts/rockchip/px30-pp1516.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30-pp1516.dtsi
@@ -33,7 +33,7 @@ emmc_pwrseq: emmc-pwrseq {
compatible = "mmc-pwrseq-emmc";
pinctrl-0 = <&emmc_reset>;
pinctrl-names = "default";
- reset-gpios = <&gpio1 RK_PB3 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&gpio1 RK_PB3 GPIO_ACTIVE_LOW>;
};
gpio-leds {
---
base-commit: 2b414a95b8f7307d42173ba9e580d6d3e2bcbfce
change-id: 20260612-pp1516-emmc-polarity-5a7fb54db917
Best regards,
--
Quentin Schulz <quentin.schulz@cherry.de>
^ permalink raw reply related
* Re: [PATCH v1 1/2] dt-bindings: spi: snps,dw-apb-ssi: Add support for snps,dwc-ssi-2.00a
From: Conor Dooley @ 2026-06-12 16:46 UTC (permalink / raw)
To: Changhuang Liang
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown,
linux-spi, linux-kernel, devicetree
In-Reply-To: <20260612125856.8530-2-changhuang.liang@starfivetech.com>
[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]
On Fri, Jun 12, 2026 at 05:58:55AM -0700, Changhuang Liang wrote:
> Add a new compatible string "snps,dwc-ssi-2.00a" for the Synopsys
> DesignWare SSI controller version 2.00a.
Two things. Firstly, driver patch suggests a fallback to 1.01a is
possible. Why haven't you added one?
Secondly, I am going to expect that when your starfive user for this
appears in my inbox that it has a device-specific compatible, so you may
as well add that now.
pw-bot: changes-requested
Cheers,
Conor.
>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
> Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index 8ebebcebca16..fb74243d4bdf 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -41,6 +41,7 @@ properties:
> enum:
> - snps,dw-apb-ssi
> - snps,dwc-ssi-1.01a
> + - snps,dwc-ssi-2.00a
> - description: Microchip Sparx5 SoC SPI Controller
> const: microchip,sparx5-spi
> - description: Amazon Alpine SPI Controller
> --
> 2.25.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v5 06/18] phy: rockchip: usbdp: Add missing mode_change update
From: sashiko-bot @ 2026-06-12 16:46 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, vkoul, neil.armstrong, linux-phy, olteanv, devicetree,
conor+dt
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-6-efc83069869f@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] State loss on transient PHY initialization failure leaves hardware permanently in reset.
- [High] Data race in `rk_udphy_dp_phy_configure()` reading shared lane mappings without holding the driver mutex.
--
commit 22525e2688f8ca90810542bfa2ad9f261c949f0b
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Add missing mode_change update
rk_udphy_set_typec_default_mapping() updates the available modes,
but does not set the mode_change as required. This results in
missing re-initialization and thus non-working DisplayPort.
Fix this issue by introducing a new helper to update the available
modes.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 97e53b933225f..febc148a754e0 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -619,6 +619,15 @@ static void rk_udphy_dp_hpd_event_trigger(struct rk_udphy *udphy, bool hpd)
> rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, hpd);
> }
>
> +static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
> +{
> + if (udphy->mode == mode)
> + return;
> +
> + udphy->mode_change = true;
[Severity: High]
This isn't a bug introduced by this patch, but it seems there is a
pre-existing issue with how this flag is handled in rk_udphy_power_on().
When the PHY is partially active and a new mode is being powered on, it
checks the re-initialization flag (also known as phy_needs_reinit in the
broader context). It unconditionally clears the flag to false before calling
rk_udphy_init():
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_on() {
...
} else if (udphy->phy_needs_reinit) {
udphy->phy_needs_reinit = false;
...
ret = rk_udphy_init(udphy);
if (ret)
return ret;
...
}
If rk_udphy_init() fails and returns an error (like -EPROBE_DEFER from a PLL
lock timeout), the flag is left as false. Would the next retry by the
consumer skip initialization and falsely return success, leaving the PHY
hardware permanently held in reset?
> + udphy->mode = mode;
> +}
> +
> static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
> {
> if (udphy->flip) {
[Severity: High]
This is a pre-existing issue, but I have a question about concurrent access
to the lane mappings.
This function mutates the lane selection arrays concurrently with
rk_udphy_dp_phy_configure() reading them:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
...
for (i = 0; i < udphy->lanes; i++) {
lane = udphy->dp_lane_sel[i];
...
regmap_update_bits(..., LN_ANA_TX_SER_TXCLK_INV,
FIELD_PREP(LN_ANA_TX_SER_TXCLK_INV, udphy->lane_mux_sel[lane]));
...
rk_udphy_dp_set_voltage(udphy, udphy->bw, dp->voltage[i], dp->pre[i], lane);
}
...
}
Since rk_udphy_dp_phy_configure() reads these arrays without holding
udphy->mutex, could a race condition result in reading partially updated
lane assignments if a Type-C switch event occurs at the same time?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=6
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: vendor-prefixes: Add Ugreen Group Limited
From: Conor Dooley @ 2026-06-12 16:43 UTC (permalink / raw)
To: Alexey Charkov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lee Jones,
Heiko Stuebner, Liam Girdwood, Mark Brown, devicetree,
linux-kernel, linux-arm-kernel, linux-rockchip
In-Reply-To: <20260612-dh2300-mcu-v1-1-ab8db1617bc0@flipper.net>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v5 03/18] phy: rockchip: usbdp: Keep clocks running on PHY re-init
From: sashiko-bot @ 2026-06-12 16:41 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, conor+dt, olteanv, devicetree, linux-phy, vkoul,
neil.armstrong
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-3-efc83069869f@collabora.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing clock disable on probe failure and driver unbind if the bootloader left the PHY enabled.
- [High] Premature consumption of the re-initialization flag leads to false success reporting on subsequent retries if PHY initialization fails.
- [High] Initialization failure of a new mode silently breaks the already active mode in the combo PHY.
- [High] Shadowing of error codes from devm_clk_bulk_get_all() breaks deferred probing.
- [High] Missing mutex_lock(&udphy->mutex) in rk_udphy_dp_phy_configure() allows data races and hardware state corruption.
--
commit 352757db3201eb1ea989ca1ac6bc600f268f254c
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Keep clocks running on PHY re-init
This commit avoids unnecessarily disabling and re-enabling clocks when
re-initializing the PHY during a mode change. Since rk_udphy_init()
asserts the reset lines anyway, keeping the clocks running simplifies
the flow and avoids delaying the process.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 744cc7c642f49..98562a888b42a 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1012,8 +1012,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> if (udphy->mode == UDPHY_MODE_DP)
> rk_udphy_u3_port_disable(udphy, true);
>
> - rk_udphy_disable(udphy);
> - ret = rk_udphy_setup(udphy);
> + ret = rk_udphy_init(udphy);
[Severity: High]
This is a pre-existing issue, but by consuming udphy->phy_needs_reinit earlier
in rk_udphy_power_on(), could this falsely report success on subsequent
retries if rk_udphy_init() fails?
When transitioning to a combo mode, udphy->phy_needs_reinit is cleared before
rk_udphy_init() is called:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_on() {
...
} else if (udphy->phy_needs_reinit) {
udphy->phy_needs_reinit = false;
...
ret = rk_udphy_init(udphy);
if (ret)
return ret;
...
}
If rk_udphy_init() returns an error, the flag remains false. On the next retry
by the framework, it might skip hardware initialization, incorrectly add the
new mode to udphy->status, and return success despite the PHY not functioning.
> if (ret)
> return ret;
> }
[Severity: High]
This is a pre-existing issue, but is there a leak of the clock enable count
during probe error paths or driver unbind if the bootloader left the PHY
enabled?
In rk_udphy_get_initial_status(), clocks are prepared and enabled:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_get_initial_status() {
...
ret = clk_bulk_prepare_enable(udphy->num_clks, udphy->clks);
...
}
If the PHY was left enabled by the bootloader, udphy->status is set to
UDPHY_MODE_DP and clocks are intentionally kept running. However, the driver
does not appear to register a devm cleanup action for these clocks. If
subsequent steps in rk_udphy_probe() fail, or if the driver is unbound, the
clock enable count is never decremented.
[Severity: High]
This is a pre-existing issue, but would an initialization failure of a new
mode in rk_udphy_init() silently break the already active mode?
If a mode change occurs (e.g., turning on DisplayPort while USB is already
active) and rk_udphy_init() fails, the error path asserts all reset lines:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_init() {
...
assert_resets:
rk_udphy_reset_assert_all(udphy);
return ret;
}
This forcefully asserts reset lines for the entire combo PHY, breaking the
already active USB PHY without notifying the USB subsystem.
[Severity: High]
This is a pre-existing issue, but by replacing the error code in
rk_udphy_clk_init(), could deferred probing be broken?
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_clk_init() {
...
udphy->num_clks = devm_clk_bulk_get_all(dev, &udphy->clks);
if (udphy->num_clks < 1)
return -ENODEV;
...
}
If devm_clk_bulk_get_all() returns -EPROBE_DEFER, it is replaced with -ENODEV,
permanently failing the driver probe instead of allowing it to defer.
[Severity: High]
This is a pre-existing issue, but without acquiring udphy->mutex in
rk_udphy_dp_phy_configure(), are there potential data races or hardware state
corruption?
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
struct rk_udphy *udphy = phy_get_drvdata(phy);
struct phy_configure_opts_dp *dp = &opts->dp;
...
}
The function modifies shared driver state (udphy->dp_lane_sel,
udphy->lane_mux_sel, udphy->bw, udphy->link_rate) and writes to shared PHY
hardware registers. Without holding the mutex, this can race with Type-C
callbacks or concurrent USB PHY operations.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=3
^ permalink raw reply
* Re: [PATCH v2 07/12] drm/rockchip: dw_dp: Implement out-of-band HPD handling
From: Sebastian Reichel @ 2026-06-12 16:37 UTC (permalink / raw)
To: Chaoyi Chen
Cc: Sandy Huang, Heiko Stübner, Andy Yan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, Andrzej Hajda, Neil Armstrong,
Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Airlie,
Simona Vetter, Dmitry Baryshkov, Luca Ceresoli,
Cristian Ciocaltea, Damon Ding, Dmitry Baryshkov, Alexey Charkov,
dri-devel, linux-rockchip, linux-kernel, devicetree, kernel,
linux-arm-kernel
In-Reply-To: <4f4ff085-80ae-4144-b040-be9b136574ee@rock-chips.com>
[-- Attachment #1: Type: text/plain, Size: 1919 bytes --]
Hello Chaoyi,
On Tue, Jun 02, 2026 at 04:30:59PM +0800, Chaoyi Chen wrote:
> On 5/1/2026 6:20 AM, Sebastian Reichel wrote:
> > Implement out-of-band hotplug handling, which will be used to receive
> > external hotplug information from the USB-C state machine. This is
> > currently handled by the USBDP PHY, which brings quite some trouble
> > as the register being accessed requires the power-domain from the DP
> > controller and also requires custom TypeC HPD info parsing in the
> > USBDP PHY driver.
> >
> > In contrast to the USBDP PHY this does not just enable the hotplug
> > signal when a DP AltMode capable adapter is plugged in, but instead
> > properly detects if a cable is plugged in for things like USB-C to
> > HDMI adapters.
> >
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
>
> [...]
>
> > +static void dw_dp_rockchip_hpd_sw_cfg(void *data, bool hpd)
> > +{
> > + struct rockchip_dw_dp *dp = data;
> > + u32 hpd_reg = dp->pdata->hpd_reg[dp->id];
> > +
> > + dev_dbg(dp->dev, "Force HPD connected=%s\n", str_yes_no(hpd));
> > +
> > + dp->hpd_cfg = hpd;
> > +
> > + regmap_write(dp->vo_grf, hpd_reg,
> > + FIELD_PREP_WM16_CONST(ROCKCHIP_VO_GRF_DP_SINK_HPD_CFG, dp->hpd_cfg));
>
> FIELD_PREP_WM16() should be used here because "dp->hpd_cfg" is not
> a constant expression.
>
> Other patches in this series have similar issues. Doesn't your
> compiler warn you about this? Thanks.
The code builds warning free for me. Also FIELD_PREP_WM16_CONST
requies that the mask is constant, which is the first argument
(i.e. ROCKCHIP_VO_GRF_DP_SINK_HPD_CFG in this case). The second
argument should be fine as a variable as far as I can tell.
But since FIELD_PREP_WM16 is shorter anyways, gives better error
handling and can be used for all occuances in the dw-dp rockchip
glue I will use that instead.
Thanks,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v5 02/18] phy: rockchip: usbdp: Do not lose USB3 PHY status
From: sashiko-bot @ 2026-06-12 16:38 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, robh, conor+dt, devicetree, vkoul, neil.armstrong,
olteanv
In-Reply-To: <20260612-rockchip-usbdp-cleanup-v5-2-efc83069869f@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] State desynchronization on `rk_udphy_init` failure causes subsequent power-on retries to falsely return success without re-initializing the hardware, leading to dead PHY or bus hangs.
Pre-existing issues:
- [High] Subsystem power-off requests permanently leak `udphy->status` bits if the Type-C mode has physically changed, causing a perpetual resource leak of the PHY clocks.
--
commit 93e296ece1bf03449fcdb7d0485cb77da80dd9b7
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Do not lose USB3 PHY status
This commit fixes an issue where the USB3 tracking information is lost
when the DisplayPort is re-initialized. It does this by preventing the
status variable from being cleared during mode changes in the PHY driver.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index fba35510d88ce..744cc7c642f49 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1009,7 +1009,6 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> rk_udphy_u3_port_disable(udphy, false);
> } else if (udphy->mode_change) {
> udphy->mode_change = false;
> - udphy->status = UDPHY_MODE_NONE;
[Severity: High]
If rk_udphy_init() fails (for example, returning -EPROBE_DEFER after a lock
timeout), will this lead to a permanent state desynchronization?
By removing the clearing of udphy->status, if the initialization fails,
udphy->mode_change is already set to false but udphy->status remains
non-zero.
If the subsystem later retries rk_udphy_power_on(), it will see
udphy->status != UDPHY_MODE_NONE and udphy->mode_change == false. Could
this cause it to bypass all initialization, blindly update the status, and
falsely return success while the PHY hardware remains disabled and in reset?
[Severity: High]
This is a pre-existing issue, but removing the forced clearing of
udphy->status here exposes a potential permanent resource leak of the PHY
clocks.
Looking at rk_udphy_power_off():
if (!(udphy->mode & mode)) {
dev_info(udphy->dev, "mode 0x%02x is not support\n", mode);
return;
}
and rk_udphy_usb3_phy_exit():
if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
return 0;
If the Type-C multiplexer switches from DP_USB to a DP_ONLY mode,
udphy->mode loses the USB bit. If the USB controller then unbinds and calls
phy_exit() while in DP_ONLY mode, the early returns above prevent clearing
the UDPHY_MODE_USB bit from udphy->status.
When DP eventually powers off, will udphy->status remain non-zero due to
this orphaned USB bit, preventing rk_udphy_disable() from ever executing
and leaking the PHY clocks and power?
> if (udphy->mode == UDPHY_MODE_DP)
> rk_udphy_u3_port_disable(udphy, true);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-rockchip-usbdp-cleanup-v5-0-efc83069869f@collabora.com?part=2
^ permalink raw reply
* Re: [PATCH 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor
From: Jonathan Cameron @ 2026-06-12 16:35 UTC (permalink / raw)
To: Joshua Crofts
Cc: Siratul Islam, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20260612154922.00003723@gmail.com>
>
> > + {
> > + /* 50ms headroom over the slowest ODR (10Hz) */
> > + ret = regmap_read_poll_timeout(data->regmap,
> > + QMC5883L_REG_STATUS1, status,
> > + (status & QMC5883L_STATUS_DRDY),
> > + 2 * USEC_PER_MSEC,
> > + 150 * USEC_PER_MSEC);
> > + if (ret)
> > + return ret;
> > +
> > + if (status & QMC5883L_STATUS_OVL)
> > + return -ERANGE;
>
> Sashiko has a remark:
>
> If we return -ERANGE here when the overflow flag (OVL) is set, does the
> sensor get permanently stuck in an overflow state?
> In typical I2C magnetometers, the Data Ready (DRDY) and Overflow (OVL)
> status bits are only cleared by reading the data registers. By returning
> early without reading the data registers via regmap_bulk_read(), the DRDY
> and OVL flags might remain set indefinitely.
> On subsequent measurement attempts, regmap_read_poll_timeout() will return
> immediately and this check will instantly fail again, potentially locking up
> the sensor until a reset.
>
I was curious so checked the datasheet.
https://www.qstcorp.com/upload/pdf/202512/13-52-04%20QMC5883L%20Datasheet%20Rev.%20B.pdf
Sashiko looses this time as overflow resets if the next value is in range.
I'm with Sashiko for explicit access being needed for data ready (or a write 1 to clear)
but overflow is more of an intermittent thing so both styles exist for devices
(with and without need for specific action to clear).
Jonathan
> > +
> > + ret = regmap_bulk_read(data->regmap, QMC5883L_REG_X_LSB, buf,
> > + sizeof(buf));
> > + if (ret)
> > + return ret;
> > +
> > + *val = (s16)le16_to_cpu(buf[index]);
> > + }
> > +
> > + return 0;
> > +}
> > +
^ permalink raw reply
* [PATCH v1 1/2] dt-bindings: spi: snps,dw-apb-ssi: Add support for snps,dwc-ssi-2.00a
From: Changhuang Liang @ 2026-06-12 12:58 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Mark Brown
Cc: linux-spi, linux-kernel, devicetree, Changhuang Liang
In-Reply-To: <20260612125856.8530-1-changhuang.liang@starfivetech.com>
Add a new compatible string "snps,dwc-ssi-2.00a" for the Synopsys
DesignWare SSI controller version 2.00a.
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index 8ebebcebca16..fb74243d4bdf 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -41,6 +41,7 @@ properties:
enum:
- snps,dw-apb-ssi
- snps,dwc-ssi-1.01a
+ - snps,dwc-ssi-2.00a
- description: Microchip Sparx5 SoC SPI Controller
const: microchip,sparx5-spi
- description: Amazon Alpine SPI Controller
--
2.25.1
^ permalink raw reply related
* Re: [PATCH 4/4] regulator: Add support for UGREEN NASync DH2300 MCU SATA power gate
From: Mark Brown @ 2026-06-12 16:30 UTC (permalink / raw)
To: Alexey Charkov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lee Jones,
Heiko Stuebner, Liam Girdwood, devicetree, linux-kernel,
linux-arm-kernel, linux-rockchip
In-Reply-To: <20260612-dh2300-mcu-v1-4-ab8db1617bc0@flipper.net>
[-- Attachment #1: Type: text/plain, Size: 717 bytes --]
On Fri, Jun 12, 2026 at 07:34:17PM +0400, Alexey Charkov wrote:
> +static int ugreen_dh2300_mcu_regulator_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct regulator_config config = { };
> + struct regulator_dev *rdev;
> + struct device_node *np;
> +
> + np = of_get_child_by_name(dev->parent->of_node, "regulator");
> + if (!np)
> + return dev_err_probe(dev, -ENODEV,
> + "missing regulator child node\n");
You should just be able to configured this in the regulator_desc rather
than describe it.
> + config.init_data = of_get_regulator_init_data(dev, np,
> + &ugreen_dh2300_sata_desc);
> +
Similarly here, there should be no need for this open coding.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ 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