* Re: [PATCH v4 4/4] iio: flow: add Sensirion SLF3S liquid flow sensor driver
From: Jonathan Cameron @ 2026-06-14 15:10 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Wadim Mueller, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
David Lechner, Nuno Sá, Andy Shevchenko, Maxwell Doose,
linux-iio, devicetree, linux-kernel, Marcelo Schmitt,
Rodrigo Alencar
In-Reply-To: <4e4ef1ae-62e7-4639-914c-19f49930be02@kernel.org>
On Sat, 13 Jun 2026 09:51:13 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 12/06/2026 19:47, Jonathan Cameron wrote:
> > On Thu, 11 Jun 2026 16:01:12 +0200
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> >> On 11/06/2026 15:27, Wadim Mueller wrote:
> >>> +
> >>> +static const struct of_device_id slf3s_of_match[] = {
> >>> + { .compatible = "sensirion,slf3s-0600f", .data = &slf3s_variants[0] },
> >>> + { .compatible = "sensirion,slf3s-1300f", .data = &slf3s_variants[1] },
> >>> + { .compatible = "sensirion,slf3s-4000b", .data = &slf3s_variants[2] },
> >>
> >> You should have only 1300f here and detect the variants. That was my
> >> point when I suggested to use the fallback.
> >>
> >
> > I'm lost. How does that work? They cannot fallback to that part because
> > it relies on in driver detection of the fact that they are incompatible.
> >
>
> I am lost too. Then why were they made compatible in the binding?
>
> Entire discussion was that these are FULLY compatible due to variant
> detection. That was the entire point of discussing more generic
> fallback. Using specific fallback does not change that - it is the same
> concept.
>
> If devices are not detectable, why were we discussing any compatibility?
They are detectable, but the feature set is not, so to me there is zero valid
in a generic fallback, we have to update the driver every time a new part comes
along. (i.e. I agree with Conor's reply to the previous version thread).
A specific fallback to a completely compatible part would be fine as there
would be sufficient info to not need the ID lookup.
The case in the binding for this version is the worst of all options because
it implies it is valid to fallback to something that gives a false impression
of being specific when it's relying on ID matching to say actually it's something
else.
So definitely not
+ compatible:
+ oneOf:
+ - const: sensirion,slf3s-1300f
+ - items:
+ - enum:
+ - sensirion,slf3s-0600f
+ - sensirion,slf3s-4000b
+ - const: sensirion,slf3s-1300f
+
Falling back to slf3s is better than this, but I'd rather not have a fallback
at all, thus allowing correct fallback to the parts listed here in future.
Jonathan
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: hamoa: Reserve low IOVA range for Iris
From: sashiko-bot @ 2026-06-14 15:03 UTC (permalink / raw)
To: Daniel J Blueman; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260614145113.84243-2-daniel@quora.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing NULL check on the return value of `of_translate_dma_region()` causes use of uninitialized variables and a guaranteed NULL pointer dereference.
- [High] The parser fails to advance the `maps` pointer when skipping entries for other devices, leading to misaligned parsing and potential out-of-bounds memory reads.
--
commit 9504aa19b1b75ded98afc02b95aff54f7fd1ba37
Author: Daniel J Blueman <daniel@quora.org>
arm64: dts: qcom: hamoa: Reserve low IOVA range for Iris
On X1-family hamoa platforms, the Iris VPU reserves IOVA addresses
below 0x25800000 (600MB), primarily for non-pixel buffers accessed
via different Stream IDs. DMA into that range triggers unhandled SMMU
page faults that cause spontaneous device reboots. This is readily
reproduced with one or more browser tabs driving multiple concurrent
video decode streams.
Link: https://github.com/qualcomm-linux/kernel-topics/issues/1157#issuecomment-4458933574
> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index 051dee0764167..ce96e7f8d8c10 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -716,6 +716,17 @@ smem_mem: smem@ffe00000 {
> hwlocks = <&tcsr_mutex 3>;
> no-map;
> };
> +
> + /*
> + * The Iris VPU reserves IOVA below 0x25800000 (600MB),
> + * primarily for non-pixel buffers using different Stream IDs.
> + * DMA into that range triggers unhandled SMMU faults and
> + * spontaneous reboots, so reserve it to keep IOMMU
> + * allocations above this boundary.
> + */
> + iris_iova: iris-iova {
> + iommu-addresses = <&iris 0x0 0x0 0x0 0x25800000>;
[Severity: High]
This isn't a bug introduced by this patch, but adding this iommu-addresses
property exercises a pre-existing issue in
drivers/iommu/of_iommu.c:of_iommu_get_resv_regions().
If of_translate_dma_region() fails (e.g. returning OF_BAD_ADDR), it returns
NULL without populating the local variables iova and length. The maps
pointer is unconditionally overwritten with this NULL value:
drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
...
maps = of_translate_dma_region(np, maps, &iova, &length);
...
}
The code then evaluates if (length == 0) using uninitialized stack garbage.
If it proceeds to the next iteration, the while (maps < end) condition
evaluates as true because NULL (0) is less than the valid kernel pointer
end. The next iteration then blindly dereferences the NULL pointer with
be32_to_cpup(maps++), causing a kernel panic.
Does the IOMMU core need to be fixed to handle a NULL return from
of_translate_dma_region() before adding new users of iommu-addresses?
[Severity: High]
This is a pre-existing issue, but there is a second bug in
drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() related to
iommu-addresses parsing that makes shared memory regions unsafe.
If an entry's phandle belongs to a different device, the loop skips the
translation block:
drivers/iommu/of_iommu.c:of_iommu_get_resv_regions() {
...
if (np == dev->of_node) {
// ...
}
// missing else branch to advance maps pointer
...
}
Because it lacks an else branch to advance the maps pointer by the required
#address-cells + #size-cells, the next iteration erroneously reads the first
cell of the skipped address as a new phandle.
This desynchronizes the entire parser, potentially causing it to interpret
an address or size value as a matching device phandle. It would then call
of_translate_dma_region() on misaligned data, risking an out-of-bounds
memory read since there is no bounds checking against the end of the
property buffer.
Should the parsing logic be corrected to properly advance the maps pointer
for non-matching devices?
> + };
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614145113.84243-1-daniel@quora.org?part=2
^ permalink raw reply
* Re: [PATCH v3 3/3] iio: adc: Fix incorrect reading when datarate changed in single mode
From: Jonathan Cameron @ 2026-06-14 14:54 UTC (permalink / raw)
To: Jakub Szczudlo
Cc: linux-iio, andy, antoniu.miclaus, conor+dt, devicetree, dlechner,
duje, jishnu.prakash, jorge.marques, krzk+dt, linusw,
linux-kernel, marcelo.schmitt, mazziesaccount, mike.looijmans,
nuno.sa, robh, sakari.ailus, wens, joshua.crofts1
In-Reply-To: <20260613190957.654798-4-jakubszczudlo40@gmail.com>
On Sat, 13 Jun 2026 21:09:57 +0200
Jakub Szczudlo <jakubszczudlo40@gmail.com> wrote:
> When device is suspended and it is in single mode then changing
> datarate doesn't make it actual wait for new measurement, so to
> be sure that read after change is correct functions that changes
> datarate and gain will wait for new data.
Fix should normally be the first patch in the series to make it
easier (or at least more obvious) to backport.
Also needs a fixes tag.
A few things inline,
thanks,
Jonathan
>
> Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
> ---
> drivers/iio/adc/ti-ads1100.c | 55 ++++++++++++++++++++++++++++++++++--
> 1 file changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index 76de2466dc53..195394665cd1 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
> @@ -123,6 +123,36 @@ static int ads1100_get_voltage_microvolts(struct ads1100_data *data)
> return ads1100_get_voltage_milivolts(data) * MICRO / MILLI;
> }
>
> +static bool ads1100_new_data_ready(struct ads1100_data *data)
> +{
> + int ret;
> + u8 buffer[3];
> +
> + ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
> + if (ret < 3) {
> + dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> + return ret;
This is odd given it returns bool (and don't print ret = 1 or 2 here as that
is rather unexpected in an error print.).
> + }
> +
> + return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
> +}
> +
> +static int ads1100_poll_data_ready(struct ads1100_data *data)
> +{
> + u8 buffer[3];
> + bool data_ready;
> + int datarate = data->ads_config->data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> + // To be sure we wait 5 times more than datarate
Left over from writing the code? If you want a comment then /* */ and
get the indent right.
> + unsigned long wait_time = DIV_ROUND_CLOSEST(MICRO, 5 * datarate);
> +
> + /* To be sure that polled value will have value after config change */
> + i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
Check return value probably even though we don't care about the data.
> +
> + return read_poll_timeout(ads1100_new_data_ready, data_ready,
> + !data_ready, wait_time,
> + ADS1100_MAX_DRDY_TIMEOUT, false, data);
> +}
> +
> static int ads1100_data_bits(struct ads1100_data *data)
> {
> return ads1100_data_rate_bits[FIELD_GET(ADS1100_DR_MASK, data->config)];
> @@ -165,6 +194,7 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
> {
> int microvolts;
> int gain;
> + int ret;
>
> /* With Vdd between 2.7 and 5V, the scale is always below 1 */
> if (val)
> @@ -185,21 +215,40 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
> if (gain < BIT(0) || gain > BIT(3))
> return -EINVAL;
>
> + ret = pm_runtime_resume_and_get(&data->client->dev);
> + if (ret < 0)
> + return ret;
Might be worth looking at the ACQUIRE macros in pm_runtime.h as they might simpify things
a little.
> +
> ads1100_set_config_bits(data, ADS1100_PGA_MASK, ffs(gain) - 1);
>
> - return 0;
> + ret = ads1100_poll_data_ready(data);
> +
> + pm_runtime_put_autosuspend(&data->client->dev);
> +
> + return ret;
> }
>
> static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
> {
> unsigned int i;
> unsigned int size;
> + int ret;
>
> size = data->supports_data_rate ? ARRAY_SIZE(ads1100_data_rate) : 1;
> for (i = 0; i < size; i++) {
> - if (data->ads_config->data_rate[i] == rate)
> - return ads1100_set_config_bits(data, ADS1100_DR_MASK,
> + if (data->ads_config->data_rate[i] != rate)
> + continue;
> +
> + ret = pm_runtime_resume_and_get(&data->client->dev);
> + if (ret < 0)
> + return ret;
> +
> + ads1100_set_config_bits(data, ADS1100_DR_MASK,
> FIELD_PREP(ADS1100_DR_MASK, i));
> + ret = ads1100_poll_data_ready(data);
> +
> + pm_runtime_put_autosuspend(&data->client->dev);
> + return ret;
> }
>
> return -EINVAL;
^ permalink raw reply
* [PATCH v2 2/2] arm64: dts: qcom: hamoa: Reserve low IOVA range for Iris
From: Daniel J Blueman @ 2026-06-14 14:51 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Abhinav Kumar,
Bjorn Andersson, Konrad Dybcio
Cc: Daniel J Blueman, Mauro Carvalho Chehab, Stephan Gerhold,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
linux-media, devicetree, linux-kernel, stable
In-Reply-To: <20260614145113.84243-1-daniel@quora.org>
On X1-family hamoa platforms, the Iris VPU reserves IOVA addresses
below 0x25800000 (600MB), primarily for non-pixel buffers accessed
via different Stream IDs. DMA into that range triggers unhandled SMMU
page faults that cause spontaneous device reboots. This is readily
reproduced with one or more browser tabs driving multiple concurrent
video decode streams.
Add a reserved-memory IOVA reservation node covering [0, 0x25800000)
and reference it from the Iris node so the IOMMU layer keeps DMA
allocations above that boundary.
This applies to all current hamoa.dtsi consumers (X1E80100/X1P42100/
X1P64100 boards); other Iris-bearing SoCs (sm8550/sm8650/sa8775p/
qcs8300) do not include hamoa.dtsi thus not affected.
Backports also require the preceding binding patch ("dt-bindings:
media: qcom,sm8550-iris: Allow IOVA reservation memory-region");
without it, dtbs_check rejects the second memory-region entry.
Link: https://github.com/qualcomm-linux/kernel-topics/issues/1157#issuecomment-4458933574
Fixes: 9065340ac04d ("arm64: dts: qcom: x1e80100: Add IRIS video codec")
Cc: stable@vger.kernel.org
Signed-off-by: Daniel J Blueman <daniel@quora.org>
---
v2:
- add Fixes tag
- clarify the reservation rationale
v1: https://lore.kernel.org/lkml/20260601041336.9497-2-daniel@quora.org/
arch/arm64/boot/dts/qcom/hamoa.dtsi | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
index 051dee076416..ce96e7f8d8c1 100644
--- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
@@ -716,6 +716,17 @@ smem_mem: smem@ffe00000 {
hwlocks = <&tcsr_mutex 3>;
no-map;
};
+
+ /*
+ * The Iris VPU reserves IOVA below 0x25800000 (600MB),
+ * primarily for non-pixel buffers using different Stream IDs.
+ * DMA into that range triggers unhandled SMMU faults and
+ * spontaneous reboots, so reserve it to keep IOMMU
+ * allocations above this boundary.
+ */
+ iris_iova: iris-iova {
+ iommu-addresses = <&iris 0x0 0x0 0x0 0x25800000>;
+ };
};
qup_opp_table_100mhz: opp-table-qup100mhz {
@@ -5479,7 +5490,7 @@ &config_noc SLAVE_VENUS_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
interconnect-names = "cpu-cfg",
"video-mem";
- memory-region = <&video_mem>;
+ memory-region = <&video_mem>, <&iris_iova>;
resets = <&gcc GCC_VIDEO_AXI0_CLK_ARES>;
reset-names = "bus";
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/2] dt-bindings: media: qcom,sm8550-iris: Allow IOVA reservation memory-region
From: Daniel J Blueman @ 2026-06-14 14:51 UTC (permalink / raw)
To: Bryan O'Donoghue, Vikash Garodia, Abhinav Kumar,
Bjorn Andersson, Konrad Dybcio
Cc: Daniel J Blueman, Mauro Carvalho Chehab, Stephan Gerhold,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
linux-media, devicetree, linux-kernel, stable
In addition to the firmware-loaded codec carveout, some Iris platforms
need to declare an IOMMU IOVA reservation (a reserved-memory node with
iommu-addresses) to keep DMA away from IOVA ranges that earlier
firmware stages have already mapped through the SMMU.
Permit a second memory-region phandle for this purpose, and describe
the meaning of each entry so the ordering is unambiguous.
Fixes: 9065340ac04d ("arm64: dts: qcom: x1e80100: Add IRIS video codec")
Cc: stable@vger.kernel.org
Signed-off-by: Daniel J Blueman <daniel@quora.org>
---
v2:
- drop redundant maxItems, keeping the items descriptions (Rob)
- add Fixes tag and Cc stable for the backport dependency
v1: https://lore.kernel.org/lkml/20260601041336.9497-1-daniel@quora.org/
.../devicetree/bindings/media/qcom,sm8550-iris.yaml | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
index 9c4b760508b5..5abcaee4101c 100644
--- a/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,sm8550-iris.yaml
@@ -80,7 +80,10 @@ properties:
dma-coherent: true
memory-region:
- maxItems: 1
+ minItems: 1
+ items:
+ - description: Firmware-loaded codec carveout
+ - description: IOMMU IOVA reservation region
operating-points-v2: true
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v3 2/3] iio: adc: Add ti-ads1110 support to ti-ads1100 driver
From: Jonathan Cameron @ 2026-06-14 14:48 UTC (permalink / raw)
To: Jakub Szczudlo
Cc: linux-iio, andy, antoniu.miclaus, conor+dt, devicetree, dlechner,
duje, jishnu.prakash, jorge.marques, krzk+dt, linusw,
linux-kernel, marcelo.schmitt, mazziesaccount, mike.looijmans,
nuno.sa, robh, sakari.ailus, wens, joshua.crofts1
In-Reply-To: <20260613190957.654798-3-jakubszczudlo40@gmail.com>
On Sat, 13 Jun 2026 21:09:56 +0200
Jakub Szczudlo <jakubszczudlo40@gmail.com> wrote:
> Add ADS1110 support that have faster datarate than ADS1100, it also uses
> internal voltage reference of 2.048V for measurement.
>
> Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
Hi Jakub
A few minor things spotted on a fresh read,
Thanks,
Jonathan
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index aa8946063c7d..76de2466dc53 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
> static int ads1100_data_bits(struct ads1100_data *data)
> {
> return ads1100_data_rate_bits[FIELD_GET(ADS1100_DR_MASK, data->config)];
> @@ -107,9 +145,9 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
>
> pm_runtime_put_autosuspend(&data->client->dev);
>
> - if (ret < 0) {
> + if (ret < 2) {
If ret < 0 we don't want to eat the error value. So normally these have some dance
like
if (ret < 0) {
dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
return ret;
} else if (reg < 2) {
dev_err(&data->client->dev, "Short I2C read\n");
return -EIO;
}
> dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> - return ret;
> + return -EIO;
> }
...
> @@ -338,6 +371,12 @@ static int ads1100_probe(struct i2c_client *client)
> if (ret)
> return ret;
>
> + model = device_get_match_data(dev);
> + if (!model)
> + return dev_err_probe(dev, -EINVAL,
> + "Can't get device data from firmware\n");
> +
> + data->ads_config = (struct ads1100_config *)model;
Why is the cast needed? If you find yourself casting away a const
it normally implies there should be a const somewhere else.
In this case I think the pointer in the structure should simply be const.
> ret = ads1100_setup(data);
> if (ret)
> return dev_err_probe(dev, ret,
^ permalink raw reply
* Re: [PATCH v2 2/2] of: reserved_mem: print skipped node name when too many regions are defined
From: Sang-Heon Jeon @ 2026-06-14 14:46 UTC (permalink / raw)
To: sashiko-reviews; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260614135041.D944B1F000E9@smtp.kernel.org>
On Sun, Jun 14, 2026 at 10:50 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Out-of-bounds write to the static `reserved_mem_array` if `memblock_alloc` fails during dynamic array reallocation.
> - [High] Ignored memory reservation failures allow unreserved memory to be successfully registered and handed out as exclusive reserved memory.
> - [Medium] Dynamically allocated `no-map` reserved memory regions are leaked from `memblock.reserved` if their initialization fails.
> --
>
> commit f95986ab58870e3d79cb320c6dc73ef80982357e
> Author: Sang-Heon Jeon <ekffu200098@gmail.com>
>
> of: reserved_mem: print skipped node name when too many regions are defined
>
> When too many reserved memory regions are defined,
> fdt_init_reserved_mem_node() logs an error and skips the region without
> naming the node.
>
> Print the node name so the skipped node can be identified, matching
> fdt_scan_reserved_mem().
>
> > diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> > index 499b769e98096..df3d90cb8dbe1 100644
> > --- a/drivers/of/of_reserved_mem.c
> > +++ b/drivers/of/of_reserved_mem.c
> > @@ -623,7 +623,8 @@ static void __init fdt_init_reserved_mem_node(unsigned long node, const char *un
> > struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> >
> > if (reserved_mem_count == total_reserved_mem_cnt) {
> > - pr_err("not enough space for all defined regions.\n");
> > + pr_err("not enough space for all defined regions, skip '%s'\n",
> > + uname);
> > return;
> > }
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does alloc_reserved_mem_array()
> fail to reset total_reserved_mem_cnt on error?
>
> If memblock_alloc() fails during dynamic array reallocation, it returns early
> without resetting total_reserved_mem_cnt back to its original value. When the
> bounds check in fdt_init_reserved_mem_node() is reached later,
> reserved_mem_count can increment beyond the statically-sized
> reserved_mem_array limit, potentially causing an out-of-bounds write:
>
> drivers/of/of_reserved_mem.c:alloc_reserved_mem_array() {
> ...
> new_array = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
> if (!new_array) {
> pr_err("Failed to allocate memory for reserved_mem array...");
> return;
> }
> ...
> }
>
> [Severity: High]
> This is a pre-existing issue, but does __reserved_mem_reserve_reg()
> unconditionally return 0 even if early_init_dt_reserve_memory() fails?
>
> If memory reservation fails, it logs an error but returns success, allowing
> unreserved memory to be successfully registered and later handed out as
> exclusive reserved memory:
>
> drivers/of/of_reserved_mem.c:__reserved_mem_reserve_reg() {
> ...
> if (early_init_dt_reserve_memory(base, size, nomap)) {
> pr_err("Reserved memory: failed to reserve...");
> }
> }
> return 0;
> }
These areas are affected by the ongoing series [1], so it might be
better not to touch them until it is merged.
[1] https://lore.kernel.org/all/20260527032917.3385849-1-chenwandun1@gmail.com/
> [Severity: Medium]
> This is a pre-existing issue, but does fdt_init_reserved_mem_node() leak
> dynamically allocated no-map regions if initialization fails?
>
> If __reserved_mem_init_node() returns an error, the cleanup path for no-map
> regions clears the MEMBLOCK_NOMAP flag but skips calling memblock_phys_free(),
> which permanently leaks the physical memory from memblock.reserved:
>
> drivers/of/of_reserved_mem.c:fdt_init_reserved_mem_node() {
> ...
> err = __reserved_mem_init_node(rmem, node);
> if (err != 0 && err != -ENODEV) {
> pr_info("node %s compatible matching fail\n", rmem->name);
> rmem->name = NULL;
>
> if (nomap)
> memblock_clear_nomap(rmem->base, rmem->size);
> else
> memblock_phys_free(rmem->base, rmem->size);
> return;
> ...
> }
It should be rare, but looks correct to me. BTW, it is not directly
related to this patchset, so I think it is better to fix it in a
separate patch.
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260614133807.2165124-1-ekffu200098@gmail.com?part=2
Best Regards,
Sang-Heon Jeon
^ permalink raw reply
* Re: [PATCH v2 7/8] dt-bindings: display: allwinner: Split H616 DE33 layer reg space
From: Jernej Škrabec @ 2026-06-14 14:08 UTC (permalink / raw)
To: wens, Krzysztof Kozlowski
Cc: samuel, mripard, maarten.lankhorst, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, mturquette, sboyd, dri-devel, devicetree,
linux-arm-kernel, linux-sunxi, linux-kernel, linux-clk
In-Reply-To: <032c1099-40ab-470e-8cc3-af6d3cad22d4@kernel.org>
Dne ponedeljek, 25. maj 2026 ob 14:10:38 Srednjeevropski poletni čas je Krzysztof Kozlowski napisal(a):
> On 24/05/2026 23:33, Chen-Yu Tsai wrote:
> > Hi,
> >
> > (resent from new email)
> >
> > On Thu, May 14, 2026 at 2:04 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Sat, May 09, 2026 at 09:00:14PM +0200, Jernej Skrabec wrote:
> >>> From: Jernej Skrabec <jernej.skrabec@gmail.com>
> >>>
> >>> As it turns out, current H616 DE33 binding was written based on
> >>> incomplete understanding of DE33 design. Namely, planes are shared
> >>> resource and not tied to specific mixer, which was the case for previous
> >>> generations of Display Engine (DE3 and earlier).
> >>>
> >>> This means that current DE33 binding doesn't properly reflect HW and
> >>> using it would mean that second mixer (used for second display output)
> >>> can't be supported.
> >>>
> >>> Remove layer register space, which will be represented with additional
> >>> node, and replace it with phandle, which will point to that new, shared
> >>> node. That way, all mixers can share same layers.
> >>>
> >>> There is no user of this binding yet, so changes can be made safely,
> >>> without breaking any backward compatibility.
> >>
> >> There is user. git grep gives me:
> >> drivers/gpu/drm/sun4i/sun8i_mixer.c
> >>
> >> which means this is a released ABI. As I understood, the old code was
> >
> > We held off on merging the DT changes so that we could rework this.
> > I can't find the actual request though. It was probably over IRC.
> >
> >> working fine but just did not support all use cases. Why this cannot be
> >> kept backwards compatible?
> >
> > AFAIK the "planes" block is shared between two display mixers. As the
> > commit message explains, this prevents using the second mixer, since
> > only one of them can claim and map the register space. And on the H700
> > (which is the same die as the H616 discussed here but with more exposed
> > interfaces), there could actually be a use case for the second mixer.
>
> It explains why you want to make the changes but not why you cannot keep
> it backwards compatible.
I guess it can be backward compatible, but I don't think it makes sense.
Yes, original driver implemented original DT bindings, but there is no node
which uses that binding. If there is no user of that, why would driver
need to support it nevertheless? Supporting only actually used DT binding
allows for better code architecture, as there is no need to support second,
unused path. It also simplifies testing, since developer doesn't need to
test both paths if code is changed in that area.
Best regards,
Jernej
Best regards,
Jernej
^ permalink raw reply
* Re: [PATCH v2 2/2] of: reserved_mem: print skipped node name when too many regions are defined
From: sashiko-bot @ 2026-06-14 13:50 UTC (permalink / raw)
To: Sang-Heon Jeon; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260614133807.2165124-3-ekffu200098@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Out-of-bounds write to the static `reserved_mem_array` if `memblock_alloc` fails during dynamic array reallocation.
- [High] Ignored memory reservation failures allow unreserved memory to be successfully registered and handed out as exclusive reserved memory.
- [Medium] Dynamically allocated `no-map` reserved memory regions are leaked from `memblock.reserved` if their initialization fails.
--
commit f95986ab58870e3d79cb320c6dc73ef80982357e
Author: Sang-Heon Jeon <ekffu200098@gmail.com>
of: reserved_mem: print skipped node name when too many regions are defined
When too many reserved memory regions are defined,
fdt_init_reserved_mem_node() logs an error and skips the region without
naming the node.
Print the node name so the skipped node can be identified, matching
fdt_scan_reserved_mem().
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 499b769e98096..df3d90cb8dbe1 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -623,7 +623,8 @@ static void __init fdt_init_reserved_mem_node(unsigned long node, const char *un
> struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
>
> if (reserved_mem_count == total_reserved_mem_cnt) {
> - pr_err("not enough space for all defined regions.\n");
> + pr_err("not enough space for all defined regions, skip '%s'\n",
> + uname);
> return;
> }
[Severity: High]
This isn't a bug introduced by this patch, but does alloc_reserved_mem_array()
fail to reset total_reserved_mem_cnt on error?
If memblock_alloc() fails during dynamic array reallocation, it returns early
without resetting total_reserved_mem_cnt back to its original value. When the
bounds check in fdt_init_reserved_mem_node() is reached later,
reserved_mem_count can increment beyond the statically-sized
reserved_mem_array limit, potentially causing an out-of-bounds write:
drivers/of/of_reserved_mem.c:alloc_reserved_mem_array() {
...
new_array = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
if (!new_array) {
pr_err("Failed to allocate memory for reserved_mem array...");
return;
}
...
}
[Severity: High]
This is a pre-existing issue, but does __reserved_mem_reserve_reg()
unconditionally return 0 even if early_init_dt_reserve_memory() fails?
If memory reservation fails, it logs an error but returns success, allowing
unreserved memory to be successfully registered and later handed out as
exclusive reserved memory:
drivers/of/of_reserved_mem.c:__reserved_mem_reserve_reg() {
...
if (early_init_dt_reserve_memory(base, size, nomap)) {
pr_err("Reserved memory: failed to reserve...");
}
}
return 0;
}
[Severity: Medium]
This is a pre-existing issue, but does fdt_init_reserved_mem_node() leak
dynamically allocated no-map regions if initialization fails?
If __reserved_mem_init_node() returns an error, the cleanup path for no-map
regions clears the MEMBLOCK_NOMAP flag but skips calling memblock_phys_free(),
which permanently leaks the physical memory from memblock.reserved:
drivers/of/of_reserved_mem.c:fdt_init_reserved_mem_node() {
...
err = __reserved_mem_init_node(rmem, node);
if (err != 0 && err != -ENODEV) {
pr_info("node %s compatible matching fail\n", rmem->name);
rmem->name = NULL;
if (nomap)
memblock_clear_nomap(rmem->base, rmem->size);
else
memblock_phys_free(rmem->base, rmem->size);
return;
...
}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614133807.2165124-1-ekffu200098@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v2 1/2] of: reserved_mem: prevent OOB when too many dynamic regions are defined
From: sashiko-bot @ 2026-06-14 13:47 UTC (permalink / raw)
To: Sang-Heon Jeon; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260614133807.2165124-2-ekffu200098@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Global buffer overflow on static reserved_mem_array when dynamic allocation fails
--
commit ce2daed7217e851888336023e70ee6c49e1f66cd
Author: Sang-Heon Jeon <ekffu200098@gmail.com>
of: reserved_mem: prevent OOB when too many dynamic regions are defined
This commit adds bounds checking for dynamically-placed /reserved-memory
subnodes to prevent out-of-bounds writes to the local dynamic_nodes array
in fdt_scan_reserved_mem().
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 8d5777cb5d1b3..499b769e98096 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
[ ... ]
> @@ -348,10 +349,17 @@ int __init fdt_scan_reserved_mem(void)
> * or marked as no-map. This is done to avoid dynamically
> * allocating from one of the statically-placed regions.
> */
> - if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) {
> - dynamic_nodes[dynamic_nodes_cnt] = child;
> - dynamic_nodes_cnt++;
> + if (err != -ENOENT || !of_get_flat_dt_prop(child, "size", NULL))
> + continue;
> +
> + if (dynamic_nodes_cnt == MAX_RESERVED_REGIONS) {
> + pr_err("too many defined dynamic regions, skip '%s'\n",
> + uname);
> + continue;
> }
> +
> + dynamic_nodes[dynamic_nodes_cnt] = child;
> + dynamic_nodes_cnt++;
> }
> for (int i = 0; i < dynamic_nodes_cnt; i++) {
> const char *uname;
[Severity: Critical]
This is a pre-existing issue, but could the initialization sequence still
leave the kernel vulnerable to a global buffer overflow later on?
In fdt_scan_reserved_mem(), total_reserved_mem_cnt is updated to the
total count of regions, which could exceed MAX_RESERVED_REGIONS due to
statically-placed regions.
Later, fdt_scan_reserved_mem_late() calls alloc_reserved_mem_array() to
dynamically allocate a larger array:
drivers/of/of_reserved_mem.c:alloc_reserved_mem_array() {
...
new_array = memblock_alloc(alloc_size, SMP_CACHE_BYTES);
if (!new_array) {
pr_err("Failed to allocate memory for reserved_mem array with err: %d", -ENOMEM);
return;
}
...
}
If memblock_alloc() fails, it returns early but does not reset
total_reserved_mem_cnt back to MAX_RESERVED_REGIONS.
When iterating over the regions, fdt_init_reserved_mem_node() relies on
total_reserved_mem_cnt for its bounds checking:
drivers/of/of_reserved_mem.c:fdt_init_reserved_mem_node() {
...
if (reserved_mem_count == total_reserved_mem_cnt) {
pr_err("not enough space for all defined regions, skip '%s'\n",
uname);
return;
}
...
}
Since total_reserved_mem_cnt remains inflated, this check will fail to protect
the statically-sized reserved_mem_array, leading to an out-of-bounds write.
Should alloc_reserved_mem_array() reset total_reserved_mem_cnt to
MAX_RESERVED_REGIONS on the error path to avoid overflowing the static
reserved_mem_array?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614133807.2165124-1-ekffu200098@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 2/5] iio: adc: Add ti-ads1262 driver
From: Jonathan Cameron @ 2026-06-14 13:39 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Kurt Borja, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Bartosz Golaszewski, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, devicetree, linux-kernel, linux-gpio
In-Reply-To: <20260613-sparkling-naughty-tuna-3e9bf1@quoll>
> > +
> > +DEFINE_RUNTIME_DEV_PM_OPS(ads1262_runtime_pm, ads1262_runtime_suspend,
> > + ads1262_runtime_resume, NULL);
> > +
> > +static const struct of_device_id ads1262_of_match[] = {
> > + { .compatible = "ti,ads1262" },
> > + { .compatible = "ti,ads1263" },
>
> So devices are fully compatible? Then it should be expressed in the
> binding and drop one entry here.
They aren't. It's relying on one of them having a subnode that spins up an
auxdev for the hardware block they don't share. A fallback would be fine
but (to the device that has the more minimal feature set). I'd prefer the
driver to have a check on whether the subnode is allowed before blindly
registering it.
Jonathan
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* [PATCH v2 2/2] of: reserved_mem: print skipped node name when too many regions are defined
From: Sang-Heon Jeon @ 2026-06-14 13:38 UTC (permalink / raw)
To: robh, saravanak; +Cc: devicetree, Sang-Heon Jeon
In-Reply-To: <20260614133807.2165124-1-ekffu200098@gmail.com>
When too many reserved memory regions are defined,
fdt_init_reserved_mem_node() logs an error and skips the region without
naming the node.
Print the node name so the skipped node can be identified, matching
fdt_scan_reserved_mem().
Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
drivers/of/of_reserved_mem.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 42e3e2d8a2b8..7b1616b9c6c2 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -641,7 +641,8 @@ static void __init fdt_init_reserved_mem_node(unsigned long node, const char *un
struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
if (reserved_mem_count == total_reserved_mem_cnt) {
- pr_err("not enough space for all defined regions.\n");
+ pr_err("not enough space for all defined regions, skip '%s'\n",
+ uname);
return;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v2 1/2] of: reserved_mem: prevent OOB when too many dynamic regions are defined
From: Sang-Heon Jeon @ 2026-06-14 13:38 UTC (permalink / raw)
To: robh, saravanak; +Cc: devicetree, Sang-Heon Jeon
In-Reply-To: <20260614133807.2165124-1-ekffu200098@gmail.com>
On boot, fdt_scan_reserved_mem() saves each dynamically-placed
/reserved-memory subnode into a local array of size
MAX_RESERVED_REGIONS.
If the device tree defines more than MAX_RESERVED_REGIONS
dynamically-placed regions, fdt_scan_reserved_mem() writes past the
end of the local array.
Add a bounds check that logs an error and skips the excess regions,
restoring the original behavior.
Fixes: 8a6e02d0c00e ("of: reserved_mem: Restructure how the reserved memory regions are processed")
Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
drivers/of/of_reserved_mem.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index 82222bd45ac6..42e3e2d8a2b8 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -359,6 +359,7 @@ int __init fdt_scan_reserved_mem(void)
err = __reserved_mem_reserve_reg(child, uname);
if (!err)
count++;
+
/*
* Save the nodes for the dynamically-placed regions
* into an array which will be used for allocation right
@@ -366,10 +367,17 @@ int __init fdt_scan_reserved_mem(void)
* or marked as no-map. This is done to avoid dynamically
* allocating from one of the statically-placed regions.
*/
- if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL)) {
- dynamic_nodes[dynamic_nodes_cnt] = child;
- dynamic_nodes_cnt++;
+ if (err != -ENOENT || !of_get_flat_dt_prop(child, "size", NULL))
+ continue;
+
+ if (dynamic_nodes_cnt == MAX_RESERVED_REGIONS) {
+ pr_err("too many defined dynamic regions, skip '%s'\n",
+ uname);
+ continue;
}
+
+ dynamic_nodes[dynamic_nodes_cnt] = child;
+ dynamic_nodes_cnt++;
}
for (int i = 0; i < dynamic_nodes_cnt; i++) {
const char *uname;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/2] of: reserved_mem: fix OOB write and name skipped nodes
From: Sang-Heon Jeon @ 2026-06-14 13:38 UTC (permalink / raw)
To: robh, saravanak; +Cc: devicetree, Sang-Heon Jeon
Patch 1 fixes an out-of-bounds write in fdt_scan_reserved_mem(). When
more than MAX_RESERVED_REGIONS dynamically-placed regions are defined,
it writes past the end of the local array.
Patch 2 names the skipped node in fdt_init_reserved_mem_node(), the other
place that drops regions.
Changes from v1 [1]
- fix bounds check to cover the missed case in v1, based on sashiko
review
- print skipped no name error message in fdt_scan_reserved_mem() and
fdt_init_reserved_mem_node() both.
[1] https://lore.kernel.org/all/20260603152709.941788-1-ekffu200098@gmail.com/
Sang-Heon Jeon (2):
of: reserved_mem: prevent OOB when too many dynamic regions are
defined
of: reserved_mem: print skipped node name when too many regions are
defined
drivers/of/of_reserved_mem.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH 4/7] drivers: staging: media: sunxi: cedrus: add H616 variant
From: Jernej Škrabec @ 2026-06-14 13:36 UTC (permalink / raw)
To: wens
Cc: Maxime Ripard, Paul Kocialkowski, Mauro Carvalho Chehab,
Jernej Skrabec, Samuel Holland, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman, linux-media, linux-staging,
devicetree, linux-sunxi, linux-arm-kernel, linux-kernel
In-Reply-To: <CAGb2v677Pi9s3eWC7aXh8j=+eJh7AV5Mucr7SDXMN9Lo8yA2nA@mail.gmail.com>
Dne sobota, 13. junij 2026 ob 16:34:00 Srednjeevropski poletni čas je Chen-Yu Tsai napisal(a):
> On Sat, Jun 13, 2026 at 6:33 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> >
> > Dne sobota, 30. maj 2026 ob 18:43:05 Srednjeevropski poletni čas je Chen-Yu Tsai napisal(a):
> > > On Tue, May 5, 2026 at 7:18 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> > > >
> > > > Dne torek, 5. maj 2026 ob 15:48:08 Srednjeevropski poletni čas je Chen-Yu Tsai napisal(a):
> > > > > The Allwinner H616 SoC has a video engine hardware block like the one
> > > > > found on previous generations such as the H6. In addition to the
> > > > > currently supported features of the H6, it is also supposed to include
> > > >
> > > > Remove "supposed".
> > >
> > > I can't actually verify that, so "supposed" is accurate from my point of
> > > view.
> >
> > Isn't info from manual good enough?
>
> The manual says the SoC supports it. Same was said for the H6. Then
> we discovered that the VP9 decoder was a separate Hantro block.
>
> So again, *I* cannot claim in the commit message that the hardware
> block supports VP9 decoding, because I have not verified it.
>
> > In the interest of unblocking this, I would be fine with "supposed" too,
> > but manual and all my experiments show VP9 is supported.
>
> Please give an ack or reviewed-by with a comment at the end stating
> VP9 verified.
Well, just go with original text.
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej
>
>
> Thanks
> ChenYu
>
>
> > Best regards,
> > Jernej
> >
> > >
> > > ChenYu
> > >
> > > > > a VP9 decoder. However software support for this is currently missing
> > > > > and still needs to be reverse engineered from the vendor BSP.
> > > > >
> > > > > Add the compatible for the H616 variant, using the H6 variant data.
> > > > >
> > > > > Signed-off-by: Chen-Yu Tsai <wens@kernel.org>
> > > >
> > > > With that:
> > > > Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > > >
> > > > Best regards,
> > > > Jernej
> > > >
> > > >
> > >
> >
> >
> >
> >
> >
>
^ permalink raw reply
* Re: [PATCH v10 4/6] dt-bindings: sun6i-a31-mipi-dphy: Add V3s SoC compatible entry
From: Paul Kocialkowski @ 2026-06-14 13:28 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-media, devicetree, linux-arm-kernel, linux-sunxi,
linux-kernel, Yong Deng, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Michael Turquette, Stephen Boyd, Brian Masney,
Maxime Ripard
In-Reply-To: <20260613-nondescript-sociable-goat-aee13a@quoll>
[-- Attachment #1: Type: text/plain, Size: 1535 bytes --]
Hi,
Le Sat 13 Jun 26, 20:22, Krzysztof Kozlowski a écrit :
> On Sat, Jun 13, 2026 at 05:26:53PM +0200, Paul Kocialkowski wrote:
> > The V3s/V3/S3 comes with a rx-only D-PHY paired with the MIPI CSI-2
> > controller. It is compatible with the D-PHY found on the A31.
> >
> > Add an entry with a new compatible and the A31 compatible as fallback.
> >
> > Signed-off-by: Paul Kocialkowski <paulk@sys-base.io>
> > ---
> > .../devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
> > index 6a4fd4929959..3ca1a1c47032 100644
> > --- a/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
> > +++ b/Documentation/devicetree/bindings/phy/allwinner,sun6i-a31-mipi-dphy.yaml
> > @@ -21,6 +21,9 @@ properties:
> > - items:
> > - const: allwinner,sun50i-a64-mipi-dphy
> > - const: allwinner,sun6i-a31-mipi-dphy
> > + - items:
> > + - const: allwinner,sun8i-v3s-mipi-dphy
>
> So that's enum with previous first entry (50i-a64) - same fallback.
Ah sorry about that. Thanks for the review!
All the best,
Paul
--
Paul Kocialkowski,
Independent contractor - sys-base - https://www.sys-base.io/
Free software developer - https://www.paulk.fr/
Expert in multimedia, graphics and embedded hardware support with Linux.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] ARM: dts: exynos: Add bluetooth support to manta
From: Krzysztof Kozlowski @ 2026-06-14 13:25 UTC (permalink / raw)
To: Lukas Timmermann, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alim Akhtar
Cc: devicetree, linux-arm-kernel, linux-samsung-soc, linux-kernel,
Alexandre Marquet
In-Reply-To: <ai2OYGlCx0x4NUs8@archstation>
On 13/06/2026 19:11, Lukas Timmermann wrote:
> #include <atomic>
> On Mon, Apr 27, 2026 at 03:49:34PM +0200, Krzysztof Kozlowski wrote:
>> On 08/04/2026 13:56, Lukas Timmermann wrote:
>>> Enable the bcm4330-bt device for manta boards on serial0.
>>> Also adds the necessary pin definitions and interrupt handling for
>>> wakeup.
>>>
>>> Signed-off-by: Lukas Timmermann <linux@timmermann.space>
>>> Co-developed-by: Alexandre Marquet <tb@a-marquet.fr>
>>> Signed-off-by: Alexandre Marquet <tb@a-marquet.fr>
>>
>> Incomplete/incorrect DCO chain. Please do not reorder tags. Git does
>> them correctly, so you HAD to change them manually.
>>
>> You send the patch or you apply the patch so you must commit with sign off.
> I developed the actual patch based on his findings. We both don't really
> care about who is mentioned first or anything.
>
> Sorry. Yes I rearranged stuff. So it should be:
>
> co-dev: alex
> sign-off: alex
> co-dev: me
> sign-off: me
>
> Correct?
>>
Yes
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v4 2/2] arm64: dts: qcom: x1e80100-lenovo-yoga-slim7x: Add Embedded Controller node
From: Daniel J Blueman @ 2026-06-14 13:06 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson
Cc: linux-arm-msm, devicetree, Sibi Sankar, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans de Goede, Randy Dunlap,
Bryan O'Donoghue, linux-kernel, Anvesh Jain P,
Maya Matuszczyk, Krzysztof Kozlowski, Dmitry Baryshkov,
Akhil P Oommen, Abel Vesa, Gaurav Kohli, Daniel J Blueman
In-Reply-To: <20260614130621.68811-1-daniel@quora.org>
The Lenovo Slim7x uses the same Embedded Controller as the Qualcomm Hamoa
X1 Customer Reference Device. Use the lenovo,yoga-slim7x-ec compatible
introduced by patch 1 for fan control, thermal sensor and suspend
behaviour.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Sibi Sankar <sibi.sankar@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Daniel J Blueman <daniel@quora.org>
---
v4:
- add reviews
v3:
- use lenovo,yoga-slim7x-ec compatible (introduced by patch 1)
v2:
- corrected DT compatible node
.../dts/qcom/x1e80100-lenovo-yoga-slim7x.dts | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
index beb1475d7fa0..1ee2a2296129 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
@@ -951,6 +951,22 @@ retimer_ss0_con_sbu_out: endpoint {
};
};
+&i2c5 {
+ clock-frequency = <400000>;
+
+ status = "okay";
+
+ embedded-controller@76 {
+ compatible = "lenovo,yoga-slim7x-ec", "qcom,hamoa-crd-ec";
+ reg = <0x76>;
+
+ interrupts-extended = <&tlmm 66 IRQ_TYPE_EDGE_FALLING>;
+
+ pinctrl-0 = <&ec_int_n_default>;
+ pinctrl-names = "default";
+ };
+};
+
&i2c7 {
clock-frequency = <400000>;
@@ -1352,6 +1368,12 @@ &tlmm {
<44 4>, /* SPI (TPM) */
<238 1>; /* UFS Reset */
+ ec_int_n_default: ec-int-n-state {
+ pins = "gpio66";
+ function = "gpio";
+ bias-disable;
+ };
+
edp_reg_en: edp-reg-en-state {
pins = "gpio70";
function = "gpio";
--
2.53.0
^ permalink raw reply related
* [PATCH v4 1/2] dt-bindings: embedded-controller: qcom,hamoa-crd-ec: add Lenovo Yoga Slim 7x
From: Daniel J Blueman @ 2026-06-14 13:06 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson
Cc: linux-arm-msm, devicetree, Sibi Sankar, Randy Dunlap, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans de Goede,
Bryan O'Donoghue, linux-kernel, Anvesh Jain P,
Maya Matuszczyk, Krzysztof Kozlowski, Dmitry Baryshkov,
Akhil P Oommen, Abel Vesa, Gaurav Kohli, Daniel J Blueman
The Lenovo Yoga Slim 7x uses the same Embedded Controller as the Qualcomm
Hamoa X1 CRD. Add a board-specific compatible with qcom,hamoa-crd-ec as
the fallback.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Daniel J Blueman <daniel@quora.org>
---
v4:
- add reviews and acknowledgements
- updated dependency URL
v3: https://lore.kernel.org/lkml/20260526112409.66325-1-daniel@quora.org/
- new patch with DT bindings
v2: https://lore.kernel.org/lkml/20260502063518.15153-1-daniel@quora.org/
v1: https://lore.kernel.org/lkml/20260429103301.17449-1-daniel@quora.org/
Dependencies:
https://lore.kernel.org/lkml/20260511-add-driver-for-ec-v9-0-e5437c39b7f8@oss.qualcomm.com/
.../bindings/embedded-controller/qcom,hamoa-crd-ec.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-crd-ec.yaml b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-crd-ec.yaml
index ac5a08f8f76d..813d41769c0b 100644
--- a/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-crd-ec.yaml
+++ b/Documentation/devicetree/bindings/embedded-controller/qcom,hamoa-crd-ec.yaml
@@ -20,6 +20,7 @@ properties:
oneOf:
- items:
- enum:
+ - lenovo,yoga-slim7x-ec
- qcom,glymur-crd-ec
- qcom,hamoa-iot-evk-ec
- const: qcom,hamoa-crd-ec
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] riscv: dts: spacemit: k1-musepi-pro: add cpu scaling
From: sashiko-bot @ 2026-06-14 12:32 UTC (permalink / raw)
To: Andre Heider; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260614122812.2287506-1-a.heider@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The devicetree file includes 'k1-opp.dtsi', but this file is missing from the tree.
--
commit dbb35bd6325bb2f268c9e7237e65d5c614a6f755
Author: Andre Heider <a.heider@gmail.com>
riscv: dts: spacemit: k1-musepi-pro: add cpu scaling
This commit enables CPU DVFS for the SpacemiT MusePi Pro board by including
an OPP table and wiring the CPU nodes to the buck1_0v9 regulator supply.
> diff --git a/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts b/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
> index 0d809e4ad3b1f..1b425011f1231 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
> +++ b/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
> @@ -9,6 +9,7 @@
>
> #include "k1.dtsi"
> #include "k1-pinctrl.dtsi"
> +#include "k1-opp.dtsi"
[Severity: Low]
Is the k1-opp.dtsi file missing from this series?
It appears this include will cause a devicetree compilation failure because
the file does not exist in the tree.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260614122812.2287506-1-a.heider@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 2/4] phy: qcom-qusb2: Fix SM6115 init sequence
From: Iskren Chernev @ 2026-06-14 12:29 UTC (permalink / raw)
To: Konrad Dybcio, Vinod Koul, Neil Armstrong, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Wesley Cheng,
Greg Kroah-Hartman, Bjorn Andersson
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, Konrad Dybcio
In-Reply-To: <20260610-topic-8996_61x5_qusb2phy-v1-2-d7135980e78f@oss.qualcomm.com>
On 6/10/26 3:04 PM, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> I don't know where the existing one came from, but it's apparently
> wrong, according to both docs and a downstream DT [1]. Fix it up.
They came from DTB extracted from a running billie2 (OnePlus Nord N100):
[1]
https://mainlining.dev/wp-content/uploads/2021/02/03_dtbdump_Qualcomm_Technologies_Inc._Bengal_SoC.dts
The phone was bough early after launch, so it could have been
wrong/updated later.
> The updated values also happen to match SM6125, which will allow us
> to fix that platform too.
>
> [1] https://android.googlesource.com/kernel/msm-extra/devicetree/+/refs/heads/android-msm-bramble-4.19-android11-qpr1/qcom/bengal-usb.dtsi#145
> Fixes: 7756f1d6369e ("phy: qcom-qusb2: Add configuration for SM4250 and SM6115")
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/phy/qualcomm/phy-qcom-qusb2.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> index eb93015be841..c304ccd9f31f 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> @@ -233,9 +233,9 @@ static const struct qusb2_phy_init_tbl msm8998_init_tbl[] = {
>
> static const struct qusb2_phy_init_tbl sm6115_init_tbl[] = {
> QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE1, 0xf8),
> - QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE2, 0x53),
> + QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE2, 0xb3),
> QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE3, 0x81),
> - QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE4, 0x17),
> + QUSB2_PHY_INIT_CFG_L(QUSB2PHY_PORT_TUNE4, 0xc0),
>
> QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_TUNE, 0x30),
> QUSB2_PHY_INIT_CFG(QUSB2PHY_PLL_USER_CTL1, 0x79),
>
^ permalink raw reply
* [PATCH] riscv: dts: spacemit: k1-musepi-pro: add cpu scaling
From: Andre Heider @ 2026-06-14 12:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Yixun Lan
Cc: devicetree, linux-riscv, spacemit, linux-kernel
In-Reply-To: <20260612-shadow-deps-v3-0-2f3ba88611ff@mailbox.org>
Enable CPU DVFS by including the OPP table and wiring the CPU nodes to the
CPU regulator supply.
Signed-off-by: Andre Heider <a.heider@gmail.com>
---
.../riscv/boot/dts/spacemit/k1-musepi-pro.dts | 35 ++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts b/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
index 0d809e4ad3b1..1b425011f123 100644
--- a/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
+++ b/arch/riscv/boot/dts/spacemit/k1-musepi-pro.dts
@@ -9,6 +9,7 @@
#include "k1.dtsi"
#include "k1-pinctrl.dtsi"
+#include "k1-opp.dtsi"
/ {
model = "SpacemiT MusePi Pro";
@@ -99,6 +100,38 @@ &combo_phy {
status = "okay";
};
+&cpu_0 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_1 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_2 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_3 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_4 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_5 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_6 {
+ cpu-supply = <&buck1_0v9>;
+};
+
+&cpu_7 {
+ cpu-supply = <&buck1_0v9>;
+};
+
ð0 {
phy-handle = <&rgmii0>;
phy-mode = "rgmii-id";
@@ -178,7 +211,7 @@ pmic@41 {
dldoin2-supply = <&buck5>;
regulators {
- buck1 {
+ buck1_0v9: buck1 {
regulator-min-microvolt = <500000>;
regulator-max-microvolt = <3450000>;
regulator-ramp-delay = <5000>;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH net-next 8/8] net: mdio: realtek-rtl9300: Add support for RTL839x
From: sashiko-bot @ 2026-06-14 11:30 UTC (permalink / raw)
To: Markus Stockhausen; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260613112946.1071411-9-markus.stockhausen@gmx.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing RTL8390_PHY_CTRL_PARK_PAGE initialization corrupts PHY BMCR
--
commit af49d45856a275a9bfebf688b76101516b5e946d
Author: Markus Stockhausen <markus.stockhausen@gmx.de>
net: mdio: realtek-rtl9300: Add support for RTL839x
The MDIO driver is prepared for multiple device support by adding
required bits for the RTL839x (aka cypress) series. The hardware
shares features with the RTL931x series, including 8192 pages and
7 MMIO registers.
> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index d9ff0b0aecbb1..2ab1aeb85eedb 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
[ ... ]
> @@ -457,6 +480,62 @@ static int otto_emdio_8380_write_c45(struct mii_bus *bus, int port,
> return otto_emdio_write_cmd(bus, RTL8380_PHY_CTRL_TYPE_C45, &cmd_data);
> }
>
> +static int otto_emdio_8390_read_c22(struct mii_bus *bus, int port, int regnum, u32 *value)
> +{
> + struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
> + struct otto_emdio_cmd_regs cmd_data = {
> + .c22_data = FIELD_PREP(RTL8390_PHY_CTRL_REG_ADDR, regnum) |
> + FIELD_PREP(RTL8390_PHY_CTRL_MAIN_PAGE, priv->page[port]),
[Severity: High]
Does this code inadvertently omit the PARK_PAGE configuration?
Without setting FIELD_PREP(RTL8390_PHY_CTRL_PARK_PAGE, 0x1f), this field
implicitly defaults to 0. Would this cause the hardware polling engine
to write the PHY page value to register 0 (the Basic Mode Control Register)
instead of register 31?
If so, this could overwrite the BMCR on every C22 command and corrupt the
PHY's core configuration.
> + .ext_page = FIELD_PREP(RTL8390_PHY_CTRL_EXT_PAGE, 0x1ff),
> + .io_data = FIELD_PREP(RTL8390_PHY_CTRL_INDATA, port),
> + };
> +
> + return otto_emdio_read_cmd(bus, RTL8390_PHY_CTRL_TYPE_C22, &cmd_data,
> + RTL8390_PHY_CTRL_DATA, value);
> +}
> +
> +static int otto_emdio_8390_write_c22(struct mii_bus *bus, int port, int regnum, u16 value)
> +{
> + struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
> + struct otto_emdio_cmd_regs cmd_data = {
> + .c22_data = FIELD_PREP(RTL8390_PHY_CTRL_REG_ADDR, regnum) |
> + FIELD_PREP(RTL8390_PHY_CTRL_MAIN_PAGE, priv->page[port]),
[Severity: High]
Are we missing the FIELD_PREP(RTL8390_PHY_CTRL_PARK_PAGE, 0x1f) configuration
here as well?
> + .ext_page = FIELD_PREP(RTL8390_PHY_CTRL_EXT_PAGE, 0x1ff),
> + .io_data = FIELD_PREP(RTL8390_PHY_CTRL_INDATA, value),
> + .port_mask_high = (u32)(BIT_ULL(port) >> 32),
> + .port_mask_low = (u32)(BIT_ULL(port)),
> + };
> +
> + return otto_emdio_write_cmd(bus, RTL8390_PHY_CTRL_TYPE_C22, &cmd_data);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613112946.1071411-1-markus.stockhausen@gmx.de?part=8
^ permalink raw reply
* Re: [PATCH net-next 4/8] net: mdio: realtek-rtl9300: Configure hardware polling during probing
From: sashiko-bot @ 2026-06-14 11:30 UTC (permalink / raw)
To: Markus Stockhausen; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260613112946.1071411-5-markus.stockhausen@gmx.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Hardware polling mitigation is bypassed by deferred, asynchronous, or late PHY driver probing.
--
commit f401bbac0c9ccd3010cafd736b0916dccaad8dca
Author: Markus Stockhausen <markus.stockhausen@gmx.de>
net: mdio: realtek-rtl9300: Configure hardware polling during probing
This patch provides helpers to enable or disable hardware polling for a
specific port. It uses these to temporarily disable hardware polling during
MDIO bus probing to prevent interference with complex PHY initialization
sequences, re-enabling it afterwards.
> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index c3a9eeca31540..a7fd075947b67 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
[ ... ]
> @@ -750,6 +759,13 @@ static int otto_emdio_probe(struct platform_device *pdev)
> if (IS_ERR(priv->regmap))
> return PTR_ERR(priv->regmap);
>
> + /* Avoid issues with complex firmware loads. */
> + for (port = 0; port < priv->info->num_ports; port++) {
> + err = otto_emdio_set_port_polling(priv, port, false);
> + if (err)
> + return err;
> + }
> +
> platform_set_drvdata(pdev, priv);
>
> err = otto_emdio_map_ports(dev);
[ ... ]
> @@ -772,6 +788,12 @@ static int otto_emdio_probe(struct platform_device *pdev)
> return err;
> }
>
> + for_each_set_bit(port, priv->valid_ports, priv->info->num_ports) {
> + err = otto_emdio_set_port_polling(priv, port, true);
> + if (err)
> + return err;
> + }
> +
[Severity: Medium]
Could this hardware polling mitigation be bypassed if a PHY driver defers
probing or loads asynchronously?
The mitigation temporarily disables polling across the MDIO bus registration
and re-enables it here unconditionally. Since PHY devices are registered as
children of the MDIO bus, they might bind to their drivers asynchronously,
return -EPROBE_DEFER, or be bound later via sysfs.
If that happens, wouldn't the PHY initialize while the hardware polling is
already re-enabled, potentially causing the interference this patch is trying
to avoid?
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613112946.1071411-1-markus.stockhausen@gmx.de?part=4
^ permalink raw reply
* Re: [PATCH net-next 5/8] net: mdio: realtek-rtl9300: Add c45 over c22 mitigation
From: sashiko-bot @ 2026-06-14 11:30 UTC (permalink / raw)
To: Markus Stockhausen; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260613112946.1071411-6-markus.stockhausen@gmx.de>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] State Machine Desynchronization and Hardware Polling Corruption on MDIO Error
--
commit 6336036b9a3d5d0b25bcff57860a0355d0dabd49
Author: Markus Stockhausen <markus.stockhausen@gmx.de>
net: mdio: realtek-rtl9300: Add c45 over c22 mitigation
Enhances the RTL9300 MDIO driver to detect the 4-step c45-over-c22 MMD
sequence. Disables hardware polling during the sequence to prevent concurrent
access from breaking the kernel's read/writes. Adds a state machine to track
sequence progress.
> diff --git a/drivers/net/mdio/mdio-realtek-rtl9300.c b/drivers/net/mdio/mdio-realtek-rtl9300.c
> index a7fd075947b67..e206ee3e2b1cc 100644
> --- a/drivers/net/mdio/mdio-realtek-rtl9300.c
> +++ b/drivers/net/mdio/mdio-realtek-rtl9300.c
[ ... ]
> @@ -254,6 +257,43 @@ static int otto_emdio_set_port_polling(struct otto_emdio_priv *priv, int port, b
> BIT(port % 32), active);
> }
>
> +static int otto_emdio_mmd_prefix(struct otto_emdio_priv *priv, int port, int regnum)
> +{
> + u8 newstate, *state = &priv->mmd_state[port];
> + int expected, ret = 0;
> +
[ ... ]
> + expected = (*state & 1) ? MII_MMD_DATA : MII_MMD_CTRL;
> + newstate = regnum == expected ? *state + 1 : 0;
> +
> + if (newstate == 1 || newstate < *state)
> + ret = otto_emdio_set_port_polling(priv, port, !newstate);
> + *state = newstate;
> +
> + return ret;
> +}
> +
> +static int otto_emdio_mmd_postfix(struct otto_emdio_priv *priv, int port, int regnum)
> +{
> + if (priv->mmd_state[port] != 4)
> + return 0;
> +
> + priv->mmd_state[port] = 0;
> +
> + return otto_emdio_set_port_polling(priv, port, true);
> +}
> +
[ ... ]
> @@ -463,7 +503,15 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
> if (regnum == 31)
> return priv->page[port];
>
> + ret = otto_emdio_mmd_prefix(priv, port, regnum);
> + if (ret)
> + return ret;
> +
> ret = priv->info->read_c22(bus, port, regnum, &value);
> + if (ret)
> + return ret;
[Severity: High]
If priv->info->read_c22() returns an error, the function returns early and
skips otto_emdio_mmd_postfix(), leaving priv->mmd_state[port] in an
intermediate state.
If an error occurs on the final step, the state is left at 4. A new MMD
sequence starting with register 13 will match the expected register logic
in otto_emdio_mmd_prefix() and increment the state to 5, then 6, etc.
Since otto_emdio_mmd_postfix() only re-enables polling when the state is
exactly 4, will hardware polling remain permanently disabled for this port
if an error occurs mid-sequence?
> +
> + ret = otto_emdio_mmd_postfix(priv, port, regnum);
> }
>
> return ret ? ret : value;
> @@ -472,7 +520,7 @@ static int otto_emdio_read_c22(struct mii_bus *bus, int phy_id, int regnum)
> static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum, u16 value)
> {
> struct otto_emdio_priv *priv = otto_emdio_bus_to_priv(bus);
> - int port;
> + int port, ret;
>
> port = otto_emdio_phy_to_port(bus, phy_id);
> if (port < 0)
> @@ -487,7 +535,15 @@ static int otto_emdio_write_c22(struct mii_bus *bus, int phy_id, int regnum, u16
> return 0;
> }
>
> - return priv->info->write_c22(bus, port, regnum, value);
> + ret = otto_emdio_mmd_prefix(priv, port, regnum);
> + if (ret)
> + return ret;
> +
> + ret = priv->info->write_c22(bus, port, regnum, value);
> + if (ret)
> + return ret;
[Severity: High]
Similarly, does returning early here on error skip otto_emdio_mmd_postfix()
and desynchronize the state machine?
> +
> + return otto_emdio_mmd_postfix(priv, port, regnum);
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260613112946.1071411-1-markus.stockhausen@gmx.de?part=5
^ 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