Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v2 2/5] dpll: zl3073x: use FIELD_MODIFY() for clear-and-set patterns
From: Ivan Vecera @ 2026-03-28  8:06 UTC (permalink / raw)
  To: netdev
  Cc: Petr Oros, Prathosh Satish, Arkadiusz Kubalewski, Jiri Pirko,
	Michal Schmidt, Simon Horman, Vadim Fedorenko, linux-kernel,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree,
	Pasi Vaananen
In-Reply-To: <20260328080624.593916-1-ivecera@redhat.com>

Replace open-coded clear-and-set bitfield operations with
FIELD_MODIFY().

Reviewed-by: Petr Oros <poros@redhat.com>
Reviewed-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/chan.h  | 17 ++++++-----------
 drivers/dpll/zl3073x/core.c  |  3 +--
 drivers/dpll/zl3073x/flash.c |  3 +--
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/dpll/zl3073x/chan.h b/drivers/dpll/zl3073x/chan.h
index e0f02d3432086..481da2133202b 100644
--- a/drivers/dpll/zl3073x/chan.h
+++ b/drivers/dpll/zl3073x/chan.h
@@ -66,8 +66,7 @@ static inline u8 zl3073x_chan_ref_get(const struct zl3073x_chan *chan)
  */
 static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
 {
-	chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_MODE;
-	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_MODE, mode);
+	FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_MODE, &chan->mode_refsel, mode);
 }
 
 /**
@@ -77,8 +76,7 @@ static inline void zl3073x_chan_mode_set(struct zl3073x_chan *chan, u8 mode)
  */
 static inline void zl3073x_chan_ref_set(struct zl3073x_chan *chan, u8 ref)
 {
-	chan->mode_refsel &= ~ZL_DPLL_MODE_REFSEL_REF;
-	chan->mode_refsel |= FIELD_PREP(ZL_DPLL_MODE_REFSEL_REF, ref);
+	FIELD_MODIFY(ZL_DPLL_MODE_REFSEL_REF, &chan->mode_refsel, ref);
 }
 
 /**
@@ -110,13 +108,10 @@ zl3073x_chan_ref_prio_set(struct zl3073x_chan *chan, u8 ref, u8 prio)
 {
 	u8 *val = &chan->ref_prio[ref / 2];
 
-	if (!(ref & 1)) {
-		*val &= ~ZL_DPLL_REF_PRIO_REF_P;
-		*val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_P, prio);
-	} else {
-		*val &= ~ZL_DPLL_REF_PRIO_REF_N;
-		*val |= FIELD_PREP(ZL_DPLL_REF_PRIO_REF_N, prio);
-	}
+	if (!(ref & 1))
+		FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_P, val, prio);
+	else
+		FIELD_MODIFY(ZL_DPLL_REF_PRIO_REF_N, val, prio);
 }
 
 /**
diff --git a/drivers/dpll/zl3073x/core.c b/drivers/dpll/zl3073x/core.c
index 6363002d48d46..7eebfc1ad1019 100644
--- a/drivers/dpll/zl3073x/core.c
+++ b/drivers/dpll/zl3073x/core.c
@@ -743,8 +743,7 @@ int zl3073x_dev_phase_avg_factor_set(struct zl3073x_dev *zldev, u8 factor)
 	value = (factor + 1) & 0x0f;
 
 	/* Update phase measurement control register */
-	dpll_meas_ctrl &= ~ZL_DPLL_MEAS_CTRL_AVG_FACTOR;
-	dpll_meas_ctrl |= FIELD_PREP(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, value);
+	FIELD_MODIFY(ZL_DPLL_MEAS_CTRL_AVG_FACTOR, &dpll_meas_ctrl, value);
 	rc = zl3073x_write_u8(zldev, ZL_REG_DPLL_MEAS_CTRL, dpll_meas_ctrl);
 	if (rc)
 		return rc;
diff --git a/drivers/dpll/zl3073x/flash.c b/drivers/dpll/zl3073x/flash.c
index 83452a77e3e98..f85535c8ad246 100644
--- a/drivers/dpll/zl3073x/flash.c
+++ b/drivers/dpll/zl3073x/flash.c
@@ -194,8 +194,7 @@ zl3073x_flash_cmd_wait(struct zl3073x_dev *zldev, u32 operation,
 	if (rc)
 		return rc;
 
-	value &= ~ZL_WRITE_FLASH_OP;
-	value |= FIELD_PREP(ZL_WRITE_FLASH_OP, operation);
+	FIELD_MODIFY(ZL_WRITE_FLASH_OP, &value, operation);
 
 	rc = zl3073x_write_u8(zldev, ZL_REG_WRITE_FLASH, value);
 	if (rc)
-- 
2.52.0


^ permalink raw reply related

* [PATCH net-next v2 3/5] dpll: zl3073x: add ref sync and output clock type helpers
From: Ivan Vecera @ 2026-03-28  8:06 UTC (permalink / raw)
  To: netdev
  Cc: Petr Oros, Prathosh Satish, Arkadiusz Kubalewski, Jiri Pirko,
	Michal Schmidt, Simon Horman, Vadim Fedorenko, linux-kernel,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree,
	Pasi Vaananen
In-Reply-To: <20260328080624.593916-1-ivecera@redhat.com>

Add ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR and ZL_REF_SYNC_CTRL_PAIR
register definitions.

Add inline helpers to get and set the sync control mode and sync pair
fields of the reference sync control register:

  zl3073x_ref_sync_mode_get/set() - ZL_REF_SYNC_CTRL_MODE field
  zl3073x_ref_sync_pair_get/set() - ZL_REF_SYNC_CTRL_PAIR field

Add inline helpers to get and set the clock type field of the output
mode register:

  zl3073x_out_clock_type_get/set() - ZL_OUTPUT_MODE_CLOCK_TYPE field

Convert existing esync callbacks to use the new helpers.

Reviewed-by: Petr Oros <poros@redhat.com>
Reviewed-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 24 ++++++++-----------
 drivers/dpll/zl3073x/out.h  | 22 ++++++++++++++++++
 drivers/dpll/zl3073x/ref.h  | 46 +++++++++++++++++++++++++++++++++++++
 drivers/dpll/zl3073x/regs.h |  2 ++
 4 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index 2476c14028210..b5b411425f6b3 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -137,7 +137,7 @@ zl3073x_dpll_input_pin_esync_get(const struct dpll_pin *dpll_pin,
 	esync->range = esync_freq_ranges;
 	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
 
-	switch (FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl)) {
+	switch (zl3073x_ref_sync_mode_get(ref)) {
 	case ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75:
 		esync->freq = ref->esync_n_div == ZL_REF_ESYNC_DIV_1HZ ? 1 : 0;
 		esync->pulse = 25;
@@ -173,8 +173,7 @@ zl3073x_dpll_input_pin_esync_set(const struct dpll_pin *dpll_pin,
 	else
 		sync_mode = ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75;
 
-	ref.sync_ctrl &= ~ZL_REF_SYNC_CTRL_MODE;
-	ref.sync_ctrl |= FIELD_PREP(ZL_REF_SYNC_CTRL_MODE, sync_mode);
+	zl3073x_ref_sync_mode_set(&ref, sync_mode);
 
 	if (freq) {
 		/* 1 Hz is only supported frequency now */
@@ -578,7 +577,7 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
 	const struct zl3073x_synth *synth;
 	const struct zl3073x_out *out;
 	u32 synth_freq, out_freq;
-	u8 clock_type, out_id;
+	u8 out_id;
 
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = zl3073x_out_state_get(zldev, out_id);
@@ -601,8 +600,7 @@ zl3073x_dpll_output_pin_esync_get(const struct dpll_pin *dpll_pin,
 	esync->range = esync_freq_ranges;
 	esync->range_num = ARRAY_SIZE(esync_freq_ranges);
 
-	clock_type = FIELD_GET(ZL_OUTPUT_MODE_CLOCK_TYPE, out->mode);
-	if (clock_type != ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC) {
+	if (zl3073x_out_clock_type_get(out) != ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC) {
 		/* No need to read esync data if it is not enabled */
 		esync->freq = 0;
 		esync->pulse = 0;
@@ -635,8 +633,8 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 	struct zl3073x_dpll_pin *pin = pin_priv;
 	const struct zl3073x_synth *synth;
 	struct zl3073x_out out;
-	u8 clock_type, out_id;
 	u32 synth_freq;
+	u8 out_id;
 
 	out_id = zl3073x_output_pin_out_get(pin->id);
 	out = *zl3073x_out_state_get(zldev, out_id);
@@ -648,15 +646,13 @@ zl3073x_dpll_output_pin_esync_set(const struct dpll_pin *dpll_pin,
 	if (zl3073x_out_is_ndiv(&out))
 		return -EOPNOTSUPP;
 
-	/* Select clock type */
+	/* Update clock type in output mode */
 	if (freq)
-		clock_type = ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC;
+		zl3073x_out_clock_type_set(&out,
+					   ZL_OUTPUT_MODE_CLOCK_TYPE_ESYNC);
 	else
-		clock_type = ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL;
-
-	/* Update clock type in output mode */
-	out.mode &= ~ZL_OUTPUT_MODE_CLOCK_TYPE;
-	out.mode |= FIELD_PREP(ZL_OUTPUT_MODE_CLOCK_TYPE, clock_type);
+		zl3073x_out_clock_type_set(&out,
+					   ZL_OUTPUT_MODE_CLOCK_TYPE_NORMAL);
 
 	/* If esync is being disabled just write mailbox and finish */
 	if (!freq)
diff --git a/drivers/dpll/zl3073x/out.h b/drivers/dpll/zl3073x/out.h
index edf40432bba5f..660889c57bffa 100644
--- a/drivers/dpll/zl3073x/out.h
+++ b/drivers/dpll/zl3073x/out.h
@@ -42,6 +42,28 @@ const struct zl3073x_out *zl3073x_out_state_get(struct zl3073x_dev *zldev,
 int zl3073x_out_state_set(struct zl3073x_dev *zldev, u8 index,
 			  const struct zl3073x_out *out);
 
+/**
+ * zl3073x_out_clock_type_get - get output clock type
+ * @out: pointer to out state
+ *
+ * Return: clock type of given output (ZL_OUTPUT_MODE_CLOCK_TYPE_*)
+ */
+static inline u8 zl3073x_out_clock_type_get(const struct zl3073x_out *out)
+{
+	return FIELD_GET(ZL_OUTPUT_MODE_CLOCK_TYPE, out->mode);
+}
+
+/**
+ * zl3073x_out_clock_type_set - set output clock type
+ * @out: pointer to out state
+ * @type: clock type (ZL_OUTPUT_MODE_CLOCK_TYPE_*)
+ */
+static inline void
+zl3073x_out_clock_type_set(struct zl3073x_out *out, u8 type)
+{
+	FIELD_MODIFY(ZL_OUTPUT_MODE_CLOCK_TYPE, &out->mode, type);
+}
+
 /**
  * zl3073x_out_signal_format_get - get output signal format
  * @out: pointer to out state
diff --git a/drivers/dpll/zl3073x/ref.h b/drivers/dpll/zl3073x/ref.h
index 06d8d4d97ea26..09fab97a71d7e 100644
--- a/drivers/dpll/zl3073x/ref.h
+++ b/drivers/dpll/zl3073x/ref.h
@@ -106,6 +106,52 @@ zl3073x_ref_freq_set(struct zl3073x_ref *ref, u32 freq)
 	return 0;
 }
 
+/**
+ * zl3073x_ref_sync_mode_get - get sync control mode
+ * @ref: pointer to ref state
+ *
+ * Return: sync control mode (ZL_REF_SYNC_CTRL_MODE_*)
+ */
+static inline u8
+zl3073x_ref_sync_mode_get(const struct zl3073x_ref *ref)
+{
+	return FIELD_GET(ZL_REF_SYNC_CTRL_MODE, ref->sync_ctrl);
+}
+
+/**
+ * zl3073x_ref_sync_mode_set - set sync control mode
+ * @ref: pointer to ref state
+ * @mode: sync control mode (ZL_REF_SYNC_CTRL_MODE_*)
+ */
+static inline void
+zl3073x_ref_sync_mode_set(struct zl3073x_ref *ref, u8 mode)
+{
+	FIELD_MODIFY(ZL_REF_SYNC_CTRL_MODE, &ref->sync_ctrl, mode);
+}
+
+/**
+ * zl3073x_ref_sync_pair_get - get sync pair reference index
+ * @ref: pointer to ref state
+ *
+ * Return: paired reference index
+ */
+static inline u8
+zl3073x_ref_sync_pair_get(const struct zl3073x_ref *ref)
+{
+	return FIELD_GET(ZL_REF_SYNC_CTRL_PAIR, ref->sync_ctrl);
+}
+
+/**
+ * zl3073x_ref_sync_pair_set - set sync pair reference index
+ * @ref: pointer to ref state
+ * @pair: paired reference index
+ */
+static inline void
+zl3073x_ref_sync_pair_set(struct zl3073x_ref *ref, u8 pair)
+{
+	FIELD_MODIFY(ZL_REF_SYNC_CTRL_PAIR, &ref->sync_ctrl, pair);
+}
+
 /**
  * zl3073x_ref_is_diff - check if the given input reference is differential
  * @ref: pointer to ref state
diff --git a/drivers/dpll/zl3073x/regs.h b/drivers/dpll/zl3073x/regs.h
index 5ae50cb761a97..d425dc67250fe 100644
--- a/drivers/dpll/zl3073x/regs.h
+++ b/drivers/dpll/zl3073x/regs.h
@@ -213,7 +213,9 @@
 #define ZL_REG_REF_SYNC_CTRL			ZL_REG(10, 0x2e, 1)
 #define ZL_REF_SYNC_CTRL_MODE			GENMASK(2, 0)
 #define ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR_OFF	0
+#define ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR	1
 #define ZL_REF_SYNC_CTRL_MODE_50_50_ESYNC_25_75	2
+#define ZL_REF_SYNC_CTRL_PAIR			GENMASK(7, 4)
 
 #define ZL_REG_REF_ESYNC_DIV			ZL_REG(10, 0x30, 4)
 #define ZL_REF_ESYNC_DIV_1HZ			0
-- 
2.52.0


^ permalink raw reply related

* [PATCH net-next v2 4/5] dt-bindings: dpll: add ref-sync-sources property
From: Ivan Vecera @ 2026-03-28  8:06 UTC (permalink / raw)
  To: netdev
  Cc: Petr Oros, Prathosh Satish, Arkadiusz Kubalewski, Jiri Pirko,
	Michal Schmidt, Simon Horman, Vadim Fedorenko, linux-kernel,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree,
	Pasi Vaananen
In-Reply-To: <20260328080624.593916-1-ivecera@redhat.com>

Add ref-sync-sources phandle-array property to the dpll-pin schema
allowing board designers to declare which input pins can serve as
sync sources in a Reference-Sync pair.  A Ref-Sync pair consists of
a clock reference and a low-frequency sync signal where the DPLL locks
to the clock but phase-aligns to the sync reference.

Update both examples in the Microchip ZL3073x binding to demonstrate
the new property with a 1 PPS sync source paired to a clock source.

Reviewed-by: Petr Oros <poros@redhat.com>
Reviewed-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 .../devicetree/bindings/dpll/dpll-pin.yaml    | 11 +++++++
 .../bindings/dpll/microchip,zl30731.yaml      | 30 ++++++++++++++-----
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
index 51db93b77306f..7084f102e274c 100644
--- a/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
+++ b/Documentation/devicetree/bindings/dpll/dpll-pin.yaml
@@ -36,6 +36,17 @@ properties:
     description: String exposed as the pin board label
     $ref: /schemas/types.yaml#/definitions/string
 
+  ref-sync-sources:
+    description: |
+      List of phandles to input pins that can serve as the sync source
+      in a Reference-Sync pair with this pin acting as the clock source.
+      A Ref-Sync pair consists of a clock reference and a low-frequency
+      sync signal.  The DPLL locks to the clock reference but
+      phase-aligns to the sync reference.
+      Only valid for input pins.  Each referenced pin must be a
+      different input pin on the same device.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+
   supported-frequencies-hz:
     description: List of supported frequencies for this pin, expressed in Hz.
 
diff --git a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
index 17747f754b845..fa5a8f8e390cd 100644
--- a/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
+++ b/Documentation/devicetree/bindings/dpll/microchip,zl30731.yaml
@@ -52,11 +52,19 @@ examples:
           #address-cells = <1>;
           #size-cells = <0>;
 
-          pin@0 { /* REF0P */
+          sync0: pin@0 { /* REF0P - 1 PPS sync source */
             reg = <0>;
             connection-type = "ext";
-            label = "Input 0";
-            supported-frequencies-hz = /bits/ 64 <1 1000>;
+            label = "SMA1";
+            supported-frequencies-hz = /bits/ 64 <1>;
+          };
+
+          pin@1 { /* REF0N - clock source, can pair with sync0 */
+            reg = <1>;
+            connection-type = "ext";
+            label = "SMA2";
+            supported-frequencies-hz = /bits/ 64 <10000 10000000>;
+            ref-sync-sources = <&sync0>;
           };
         };
 
@@ -90,11 +98,19 @@ examples:
           #address-cells = <1>;
           #size-cells = <0>;
 
-          pin@0 { /* REF0P */
+          sync1: pin@0 { /* REF0P - 1 PPS sync source */
             reg = <0>;
-            connection-type = "ext";
-            label = "Input 0";
-            supported-frequencies-hz = /bits/ 64 <1 1000>;
+            connection-type = "gnss";
+            label = "GNSS_1PPS_IN";
+            supported-frequencies-hz = /bits/ 64 <1>;
+          };
+
+          pin@1 { /* REF0N - clock source */
+            reg = <1>;
+            connection-type = "gnss";
+            label = "GNSS_10M_IN";
+            supported-frequencies-hz = /bits/ 64 <10000000>;
+            ref-sync-sources = <&sync1>;
           };
         };
 
-- 
2.52.0


^ permalink raw reply related

* [PATCH net-next v2 5/5] dpll: zl3073x: add ref-sync pair support
From: Ivan Vecera @ 2026-03-28  8:06 UTC (permalink / raw)
  To: netdev
  Cc: Petr Oros, Prathosh Satish, Arkadiusz Kubalewski, Jiri Pirko,
	Michal Schmidt, Simon Horman, Vadim Fedorenko, linux-kernel,
	Conor Dooley, Krzysztof Kozlowski, Rob Herring, devicetree,
	Pasi Vaananen
In-Reply-To: <20260328080624.593916-1-ivecera@redhat.com>

Add support for ref-sync pair registration using the 'ref-sync-sources'
phandle property from device tree. A ref-sync pair consists of a clock
reference and a low-frequency sync signal where the DPLL locks to the
clock reference but phase-aligns to the sync reference.

The implementation:
- Stores fwnode handle in zl3073x_dpll_pin during pin registration
- Adds ref_sync_get/set callbacks to read and write the sync control
  mode and pair registers
- Validates ref-sync frequency constraints: sync signal must be 8 kHz
  or less, clock reference must be 1 kHz or more and higher than sync
- Excludes sync source from automatic reference selection by setting
  its priority to NONE on connect; on disconnect the priority is left
  as NONE and the user must explicitly make the pin selectable again
- Iterates ref-sync-sources phandles to register declared pairings
  via dpll_pin_ref_sync_pair_add()

Reviewed-by: Petr Oros <poros@redhat.com>
Reviewed-by: Prathosh Satish <Prathosh.Satish@microchip.com>
Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/dpll/zl3073x/dpll.c | 207 +++++++++++++++++++++++++++++++++++-
 1 file changed, 206 insertions(+), 1 deletion(-)

diff --git a/drivers/dpll/zl3073x/dpll.c b/drivers/dpll/zl3073x/dpll.c
index b5b411425f6b3..79589400f0cc2 100644
--- a/drivers/dpll/zl3073x/dpll.c
+++ b/drivers/dpll/zl3073x/dpll.c
@@ -13,6 +13,7 @@
 #include <linux/module.h>
 #include <linux/netlink.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/sprintf.h>
 
@@ -30,6 +31,7 @@
  * @dpll: DPLL the pin is registered to
  * @dpll_pin: pointer to registered dpll_pin
  * @tracker: tracking object for the acquired reference
+ * @fwnode: firmware node handle
  * @label: package label
  * @dir: pin direction
  * @id: pin id
@@ -45,6 +47,7 @@ struct zl3073x_dpll_pin {
 	struct zl3073x_dpll	*dpll;
 	struct dpll_pin		*dpll_pin;
 	dpll_tracker		tracker;
+	struct fwnode_handle	*fwnode;
 	char			label[8];
 	enum dpll_pin_direction	dir;
 	u8			id;
@@ -184,6 +187,109 @@ zl3073x_dpll_input_pin_esync_set(const struct dpll_pin *dpll_pin,
 	return zl3073x_ref_state_set(zldev, ref_id, &ref);
 }
 
+static int
+zl3073x_dpll_input_pin_ref_sync_get(const struct dpll_pin *dpll_pin,
+				    void *pin_priv,
+				    const struct dpll_pin *ref_sync_pin,
+				    void *ref_sync_pin_priv,
+				    enum dpll_pin_state *state,
+				    struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
+	struct zl3073x_dpll_pin *pin = pin_priv;
+	struct zl3073x_dpll *zldpll = pin->dpll;
+	struct zl3073x_dev *zldev = zldpll->dev;
+	const struct zl3073x_ref *ref;
+	u8 ref_id, mode, pair;
+
+	ref_id = zl3073x_input_pin_ref_get(pin->id);
+	ref = zl3073x_ref_state_get(zldev, ref_id);
+	mode = zl3073x_ref_sync_mode_get(ref);
+	pair = zl3073x_ref_sync_pair_get(ref);
+
+	if (mode == ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR &&
+	    pair == zl3073x_input_pin_ref_get(sync_pin->id))
+		*state = DPLL_PIN_STATE_CONNECTED;
+	else
+		*state = DPLL_PIN_STATE_DISCONNECTED;
+
+	return 0;
+}
+
+static int
+zl3073x_dpll_input_pin_ref_sync_set(const struct dpll_pin *dpll_pin,
+				    void *pin_priv,
+				    const struct dpll_pin *ref_sync_pin,
+				    void *ref_sync_pin_priv,
+				    const enum dpll_pin_state state,
+				    struct netlink_ext_ack *extack)
+{
+	struct zl3073x_dpll_pin *sync_pin = ref_sync_pin_priv;
+	struct zl3073x_dpll_pin *pin = pin_priv;
+	struct zl3073x_dpll *zldpll = pin->dpll;
+	struct zl3073x_dev *zldev = zldpll->dev;
+	u8 mode, ref_id, sync_ref_id;
+	struct zl3073x_chan chan;
+	struct zl3073x_ref ref;
+	int rc;
+
+	ref_id = zl3073x_input_pin_ref_get(pin->id);
+	sync_ref_id = zl3073x_input_pin_ref_get(sync_pin->id);
+	ref = *zl3073x_ref_state_get(zldev, ref_id);
+
+	if (state == DPLL_PIN_STATE_CONNECTED) {
+		const struct zl3073x_ref *sync_ref;
+		u32 ref_freq, sync_freq;
+
+		sync_ref = zl3073x_ref_state_get(zldev, sync_ref_id);
+		ref_freq = zl3073x_ref_freq_get(&ref);
+		sync_freq = zl3073x_ref_freq_get(sync_ref);
+
+		/* Sync signal must be 8 kHz or less and clock reference
+		 * must be 1 kHz or more and higher than the sync signal.
+		 */
+		if (sync_freq > 8000) {
+			NL_SET_ERR_MSG(extack,
+				       "sync frequency must be 8 kHz or less");
+			return -EINVAL;
+		}
+		if (ref_freq < 1000) {
+			NL_SET_ERR_MSG(extack,
+				       "clock frequency must be 1 kHz or more");
+			return -EINVAL;
+		}
+		if (ref_freq <= sync_freq) {
+			NL_SET_ERR_MSG(extack,
+				       "clock frequency must be higher than sync frequency");
+			return -EINVAL;
+		}
+
+		zl3073x_ref_sync_pair_set(&ref, sync_ref_id);
+		mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR;
+	} else {
+		mode = ZL_REF_SYNC_CTRL_MODE_REFSYNC_PAIR_OFF;
+	}
+
+	zl3073x_ref_sync_mode_set(&ref, mode);
+
+	rc = zl3073x_ref_state_set(zldev, ref_id, &ref);
+	if (rc)
+		return rc;
+
+	/* Exclude sync source from automatic reference selection by setting
+	 * its priority to NONE. On disconnect the priority is left as NONE
+	 * and the user must explicitly make the pin selectable again.
+	 */
+	if (state == DPLL_PIN_STATE_CONNECTED) {
+		chan = *zl3073x_chan_state_get(zldev, zldpll->id);
+		zl3073x_chan_ref_prio_set(&chan, sync_ref_id,
+					  ZL_DPLL_REF_PRIO_NONE);
+		return zl3073x_chan_state_set(zldev, zldpll->id, &chan);
+	}
+
+	return 0;
+}
+
 static int
 zl3073x_dpll_input_pin_ffo_get(const struct dpll_pin *dpll_pin, void *pin_priv,
 			       const struct dpll_device *dpll, void *dpll_priv,
@@ -1100,6 +1206,8 @@ static const struct dpll_pin_ops zl3073x_dpll_input_pin_ops = {
 	.phase_adjust_set = zl3073x_dpll_input_pin_phase_adjust_set,
 	.prio_get = zl3073x_dpll_input_pin_prio_get,
 	.prio_set = zl3073x_dpll_input_pin_prio_set,
+	.ref_sync_get = zl3073x_dpll_input_pin_ref_sync_get,
+	.ref_sync_set = zl3073x_dpll_input_pin_ref_sync_set,
 	.state_on_dpll_get = zl3073x_dpll_input_pin_state_on_dpll_get,
 	.state_on_dpll_set = zl3073x_dpll_input_pin_state_on_dpll_set,
 };
@@ -1190,8 +1298,11 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
 	if (IS_ERR(props))
 		return PTR_ERR(props);
 
-	/* Save package label, esync capability and phase adjust granularity */
+	/* Save package label, fwnode, esync capability and phase adjust
+	 * granularity.
+	 */
 	strscpy(pin->label, props->package_label);
+	pin->fwnode = fwnode_handle_get(props->fwnode);
 	pin->esync_control = props->esync_control;
 	pin->phase_gran = props->dpll_props.phase_gran;
 
@@ -1236,6 +1347,8 @@ zl3073x_dpll_pin_register(struct zl3073x_dpll_pin *pin, u32 index)
 	dpll_pin_put(pin->dpll_pin, &pin->tracker);
 	pin->dpll_pin = NULL;
 err_pin_get:
+	fwnode_handle_put(pin->fwnode);
+	pin->fwnode = NULL;
 	zl3073x_pin_props_put(props);
 
 	return rc;
@@ -1265,6 +1378,9 @@ zl3073x_dpll_pin_unregister(struct zl3073x_dpll_pin *pin)
 
 	dpll_pin_put(pin->dpll_pin, &pin->tracker);
 	pin->dpll_pin = NULL;
+
+	fwnode_handle_put(pin->fwnode);
+	pin->fwnode = NULL;
 }
 
 /**
@@ -1735,6 +1851,88 @@ zl3073x_dpll_free(struct zl3073x_dpll *zldpll)
 	kfree(zldpll);
 }
 
+/**
+ * zl3073x_dpll_ref_sync_pair_register - register ref_sync pairs for a pin
+ * @pin: pointer to zl3073x_dpll_pin structure
+ *
+ * Iterates 'ref-sync-sources' phandles in the pin's firmware node and
+ * registers each declared pairing.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_dpll_ref_sync_pair_register(struct zl3073x_dpll_pin *pin)
+{
+	struct zl3073x_dev *zldev = pin->dpll->dev;
+	struct fwnode_handle *fwnode;
+	struct dpll_pin *sync_pin;
+	dpll_tracker tracker;
+	int n, rc;
+
+	for (n = 0; ; n++) {
+		/* Get n'th ref-sync source */
+		fwnode = fwnode_find_reference(pin->fwnode, "ref-sync-sources",
+					       n);
+		if (IS_ERR(fwnode)) {
+			rc = PTR_ERR(fwnode);
+			break;
+		}
+
+		/* Find associated dpll pin */
+		sync_pin = fwnode_dpll_pin_find(fwnode, &tracker);
+		fwnode_handle_put(fwnode);
+		if (!sync_pin) {
+			dev_warn(zldev->dev, "%s: ref-sync source %d not found",
+				 pin->label, n);
+			continue;
+		}
+
+		/* Register new ref-sync pair */
+		rc = dpll_pin_ref_sync_pair_add(pin->dpll_pin, sync_pin);
+		dpll_pin_put(sync_pin, &tracker);
+
+		/* -EBUSY means pairing already exists from another DPLL's
+		 * registration.
+		 */
+		if (rc && rc != -EBUSY) {
+			dev_err(zldev->dev,
+				"%s: failed to add ref-sync source %d: %pe",
+				pin->label, n, ERR_PTR(rc));
+			break;
+		}
+	}
+
+	return rc != -ENOENT ? rc : 0;
+}
+
+/**
+ * zl3073x_dpll_ref_sync_pairs_register - register ref_sync pairs for a DPLL
+ * @zldpll: pointer to zl3073x_dpll structure
+ *
+ * Iterates all registered input pins of the given DPLL and establishes
+ * ref_sync pairings declared by 'ref-sync-sources' phandles in the
+ * device tree.
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int
+zl3073x_dpll_ref_sync_pairs_register(struct zl3073x_dpll *zldpll)
+{
+	struct zl3073x_dpll_pin *pin;
+	int rc;
+
+	list_for_each_entry(pin, &zldpll->pins, list) {
+		if (!zl3073x_dpll_is_input_pin(pin) || !pin->fwnode)
+			continue;
+
+		rc = zl3073x_dpll_ref_sync_pair_register(pin);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
 /**
  * zl3073x_dpll_register - register DPLL device and all its pins
  * @zldpll: pointer to zl3073x_dpll structure
@@ -1758,6 +1956,13 @@ zl3073x_dpll_register(struct zl3073x_dpll *zldpll)
 		return rc;
 	}
 
+	rc = zl3073x_dpll_ref_sync_pairs_register(zldpll);
+	if (rc) {
+		zl3073x_dpll_pins_unregister(zldpll);
+		zl3073x_dpll_device_unregister(zldpll);
+		return rc;
+	}
+
 	return 0;
 }
 
-- 
2.52.0


^ permalink raw reply related

* Re: [PATCH v3] net: stmmac: skip VLAN restore when VLAN hash ops are missing
From: Michal Piekos @ 2026-03-28  8:17 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Ovidiu Panait,
	netdev, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <aceIeFWGAZwr1njk@shell.armlinux.org.uk>

On Sat, Mar 28, 2026 at 07:51:20AM +0000, Russell King (Oracle) wrote:
> On Sat, Mar 28, 2026 at 07:51:53AM +0100, Michal Piekos wrote:
> > stmmac_vlan_restore() unconditionally calls stmmac_vlan_update() when
> > NETIF_F_VLAN_FEATURES is set. On platforms where priv->hw->vlan (or
> > ->update_vlan_hash) is not provided, stmmac_update_vlan_hash() returns
> > -EINVAL via stmmac_do_void_callback(), resulting in a spurious
> > "Failed to restore VLANs" error even when no VLAN filtering is in use.
> > 
> > Remove the unneeded comment.
> > 
> > Tested on Orange Pi Zero 3.
> > 
> > Fixes: bd7ad51253a7 ("net: stmmac: Fix VLAN HW state restore")
> > Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
> > ---
> > This patch fixes a noisy "Failed to restore VLANs" message on platforms
> > where stmmac VLAN hash ops are not implemented.
> > stmmac_vlan_restore() calls stmmac_vlan_update() without checking for
> > VLAN hash ops presence which results in -EINVAL. 
> > ---
> > Changes in v3:
> > - Remove the offending comment
> > - Restore the original check for NETIF_F_VLAN_FEATURES
> > - Link to v2: https://lore.kernel.org/r/20260321-vlan-restore-error-v2-1-45cf56a5223d@mmpsystems.pl
> > 
> > Changes in v2:
> > - Replace check for hash ops with check for HW FILTER flags
> > - Link to v1: https://lore.kernel.org/r/20260314-vlan-restore-error-v1-1-4fc6c3e2115f@mmpsystems.pl
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > index 6827c99bde8c..0f3e5ac05faa 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > @@ -6861,19 +6861,13 @@ static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vi
> >  
> >  static int stmmac_vlan_restore(struct stmmac_priv *priv)
> >  {
> > -	int ret;
> > -
> >  	if (!(priv->dev->features & NETIF_F_VLAN_FEATURES))
> >  		return 0;
> >  
> >  	if (priv->hw->num_vlan)
> >  		stmmac_restore_hw_vlan_rx_fltr(priv, priv->dev, priv->hw);
> >  
> > -	ret = stmmac_vlan_update(priv, priv->num_double_vlans);
> > -	if (ret)
> > -		netdev_err(priv->dev, "Failed to restore VLANs\n");
> > -
> > -	return ret;
> > +	return stmmac_vlan_update(priv, priv->num_double_vlans);
> >  }
> 
> Yes, but as both Andrew and myself have pointed out, no one checks
> the return value of stmmac_vlan_restore(), so why does it return
> int?

Was too quick with the patch. Will spin another version.

Michal
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* Re: [PATCH net-next 2/2] net: stmmac: simplify GSO/TSO test in stmmac_xmit()
From: Russell King (Oracle) @ 2026-03-28  8:24 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, linux-arm-kernel, linux-stm32, netdev,
	Paolo Abeni
In-Reply-To: <E1w63fl-0000000E3Vb-3JkY@rmk-PC.armlinux.org.uk>

On Fri, Mar 27, 2026 at 09:40:09AM +0000, Russell King (Oracle) wrote:
> The test in stmmac_xmit() to see whether we should pass the skbuff to
> stmmac_tso_xmit() is more complex than it needs to be. This test can
> be simplified by storing the mask of GSO types that we will pass, and
> setting it according to the enabled features.
> 
> Note that "tso" is a mis-nomer since commit b776620651a1 ("net:
> stmmac: Implement UDP Segmentation Offload"). Also note that this
> commit controls both via the TSO feature. We preserve this behaviour
> in this commit.
> 
> Also, this commit unconditionally accessed skb_shinfo(skb)->gso_type
> for all frames, even when skb_is_gso() was false. This access is
> eliminated.
> 
> Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

AI review of this patch regurgitates Jakub's point that was discussed.

> @@ -3700,7 +3700,7 @@ static int stmmac_hw_setup(struct net_device *dev)
>  	stmmac_set_rings_length(priv);
>  
>  	/* Enable TSO */
> -	if (priv->tso) {
> +	if (priv->gso_enabled_types) {
>  		for (chan = 0; chan < tx_cnt; chan++) {
>  			struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[chan];
>  

...

> @@ -7828,7 +7834,7 @@ static int __stmmac_dvr_probe(struct device *device,
>  		ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
>  		if (priv->plat->core_type == DWMAC_CORE_GMAC4)
>  			ndev->hw_features |= NETIF_F_GSO_UDP_L4;
> -		priv->tso = true;
> +		stmmac_set_gso_types(priv, true);

Clearly, the issue it is regurgitating has been there for a long time
and isn't a new issue introduced by this patch.

AI needs to stop doing this, because it is encouraging multiple changes
in a single patch, which is against the normal kernel process.

As already pointed out, there are multiple issues with stmmac TSO
support, particularly with glue drivers that enable TSO on some
queues/channels and not others, since netdev core TSO support is
global across all channels.

So, won't the AI response in this patch - it's just another pre-
existing issue that needs fixing in a separate patch.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* [PATCH iproute2-next] dpll: Send object per event in JSON monitor mode
From: Vitaly Grinberg @ 2026-03-28  8:45 UTC (permalink / raw)
  To: netdev; +Cc: stephen

Previously, monitor mode wrapped all events in a single JSON array
inside a top-level object, which made piping the output to external
tools (such as `jq`) impossible.
Send a separate JSON object for each event in monitor mode,
making the output suitable for line-by-line consumers. Skip the
global JSON wrapper for monitor mode.

Signed-off-by: Vitaly Grinberg <vgrinber@redhat.com>
---
 dpll/dpll.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/dpll/dpll.c b/dpll/dpll.c
index 7b05bf86..6736c85e 100644
--- a/dpll/dpll.c
+++ b/dpll/dpll.c
@@ -571,8 +571,15 @@ int main(int argc, char **argv)
  argc -= optind;
  argv += optind;

- new_json_obj_plain(json);
- open_json_object(NULL);
+ /* Monitor emits one JSON object per event for streaming;
+ * other commands use a single JSON wrapper object.
+ */
+ bool is_monitor = argc > 0 && strcmp(argv[0], "monitor") == 0;
+
+ if (!is_monitor) {
+ new_json_obj_plain(json);
+ open_json_object(NULL);
+ }

  /* Skip netlink init for help commands */
  bool need_nl = true;
@@ -602,8 +609,10 @@ dpll_fini:
  if (need_nl)
  dpll_fini(dpll);
 json_cleanup:
- close_json_object();
- delete_json_obj_plain();
+ if (!is_monitor) {
+ close_json_object();
+ delete_json_obj_plain();
+ }
 dpll_free:
  dpll_free(dpll);
  return ret;
@@ -1840,6 +1849,7 @@ static int cmd_monitor_cb(const struct nlmsghdr
*nlh, void *data)

  mnl_attr_parse(nlh, sizeof(struct genlmsghdr), attr_cb, tb);

+ new_json_obj_plain(json);
  open_json_object(NULL);
  print_string(PRINT_JSON, "name", NULL, json_name);
  open_json_object("msg");
@@ -1849,6 +1859,7 @@ static int cmd_monitor_cb(const struct nlmsghdr
*nlh, void *data)

  close_json_object();
  close_json_object();
+ delete_json_obj_plain();
  break;
  }
  case DPLL_CMD_PIN_CREATE_NTF:
@@ -1867,6 +1878,7 @@ static int cmd_monitor_cb(const struct nlmsghdr
*nlh, void *data)
  json_name = "pin-delete-ntf";
  }

+ new_json_obj_plain(json);
  open_json_object(NULL);
  print_string(PRINT_JSON, "name", NULL, json_name);
  open_json_object("msg");
@@ -1876,6 +1888,7 @@ static int cmd_monitor_cb(const struct nlmsghdr
*nlh, void *data)

  close_json_object();
  close_json_object();
+ delete_json_obj_plain();
  break;
  }
  default:
@@ -1941,8 +1954,6 @@ static int cmd_monitor(struct dpll *dpll)
  goto err_signalfd;
  }

- open_json_array(PRINT_JSON, "monitor");
-
  pfds[0].fd = signal_fd;
  pfds[0].events = POLLIN;
  pfds[1].fd = netlink_fd;
@@ -1975,8 +1986,6 @@ static int cmd_monitor(struct dpll *dpll)
  }
  }

- close_json_array(PRINT_JSON, NULL);
-
 err_signalfd:
  if (signal_fd >= 0)
  close(signal_fd);
-- 
2.52.0


^ permalink raw reply related

* [PATCH v4] net: stmmac: skip VLAN restore when VLAN hash ops are missing
From: Michal Piekos @ 2026-03-28  8:55 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue, Ovidiu Panait
  Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
	Michal Piekos

stmmac_vlan_restore() unconditionally calls stmmac_vlan_update() when
NETIF_F_VLAN_FEATURES is set. On platforms where priv->hw->vlan (or
->update_vlan_hash) is not provided, stmmac_update_vlan_hash() returns
-EINVAL via stmmac_do_void_callback(), resulting in a spurious
"Failed to restore VLANs" error even when no VLAN filtering is in use.

Remove not needed comment.
Remove not used return value from stmmac_vlan_restore().

Tested on Orange Pi Zero 3.

Fixes: bd7ad51253a7 ("net: stmmac: Fix VLAN HW state restore")
Signed-off-by: Michal Piekos <michal.piekos@mmpsystems.pl>
---
This patch fixes a noisy "Failed to restore VLANs" message on platforms
where stmmac VLAN hash ops are not implemented.
stmmac_vlan_restore() calls stmmac_vlan_update() without checking for
VLAN hash ops presence which results in -EINVAL. 
---
Changes in v4:
- Remove not used return value of stmmac_vlan_restore()
- Link to v3: https://lore.kernel.org/r/20260328-vlan-restore-error-v3-1-df47a039c6f6@mmpsystems.pl

Changes in v3:
- Remove the offending comment
- Restore the original check for NETIF_F_VLAN_FEATURES
- Link to v2: https://lore.kernel.org/r/20260321-vlan-restore-error-v2-1-45cf56a5223d@mmpsystems.pl

Changes in v2:
- Replace check for hash ops with check for HW FILTER flags
- Link to v1: https://lore.kernel.org/r/20260314-vlan-restore-error-v1-1-4fc6c3e2115f@mmpsystems.pl
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6827c99bde8c..13d3cac056be 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -156,7 +156,7 @@ static void stmmac_tx_timer_arm(struct stmmac_priv *priv, u32 queue);
 static void stmmac_flush_tx_descriptors(struct stmmac_priv *priv, int queue);
 static void stmmac_set_dma_operation_mode(struct stmmac_priv *priv, u32 txmode,
 					  u32 rxmode, u32 chan);
-static int stmmac_vlan_restore(struct stmmac_priv *priv);
+static void stmmac_vlan_restore(struct stmmac_priv *priv);
 
 #ifdef CONFIG_DEBUG_FS
 static const struct net_device_ops stmmac_netdev_ops;
@@ -6859,21 +6859,15 @@ static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vi
 	return ret;
 }
 
-static int stmmac_vlan_restore(struct stmmac_priv *priv)
+static void stmmac_vlan_restore(struct stmmac_priv *priv)
 {
-	int ret;
-
 	if (!(priv->dev->features & NETIF_F_VLAN_FEATURES))
-		return 0;
+		return;
 
 	if (priv->hw->num_vlan)
 		stmmac_restore_hw_vlan_rx_fltr(priv, priv->dev, priv->hw);
 
-	ret = stmmac_vlan_update(priv, priv->num_double_vlans);
-	if (ret)
-		netdev_err(priv->dev, "Failed to restore VLANs\n");
-
-	return ret;
+	stmmac_vlan_update(priv, priv->num_double_vlans);
 }
 
 static int stmmac_bpf(struct net_device *dev, struct netdev_bpf *bpf)

---
base-commit: be762d8b6dd7efacb61937d20f8475db8f207655
change-id: 20260314-vlan-restore-error-f8b3a1c7f50a

Best regards,
-- 
Michal Piekos <michal.piekos@mmpsystems.pl>


^ permalink raw reply related

* [PATCH] net/ftgmac100: fix ring allocation unwind leaks on open failure
From: Yufan Chen @ 2026-03-28  9:24 UTC (permalink / raw)
  To: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel
  Cc: yufan.chen, Yufan Chen

From: Yufan Chen <ericterminal@gmail.com>

ftgmac100_alloc_rings() allocated rx_skbs, tx_skbs, rxdes, txdes, and rx_scratch in stages but returned directly on any intermediate allocation failure. This left previously allocated objects unreleased and could accumulate leaks across repeated ifup retries under memory pressure.

Switch ftgmac100_alloc_rings() to a centralized goto-based unwind path that calls ftgmac100_free_rings() on failure so partially allocated ring resources are always released before returning -ENOMEM.

Also clear rx_skbs, tx_skbs, and rx_scratch pointers in ftgmac100_free_rings() after freeing. This prevents stale pointers from being reused during later retry failures and keeps ring teardown idempotent.

Signed-off-by: Yufan Chen <ericterminal@gmail.com>
---
 drivers/net/ethernet/faraday/ftgmac100.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 1e91e79c8..147300e60 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -946,7 +946,9 @@ static void ftgmac100_free_rings(struct ftgmac100 *priv)
 {
 	/* Free skb arrays */
 	kfree(priv->rx_skbs);
+	priv->rx_skbs = NULL;
 	kfree(priv->tx_skbs);
+	priv->tx_skbs = NULL;
 
 	/* Free descriptors */
 	if (priv->rxdes)
@@ -965,31 +967,34 @@ static void ftgmac100_free_rings(struct ftgmac100 *priv)
 	if (priv->rx_scratch)
 		dma_free_coherent(priv->dev, RX_BUF_SIZE,
 				  priv->rx_scratch, priv->rx_scratch_dma);
+	priv->rx_scratch = NULL;
 }
 
 static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
 {
+	int err = -ENOMEM;
+
 	/* Allocate skb arrays */
 	priv->rx_skbs = kcalloc(MAX_RX_QUEUE_ENTRIES, sizeof(void *),
 				GFP_KERNEL);
 	if (!priv->rx_skbs)
-		return -ENOMEM;
+		goto out;
 	priv->tx_skbs = kcalloc(MAX_TX_QUEUE_ENTRIES, sizeof(void *),
 				GFP_KERNEL);
 	if (!priv->tx_skbs)
-		return -ENOMEM;
+		goto out;
 
 	/* Allocate descriptors */
 	priv->rxdes = dma_alloc_coherent(priv->dev,
 					 MAX_RX_QUEUE_ENTRIES * sizeof(struct ftgmac100_rxdes),
 					 &priv->rxdes_dma, GFP_KERNEL);
 	if (!priv->rxdes)
-		return -ENOMEM;
+		goto out;
 	priv->txdes = dma_alloc_coherent(priv->dev,
 					 MAX_TX_QUEUE_ENTRIES * sizeof(struct ftgmac100_txdes),
 					 &priv->txdes_dma, GFP_KERNEL);
 	if (!priv->txdes)
-		return -ENOMEM;
+		goto out;
 
 	/* Allocate scratch packet buffer */
 	priv->rx_scratch = dma_alloc_coherent(priv->dev,
@@ -997,9 +1002,13 @@ static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
 					      &priv->rx_scratch_dma,
 					      GFP_KERNEL);
 	if (!priv->rx_scratch)
-		return -ENOMEM;
+		goto out;
 
 	return 0;
+
+out:
+	ftgmac100_free_rings(priv);
+	return err;
 }
 
 static void ftgmac100_init_rings(struct ftgmac100 *priv)
-- 
2.47.3


^ permalink raw reply related

* Re: [PATCH net-next 2/2] net: stmmac: simplify GSO/TSO test in stmmac_xmit()
From: Russell King (Oracle) @ 2026-03-28  9:31 UTC (permalink / raw)
  To: Andrew Lunn, Ong Boon Leong
  Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, linux-arm-kernel, linux-stm32, netdev,
	Paolo Abeni
In-Reply-To: <aceQRSc5o4D-HHmq@shell.armlinux.org.uk>

On Sat, Mar 28, 2026 at 08:24:37AM +0000, Russell King (Oracle) wrote:
> On Fri, Mar 27, 2026 at 09:40:09AM +0000, Russell King (Oracle) wrote:
> > The test in stmmac_xmit() to see whether we should pass the skbuff to
> > stmmac_tso_xmit() is more complex than it needs to be. This test can
> > be simplified by storing the mask of GSO types that we will pass, and
> > setting it according to the enabled features.
> > 
> > Note that "tso" is a mis-nomer since commit b776620651a1 ("net:
> > stmmac: Implement UDP Segmentation Offload"). Also note that this
> > commit controls both via the TSO feature. We preserve this behaviour
> > in this commit.
> > 
> > Also, this commit unconditionally accessed skb_shinfo(skb)->gso_type
> > for all frames, even when skb_is_gso() was false. This access is
> > eliminated.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
> 
> AI review of this patch regurgitates Jakub's point that was discussed.
> 
> > @@ -3700,7 +3700,7 @@ static int stmmac_hw_setup(struct net_device *dev)
> >  	stmmac_set_rings_length(priv);
> >  
> >  	/* Enable TSO */
> > -	if (priv->tso) {
> > +	if (priv->gso_enabled_types) {
> >  		for (chan = 0; chan < tx_cnt; chan++) {
> >  			struct stmmac_tx_queue *tx_q = &priv->dma_conf.tx_queue[chan];
> >  
> 
> ...
> 
> > @@ -7828,7 +7834,7 @@ static int __stmmac_dvr_probe(struct device *device,
> >  		ndev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
> >  		if (priv->plat->core_type == DWMAC_CORE_GMAC4)
> >  			ndev->hw_features |= NETIF_F_GSO_UDP_L4;
> > -		priv->tso = true;
> > +		stmmac_set_gso_types(priv, true);
> 
> Clearly, the issue it is regurgitating has been there for a long time
> and isn't a new issue introduced by this patch.
> 
> AI needs to stop doing this, because it is encouraging multiple changes
> in a single patch, which is against the normal kernel process.
> 
> As already pointed out, there are multiple issues with stmmac TSO
> support, particularly with glue drivers that enable TSO on some
> queues/channels and not others, since netdev core TSO support is
> global across all channels.
> 
> So, won't the AI response in this patch - it's just another pre-
> existing issue that needs fixing in a separate patch.

Looking at the TSO vs TBS issue (which precludes the use of TSO on a
channel in stmmac) I can't find an obvious reason for this in the
available documentation. However, unfortunately, iMX8MP doesn't support
TSO, so the TSO bits are elided there, but does support TBS (needing
enhanced descriptors to be enabled). STM32MP151 on the other hand
supports TSO but not TBS, and thus fails to mention anything about
enhanced descriptors or TBS.

When stmmac_enable_tbs() enables TBS, it isn't actually enabling a
feature specific bit, but switching the channel to use enhanced
descriptor format. This format extends the basic descriptors by
placing four extra 32-bit words before the basic descriptor.

Looking at the enhanced normal descriptor format for TDES3, it
indicates that the format includes bit 18 in the control field, which
is the TSE bit (TCP segmentation enable for this packet.) So, it seems
it's not a limitation of the descriptor format.

So, either "TSO and TBS cannot co-exist" is incorrect, or there is a
hardware limitation that isn't documented between these two manuals.

One other interesting point is that stmmac_tso_xmit() seems to
handle the case where TSO and TBS are enabled on the channel:

                if (tx_q->tbs & STMMAC_TBS_AVAIL)
                        mss_desc = &tx_q->dma_entx[tx_q->cur_tx].basic;
                else
                        mss_desc = &tx_q->dma_tx[tx_q->cur_tx];

                stmmac_set_mss(priv, mss_desc, mss);
...
        if (tx_q->tbs & STMMAC_TBS_AVAIL)
                desc = &tx_q->dma_entx[first_entry].basic;
        else
                desc = &tx_q->dma_tx[first_entry];
        first = desc;

etc.

Avoiding enabling TSO for a TBS channel was added by this commit:

commit 5e6038b88a5718910dd74b949946d9d9cee9a041
Author: Ong Boon Leong <boon.leong.ong@intel.com>
Date:   Wed Apr 21 17:11:49 2021 +0800

    net: stmmac: fix TSO and TBS feature enabling during driver open

    TSO and TBS cannot co-exist and current implementation requires two
    fixes:

     1) stmmac_open() does not need to call stmmac_enable_tbs() because
        the MAC is reset in stmmac_init_dma_engine() anyway.
     2) Inside stmmac_hw_setup(), we should call stmmac_enable_tso() for
        TX Q that is _not_ configured for TBS.

    Fixes: 579a25a854d4 ("net: stmmac: Initial support for TBS")
    Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

which doesn't really explain the background, and leaves all the TBS
cruft in the TSO transmit path (nothing like properly updating the
driver, eh? No wonder stmmac is such a mess!)

Maybe Ong Boon Leong can indicate where this restriction comes from?
Note: as mentioned previously, disabling TSO only on some channels is
actually wrong - the netdev core doesn't know which channels support
TSO and which don't, so the driver is likely to still get TSO skbuffs
for channels that the above commit has disabled TSO support. So, if
TBS is enabled and it is incompatible with TSO, then either we need
to use software TSO support _or_ disable TSO for the entire interface.

Incidentally, while looking at this, I found a few more pre-conditions
for TSO:

- TxPBL must be >= 4
- MSS[13:0] must be more than the configured data width in bytes, up to
  a maximum of 1023 bytes.

I'm fairly certain that the driver does nothing to ensure that this
in the case for either of these two points.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* [PATCH net-next 0/4] net: macb: Remove dedicated IRQ handler for WoL
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
  To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Kevin Hao, netdev

During debugging of a suspend/resume issue, I observed that the macb driver
employs a dedicated IRQ handler for Wake-on-LAN (WoL) support. To my knowledge,
no other Ethernet driver adopts this approach. This implementation unnecessarily
complicates the suspend/resume process without providing any clear benefit.
Instead, we can easily modify the existing IRQ handler to manage WoL events,
avoiding any overhead in the TX/RX hot path.

I am skeptical that the minor optimizations to the IRQ handler proposed in this
patch series would yield any measurable performance improvement. However, it
does appear that the execution time of the macb_interrupt() function is
slightly reduced.

The following data(net throughput and execution time of macb_interrupt) were
collected from my AMD Zynqmp board using the commands:
  taskset -c 1,2,3 iperf3 -c 192.168.3.4 -t 60 -Z -P 3 -R
  cat /sys/kernel/debug/tracing/trace_stat/function0

Before:
-------
  [SUM]   0.00-60.00  sec  5.99 GBytes   857 Mbits/sec    0             sender
  [SUM]   0.00-60.00  sec  5.98 GBytes   856 Mbits/sec                  receiver

  Function                               Hit    Time            Avg             s^2
  --------                               ---    ----            ---             ---
  macb_interrupt                      218538    723327.5 us     3.309 us        1.022 us

After:
------
  [SUM]   0.00-60.00  sec  5.99 GBytes   857 Mbits/sec    0             sender
  [SUM]   0.00-60.00  sec  5.98 GBytes   857 Mbits/sec                  receiver

Function                               Hit    Time            Avg             s^2
  --------                               ---    ----            ---             ---
  macb_interrupt                      218558    646355.1 us     2.957 us        1.290 us

---
Kevin Hao (4):
      net: macb: Replace open-coded implementation with napi_schedule()
      net: macb: Consolidate MACB_CAPS_ISR_CLEAR_ON_WRITE checks in IRQ handler
      net: macb: Factor out the handling of non-hot IRQ events into a separate function
      net: macb: Remove dedicated IRQ handler for WoL

 drivers/net/ethernet/cadence/macb_main.c | 244 +++++++++++++------------------
 1 file changed, 102 insertions(+), 142 deletions(-)
---
base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
change-id: 20260321-macb-irq-453ee09b3394

Best regards,
-- 
Kevin Hao <haokexin@gmail.com>


^ permalink raw reply

* [PATCH net-next 1/4] net: macb: Replace open-coded implementation with napi_schedule()
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
  To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Kevin Hao, netdev
In-Reply-To: <20260328-macb-irq-v1-0-7b3e622fb46c@gmail.com>

The driver currently duplicates the logic of napi_schedule() primarily
to include additional debug information. However, these debug details
are not essential for a specific driver and can be effectively obtained
through existing tracepoints in the networking core, such as
/sys/kernel/tracing/events/napi/napi_poll. Therefore, this patch
replaces the open-coded implementation with napi_schedule() to
simplify the driver's code.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c8182559edf602e7f5d94644cffb22e8f58423cc..886246a6f6bdd0b6a8cb4b86d7788ac181ee602a 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2120,10 +2120,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
 				queue_writel(queue, ISR, MACB_BIT(RCOMP));
 
-			if (napi_schedule_prep(&queue->napi_rx)) {
-				netdev_vdbg(bp->dev, "scheduling RX softirq\n");
-				__napi_schedule(&queue->napi_rx);
-			}
+			napi_schedule(&queue->napi_rx);
 		}
 
 		if (status & (MACB_BIT(TCOMP) |
@@ -2138,10 +2135,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 				wmb(); // ensure softirq can see update
 			}
 
-			if (napi_schedule_prep(&queue->napi_tx)) {
-				netdev_vdbg(bp->dev, "scheduling TX softirq\n");
-				__napi_schedule(&queue->napi_tx);
-			}
+			napi_schedule(&queue->napi_tx);
 		}
 
 		if (unlikely(status & (MACB_TX_ERR_FLAGS))) {

-- 
2.53.0


^ permalink raw reply related

* [PATCH net-next 2/4] net: macb: Consolidate MACB_CAPS_ISR_CLEAR_ON_WRITE checks in IRQ handler
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
  To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Kevin Hao, netdev
In-Reply-To: <20260328-macb-irq-v1-0-7b3e622fb46c@gmail.com>

Currently, the MACB_CAPS_ISR_CLEAR_ON_WRITE flag is checked in every
branch of the IRQ handler. This repeated evaluation is unnecessary.
By consolidating the flag check, we eliminate redundant loads of
bp->caps when TX and RX events occur simultaneously, a common scenario
under high network throughput. Additionally, this optimization reduces
the function size from 0x2e8 to 0x2c4.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 886246a6f6bdd0b6a8cb4b86d7788ac181ee602a..743abe11324c690c11993d7be9ed5b73422dd17c 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2088,19 +2088,22 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 	struct macb *bp = queue->bp;
 	struct net_device *dev = bp->dev;
 	u32 status, ctrl;
+	bool isr_clear;
 
 	status = queue_readl(queue, ISR);
 
 	if (unlikely(!status))
 		return IRQ_NONE;
 
+	isr_clear = bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE;
+
 	spin_lock(&bp->lock);
 
 	while (status) {
 		/* close possible race with dev_close */
 		if (unlikely(!netif_running(dev))) {
 			queue_writel(queue, IDR, -1);
-			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+			if (isr_clear)
 				queue_writel(queue, ISR, -1);
 			break;
 		}
@@ -2117,7 +2120,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 			 * now.
 			 */
 			queue_writel(queue, IDR, bp->rx_intr_mask);
-			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+			if (isr_clear)
 				queue_writel(queue, ISR, MACB_BIT(RCOMP));
 
 			napi_schedule(&queue->napi_rx);
@@ -2126,7 +2129,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 		if (status & (MACB_BIT(TCOMP) |
 			      MACB_BIT(TXUBR))) {
 			queue_writel(queue, IDR, MACB_BIT(TCOMP));
-			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+			if (isr_clear)
 				queue_writel(queue, ISR, MACB_BIT(TCOMP) |
 							 MACB_BIT(TXUBR));
 
@@ -2142,7 +2145,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 			queue_writel(queue, IDR, MACB_TX_INT_FLAGS);
 			schedule_work(&queue->tx_error_task);
 
-			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+			if (isr_clear)
 				queue_writel(queue, ISR, MACB_TX_ERR_FLAGS);
 
 			break;
@@ -2165,7 +2168,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 			wmb();
 			macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
 
-			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+			if (isr_clear)
 				queue_writel(queue, ISR, MACB_BIT(RXUBR));
 		}
 
@@ -2178,7 +2181,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 				bp->hw_stats.macb.rx_overruns++;
 			spin_unlock(&bp->stats_lock);
 
-			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+			if (isr_clear)
 				queue_writel(queue, ISR, MACB_BIT(ISR_ROVR));
 		}
 
@@ -2186,7 +2189,7 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 			queue_work(system_bh_wq, &bp->hresp_err_bh_work);
 			netdev_err(dev, "DMA bus error: HRESP not OK\n");
 
-			if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+			if (isr_clear)
 				queue_writel(queue, ISR, MACB_BIT(HRESP));
 		}
 		status = queue_readl(queue, ISR);

-- 
2.53.0


^ permalink raw reply related

* [PATCH net-next 3/4] net: macb: Factor out the handling of non-hot IRQ events into a separate function
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
  To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Kevin Hao, netdev
In-Reply-To: <20260328-macb-irq-v1-0-7b3e622fb46c@gmail.com>

In the current code, the IRQ handler checks each IRQ event sequentially.
Since most IRQ events are related to TX/RX operations, while other
events occur infrequently, this approach introduces unnecessary overhead
in the hot path for TX/RX processing. This patch reduces such overhead
by extracting the handling of all non-TX/RX events into a new function
and consolidating these events under a new flag. As a result, only a
single check is required to determine whether any non-TX/RX events have
occurred. If such events exist, the handler jumps to the new function.
This optimization reduces four conditional checks to one and prevents
the instruction cache from being polluted with rarely used code in the
hot path.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 123 ++++++++++++++++++-------------
 1 file changed, 72 insertions(+), 51 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 743abe11324c690c11993d7be9ed5b73422dd17c..c53b28b42a46489722461957625e8377be63e427 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -70,6 +70,9 @@ struct sifive_fu540_macb_mgmt {
 #define MACB_TX_INT_FLAGS	(MACB_TX_ERR_FLAGS | MACB_BIT(TCOMP)	\
 					| MACB_BIT(TXUBR))
 
+#define MACB_INT_MISC_FLAGS	(MACB_TX_ERR_FLAGS | MACB_BIT(RXUBR) | \
+				 MACB_BIT(ISR_ROVR) | MACB_BIT(HRESP))
+
 /* Max length of transmit frame must be a multiple of 8 bytes */
 #define MACB_TX_LEN_ALIGN	8
 #define MACB_MAX_TX_LEN		((unsigned int)((1 << MACB_TX_FRMLEN_SIZE) - 1) & ~((unsigned int)(MACB_TX_LEN_ALIGN - 1)))
@@ -2082,12 +2085,77 @@ static irqreturn_t gem_wol_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
+{
+	struct macb *bp = queue->bp;
+	struct net_device *dev;
+	bool isr_clear;
+	u32 ctrl;
+
+	dev = bp->dev;
+	isr_clear = bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE;
+
+	if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
+		queue_writel(queue, IDR, MACB_TX_INT_FLAGS);
+		schedule_work(&queue->tx_error_task);
+
+		if (isr_clear)
+			queue_writel(queue, ISR, MACB_TX_ERR_FLAGS);
+
+		return -1;
+	}
+
+	/* Link change detection isn't possible with RMII, so we'll
+	 * add that if/when we get our hands on a full-blown MII PHY.
+	 */
+
+	/* There is a hardware issue under heavy load where DMA can
+	 * stop, this causes endless "used buffer descriptor read"
+	 * interrupts but it can be cleared by re-enabling RX. See
+	 * the at91rm9200 manual, section 41.3.1 or the Zynq manual
+	 * section 16.7.4 for details. RXUBR is only enabled for
+	 * these two versions.
+	 */
+	if (status & MACB_BIT(RXUBR)) {
+		ctrl = macb_readl(bp, NCR);
+		macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+		wmb();
+		macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+
+		if (isr_clear)
+			queue_writel(queue, ISR, MACB_BIT(RXUBR));
+	}
+
+	if (status & MACB_BIT(ISR_ROVR)) {
+		/* We missed at least one packet */
+		spin_lock(&bp->stats_lock);
+		if (macb_is_gem(bp))
+			bp->hw_stats.gem.rx_overruns++;
+		else
+			bp->hw_stats.macb.rx_overruns++;
+		spin_unlock(&bp->stats_lock);
+
+		if (isr_clear)
+			queue_writel(queue, ISR, MACB_BIT(ISR_ROVR));
+	}
+
+	if (status & MACB_BIT(HRESP)) {
+		queue_work(system_bh_wq, &bp->hresp_err_bh_work);
+		netdev_err(dev, "DMA bus error: HRESP not OK\n");
+
+		if (isr_clear)
+			queue_writel(queue, ISR, MACB_BIT(HRESP));
+	}
+
+	return 0;
+}
+
 static irqreturn_t macb_interrupt(int irq, void *dev_id)
 {
 	struct macb_queue *queue = dev_id;
 	struct macb *bp = queue->bp;
 	struct net_device *dev = bp->dev;
-	u32 status, ctrl;
+	u32 status;
 	bool isr_clear;
 
 	status = queue_readl(queue, ISR);
@@ -2141,57 +2209,10 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
 			napi_schedule(&queue->napi_tx);
 		}
 
-		if (unlikely(status & (MACB_TX_ERR_FLAGS))) {
-			queue_writel(queue, IDR, MACB_TX_INT_FLAGS);
-			schedule_work(&queue->tx_error_task);
+		if (unlikely(status & MACB_INT_MISC_FLAGS))
+			if (macb_interrupt_misc(queue, status))
+				break;
 
-			if (isr_clear)
-				queue_writel(queue, ISR, MACB_TX_ERR_FLAGS);
-
-			break;
-		}
-
-		/* Link change detection isn't possible with RMII, so we'll
-		 * add that if/when we get our hands on a full-blown MII PHY.
-		 */
-
-		/* There is a hardware issue under heavy load where DMA can
-		 * stop, this causes endless "used buffer descriptor read"
-		 * interrupts but it can be cleared by re-enabling RX. See
-		 * the at91rm9200 manual, section 41.3.1 or the Zynq manual
-		 * section 16.7.4 for details. RXUBR is only enabled for
-		 * these two versions.
-		 */
-		if (status & MACB_BIT(RXUBR)) {
-			ctrl = macb_readl(bp, NCR);
-			macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
-			wmb();
-			macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
-
-			if (isr_clear)
-				queue_writel(queue, ISR, MACB_BIT(RXUBR));
-		}
-
-		if (status & MACB_BIT(ISR_ROVR)) {
-			/* We missed at least one packet */
-			spin_lock(&bp->stats_lock);
-			if (macb_is_gem(bp))
-				bp->hw_stats.gem.rx_overruns++;
-			else
-				bp->hw_stats.macb.rx_overruns++;
-			spin_unlock(&bp->stats_lock);
-
-			if (isr_clear)
-				queue_writel(queue, ISR, MACB_BIT(ISR_ROVR));
-		}
-
-		if (status & MACB_BIT(HRESP)) {
-			queue_work(system_bh_wq, &bp->hresp_err_bh_work);
-			netdev_err(dev, "DMA bus error: HRESP not OK\n");
-
-			if (isr_clear)
-				queue_writel(queue, ISR, MACB_BIT(HRESP));
-		}
 		status = queue_readl(queue, ISR);
 	}
 

-- 
2.53.0


^ permalink raw reply related

* [PATCH net-next 4/4] net: macb: Remove dedicated IRQ handler for WoL
From: Kevin Hao @ 2026-03-28 10:17 UTC (permalink / raw)
  To: Nicolas Ferre, Claudiu Beznea, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Kevin Hao, netdev
In-Reply-To: <20260328-macb-irq-v1-0-7b3e622fb46c@gmail.com>

In the current implementation, the suspend/resume path frees the
existing IRQ handler and sets up a dedicated WoL IRQ handler, then
restores the original handler upon resume. This approach is not used
by any other Ethernet driver and unnecessarily complicates the
suspend/resume process. After adjusting the IRQ handler in the previous
patches, we can now handle WoL interrupts without introducing any
overhead in the TX/RX hot path. Therefore, the dedicated WoL IRQ
handler is removed.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 drivers/net/ethernet/cadence/macb_main.c | 116 ++++++++-----------------------
 1 file changed, 29 insertions(+), 87 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c53b28b42a46489722461957625e8377be63e427..de166dd9e637f5e436274b99f2c5eef99dcdb351 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -71,7 +71,8 @@ struct sifive_fu540_macb_mgmt {
 					| MACB_BIT(TXUBR))
 
 #define MACB_INT_MISC_FLAGS	(MACB_TX_ERR_FLAGS | MACB_BIT(RXUBR) | \
-				 MACB_BIT(ISR_ROVR) | MACB_BIT(HRESP))
+				 MACB_BIT(ISR_ROVR) | MACB_BIT(HRESP) | \
+				 GEM_BIT(WOL) | MACB_BIT(WOL))
 
 /* Max length of transmit frame must be a multiple of 8 bytes */
 #define MACB_TX_LEN_ALIGN	8
@@ -2027,62 +2028,32 @@ static void macb_hresp_error_task(struct work_struct *work)
 	netif_tx_start_all_queues(dev);
 }
 
-static irqreturn_t macb_wol_interrupt(int irq, void *dev_id)
+static void macb_wol_interrupt(struct macb_queue *queue, u32 status)
 {
-	struct macb_queue *queue = dev_id;
 	struct macb *bp = queue->bp;
-	u32 status;
 
-	status = queue_readl(queue, ISR);
-
-	if (unlikely(!status))
-		return IRQ_NONE;
-
-	spin_lock(&bp->lock);
-
-	if (status & MACB_BIT(WOL)) {
-		queue_writel(queue, IDR, MACB_BIT(WOL));
-		macb_writel(bp, WOL, 0);
-		netdev_vdbg(bp->dev, "MACB WoL: queue = %u, isr = 0x%08lx\n",
-			    (unsigned int)(queue - bp->queues),
-			    (unsigned long)status);
-		if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
-			queue_writel(queue, ISR, MACB_BIT(WOL));
-		pm_wakeup_event(&bp->pdev->dev, 0);
-	}
-
-	spin_unlock(&bp->lock);
-
-	return IRQ_HANDLED;
+	queue_writel(queue, IDR, MACB_BIT(WOL));
+	macb_writel(bp, WOL, 0);
+	netdev_vdbg(bp->dev, "MACB WoL: queue = %u, isr = 0x%08lx\n",
+		    (unsigned int)(queue - bp->queues),
+		    (unsigned long)status);
+	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+		queue_writel(queue, ISR, MACB_BIT(WOL));
+	pm_wakeup_event(&bp->pdev->dev, 0);
 }
 
-static irqreturn_t gem_wol_interrupt(int irq, void *dev_id)
+static void gem_wol_interrupt(struct macb_queue *queue, u32 status)
 {
-	struct macb_queue *queue = dev_id;
 	struct macb *bp = queue->bp;
-	u32 status;
 
-	status = queue_readl(queue, ISR);
-
-	if (unlikely(!status))
-		return IRQ_NONE;
-
-	spin_lock(&bp->lock);
-
-	if (status & GEM_BIT(WOL)) {
-		queue_writel(queue, IDR, GEM_BIT(WOL));
-		gem_writel(bp, WOL, 0);
-		netdev_vdbg(bp->dev, "GEM WoL: queue = %u, isr = 0x%08lx\n",
-			    (unsigned int)(queue - bp->queues),
-			    (unsigned long)status);
-		if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
-			queue_writel(queue, ISR, GEM_BIT(WOL));
-		pm_wakeup_event(&bp->pdev->dev, 0);
-	}
-
-	spin_unlock(&bp->lock);
-
-	return IRQ_HANDLED;
+	queue_writel(queue, IDR, GEM_BIT(WOL));
+	gem_writel(bp, WOL, 0);
+	netdev_vdbg(bp->dev, "GEM WoL: queue = %u, isr = 0x%08lx\n",
+		    (unsigned int)(queue - bp->queues),
+		    (unsigned long)status);
+	if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
+		queue_writel(queue, ISR, GEM_BIT(WOL));
+	pm_wakeup_event(&bp->pdev->dev, 0);
 }
 
 static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
@@ -2147,6 +2118,14 @@ static int macb_interrupt_misc(struct macb_queue *queue, u32 status)
 			queue_writel(queue, ISR, MACB_BIT(HRESP));
 	}
 
+	if (macb_is_gem(bp)) {
+		if (status & GEM_BIT(WOL))
+			gem_wol_interrupt(queue, status);
+	} else {
+		if (status & MACB_BIT(WOL))
+			macb_wol_interrupt(queue, status);
+	}
+
 	return 0;
 }
 
@@ -5943,7 +5922,6 @@ static int __maybe_unused macb_suspend(struct device *dev)
 	unsigned long flags;
 	u32 tmp, ifa_local;
 	unsigned int q;
-	int err;
 
 	if (!device_may_wakeup(&bp->dev->dev))
 		phy_exit(bp->phy);
@@ -6007,39 +5985,15 @@ static int __maybe_unused macb_suspend(struct device *dev)
 			/* write IP address into register */
 			tmp |= MACB_BFEXT(IP, ifa_local);
 		}
-		spin_unlock_irqrestore(&bp->lock, flags);
 
-		/* Change interrupt handler and
-		 * Enable WoL IRQ on queue 0
-		 */
-		devm_free_irq(dev, bp->queues[0].irq, bp->queues);
 		if (macb_is_gem(bp)) {
-			err = devm_request_irq(dev, bp->queues[0].irq, gem_wol_interrupt,
-					       IRQF_SHARED, netdev->name, bp->queues);
-			if (err) {
-				dev_err(dev,
-					"Unable to request IRQ %d (error %d)\n",
-					bp->queues[0].irq, err);
-				return err;
-			}
-			spin_lock_irqsave(&bp->lock, flags);
 			queue_writel(bp->queues, IER, GEM_BIT(WOL));
 			gem_writel(bp, WOL, tmp);
-			spin_unlock_irqrestore(&bp->lock, flags);
 		} else {
-			err = devm_request_irq(dev, bp->queues[0].irq, macb_wol_interrupt,
-					       IRQF_SHARED, netdev->name, bp->queues);
-			if (err) {
-				dev_err(dev,
-					"Unable to request IRQ %d (error %d)\n",
-					bp->queues[0].irq, err);
-				return err;
-			}
-			spin_lock_irqsave(&bp->lock, flags);
 			queue_writel(bp->queues, IER, MACB_BIT(WOL));
 			macb_writel(bp, WOL, tmp);
-			spin_unlock_irqrestore(&bp->lock, flags);
 		}
+		spin_unlock_irqrestore(&bp->lock, flags);
 
 		enable_irq_wake(bp->queues[0].irq);
 	}
@@ -6081,7 +6035,6 @@ static int __maybe_unused macb_resume(struct device *dev)
 	struct macb_queue *queue;
 	unsigned long flags;
 	unsigned int q;
-	int err;
 
 	if (!device_may_wakeup(&bp->dev->dev))
 		phy_init(bp->phy);
@@ -6108,17 +6061,6 @@ static int __maybe_unused macb_resume(struct device *dev)
 			queue_writel(bp->queues, ISR, -1);
 		spin_unlock_irqrestore(&bp->lock, flags);
 
-		/* Replace interrupt handler on queue 0 */
-		devm_free_irq(dev, bp->queues[0].irq, bp->queues);
-		err = devm_request_irq(dev, bp->queues[0].irq, macb_interrupt,
-				       IRQF_SHARED, netdev->name, bp->queues);
-		if (err) {
-			dev_err(dev,
-				"Unable to request IRQ %d (error %d)\n",
-				bp->queues[0].irq, err);
-			return err;
-		}
-
 		disable_irq_wake(bp->queues[0].irq);
 
 		/* Now make sure we disable phy before moving

-- 
2.53.0


^ permalink raw reply related

* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Russell King (Oracle) @ 2026-03-28 10:25 UTC (permalink / raw)
  To: Eric Woudstra
  Cc: Lucien.Jheng, andrew, hkallweit1, davem, edumazet, kuba, pabeni,
	netdev, linux-kernel, bjorn, frank-w, daniel, lucien.jheng
In-Reply-To: <6f42d92b-a26d-49a4-bef7-2227c252eea7@gmail.com>

On Sat, Mar 28, 2026 at 11:21:59AM +0100, Eric Woudstra wrote:
> static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> 			      u32 pbus_data)
> {
> 	int ret;
> 
> 	mutex_lock(&mdio->bus->mdio_lock);

I wonder why we have phy_lock_mdio_bus() but not mdio_bus_lock()...
Should a helper be provided at mdio bus level to complement the PHY
level helper if we're going to have MDIO drivers directly manipulating
the lock?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply

* Re: [PATCH 6.1.y] wifi: brcmfmac: fix use-after-free when rescheduling brcmf_btcoex_info work
From: Arend van Spriel @ 2026-03-28 10:54 UTC (permalink / raw)
  To: Robert Garcia, stable, Duoming Zhou
  Cc: Johannes Berg, Kalle Valo, Franky Lin, Hante Meuleman,
	David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Pieter-Paul Giesberts, Piotr Haber, John W . Linville,
	linux-wireless, brcm80211-dev-list.pdl, SHA-cyfmac-dev-list,
	netdev, linux-kernel
In-Reply-To: <20260312031429.3432419-1-rob_garcia@163.com>

On 12/03/2026 04:14, Robert Garcia wrote:
> From: Duoming Zhou <duoming@zju.edu.cn>
> 
> [ Upstream commit 9cb83d4be0b9b697eae93d321e0da999f9cdfcfc ]
> 
> The brcmf_btcoex_detach() only shuts down the btcoex timer, if the
> flag timer_on is false. However, the brcmf_btcoex_timerfunc(), which
> runs as timer handler, sets timer_on to false. This creates critical
> race conditions:

[...]

> To resolve the race conditions, drop the conditional check and call
> timer_shutdown_sync() directly. It can deactivate the timer reliably,
> regardless of its current state. Once stopped, the timer_on state is
> then set to false.
> 
> Fixes: 61730d4dfffc ("brcmfmac: support critical protocol API for DHCP")
> Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
> Link: https://patch.msgid.link/20250822050839.4413-1-duoming@zju.edu.cn
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> [ Keep del_timer_sync() instead of timer_shutdown_sync() here. ]
> Signed-off-by: Robert Garcia <rob_garcia@163.com>

What tree should this go to. This looks like a stable patch so probably 
it should have been CCed to stable@vger.kernel.org?

Regards,
Arend

^ permalink raw reply

* Re: [PATCH bpf v3 2/2] selftests/bpf: Add protocol check test for bpf_sk_assign_tcp_reqsk()
From: Jiayuan Chen @ 2026-03-28 10:58 UTC (permalink / raw)
  To: Martin KaFai Lau
  Cc: Jiayuan Chen, Daniel Borkmann, John Fastabend, Stanislav Fomichev,
	Alexei Starovoitov, Andrii Nakryiko, Eduard Zingerman, Song Liu,
	Yonghong Song, KP Singh, Hao Luo, Jiri Olsa, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Shuah Khan, Kuniyuki Iwashima, bpf, netdev, linux-kernel,
	linux-kselftest
In-Reply-To: <8d619537-cb9a-4fdb-a440-20b59b7f5088@linux.dev>


On 3/28/26 3:53 AM, Martin KaFai Lau wrote:
>> From: Jiayuan Chen <jiayuan.chen@shopee.com>
>>
>> Add test_tcp_custom_syncookie_protocol_check to verify that
>> bpf_sk_assign_tcp_reqsk() rejects non-TCP skbs. The test sends a UDP
>> packet through TC ingress where a BPF program calls
>> bpf_sk_assign_tcp_reqsk() on it and checks that the kfunc returns an
>> error. A UDP server recv() is used as synchronization to ensure the
>> BPF program has finished processing before checking the result.
>>
>> Without the fix in bpf_sk_assign_tcp_reqsk(), the kfunc succeeds and
>> attaches a TCP reqsk to the UDP skb, which causes a null pointer
>> dereference panic when the kernel processes it through the UDP receive
>> path.
>>
>> Test result:
>>
>>    ./test_progs -a tcp_custom_syncookie_protocol_check -v
>>    setup_netns:PASS:create netns 0 nsec
>>    setup_netns:PASS:ip 0 nsec
>>    write_sysctl:PASS:open sysctl 0 nsec
>>    write_sysctl:PASS:write sysctl 0 nsec
>>    setup_netns:PASS:write_sysctl 0 nsec
>>    test_tcp_custom_syncookie_protocol_check:PASS:open_and_load 0 nsec
>>    setup_tc:PASS:qdisc add dev lo clsact 0 nsec
>>    setup_tc:PASS:filter add dev lo ingress 0 nsec
>>    run_protocol_check:PASS:start tcp_server 0 nsec
>>    run_protocol_check:PASS:start udp_server 0 nsec
>>    run_protocol_check:PASS:connect udp_client 0 nsec
>>    run_protocol_check:PASS:send udp 0 nsec
>>    run_protocol_check:PASS:recv udp 0 nsec
>>    run_protocol_check:PASS:udp_intercepted 0 nsec
>>    run_protocol_check:PASS:assign_ret 0 nsec
>>    #471/1   tcp_custom_syncookie_protocol_check/IPv4 TCP:OK
>>    run_protocol_check:PASS:start tcp_server 0 nsec
>>    run_protocol_check:PASS:start udp_server 0 nsec
>>    run_protocol_check:PASS:connect udp_client 0 nsec
>>    run_protocol_check:PASS:send udp 0 nsec
>>    run_protocol_check:PASS:recv udp 0 nsec
>>    run_protocol_check:PASS:udp_intercepted 0 nsec
>>    run_protocol_check:PASS:assign_ret 0 nsec
>>    #471/2   tcp_custom_syncookie_protocol_check/IPv6 TCP:OK
>>    #471     tcp_custom_syncookie_protocol_check:OK
>>    Summary: 1/2 PASSED, 0 SKIPPED, 0 FAILED
>>
>> Cc: Jiayuan Chen <jiayuan.chen@linux.dev>
>> Signed-off-by: Jiayuan Chen <jiayuan.chen@shopee.com>
>> ---
>>   .../bpf/prog_tests/tcp_custom_syncookie.c     |  94 ++++++++++++++-
>>   .../bpf/progs/test_tcp_custom_syncookie.c     | 109 ++++++++++++++++++
>>   2 files changed, 199 insertions(+), 4 deletions(-)
>>
>> diff --git 
>> a/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c 
>> b/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c
>> index eaf441dc7e79..e46031d0786b 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/tcp_custom_syncookie.c
>> @@ -5,6 +5,7 @@
>>   #include <sched.h>
>>   #include <stdlib.h>
>>   #include <net/if.h>
>> +#include <netinet/in.h>
>>     #include "test_progs.h"
>>   #include "cgroup_helpers.h"
>> @@ -47,11 +48,10 @@ static int setup_netns(void)
>>       return -1;
>>   }
>>   -static int setup_tc(struct test_tcp_custom_syncookie *skel)
>> +static int setup_tc(int prog_fd)
>>   {
>>       LIBBPF_OPTS(bpf_tc_hook, qdisc_lo, .attach_point = 
>> BPF_TC_INGRESS);
>> -    LIBBPF_OPTS(bpf_tc_opts, tc_attach,
>> -            .prog_fd = 
>> bpf_program__fd(skel->progs.tcp_custom_syncookie));
>> +    LIBBPF_OPTS(bpf_tc_opts, tc_attach, .prog_fd = prog_fd);
>>         qdisc_lo.ifindex = if_nametoindex("lo");
>>       if (!ASSERT_OK(bpf_tc_hook_create(&qdisc_lo), "qdisc add dev lo 
>> clsact"))
>> @@ -127,7 +127,7 @@ void test_tcp_custom_syncookie(void)
>>       if (!ASSERT_OK_PTR(skel, "open_and_load"))
>>           return;
>>   -    if (setup_tc(skel))
>> +    if (setup_tc(bpf_program__fd(skel->progs.tcp_custom_syncookie)))
>>           goto destroy_skel;
>>         for (i = 0; i < ARRAY_SIZE(test_cases); i++) {
>> @@ -145,6 +145,92 @@ void test_tcp_custom_syncookie(void)
>>     destroy_skel:
>>       system("tc qdisc del dev lo clsact");
>> +    test_tcp_custom_syncookie__destroy(skel);
>> +}
>> +
>> +/* Test: bpf_sk_assign_tcp_reqsk() should reject non-TCP skb.
>> + *
>> + * Send a UDP packet through TC ingress where a BPF program calls
>> + * bpf_sk_assign_tcp_reqsk() on it. The kfunc should return an error
>> + * because the skb carries UDP, not TCP.
>> + *
>> + * TCP and UDP servers share the same port. The BPF program intercepts
>> + * the UDP packet, looks up the TCP listener via the dest port, and
>> + * attempts to assign a TCP reqsk to the UDP skb.
>> + */
>> +static void run_protocol_check(struct test_tcp_custom_syncookie *skel,
>> +                   int family, const char *addr)
>> +{
>> +    int tcp_server = -1, udp_server = -1, udp_client = -1;
>> +    char buf[32] = "test";
>> +    int port, ret;
>> +
>> +    tcp_server = start_server(family, SOCK_STREAM, addr, 0, 0);
>> +    if (!ASSERT_NEQ(tcp_server, -1, "start tcp_server"))
>> +        return;
>> +
>> +    port = ntohs(get_socket_local_port(tcp_server));
>> +
>> +    /* UDP server on same port for synchronization and port sharing */
>> +    udp_server = start_server(family, SOCK_DGRAM, addr, port, 0);
>> +    if (!ASSERT_NEQ(udp_server, -1, "start udp_server"))
>> +        goto close_tcp;
>> +
>> +    skel->bss->udp_intercepted = false;
>> +    skel->data->assign_ret = -1;
>
> assign_ret is init to -1
>
>> +
>> +    udp_client = connect_to_fd(udp_server, 0);
>> +    if (!ASSERT_NEQ(udp_client, -1, "connect udp_client"))
>> +        goto close_udp_server;
>>   +    ret = send(udp_client, buf, sizeof(buf), 0);
>> +    if (!ASSERT_EQ(ret, sizeof(buf), "send udp"))
>> +        goto close_udp_client;
>> +
>> +    /* recv() ensures TC ingress BPF has processed the skb */
>> +    ret = recv(udp_server, buf, sizeof(buf), 0);
>> +    if (!ASSERT_EQ(ret, sizeof(buf), "recv udp"))
>> +        goto close_udp_client;
>> +
>> +    ASSERT_EQ(skel->bss->udp_intercepted, true, "udp_intercepted");
>> +
>> +    /* assign_ret == 0 means kfunc accepted UDP skb (bug).
>> +     * assign_ret < 0 means kfunc correctly rejected it (fixed).
>> +     */
>> +    ASSERT_NEQ(skel->data->assign_ret, 0, "assign_ret");
>
> and here it checks NEQ 0....
>
> Maybe init assign_ret to 0?
>
> Also, why not specifically check for -EINVAL?


You're right, checking for -EINVAL is more precise and avoids hiding a 
silent failure in the lookup path.

Will init assign_ret to 0 and check for -EINVAL instead.


^ permalink raw reply

* Re: [PATCH net v2] ipv6: fix data race in fib6_metric_set() using cmpxchg
From: Jiayuan Chen @ 2026-03-28 11:22 UTC (permalink / raw)
  To: Hangbin Liu, David S. Miller, David Ahern, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Jiayuan Chen
  Cc: David Ahern, netdev, linux-kernel, Fei Liu
In-Reply-To: <20260327-b4-fib6_metric_set-kmemleak-v2-1-366b2c78b5c2@gmail.com>


On 3/27/26 10:24 AM, Hangbin Liu wrote:
> fib6_metric_set() may be called concurrently from softirq context without
> holding the FIB table lock. A typical path is:
>
>    ndisc_router_discovery()
>      spin_unlock_bh(&table->tb6_lock)        <- lock released
>      fib6_metric_set(rt, RTAX_HOPLIMIT, ...) <- lockless call
>
> When two CPUs process Router Advertisement packets for the same router
> simultaneously, they can both arrive at fib6_metric_set() with the same
> fib6_info pointer whose fib6_metrics still points to dst_default_metrics.
>
>    if (f6i->fib6_metrics == &dst_default_metrics) {   /* both CPUs: true */
>        struct dst_metrics *p = kzalloc_obj(*p, GFP_ATOMIC);
>        refcount_set(&p->refcnt, 1);
>        f6i->fib6_metrics = p;   /* CPU1 overwrites CPU0's p -> p0 leaked */
>    }
>
> The dst_metrics allocated by the losing CPU has refcnt=1 but no pointer
> to it anywhere in memory, producing a kmemleak report:
>
>    unreferenced object 0xff1100025aca1400 (size 96):
>      comm "softirq", pid 0, jiffies 4299271239
>      backtrace:
>        kmalloc_trace+0x28a/0x380
>        fib6_metric_set+0xcd/0x180
>        ndisc_router_discovery+0x12dc/0x24b0
>        icmpv6_rcv+0xc16/0x1360
>
> Fix this by:
>   - Set val for p->metrics before published via cmpxchg() so the metrics
>     value is ready before the pointer becomes visible to other CPUs.
>   - Replace the plain pointer store with cmpxchg() and free the allocation
>     safely when competition failed.
>   - Add READ_ONCE()/WRITE_ONCE() for metrics[] setting in the non-default
>     metrics path to prevent compiler-based data races.
>
> Fixes: d4ead6b34b67 ("net/ipv6: move metrics from dst to rt6_info")
> Reported-by: Fei Liu <feliu@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>

https://sashiko.dev/#/patchset/20260327-b4-fib6_metric_set-kmemleak-v2-1-366b2c78b5c2%40gmail.com


The concern about reader paths (e.g., ip_dst_init_metrics, fib6_pmtu) 
lacking READ_ONCE()
annotations is valid — if the compiler reloads from->fib6_metrics after 
inlining, it could produce
an inconsistent pointer/flags combination in dst->_metrics, potentially 
leading to a refcount_dec
on the read-only dst_default_metrics.

However, this is a pre-existing issue that exists before this patch.
The plain store f6i->fib6_metrics = p in the original code has the same 
read-side race.
This patch focuses on fixing the writer-side data race that causes 
kmemleak, and it
does so correctly.

BTW, please consider moving the declaration of m to the top of the 
function if you have a next version



^ permalink raw reply

* [PATCH] NFC: s3fwrn5: bound the UART receive buffer
From: Pengpeng Hou @ 2026-03-28 11:36 UTC (permalink / raw)
  To: netdev, krzk
  Cc: bongsu.jeon, davem, edumazet, kuba, pabeni, linux-kernel,
	pengpeng

s3fwrn82_uart_read() appends bytes to phy->recv_skb with skb_put_u8().
It only resets the skb after a complete NCI frame is assembled.

The receive skb is allocated with NCI_SKB_BUFF_LEN bytes, but
malformed UART traffic can keep the frame incomplete and continue
growing recv_skb until skb_put_u8() overruns the tailroom.

Drop the accumulated partial frame once the fixed receive buffer is full
and no complete frame has been seen yet so malformed UART traffic cannot
grow the skb past NCI_SKB_BUFF_LEN.

Fixes: 3f52c2cb7e3a ("nfc: s3fwrn5: Support a UART interface")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/nfc/s3fwrn5/uart.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/s3fwrn5/uart.c b/drivers/nfc/s3fwrn5/uart.c
index 9c09c10c2a46..c664fed78c1f 100644
--- a/drivers/nfc/s3fwrn5/uart.c
+++ b/drivers/nfc/s3fwrn5/uart.c
@@ -64,8 +64,17 @@ static size_t s3fwrn82_uart_read(struct serdev_device *serdev,
 			continue;
 
 		if ((phy->recv_skb->len - S3FWRN82_NCI_HEADER)
-				< phy->recv_skb->data[S3FWRN82_NCI_IDX])
+				< phy->recv_skb->data[S3FWRN82_NCI_IDX]) {
+			if (phy->recv_skb->len < NCI_SKB_BUFF_LEN)
+				continue;
+
+			dev_dbg(&serdev->dev, "dropping oversized UART frame\n");
+			kfree_skb(phy->recv_skb);
+			phy->recv_skb = alloc_skb(NCI_SKB_BUFF_LEN, GFP_KERNEL);
+			if (!phy->recv_skb)
+				return 0;
 			continue;
+		}
 
 		s3fwrn5_recv_frame(phy->common.ndev, phy->recv_skb,
 				   phy->common.mode);
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related

* Re: [PATCH RFC 1/8] dt-bindings: clk: sun60i-a733-ccu: Add allwinner A733 support
From: Chen-Yu Tsai @ 2026-03-28 12:07 UTC (permalink / raw)
  To: Junhui Liu
  Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jernej Skrabec, Samuel Holland, Philipp Zabel,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Richard Cochran, linux-clk, devicetree, linux-arm-kernel,
	linux-sunxi, linux-kernel, linux-riscv, netdev
In-Reply-To: <20260310-a733-clk-v1-1-36b4e9b24457@pigmoral.tech>

On Tue, Mar 10, 2026 at 4:42 PM Junhui Liu <junhui.liu@pigmoral.tech> wrote:
>
> The CCU and R-CCU (PRCM) modules provide clocks and reset functions for
> the Allwinner A733 SoC. The clock architecture of the A733 is evolved
> from the A523, though the root clocking strategy transitions from a
> static oscillator frequency in the Devicetree to the "hosc" clock, which
> is determined by choosing from three possible frequencies (19.2MHz,
> 24MHz, or 26MHz) by the RTC hardware, and finally feeds the CCU and
> R-CCU.
>
> Additionally, the MCU_CCU module found in previous designs is removed
> from the A733, and the clock tree is expanded with more clock outputs
> to support new functional modules.
>
> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
> ---
>  .../bindings/clock/allwinner,sun60i-a733-ccu.yaml  | 107 ++++++++
>  include/dt-bindings/clock/sun60i-a733-ccu.h        | 289 +++++++++++++++++++++
>  include/dt-bindings/clock/sun60i-a733-r-ccu.h      |  39 +++
>  include/dt-bindings/reset/sun60i-a733-ccu.h        | 131 ++++++++++
>  include/dt-bindings/reset/sun60i-a733-r-ccu.h      |  23 ++
>  5 files changed, 589 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/allwinner,sun60i-a733-ccu.yaml b/Documentation/devicetree/bindings/clock/allwinner,sun60i-a733-ccu.yaml
> new file mode 100644
> index 000000000000..aff3ff731285
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/allwinner,sun60i-a733-ccu.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/allwinner,sun60i-a733-ccu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A733 Clock Control Unit
> +
> +maintainers:
> +  - Junhui Liu <junhui.liu@pigmoral.tech>
> +
> +properties:
> +  "#clock-cells":
> +    const: 1
> +
> +  "#reset-cells":
> +    const: 1
> +
> +  compatible:
> +    enum:
> +      - allwinner,sun60i-a733-ccu
> +      - allwinner,sun60i-a733-r-ccu
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 4
> +    maxItems: 7
> +
> +  clock-names:
> +    minItems: 4
> +    maxItems: 7
> +
> +required:
> +  - "#clock-cells"
> +  - "#reset-cells"
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun60i-a733-ccu
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: High Frequency Oscillator (19.2MHz, 24MHz, or 26MHz)
> +            - description: Low Frequency Oscillator (usually at 32kHz)
> +            - description: Internal Oscillator
> +            - description: Low Frequency Oscillator fanout
> +
> +        clock-names:
> +          items:
> +            - const: hosc
> +            - const: losc
> +            - const: iosc
> +            - const: losc-fanout
> +
> +  - if:
> +      properties:
> +        compatible:
> +          enum:
> +            - allwinner,sun60i-a733-r-ccu
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: High Frequency Oscillator (19.2MHz, 24MHz, or 26MHz)
> +            - description: Low Frequency Oscillator (usually at 32kHz)
> +            - description: Internal Oscillator
> +            - description: System 24MHz Clock
> +            - description: Peripherals PLL 0 (200 MHz output)
> +            - description: Peripherals PLL 0 (300 MHz output)
> +            - description: Peripherals PLL 1 (300 MHz output)
> +
> +        clock-names:
> +          items:
> +            - const: hosc
> +            - const: losc
> +            - const: iosc
> +            - const: sys-24m
> +            - const: pll-periph0-200m
> +            - const: pll-periph0-300m
> +            - const: pll-periph1-300m
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    clock-controller@2002000 {
> +      compatible = "allwinner,sun60i-a733-ccu";
> +      reg = <0x02002000 0x2000>;
> +      clocks = <&rtc 2>, <&rtc 1>, <&rtc 0>, <&rtc 4>;
> +      clock-names = "hosc", "losc", "iosc", "losc-fanout";
> +      #clock-cells = <1>;
> +      #reset-cells = <1>;
> +    };
> +
> +...

This part looks correct.

> diff --git a/include/dt-bindings/clock/sun60i-a733-ccu.h b/include/dt-bindings/clock/sun60i-a733-ccu.h
> new file mode 100644
> index 000000000000..1a98bea8ca9a
> --- /dev/null
> +++ b/include/dt-bindings/clock/sun60i-a733-ccu.h
> @@ -0,0 +1,289 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (C) 2026 Junhui Liu <junhui.liu@pigmoral.tech>
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_SUN60I_A733_CCU_H_
> +#define _DT_BINDINGS_CLK_SUN60I_A733_CCU_H_
> +
> +#define CLK_PLL_REF            0
> +#define CLK_SYS_24M            1
> +#define CLK_PLL_DDR            2
> +#define CLK_PLL_PERIPH0_4X     3
> +#define CLK_PLL_PERIPH0_2X     4
> +#define CLK_PLL_PERIPH0_800M   5
> +#define CLK_PLL_PERIPH0_480M   6
> +#define CLK_PLL_PERIPH0_600M   7
> +#define CLK_PLL_PERIPH0_400M   8
> +#define CLK_PLL_PERIPH0_300M   9
> +#define CLK_PLL_PERIPH0_200M   10
> +#define CLK_PLL_PERIPH0_160M   11
> +#define CLK_PLL_PERIPH0_150M   12
> +#define CLK_PLL_PERIPH1_4X     13
> +#define CLK_PLL_PERIPH1_2X     14
> +#define CLK_PLL_PERIPH1_800M   15
> +#define CLK_PLL_PERIPH1_480M   16
> +#define CLK_PLL_PERIPH1_600M   17
> +#define CLK_PLL_PERIPH1_400M   18
> +#define CLK_PLL_PERIPH1_300M   19
> +#define CLK_PLL_PERIPH1_200M   20
> +#define CLK_PLL_PERIPH1_160M   21
> +#define CLK_PLL_PERIPH1_150M   22
> +#define CLK_PLL_GPU            23

Please keep the unit number even if there is only one instance.
This is PLL_GPU0.

> +#define CLK_PLL_VIDEO0_8X      24

If this is the common parent for the 4X and 3X dividers, shouldn't it
be 12X?

> +#define CLK_PLL_VIDEO0_4X      25
> +#define CLK_PLL_VIDEO0_3X      26

> +#define CLK_PLL_VIDEO1_8X      27

Same here.

> +#define CLK_PLL_VIDEO1_4X      28
> +#define CLK_PLL_VIDEO1_3X      29

> +#define CLK_PLL_VIDEO2_8X      30

And here.

> +#define CLK_PLL_VIDEO2_4X      31
> +#define CLK_PLL_VIDEO2_3X      32
> +#define CLK_PLL_VE0            33
> +#define CLK_PLL_VE1            34
> +#define CLK_PLL_AUDIO0_4X      35
> +#define CLK_PLL_AUDIO1         36
> +#define CLK_PLL_AUDIO1_DIV2    37
> +#define CLK_PLL_AUDIO1_DIV5    38
> +#define CLK_PLL_NPU            39

> +#define CLK_PLL_DE             40

Should be DE_12X?

> +#define CLK_PLL_DE_4X          41
> +#define CLK_PLL_DE_3X          42
> +#define CLK_AHB                        43
> +#define CLK_APB0               44
> +#define CLK_APB1               45
> +#define CLK_APB_UART           46
> +#define CLK_TRACE              47
> +#define CLK_GIC                        48
> +#define CLK_CPU_PERI           49
> +#define CLK_BUS_ITS_PCIE       50

Please preserve the _ACLK suffix.

Please keep the unit number even if there is only one instance.
Basically, use the naming from the user manual. This is ITS0.

> +#define CLK_NSI                        51
> +#define CLK_BUS_NSI            52

CLK_BUS_NSI_CFG

> +#define CLK_MBUS               53

> +#define CLK_MBUS_IOMMU0_SYS    54
> +#define CLK_APB_IOMMU0_SYS     55
> +#define CLK_AHB_IOMMU0_SYS     56

Please add a comment at the top describing the renaming to let the
reader know how to search for the in the user manual.

    PCLK -> APB
    HCLK -> AHB
    MCLK -> MBUS

And that the bus name was moved from the end of the name to the front.

> +#define CLK_BUS_MSI_LITE0      57
> +#define CLK_BUS_MSI_LITE1      58
> +#define CLK_BUS_MSI_LITE2      59
> +#define CLK_MBUS_IOMMU1_SYS    60
> +#define CLK_APB_IOMMU1_SYS     61
> +#define CLK_AHB_IOMMU1_SYS     62
> +#define CLK_AHB_VE_DEC         63
> +#define CLK_AHB_VE_ENC         64
> +#define CLK_AHB_VID_IN         65
> +#define CLK_AHB_VID_COUT0      66
> +#define CLK_AHB_VID_COUT1      67
> +#define CLK_AHB_DE             68
> +#define CLK_AHB_NPU            69
> +#define CLK_AHB_GPU0           70
> +#define CLK_AHB_SERDES         71
> +#define CLK_AHB_USB_SYS                72
> +#define CLK_AHB_MSI_LITE0      73
> +#define CLK_AHB_STORE          74
> +#define CLK_AHB_CPUS           75
> +#define CLK_MBUS_IOMMU0                76
> +#define CLK_MBUS_IOMMU1                77
> +#define CLK_MBUS_DESYS         78
> +#define CLK_MBUS_VE_ENC_GATE   79

Please keep the unit number even if there is only one instance.
This is VE_ENC0.

> +#define CLK_MBUS_VE_DEC_GATE   80
> +#define CLK_MBUS_GPU0          81
> +#define CLK_MBUS_NPU           82
> +#define CLK_MBUS_VID_IN                83
> +#define CLK_MBUS_SERDES                84
> +#define CLK_MBUS_MSI_LITE0     85
> +#define CLK_MBUS_STORE         86
> +#define CLK_MBUS_MSI_LITE2     87
> +#define CLK_MBUS_DMA0          88
> +#define CLK_MBUS_VE_ENC                89
> +#define CLK_MBUS_CE            90
> +#define CLK_MBUS_DMA1          91
> +#define CLK_MBUS_NAND          92
> +#define CLK_MBUS_CSI           93
> +#define CLK_MBUS_ISP           94
> +#define CLK_MBUS_GMAC0         95
> +#define CLK_MBUS_GMAC1         96

There is no GMAC1 in the A733 user manual v0.92. Please add a note about
where this came from.

> +#define CLK_MBUS_VE_DEC                97
> +#define CLK_BUS_DMA0           98
> +#define CLK_BUS_DMA1           99
> +#define CLK_BUS_SPINLOCK       100
> +#define CLK_BUS_MSGBOX         101

Please keep the unit number even if there is only one instance.
This is MSGBOX0.

> +#define CLK_BUS_PWM0           102
> +#define CLK_BUS_PWM1           103
> +#define CLK_BUS_DBG            104
> +#define CLK_BUS_SYSDAP         105
> +#define CLK_TIMER0             106
> +#define CLK_TIMER1             107
> +#define CLK_TIMER2             108
> +#define CLK_TIMER3             109
> +#define CLK_TIMER4             110
> +#define CLK_TIMER5             111
> +#define CLK_TIMER6             112
> +#define CLK_TIMER7             113
> +#define CLK_TIMER8             114
> +#define CLK_TIMER9             115
> +#define CLK_BUS_TIMER          116
> +#define CLK_AVS                        117

There is no AVS in the A733 user manual v0.92. Please add a note about
where this came from.

> +#define CLK_DE                 118

Please keep the unit number even if there is only one instance.
This is DE0.

> +#define CLK_BUS_DE             119

Same here.

> +#define CLK_DI                 120
> +#define CLK_BUS_DI             121
> +#define CLK_G2D                        122
> +#define CLK_BUS_G2D            123
> +#define CLK_EINK               124
> +#define CLK_EINK_PANEL         125
> +#define CLK_BUS_EINK           126
> +#define CLK_VE_ENC             127

Please keep the unit number even if there is only one instance.
This is VE_ENC0.

> +#define CLK_VE_DEC             128
> +#define CLK_BUS_VE_ENC         129

Same here.

> +#define CLK_BUS_VE_DEC         130
> +#define CLK_CE                 131
> +#define CLK_BUS_CE             132
> +#define CLK_BUS_CE_SYS         133
> +#define CLK_NPU                        134
> +#define CLK_BUS_NPU            135

> +#define CLK_GPU                        136
> +#define CLK_BUS_GPU            137

Please keep the unit number even if there is only one instance.
This is GPU0.

> +#define CLK_DRAM               138
> +#define CLK_BUS_DRAM           139

This is DRAM0.

> +#define CLK_NAND0              140

CLK_NAND0_CLK0

> +#define CLK_NAND1              141

CLK_NAND0_CLK1

> +#define CLK_BUS_NAND           142

CLK_BUS_NAND0

> +#define CLK_MMC0               143
> +#define CLK_MMC1               144
> +#define CLK_MMC2               145
> +#define CLK_MMC3               146
> +#define CLK_BUS_MMC0           147
> +#define CLK_BUS_MMC1           148
> +#define CLK_BUS_MMC2           149
> +#define CLK_BUS_MMC3           150

The ordering is wrong. The module clocks and bus clocks are interleaved.

MMC0, BUS_MMC0, MMC1, BUS_MMC1, ...

> +#define CLK_UFS_AXI            151
> +#define CLK_UFS_CFG            152
> +#define CLK_BUS_UFS            153
> +#define CLK_BUS_UART0          154
> +#define CLK_BUS_UART1          155
> +#define CLK_BUS_UART2          156
> +#define CLK_BUS_UART3          157
> +#define CLK_BUS_UART4          158
> +#define CLK_BUS_UART5          159
> +#define CLK_BUS_UART6          160
> +#define CLK_BUS_I2C0           161
> +#define CLK_BUS_I2C1           162
> +#define CLK_BUS_I2C2           163
> +#define CLK_BUS_I2C3           164
> +#define CLK_BUS_I2C4           165
> +#define CLK_BUS_I2C5           166
> +#define CLK_BUS_I2C6           167
> +#define CLK_BUS_I2C7           168
> +#define CLK_BUS_I2C8           169
> +#define CLK_BUS_I2C9           170
> +#define CLK_BUS_I2C10          171
> +#define CLK_BUS_I2C11          172
> +#define CLK_BUS_I2C12          173

> +#define CLK_SPI0               174
> +#define CLK_SPI1               175
> +#define CLK_SPI2               176
> +#define CLK_SPI3               177
> +#define CLK_SPI4               178
> +#define CLK_BUS_SPI0           179
> +#define CLK_BUS_SPI1           180
> +#define CLK_BUS_SPI2           181
> +#define CLK_BUS_SPI3           182
> +#define CLK_BUS_SPI4           183

Same as MMC ones, SPI module clocks and bus clocks are interleaved.

The user manual doesn't show SPI4. Please add a note saying where the
information is from.

> +#define CLK_SPIF               184
> +#define CLK_BUS_SPIF           185

SPIF comes before SPI3. Please keep the ordering based on the registers.

> +#define CLK_GPADC              186

CLK_GPADC_24M

> +#define CLK_BUS_GPADC          187

CLK_BUS_GPADC0

> +#define CLK_BUS_THS            188

CLK_BUS_THS0

> +#define CLK_IRRX               189
> +#define CLK_BUS_IRRX           190
> +#define CLK_IRTX               191
> +#define CLK_BUS_IRTX           192
> +#define CLK_BUS_LRADC          193


> +#define CLK_SGPIO              194
> +#define CLK_BUS_SGPIO          195
> +#define CLK_LPC                        196
> +#define CLK_BUS_LPC            197

These aren't found in the user manual. Please document where they were found.

> +#define CLK_I2SPCM0            198
> +#define CLK_I2SPCM1            199
> +#define CLK_I2SPCM2            200
> +#define CLK_I2SPCM3            201
> +#define CLK_I2SPCM4            202
> +#define CLK_BUS_I2SPCM0                203
> +#define CLK_BUS_I2SPCM1                204
> +#define CLK_BUS_I2SPCM2                205
> +#define CLK_BUS_I2SPCM3                206
> +#define CLK_BUS_I2SPCM4                207

Module clocks and bus clocks are interleaved.

> +#define CLK_I2SPCM2_ASRC       208

This would come before BUS_I2SPCM2.

> +#define CLK_OWA_TX             209
> +#define CLK_OWA_RX             210
> +#define CLK_BUS_OWA            211

s/OWA/SPDIF/

> +#define CLK_DMIC               212
> +#define CLK_BUS_DMIC           213
> +#define CLK_USB_OHCI0          214
> +#define CLK_BUS_OTG            215
> +#define CLK_BUS_EHCI0          216
> +#define CLK_BUS_OHCI0          217

Order should be OHCI0 -> EHCI0 -> OTG

> +#define CLK_USB_OHCI1          218
> +#define CLK_BUS_EHCI1          219
> +#define CLK_BUS_OHCI1          220

OHCI1, then EHCI1.

> +#define CLK_USB_REF            221

USB01_REF to keep the numbering but keeping it short.

> +#define CLK_USB2_U2_REF                222
> +#define CLK_USB2_SUSPEND       223
> +#define CLK_USB2_MF            224

> +#define CLK_USB2_U3_UTMI       225
> +#define CLK_USB2_U2_PIPE       226

Side-note:
I think the manual got this reversed, since UTMI is used for USB 2.0,
while PIPE is used for USB 3.0. But we will stick to whatever the user
manual says.

> +#define CLK_PCIE_AUX           227
> +#define CLK_PCIE_AXI_SLV       228
> +#define CLK_SERDES_PHY         229
> +#define CLK_GMAC_PTP           230
> +#define CLK_GMAC0_PHY          231
> +#define CLK_GMAC1_PHY          232
> +#define CLK_BUS_GMAC0          233

Based on your driver, BUS_GMAC0 should come before GMAC1_PHY.

> +#define CLK_BUS_GMAC1          234

There is no GMAC1 mentioned in the A733 manual. Please document where
you found this.

> +#define CLK_TCON_LCD0          235
> +#define CLK_TCON_LCD1          236
> +#define CLK_TCON_LCD2          237
> +#define CLK_BUS_TCON_LCD0      238
> +#define CLK_BUS_TCON_LCD1      239
> +#define CLK_BUS_TCON_LCD2      240

Module clocks are interleaved with bus clock gates.

> +#define CLK_DSI0               241
> +#define CLK_DSI1               242
> +#define CLK_BUS_DSI0           243
> +#define CLK_BUS_DSI1           244

Same for DSI.

> +#define CLK_COMBPHY0           245
> +#define CLK_COMBPHY1           246
> +#define CLK_BUS_TCON_TV0       247
> +#define CLK_BUS_TCON_TV1       248
> +#define CLK_EDP_TV             249
> +#define CLK_BUS_EDP_TV         250
> +#define CLK_HDMI_CEC_32K       251
> +#define CLK_HDMI_CEC           252
> +#define CLK_HDMI_TV            253
> +#define CLK_BUS_HDMI_TV                254
> +#define CLK_HDMI_SFR           255
> +#define CLK_HDMI_ESM           256

HDCP_ESM.

> +#define CLK_BUS_DPSS_TOP0      257
> +#define CLK_BUS_DPSS_TOP1      258
> +#define CLK_LEDC               259
> +#define CLK_BUS_LEDC           260
> +#define CLK_BUS_DSC            261
> +#define CLK_CSI_MASTER0                262
> +#define CLK_CSI_MASTER1                263
> +#define CLK_CSI_MASTER2                264
> +#define CLK_CSI                        265
> +#define CLK_BUS_CSI            266
> +#define CLK_ISP                        267

I guess we don't need to add any of the PLL output gates since they are
auto-gated?

Also, there is a RES_DCAP_24M_GATE at 0x1a00. Any idea about this one?

> +#define CLK_APB2JTAG           268
> +#define CLK_FANOUT_24M         269
> +#define CLK_FANOUT_12M         270
> +#define CLK_FANOUT_16M         271
> +#define CLK_FANOUT_25M         272
> +#define CLK_FANOUT_27M         273
> +#define CLK_FANOUT_PCLK                274
> +#define CLK_FANOUT0            275
> +#define CLK_FANOUT1            276
> +#define CLK_FANOUT2            277
> +#define CLK_FANOUT3            278

The rest are fine.

> +#endif /* _DT_BINDINGS_CLK_SUN60I_A733_CCU_H_ */
> diff --git a/include/dt-bindings/clock/sun60i-a733-r-ccu.h b/include/dt-bindings/clock/sun60i-a733-r-ccu.h
> new file mode 100644
> index 000000000000..1c717cc588b8
> --- /dev/null
> +++ b/include/dt-bindings/clock/sun60i-a733-r-ccu.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (C) 2026 Junhui Liu <junhui.liu@pigmoral.tech>
> + */
> +
> +#ifndef _DT_BINDINGS_CLK_SUN60I_A733_R_CCU_H_
> +#define _DT_BINDINGS_CLK_SUN60I_A733_R_CCU_H_
> +
> +#define CLK_R_AHB              0
> +#define CLK_R_APB0             1
> +#define CLK_R_APB1             2
> +#define CLK_R_TIMER0           3
> +#define CLK_R_TIMER1           4
> +#define CLK_R_TIMER2           5
> +#define CLK_R_TIMER3           6
> +#define CLK_BUS_R_TIMER                7
> +#define CLK_BUS_R_TWD          8

> +#define CLK_R_PWMCTRL          9
> +#define CLK_BUS_R_PWMCTRL      10

*_R_PWM.

> +#define CLK_R_SPI              11
> +#define CLK_BUS_R_SPI          12
> +#define CLK_BUS_R_MSGBOX       13
> +#define CLK_BUS_R_UART0                14
> +#define CLK_BUS_R_UART1                15
> +#define CLK_BUS_R_I2C0         16
> +#define CLK_BUS_R_I2C1         17
> +#define CLK_BUS_R_I2C2         18
> +#define CLK_BUS_R_PPU          19
> +#define CLK_BUS_R_TZMA         20
> +#define CLK_BUS_R_CPU_BIST     21
> +#define CLK_R_IR_RX            22
> +#define CLK_BUS_R_IR_RX                23
> +#define CLK_BUS_R_RTC          24
> +#define CLK_R_RISCV            25
> +#define CLK_BUS_R_RISCV                26
> +#define CLK_BUS_R_RISCV_CFG    27
> +#define CLK_BUS_R_CPUCFG       28
> +
> +#endif /* _DT_BINDINGS_CLK_SUN60I_A733_R_CCU_H_ */
> diff --git a/include/dt-bindings/reset/sun60i-a733-ccu.h b/include/dt-bindings/reset/sun60i-a733-ccu.h
> new file mode 100644
> index 000000000000..11ce78cf04dd
> --- /dev/null
> +++ b/include/dt-bindings/reset/sun60i-a733-ccu.h
> @@ -0,0 +1,131 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (C) 2026 Junhui Liu <junhui.liu@pigmoral.tech>
> + */
> +
> +#ifndef _DT_BINDINGS_RST_SUN60I_A733_CCU_H_
> +#define _DT_BINDINGS_RST_SUN60I_A733_CCU_H_
> +
> +#define RST_BUS_ITS_PCIE       0
> +#define RST_BUS_NSI            1
> +#define RST_BUS_NSI_CFG                2
> +#define RST_BUS_IOMMU0_SYS     3
> +#define RST_BUS_MSI_LITE0_AHB  4
> +#define RST_BUS_MSI_LITE0_MBUS 5
> +#define RST_BUS_MSI_LITE1_AHB  6
> +#define RST_BUS_MSI_LITE1_MBUS 7
> +#define RST_BUS_MSI_LITE2_AHB  8
> +#define RST_BUS_MSI_LITE2_MBUS 9
> +#define RST_BUS_IOMMU1_SYS     10
> +#define RST_BUS_DMA0           11
> +#define RST_BUS_DMA1           12
> +#define RST_BUS_SPINLOCK       13
> +#define RST_BUS_MSGBOX         14
> +#define RST_BUS_PWM0           15
> +#define RST_BUS_PWM1           16
> +#define RST_BUS_DBG            17
> +#define RST_BUS_SYSDAP         18
> +#define RST_BUS_TIMER0         19
> +#define RST_BUS_DE             20

Please keep the unit number even if there is only one instance.
This is DE0.

> +#define RST_BUS_DI             21
> +#define RST_BUS_G2D            22
> +#define RST_BUS_EINK           23
> +#define RST_BUS_DE_SYS         24
> +#define RST_BUS_VE_ENC         25

Please keep the unit number even if there is only one instance.
This is VE_ENC0.

> +#define RST_BUS_VE_DEC         26
> +#define RST_BUS_CE             27
> +#define RST_BUS_CE_SYS         28
> +#define RST_BUS_NPU_CORE       29
> +#define RST_BUS_NPU_AXI                30
> +#define RST_BUS_NPU_AHB                31
> +#define RST_BUS_NPU_SRAM       32
> +#define RST_BUS_GPU            33
> +#define RST_BUS_DRAM           34
> +#define RST_BUS_NAND           35
> +#define RST_BUS_MMC0           36
> +#define RST_BUS_MMC1           37
> +#define RST_BUS_MMC2           38
> +#define RST_BUS_MMC3           39
> +#define RST_BUS_UFS_AHB                40
> +#define RST_BUS_UFS_AXI                41
> +#define RST_BUS_UFS_PHY                42
> +#define RST_BUS_UFS_CORE       43
> +#define RST_BUS_UART0          44
> +#define RST_BUS_UART1          45
> +#define RST_BUS_UART2          46
> +#define RST_BUS_UART3          47
> +#define RST_BUS_UART4          48
> +#define RST_BUS_UART5          49
> +#define RST_BUS_UART6          50
> +#define RST_BUS_I2C0           51
> +#define RST_BUS_I2C1           52
> +#define RST_BUS_I2C2           53
> +#define RST_BUS_I2C3           54
> +#define RST_BUS_I2C4           55
> +#define RST_BUS_I2C5           56
> +#define RST_BUS_I2C6           57
> +#define RST_BUS_I2C7           58
> +#define RST_BUS_I2C8           59
> +#define RST_BUS_I2C9           60
> +#define RST_BUS_I2C10          61
> +#define RST_BUS_I2C11          62
> +#define RST_BUS_I2C12          63
> +#define RST_BUS_SPI0           64
> +#define RST_BUS_SPI1           65
> +#define RST_BUS_SPI2           66
> +#define RST_BUS_SPIF           67

Here you have SPIF in the correct position.

> +#define RST_BUS_SPI3           68
> +#define RST_BUS_SPI4           69

Please add a note saying where SPI4 was found.

> +#define RST_BUS_GPADC          70

RST_BUS_GPADC0

> +#define RST_BUS_THS            71

RST_BUS_THS0

> +#define RST_BUS_IRRX           72
> +#define RST_BUS_IRTX           73
> +#define RST_BUS_LRADC          74
> +#define RST_BUS_SGPIO          75
> +#define RST_BUS_LPC            76
> +#define RST_BUS_I2SPCM0                77
> +#define RST_BUS_I2SPCM1                78
> +#define RST_BUS_I2SPCM2                79
> +#define RST_BUS_I2SPCM3                80
> +#define RST_BUS_I2SPCM4                81
> +#define RST_BUS_OWA            82
> +#define RST_BUS_DMIC           83
> +#define RST_USB_PHY0           84
> +#define RST_BUS_OHCI0          85
> +#define RST_BUS_EHCI0          86
> +#define RST_BUS_OTG            87
> +#define RST_USB_PHY1           88
> +#define RST_BUS_OHCI1          89
> +#define RST_BUS_EHCI1          90
> +#define RST_BUS_USB2           91

> +#define RST_BUS_PCIE           92
> +#define RST_BUS_PCIE_PWRUP     93

PCIE_PWRUP comes before PCIE.

> +#define RST_BUS_SERDES         94
> +#define RST_BUS_GMAC0          95
> +#define RST_BUS_GMAC0_AXI      96
> +#define RST_BUS_GMAC1          97
> +#define RST_BUS_GMAC1_AXI      98
> +#define RST_BUS_TCON_LCD0      99
> +#define RST_BUS_TCON_LCD1      100
> +#define RST_BUS_TCON_LCD2      101
> +#define RST_BUS_LVDS0          102
> +#define RST_BUS_LVDS1          103
> +#define RST_BUS_DSI0           104
> +#define RST_BUS_DSI1           105
> +#define RST_BUS_TCON_TV0       106
> +#define RST_BUS_TCON_TV1       107
> +#define RST_BUS_EDP            108
> +#define RST_BUS_HDMI_MAIN      109
> +#define RST_BUS_HDMI_SUB       110
> +#define RST_BUS_HDMI_HDCP      111
> +#define RST_BUS_DPSS_TOP0      112
> +#define RST_BUS_DPSS_TOP1      113
> +#define RST_BUS_VIDEO_OUT0     114
> +#define RST_BUS_VIDEO_OUT1     115
> +#define RST_BUS_LEDC           116
> +#define RST_BUS_DSC            117
> +#define RST_BUS_CSI            118
> +#define RST_BUS_VIDEO_IN       119
> +#define RST_BUS_APB2JTAG       120
> +

The rest look correct.

> +#endif /* _DT_BINDINGS_RST_SUN60I_A733_CCU_H_ */
> diff --git a/include/dt-bindings/reset/sun60i-a733-r-ccu.h b/include/dt-bindings/reset/sun60i-a733-r-ccu.h
> new file mode 100644
> index 000000000000..629e546d1998
> --- /dev/null
> +++ b/include/dt-bindings/reset/sun60i-a733-r-ccu.h
> @@ -0,0 +1,23 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (C) 2026 Junhui Liu <junhui.liu@pigmoral.tech>
> + */
> +
> +#ifndef _DT_BINDINGS_RST_SUN60I_A733_R_CCU_H_
> +#define _DT_BINDINGS_RST_SUN60I_A733_R_CCU_H_
> +
> +#define RST_BUS_R_TIMER                0
> +#define RST_BUS_R_PWM          1
> +#define RST_BUS_R_SPI          2
> +#define RST_BUS_R_MSGBOX       3
> +#define RST_BUS_R_UART0                4
> +#define RST_BUS_R_UART1                5
> +#define RST_BUS_R_I2C0         6
> +#define RST_BUS_R_I2C1         7
> +#define RST_BUS_R_I2C2         8
> +#define RST_BUS_R_IR_RX                9
> +#define RST_BUS_R_RTC          10
> +#define RST_BUS_R_RISCV_CFG    11
> +#define RST_BUS_R_CPUCFG       12
> +
> +#endif /* _DT_BINDINGS_RST_SUN60I_A733_R_CCU_H_ */

This part looks correct.


ChenYu

^ permalink raw reply

* [PATCH] net: stmmac: dwmac-rk: Fix typo in comment
From: 谢致邦 (XIE Zhibang) @ 2026-03-28 13:43 UTC (permalink / raw)
  To: linux-rockchip
  Cc: 谢致邦 (XIE Zhibang), Heiko Stuebner,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
	Russell King (Oracle), linux-arm-kernel, netdev, linux-stm32,
	linux-kernel

Correct the typo "rk3520" to "rk3528" in comment.

Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
index b0441a368cb1..8d7042e68926 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
@@ -214,7 +214,7 @@ static int rk_configure_io_clksel(struct rk_priv_data *bsp_priv)
 	cru = !io;
 
 	/* The io_clksel configuration can be either:
-	 *  0=CRU, 1=IO (rk3506, rk3520, rk3576) or
+	 *  0=CRU, 1=IO (rk3506, rk3528, rk3576) or
 	 *  0=IO, 1=CRU (rk3588)
 	 * where CRU means the transmit clock comes from the CRU and IO
 	 * means the transmit clock comes from IO.
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Andrew Lunn @ 2026-03-28 14:15 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Eric Woudstra, Lucien.Jheng, hkallweit1, davem, edumazet, kuba,
	pabeni, netdev, linux-kernel, bjorn, frank-w, daniel,
	lucien.jheng
In-Reply-To: <acesgklpv8GlNeVf@shell.armlinux.org.uk>

On Sat, Mar 28, 2026 at 10:25:06AM +0000, Russell King (Oracle) wrote:
> On Sat, Mar 28, 2026 at 11:21:59AM +0100, Eric Woudstra wrote:
> > static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> > 			      u32 pbus_data)
> > {
> > 	int ret;
> > 
> > 	mutex_lock(&mdio->bus->mdio_lock);
> 
> I wonder why we have phy_lock_mdio_bus() but not mdio_bus_lock()...
> Should a helper be provided at mdio bus level to complement the PHY
> level helper if we're going to have MDIO drivers directly manipulating
> the lock?

DSA is pretty much the only class of mdio driver. And they mostly need
to use:

mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);

Not the plain mutex_lock(). 

Hence it has not been added up until now.

But yes, such a helper would make sense for a PHY driver having to do
odd things.

	Andrew


^ permalink raw reply

* Re: [PATCH v2] net: stmmac: avoid repeated devm_gpiod_get_optional in mdio
From: Andrew Lunn @ 2026-03-28 14:31 UTC (permalink / raw)
  To: Liu Xiang; +Cc: netdev, andrew+netdev, davem, edumazet, kuba, pabeni
In-Reply-To: <20260328032520.7866-1-liu.xiang@zlingsmart.com>

On Sat, Mar 28, 2026 at 11:25:20AM +0800, Liu Xiang wrote:
> During system resume, stmmac_resume() calls stmmac_mdio_reset().
> This results in repeated calls to devm_gpiod_get_optional() for the same
> GPIO. The second invocation returns an error since the GPIO descriptor has
> already been obtained.

How is this different to v1?

> @@ -386,15 +386,15 @@ int stmmac_mdio_reset(struct mii_bus *bus)
>  
>  #ifdef CONFIG_OF
>  	if (priv->device->of_node) {
> -		struct gpio_desc *reset_gpio;
>  		u32 delays[3] = { 0, 0, 0 };
>  
> -		reset_gpio = devm_gpiod_get_optional(priv->device,
> -						     "snps,reset",
> -						     GPIOD_OUT_LOW);
> -		if (IS_ERR(reset_gpio))
> -			return PTR_ERR(reset_gpio);
> -
> +		if (IS_ERR_OR_NULL(priv->reset_gpio)) {
> +			priv->reset_gpio = devm_gpiod_get_optional(priv->device,
> +								   "snps,reset",
> +								 GPIOD_OUT_LOW);
> +			if (IS_ERR(priv->reset_gpio))
> +				return PTR_ERR(priv->reset_gpio);
> +		}

In general, a driver should get the resources it needs in probe, since
probe can only be successful once. So i suggest moving this into
stmmac_mdio_register() which is the equivalent of probe. To keep the
behaviour the same, pass GPIOD_ASIS, and then here in _reset set it
low.

	Andrew

^ permalink raw reply

* Re: [PATCH] net/ftgmac100: fix ring allocation unwind leaks on open failure
From: Andrew Lunn @ 2026-03-28 14:37 UTC (permalink / raw)
  To: Yufan Chen
  Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel, Yufan Chen
In-Reply-To: <20260328092428.75430-1-yufan.chen@linux.dev>

On Sat, Mar 28, 2026 at 05:24:28PM +0800, Yufan Chen wrote:
> From: Yufan Chen <ericterminal@gmail.com>
> 
> ftgmac100_alloc_rings() allocated rx_skbs, tx_skbs, rxdes, txdes, and rx_scratch in stages but returned directly on any intermediate allocation failure. This left previously allocated objects unreleased and could accumulate leaks across repeated ifup retries under memory pressure.

Please take a read of

https://docs.kernel.org/process/submitting-patches.html

and

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html


You have a number of process issues.

>  static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
>  {
> +	int err = -ENOMEM;
> +
>  	/* Allocate skb arrays */
>  	priv->rx_skbs = kcalloc(MAX_RX_QUEUE_ENTRIES, sizeof(void *),
>  				GFP_KERNEL);
>  	if (!priv->rx_skbs)
> -		return -ENOMEM;
> +		goto out;
>  	priv->tx_skbs = kcalloc(MAX_TX_QUEUE_ENTRIES, sizeof(void *),
>  				GFP_KERNEL);
>  	if (!priv->tx_skbs)
> -		return -ENOMEM;
> +		goto out;
>  
>  	/* Allocate descriptors */
>  	priv->rxdes = dma_alloc_coherent(priv->dev,
>  					 MAX_RX_QUEUE_ENTRIES * sizeof(struct ftgmac100_rxdes),
>  					 &priv->rxdes_dma, GFP_KERNEL);
>  	if (!priv->rxdes)
> -		return -ENOMEM;
> +		goto out;
>  	priv->txdes = dma_alloc_coherent(priv->dev,
>  					 MAX_TX_QUEUE_ENTRIES * sizeof(struct ftgmac100_txdes),
>  					 &priv->txdes_dma, GFP_KERNEL);
>  	if (!priv->txdes)
> -		return -ENOMEM;
> +		goto out;
>  
>  	/* Allocate scratch packet buffer */
>  	priv->rx_scratch = dma_alloc_coherent(priv->dev,
> @@ -997,9 +1002,13 @@ static int ftgmac100_alloc_rings(struct ftgmac100 *priv)
>  					      &priv->rx_scratch_dma,
>  					      GFP_KERNEL);
>  	if (!priv->rx_scratch)
> -		return -ENOMEM;
> +		goto out;
>  
>  	return 0;
> +
> +out:
> +	ftgmac100_free_rings(priv);
> +	return err;

If you look at other drivers, you will notice that functions that do
allocations pretty much always do their own cleanup, rather than
calling a helper. You generally see multiple labels, one per
allocation, and gotos jumping to the needed code.

    Andrew

---
pw-bot: cr

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox