* Re: [PATCH RFC v3 09/11] platform/x86: ideapad-laptop: Decouple hardware & classdev brightness for keyboard backlight
From: Ilpo Järvinen @ 2026-07-21 17:08 UTC (permalink / raw)
To: Rong Zhang
Cc: Lee Jones, Pavel Machek, Jonathan Corbet, Shuah Khan,
Thomas Weißschuh, Benson Leung, Guenter Roeck,
Marek Behún, Mark Pearson, Derek J. Clark, Hans de Goede,
Ike Panhc, Andrew Lunn, Jakub Kicinski, Vishnu Sankar,
Vishnu Sankar, linux-leds, Netdev, linux-doc, LKML,
chrome-platform, platform-driver-x86
In-Reply-To: <20260719-leds-trigger-hw-changed-v3-9-5fb55722e36e@rong.moe>
On Sun, 19 Jul 2026, Rong Zhang wrote:
> Some recent models come with an ambient light sensor (ALS). On these
> models, their EC will automatically set the keyboard backlight to an
> appropriate brightness when the effective "hardware brightness" is 3.
> "Hardware brightness" can't be perfectly mapped to an LED classdev
> brightness, but the EC does use this predefined brightness value to
> represent auto mode.
>
> Currently, the code processing keyboard backlight is coupled with LED
> classdev, making it hard to expose the auto brightness (ALS) mode to the
> userspace.
>
> As the first step toward the goal, decouple hardware brightness from LED
> classdev brightness, and update comments about corresponding backlight
> modes.
>
> Since upcoming changes will heavily rely on kbd_bl.last_hw_brightness,
> also convert it into an atomic_t to prevent potential race conditions.
>
> To minimalize the diff set in upcoming changes, a trivial refactor
> also converts the initialization path into another equivalent form.
>
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
> drivers/platform/x86/lenovo/Kconfig | 1 +
> drivers/platform/x86/lenovo/ideapad-laptop.c | 144 ++++++++++++++++++---------
> 2 files changed, 100 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/Kconfig b/drivers/platform/x86/lenovo/Kconfig
> index 4443f40ef8aa..e92b1e900795 100644
> --- a/drivers/platform/x86/lenovo/Kconfig
> +++ b/drivers/platform/x86/lenovo/Kconfig
> @@ -16,6 +16,7 @@ config IDEAPAD_LAPTOP
> select INPUT_SPARSEKMAP
> select NEW_LEDS
> select LEDS_CLASS
> + select LEDS_TRIGGERS
> help
> This is a driver for Lenovo IdeaPad netbooks contains drivers for
> rfkill switch, hotkey, fan control and backlight control.
> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index 4fbc904f1fc3..5aa2fedb8472 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> @@ -9,6 +9,7 @@
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> #include <linux/acpi.h>
> +#include <linux/atomic.h>
> #include <linux/backlight.h>
> #include <linux/bitfield.h>
> #include <linux/bitops.h>
> @@ -134,10 +135,31 @@ enum {
> };
>
> /*
> - * These correspond to the number of supported states - 1
> - * Future keyboard types may need a new system, if there's a collision
> - * KBD_BL_TRISTATE_AUTO has no way to report or set the auto state
> - * so it effectively has 3 states, but needs to handle 4
> + * The enumeration has two purposes:
> + * - as an internal identifier for all known types of keyboard backlight
> + * - as a mandatory parameter of the KBLC command
> + *
> + * For each type, the hardware brightness values are defined as follows:
> + * +--------------------------+----------+-----+------+------+
> + * | Hardware brightness | 0 | 1 | 2 | 3 |
> + * | Type | | | | |
> + * +--------------------------+----------+-----+------+------+
> + * | KBD_BL_STANDARD | off | on | N/A | N/A |
> + * +--------------------------+----------+-----+------+------+
> + * | KBD_BL_TRISTATE | off | low | high | N/A |
> + * +--------------------------+----------+-----+------+------+
> + * | KBD_BL_TRISTATE_AUTO | off | low | high | auto |
> + * +--------------------------+----------+-----+------+------+
> + *
> + * We map LED classdev brightness for KBD_BL_TRISTATE_AUTO as follows:
> + * +--------------------------+----------+-----+------+
> + * | LED classdev brightness | 0 | 1 | 2 |
> + * | Operation | | | |
> + * +--------------------------+----------+-----+------+
> + * | Read | off/auto | low | high |
> + * +--------------------------+----------+-----+------+
> + * | Write | off | low | high |
> + * +--------------------------+----------+-----+------+
> */
> enum {
> KBD_BL_STANDARD = 1,
> @@ -145,6 +167,8 @@ enum {
> KBD_BL_TRISTATE_AUTO = 3,
> };
>
> +#define KBD_BL_AUTO_MODE_HW_BRIGHTNESS 3
> +
> #define KBD_BL_QUERY_TYPE 0x1
> #define KBD_BL_TRISTATE_TYPE 0x5
> #define KBD_BL_TRISTATE_AUTO_TYPE 0x7
> @@ -203,7 +227,7 @@ struct ideapad_private {
> bool initialized;
> int type;
> struct led_classdev led;
> - unsigned int last_brightness;
> + atomic_t last_hw_brightness;
> } kbd_bl;
> struct {
> bool initialized;
> @@ -1592,7 +1616,24 @@ static int ideapad_kbd_bl_check_tristate(int type)
> return (type == KBD_BL_TRISTATE) || (type == KBD_BL_TRISTATE_AUTO);
> }
>
> -static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
> +static int ideapad_kbd_bl_brightness_parse(struct ideapad_private *priv, int hw_brightness)
> +{
> + /* Off, low or high */
> + if (hw_brightness <= priv->kbd_bl.led.max_brightness)
> + return hw_brightness;
> +
> + /* Auto (controlled by EC according to ALS), report as off */
> + if (priv->kbd_bl.type == KBD_BL_TRISTATE_AUTO &&
> + hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS)
> + return 0;
> +
> + /* Unknown value */
> + dev_warn(&priv->platform_device->dev,
Please (finally) add the include.
> + "Unknown keyboard backlight value: %d", hw_brightness);
> + return -EINVAL;
> +}
> +
> +static int ideapad_kbd_bl_hw_brightness_get(struct ideapad_private *priv)
> {
> unsigned long value;
> int err;
> @@ -1606,21 +1647,7 @@ static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
> if (err)
> return err;
>
> - /* Convert returned value to brightness level */
> - value = FIELD_GET(KBD_BL_GET_BRIGHTNESS, value);
> -
> - /* Off, low or high */
> - if (value <= priv->kbd_bl.led.max_brightness)
> - return value;
> -
> - /* Auto, report as off */
> - if (value == priv->kbd_bl.led.max_brightness + 1)
> - return 0;
> -
> - /* Unknown value */
> - dev_warn(&priv->platform_device->dev,
> - "Unknown keyboard backlight value: %lu", value);
> - return -EINVAL;
> + return FIELD_GET(KBD_BL_GET_BRIGHTNESS, value);
> }
>
> err = eval_hals(priv->adev->handle, &value);
> @@ -1630,6 +1657,16 @@ static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
> return !!test_bit(HALS_KBD_BL_STATE_BIT, &value);
> }
>
> +static int ideapad_kbd_bl_brightness_get(struct ideapad_private *priv)
> +{
> + int hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> +
> + if (hw_brightness < 0)
> + return hw_brightness;
> +
> + return ideapad_kbd_bl_brightness_parse(priv, hw_brightness);
> +}
> +
> static enum led_brightness ideapad_kbd_bl_led_cdev_brightness_get(struct led_classdev *led_cdev)
> {
> struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
> @@ -1637,32 +1674,37 @@ static enum led_brightness ideapad_kbd_bl_led_cdev_brightness_get(struct led_cla
> return ideapad_kbd_bl_brightness_get(priv);
> }
>
> -static int ideapad_kbd_bl_brightness_set(struct ideapad_private *priv, unsigned int brightness)
> +static int ideapad_kbd_bl_hw_brightness_set(struct ideapad_private *priv, int hw_brightness)
> {
> - int err;
> unsigned long value;
> int type = priv->kbd_bl.type;
> + int err;
>
> if (ideapad_kbd_bl_check_tristate(type)) {
> - if (brightness > priv->kbd_bl.led.max_brightness)
> - return -EINVAL;
> -
> - value = FIELD_PREP(KBD_BL_SET_BRIGHTNESS, brightness) |
> + value = FIELD_PREP(KBD_BL_SET_BRIGHTNESS, hw_brightness) |
> FIELD_PREP(KBD_BL_COMMAND_TYPE, type) |
> KBD_BL_COMMAND_SET;
> err = exec_kblc(priv->adev->handle, value);
> } else {
> - err = exec_sals(priv->adev->handle, brightness ? SALS_KBD_BL_ON : SALS_KBD_BL_OFF);
> + value = hw_brightness ? SALS_KBD_BL_ON : SALS_KBD_BL_OFF;
> + err = exec_sals(priv->adev->handle, value);
> }
> -
> if (err)
> return err;
>
> - priv->kbd_bl.last_brightness = brightness;
> + atomic_set(&priv->kbd_bl.last_hw_brightness, hw_brightness);
>
> return 0;
> }
>
> +static int ideapad_kbd_bl_brightness_set(struct ideapad_private *priv, int brightness)
> +{
> + if (brightness > priv->kbd_bl.led.max_brightness)
> + return -EINVAL;
> +
> + return ideapad_kbd_bl_hw_brightness_set(priv, brightness);
> +}
> +
> static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
> enum led_brightness brightness)
> {
> @@ -1673,26 +1715,29 @@ static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
>
> static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
> {
> - int brightness;
> + int hw_brightness, brightness, last_hw_brightness;
>
> if (!priv->kbd_bl.initialized)
> return;
>
> - brightness = ideapad_kbd_bl_brightness_get(priv);
> - if (brightness < 0)
> + hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> + if (hw_brightness < 0)
> return;
>
> - if (brightness == priv->kbd_bl.last_brightness)
> - return;
> + brightness = ideapad_kbd_bl_brightness_parse(priv, hw_brightness);
> + if (brightness < 0)
> + return; /* Reject insane values early. */
>
> - priv->kbd_bl.last_brightness = brightness;
> + last_hw_brightness = atomic_xchg(&priv->kbd_bl.last_hw_brightness, hw_brightness);
> + if (hw_brightness == last_hw_brightness)
> + return;
>
> led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
> }
>
> static int ideapad_kbd_bl_init(struct ideapad_private *priv)
> {
> - int brightness, err;
> + int hw_brightness, err;
>
> if (!priv->features.kbd_bl)
> return -ENODEV;
> @@ -1700,21 +1745,30 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
> if (WARN_ON(priv->kbd_bl.initialized))
> return -EEXIST;
>
> - if (ideapad_kbd_bl_check_tristate(priv->kbd_bl.type))
> - priv->kbd_bl.led.max_brightness = 2;
> - else
> - priv->kbd_bl.led.max_brightness = 1;
> + hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> + if (hw_brightness < 0)
> + return hw_brightness;
>
> - brightness = ideapad_kbd_bl_brightness_get(priv);
> - if (brightness < 0)
> - return brightness;
> + atomic_set(&priv->kbd_bl.last_hw_brightness, hw_brightness);
>
> - priv->kbd_bl.last_brightness = brightness;
> priv->kbd_bl.led.name = "platform::" LED_FUNCTION_KBD_BACKLIGHT;
> priv->kbd_bl.led.brightness_get = ideapad_kbd_bl_led_cdev_brightness_get;
> priv->kbd_bl.led.brightness_set_blocking = ideapad_kbd_bl_led_cdev_brightness_set;
> priv->kbd_bl.led.flags = LED_BRIGHT_HW_CHANGED | LED_RETAIN_AT_SHUTDOWN;
>
> + switch (priv->kbd_bl.type) {
> + case KBD_BL_TRISTATE_AUTO:
> + case KBD_BL_TRISTATE:
> + priv->kbd_bl.led.max_brightness = 2;
> + break;
> + case KBD_BL_STANDARD:
> + priv->kbd_bl.led.max_brightness = 1;
> + break;
> + default:
> + /* This has already been validated by ideapad_check_features(). */
> + unreachable();
Please add include.
> + }
> +
> err = led_classdev_register(&priv->platform_device->dev, &priv->kbd_bl.led);
> if (err)
> return err;
>
>
--
i.
^ permalink raw reply
* Re: [PATCH RFC v3 10/11] platform/x86: ideapad-laptop: Serialize keyboard backlight notifications
From: Ilpo Järvinen @ 2026-07-21 17:09 UTC (permalink / raw)
To: Rong Zhang
Cc: Lee Jones, Pavel Machek, Jonathan Corbet, Shuah Khan,
Thomas Weißschuh, Benson Leung, Guenter Roeck,
Marek Behún, Mark Pearson, Derek J. Clark, Hans de Goede,
Ike Panhc, Andrew Lunn, Jakub Kicinski, Vishnu Sankar,
Vishnu Sankar, linux-leds, Netdev, linux-doc, LKML,
chrome-platform, platform-driver-x86
In-Reply-To: <20260719-leds-trigger-hw-changed-v3-10-5fb55722e36e@rong.moe>
On Sun, 19 Jul 2026, Rong Zhang wrote:
> ACPI notifications are delivered in dedicated work contexts and may
> arrive simultaneously. In the following change, much work will be done
> while handling the notification, which could lead to potential race
> conditions.
>
> Introduce a new mutex to serialize keyboard backlight notifications to
> prevent potential race conditions.
>
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
> drivers/platform/x86/lenovo/ideapad-laptop.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index 5aa2fedb8472..66e16abda5e3 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> @@ -26,7 +26,9 @@
> #include <linux/jiffies.h>
> #include <linux/kernel.h>
> #include <linux/leds.h>
> +#include <linux/lockdep.h>
Why is this being added?
> #include <linux/module.h>
> +#include <linux/mutex.h>
> #include <linux/platform_device.h>
> #include <linux/platform_profile.h>
> #include <linux/power_supply.h>
> @@ -228,6 +230,8 @@ struct ideapad_private {
> int type;
> struct led_classdev led;
> atomic_t last_hw_brightness;
> +
> + struct mutex notif_mutex; /* protects notifications */
> } kbd_bl;
> struct {
> bool initialized;
> @@ -1720,6 +1724,8 @@ static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
> if (!priv->kbd_bl.initialized)
> return;
>
> + guard(mutex)(&priv->kbd_bl.notif_mutex);
> +
> hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> if (hw_brightness < 0)
> return;
> @@ -1745,6 +1751,10 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
> if (WARN_ON(priv->kbd_bl.initialized))
> return -EEXIST;
>
> + err = devm_mutex_init(&priv->platform_device->dev, &priv->kbd_bl.notif_mutex);
> + if (err)
> + return err;
> +
> hw_brightness = ideapad_kbd_bl_hw_brightness_get(priv);
> if (hw_brightness < 0)
> return hw_brightness;
>
>
--
i.
^ permalink raw reply
* Re: [PATCH RFC v3 11/11] platform/x86: ideapad-laptop: Fully support auto keyboard backlight
From: Ilpo Järvinen @ 2026-07-21 17:14 UTC (permalink / raw)
To: Rong Zhang
Cc: Lee Jones, Pavel Machek, Jonathan Corbet, Shuah Khan,
Thomas Weißschuh, Benson Leung, Guenter Roeck,
Marek Behún, Mark Pearson, Derek J. Clark, Hans de Goede,
Ike Panhc, Andrew Lunn, Jakub Kicinski, Vishnu Sankar,
Vishnu Sankar, linux-leds, Netdev, linux-doc, LKML,
chrome-platform, platform-driver-x86
In-Reply-To: <20260719-leds-trigger-hw-changed-v3-11-5fb55722e36e@rong.moe>
On Sun, 19 Jul 2026, Rong Zhang wrote:
> Currently, the auto brightness mode of keyboard backlight maps to
> brightness=0 in LED classdev. The only method to switch to such a mode
> is by pressing the manufacturer-defined shortcut (Fn+Space). However, 0
> is a multiplexed brightness value; writing 0 simply results in the
> backlight being turned off.
>
> With brightness processing code decoupled from LED classdev, we can now
> fully support the auto brightness mode. In this mode, the keyboard
> backlight is controlled by the EC according to the ambient light sensor
> (ALS).
>
> To utilize this, a private hardware control trigger "ideapad-auto" is
> added, with the event handling procedure calling the
> led_trigger_notify_hw_control_changed() interface to activate/deactivate
> the private trigger according to the current LED trigger state.
>
> Meanwhile, block brightness changes on exit to prevent the side effect
> of LED device unregistration when the private trigger is active from
> resetting the brightness to zero, so that we can retain the state of
> auto mode among boots.
>
> Signed-off-by: Rong Zhang <i@rong.moe>
> ---
> Changes in v3:
> - Address concerns from Sashiko
> - Fix a race condition in ideapad_kbd_bl_led_cdev_brightness_set()
> - Fix trigger re-registration of ideapad_kbd_bl_auto_trigger
> - https://sashiko.dev/#/patchset/20260618-leds-trigger-hw-changed-v2-0-c28c44053cf3%40rong.moe
> - Make registration failures of ideapad_kbd_bl_auto_trigger non-fatal
> ---
> drivers/platform/x86/lenovo/ideapad-laptop.c | 112 ++++++++++++++++++++++++---
> 1 file changed, 103 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/x86/lenovo/ideapad-laptop.c b/drivers/platform/x86/lenovo/ideapad-laptop.c
> index 66e16abda5e3..253d2962b927 100644
> --- a/drivers/platform/x86/lenovo/ideapad-laptop.c
> +++ b/drivers/platform/x86/lenovo/ideapad-laptop.c
> @@ -1714,9 +1714,58 @@ static int ideapad_kbd_bl_led_cdev_brightness_set(struct led_classdev *led_cdev,
> {
> struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
>
> + /*
> + * When deinitializing: It must be the side effect of led_cdev
> + * unregistration when our private trigger is active. We've set
> + * LED_RETAIN_AT_SHUTDOWN to retain led_cdev brightness level.
> + * To do the same for auto mode, gate changes and return early.
> + */
> + if (unlikely(!priv->kbd_bl.initialized))
This too would need include, but I think addressing some earlier include
request will cover it.
> + return 0;
> +
> return ideapad_kbd_bl_brightness_set(priv, brightness);
> }
>
> +static bool ideapad_kbd_bl_auto_trigger_offloaded(struct led_classdev *led_cdev)
> +{
> + struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
Add include for container_of().
> +
> + return atomic_read(&priv->kbd_bl.last_hw_brightness) == KBD_BL_AUTO_MODE_HW_BRIGHTNESS;
> +}
> +
> +static int ideapad_kbd_bl_auto_trigger_activate(struct led_classdev *led_cdev)
> +{
> + struct ideapad_private *priv = container_of(led_cdev, struct ideapad_private, kbd_bl.led);
> +
> + return ideapad_kbd_bl_hw_brightness_set(priv, KBD_BL_AUTO_MODE_HW_BRIGHTNESS);
> +}
> +
> +static struct led_hw_trigger_type ideapad_kbd_bl_auto_trigger_type;
> +
> +static struct led_trigger ideapad_kbd_bl_auto_trigger = {
> + .name = "ideapad-auto",
> + .trigger_type = &ideapad_kbd_bl_auto_trigger_type,
> + .activate = ideapad_kbd_bl_auto_trigger_activate,
> + .offloaded = ideapad_kbd_bl_auto_trigger_offloaded,
> +};
> +
> +static bool ideapad_kbd_bl_auto_trigger_registered;
> +
> +static void ideapad_kbd_bl_notify_hw_control(struct ideapad_private *priv,
> + int hw_brightness, int last_hw_brightness)
> +{
> + bool hw_control, last_hw_control;
> +
> + if (priv->kbd_bl.type != KBD_BL_TRISTATE_AUTO)
> + return;
> +
> + hw_control = hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS;
> + last_hw_control = last_hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS;
> +
> + if (hw_control != last_hw_control)
> + led_trigger_notify_hw_control_changed(&priv->kbd_bl.led, hw_control);
> +}
> +
> static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
> {
> int hw_brightness, brightness, last_hw_brightness;
> @@ -1738,6 +1787,8 @@ static void ideapad_kbd_bl_notify(struct ideapad_private *priv)
> if (hw_brightness == last_hw_brightness)
> return;
>
> + ideapad_kbd_bl_notify_hw_control(priv, hw_brightness, last_hw_brightness);
> +
> led_classdev_notify_brightness_hw_changed(&priv->kbd_bl.led, brightness);
> }
>
> @@ -1768,6 +1819,24 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
>
> switch (priv->kbd_bl.type) {
> case KBD_BL_TRISTATE_AUTO:
> + priv->kbd_bl.led.max_brightness = 2;
> +
> + if (!ideapad_kbd_bl_auto_trigger_registered) {
> + dev_warn(&priv->platform_device->dev,
> + "Could not provide LED trigger %s for keyboard backlight\n",
> + ideapad_kbd_bl_auto_trigger.name);
> + break;
> + }
> +
> + priv->kbd_bl.led.flags |= LED_TRIG_HW_CHANGED;
> + priv->kbd_bl.led.hw_control_trigger = ideapad_kbd_bl_auto_trigger.name;
> + priv->kbd_bl.led.trigger_type = &ideapad_kbd_bl_auto_trigger_type;
I'm skeptical aligning makes things better here.
> +
> + /* Hardware remembers the last brightness level, including auto mode. */
> + if (hw_brightness == KBD_BL_AUTO_MODE_HW_BRIGHTNESS)
> + priv->kbd_bl.led.default_trigger = ideapad_kbd_bl_auto_trigger.name;
> +
> + break;
> case KBD_BL_TRISTATE:
> priv->kbd_bl.led.max_brightness = 2;
> break;
> @@ -1779,13 +1848,22 @@ static int ideapad_kbd_bl_init(struct ideapad_private *priv)
> unreachable();
> }
>
> - err = led_classdev_register(&priv->platform_device->dev, &priv->kbd_bl.led);
> - if (err)
> - return err;
> + /* Queue notifications, as kbd_bl.initialized is about to be set. */
> + guard(mutex)(&priv->kbd_bl.notif_mutex);
>
> + /*
> + * Setting kbd_bl.initialized after led_classdev_register() could lead
> + * to race conditions in ideapad_kbd_bl_led_cdev_brightness_set() where
> + * kbd_bl.initialized is checked, so set it now. It can be reverted back
> + * if the LED classdev failed to register.
> + */
> priv->kbd_bl.initialized = true;
>
> - return 0;
> + err = led_classdev_register(&priv->platform_device->dev, &priv->kbd_bl.led);
> + if (err)
> + priv->kbd_bl.initialized = false;
> +
> + return err;
> }
>
> static void ideapad_kbd_bl_exit(struct ideapad_private *priv)
> @@ -2612,17 +2690,30 @@ static int __init ideapad_laptop_init(void)
> {
> int err;
>
> + err = led_trigger_register(&ideapad_kbd_bl_auto_trigger);
> + if (err) {
> + pr_warn("Failed to register LED trigger %s: %d\n",
include missing.
> + ideapad_kbd_bl_auto_trigger.name, err);
> + } else {
> + ideapad_kbd_bl_auto_trigger_registered = true;
> + }
> +
> err = ideapad_wmi_driver_register();
> if (err)
> - return err;
> + goto err_ledtrig;
>
> err = platform_driver_register(&ideapad_acpi_driver);
> - if (err) {
> - ideapad_wmi_driver_unregister();
> - return err;
> - }
> + if (err)
> + goto err_wmi;
>
> return 0;
> +
> +err_wmi:
> + ideapad_wmi_driver_unregister();
> +err_ledtrig:
> + if (ideapad_kbd_bl_auto_trigger_registered)
> + led_trigger_unregister(&ideapad_kbd_bl_auto_trigger);
> + return err;
> }
> module_init(ideapad_laptop_init)
>
> @@ -2630,6 +2721,9 @@ static void __exit ideapad_laptop_exit(void)
> {
> ideapad_wmi_driver_unregister();
> platform_driver_unregister(&ideapad_acpi_driver);
Why is the order not the reverse of the init order?
> +
> + if (ideapad_kbd_bl_auto_trigger_registered)
> + led_trigger_unregister(&ideapad_kbd_bl_auto_trigger);
> }
> module_exit(ideapad_laptop_exit)
>
>
>
--
i.
^ permalink raw reply
* Re: [PATCH net] geneve: fix hint header definition wrt endianness
From: patchwork-bot+netdevbpf @ 2026-07-21 17:30 UTC (permalink / raw)
To: Antoine Tenart; +Cc: davem, kuba, pabeni, edumazet, netdev, sashiko-bot
In-Reply-To: <20260709124801.140632-1-atenart@kernel.org>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 9 Jul 2026 14:48:00 +0200 you wrote:
> Bitfields are packed differently depending on the endianness, take it into
> account in the GRO hint header definition.
>
> Fixes: e0a12cbf262b ("geneve: add GRO hint output path")
> Cc: Paolo Abeni <pabeni@redhat.com>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260529144713.780938-1-atenart%40kernel.org
> Signed-off-by: Antoine Tenart <atenart@kernel.org>
>
> [...]
Here is the summary with links:
- [net] geneve: fix hint header definition wrt endianness
https://git.kernel.org/netdev/net/c/751bfa982b4a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net] geneve: ensure the skb is writable before fixing its headers
From: patchwork-bot+netdevbpf @ 2026-07-21 17:30 UTC (permalink / raw)
To: Antoine Tenart; +Cc: davem, kuba, pabeni, edumazet, netdev, sashiko-bot
In-Reply-To: <20260709125000.141092-1-atenart@kernel.org>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 9 Jul 2026 14:50:00 +0200 you wrote:
> Make sure the IPv4/6 and UDP headers are writable before fixing them up in
> geneve_post_decap_hint. As skb_ensure_writable can reallocate the skb linear
> area, reload the GRO hint header pointer and only set the IPv4/6 header ones
> after the call.
>
> Fixes: fd0dd796576e ("geneve: use GRO hint option in the RX path")
> Cc: Paolo Abeni <pabeni@redhat.com>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260529144713.780938-1-atenart%40kernel.org
> Signed-off-by: Antoine Tenart <atenart@kernel.org>
>
> [...]
Here is the summary with links:
- [net] geneve: ensure the skb is writable before fixing its headers
https://git.kernel.org/netdev/net/c/447ec540233c
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH net-next v6 3/3] net: dsa: motorcomm: Add LED support
From: Jakub Kicinski @ 2026-07-21 17:32 UTC (permalink / raw)
To: David Yang
Cc: netdev, Andrew Lunn, Vladimir Oltean, David S. Miller,
Eric Dumazet, Paolo Abeni, linux-kernel
In-Reply-To: <20260709014812.1158178-4-mmyangfl@gmail.com>
On Thu, 9 Jul 2026 09:47:47 +0800 David Yang wrote:
> LEDs can be described in the device tree using the same format as qca8k.
> Each port can configure up to 3 LEDs.
>
> Currently, only parallel mode and strict 1:1 mapping are supported.
> +config NET_DSA_YT921X_LEDS
> + bool "LED support for Motorcomm YT9215"
> + default y
> + depends on NET_DSA_YT921X
> + depends on LEDS_CLASS=y || LEDS_CLASS=NET_DSA_YT921X
is it really useful to prompt the user with this option?
Shouldn't this be a hidden option which gets auto-enabled
when dependencies are met?
> +static bool
> +yt921x_led_trigger_is_supported(const struct yt921x_priv *priv, int port,
> + int group, unsigned long flags)
> +{
> + if (!flags)
> + return true;
> +
> + for (unsigned int i = 0; i < ARRAY_SIZE(yt921x_led_trigger_maps); i++) {
> + const struct yt921x_led_trigger_map *map = &yt921x_led_trigger_maps[i];
> +
> + if ((flags & map->flags) == map->flags) {
> + flags &= ~map->flags;
> + if (!flags)
> + return true;
> + }
Isn't this conditions simply:
if (flags == map->flags)
return true;
?
> + }
> +
> + return false;
> +#if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
> +
> +void yt921x_leds_remove(struct yt921x_priv *priv);
> +int yt921x_leds_setup(struct yt921x_priv *priv);
> +
> +#else
> +
> +static inline void yt921x_leds_remove(struct yt921x_priv *priv) {}
> +
> +static inline int yt921x_leds_setup(struct yt921x_priv *priv)
> +{
> + return 0;
> +}
There are static inlines but I think you still protect the calls to
these functions with itdef?
+#if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
+ res = yt921x_leds_setup(priv);
+ if (res)
+ dev_warn(dev, "Failed to setup LEDs: %d\n", res);
+#endif
> +#endif
> +
> +#endif
^ permalink raw reply
* Re: [PATCH net-next v6 3/3] net: dsa: motorcomm: Add LED support
From: Jakub Kicinski @ 2026-07-21 17:32 UTC (permalink / raw)
To: mmyangfl
Cc: Jakub Kicinski, netdev, andrew, olteanv, davem, edumazet, pabeni,
linux-kernel
In-Reply-To: <20260709014812.1158178-4-mmyangfl@gmail.com>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: dsa: motorcomm: Add LED support
This patch adds LED support for the Motorcomm YT9215 DSA switch, allowing
up to 3 LEDs per port to be described in the device tree using a format
similar to qca8k. Only parallel mode and strict 1:1 mapping are supported.
> diff --git a/drivers/net/dsa/motorcomm/Kconfig b/drivers/net/dsa/motorcomm/Kconfig
> index 1fddd386f866..3438e0b14361 100644
> --- a/drivers/net/dsa/motorcomm/Kconfig
> +++ b/drivers/net/dsa/motorcomm/Kconfig
> @@ -6,3 +6,12 @@ config NET_DSA_YT921X
> help
> This enables support for the Motorcomm YT9215 ethernet switch
> chip.
> +
> +config NET_DSA_YT921X_LEDS
> + bool "LED support for Motorcomm YT9215"
> + default y
> + depends on NET_DSA_YT921X
> + depends on LEDS_CLASS=y || LEDS_CLASS=NET_DSA_YT921X
[ ... ]
> diff --git a/drivers/net/dsa/motorcomm/chip.c b/drivers/net/dsa/motorcomm/chip.c
> index 6dee25b6754a..553efacd4f5e 100644
> --- a/drivers/net/dsa/motorcomm/chip.c
> +++ b/drivers/net/dsa/motorcomm/chip.c
> @@ -4581,6 +4580,12 @@ static int yt921x_dsa_setup(struct dsa_switch *ds)
> if (res)
> return res;
>
> +#if IS_ENABLED(CONFIG_NET_DSA_YT921X_LEDS)
> + res = yt921x_leds_setup(priv);
> + if (res)
> + dev_warn(dev, "Failed to setup LEDs: %d\n", res);
> +#endif
[Medium]
When yt921x_leds_setup() fails, the current handling is a dev_warn plus
continue. Looking at the error path inside yt921x_leds_setup():
for_each_child_of_node_scoped(leds_np, led_np) {
res = yt921x_led_setup(priv, port,
of_fwnode_handle(led_np),
&inv_mask, &inv_ctrl);
if (res) {
of_node_put(leds_np);
goto err;
}
}
...
err:
yt921x_leds_remove(priv);
A single malformed subnode on any port (for example reg >= 3, both
active-high and active-low set, or an LED entry attached to the MCU port)
makes yt921x_led_setup() return non-zero, and yt921x_leds_setup() then
tears down every LED that was previously registered on every port. From
the user's perspective this appears as a single dev_warn followed by a
switch that has no working LEDs at all, even on correctly-configured
ports.
Would it be preferable to log the failing LED and continue registering
the rest, so a single DT typo does not disable the whole LED subsystem?
> +#endif
>
> return 0;
> }
>
> @@ -4681,6 +4686,10 @@ static void yt921x_mdio_remove(struct mdio_device *mdiodev)
[ ... ]
> diff --git a/drivers/net/dsa/motorcomm/leds.c b/drivers/net/dsa/motorcomm/leds.c
> new file mode 100644
> index 000000000000..46749b57fee3
> --- /dev/null
> +++ b/drivers/net/dsa/motorcomm/leds.c
> @@ -0,0 +1,630 @@
[ ... ]
> +static const struct yt921x_led_trigger_map yt921x_led_trigger_maps[] = {
> + {BIT(TRIGGER_NETDEV_LINK),
> + YT921X_LEDx_PORT_ACT_DUPLEX_HALF | YT921X_LEDx_PORT_ACT_DUPLEX_FULL},
> + {BIT(TRIGGER_NETDEV_LINK_10), YT921X_LEDx_PORT_ACT_10M},
> + {BIT(TRIGGER_NETDEV_LINK_100), YT921X_LEDx_PORT_ACT_100M},
> + {BIT(TRIGGER_NETDEV_LINK_1000), YT921X_LEDx_PORT_ACT_1000M},
> + {BIT(TRIGGER_NETDEV_HALF_DUPLEX), YT921X_LEDx_PORT_ACT_DUPLEX_HALF},
> + {BIT(TRIGGER_NETDEV_FULL_DUPLEX), YT921X_LEDx_PORT_ACT_DUPLEX_FULL},
[ ... ]
> +static int
> +yt921x_led_trigger_get(struct yt921x_priv *priv, int port, int group,
> + unsigned long *flagsp)
> +{
[ ... ]
> + *flagsp = 0;
> + for (unsigned int i = 0; i < ARRAY_SIZE(yt921x_led_trigger_maps); i++) {
> + const struct yt921x_led_trigger_map *map = &yt921x_led_trigger_maps[i];
> +
> + if ((val & map->mask) == map->mask)
> + *flagsp |= map->flags;
> + }
> +
> + return 0;
> +}
[Medium]
Does this iteration over-report the flags for TRIGGER_NETDEV_LINK?
The LINK entry in yt921x_led_trigger_maps sets both DUPLEX_HALF and
DUPLEX_FULL bits. After yt921x_led_trigger_set(LINK) programs the
register with those two bits, yt921x_led_trigger_get() scans every map
entry without breaking:
/* LINK entry: (val & (HALF|FULL)) == (HALF|FULL) -> match */
/* HALF_DUPLEX entry: (val & HALF) == HALF -> match */
/* FULL_DUPLEX entry: (val & FULL) == FULL -> match */
So *flagsp ends up as LINK | HALF_DUPLEX | FULL_DUPLEX rather than just
LINK. Should the map entries with overlapping masks be handled specially,
or should the loop break/clear after a compound match like yt921x_led_
trigger_set() does with flags &= ~map->flags?
[ ... ]
> +int yt921x_leds_setup(struct yt921x_priv *priv)
> +{
[ ... ]
> + dsa_switch_for_each_port(dp, ds) {
[ ... ]
> + for_each_child_of_node_scoped(leds_np, led_np) {
> + res = yt921x_led_setup(priv, port,
> + of_fwnode_handle(led_np),
> + &inv_mask, &inv_ctrl);
> + if (res) {
> + of_node_put(leds_np);
> + goto err;
> + }
> + }
> +
> + of_node_put(leds_np);
> + }
> +
> + mutex_lock(&priv->reg_lock);
> +
> + mask = YT921X_LED_CTRL_MODE_M | YT921X_LED_CTRL_PORT_NUM_M |
> + YT921X_LED_CTRL_EN;
> + ctrl = YT921X_LED_CTRL_MODE_PARALLEL |
> + YT921X_LED_CTRL_PORT_NUM(YT921X_PORT_NUM - 1) |
> + YT921X_LED_CTRL_EN;
> + res = yt921x_reg_update_bits(priv, YT921X_LED_CTRL, mask, ctrl);
> + if (res)
> + goto end;
> +
> + /* Inversion is internal - force on will give low logic.
> + * In the rest of the file, treat LEDs as if active-low.
> + */
> + if (inv_mask)
> + res = yt921x_reg_update_bits(priv, YT921X_LED_PAR_INV, inv_mask,
> + inv_ctrl);
> +
> +end:
> + mutex_unlock(&priv->reg_lock);
[Medium]
The commit message says "strict 1:1 mapping". Where is the port-to-LED
mapping actually being programmed to identity?
The header defines the field macros for these registers:
#define YT921X_LED_PAR_MAPn(port) (0xd01d0 + 4 * (port))
#define YT921X_LED_PAR_MAP_DSTn_PORT_M(grp) ...
#define YT921X_LED_PAR_MAP_DSTn_PORT(grp, x) ...
#define YT921X_LED_PAR_MAP_DSTn_LED_M(grp) ...
#define YT921X_LED_PAR_MAP_DSTn_LED(grp, x) ...
but there is no writer for YT921X_LED_PAR_MAPn anywhere in the driver.
The 1:1 mapping seems to rely on the reset / bootloader default. If those
values are not identity for some reason, LED operations for port X would
end up steering to a different pin.
Should the driver write an identity mapping into YT921X_LED_PAR_MAPn, or
should the commit message drop the "strict 1:1 mapping" claim?
[Low]
The order in which the LED controller is enabled versus the inversion
register is programmed looks reversed:
/* enables LED controller first */
res = yt921x_reg_update_bits(priv, YT921X_LED_CTRL, mask, ctrl);
...
/* programs inversion after */
if (inv_mask)
res = yt921x_reg_update_bits(priv, YT921X_LED_PAR_INV, ...);
Per the comment "force on will give low logic", between LED_CTRL_EN
being written and YT921X_LED_PAR_INV being programmed, every
active-high LED with default-state=on would be driven low (LED off)
and only flip to on once the inversion register is written.
If the second write fails, yt921x_leds_remove() is called, which
unregisters classdevs but leaves LED_CTRL_EN set with the wrong
polarity on the physical pins.
Would it be cleaner to program per-LED state and YT921X_LED_PAR_INV
first, and set YT921X_LED_CTRL_EN last?
[High]
Are Documentation/devicetree/bindings updates missing for this change?
The existing binding at
Documentation/devicetree/bindings/net/dsa/motorcomm,yt921x.yaml uses
unevaluatedProperties: false and only references
dsa.yaml#/$defs/ethernet-ports for the port schema, which does not
describe a leds subnode. A device tree that uses the new LED feature
would fail make dtbs_check with unevaluated property errors.
Should the YAML be extended to describe the leds subnode and the
per-LED properties reg, active-high, active-low, and default-state that
the driver consumes here?
[Medium]
The driver reads active-high and active-low as boolean properties on
each LED subnode:
force_high = fwnode_property_read_bool(fwnode, "active-high");
force_low = fwnode_property_read_bool(fwnode, "active-low");
active-low is a well-established convention used elsewhere (for
example as a GPIO flag), and the common LED subnode binding in
leds/common.yaml does not define active-high/active-low booleans on an
LED node. Since DT properties are ABI once shipped, would it be better
to reuse an already-standard property, or to invent a driver-namespaced
one, so the naming does not collide with existing conventions?
--
pw-bot: cr
^ permalink raw reply
* [PATCH 6.18 0134/1611] uaccess: fix ignored_trailing logic in copy_struct_to_user()
From: Greg Kroah-Hartman @ 2026-07-21 15:04 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, Dmitry Safonov, Dmitry Safonov,
Francesco Ruggeri, Salam Noureddine, David Ahern, David S. Miller,
Michal Luczaj, David Wei, Luiz Augusto von Dentz,
Luiz Augusto von Dentz, Marcel Holtmann, Xin Long, Eric Dumazet,
Kuniyuki Iwashima, Paolo Abeni, Willem de Bruijn, Neal Cardwell,
Jakub Kicinski, Simon Horman, Aleksa Sarai, Christian Brauner,
Kees Cook, netdev, linux-bluetooth, linux-kernel,
Stefan Metzmacher, Aleksa Sarai, Sasha Levin
In-Reply-To: <20260721152514.750365251@linuxfoundation.org>
6.18-stable review patch. If anyone has any objections, please let me know.
------------------
From: Stefan Metzmacher <metze@samba.org>
[ Upstream commit 4911de3145a797389577abfdf9a5185d36cc18d7 ]
Currently all callers pass ignored_trailing=NULL, but I have
code that will make use of.
Now it actually behaves like documented:
* If @usize < @ksize, then the kernel is trying to pass userspace a newer
struct than it supports. Thus we only copy the interoperable portions
(@usize) and ignore the rest (but @ignored_trailing is set to %true if
any of the trailing (@ksize - @usize) bytes are non-zero).
Fixes: 424a55a4a908 ("uaccess: add copy_struct_to_user helper")
Cc: Dmitry Safonov <0x7f454c46@gmail.com>
Cc: Dmitry Safonov <dima@arista.com>
Cc: Francesco Ruggeri <fruggeri@arista.com>
Cc: Salam Noureddine <noureddine@arista.com>
Cc: David Ahern <dsahern@kernel.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Michal Luczaj <mhal@rbox.co>
Cc: David Wei <dw@davidwei.uk>
Cc: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Xin Long <lucien.xin@gmail.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Kuniyuki Iwashima <kuniyu@google.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Simon Horman <horms@kernel.org>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Christian Brauner <brauner@kernel.org>
CC: Kees Cook <keescook@chromium.org>
Cc: netdev@vger.kernel.org
Cc: linux-bluetooth@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Link: https://patch.msgid.link/71f69442410c1186ed8ce6d5b4b9d4a5a70edbad.1775576651.git.metze@samba.org
Reviewed-by: Aleksa Sarai <aleksa@amutable.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
include/linux/uaccess.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 7657904c8db9c8..6973fee49f091c 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -499,7 +499,7 @@ copy_struct_to_user(void __user *dst, size_t usize, const void *src,
return -EFAULT;
}
if (ignored_trailing)
- *ignored_trailing = ksize < usize &&
+ *ignored_trailing = usize < ksize &&
memchr_inv(src + size, 0, rest) != NULL;
/* Copy the interoperable parts of the struct. */
if (copy_to_user(dst, src, size))
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v4 2/5] drm/ras: Introduce error threshold
From: Rodrigo Vivi @ 2026-07-21 17:43 UTC (permalink / raw)
To: Raag Jadav
Cc: intel-xe, dri-devel, netdev, simona.vetter, airlied, kuba,
lijo.lazar, Hawking.Zhang, davem, pabeni, edumazet, dev,
zachary.mckevitt, riana.tauro, michal.wajdeczko, matthew.d.roper,
mallesh.koujalagi
In-Reply-To: <aktha9xVZQg8zwNt@black.igk.intel.com>
On Mon, Jul 06, 2026 at 10:03:55AM +0200, Raag Jadav wrote:
> Hi Jakub,
>
> On Tue, Jun 23, 2026 at 03:39:56PM +0530, Raag Jadav wrote:
> > Add get-error-threshold and set-error-threshold command support which
> > allows querying/setting error threshold of the counter. Threshold in RAS
> > context means the number of errors the hardware is expected to accumulate
> > before it raises them to software. This is to have a fine grained control
> > over error notifications that are raised by the hardware.
>
> Anything I can do to move this forward?
Could you please rebase and resend it?
I'm sorry for missing this. I thought there were still unreviewed patches in here.
But now it is failing to apply.
Also might be worth to see if the potential pre-existing issues that Sashiko pointed
out can be addressed. It is okay if we do that later in a follow-up, but at least
do a quick check please.
Thanks,
Rodrigo.
>
> > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > ---
> > v2: Document threshold definition (Riana)
> > Return -EOPNOTSUPP on threshold callbacks absence (Riana)
> > Cancel and free genlmsg on failure (Riana)
> > Document threshold bounds checking responsibility (Riana)
> > v3: Move documentation from yaml to rst file (Riana)
> > s/value/threshold (Riana)
> > Use goto for error handling (Riana)
> > v4: Clarify 0 threshold expectations (Riana)
> > Drop redundant wrapping (Riana)
> > ---
> > Documentation/gpu/drm-ras.rst | 18 +++
> > Documentation/netlink/specs/drm_ras.yaml | 32 +++++
> > drivers/gpu/drm/drm_ras.c | 161 +++++++++++++++++++++++
> > drivers/gpu/drm/drm_ras_nl.c | 27 ++++
> > drivers/gpu/drm/drm_ras_nl.h | 4 +
> > include/drm/drm_ras.h | 28 ++++
> > include/uapi/drm/drm_ras.h | 3 +
> > 7 files changed, 273 insertions(+)
> >
> > diff --git a/Documentation/gpu/drm-ras.rst b/Documentation/gpu/drm-ras.rst
> > index 83c21853b74b..2718f8aee09d 100644
> > --- a/Documentation/gpu/drm-ras.rst
> > +++ b/Documentation/gpu/drm-ras.rst
> > @@ -56,6 +56,10 @@ User space tools can:
> > ``node-id`` and ``error-id`` as parameters.
> > * Clear specific error counters with the ``clear-error-counter`` command, using both
> > ``node-id`` and ``error-id`` as parameters.
> > +* Query specific error counter threshold with the ``get-error-threshold`` command, using both
> > + ``node-id`` and ``error-id`` as parameters.
> > +* Set specific error counter threshold with the ``set-error-threshold`` command, using
> > + ``node-id``, ``error-id`` and ``error-threshold`` as parameters.
> >
> > YAML-based Interface
> > --------------------
> > @@ -111,3 +115,17 @@ Example: Clear an error counter for a given node
> >
> > sudo ynl --family drm_ras --do clear-error-counter --json '{"node-id":0, "error-id":1}'
> > None
> > +
> > +Example: Query error threshold of a given counter
> > +
> > +.. code-block:: bash
> > +
> > + sudo ynl --family drm_ras --do get-error-threshold --json '{"node-id":0, "error-id":1}'
> > + {'error-id': 1, 'error-name': 'error_name1', 'error-threshold': 16}
> > +
> > +Example: Set error threshold of a given counter
> > +
> > +.. code-block:: bash
> > +
> > + sudo ynl --family drm_ras --do set-error-threshold --json '{"node-id":0, "error-id":1, "error-threshold":8}'
> > + None
> > diff --git a/Documentation/netlink/specs/drm_ras.yaml b/Documentation/netlink/specs/drm_ras.yaml
> > index e113056f8c01..9cf7f9cde242 100644
> > --- a/Documentation/netlink/specs/drm_ras.yaml
> > +++ b/Documentation/netlink/specs/drm_ras.yaml
> > @@ -69,6 +69,10 @@ attribute-sets:
> > name: error-value
> > type: u32
> > doc: Current value of the requested error counter.
> > + -
> > + name: error-threshold
> > + type: u32
> > + doc: Error threshold of the counter.
> >
> > operations:
> > list:
> > @@ -124,3 +128,31 @@ operations:
> > do:
> > request:
> > attributes: *id-attrs
> > + -
> > + name: get-error-threshold
> > + doc: >-
> > + Retrieve error threshold of a given counter.
> > + The response includes the id, the name, and current threshold
> > + of the counter.
> > + attribute-set: error-counter-attrs
> > + flags: [admin-perm]
> > + do:
> > + request:
> > + attributes: *id-attrs
> > + reply:
> > + attributes:
> > + - error-id
> > + - error-name
> > + - error-threshold
> > + -
> > + name: set-error-threshold
> > + doc: >-
> > + Set error threshold of a given counter.
> > + attribute-set: error-counter-attrs
> > + flags: [admin-perm]
> > + do:
> > + request:
> > + attributes:
> > + - node-id
> > + - error-id
> > + - error-threshold
> > diff --git a/drivers/gpu/drm/drm_ras.c b/drivers/gpu/drm/drm_ras.c
> > index 467a169026fc..d60c40ac5427 100644
> > --- a/drivers/gpu/drm/drm_ras.c
> > +++ b/drivers/gpu/drm/drm_ras.c
> > @@ -41,6 +41,13 @@
> > * Userspace must provide Node ID, Error ID.
> > * Clears specific error counter of a node if supported.
> > *
> > + * 4. GET_ERROR_THRESHOLD: Query error threshold of a given counter.
> > + * Userspace must provide Node ID and Error ID.
> > + * Returns the error threshold of a specific counter.
> > + *
> > + * 5. SET_ERROR_THRESHOLD: Set error threshold of a given counter.
> > + * Userspace must provide Node ID, Error ID and threshold to be set.
> > + *
> > * Node registration:
> > *
> > * - drm_ras_node_register(): Registers a new node and assigns
> > @@ -61,6 +68,16 @@
> > * + The error counters in the driver doesn't need to be contiguous, but the
> > * driver must return -ENOENT to the query_error_counter as an indication
> > * that the ID should be skipped and not listed in the netlink API.
> > + * + The driver can optionally implement query_error_threshold() and
> > + * set_error_threshold() callbacks to facilitate getting/setting error
> > + * threshold of the counter. Threshold in RAS context means the number of
> > + * errors the hardware is expected to accumulate before it raises them to
> > + * software. This is to have a fine grained control over error notifications
> > + * that are raised by the hardware.
> > + * + The driver is responsible for error threshold bounds checking.
> > + * + Threshold of 0 can mean invalid threshold or act as a disable notifications
> > + * toggle for that counter depending on usecase and the driver is responsible
> > + * for handling it as needed.
> > *
> > * Netlink handlers:
> > *
> > @@ -72,6 +89,10 @@
> > * operation, fetching a counter value from a specific node.
> > * - drm_ras_nl_clear_error_counter_doit(): Implements the CLEAR_ERROR_COUNTER doit
> > * operation, clearing a counter value from a specific node.
> > + * - drm_ras_nl_get_error_threshold_doit(): Implements the GET_ERROR_THRESHOLD doit
> > + * operation, fetching the error threshold of a specific counter.
> > + * - drm_ras_nl_set_error_threshold_doit(): Implements the SET_ERROR_THRESHOLD doit
> > + * operation, setting the error threshold of a specific counter.
> > */
> >
> > static DEFINE_XARRAY_ALLOC(drm_ras_xa);
> > @@ -168,6 +189,40 @@ static int get_node_error_counter(u32 node_id, u32 error_id,
> > return node->query_error_counter(node, error_id, name, value);
> > }
> >
> > +static int get_node_error_threshold(u32 node_id, u32 error_id, const char **name, u32 *threshold)
> > +{
> > + struct drm_ras_node *node;
> > +
> > + node = xa_load(&drm_ras_xa, node_id);
> > + if (!node)
> > + return -ENOENT;
> > +
> > + if (!node->query_error_threshold)
> > + return -EOPNOTSUPP;
> > +
> > + if (error_id < node->error_counter_range.first || error_id > node->error_counter_range.last)
> > + return -EINVAL;
> > +
> > + return node->query_error_threshold(node, error_id, name, threshold);
> > +}
> > +
> > +static int set_node_error_threshold(u32 node_id, u32 error_id, u32 threshold)
> > +{
> > + struct drm_ras_node *node;
> > +
> > + node = xa_load(&drm_ras_xa, node_id);
> > + if (!node)
> > + return -ENOENT;
> > +
> > + if (!node->set_error_threshold)
> > + return -EOPNOTSUPP;
> > +
> > + if (error_id < node->error_counter_range.first || error_id > node->error_counter_range.last)
> > + return -EINVAL;
> > +
> > + return node->set_error_threshold(node, error_id, threshold);
> > +}
> > +
> > static int msg_reply_value(struct sk_buff *msg, u32 error_id,
> > const char *error_name, u32 value)
> > {
> > @@ -186,6 +241,22 @@ static int msg_reply_value(struct sk_buff *msg, u32 error_id,
> > value);
> > }
> >
> > +static int msg_reply_threshold(struct sk_buff *msg, u32 error_id, const char *error_name,
> > + u32 threshold)
> > +{
> > + int ret;
> > +
> > + ret = nla_put_u32(msg, DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID, error_id);
> > + if (ret)
> > + return ret;
> > +
> > + ret = nla_put_string(msg, DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_NAME, error_name);
> > + if (ret)
> > + return ret;
> > +
> > + return nla_put_u32(msg, DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD, threshold);
> > +}
> > +
> > static int doit_reply_value(struct genl_info *info, u32 node_id,
> > u32 error_id)
> > {
> > @@ -225,6 +296,43 @@ static int doit_reply_value(struct genl_info *info, u32 node_id,
> > return ret;
> > }
> >
> > +static int doit_reply_threshold(struct genl_info *info, u32 node_id, u32 error_id)
> > +{
> > + const char *error_name;
> > + struct sk_buff *msg;
> > + struct nlattr *hdr;
> > + u32 threshold;
> > + int ret;
> > +
> > + msg = genlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
> > + if (!msg)
> > + return -ENOMEM;
> > +
> > + hdr = genlmsg_iput(msg, info);
> > + if (!hdr) {
> > + ret = -EMSGSIZE;
> > + goto free_msg;
> > + }
> > +
> > + ret = get_node_error_threshold(node_id, error_id, &error_name, &threshold);
> > + if (ret)
> > + goto cancel_msg;
> > +
> > + ret = msg_reply_threshold(msg, error_id, error_name, threshold);
> > + if (ret)
> > + goto cancel_msg;
> > +
> > + genlmsg_end(msg, hdr);
> > +
> > + return genlmsg_reply(msg, info);
> > +
> > +cancel_msg:
> > + genlmsg_cancel(msg, hdr);
> > +free_msg:
> > + nlmsg_free(msg);
> > + return ret;
> > +}
> > +
> > /**
> > * drm_ras_nl_get_error_counter_dumpit() - Dump all Error Counters
> > * @skb: Netlink message buffer
> > @@ -358,6 +466,59 @@ int drm_ras_nl_clear_error_counter_doit(struct sk_buff *skb,
> > return node->clear_error_counter(node, error_id);
> > }
> >
> > +/**
> > + * drm_ras_nl_get_error_threshold_doit() - Query error threshold of a counter
> > + * @skb: Netlink message buffer
> > + * @info: Generic Netlink info containing attributes of the request
> > + *
> > + * Extracts the Node ID and Error ID from the netlink attributes and retrieves
> > + * the error threshold of the corresponding counter. Sends the result back to
> > + * the requesting user via the standard Genl reply.
> > + *
> > + * Return: 0 on success, or negative errno on failure.
> > + */
> > +int drm_ras_nl_get_error_threshold_doit(struct sk_buff *skb, struct genl_info *info)
> > +{
> > + u32 node_id, error_id;
> > +
> > + if (!info->attrs ||
> > + GENL_REQ_ATTR_CHECK(info, DRM_RAS_A_ERROR_COUNTER_ATTRS_NODE_ID) ||
> > + GENL_REQ_ATTR_CHECK(info, DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID))
> > + return -EINVAL;
> > +
> > + node_id = nla_get_u32(info->attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_NODE_ID]);
> > + error_id = nla_get_u32(info->attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID]);
> > +
> > + return doit_reply_threshold(info, node_id, error_id);
> > +}
> > +
> > +/**
> > + * drm_ras_nl_set_error_threshold_doit() - Set error threshold of a counter
> > + * @skb: Netlink message buffer
> > + * @info: Generic Netlink info containing attributes of the request
> > + *
> > + * Extracts the Node ID, Error ID and threshold from the netlink attributes and
> > + * sets the error threshold of the corresponding counter.
> > + *
> > + * Return: 0 on success, or negative errno on failure.
> > + */
> > +int drm_ras_nl_set_error_threshold_doit(struct sk_buff *skb, struct genl_info *info)
> > +{
> > + u32 node_id, error_id, threshold;
> > +
> > + if (!info->attrs ||
> > + GENL_REQ_ATTR_CHECK(info, DRM_RAS_A_ERROR_COUNTER_ATTRS_NODE_ID) ||
> > + GENL_REQ_ATTR_CHECK(info, DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID) ||
> > + GENL_REQ_ATTR_CHECK(info, DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD))
> > + return -EINVAL;
> > +
> > + node_id = nla_get_u32(info->attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_NODE_ID]);
> > + error_id = nla_get_u32(info->attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID]);
> > + threshold = nla_get_u32(info->attrs[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD]);
> > +
> > + return set_node_error_threshold(node_id, error_id, threshold);
> > +}
> > +
> > /**
> > * drm_ras_node_register() - Register a new RAS node
> > * @node: Node structure to register
> > diff --git a/drivers/gpu/drm/drm_ras_nl.c b/drivers/gpu/drm/drm_ras_nl.c
> > index dea1c1b2494e..02e8e5054d05 100644
> > --- a/drivers/gpu/drm/drm_ras_nl.c
> > +++ b/drivers/gpu/drm/drm_ras_nl.c
> > @@ -28,6 +28,19 @@ static const struct nla_policy drm_ras_clear_error_counter_nl_policy[DRM_RAS_A_E
> > [DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID] = { .type = NLA_U32, },
> > };
> >
> > +/* DRM_RAS_CMD_GET_ERROR_THRESHOLD - do */
> > +static const struct nla_policy drm_ras_get_error_threshold_nl_policy[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID + 1] = {
> > + [DRM_RAS_A_ERROR_COUNTER_ATTRS_NODE_ID] = { .type = NLA_U32, },
> > + [DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID] = { .type = NLA_U32, },
> > +};
> > +
> > +/* DRM_RAS_CMD_SET_ERROR_THRESHOLD - do */
> > +static const struct nla_policy drm_ras_set_error_threshold_nl_policy[DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD + 1] = {
> > + [DRM_RAS_A_ERROR_COUNTER_ATTRS_NODE_ID] = { .type = NLA_U32, },
> > + [DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID] = { .type = NLA_U32, },
> > + [DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD] = { .type = NLA_U32, },
> > +};
> > +
> > /* Ops table for drm_ras */
> > static const struct genl_split_ops drm_ras_nl_ops[] = {
> > {
> > @@ -56,6 +69,20 @@ static const struct genl_split_ops drm_ras_nl_ops[] = {
> > .maxattr = DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID,
> > .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> > },
> > + {
> > + .cmd = DRM_RAS_CMD_GET_ERROR_THRESHOLD,
> > + .doit = drm_ras_nl_get_error_threshold_doit,
> > + .policy = drm_ras_get_error_threshold_nl_policy,
> > + .maxattr = DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID,
> > + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> > + },
> > + {
> > + .cmd = DRM_RAS_CMD_SET_ERROR_THRESHOLD,
> > + .doit = drm_ras_nl_set_error_threshold_doit,
> > + .policy = drm_ras_set_error_threshold_nl_policy,
> > + .maxattr = DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD,
> > + .flags = GENL_ADMIN_PERM | GENL_CMD_CAP_DO,
> > + },
> > };
> >
> > struct genl_family drm_ras_nl_family __ro_after_init = {
> > diff --git a/drivers/gpu/drm/drm_ras_nl.h b/drivers/gpu/drm/drm_ras_nl.h
> > index a398643572a5..57b1e647d833 100644
> > --- a/drivers/gpu/drm/drm_ras_nl.h
> > +++ b/drivers/gpu/drm/drm_ras_nl.h
> > @@ -20,6 +20,10 @@ int drm_ras_nl_get_error_counter_dumpit(struct sk_buff *skb,
> > struct netlink_callback *cb);
> > int drm_ras_nl_clear_error_counter_doit(struct sk_buff *skb,
> > struct genl_info *info);
> > +int drm_ras_nl_get_error_threshold_doit(struct sk_buff *skb,
> > + struct genl_info *info);
> > +int drm_ras_nl_set_error_threshold_doit(struct sk_buff *skb,
> > + struct genl_info *info);
> >
> > extern struct genl_family drm_ras_nl_family;
> >
> > diff --git a/include/drm/drm_ras.h b/include/drm/drm_ras.h
> > index f2a787bc4f64..683a3844f84f 100644
> > --- a/include/drm/drm_ras.h
> > +++ b/include/drm/drm_ras.h
> > @@ -69,6 +69,34 @@ struct drm_ras_node {
> > */
> > int (*clear_error_counter)(struct drm_ras_node *node, u32 error_id);
> >
> > + /**
> > + * @query_error_threshold:
> > + *
> > + * This callback is used by drm-ras to query error threshold of a
> > + * specific counter.
> > + *
> > + * Driver should expect query_error_threshold() to be called with
> > + * error_id from `error_counter_range.first` to
> > + * `error_counter_range.last`.
> > + *
> > + * Returns: 0 on success, negative error code on failure.
> > + */
> > + int (*query_error_threshold)(struct drm_ras_node *node, u32 error_id, const char **name,
> > + u32 *threshold);
> > + /**
> > + * @set_error_threshold:
> > + *
> > + * This callback is used by drm-ras to set error threshold of a specific
> > + * counter.
> > + *
> > + * Driver should expect set_error_threshold() to be called with error_id
> > + * from `error_counter_range.first` to `error_counter_range.last`.
> > + * Driver is responsible for error threshold bounds checking.
> > + *
> > + * Returns: 0 on success, negative error code on failure.
> > + */
> > + int (*set_error_threshold)(struct drm_ras_node *node, u32 error_id, u32 threshold);
> > +
> > /** @priv: Driver private data */
> > void *priv;
> > };
> > diff --git a/include/uapi/drm/drm_ras.h b/include/uapi/drm/drm_ras.h
> > index 218a3ee86805..27c68956495f 100644
> > --- a/include/uapi/drm/drm_ras.h
> > +++ b/include/uapi/drm/drm_ras.h
> > @@ -33,6 +33,7 @@ enum {
> > DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_ID,
> > DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_NAME,
> > DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_VALUE,
> > + DRM_RAS_A_ERROR_COUNTER_ATTRS_ERROR_THRESHOLD,
> >
> > __DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX,
> > DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX = (__DRM_RAS_A_ERROR_COUNTER_ATTRS_MAX - 1)
> > @@ -42,6 +43,8 @@ enum {
> > DRM_RAS_CMD_LIST_NODES = 1,
> > DRM_RAS_CMD_GET_ERROR_COUNTER,
> > DRM_RAS_CMD_CLEAR_ERROR_COUNTER,
> > + DRM_RAS_CMD_GET_ERROR_THRESHOLD,
> > + DRM_RAS_CMD_SET_ERROR_THRESHOLD,
> >
> > __DRM_RAS_CMD_MAX,
> > DRM_RAS_CMD_MAX = (__DRM_RAS_CMD_MAX - 1)
> > --
> > 2.43.0
> >
^ permalink raw reply
* Re: [PATCH net-next v5] selftests/net/openvswitch: add ICMPv6 echo type match test
From: Jakub Kicinski @ 2026-07-21 17:52 UTC (permalink / raw)
To: Minxi Hou; +Cc: netdev, aconole, echaudro, i.maximets
In-Reply-To: <20260720032448.3709753-1-houminxi@gmail.com>
On Sun, 19 Jul 2026 23:24:48 -0400 Minxi Hou wrote:
> Gentle ping on this patch. It's been about 10 days without review
> comments. Please let me know if there are any issues or if a respin
> is needed.
Read the mailing list. There was an announcement.
You're just wasting our time with such stupid pings :/
^ permalink raw reply
* [PATCH v45 0/7] MCTP over PCC
From: Adam Young @ 2026-07-21 17:52 UTC (permalink / raw)
Cc: netdev, linux-kernel, Jeremy Kerr, Matt Johnston,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Sudeep Holla, Jonathan Cameron, Huisong Li
From: Linux Bot <linuxbot@amperecomputing.com>
THis patch sereis is long lived, and the main
changelog will saty with the MCTP over PCC patch.
Recent changes in AI code review haved shown required
fixes in the PC mailbox layer. The main changes to this
patch series are the addition of those patches.
Adam Young (7):
mailbox/pcc.c: shmem map/unmap startup/teardown
mailbox/pcc.c: ignore errors on type 4 channels.
mailbox/pcc.c: report errors for PCC clients
mailbox/pcc.c: add query channel function
mctp pcc: Implement MCTP over PCC Transport
synchronize IRQ before releasing shared memory
wrap pchan->chan_in_use in READ/WRITE_ONCE
MAINTAINERS | 5 +
drivers/mailbox/pcc.c | 112 ++++++---
drivers/net/mctp/Kconfig | 15 ++
drivers/net/mctp/Makefile | 1 +
drivers/net/mctp/mctp-pcc.c | 466 ++++++++++++++++++++++++++++++++++++
include/acpi/pcc.h | 9 +
6 files changed, 576 insertions(+), 32 deletions(-)
create mode 100644 drivers/net/mctp/mctp-pcc.c
--
2.43.0
^ permalink raw reply
* [PATCH v45 1/7] mailbox/pcc.c: shmem map/unmap startup/teardown
From: Adam Young @ 2026-07-21 17:52 UTC (permalink / raw)
To: Sudeep Holla, Jassi Brar, Huisong Li
Cc: netdev, linux-kernel, Jeremy Kerr, Matt Johnston,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Sudeep Holla, Jonathan Cameron
In-Reply-To: <20260721175258.87600-1-admiyo@os.amperecomputing.com>
The mailbox IRQ and shmems are not cleaned up atomically, so there is a
race condition. If the shmem is torn down while the IRQ is active, a late
interrupt can trigger a write to un-mapped memory.
If the shmem is torn down while the IRQ is active, and another thread
requests the channel again, we can end up with a channel that has had
its shmem unmapped.
By moving the map to start up and the unmap to teardown, we can let
the mailbox mechanism prevent re-entrance into the startup/teardown
functions.
Avoid doubly unmapping the region by removing the unmap in the
direct error handler for the request.
Assisted-by: Codex:gpt-5.4
Fixes: fa362ffafa51 ("mailbox: pcc: Always map the shared memory communication address")
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
drivers/mailbox/pcc.c | 48 +++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 636879ae1db7..da26578a8aab 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -360,7 +360,6 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
struct pcc_mbox_chan *
pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
{
- struct pcc_mbox_chan *pcc_mchan;
struct pcc_chan_info *pchan;
struct mbox_chan *chan;
int rc;
@@ -375,20 +374,10 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
return ERR_PTR(-EBUSY);
}
- pcc_mchan = &pchan->chan;
- pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
- pcc_mchan->shmem_size);
- if (!pcc_mchan->shmem)
- return ERR_PTR(-ENXIO);
-
rc = mbox_bind_client(chan, cl);
- if (rc) {
- iounmap(pcc_mchan->shmem);
- pcc_mchan->shmem = NULL;
- return ERR_PTR(rc);
- }
-
- return pcc_mchan;
+ if (rc)
+ return ERR_PTR(-ENXIO);
+ return &pchan->chan;
}
EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
@@ -400,19 +389,13 @@ EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
*/
void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
{
- struct mbox_chan *chan = pchan->mchan;
- struct pcc_chan_info *pchan_info;
- struct pcc_mbox_chan *pcc_mbox_chan;
+ struct mbox_chan *chan;
+ if (!pchan)
+ return;
+ chan = pchan->mchan;
if (!chan || !chan->cl)
return;
- pchan_info = chan->con_priv;
- pcc_mbox_chan = &pchan_info->chan;
- if (pcc_mbox_chan->shmem) {
- iounmap(pcc_mbox_chan->shmem);
- pcc_mbox_chan->shmem = NULL;
- }
-
mbox_free_channel(chan);
}
EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
@@ -462,9 +445,15 @@ static bool pcc_last_tx_done(struct mbox_chan *chan)
static int pcc_startup(struct mbox_chan *chan)
{
struct pcc_chan_info *pchan = chan->con_priv;
+ struct pcc_mbox_chan *pcc_mchan;
unsigned long irqflags;
int rc;
+ pcc_mchan = &pchan->chan;
+ pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
+ pcc_mchan->shmem_size);
+ if (pcc_mchan->shmem == NULL)
+ return -ENOMEM;
/*
* Clear and acknowledge any pending interrupts on responder channel
* before enabling the interrupt
@@ -479,6 +468,8 @@ static int pcc_startup(struct mbox_chan *chan)
if (unlikely(rc)) {
dev_err(chan->mbox->dev, "failed to register PCC interrupt %d\n",
pchan->plat_irq);
+ iounmap(pcc_mchan->shmem);
+ pcc_mchan->shmem = NULL;
return rc;
}
}
@@ -488,15 +479,22 @@ static int pcc_startup(struct mbox_chan *chan)
/**
* pcc_shutdown - Called from Mailbox Controller code. Used here
- * to free the interrupt.
+ * to free the interrupt and unmap the shared memory.
* @chan: Pointer to Mailbox channel to shutdown.
*/
static void pcc_shutdown(struct mbox_chan *chan)
{
struct pcc_chan_info *pchan = chan->con_priv;
+ struct pcc_mbox_chan *pcc_mbox_chan;
if (pchan->plat_irq > 0)
devm_free_irq(chan->mbox->dev, pchan->plat_irq, chan);
+
+ pcc_mbox_chan = &pchan->chan;
+ if (pcc_mbox_chan->shmem) {
+ iounmap(pcc_mbox_chan->shmem);
+ pcc_mbox_chan->shmem = NULL;
+ }
}
static const struct mbox_chan_ops pcc_chan_ops = {
--
2.43.0
^ permalink raw reply related
* [PATCH v45 2/7] mailbox/pcc.c: ignore errors on type 4 channels.
From: Adam Young @ 2026-07-21 17:52 UTC (permalink / raw)
To: Sudeep Holla, Jassi Brar
Cc: netdev, linux-kernel, Jeremy Kerr, Matt Johnston,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Sudeep Holla, Jonathan Cameron, Huisong Li
In-Reply-To: <20260721175258.87600-1-admiyo@os.amperecomputing.com>
THE ACPI spec states:
"[The Error status register] Contains the processor relative address,
represented in Generic Address Structure (GAS) format, of the Error status
register. This field is ignored by the OSPM on slave channels"
Referring to type 4 channels.
https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/14_Platform_Communications_Channel/Platform_Comm_Channel.html#hw-registers-based-communications-subspace-structure-type-5
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
drivers/mailbox/pcc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index da26578a8aab..3c059fe87ce4 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -270,6 +270,9 @@ static int pcc_mbox_error_check_and_clear(struct pcc_chan_info *pchan)
u64 val;
int ret;
+ if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+ return 0;
+
ret = pcc_chan_reg_read(&pchan->error, &val);
if (ret)
return ret;
--
2.43.0
^ permalink raw reply related
* [PATCH v45 3/7] mailbox/pcc.c: report errors for PCC clients
From: Adam Young @ 2026-07-21 17:52 UTC (permalink / raw)
To: Sudeep Holla, Jassi Brar
Cc: netdev, linux-kernel, Jeremy Kerr, Matt Johnston,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Sudeep Holla, Jonathan Cameron, Huisong Li
In-Reply-To: <20260721175258.87600-1-admiyo@os.amperecomputing.com>
The tx_done callback function has a return code (rc) parameter
that the tx_done callback can use to determine how to handle an error.
However the IRQ handler was not setting that value if there is an error.
The following clients are affected:
drivers/acpi/cppc_acpi.c
drivers/i2c/busses/i2c-xgene-slimpro.c
drivers/hwmon/xgene-hwmon.c
drivers/soc/hisilicon/kunpeng_hccs.c
drivers/devfreq/hisi_uncore_freq.c
All of these only use the error code to report, so they
are expecting an error code to come thorugh, but they
do not modify behavior based on this code.
In the case of an error code in the IRQ, the handler was returning
IRQ_NONE which is not correct: the IRQ handler was matched
to the IRQ. This mean that multiple error codes returned from
a PCC triggered interrupt would end up disabling the device.
In addition, if the error code IRQ was coming from a Type4 Device that was
expecting an IRQ response, that device would then be hung.
Fixes: c45ded7e1135 ("mailbox: pcc: Add support for PCCT extended PCC subspaces(type 3/4)")
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
---
drivers/mailbox/pcc.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 3c059fe87ce4..b1c7d3e4c5e3 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -317,6 +317,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
{
struct pcc_chan_info *pchan;
struct mbox_chan *chan = p;
+ int rc;
pchan = chan->con_priv;
@@ -330,8 +331,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
if (!pcc_mbox_cmd_complete_check(pchan))
return IRQ_NONE;
- if (pcc_mbox_error_check_and_clear(pchan))
- return IRQ_NONE;
+ rc = pcc_mbox_error_check_and_clear(pchan);
/*
* Clear this flag after updating interrupt ack register and just
@@ -340,8 +340,9 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
* required to avoid any possible race in updatation of this flag.
*/
pchan->chan_in_use = false;
- mbox_chan_received_data(chan, NULL);
- mbox_chan_txdone(chan, 0);
+ if (!rc)
+ mbox_chan_received_data(chan, NULL);
+ mbox_chan_txdone(chan, rc);
pcc_chan_acknowledge(pchan);
--
2.43.0
^ permalink raw reply related
* [PATCH v45 4/7] mailbox/pcc.c: add query channel function
From: Adam Young @ 2026-07-21 17:52 UTC (permalink / raw)
To: Sudeep Holla, Jassi Brar, Rafael J. Wysocki, Saket Dumbre,
Len Brown
Cc: netdev, linux-kernel, Jeremy Kerr, Matt Johnston,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Sudeep Holla, Jonathan Cameron, Huisong Li
In-Reply-To: <20260721175258.87600-1-admiyo@os.amperecomputing.com>
Drivers need information about a channel prior to creating a channel
or they risk triggering message delivery on the remote side of a
connection.
Add PCC channel type to records and expose PCC channel type to client.
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
drivers/mailbox/pcc.c | 45 +++++++++++++++++++++++++++++++++++++++++++
include/acpi/pcc.h | 9 +++++++++
2 files changed, 54 insertions(+)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index b1c7d3e4c5e3..b9a01bfdc95d 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -349,6 +349,50 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
return IRQ_HANDLED;
}
+/**
+ * pcc_mbox_query_channel - returns information about the channel
+ * without activating the channel.
+ *
+ * @q_chan: a pointer to an already allocated struct pcc_mbox_chan
+ * that will be populated with the channel data.
+ * @subspace_id: The PCC Subspace index as parsed in the PCC client
+ * ACPI package. This is used to lookup the array of PCC
+ * subspaces as parsed by the PCC Mailbox controller.
+ *
+ * Return: 0 upon success or non-zero upon error.
+ */
+int
+pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id)
+{
+ struct pcc_mbox_chan *pcc_mchan;
+ struct pcc_chan_info *pchan;
+ struct mbox_chan *chan;
+
+ if (!q_chan)
+ return -EINVAL;
+
+ if (subspace_id < 0 || subspace_id >= pcc_chan_count)
+ return -ENOENT;
+ pchan = chan_info + subspace_id;
+ chan = pchan->chan.mchan;
+ if (IS_ERR(chan)) {
+ pr_err("Channel not found for idx: %d\n", subspace_id);
+ return -EBUSY;
+ }
+ pcc_mchan = &pchan->chan;
+
+ q_chan->shmem_base_addr = pcc_mchan->shmem_base_addr;
+ q_chan->shmem = NULL;
+ q_chan->shmem_size = pcc_mchan->shmem_size;
+ q_chan->latency = pcc_mchan->latency;
+ q_chan->max_access_rate = pcc_mchan->max_access_rate;
+ q_chan->min_turnaround_time = pcc_mchan->min_turnaround_time;
+ q_chan->type = pcc_mchan->type;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pcc_mbox_query_channel);
+
/**
* pcc_mbox_request_channel - PCC clients call this function to
* request a pointer to their PCC subspace, from which they
@@ -833,6 +877,7 @@ static int pcc_mbox_probe(struct platform_device *pdev)
pcc_parse_subspace_shmem(pchan, pcct_entry);
pchan->type = pcct_entry->type;
+ pchan->chan.type = pcct_entry->type;
pcct_entry = (struct acpi_subtable_header *)
((unsigned long) pcct_entry + pcct_entry->length);
}
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 840bfc95bae3..bf97f407683d 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -8,6 +8,7 @@
#include <linux/mailbox_controller.h>
#include <linux/mailbox_client.h>
+#include <linux/acpi.h>
struct pcc_mbox_chan {
struct mbox_chan *mchan;
@@ -17,6 +18,7 @@ struct pcc_mbox_chan {
u32 latency;
u32 max_access_rate;
u16 min_turnaround_time;
+ enum acpi_pcct_type type;
};
/* Generic Communications Channel Shared Memory Region */
@@ -37,6 +39,8 @@ struct pcc_mbox_chan {
extern struct pcc_mbox_chan *
pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
extern void pcc_mbox_free_channel(struct pcc_mbox_chan *chan);
+extern int
+pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id);
#else
static inline struct pcc_mbox_chan *
pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
@@ -44,6 +48,11 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
return ERR_PTR(-ENODEV);
}
static inline void pcc_mbox_free_channel(struct pcc_mbox_chan *chan) { }
+static inline int
+pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id)
+{
+ return -ENODEV;
+}
#endif
#endif /* _PCC_H */
--
2.43.0
^ permalink raw reply related
* [PATCH v45 5/7] mctp pcc: Implement MCTP over PCC Transport
From: Adam Young @ 2026-07-21 17:52 UTC (permalink / raw)
To: Jeremy Kerr, Matt Johnston, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Sudeep Holla, Jonathan Cameron, Huisong Li
In-Reply-To: <20260721175258.87600-1-admiyo@os.amperecomputing.com>
Implementation of network driver for
Management Component Transport Protocol(MCTP)
over Platform Communication Channel(PCC)
DMTF DSP:0292
Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0292.pdf
The transport mechanism is called Platform Communication Channels (PCC)
is part of the ACPI spec:
Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/14_Platform_Communications_Channel/Platform_Comm_Channel.html
The PCC mechanism is managed via a mailbox implemented at
drivers/mailbox/pcc.c
MCTP devices are specified via ACPI by entries in DSDT/SSDT and
reference channels specified in the PCCT. Messages are sent on a type
3 and received on a type 4 channel. Communication with other devices
use the PCC based doorbell mechanism; a shared memory segment with a
corresponding interrupt and a memory register used to trigger remote
interrupts.
The shared buffer must be at least 68 bytes long as that is the minimum
MTU as defined by the MCTP specification.
Unlike the existing PCC Type 2 based drivers, the mssg parameter to
mbox_send_msg is actively used. The data section of the struct sk_buff
that contains the outgoing packet is sent to the mailbox, already
properly formatted as a PCC exctended message.
If the mailbox ring buffer is full, the driver stops the incoming
packet queues until a message has been sent, freeing space in the
ring buffer.
When the Type 3 channel outbox receives a txdone response interrupt,
it consumes the outgoing sk_buff, allowing it to be freed.
Bringing up an interface creates the channel between the network driver
and the mailbox driver. This enables communication with the remote
endpoint, to include the receipt of new messages. Bringing down an
interface removes the channel, and no new messages can be delivered.
Stopping the interface will leave any packets that are cached in the
mailbox ringbuffer. They cannot safely be freed until the PCC mailbox
attempts to deliver them and has removed them from the ring buffer.
PCC is based on a shared buffer and a set of I/O mapped memory locations
that the Spec calls registers. This mechanism exists regardless of the
existence of the driver. If the user has the ability to map these
physical location to virtual locations, they have the ability to drive the
hardware. Thus, there is a security aspect to this mechanism that extends
beyond the responsibilities of the operating system.
If the hardware does not expose the PCC in the ACPI table, this device
will never be enabled. Thus it is only an issue on hardware that does
support PCC. In that case, it is up to the remote controller to sanitize
communication; MCTP will be exposed as a socket interface, and userland
can send any crafted packet it wants. It would also be incumbent on
the hardware manufacturer to allow the end user to disable MCTP over PCC
communication if they did not want to expose it.
Although the config Allow 32Bit builds, they are untested.
Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0292_1.0.0WIP50.pdf
Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/14_Platform_Communications_Channel/Platform_Comm_Channel.html
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
Previous Version:
https://lore.kernel.org/lkml/20260522193610.234166-1-admiyo@os.amperecomputing.com/
Changes from Previous version
- Compares inbox and outbox buffer size to get smallest MTU
- uses mctp-pcc query channel information without opening channel.
Opening the channel can trigger the sending of
a message from the remote side before the driver
is ready to read it. Take advantage of the API that allows
querying of the channel data without opening the channel.
- Sets network queue to default length instead of 0
- Removed reference to MCTP spec
- Remove reference to endianess
- Rebased on top of PCC mailbox change for atomit startup/teardown
Remove reference to endianness
remove reference to DSP0256
---
MAINTAINERS | 5 +
drivers/net/mctp/Kconfig | 15 ++
drivers/net/mctp/Makefile | 1 +
drivers/net/mctp/mctp-pcc.c | 466 ++++++++++++++++++++++++++++++++++++
4 files changed, 487 insertions(+)
create mode 100644 drivers/net/mctp/mctp-pcc.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6940aa3d498b..9f48dee0f4c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15582,6 +15582,11 @@ F: include/net/mctpdevice.h
F: include/net/netns/mctp.h
F: net/mctp/
+MANAGEMENT COMPONENT TRANSPORT PROTOCOL (MCTP) over PCC (MCTP-PCC) Driver
+M: Adam Young <admiyo@os.amperecomputing.com>
+S: Maintained
+F: drivers/net/mctp/mctp-pcc.c
+
MAPLE TREE
M: Liam R. Howlett <liam@infradead.org>
R: Alice Ryhl <aliceryhl@google.com>
diff --git a/drivers/net/mctp/Kconfig b/drivers/net/mctp/Kconfig
index cf325ab0b1ef..c9b8f38ff0fb 100644
--- a/drivers/net/mctp/Kconfig
+++ b/drivers/net/mctp/Kconfig
@@ -47,6 +47,21 @@ config MCTP_TRANSPORT_I3C
A MCTP protocol network device is created for each I3C bus
having a "mctp-controller" devicetree property.
+config MCTP_TRANSPORT_PCC
+ tristate "MCTP PCC transport"
+ depends on ACPI
+ depends on PCC
+ depends on CPU_LITTLE_ENDIAN
+ help
+ Provides a driver to access MCTP devices over PCC transport,
+ A MCTP protocol network device is created via ACPI for each
+ entry in the DSDT/SSDT that matches the identifier. The Platform
+ communication channels are selected from the corresponding
+ entries in the PCCT.
+
+ Say y here if you need to connect to MCTP endpoints over PCC. To
+ compile as a module, use m; the module will be called mctp-pcc.
+
config MCTP_TRANSPORT_USB
tristate "MCTP USB transport"
depends on USB
diff --git a/drivers/net/mctp/Makefile b/drivers/net/mctp/Makefile
index c36006849a1e..0a591299ffa9 100644
--- a/drivers/net/mctp/Makefile
+++ b/drivers/net/mctp/Makefile
@@ -1,4 +1,5 @@
obj-$(CONFIG_MCTP_SERIAL) += mctp-serial.o
obj-$(CONFIG_MCTP_TRANSPORT_I2C) += mctp-i2c.o
obj-$(CONFIG_MCTP_TRANSPORT_I3C) += mctp-i3c.o
+obj-$(CONFIG_MCTP_TRANSPORT_PCC) += mctp-pcc.o
obj-$(CONFIG_MCTP_TRANSPORT_USB) += mctp-usb.o
diff --git a/drivers/net/mctp/mctp-pcc.c b/drivers/net/mctp/mctp-pcc.c
new file mode 100644
index 000000000000..57c4f3b21513
--- /dev/null
+++ b/drivers/net/mctp/mctp-pcc.c
@@ -0,0 +1,466 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mctp-pcc.c - Driver for MCTP over PCC.
+ * Copyright (c) 2024-2026, Ampere Computing LLC
+ *
+ */
+
+/* Implementation of
+ * https://www.dmtf.org/sites/default/files/standards/documents/DSP0292.pdf
+ */
+
+#include <linux/acpi.h>
+#include <linux/hrtimer.h>
+#include <linux/if_arp.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/platform_device.h>
+#include <linux/skbuff.h>
+#include <linux/string.h>
+
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+#include <acpi/acrestyp.h>
+#include <acpi/actbl.h>
+#include <acpi/pcc.h>
+#include <net/mctp.h>
+#include <net/mctpdevice.h>
+#include <net/pkt_sched.h>
+
+#define MCTP_SIGNATURE "MCTP"
+#define MCTP_SIGNATURE_LENGTH (sizeof(MCTP_SIGNATURE) - 1)
+#define MCTP_MIN_MTU 68
+#define PCC_HEADER_SIZE sizeof(struct acpi_pcct_ext_pcc_shared_memory)
+#define MCTP_PCC_MIN_SIZE (PCC_HEADER_SIZE + MCTP_MIN_MTU)
+#define PCC_EXTRA_LEN (PCC_HEADER_SIZE - sizeof(pcc_header.command))
+struct mctp_pcc_mailbox {
+ u32 index;
+ struct pcc_mbox_chan *chan;
+ struct mbox_client client;
+};
+
+/* The netdev structure. One of these per PCC adapter. */
+struct mctp_pcc_ndev {
+ struct net_device *ndev;
+ struct acpi_device *acpi_device;
+ struct mctp_pcc_mailbox inbox;
+ struct mctp_pcc_mailbox outbox;
+};
+
+static void mctp_pcc_client_rx_callback(struct mbox_client *cl, void *mssg)
+{
+ struct acpi_pcct_ext_pcc_shared_memory pcc_header;
+ struct mctp_pcc_ndev *mctp_pcc_ndev;
+ struct mctp_pcc_mailbox *inbox;
+ struct mctp_skb_cb *cb;
+ struct sk_buff *skb;
+ int size;
+
+ mctp_pcc_ndev = container_of(cl, struct mctp_pcc_ndev, inbox.client);
+ inbox = &mctp_pcc_ndev->inbox;
+ memcpy_fromio(&pcc_header, inbox->chan->shmem, sizeof(pcc_header));
+
+ // The message must at least have the PCC command indicating it is an MCTP
+ // message followed by the MCTP header, or we have a malformed message.
+ if (pcc_header.length < sizeof(pcc_header.command) + sizeof(struct mctp_hdr))
+ goto error;
+
+ // If the reported size is larger than the shared memory minus headers,
+ // something is wrong and treat the buffer as corrupted data.
+ if (pcc_header.length > inbox->chan->shmem_size - PCC_EXTRA_LEN)
+ goto error;
+
+ if (memcmp(&pcc_header.command, MCTP_SIGNATURE, MCTP_SIGNATURE_LENGTH) != 0)
+ goto error;
+
+ size = pcc_header.length + PCC_EXTRA_LEN;
+ skb = netdev_alloc_skb(mctp_pcc_ndev->ndev, size);
+ if (!skb)
+ goto error;
+
+ skb_put(skb, size);
+ skb->protocol = htons(ETH_P_MCTP);
+ memcpy_fromio(skb->data, inbox->chan->shmem, size);
+ dev_dstats_rx_add(mctp_pcc_ndev->ndev, size);
+ skb_pull(skb, sizeof(pcc_header));
+ skb_reset_mac_header(skb);
+ skb_reset_network_header(skb);
+ cb = __mctp_cb(skb);
+ cb->halen = 0;
+ netif_rx(skb);
+ return;
+
+error:
+ dev_dstats_rx_dropped(mctp_pcc_ndev->ndev);
+}
+
+static netdev_tx_t mctp_pcc_tx(struct sk_buff *skb, struct net_device *ndev)
+{
+ struct acpi_pcct_ext_pcc_shared_memory *pcc_header;
+ struct mctp_pcc_ndev *mpnd = netdev_priv(ndev);
+ int len = skb->len;
+
+ if (skb_cow_head(skb, sizeof(*pcc_header)))
+ goto error;
+
+ pcc_header = skb_push(skb, sizeof(*pcc_header));
+ pcc_header->signature = PCC_SIGNATURE | mpnd->outbox.index;
+ pcc_header->flags = PCC_CMD_COMPLETION_NOTIFY;
+ memcpy(&pcc_header->command, MCTP_SIGNATURE, MCTP_SIGNATURE_LENGTH);
+ pcc_header->length = len + MCTP_SIGNATURE_LENGTH;
+
+ if (skb->len > mpnd->outbox.chan->shmem_size)
+ goto error;
+
+ /*
+ * There is a possibility that the mailbox can be cleared on
+ * another thread. If that is the case, and we don't restart
+ * the queue, it will remain permanently stopped.
+ * Stopping the queue before attempting to send the message
+ * allows us to always restart it if mbox_send_message succeeds.
+ */
+ netif_stop_queue(ndev);
+ if (mbox_send_message(mpnd->outbox.chan->mchan, skb) >= 0) {
+ netif_wake_queue(ndev);
+ } else {
+ // Remove the header in case it gets sent again
+ skb_pull(skb, sizeof(*pcc_header));
+ return NETDEV_TX_BUSY;
+ }
+ return NETDEV_TX_OK;
+
+error:
+ dev_dstats_tx_dropped(ndev);
+ kfree_skb(skb);
+ return NETDEV_TX_OK;
+}
+
+static void mctp_pcc_tx_prepare(struct mbox_client *cl, void *mssg)
+{
+ struct mctp_pcc_ndev *mctp_pcc_ndev;
+ struct mctp_pcc_mailbox *outbox;
+ struct sk_buff *skb = mssg;
+
+ mctp_pcc_ndev = container_of(cl, struct mctp_pcc_ndev, outbox.client);
+ outbox = &mctp_pcc_ndev->outbox;
+
+ /* The PCC Mailbox typically does not make use of the mssg pointer
+ * The mctp-over pcc driver is the only client that uses it.
+ * This value should always be non-null; it is possible
+ * that a change in the Mailbox level will break that assumption.
+ */
+ if (!skb) {
+ netdev_warn_once(mctp_pcc_ndev->ndev,
+ "%s called with null message.\n", __func__);
+ return;
+ }
+ memcpy_toio(outbox->chan->shmem, skb->data, skb->len);
+}
+
+static void mctp_pcc_tx_done(struct mbox_client *c, void *mssg, int rc)
+{
+ struct mctp_pcc_ndev *mctp_pcc_ndev;
+ struct pcpu_dstats *dstats;
+ struct sk_buff *skb = mssg;
+ unsigned long flags;
+
+ /*
+ * If there is a packet in flight during driver cleanup
+ * It may have been freed already.
+ */
+ if (!mssg)
+ return;
+ mctp_pcc_ndev = container_of(c, struct mctp_pcc_ndev, outbox.client);
+
+ /* Use an IRQ safe update as this is called from HARD IRQ instead of
+ * dev_dstats_tx_add(mctp_pcc_ndev->ndev, skb->len);
+ */
+ dstats = this_cpu_ptr(mctp_pcc_ndev->ndev->dstats);
+ flags = u64_stats_update_begin_irqsave(&dstats->syncp);
+
+ if (rc) {
+ u64_stats_inc(&dstats->tx_drops);
+ } else {
+ u64_stats_inc(&dstats->tx_packets);
+ u64_stats_add(&dstats->tx_bytes, skb->len);
+ }
+ u64_stats_update_end_irqrestore(&dstats->syncp, flags);
+ dev_consume_skb_any(skb);
+ netif_wake_queue(mctp_pcc_ndev->ndev);
+}
+
+static int mctp_pcc_open(struct net_device *ndev)
+{
+ struct mctp_pcc_ndev *mctp_pcc_ndev = netdev_priv(ndev);
+ struct mctp_pcc_mailbox *outbox, *inbox;
+
+ outbox = &mctp_pcc_ndev->outbox;
+ inbox = &mctp_pcc_ndev->inbox;
+
+ outbox->chan = pcc_mbox_request_channel(&outbox->client, outbox->index);
+ if (IS_ERR(outbox->chan))
+ return PTR_ERR(outbox->chan);
+ if (outbox->chan->shmem_size < MCTP_PCC_MIN_SIZE) {
+ pcc_mbox_free_channel(outbox->chan);
+ return -EINVAL;
+ }
+
+ inbox->client.rx_callback = mctp_pcc_client_rx_callback;
+ inbox->chan = pcc_mbox_request_channel(&inbox->client, inbox->index);
+ if (IS_ERR(inbox->chan)) {
+ pcc_mbox_free_channel(outbox->chan);
+ return PTR_ERR(inbox->chan);
+ }
+ if (inbox->chan->shmem_size < MCTP_PCC_MIN_SIZE) {
+ pcc_mbox_free_channel(outbox->chan);
+ pcc_mbox_free_channel(inbox->chan);
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int mctp_pcc_stop(struct net_device *ndev)
+{
+ struct mctp_pcc_ndev *mctp_pcc_ndev;
+ unsigned int count, idx;
+ struct mbox_chan *chan;
+ struct sk_buff *skb;
+
+ mctp_pcc_ndev = netdev_priv(ndev);
+ chan = mctp_pcc_ndev->outbox.chan->mchan;
+ pcc_mbox_free_channel(mctp_pcc_ndev->inbox.chan);
+ mctp_pcc_ndev->inbox.chan = NULL;
+ scoped_guard(spinlock_irqsave, &chan->lock) {
+ if (chan->active_req != MBOX_NO_MSG) {
+ skb = chan->active_req;
+ chan->active_req = MBOX_NO_MSG;
+ dev_dstats_tx_dropped(ndev);
+ dev_consume_skb_any(skb);
+ }
+ while (chan->msg_count > 0) {
+ count = chan->msg_count;
+ idx = chan->msg_free;
+ if (idx >= count)
+ idx -= count;
+ else
+ idx += MBOX_TX_QUEUE_LEN - count;
+ skb = chan->msg_data[idx];
+ dev_dstats_tx_dropped(ndev);
+ dev_consume_skb_any(skb);
+ chan->msg_count--;
+ }
+ }
+ pcc_mbox_free_channel(mctp_pcc_ndev->outbox.chan);
+ mctp_pcc_ndev->outbox.chan = NULL;
+ /*
+ * If the queue was stopped because the ring buffer was full
+ * we can restart it here as we now know the ring buffer has
+ * been emptied and the queue can be used again if the
+ * netdev is re-opened.
+ */
+ netif_wake_queue(mctp_pcc_ndev->ndev);
+ return 0;
+}
+
+static const struct net_device_ops mctp_pcc_netdev_ops = {
+ .ndo_open = mctp_pcc_open,
+ .ndo_stop = mctp_pcc_stop,
+ .ndo_start_xmit = mctp_pcc_tx,
+};
+
+static void mctp_pcc_setup(struct net_device *ndev)
+{
+ ndev->type = ARPHRD_MCTP;
+ ndev->hard_header_len = sizeof(struct acpi_pcct_ext_pcc_shared_memory);
+ ndev->tx_queue_len = DEFAULT_TX_QUEUE_LEN;
+ ndev->flags = IFF_NOARP;
+ ndev->netdev_ops = &mctp_pcc_netdev_ops;
+ ndev->needs_free_netdev = true;
+ ndev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
+}
+
+struct mctp_pcc_lookup_context {
+ int index;
+ u32 inbox_index;
+ u32 outbox_index;
+};
+
+static acpi_status lookup_pcct_indices(struct acpi_resource *ares,
+ void *context)
+{
+ struct mctp_pcc_lookup_context *luc = context;
+ struct acpi_resource_address32 *addr;
+
+ if (ares->type != ACPI_RESOURCE_TYPE_ADDRESS32)
+ return AE_OK;
+
+ addr = ACPI_CAST_PTR(struct acpi_resource_address32, &ares->data);
+ switch (luc->index) {
+ case 0:
+ luc->outbox_index = addr[0].address.minimum;
+ break;
+ case 1:
+ luc->inbox_index = addr[0].address.minimum;
+ break;
+ default:
+ return AE_ERROR;
+ }
+ luc->index++;
+ return AE_OK;
+}
+
+static void mctp_cleanup_netdev(void *data)
+{
+ struct net_device *ndev = data;
+
+ mctp_unregister_netdev(ndev);
+}
+
+static int check_channel_types(struct mctp_pcc_ndev *mctp_pcc_ndev)
+{
+ struct mctp_pcc_mailbox *outbox;
+ struct mctp_pcc_mailbox *inbox;
+ struct pcc_mbox_chan chan;
+ int actual_type;
+
+ outbox = &mctp_pcc_ndev->outbox;
+ if (pcc_mbox_query_channel(&chan, outbox->index))
+ return -EINVAL;
+ actual_type = chan.type;
+ if (actual_type != ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE) {
+ pr_err("MCTP-PCC outbox channel wrong type: %d", actual_type);
+ return -EINVAL;
+ }
+
+ inbox = &mctp_pcc_ndev->inbox;
+ if (pcc_mbox_query_channel(&chan, inbox->index))
+ return -EINVAL;
+ actual_type = chan.type;
+ if (actual_type != ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE) {
+ pr_err("MCTP-PCC inbox channel wrong type: %d", actual_type);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int initialize_mtu(struct net_device *ndev)
+{
+ struct mctp_pcc_ndev *mctp_pcc_ndev;
+ struct mctp_pcc_mailbox *outbox;
+ struct mctp_pcc_mailbox *inbox;
+ struct pcc_mbox_chan out_chan;
+ struct pcc_mbox_chan in_chan;
+ int mctp_pcc_max_mtu;
+ int inbox_max_mtu;
+
+ mctp_pcc_ndev = netdev_priv(ndev);
+ outbox = &mctp_pcc_ndev->outbox;
+ if (pcc_mbox_query_channel(&out_chan, outbox->index))
+ return -EINVAL;
+ if (out_chan.shmem_size < MCTP_MIN_MTU + sizeof(struct acpi_pcct_ext_pcc_shared_memory))
+ return -EINVAL;
+ mctp_pcc_max_mtu = out_chan.shmem_size - sizeof(struct acpi_pcct_ext_pcc_shared_memory);
+ inbox = &mctp_pcc_ndev->inbox;
+ if (pcc_mbox_query_channel(&in_chan, inbox->index))
+ return -EINVAL;
+ if (in_chan.shmem_size < MCTP_MIN_MTU + sizeof(struct acpi_pcct_ext_pcc_shared_memory))
+ return -EINVAL;
+ inbox_max_mtu = in_chan.shmem_size - sizeof(struct acpi_pcct_ext_pcc_shared_memory);
+
+ if (inbox_max_mtu < mctp_pcc_max_mtu)
+ mctp_pcc_max_mtu = inbox_max_mtu;
+
+ ndev->mtu = MCTP_MIN_MTU;
+ ndev->max_mtu = mctp_pcc_max_mtu;
+ ndev->min_mtu = MCTP_MIN_MTU;
+
+ return 0;
+}
+
+static int mctp_pcc_driver_add(struct acpi_device *acpi_dev)
+{
+ struct mctp_pcc_lookup_context context = {0};
+ struct mctp_pcc_ndev *mctp_pcc_ndev;
+ struct device *dev = &acpi_dev->dev;
+ struct net_device *ndev;
+ acpi_handle dev_handle;
+ acpi_status status;
+ char name[32];
+ int rc;
+
+ dev_dbg(dev, "Adding mctp_pcc device for HID %s\n",
+ acpi_device_hid(acpi_dev));
+ dev_handle = acpi_device_handle(acpi_dev);
+ status = acpi_walk_resources(dev_handle, "_CRS", lookup_pcct_indices,
+ &context);
+ if (!ACPI_SUCCESS(status)) {
+ dev_err(dev, "FAILED to lookup PCC indexes from CRS\n");
+ return -EINVAL;
+ }
+
+ /*
+ * Ensure we have exactly 2 channels: an outbox and an inbox.
+ */
+ if (context.index != 2)
+ return -EINVAL;
+
+ snprintf(name, sizeof(name), "mctppcc%d", context.inbox_index);
+ ndev = alloc_netdev(sizeof(*mctp_pcc_ndev), name, NET_NAME_PREDICTABLE,
+ mctp_pcc_setup);
+ if (!ndev)
+ return -ENOMEM;
+
+ mctp_pcc_ndev = netdev_priv(ndev);
+ mctp_pcc_ndev->inbox.index = context.inbox_index;
+ mctp_pcc_ndev->inbox.client.dev = dev;
+ mctp_pcc_ndev->outbox.index = context.outbox_index;
+ mctp_pcc_ndev->outbox.client.dev = dev;
+
+ mctp_pcc_ndev->outbox.client.tx_prepare = mctp_pcc_tx_prepare;
+ mctp_pcc_ndev->outbox.client.tx_done = mctp_pcc_tx_done;
+ mctp_pcc_ndev->acpi_device = acpi_dev;
+ mctp_pcc_ndev->ndev = ndev;
+ acpi_dev->driver_data = mctp_pcc_ndev;
+ rc = check_channel_types(mctp_pcc_ndev);
+ if (rc != 0)
+ goto free_netdev;
+
+ rc = initialize_mtu(ndev);
+ if (rc)
+ goto free_netdev;
+
+ rc = mctp_register_netdev(ndev, NULL, MCTP_PHYS_BINDING_PCC);
+ if (rc)
+ goto free_netdev;
+
+ return devm_add_action_or_reset(dev, mctp_cleanup_netdev, ndev);
+free_netdev:
+ free_netdev(ndev);
+ return rc;
+}
+
+static const struct acpi_device_id mctp_pcc_device_ids[] = {
+ { "DMT0001" },
+ {}
+};
+
+static struct acpi_driver mctp_pcc_driver = {
+ .name = "mctp_pcc",
+ .class = "Unknown",
+ .ids = mctp_pcc_device_ids,
+ .ops = {
+ .add = mctp_pcc_driver_add,
+ },
+};
+
+module_acpi_driver(mctp_pcc_driver);
+
+MODULE_DEVICE_TABLE(acpi, mctp_pcc_device_ids);
+
+MODULE_DESCRIPTION("MCTP PCC ACPI device");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Adam Young <admiyo@os.amperecomputing.com>");
--
2.43.0
^ permalink raw reply related
* [PATCH v45 6/7] synchronize IRQ before releasing shared memory
From: Adam Young @ 2026-07-21 17:52 UTC (permalink / raw)
To: Sudeep Holla, Jassi Brar
Cc: netdev, linux-kernel, Jeremy Kerr, Matt Johnston,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Sudeep Holla, Jonathan Cameron, Huisong Li
In-Reply-To: <20260721175258.87600-1-admiyo@os.amperecomputing.com>
Make sure a final interrupt request does not
asccess the shared buffer after it has been release.
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
drivers/mailbox/pcc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index b9a01bfdc95d..2ce2afd255f0 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -537,6 +537,7 @@ static void pcc_shutdown(struct mbox_chan *chan)
if (pchan->plat_irq > 0)
devm_free_irq(chan->mbox->dev, pchan->plat_irq, chan);
+ synchronize_irq(pchan->plat_irq);
pcc_mbox_chan = &pchan->chan;
if (pcc_mbox_chan->shmem) {
--
2.43.0
^ permalink raw reply related
* [PATCH v45 7/7] wrap pchan->chan_in_use in READ/WRITE_ONCE
From: Adam Young @ 2026-07-21 17:52 UTC (permalink / raw)
To: Sudeep Holla, Jassi Brar
Cc: netdev, linux-kernel, Jeremy Kerr, Matt Johnston,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Sudeep Holla, Jonathan Cameron, Huisong Li
In-Reply-To: <20260721175258.87600-1-admiyo@os.amperecomputing.com>
read/write happening from both userspace and Hard IRQ context
chan_in_use is used a flag to sychronize access. Volitile
semantics ensure we don't have a reordering that accidentally
provide dual access.
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
drivers/mailbox/pcc.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 2ce2afd255f0..dbfe8dd92ddd 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -325,7 +325,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
return IRQ_NONE;
if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
- !pchan->chan_in_use)
+ !READ_ONCE(pchan->chan_in_use))
return IRQ_NONE;
if (!pcc_mbox_cmd_complete_check(pchan))
@@ -339,7 +339,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
* where the flag is set again to start new transfer. This is
* required to avoid any possible race in updatation of this flag.
*/
- pchan->chan_in_use = false;
+ WRITE_ONCE(pchan->chan_in_use, false);
if (!rc)
mbox_chan_received_data(chan, NULL);
mbox_chan_txdone(chan, rc);
@@ -471,7 +471,7 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
ret = pcc_chan_reg_read_modify_write(&pchan->db);
if (!ret && pchan->plat_irq > 0)
- pchan->chan_in_use = true;
+ WRITE_ONCE(pchan->chan_in_use, true);
return ret;
}
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next v5] selftests/net/openvswitch: add ICMPv6 echo type match test
From: patchwork-bot+netdevbpf @ 2026-07-21 18:00 UTC (permalink / raw)
To: Minxi Hou
Cc: netdev, aconole, echaudro, i.maximets, i.maximets, davem,
edumazet, kuba, pabeni, horms, shuah, dev, linux-kselftest
In-Reply-To: <20260709120541.3556748-1-houminxi@gmail.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 9 Jul 2026 08:05:41 -0400 you wrote:
> Register OVS_KEY_ATTR_ICMPV6 in the flow key parser so that
> icmpv6(type=...) can be used in flow specifications. Without this
> registration the parser silently drops the token and the kernel
> rejects the flow with EINVAL because the expected ICMPv6 key
> attribute is missing.
>
> While here, add convert_int() to the ovs_key_ipv6 and ovs_key_icmp
> fields_map entries so that specifying a field value produces the
> correct wildcard mask. The IPv6 flow label uses convert_int(20) to
> produce a 20-bit mask (0x000FFFFF), matching the kernel constraint in
> flow_netlink.c that rejects masks with bits 20-31 set; byte-wide
> fields use convert_int(8). The ipv4 counterpart already does this via
> convert_int(); the ipv6 and icmp classes were simply missing the fifth
> tuple element. Existing callers that pass empty parentheses are
> unaffected because convert_int("") returns (0, 0).
>
> [...]
Here is the summary with links:
- [net-next,v5] selftests/net/openvswitch: add ICMPv6 echo type match test
https://git.kernel.org/netdev/net-next/c/6deab902b4c0
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* [PATCH net-next v2 0/2] net: macb: Updated patches for 1000BASE-X support
From: Nathan Whitehorn @ 2026-07-21 17:56 UTC (permalink / raw)
To: netdev; +Cc: theo.lebrun, conor.dooley, charles.perry, andrew
In-Reply-To: <20260714200904.70428-1-nwhitehorn@pa.msu.edu>
This series adds support for 1000BASE-X autonegotiation to the Cadence macb driver
when using the MAC-internal PCS. The existing driver code is oriented toward the PCS
being used with an on-board SGMII PHY, so uses Cisco SGMII-style autonegotiation
exclusively and does not anticipate e.g. link state changes arising from fiber
attach/detach events. The first patch changes the driver to monitor the PCS's
link; the second extends the existing SGMII autonegotiation code to also support
1000BASE-X autonegotiation.
Thanks to Charles Perry and Andrew Lunn for comments on the initial revision of
this patch.
-Nathan
^ permalink raw reply
* [PATCH net-next v2 1/2] net: macb: Poll for link state changes when using the internal PCS.
From: Nathan Whitehorn @ 2026-07-21 17:56 UTC (permalink / raw)
To: netdev; +Cc: theo.lebrun, conor.dooley, charles.perry, andrew,
Nathan Whitehorn
In-Reply-To: <20260721180158.13014-1-nwhitehorn@pa.msu.edu>
There is current support for signalling to software when this changes
via an interrupt etc., so poll once per second. This is required when
there is no external PHY (including an SGMII-attached PHY) to convey
link state information to the MAC.
Signed-off-by: Nathan Whitehorn <nwhitehorn@pa.msu.edu>
---
drivers/net/ethernet/cadence/macb_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index d394f1f43b68..c15a9c7e69d3 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1025,6 +1025,7 @@ static int macb_mii_probe(struct net_device *dev)
struct macb *bp = netdev_priv(dev);
bp->phylink_sgmii_pcs.ops = &macb_phylink_pcs_ops;
+ bp->phylink_sgmii_pcs.poll = true;
bp->phylink_usx_pcs.ops = &macb_phylink_usx_pcs_ops;
bp->phylink_config.dev = &dev->dev;
--
2.54.0
^ permalink raw reply related
* [PATCH net-next v2 2/2] net: macb: add support for 1000BASE-X autonegotiation to PCS
From: Nathan Whitehorn @ 2026-07-21 17:56 UTC (permalink / raw)
To: netdev; +Cc: theo.lebrun, conor.dooley, charles.perry, andrew,
Nathan Whitehorn
In-Reply-To: <20260721180158.13014-1-nwhitehorn@pa.msu.edu>
The current PCS code unconditionally uses SGMII autonegotiation, though
the hardware supports both SGMII and 1000BASE-X modes. Decouple the
choice of PCS enablement from use of the SGMII mode when running at
gigabit rates and announce to phylink that 1000BASE-X is a supported
operating mode. This enables direct attachment of the PCS to e.g. an
SFP.
The 1000BASE-X code in phylink also sometimes calls the autonegotiation
restart method, so add an implementation of autonegotiation restart.
Signed-off-by: Nathan Whitehorn <nwhitehorn@pa.msu.edu>
---
drivers/net/ethernet/cadence/macb_main.c | 26 ++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index c15a9c7e69d3..310ad8eac21f 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -583,7 +583,12 @@ static void macb_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
static void macb_pcs_an_restart(struct phylink_pcs *pcs)
{
- /* Not supported */
+ struct macb *bp = container_of(pcs, struct macb, phylink_sgmii_pcs);
+ u32 old, new;
+
+ old = gem_readl(bp, PCSCNTRL);
+ new = old | BMCR_ANRESTART;
+ gem_writel(bp, PCSCNTRL, new);
}
static int macb_pcs_config(struct phylink_pcs *pcs,
@@ -750,7 +755,9 @@ static void macb_mac_config(struct phylink_config *config, unsigned int mode,
ctrl &= ~(GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL));
ncr &= ~GEM_BIT(ENABLE_HS_MAC);
- if (state->interface == PHY_INTERFACE_MODE_SGMII) {
+ if (state->interface == PHY_INTERFACE_MODE_1000BASEX) {
+ ctrl |= GEM_BIT(PCSSEL);
+ } else if (state->interface == PHY_INTERFACE_MODE_SGMII) {
ctrl |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
} else if (state->interface == PHY_INTERFACE_MODE_10GBASER) {
ctrl |= GEM_BIT(PCSSEL);
@@ -957,7 +964,8 @@ static struct phylink_pcs *macb_mac_select_pcs(struct phylink_config *config,
if (interface == PHY_INTERFACE_MODE_10GBASER)
return &bp->phylink_usx_pcs;
- else if (interface == PHY_INTERFACE_MODE_SGMII)
+ else if (interface == PHY_INTERFACE_MODE_1000BASEX ||
+ interface == PHY_INTERFACE_MODE_SGMII)
return &bp->phylink_sgmii_pcs;
else
return NULL;
@@ -1032,7 +1040,8 @@ static int macb_mii_probe(struct net_device *dev)
bp->phylink_config.type = PHYLINK_NETDEV;
bp->phylink_config.mac_managed_pm = true;
- if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
+ if (bp->phy_interface == PHY_INTERFACE_MODE_1000BASEX ||
+ bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
bp->phylink_config.poll_fixed_state = true;
bp->phylink_config.get_fixed_state = macb_get_pcs_fixed_state;
/* The PCSAUTONEG bit in PCSCNTRL is on out of reset. Setting
@@ -1061,9 +1070,12 @@ static int macb_mii_probe(struct net_device *dev)
bp->phylink_config.supported_interfaces);
phy_interface_set_rgmii(bp->phylink_config.supported_interfaces);
- if (bp->caps & MACB_CAPS_PCS)
+ if (bp->caps & MACB_CAPS_PCS) {
+ __set_bit(PHY_INTERFACE_MODE_1000BASEX,
+ bp->phylink_config.supported_interfaces);
__set_bit(PHY_INTERFACE_MODE_SGMII,
bp->phylink_config.supported_interfaces);
+ }
if (bp->caps & MACB_CAPS_HIGH_SPEED) {
__set_bit(PHY_INTERFACE_MODE_10GBASER,
@@ -4932,7 +4944,9 @@ static int macb_init_dflt(struct platform_device *pdev)
/* Set MII management clock divider */
val = macb_mdc_clk_div(bp);
val |= macb_dbw(bp);
- if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
+ if (bp->phy_interface == PHY_INTERFACE_MODE_1000BASEX)
+ val |= GEM_BIT(PCSSEL);
+ else if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
val |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
macb_writel(bp, NCFGR, val);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v4 net 2/6] xsk: drain continuation descs after overflow in xsk_build_skb()
From: Maciej Fijalkowski @ 2026-07-21 18:04 UTC (permalink / raw)
To: netdev
Cc: bpf, magnus.karlsson, stfomichev, kuba, pabeni, horms, bjorn,
kerneljasonxing, Jason Xing
In-Reply-To: <20260719135609.147823-3-maciej.fijalkowski@intel.com>
On Sun, Jul 19, 2026 at 03:56:05PM +0200, Maciej Fijalkowski wrote:
> From: Jason Xing <kernelxing@tencent.com>
>
> Fix generic xmit path multi-buffer logic when packets are either too big
> (count of descriptors exceed MAX_SKB_FRAGS) or an invalid descriptor is
> included in fragmented packet. Introduce xdp_sock::drain_cont and act
> upon this flag - when it is set, keep on consuming descriptors from
> AF_XDP Tx ring and put them directly onto Cq. Previously these
> descriptors were silently lost and could never be reached again.
>
> Fixes: cf24f5a5feea ("xsk: add support for AF_XDP multi-buffer on Tx path")
> Closes: https://lore.kernel.org/all/20260425041726.85FB3C2BCB2@smtp.kernel.org/
> Reviewed-by: Jason Xing <kernelxing@tencent.com>
> Co-developed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> # wrapped cq addr submission onto routine
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> include/net/xdp_sock.h | 1 +
> net/xdp/xsk.c | 45 +++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index ebac60a3d8a1..8b51876efbed 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -80,6 +80,7 @@ struct xdp_sock {
> * call of __xsk_generic_xmit().
> */
> struct sk_buff *skb;
> + bool drain_cont;
>
> struct list_head map_list;
> /* Protects map_list */
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index a7a83dc4546a..12a845d012f6 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -737,6 +737,19 @@ static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool,
> spin_unlock_irqrestore(&pool->cq_prod_lock, flags);
> }
>
> +static void xsk_cq_submit_addr_single_locked(struct xsk_buff_pool *pool,
> + struct xdp_desc *desc)
> +{
> + unsigned long flags;
> + u32 idx;
> +
> + spin_lock_irqsave(&pool->cq_prod_lock, flags);
> + idx = xskq_get_prod(pool->cq);
> + xskq_prod_write_addr(pool->cq, idx, desc->addr);
> + xskq_prod_submit_n(pool->cq, 1);
> + spin_unlock_irqrestore(&pool->cq_prod_lock, flags);
> +}
> +
> static void xsk_cq_cancel_locked(struct xsk_buff_pool *pool, u32 n)
> {
> spin_lock(&pool->cq->cq_cached_prod_lock);
> @@ -1028,13 +1041,14 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
> static int __xsk_generic_xmit(struct sock *sk)
> {
> struct xdp_sock *xs = xdp_sk(sk);
> - bool sent_frame = false;
> struct xdp_desc desc;
> struct sk_buff *skb;
> + u32 cached_cons;
> u32 max_batch;
> int err = 0;
>
> mutex_lock(&xs->mutex);
> + cached_cons = xs->tx->cached_cons;
>
> /* Since we dropped the RCU read lock, the socket state might have changed. */
> if (unlikely(!xsk_is_bound(xs))) {
> @@ -1063,11 +1077,21 @@ static int __xsk_generic_xmit(struct sock *sk)
> goto out;
> }
>
> + if (unlikely(xs->drain_cont)) {
> + xsk_cq_submit_addr_single_locked(xs->pool, &desc);
> + xs->tx->invalid_descs++;
> + xskq_cons_release(xs->tx);
> + xs->drain_cont = xp_mb_desc(&desc);
> + continue;
> + }
> +
> skb = xsk_build_skb(xs, &desc);
> if (IS_ERR(skb)) {
> err = PTR_ERR(skb);
> if (err != -EOVERFLOW)
> goto out;
> + if (xp_mb_desc(&desc))
> + xs->drain_cont = true;
> err = 0;
> continue;
> }
> @@ -1096,18 +1120,33 @@ static int __xsk_generic_xmit(struct sock *sk)
> goto out;
> }
>
> - sent_frame = true;
> xs->skb = NULL;
> }
>
> if (xskq_has_descs(xs->tx)) {
> + bool drain = xs->skb || xs->drain_cont || xp_mb_desc(&desc);
> +
> + err = xsk_cq_reserve_locked(xs->pool);
> + if (err) {
> + xs->tx->invalid_descs--;
> + if (xs->skb)
> + xsk_drop_skb(xs->skb);
> + xs->drain_cont = drain;
> + err = -EAGAIN;
> + goto out;
> + }
> +
> if (xs->skb)
> xsk_drop_skb(xs->skb);
> +
> + xsk_cq_submit_addr_single_locked(xs->pool, &desc);
> +
> xskq_cons_release(xs->tx);
> + xs->drain_cont = xp_mb_desc(&desc);
> }
>
> out:
> - if (sent_frame)
> + if (xs->tx->cached_cons != cached_cons)
Sashiko says:
[Severity: High]
This isn't a bug introduced by this patch, but does this change to the
release condition expose a pre-existing issue where rolling back a partial
multi-buffer packet corrupts the Tx ring?
During multi-buffer packet assembly, if the max_batch limit is reached,
__xsk_generic_xmit() exits and commits the partially consumed descriptors
to ring->consumer. User-space observes this and may overwrite those slots.
On the next call to __xsk_generic_xmit(), packet assembly resumes. If
__dev_direct_xmit() subsequently returns NETDEV_TX_BUSY, the code calls
xskq_cons_cancel_n() to rewind the local cached_cons by the full packet
length:
if (err == NETDEV_TX_BUSY) {
/* Tell MAC device to consume more and retry later. */
xskq_cons_cancel_n(xs->tx, xsk_get_num_desc(skb));
...
Because cached_cons is rewound by the full length (including fragments
consumed in the previous call), it becomes smaller than the cached_cons
saved at the start of this current call.
Will this new condition (xs->tx->cached_cons != cached_cons) then evaluate
to true and commit the rewound index to the globally visible ring->consumer?
If ring->consumer jumps backwards, wouldn't the kernel fetch descriptors
that user-space may have already overwritten with new data upon retry?
Maciej says:
So generic xmit is still not bullet-proof, sigh. It's a problem that was
present even when `sent_frame` based consumer pointer update was used, so
sashiko correctly classified it as pre-existing issue.
I think this can be addressed after current set lands, as no new bugs are
introduced and seems it got acks from Stan and Jason.
> __xsk_tx_release(xs);
>
> mutex_unlock(&xs->mutex);
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH 3/3 net-next] ipv6: add CAP_NET_ADMIN check for forwarding and force_forwarding sysctl
From: Ido Schimmel @ 2026-07-21 18:05 UTC (permalink / raw)
To: Fernando Fernandez Mancera
Cc: netdev, horms, pabeni, kuba, edumazet, davem, dsahern
In-Reply-To: <4087bf3c-90ac-40d3-9c46-f0f95a60284c@suse.de>
On Tue, Jul 21, 2026 at 03:46:38PM +0200, Fernando Fernandez Mancera wrote:
> On 7/21/26 2:24 PM, Ido Schimmel wrote:
> > On Thu, Jul 16, 2026 at 10:37:13PM +0200, Fernando Fernandez Mancera wrote:
> > > As commit 8292d7f6e871 ("net: ipv4: add capability check for net
> > > administration") did for IPv4, make sure that CAP_NET_ADMIN is required
> > > to modify IPv6 forwarding and force_forwarding sysctl. This keep the
> > > consistency of permission check logic between both protocols.
> >
> > What's the value beyond protocol consistency given that it creates
> > intra-protocol inconsistency (none of the other IPv6 sysctl handlers
> > have this check)? 8292d7f6e871 does not explain why only forwarding was
> > changed. As-is, I don't see much value in this change. Tested using
> > [1].
> >
>
> Ideally, all IPv6 and IPv4 should require CAP_NET_ADMIN as they are indeed
> configuring networking interface configuration. I thought changing all of
> them all of a sudden would be too much so I went for forwarding only.
>
> As to answer why it was done for IPv4 in first place, I guess it was done
> because forwarding is the most popular option and also quite relevant in
> terms of impact in the system behaviour.
>
> If we don't want to merge this as it is, I see 3 ways forward:
>
> 1. drop this patch from the series
> 2. drop the permission check in IPv4 forwarding sysctl
> 3. extend the check to all sysctls of IPv4 and IPv6
>
> Ideally I would go for 3. if it isn't too much in your opinion. Otherwise,
> likely 1.
Please drop the patch given that it's merely trying to be consistent
with a change whose scope was never justified. Option 3 is not something
that fits in v2 of "Misc. minor improvements on IPv4/IPv6 sysctl
handling" and requires careful analysis / breakage assessment.
^ permalink raw reply
* Re: [PATCH net-next] nfc: digital: fix use-after-free in nfc_digital_unregister_device()
From: Daniel Zahka @ 2026-07-21 18:07 UTC (permalink / raw)
To: Weiming Shi, David Heidelberg, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman
Cc: oe-linux-nfc, netdev, linux-kernel, Xiang Mei
In-Reply-To: <20260721163632.1570651-1-bestswngs@gmail.com>
On 7/21/26 12:36 PM, Weiming Shi wrote:
> nfc_digital_unregister_device() cancels cmd_work and cmd_complete_work
> once each and then frees the command queue. The two works re-arm each
> other: digital_wq_cmd_complete() ends with schedule_work(&ddev->cmd_work),
> and digital_wq_cmd() hands a command to the driver whose asynchronous
> completion schedules cmd_complete_work. cancel_work_sync() only waits for
> the instance it cancels; it does not stop the work from being queued again.
> A work re-armed after its cancel_work_sync() therefore runs concurrently
> with the cmd_queue cleanup and dereferences a digital_cmd the cleanup has
> already freed. digital_wq_cmd() widens the window by dropping cmd_lock
> before using the command it took from the queue, while the cleanup loop
> frees the commands without holding cmd_lock.
>
> It is reproducible with the software NFC simulator (CONFIG_NFC_SIM): start
> an NFC-DEP exchange between the two nfcsim devices and unload the module
> while it is running.
>
> BUG: KASAN: slab-use-after-free in digital_wq_cmd (net/nfc/digital_core.c:174)
> Read of size 1 by task kworker/1:5
> Workqueue: events digital_wq_cmd
> digital_wq_cmd (net/nfc/digital_core.c:174)
> process_one_work
> worker_thread
> kthread
>
> Allocated by task 5124:
> digital_send_cmd (net/nfc/digital_core.c:234)
> digital_in_send_sdd_req
> digital_in_recv_sens_res
> digital_wq_cmd_complete (net/nfc/digital_core.c:134)
>
> Freed by task 4994:
> kfree
> nfc_digital_unregister_device (net/nfc/digital_core.c:859)
> nfcsim_device_free [nfcsim]
> nfcsim_exit [nfcsim]
> __do_sys_delete_module
>
> Use disable_work_sync() instead of cancel_work_sync() for the two command
> works. disable_work_sync() cancels the work and disables it, so any later
> schedule_work() -- whether from the sibling work re-arming it or from the
> driver's completion callback -- becomes a no-op. Once both works are
> disabled no work can run, and the cleanup loop frees the queue with no work
> able to reach a freed command.
>
> Fixes: 59ee2361c924 ("NFC Digital: Implement driver commands mechanism")
Fix for this commit should target the net tree instead of net-next.
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> net/nfc/digital_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/nfc/digital_core.c b/net/nfc/digital_core.c
> index 7cb1e6aaae90..6def5132a4a6 100644
> --- a/net/nfc/digital_core.c
> +++ b/net/nfc/digital_core.c
> @@ -843,8 +843,8 @@ void nfc_digital_unregister_device(struct nfc_digital_dev *ddev)
> mutex_unlock(&ddev->poll_lock);
>
> cancel_delayed_work_sync(&ddev->poll_work);
> - cancel_work_sync(&ddev->cmd_work);
> - cancel_work_sync(&ddev->cmd_complete_work);
> + disable_work_sync(&ddev->cmd_work);
> + disable_work_sync(&ddev->cmd_complete_work);
>
> list_for_each_entry_safe(cmd, n, &ddev->cmd_queue, queue) {
> list_del(&cmd->queue);
>
> base-commit: d4932951a19a5f1ec93200260b85e1a4c080ff77
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox