* Re: [PATCH v4 2/9] serdev: Add an API to find the serdev controller associated with the devicetree node
From: Bartosz Golaszewski @ 2026-01-13 13:54 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Rob Herring, Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor,
Nicolas Schier, Hans de Goede, Ilpo Järvinen, Mark Pearson,
Derek J. Clark, Manivannan Sadhasivam, Krzysztof Kozlowski,
Conor Dooley, Marcel Holtmann, Luiz Augusto von Dentz,
Bartosz Golaszewski, Andy Shevchenko, Bartosz Golaszewski,
Manivannan Sadhasivam via B4 Relay, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi
In-Reply-To: <20260112-pci-m2-e-v4-2-eff84d2c6d26@oss.qualcomm.com>
On Mon, 12 Jan 2026 17:26:01 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> Add of_find_serdev_controller_by_node() API to find the serdev controller
> device associated with the devicetree node.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> drivers/tty/serdev/core.c | 16 ++++++++++++++++
> include/linux/serdev.h | 9 +++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index b33e708cb245..25382c2d63e6 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -504,6 +504,22 @@ struct serdev_controller *serdev_controller_alloc(struct device *host,
> }
> EXPORT_SYMBOL_GPL(serdev_controller_alloc);
>
> +/**
> + * of_find_serdev_controller_by_node() - Find the serdev controller associated
> + * with the devicetree node
> + * @node: Devicetree node
> + *
> + * Return: Pointer to the serdev controller associated with the node. NULL if
> + * the controller is not found.
> + */
Please also say that the caller is responsible for calling
serdev_controller_put() on the returned object.
Bart
> +struct serdev_controller *of_find_serdev_controller_by_node(struct device_node *node)
> +{
> + struct device *dev = bus_find_device_by_of_node(&serdev_bus_type, node);
> +
> + return (dev && dev->type == &serdev_ctrl_type) ? to_serdev_controller(dev) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(of_find_serdev_controller_by_node);
> +
> static int of_serdev_register_devices(struct serdev_controller *ctrl)
> {
> struct device_node *node;
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index ecde0ad3e248..db9bfaba0662 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -333,4 +333,13 @@ static inline bool serdev_acpi_get_uart_resource(struct acpi_resource *ares,
> }
> #endif /* CONFIG_ACPI */
>
> +#ifdef CONFIG_OF
> +struct serdev_controller *of_find_serdev_controller_by_node(struct device_node *node);
> +#else
> +struct serdev_controller *of_find_serdev_controller_by_node(struct device_node *node)
> +{
> + return NULL;
> +}
> +#endif /* CONFIG_OF */
> +
> #endif /*_LINUX_SERDEV_H */
>
> --
> 2.48.1
>
>
>
^ permalink raw reply
* Re: [PATCH v4 4/9] dt-bindings: serial: Document the graph port
From: Bartosz Golaszewski @ 2026-01-13 13:55 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Manivannan Sadhasivam via B4 Relay, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Nathan Chancellor, Nicolas Schier, Hans de Goede,
Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Manivannan Sadhasivam, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Andy Shevchenko, Bartosz Golaszewski
In-Reply-To: <20260112-pci-m2-e-v4-4-eff84d2c6d26@oss.qualcomm.com>
On Mon, 12 Jan 2026 17:26:03 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> A serial controller could be connected to an external connector like PCIe
> M.2 for controlling the serial interface of the card. Hence, document the
> OF graph port.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/serial/serial.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/serial.yaml b/Documentation/devicetree/bindings/serial/serial.yaml
> index 6aa9cfae417b..96eb1de8771e 100644
> --- a/Documentation/devicetree/bindings/serial/serial.yaml
> +++ b/Documentation/devicetree/bindings/serial/serial.yaml
> @@ -87,6 +87,9 @@ properties:
> description:
> TX FIFO threshold configuration (in bytes).
>
> + port:
> + $ref: /schemas/graph.yaml#/properties/port
> +
> patternProperties:
> "^(bluetooth|bluetooth-gnss|embedded-controller|gnss|gps|mcu|onewire)$":
> if:
>
> --
> 2.48.1
>
>
>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v4 7/9] Bluetooth: hci_qca: Add M.2 Bluetooth device support using pwrseq
From: Bartosz Golaszewski @ 2026-01-13 13:56 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Rob Herring, Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor,
Nicolas Schier, Hans de Goede, Ilpo Järvinen, Mark Pearson,
Derek J. Clark, Manivannan Sadhasivam, Krzysztof Kozlowski,
Conor Dooley, Marcel Holtmann, Luiz Augusto von Dentz,
Bartosz Golaszewski, Andy Shevchenko, Bartosz Golaszewski,
Manivannan Sadhasivam via B4 Relay, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi
In-Reply-To: <20260112-pci-m2-e-v4-7-eff84d2c6d26@oss.qualcomm.com>
On Mon, 12 Jan 2026 17:26:06 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> Power supply to the M.2 Bluetooth device attached to the host using M.2
> connector is controlled using the 'uart' pwrseq device. So add support for
> getting the pwrseq device if the OF graph link is present. Once obtained,
> the existing pwrseq APIs can be used to control the power supplies of the
> M.2 card.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> drivers/bluetooth/hci_qca.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> index 888176b0faa9..4d562596ebf9 100644
> --- a/drivers/bluetooth/hci_qca.c
> +++ b/drivers/bluetooth/hci_qca.c
> @@ -26,6 +26,7 @@
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_graph.h>
> #include <linux/acpi.h>
> #include <linux/platform_device.h>
> #include <linux/pwrseq/consumer.h>
> @@ -2384,6 +2385,14 @@ static int qca_serdev_probe(struct serdev_device *serdev)
> case QCA_WCN6855:
> case QCA_WCN7850:
> case QCA_WCN6750:
> + if (of_graph_is_present(dev_of_node(&serdev->ctrl->dev))) {
> + qcadev->bt_power->pwrseq = devm_pwrseq_get(&serdev->ctrl->dev,
> + "uart");
> + if (IS_ERR(qcadev->bt_power->pwrseq))
> + return PTR_ERR(qcadev->bt_power->pwrseq);
> + break;
> + }
> +
> if (!device_property_present(&serdev->dev, "enable-gpios")) {
> /*
> * Backward compatibility with old DT sources. If the
>
> --
> 2.48.1
>
>
>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v4 12/15] arm64: dts: microchip: add LAN969x clock header file
From: Robert Marko @ 2026-01-13 13:57 UTC (permalink / raw)
To: claudiu beznea
Cc: robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni,
herbert, davem, vkoul, andi.shyti, lee, andrew+netdev, edumazet,
kuba, pabeni, linusw, Steen.Hegelund, daniel.machon,
UNGLinuxDriver, olivia, radu_nicolae.pirea, richard.genoud,
gregkh, jirislaby, broonie, lars.povlsen, devicetree,
linux-arm-kernel, linux-kernel, linux-crypto, dmaengine,
linux-i2c, netdev, linux-gpio, linux-spi, linux-serial, linux-usb,
luka.perkov
In-Reply-To: <2cdac084-4924-4ca2-85d6-2e6d9bf284aa@tuxon.dev>
On Sun, Jan 11, 2026 at 3:42 PM claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>
>
>
> On 12/29/25 20:37, Robert Marko wrote:
> > LAN969x uses hardware clock indexes, so document theses in a header to make
> > them humanly readable.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> > Changes in v4:
> > * Move clock indexes from dt-bindings to a DTS header
> >
> > Changes in v2:
> > * Rename file to microchip,lan9691.h
> >
> > arch/arm64/boot/dts/microchip/clk-lan9691.h | 24 +++++++++++++++++++++
> > 1 file changed, 24 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/microchip/clk-lan9691.h
> >
> > diff --git a/arch/arm64/boot/dts/microchip/clk-lan9691.h b/arch/arm64/boot/dts/microchip/clk-lan9691.h
> > new file mode 100644
> > index 000000000000..f0006a603747
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/microchip/clk-lan9691.h
> > @@ -0,0 +1,24 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>
> Shouldn't this use the same license as the dtsi including it?
Yes, its a mistake on my side, will fix it in v5.
Regards,
Robert
>
> > +
> > +#ifndef _DTS_CLK_LAN9691_H
> > +#define _DTS_CLK_LAN9691_H
> > +
> > +#define GCK_ID_QSPI0 0
> > +#define GCK_ID_QSPI2 1
> > +#define GCK_ID_SDMMC0 2
> > +#define GCK_ID_SDMMC1 3
> > +#define GCK_ID_MCAN0 4
> > +#define GCK_ID_MCAN1 5
> > +#define GCK_ID_FLEXCOM0 6
> > +#define GCK_ID_FLEXCOM1 7
> > +#define GCK_ID_FLEXCOM2 8
> > +#define GCK_ID_FLEXCOM3 9
> > +#define GCK_ID_TIMER 10
> > +#define GCK_ID_USB_REFCLK 11
> > +
> > +/* Gate clocks */
> > +#define GCK_GATE_USB_DRD 12
> > +#define GCK_GATE_MCRAMC 13
> > +#define GCK_GATE_HMATRIX 14
> > +
> > +#endif
>
--
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr
^ permalink raw reply
* Re: [PATCH v4 8/9] power: sequencing: pcie-m2: Add support for PCIe M.2 Key E connectors
From: Bartosz Golaszewski @ 2026-01-13 14:00 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Manivannan Sadhasivam via B4 Relay, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Nathan Chancellor, Nicolas Schier, Hans de Goede,
Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Manivannan Sadhasivam, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Andy Shevchenko, Bartosz Golaszewski
In-Reply-To: <20260112-pci-m2-e-v4-8-eff84d2c6d26@oss.qualcomm.com>
On Mon, 12 Jan 2026 17:26:07 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> Add support for handling the power sequence of the PCIe M.2 Key E
> connectors. These connectors are used to attach the Wireless Connectivity
> devices to the host machine including combinations of WiFi, BT, NFC using
> interfaces such as PCIe/SDIO for WiFi, USB/UART for BT and I2C for NFC.
>
> Currently, this driver supports only the PCIe interface for WiFi and UART
> interface for BT. The driver also only supports driving the 3.3v/1.8v power
> supplies and W_DISABLE{1/2}# GPIOs. The optional signals of the Key E
> connectors are not currently supported.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> drivers/power/sequencing/Kconfig | 1 +
> drivers/power/sequencing/pwrseq-pcie-m2.c | 110 ++++++++++++++++++++++++++++--
> 2 files changed, 104 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> index f5fff84566ba..29bd204319cc 100644
> --- a/drivers/power/sequencing/Kconfig
> +++ b/drivers/power/sequencing/Kconfig
> @@ -38,6 +38,7 @@ config POWER_SEQUENCING_TH1520_GPU
> config POWER_SEQUENCING_PCIE_M2
> tristate "PCIe M.2 connector power sequencing driver"
> depends on OF || COMPILE_TEST
> + depends on PCI
Now we can no longer compile-test it without PCI, I don't think this was the
goal? Maybe "depends on (PCI && OF) || COMPILE_TEST"?
> help
> Say Y here to enable the power sequencing driver for PCIe M.2
> connectors. This driver handles the power sequencing for the M.2
> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index e01e19123415..4b85a40d7692 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -4,12 +4,16 @@
> * Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> */
>
> +#include <linux/err.h>
> #include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_graph.h>
> #include <linux/of_platform.h>
> +#include <linux/pci.h>
> #include <linux/platform_device.h>
> #include <linux/pwrseq/provider.h>
> #include <linux/regulator/consumer.h>
> @@ -25,17 +29,19 @@ struct pwrseq_pcie_m2_ctx {
> const struct pwrseq_pcie_m2_pdata *pdata;
> struct regulator_bulk_data *regs;
> size_t num_vregs;
> - struct notifier_block nb;
Should this even be part of [1] at all then? It doesn't seem used now when
I looked again?
> + struct gpio_desc *w_disable1_gpio;
> + struct gpio_desc *w_disable2_gpio;
> + struct device *dev;
> };
>
> -static int pwrseq_pcie_m2_m_vregs_enable(struct pwrseq_device *pwrseq)
> +static int pwrseq_pcie_m2_vregs_enable(struct pwrseq_device *pwrseq)
> {
> struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
>
> return regulator_bulk_enable(ctx->num_vregs, ctx->regs);
> }
>
> -static int pwrseq_pcie_m2_m_vregs_disable(struct pwrseq_device *pwrseq)
> +static int pwrseq_pcie_m2_vregs_disable(struct pwrseq_device *pwrseq)
> {
> struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
>
> @@ -44,18 +50,84 @@ static int pwrseq_pcie_m2_m_vregs_disable(struct pwrseq_device *pwrseq)
>
> static const struct pwrseq_unit_data pwrseq_pcie_m2_vregs_unit_data = {
> .name = "regulators-enable",
> - .enable = pwrseq_pcie_m2_m_vregs_enable,
> - .disable = pwrseq_pcie_m2_m_vregs_disable,
> + .enable = pwrseq_pcie_m2_vregs_enable,
> + .disable = pwrseq_pcie_m2_vregs_disable,
> };
>
> -static const struct pwrseq_unit_data *pwrseq_pcie_m2_m_unit_deps[] = {
> +static const struct pwrseq_unit_data *pwrseq_pcie_m2_unit_deps[] = {
> &pwrseq_pcie_m2_vregs_unit_data,
> NULL
> };
>
> +static int pwrseq_pci_m2_e_uart_enable(struct pwrseq_device *pwrseq)
> +{
> + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> + return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 0);
> +}
> +
> +static int pwrseq_pci_m2_e_uart_disable(struct pwrseq_device *pwrseq)
> +{
> + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> + return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 1);
> +}
> +
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_uart_unit_data = {
> + .name = "uart-enable",
> + .deps = pwrseq_pcie_m2_unit_deps,
> + .enable = pwrseq_pci_m2_e_uart_enable,
> + .disable = pwrseq_pci_m2_e_uart_disable,
> +};
> +
> +static int pwrseq_pci_m2_e_pcie_enable(struct pwrseq_device *pwrseq)
> +{
> + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> + return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 0);
> +}
> +
> +static int pwrseq_pci_m2_e_pcie_disable(struct pwrseq_device *pwrseq)
> +{
> + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> + return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 1);
> +}
> +
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_pcie_unit_data = {
> + .name = "pcie-enable",
> + .deps = pwrseq_pcie_m2_unit_deps,
> + .enable = pwrseq_pci_m2_e_pcie_enable,
> + .disable = pwrseq_pci_m2_e_pcie_disable,
> +};
> +
> static const struct pwrseq_unit_data pwrseq_pcie_m2_m_pcie_unit_data = {
> .name = "pcie-enable",
> - .deps = pwrseq_pcie_m2_m_unit_deps,
> + .deps = pwrseq_pcie_m2_unit_deps,
> +};
> +
> +static int pwrseq_pcie_m2_e_pwup_delay(struct pwrseq_device *pwrseq)
> +{
> + /*
> + * FIXME: This delay is only required for some Qcom WLAN/BT cards like
> + * WCN7850 and not for all devices. But currently, there is no way to
> + * identify the device model before enumeration.
> + */
> + msleep(50);
> +
> + return 0;
> +}
> +
> +static const struct pwrseq_target_data pwrseq_pcie_m2_e_uart_target_data = {
> + .name = "uart",
> + .unit = &pwrseq_pcie_m2_e_uart_unit_data,
> + .post_enable = pwrseq_pcie_m2_e_pwup_delay,
> +};
> +
> +static const struct pwrseq_target_data pwrseq_pcie_m2_e_pcie_target_data = {
> + .name = "pcie",
> + .unit = &pwrseq_pcie_m2_e_pcie_unit_data,
> + .post_enable = pwrseq_pcie_m2_e_pwup_delay,
> };
>
> static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
> @@ -63,11 +135,21 @@ static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
> .unit = &pwrseq_pcie_m2_m_pcie_unit_data,
> };
>
> +static const struct pwrseq_target_data *pwrseq_pcie_m2_e_targets[] = {
> + &pwrseq_pcie_m2_e_pcie_target_data,
> + &pwrseq_pcie_m2_e_uart_target_data,
> + NULL
> +};
> +
> static const struct pwrseq_target_data *pwrseq_pcie_m2_m_targets[] = {
> &pwrseq_pcie_m2_m_pcie_target_data,
> NULL
> };
>
> +static const struct pwrseq_pcie_m2_pdata pwrseq_pcie_m2_e_of_data = {
> + .targets = pwrseq_pcie_m2_e_targets,
> +};
> +
> static const struct pwrseq_pcie_m2_pdata pwrseq_pcie_m2_m_of_data = {
> .targets = pwrseq_pcie_m2_m_targets,
> };
> @@ -126,6 +208,16 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> return dev_err_probe(dev, ret,
> "Failed to get all regulators\n");
>
> + ctx->w_disable1_gpio = devm_gpiod_get_optional(dev, "w-disable1", GPIOD_OUT_HIGH);
> + if (IS_ERR(ctx->w_disable1_gpio))
> + return dev_err_probe(dev, PTR_ERR(ctx->w_disable1_gpio),
> + "Failed to get the W_DISABLE_1# GPIO\n");
> +
> + ctx->w_disable2_gpio = devm_gpiod_get_optional(dev, "w-disable2", GPIOD_OUT_HIGH);
> + if (IS_ERR(ctx->w_disable2_gpio))
> + return dev_err_probe(dev, PTR_ERR(ctx->w_disable2_gpio),
> + "Failed to get the W_DISABLE_2# GPIO\n");
> +
> ctx->num_vregs = ret;
>
> ret = devm_add_action_or_reset(dev, pwrseq_pcie_free_resources, ctx);
> @@ -151,6 +243,10 @@ static const struct of_device_id pwrseq_pcie_m2_of_match[] = {
> .compatible = "pcie-m2-m-connector",
> .data = &pwrseq_pcie_m2_m_of_data,
> },
> + {
> + .compatible = "pcie-m2-e-connector",
> + .data = &pwrseq_pcie_m2_e_of_data,
> + },
> { }
> };
> MODULE_DEVICE_TABLE(of, pwrseq_pcie_m2_of_match);
>
> --
> 2.48.1
>
>
>
Otherwise LGTM.
Bart
[1] https://lore.kernel.org/all/20260107-pci-m2-v5-5-8173d8a72641@oss.qualcomm.com/
^ permalink raw reply
* Re: [PATCH v4 9/9] power: sequencing: pcie-m2: Create serdev device for WCN7850 bluetooth
From: Bartosz Golaszewski @ 2026-01-13 14:11 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: Manivannan Sadhasivam via B4 Relay, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Nathan Chancellor, Nicolas Schier, Hans de Goede,
Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Manivannan Sadhasivam, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Andy Shevchenko, Bartosz Golaszewski
In-Reply-To: <20260112-pci-m2-e-v4-9-eff84d2c6d26@oss.qualcomm.com>
On Mon, 12 Jan 2026 17:26:08 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> For supporting bluetooth over the non-discoverable UART interface of
> WCN7850, create the serdev device after enumerating the PCIe interface.
> This is mandatory since the device ID is only known after the PCIe
> enumeration and the ID is used for creating the serdev device.
>
> Since by default there is no OF or ACPI node for the created serdev,
> create a dynamic OF 'bluetooth' node with the 'compatible' property and
> attach it to the serdev device. This will allow the serdev device to bind
> to the existing bluetooth driver.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> drivers/power/sequencing/pwrseq-pcie-m2.c | 170 +++++++++++++++++++++++++++++-
> 1 file changed, 169 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index 4b85a40d7692..5f9232e6c700 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -17,6 +17,7 @@
> #include <linux/platform_device.h>
> #include <linux/pwrseq/provider.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/serdev.h>
> #include <linux/slab.h>
>
> struct pwrseq_pcie_m2_pdata {
> @@ -32,6 +33,9 @@ struct pwrseq_pcie_m2_ctx {
> struct gpio_desc *w_disable1_gpio;
> struct gpio_desc *w_disable2_gpio;
> struct device *dev;
> + struct serdev_device *serdev;
> + struct notifier_block nb;
Ah, looks like this should have not appeared here before this patch?
> + struct of_changeset *ocs;
> };
>
> static int pwrseq_pcie_m2_vregs_enable(struct pwrseq_device *pwrseq)
> @@ -178,9 +182,169 @@ static void pwrseq_pcie_free_resources(void *data)
> {
> struct pwrseq_pcie_m2_ctx *ctx = data;
>
> + serdev_device_remove(ctx->serdev);
> + of_changeset_revert(ctx->ocs);
> + of_changeset_destroy(ctx->ocs);
Don't you need to depend on CONFIG_OF_DYNAMIC? These symbols are not
stubbed out.
> + bus_unregister_notifier(&pci_bus_type, &ctx->nb);
Shouldn't this happen earlier? What if a notification happens when you're
reverting previous changesets?
> regulator_bulk_free(ctx->num_vregs, ctx->regs);
> }
>
> +static int pwrseq_m2_pcie_create_bt_node(struct pwrseq_pcie_m2_ctx *ctx,
> + struct device_node *parent)
> +{
> + struct device *dev = ctx->dev;
> + struct device_node *np;
> + int ret;
Are we sure this will not happen twice for some reason?
> +
> + ctx->ocs = devm_kzalloc(dev, sizeof(*ctx->ocs), GFP_KERNEL);
> + if (!ctx->ocs)
> + return -ENOMEM;
> +
> + of_changeset_init(ctx->ocs);
> +
> + np = of_changeset_create_node(ctx->ocs, parent, "bluetooth");
> + if (!np) {
> + dev_err(dev, "Failed to create bluetooth node\n");
> + ret = -ENODEV;
> + goto err_destroy_changeset;
> + }
> +
> + ret = of_changeset_add_prop_string(ctx->ocs, np, "compatible", "qcom,wcn7850-bt");
> + if (ret) {
> + dev_err(dev, "Failed to add bluetooth compatible: %d\n", ret);
> + goto err_destroy_changeset;
> + }
> +
> + ret = of_changeset_apply(ctx->ocs);
> + if (ret) {
> + dev_err(dev, "Failed to apply changeset: %d\n", ret);
> + goto err_destroy_changeset;
> + }
> +
> + ret = device_add_of_node(&ctx->serdev->dev, np);
> + if (ret) {
> + dev_err(dev, "Failed to add OF node: %d\n", ret);
> + goto err_revert_changeset;
> + }
> +
> + return 0;
> +
> +err_revert_changeset:
> + of_changeset_revert(ctx->ocs);
> +err_destroy_changeset:
> + of_changeset_destroy(ctx->ocs);
> +
> + return ret;
> +}
> +
> +static int pwrseq_m2_pcie_notify(struct notifier_block *nb, unsigned long action,
> + void *data)
> +{
> + struct pwrseq_pcie_m2_ctx *ctx = container_of(nb, struct pwrseq_pcie_m2_ctx, nb);
> + struct pci_dev *pdev = to_pci_dev(data);
> + struct serdev_controller *serdev_ctrl;
> + struct device *dev = ctx->dev;
> + struct device_node *pci_parent;
> + int ret;
> +
> + /*
> + * Check whether the PCI device is associated with this M.2 connector or
> + * not, by comparing the OF node of the PCI device parent and the Port 0
> + * (PCIe) remote node parent OF node.
> + */
> + pci_parent = of_graph_get_remote_node(dev_of_node(ctx->dev), 0, 0);
struct device_node *pci_parent __free(device_node) ?
> + if (!pci_parent || (pci_parent != pdev->dev.parent->of_node)) {
> + of_node_put(pci_parent);
> + return NOTIFY_DONE;
> + }
> + of_node_put(pci_parent);
> +
> + switch (action) {
> + case BUS_NOTIFY_ADD_DEVICE:
> + /* Create serdev device for WCN7850 */
> + if (pdev->vendor == PCI_VENDOR_ID_QCOM && pdev->device == 0x1107) {
> + struct device_node *serdev_parent __free(device_node) =
> + of_graph_get_remote_node(dev_of_node(ctx->dev), 1, 1);
> + if (!serdev_parent)
> + return NOTIFY_DONE;
> +
> + serdev_ctrl = of_find_serdev_controller_by_node(serdev_parent);
> + if (!serdev_ctrl)
> + return NOTIFY_DONE;
> +
> + ctx->serdev = serdev_device_alloc(serdev_ctrl);
> + if (!ctx->serdev)
> + return NOTIFY_BAD;
> +
> + ret = pwrseq_m2_pcie_create_bt_node(ctx, serdev_parent);
> + if (ret) {
> + serdev_device_put(ctx->serdev);
> + return notifier_from_errno(ret);
> + }
> +
> + ret = serdev_device_add(ctx->serdev);
> + if (ret) {
> + dev_err(dev, "Failed to add serdev for WCN7850: %d\n", ret);
> + of_changeset_revert(ctx->ocs);
> + of_changeset_destroy(ctx->ocs);
You're almost always doing both, maybe it's time to add of_changeset_undo() or
something that wraps these?
> + serdev_device_put(ctx->serdev);
And since you're touching serdev, maybe DEFINE_FREE(serdev_device_put)?
> + return notifier_from_errno(ret);
Thanks for including this.
> + }
> + }
> + break;
> + case BUS_NOTIFY_REMOVED_DEVICE:
> + /* Destroy serdev device for WCN7850 */
> + if (pdev->vendor == PCI_VENDOR_ID_QCOM && pdev->device == 0x1107) {
> + serdev_device_remove(ctx->serdev);
> + of_changeset_revert(ctx->ocs);
> + of_changeset_destroy(ctx->ocs);
> + }
> + break;
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> +static bool pwrseq_pcie_m2_check_remote_node(struct device *dev, u8 port, u8 endpoint,
> + const char *node)
> +{
> + struct device_node *remote __free(device_node) =
> + of_graph_get_remote_node(dev_of_node(dev), port, endpoint);
> +
> + if (remote && of_node_name_eq(remote, node))
> + return true;
> +
> + return false;
> +}
> +
> +/*
> + * If the connector exposes a non-discoverable bus like UART, the respective
> + * protocol device needs to be created manually with the help of the notifier
> + * of the discoverable bus like PCIe.
> + */
> +static int pwrseq_pcie_m2_register_notifier(struct pwrseq_pcie_m2_ctx *ctx, struct device *dev)
> +{
> + int ret;
> +
> + /*
> + * Register a PCI notifier for Key E connector that has PCIe as Port
> + * 0/Endpoint 0 interface and Serial as Port 1/Endpoint 1 interface.
> + */
> + if (pwrseq_pcie_m2_check_remote_node(dev, 1, 1, "serial")) {
> + if (pwrseq_pcie_m2_check_remote_node(dev, 0, 0, "pcie")) {
> + ctx->dev = dev;
> + ctx->nb.notifier_call = pwrseq_m2_pcie_notify;
> + ret = bus_register_notifier(&pci_bus_type, &ctx->nb);
> + if (ret) {
> + dev_err_probe(dev, ret, "Failed to register notifier for serdev\n");
> + return ret;
Return dev_err_probe()?
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -235,7 +399,11 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(ctx->pwrseq),
> "Failed to register the power sequencer\n");
>
> - return 0;
> + /*
> + * Register a notifier for creating protocol devices for
> + * non-discoverable busses like UART.
> + */
> + return pwrseq_pcie_m2_register_notifier(ctx, dev);
> }
>
> static const struct of_device_id pwrseq_pcie_m2_of_match[] = {
>
> --
> 2.48.1
>
>
>
Bart
^ permalink raw reply
* Re: [PATCH v4 8/9] power: sequencing: pcie-m2: Add support for PCIe M.2 Key E connectors
From: Sean Anderson @ 2026-01-13 15:26 UTC (permalink / raw)
To: manivannan.sadhasivam, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Nathan Chancellor, Nicolas Schier, Hans de Goede,
Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Manivannan Sadhasivam, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Andy Shevchenko, Bartosz Golaszewski
Cc: linux-serial, linux-kernel, linux-kbuild, platform-driver-x86,
linux-pci, devicetree, linux-arm-msm, linux-bluetooth, linux-pm,
Stephan Gerhold, Dmitry Baryshkov, linux-acpi
In-Reply-To: <20260112-pci-m2-e-v4-8-eff84d2c6d26@oss.qualcomm.com>
On 1/12/26 11:26, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> Add support for handling the power sequence of the PCIe M.2 Key E
> connectors. These connectors are used to attach the Wireless Connectivity
> devices to the host machine including combinations of WiFi, BT, NFC using
> interfaces such as PCIe/SDIO for WiFi, USB/UART for BT and I2C for NFC.
>
> Currently, this driver supports only the PCIe interface for WiFi and UART
> interface for BT. The driver also only supports driving the 3.3v/1.8v power
> supplies and W_DISABLE{1/2}# GPIOs. The optional signals of the Key E
> connectors are not currently supported.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> drivers/power/sequencing/Kconfig | 1 +
> drivers/power/sequencing/pwrseq-pcie-m2.c | 110 ++++++++++++++++++++++++++++--
> 2 files changed, 104 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> index f5fff84566ba..29bd204319cc 100644
> --- a/drivers/power/sequencing/Kconfig
> +++ b/drivers/power/sequencing/Kconfig
> @@ -38,6 +38,7 @@ config POWER_SEQUENCING_TH1520_GPU
> config POWER_SEQUENCING_PCIE_M2
> tristate "PCIe M.2 connector power sequencing driver"
> depends on OF || COMPILE_TEST
> + depends on PCI
> help
> Say Y here to enable the power sequencing driver for PCIe M.2
> connectors. This driver handles the power sequencing for the M.2
> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index e01e19123415..4b85a40d7692 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -4,12 +4,16 @@
> * Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> */
>
> +#include <linux/err.h>
> #include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_graph.h>
> #include <linux/of_platform.h>
> +#include <linux/pci.h>
> #include <linux/platform_device.h>
> #include <linux/pwrseq/provider.h>
> #include <linux/regulator/consumer.h>
> @@ -25,17 +29,19 @@ struct pwrseq_pcie_m2_ctx {
> const struct pwrseq_pcie_m2_pdata *pdata;
> struct regulator_bulk_data *regs;
> size_t num_vregs;
> - struct notifier_block nb;
> + struct gpio_desc *w_disable1_gpio;
> + struct gpio_desc *w_disable2_gpio;
> + struct device *dev;
> };
>
> -static int pwrseq_pcie_m2_m_vregs_enable(struct pwrseq_device *pwrseq)
> +static int pwrseq_pcie_m2_vregs_enable(struct pwrseq_device *pwrseq)
> {
> struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
>
> return regulator_bulk_enable(ctx->num_vregs, ctx->regs);
> }
>
> -static int pwrseq_pcie_m2_m_vregs_disable(struct pwrseq_device *pwrseq)
> +static int pwrseq_pcie_m2_vregs_disable(struct pwrseq_device *pwrseq)
> {
> struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
>
> @@ -44,18 +50,84 @@ static int pwrseq_pcie_m2_m_vregs_disable(struct pwrseq_device *pwrseq)
>
> static const struct pwrseq_unit_data pwrseq_pcie_m2_vregs_unit_data = {
> .name = "regulators-enable",
> - .enable = pwrseq_pcie_m2_m_vregs_enable,
> - .disable = pwrseq_pcie_m2_m_vregs_disable,
> + .enable = pwrseq_pcie_m2_vregs_enable,
> + .disable = pwrseq_pcie_m2_vregs_disable,
> };
>
> -static const struct pwrseq_unit_data *pwrseq_pcie_m2_m_unit_deps[] = {
> +static const struct pwrseq_unit_data *pwrseq_pcie_m2_unit_deps[] = {
> &pwrseq_pcie_m2_vregs_unit_data,
> NULL
> };
>
> +static int pwrseq_pci_m2_e_uart_enable(struct pwrseq_device *pwrseq)
> +{
> + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> + return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 0);
> +}
> +
> +static int pwrseq_pci_m2_e_uart_disable(struct pwrseq_device *pwrseq)
> +{
> + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> + return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 1);
> +}
> +
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_uart_unit_data = {
> + .name = "uart-enable",
> + .deps = pwrseq_pcie_m2_unit_deps,
> + .enable = pwrseq_pci_m2_e_uart_enable,
> + .disable = pwrseq_pci_m2_e_uart_disable,
> +};
> +
> +static int pwrseq_pci_m2_e_pcie_enable(struct pwrseq_device *pwrseq)
> +{
> + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> + return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 0);
> +}
> +
> +static int pwrseq_pci_m2_e_pcie_disable(struct pwrseq_device *pwrseq)
> +{
> + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> +
> + return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 1);
> +}
> +
> +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_pcie_unit_data = {
> + .name = "pcie-enable",
> + .deps = pwrseq_pcie_m2_unit_deps,
> + .enable = pwrseq_pci_m2_e_pcie_enable,
> + .disable = pwrseq_pci_m2_e_pcie_disable,
> +};
> +
> static const struct pwrseq_unit_data pwrseq_pcie_m2_m_pcie_unit_data = {
> .name = "pcie-enable",
> - .deps = pwrseq_pcie_m2_m_unit_deps,
> + .deps = pwrseq_pcie_m2_unit_deps,
> +};
> +
> +static int pwrseq_pcie_m2_e_pwup_delay(struct pwrseq_device *pwrseq)
> +{
> + /*
> + * FIXME: This delay is only required for some Qcom WLAN/BT cards like
> + * WCN7850 and not for all devices. But currently, there is no way to
> + * identify the device model before enumeration.
> + */
> + msleep(50);
Section 3.1.4 of the M.2 spec says that "Power Valid to PERST# input
inactive" (T_PVPGL) is "Implementation specific recommended 50 ms." So I
think we should delay for at least 50 ms for all M.2 cards.
Additionally, the PCIe CEM specifies that "Power stable to PERST#
inactive" (T_PVPERL) must be at least 100 ms. So I think we should just
delay for 100 ms regardless of the slot, perhaps making this
configurable in the devicetree if e.g. the system integrator knows the
soldered-down M.2 requires less initialization time. This is exactly
what I proposed in [1].
--Sean
[1] https://lore.kernel.org/linux-pci/20251219172222.2808195-2-sean.anderson@linux.dev/
> + return 0;
> +}
> +
> +static const struct pwrseq_target_data pwrseq_pcie_m2_e_uart_target_data = {
> + .name = "uart",
> + .unit = &pwrseq_pcie_m2_e_uart_unit_data,
> + .post_enable = pwrseq_pcie_m2_e_pwup_delay,
> +};
> +
> +static const struct pwrseq_target_data pwrseq_pcie_m2_e_pcie_target_data = {
> + .name = "pcie",
> + .unit = &pwrseq_pcie_m2_e_pcie_unit_data,
> + .post_enable = pwrseq_pcie_m2_e_pwup_delay,
> };
>
> static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
> @@ -63,11 +135,21 @@ static const struct pwrseq_target_data pwrseq_pcie_m2_m_pcie_target_data = {
> .unit = &pwrseq_pcie_m2_m_pcie_unit_data,
> };
>
> +static const struct pwrseq_target_data *pwrseq_pcie_m2_e_targets[] = {
> + &pwrseq_pcie_m2_e_pcie_target_data,
> + &pwrseq_pcie_m2_e_uart_target_data,
> + NULL
> +};
> +
> static const struct pwrseq_target_data *pwrseq_pcie_m2_m_targets[] = {
> &pwrseq_pcie_m2_m_pcie_target_data,
> NULL
> };
>
> +static const struct pwrseq_pcie_m2_pdata pwrseq_pcie_m2_e_of_data = {
> + .targets = pwrseq_pcie_m2_e_targets,
> +};
> +
> static const struct pwrseq_pcie_m2_pdata pwrseq_pcie_m2_m_of_data = {
> .targets = pwrseq_pcie_m2_m_targets,
> };
> @@ -126,6 +208,16 @@ static int pwrseq_pcie_m2_probe(struct platform_device *pdev)
> return dev_err_probe(dev, ret,
> "Failed to get all regulators\n");
>
> + ctx->w_disable1_gpio = devm_gpiod_get_optional(dev, "w-disable1", GPIOD_OUT_HIGH);
> + if (IS_ERR(ctx->w_disable1_gpio))
> + return dev_err_probe(dev, PTR_ERR(ctx->w_disable1_gpio),
> + "Failed to get the W_DISABLE_1# GPIO\n");
> +
> + ctx->w_disable2_gpio = devm_gpiod_get_optional(dev, "w-disable2", GPIOD_OUT_HIGH);
> + if (IS_ERR(ctx->w_disable2_gpio))
> + return dev_err_probe(dev, PTR_ERR(ctx->w_disable2_gpio),
> + "Failed to get the W_DISABLE_2# GPIO\n");
> +
> ctx->num_vregs = ret;
>
> ret = devm_add_action_or_reset(dev, pwrseq_pcie_free_resources, ctx);
> @@ -151,6 +243,10 @@ static const struct of_device_id pwrseq_pcie_m2_of_match[] = {
> .compatible = "pcie-m2-m-connector",
> .data = &pwrseq_pcie_m2_m_of_data,
> },
> + {
> + .compatible = "pcie-m2-e-connector",
> + .data = &pwrseq_pcie_m2_e_of_data,
> + },
> { }
> };
> MODULE_DEVICE_TABLE(of, pwrseq_pcie_m2_of_match);
>
^ permalink raw reply
* Re: [PATCH RFC 2/4] rust: add basic serial device bus abstractions
From: Markus Probst @ 2026-01-13 16:15 UTC (permalink / raw)
To: Kari Argillander
Cc: Rob Herring, Greg Kroah-Hartman, Jiri Slaby, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
linux-serial, linux-kernel, rust-for-linux
In-Reply-To: <CAC=eVgSmD+bYh48gZteAaqwSHvcHes3CFmbUUBu=6UQ6fKCUJg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 43638 bytes --]
On Thu, 2025-12-25 at 17:13 +0200, Kari Argillander wrote:
> On 20.12.2025 klo 20.44 Markus Probst (markus.probst@posteo.de) kirjoitti:
> >
> > Implement the basic serial device bus abstractions required to write a
> > serial device bus device driver with or without the need for initial device
> > data. This includes the following data structures:
> >
> > The `serdev::Driver` trait represents the interface to the driver.
> >
> > The `serdev::Device` abstraction represents a `struct serdev_device`.
> >
> > In order to provide the Serdev specific parts to a generic
> > `driver::Registration` the `driver::RegistrationOps` trait is
> > implemented by `serdev::Adapter`.
> >
> > Co-developed-by: Kari Argillander <kari.argillander@gmail.com>
> > Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> > rust/bindings/bindings_helper.h | 1 +
> > rust/helpers/helpers.c | 1 +
> > rust/helpers/serdev.c | 22 ++
> > rust/kernel/lib.rs | 2 +
> > rust/kernel/serdev.rs | 815 ++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 841 insertions(+)
> >
> > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> > index a067038b4b42..bec6c6d0913a 100644
> > --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -79,6 +79,7 @@
> > #include <linux/regulator/consumer.h>
> > #include <linux/sched.h>
> > #include <linux/security.h>
> > +#include <linux/serdev.h>
> > #include <linux/slab.h>
> > #include <linux/task_work.h>
> > #include <linux/tracepoint.h>
> > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> > index 79c72762ad9c..834e9fbb897d 100644
> > --- a/rust/helpers/helpers.c
> > +++ b/rust/helpers/helpers.c
> > @@ -50,6 +50,7 @@
> > #include "regulator.c"
> > #include "scatterlist.c"
> > #include "security.c"
> > +#include "serdev.c"
> > #include "signal.c"
> > #include "slab.c"
> > #include "spinlock.c"
> > diff --git a/rust/helpers/serdev.c b/rust/helpers/serdev.c
> > new file mode 100644
> > index 000000000000..c52b78ca3fc7
> > --- /dev/null
> > +++ b/rust/helpers/serdev.c
> > @@ -0,0 +1,22 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/serdev.h>
> > +
> > +__rust_helper
> > +void rust_helper_serdev_device_driver_unregister(struct serdev_device_driver *sdrv)
> > +{
> > + serdev_device_driver_unregister(sdrv);
> > +}
> > +
> > +__rust_helper
> > +void rust_helper_serdev_device_put(struct serdev_device *serdev)
> > +{
> > + serdev_device_put(serdev);
> > +}
> > +
> > +__rust_helper
> > +void rust_helper_serdev_device_set_client_ops(struct serdev_device *serdev,
> > + const struct serdev_device_ops *ops)
> > +{
> > + serdev_device_set_client_ops(serdev, ops);
> > +}
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index f812cf120042..cc71195466b6 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -136,6 +136,8 @@
> > pub mod scatterlist;
> > pub mod security;
> > pub mod seq_file;
> > +#[cfg(CONFIG_SERIAL_DEV_BUS)]
> > +pub mod serdev;
> > pub mod sizes;
> > pub mod slice;
> > mod static_assert;
> > diff --git a/rust/kernel/serdev.rs b/rust/kernel/serdev.rs
> > new file mode 100644
> > index 000000000000..0f5ef325a054
> > --- /dev/null
> > +++ b/rust/kernel/serdev.rs
> > @@ -0,0 +1,815 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Abstractions for the serial device bus.
> > +//!
> > +//! C header: [`include/linux/serdev.h`](srctree/include/linux/serdev.h)
> > +
> > +use crate::{
> > + acpi,
> > + container_of,
> > + device,
> > + driver,
> > + error::{
> > + from_result,
> > + to_result,
> > + VTABLE_DEFAULT_ERROR, //
> > + },
> > + of,
> > + prelude::*,
> > + time::Jiffies,
> > + types::{AlwaysRefCounted, Opaque}, //
> > +};
> > +
> > +use core::{
> > + cell::UnsafeCell,
> > + marker::PhantomData,
> > + mem::offset_of,
> > + num::NonZero,
> > + ptr::{
> > + addr_of_mut, //
> > + NonNull,
> > + }, //
> > +};
> > +
> > +/// Parity bit to use with a serial device.
> > +#[repr(u32)]
> > +pub enum Parity {
> > + /// No parity bit.
> > + None = bindings::serdev_parity_SERDEV_PARITY_NONE,
> > + /// Even partiy.
> > + Even = bindings::serdev_parity_SERDEV_PARITY_EVEN,
> > + /// Odd parity.
> > + Odd = bindings::serdev_parity_SERDEV_PARITY_ODD,
> > +}
> > +
> > +/// Timeout in Jiffies.
> > +pub enum Timeout {
> > + /// Wait for a specific amount of [`Jiffies`].
> > + Value(NonZero<Jiffies>),
> > + /// Wait as long as possible.
> > + ///
> > + /// This is equivalent to [`kernel::task::MAX_SCHEDULE_TIMEOUT`].
> > + MaxScheduleTimeout,
> > +}
> > +
> > +impl Timeout {
> > + fn into_jiffies(self) -> isize {
> > + match self {
> > + Self::Value(value) => value.get().try_into().unwrap_or_default(),
> > + Self::MaxScheduleTimeout => 0,
> > + }
> > + }
> > +}
> > +
> > +/// An adapter for the registration of serial device bus device drivers.
> > +pub struct Adapter<T: Driver>(T);
> > +
> > +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
> > +// a preceding call to `register` has been successful.
> > +unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
> > + type RegType = bindings::serdev_device_driver;
> > +
> > + unsafe fn register(
> > + sdrv: &Opaque<Self::RegType>,
> > + name: &'static CStr,
> > + module: &'static ThisModule,
> > + ) -> Result {
> > + let of_table = match T::OF_ID_TABLE {
> > + Some(table) => table.as_ptr(),
> > + None => core::ptr::null(),
> > + };
> > +
> > + let acpi_table = match T::ACPI_ID_TABLE {
> > + Some(table) => table.as_ptr(),
> > + None => core::ptr::null(),
> > + };
> > +
> > + // SAFETY: It's safe to set the fields of `struct serdev_device_driver` on initialization.
> > + unsafe {
> > + (*sdrv.get()).driver.name = name.as_char_ptr();
> > + (*sdrv.get()).probe = Some(Self::probe_callback);
> > + (*sdrv.get()).remove = Some(Self::remove_callback);
> > + (*sdrv.get()).driver.of_match_table = of_table;
> > + (*sdrv.get()).driver.acpi_match_table = acpi_table;
> > + }
> > +
> > + // SAFETY: `sdrv` is guaranteed to be a valid `RegType`.
> > + to_result(unsafe { bindings::__serdev_device_driver_register(sdrv.get(), module.0) })
> > + }
> > +
> > + unsafe fn unregister(sdrv: &Opaque<Self::RegType>) {
> > + // SAFETY: `sdrv` is guaranteed to be a valid `RegType`.
> > + unsafe { bindings::serdev_device_driver_unregister(sdrv.get()) };
> > + }
> > +}
> > +
> > +#[pin_data]
> > +struct Drvdata<T: Driver + 'static> {
> > + #[pin]
> > + driver: T,
> > + initial_data: UnsafeCell<Option<T::InitialData>>,
> > + late_probe_data: UnsafeCell<Option<Pin<KBox<T::LateProbeData>>>>,
> > +}
> > +
> > +impl<T: Driver + 'static> Adapter<T> {
> > + const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > + receive_buf: if T::HAS_RECEIVE {
> > + Some(Self::receive_buf_callback)
> > + } else {
> > + None
> > + },
> > + write_wakeup: if T::HAS_WRITE_WAKEUP {
> > + Some(Self::write_wakeup_callback)
> > + } else {
> > + Some(bindings::serdev_device_write_wakeup)
> > + },
> > + };
> > + const INITIAL_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > + receive_buf: Some(Self::initial_receive_buf_callback),
> > + write_wakeup: if T::HAS_WRITE_WAKEUP_INITIAL {
> > + Some(Self::initial_write_wakeup_callback)
> > + } else {
> > + Some(bindings::serdev_device_write_wakeup)
> > + },
> > + };
> > + const NO_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > + receive_buf: None,
> > + write_wakeup: Some(bindings::serdev_device_write_wakeup),
> > + };
> > +
> > + extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi::c_int {
> > + // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer to
> > + // a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `probe_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > + let info = <Self as driver::Adapter>::id_info(sdev.as_ref());
> > +
> > + from_result(|| {
> > + let data = try_pin_init!(Drvdata {
> > + driver <- T::probe(sdev, info),
> > + initial_data: Some(Default::default()).into(),
> > + late_probe_data: None.into(),
> > + });
> > +
> > + sdev.as_ref().set_drvdata(data)?;
> > +
> > + // SAFETY: We just set drvdata to `Drvdata<T>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::INITIAL_OPS) };
> > +
> > + // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer
> > + // to a `struct serdev_device`.
> > + to_result(unsafe {
> > + bindings::devm_serdev_device_open(sdev.as_ref().as_raw(), sdev.as_raw())
> > + })?;
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + T::configure(sdev, unsafe { Pin::new_unchecked(&data.driver) }, info)?;
> > +
> > + if !T::HAS_RECEIVE_INITIAL {
> > + // SAFETY:
> > + // - It is guaranteed that we have exclusive access to `data.initial_data` and
> > + // `data.late_probe_data`.
> > + // - We just initialized `data.initial_data` with Some.
> > + unsafe { Self::do_late_probe(sdev, data)? };
> > + }
> > +
> > + Ok(0)
> > + })
> > + }
> > +
> > + /// # Safety
> > + ///
> > + /// The caller must guarantee, that we have exclusive access to `data.initial_data` and
> > + /// `data.late_probe_data`. `data.initial_data` must be Some.
> > + /// (i. e. `late_probe` has not been called yet).
> > + unsafe fn do_late_probe(sdev: &Device<device::CoreInternal>, data: Pin<&Drvdata<T>>) -> Result {
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY: The function contract guarantees that we have exclusive access to
> > + // `data.initial_data`.
> > + let initial_data = unsafe { &mut *data.initial_data.get() };
> > +
> > + // SAFETY: The function contract guarantees that we have exclusive access to
> > + // `data.late_probe_data`.
> > + let late_probe_data = unsafe { &mut *data.late_probe_data.get() };
> > +
> > + *late_probe_data = Some(KBox::pin_init(
> > + T::late_probe(
> > + sdev,
> > + data_driver,
> > + // SAFETY: The function contract guarantees that `data.initial_data` is Some.
> > + unsafe { initial_data.take().unwrap_unchecked() },
> > + ),
> > + GFP_KERNEL,
> > + )?);
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::OPS) };
> > + Ok(())
> > + }
> > +
> > + extern "C" fn remove_callback(sdev: *mut bindings::serdev_device) {
> > + // SAFETY: The serial device bus only ever calls the remove callback with a valid pointer
> > + // to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `remove_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `remove_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<T>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_obtain::<Drvdata<T>>() };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY:
> > + // - `data.late_probe_data` is guaranteed to be pinned.
> > + // - It is guaranteed that we have exclusive access to `data.late_probe_data`.
> > + let late_probe_data = unsafe {
> > + (*data.late_probe_data.get())
> > + .as_deref()
> > + .map(|data| Pin::new_unchecked(data))
> > + };
> > +
> > + T::unbind(sdev, data_driver, late_probe_data);
> > + }
> > +
> > + extern "C" fn receive_buf_callback(
> > + sdev: *mut bindings::serdev_device,
> > + buf: *const u8,
> > + length: usize,
> > + ) -> usize {
> > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `receive_buf_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<Drvdata<T>>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY:
> > + // - `data.late_probe_data` is guaranteed to be Some.
> > + // - `data.late_probe_data` is guaranteed to be pinned.
> > + let late_probe_data = unsafe {
> > + Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
> > + };
> > +
> > + T::receive(sdev, data_driver, late_probe_data, buf)
> > + }
> > +
> > + extern "C" fn write_wakeup_callback(sdev: *mut bindings::serdev_device) {
> > + // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `write_wakeup_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<Drvdata<T>>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY:
> > + // - `data.late_probe_data` is guaranteed to be Some.
> > + // - `data.late_probe_data` is guaranteed to be pinned.
> > + let late_probe_data = unsafe {
> > + Pin::new_unchecked((*data.late_probe_data.get()).as_deref().unwrap_unchecked())
> > + };
> > +
> > + // SAFETY: As long as the driver implementation meets the safety requirements, this call
> > + // is safe.
> > + unsafe { T::write_wakeup(sdev, data_driver, late_probe_data) };
> > + }
> > +
> > + extern "C" fn initial_receive_buf_callback(
> > + sdev: *mut bindings::serdev_device,
> > + buf: *const u8,
> > + length: usize,
> > + ) -> usize {
> > + if !T::HAS_RECEIVE_INITIAL {
> > + return 0;
> > + }
> > +
> > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > +
> > + // SAFETY: `initial_receive_buf_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<Drvdata<T>>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let driver_data = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY:
> > + // - `data.initial_data` is always Some until `InitialReceiveResult::Ready` is
> > + // returned below.
> > + // - It is guaranteed that we have exclusive access to `data.initial_data`.
> > + let initial_data = unsafe { (*data.initial_data.get()).as_mut().unwrap_unchecked() };
> > +
> > + match T::receive_initial(sdev, driver_data, initial_data, buf) {
> > + Ok(InitialReceiveResult::Pending(size)) => size,
> > + Ok(InitialReceiveResult::Ready(size)) => {
> > + // SAFETY:
> > + // - It is guaranteed that we have exclusive access to `data.initial_data` and
> > + // `data.late_probe_data`.
> > + // - We just initialized `data.initial_data` with Some.
> > + if let Err(err) = unsafe { Self::do_late_probe(sdev, data) } {
> > + dev_err!(sdev.as_ref(), "Unable to late probe device: {err:?}\n");
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to
> > + // `serdev_device`.
> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::NO_OPS) };
> > + return length;
> > + }
> > + size
> > + }
> > + Err(err) => {
> > + dev_err!(
> > + sdev.as_ref(),
> > + "Unable to receive initial data for probe: {err:?}.\n"
> > + );
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::NO_OPS) };
> > + length
> > + }
> > + }
> > + }
> > +
> > + extern "C" fn initial_write_wakeup_callback(sdev: *mut bindings::serdev_device) {
> > + // SAFETY: The serial device bus only ever calls the write wakeup callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `write_wakeup_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > +
> > + // SAFETY: `initial_write_wakeup_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<Drvdata<T>>>`.
> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > +
> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_write_wakeup(sdev.as_raw()) };
> > +
> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > +
> > + // SAFETY: As long as the driver implementation meets the safety requirements, this call
> > + // is safe.
> > + unsafe { T::write_wakeup_initial(sdev, data_driver) };
> > + }
> > +}
> > +
> > +impl<T: Driver + 'static> driver::Adapter for Adapter<T> {
> > + type IdInfo = T::IdInfo;
> > +
> > + fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
> > + T::OF_ID_TABLE
> > + }
> > +
> > + fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
> > + T::ACPI_ID_TABLE
> > + }
> > +}
> > +
> > +/// Declares a kernel module that exposes a single serial device bus device driver.
> > +///
> > +/// # Examples
> > +///
> > +/// ```ignore
> > +/// kernel::module_serdev_device_driver! {
> > +/// type: MyDriver,
> > +/// name: "Module name",
> > +/// authors: ["Author name"],
> > +/// description: "Description",
> > +/// license: "GPL v2",
> > +/// }
> > +/// ```
> > +#[macro_export]
> > +macro_rules! module_serdev_device_driver {
> > + ($($f:tt)*) => {
> > + $crate::module_driver!(<T>, $crate::serdev::Adapter<T>, { $($f)* });
> > + };
> > +}
> > +
> > +/// Result for `receive_initial` in [`Driver`].
> > +pub enum InitialReceiveResult {
> > + /// More data is required.
> > + ///
> > + /// The inner data is the number of bytes accepted.
> > + Pending(usize),
> > + /// Ready for late probe.
> > + ///
> > + /// The inner data is the number of bytes accepted.
> > + Ready(usize),
> > +}
> > +
> > +/// The serial device bus device driver trait.
> > +///
> > +/// Drivers must implement this trait in order to get a serial device bus device driver registered.
> > +///
> > +/// # Examples
> > +///
> > +///```
> > +/// # use kernel::{
> > +/// acpi,
> > +/// bindings,
> > +/// device::{
> > +/// Bound,
> > +/// Core, //
> > +/// },
> > +/// of,
> > +/// serdev, //
> > +/// };
> > +///
> > +/// struct MyDriver;
> > +///
> > +/// kernel::of_device_table!(
> > +/// OF_TABLE,
> > +/// MODULE_OF_TABLE,
> > +/// <MyDriver as serdev::Driver>::IdInfo,
> > +/// [
> > +/// (of::DeviceId::new(c"test,device"), ())
> > +/// ]
> > +/// );
> > +///
> > +/// kernel::acpi_device_table!(
> > +/// ACPI_TABLE,
> > +/// MODULE_ACPI_TABLE,
> > +/// <MyDriver as serdev::Driver>::IdInfo,
> > +/// [
> > +/// (acpi::DeviceId::new(c"LNUXBEEF"), ())
> > +/// ]
> > +/// );
> > +///
> > +/// #[vtable]
> > +/// impl serdev::Driver for MyDriver {
> > +/// type IdInfo = ();
> > +/// type LateProbeData = ();
> > +/// type InitialData = ();
> > +/// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> > +/// const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
> > +///
> > +/// fn probe(
> > +/// _sdev: &serdev::Device,
> > +/// _id_info: Option<&Self::IdInfo>,
> > +/// ) -> impl PinInit<Self, Error> {
> > +/// Err(ENODEV)
> > +/// }
> > +///
> > +/// fn configure(
> > +/// dev: &serdev::Device<Core>,
> > +/// _this: Pin<&Self>,
> > +/// _id_info: Option<&Self::IdInfo>,
> > +/// ) -> Result {
> > +/// dev.set_baudrate(115200);
> > +/// Ok(())
> > +/// }
> > +///
> > +/// fn late_probe(
> > +/// _dev: &serdev::Device<Bound>,
> > +/// _this: Pin<&Self>,
> > +/// _initial_data: Self::InitialData,
> > +/// ) -> impl PinInit<Self::LateProbeData, Error> {
> > +/// Ok(())
> > +/// }
> > +/// }
> > +///```
> > +#[vtable]
> > +pub trait Driver: Send {
> > + /// The type holding driver private data about each device id supported by the driver.
> > + // TODO: Use associated_type_defaults once stabilized:
> > + //
> > + // ```
> > + // type IdInfo: 'static = ();
> > + // type LateProbeData: Send + 'static = ();
> > + // type InitialData: Default + Send + 'static = ();
> > + // ```
> > + type IdInfo: 'static;
> > +
> > + /// Data returned in `late_probe` function.
> > + type LateProbeData: Send + 'static;
> > + /// Data used for initial data.
> > + type InitialData: Default + Send + 'static;
> > +
> > + /// The table of OF device ids supported by the driver.
> > + const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
> > +
> > + /// The table of ACPI device ids supported by the driver.
> > + const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None;
> > +
> > + /// Serial device bus device driver probe.
> > + ///
> > + /// Called when a new serial device bus device is added or discovered.
> > + /// Implementers should attempt to initialize the device here.
> > + fn probe(sdev: &Device, id_info: Option<&Self::IdInfo>) -> impl PinInit<Self, Error>;
> > +
> > + /// Serial device bus device driver configure.
> > + ///
> > + /// Called directly after the serial device bus device has been opened.
> > + /// This should be used for setting up the communication (i. e. baudrate, flow control, parity)
> > + /// and sending an initial hello if required.
> > + fn configure(
> > + sdev: &Device<device::Core>,
> > + this: Pin<&Self>,
> > + id_info: Option<&Self::IdInfo>,
> > + ) -> Result;
> > +
> > + /// Serial device bus device data receive callback (initial).
> > + ///
> > + /// Called when data got received from device, before `late_probe` has been called.
> > + /// This should be used, to get information about the device for `late_probe`.
> > + ///
> > + /// Returns `InitialReceiveResult::Pending` if more data is still required for `late_probe`.
> > + /// Otherwise `InitialReceiveResult::Ready`. The inner data is the number of bytes accepted.
> > + fn receive_initial(
> > + sdev: &Device<device::Bound>,
> > + this: Pin<&Self>,
> > + initial_data: &mut Self::InitialData,
> > + data: &[u8],
> > + ) -> Result<InitialReceiveResult> {
> > + let _ = (sdev, this, initial_data, data);
> > + build_error!(VTABLE_DEFAULT_ERROR)
> > + }
> > +
> > + /// Serial device bus device write wakeup callback (initial).
> > + ///
> > + /// Called when ready to transmit more data, before `late_probe` has been called.
> > + ///
> > + /// # Safety
> > + ///
> > + /// This function must not sleep.
> > + unsafe fn write_wakeup_initial(sdev: &Device<device::Core>, this: Pin<&Self>) {
> > + let _ = (sdev, this);
> > + build_error!(VTABLE_DEFAULT_ERROR)
> > + }
> > +
> > + /// Serial device bus device late probe.
> > + ///
> > + /// Called after the initial data is available.
> > + /// If `receive_initial` isn't implemented, this will be called directly after configure.
> > + fn late_probe(
> > + sdev: &Device<device::Bound>,
> > + this: Pin<&Self>,
> > + initial_data: Self::InitialData,
> > + ) -> impl PinInit<Self::LateProbeData, Error>;
> > +
> > + /// Serial device bus device driver unbind.
> > + ///
> > + /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback
> > + /// is optional.
> > + ///
> > + /// This callback serves as a place for drivers to perform teardown operations that require a
> > + /// `&Device<Core>` or `&Device<Bound>` reference. For instance.
> > + ///
> > + /// Otherwise, release operations for driver resources should be performed in `Self::drop`.
> > + fn unbind(
> > + sdev: &Device<device::Core>,
> > + this: Pin<&Self>,
> > + late_probe_this: Option<Pin<&Self::LateProbeData>>,
> > + ) {
> > + let _ = (sdev, this, late_probe_this);
> > + }
> > +
> > + /// Serial device bus device data receive callback.
> > + ///
> > + /// Called when data got received from device.
> > + ///
> > + /// Returns the number of bytes accepted.
> > + fn receive(
> > + sdev: &Device<device::Bound>,
> > + this: Pin<&Self>,
> > + late_probe_this: Pin<&Self::LateProbeData>,
> > + data: &[u8],
> > + ) -> usize {
> > + let _ = (sdev, this, late_probe_this, data);
> > + build_error!(VTABLE_DEFAULT_ERROR)
> > + }
> > +
> > + /// Serial device bus device write wakeup callback.
> > + ///
> > + /// Called when ready to transmit more data.
> > + ///
> > + /// # Safety
> > + ///
> > + /// This function must not sleep.
> > + unsafe fn write_wakeup(
> > + sdev: &Device<device::Bound>,
> > + this: Pin<&Self>,
> > + late_probe_this: Pin<&Self::LateProbeData>,
> > + ) {
> > + let _ = (sdev, this, late_probe_this);
> > + build_error!(VTABLE_DEFAULT_ERROR)
> > + }
> > +}
> > +
> > +/// The serial device bus device representation.
> > +///
> > +/// This structure represents the Rust abstraction for a C `struct serdev_device`. The
> > +/// implementation abstracts the usage of an already existing C `struct serdev_device` within Rust
> > +/// code that we get passed from the C side.
> > +///
> > +/// # Invariants
> > +///
> > +/// A [`Device`] instance represents a valid `struct serdev_device` created by the C portion of
> > +/// the kernel.
> > +#[repr(transparent)]
> > +pub struct Device<Ctx: device::DeviceContext = device::Normal>(
> > + Opaque<bindings::serdev_device>,
> > + PhantomData<Ctx>,
> > +);
> > +
> > +impl<Ctx: device::DeviceContext> Device<Ctx> {
> > + fn as_raw(&self) -> *mut bindings::serdev_device {
> > + self.0.get()
> > + }
> > +}
> > +
> > +impl Device<device::Bound> {
> > + /// Set the baudrate in bits per second.
> > + ///
> > + /// Common baudrates are 115200, 9600, 19200, 57600, 4800.
> > + ///
> > + /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
> > + pub fn set_baudrate(&self, speed: u32) -> u32 {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_set_baudrate(self.as_raw(), speed) }
> > + }
> > +
> > + /// Set if flow control should be enabled.
> > + ///
> > + /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
> > + pub fn set_flow_control(&self, enable: bool) {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_set_flow_control(self.as_raw(), enable) };
> > + }
> > +
> > + /// Set parity to use.
> > + ///
> > + /// Use [`Device::write_flush`] before calling this if you have written data prior to this call.
> > + pub fn set_parity(&self, parity: Parity) -> Result {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + to_result(unsafe { bindings::serdev_device_set_parity(self.as_raw(), parity as u32) })
> > + }
> > +
> > + /// Write data to the serial device until the controller has accepted all the data or has
> > + /// been interrupted by a timeout or signal.
> > + ///
> > + /// Note that any accepted data has only been buffered by the controller. Use
> > + /// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
> > + /// emptied.
> > + ///
> > + /// Returns the number of bytes written (less than data if interrupted).
> > + /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
> > + /// before any bytes were written.
> > + pub fn write_all(&self, data: &[u8], timeout: Timeout) -> Result<usize> {
> > + // SAFETY:
> > + // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
> > + // `data.len()`.
> > + let ret = unsafe {
> > + bindings::serdev_device_write(
> > + self.as_raw(),
> > + data.as_ptr(),
> > + data.len(),
> > + timeout.into_jiffies(),
> > + )
> > + };
> > + if ret < 0 {
> > + // CAST: negative return values are guaranteed to be between `-MAX_ERRNO` and `-1`,
> > + // which always fit into a `i32`.
> > + Err(Error::from_errno(ret as i32))
> > + } else {
> > + Ok(ret.unsigned_abs())
> > + }
> > + }
> > +
> > + /// Write data to the serial device.
> > + ///
> > + /// If you want to write until the controller has accepted all the data, use
> > + /// [`Device::write_all`].
> > + ///
> > + /// Note that any accepted data has only been buffered by the controller. Use
> > + /// [ Device::wait_until_sent`] to make sure the controller write buffer has actually been
> > + /// emptied.
> > + ///
> > + /// Returns the number of bytes written (less than data if interrupted).
> > + /// [`kernel::error::code::ETIMEDOUT`] or [`kernel::error::code::ERESTARTSYS`] if interrupted
> > + /// before any bytes were written.
> > + pub fn write(&self, data: &[u8]) -> Result<u32> {
> > + // SAFETY:
> > + // - `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + // - `data.as_ptr()` is guaranteed to be a valid array pointer with the size of
> > + // `data.len()`.
> > + let ret =
> > + unsafe { bindings::serdev_device_write_buf(self.as_raw(), data.as_ptr(), data.len()) };
> > + if ret < 0 {
> > + Err(Error::from_errno(ret))
> > + } else {
> > + Ok(ret.unsigned_abs())
> > + }
> > + }
> > +
> > + /// Send data to the serial device immediately.
> > + ///
> > + /// Note that this doesn't guarantee that the data has been transmitted.
> > + /// Use [`Device::wait_until_sent`] for this purpose.
> > + pub fn write_flush(&self) {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_write_flush(self.as_raw()) };
> > + }
> > +
> > + /// Wait for the data to be sent.
> > + ///
> > + /// After this function, the write buffer of the controller should be empty.
> > + pub fn wait_until_sent(&self, timeout: Timeout) {
> > + // SAFETY: `self.as_raw()` is guaranteed to be a pointer to a valid `serdev_device`.
> > + unsafe { bindings::serdev_device_wait_until_sent(self.as_raw(), timeout.into_jiffies()) };
> > + }
> > +}
> > +
> > +// SAFETY: `serdev::Device` is a transparent wrapper of `struct serdev_device`.
> > +// The offset is guaranteed to point to a valid device field inside `serdev::Device`.
> > +unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx> {
> > + const OFFSET: usize = offset_of!(bindings::serdev_device, dev);
> > +}
> > +
> > +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
> > +// argument.
> > +kernel::impl_device_context_deref!(unsafe { Device });
> > +kernel::impl_device_context_into_aref!(Device);
> > +
> > +// SAFETY: Instances of `Device` are always reference-counted.
> > +unsafe impl AlwaysRefCounted for Device {
> > + fn inc_ref(&self) {
> > + self.as_ref().inc_ref();
> > + }
> > +
> > + unsafe fn dec_ref(obj: NonNull<Self>) {
> > + // SAFETY: The safety requirements guarantee that the refcount is non-zero.
> > + unsafe { bindings::serdev_device_put(obj.cast().as_ptr()) }
> > + }
> > +}
> > +
> > +impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> > + fn as_ref(&self) -> &device::Device<Ctx> {
> > + // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
> > + // `struct serdev_device`.
> > + let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) };
> > +
> > + // SAFETY: `dev` points to a valid `struct device`.
> > + unsafe { device::Device::from_raw(dev) }
> > + }
> > +}
> > +
> > +impl<Ctx: device::DeviceContext> TryFrom<&device::Device<Ctx>> for &Device<Ctx> {
> > + type Error = kernel::error::Error;
> > +
> > + fn try_from(dev: &device::Device<Ctx>) -> Result<Self, Self::Error> {
> > + // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a
> > + // `struct device`.
> > + if !unsafe { bindings::is_serdev_device(dev.as_raw()) } {
> > + return Err(EINVAL);
> > + }
> > +
> > + // SAFETY: We've just verified that the device_type of `dev` is
> > + // `serdev_device_type`, hence `dev` must be embedded in a valid `struct
> > + // serdev_device_driver` as guaranteed by the corresponding C code.
> > + let sdev = unsafe { container_of!(dev.as_raw(), bindings::serdev_device, dev) };
> > +
> > + // SAFETY: `sdev` is a valid pointer to a `struct serdev_device_driver`.
> > + Ok(unsafe { &*sdev.cast() })
> > + }
> > +}
> > +
> > +// SAFETY: A `Device` is always reference-counted and can be released from any thread.
> > +unsafe impl Send for Device {}
> > +
> > +// SAFETY: `Device` can be shared among threads because all methods of `Device`
> > +// (i.e. `Device<Normal>) are thread safe.
> > +unsafe impl Sync for Device {}
> >
> > --
> > 2.51.2
> >
> >
>
> Currently I'm not a huge fan that we need to have configure(),
> receive_initial(), write_wakeup_initial() and late_probe(). Also I do not
> like that we hide hide open() completely from user. Maybe user want's to do
> something if there is error. This does not feel very ergonomic or standard
> kernel way. So after thinking a bit more about this idea I have come up with
> a different approach.
>
> We will have just probe() as usual. In probe() user will be forced to do open()
> and finnish() on serdev device. Between open() and finnish() user can
> configure the device and do initial reads / writes as needed. After probe is
> finished we will switch to runtime ops.
>
> This is not production ready code and I have just tested the idea locally
> without kernel. Here is example how probe() could look like:
>
> ```rust
> fn probe(sdev: &serdev::Device<Core>, _id_info:
> Option<&Self::IdInfo>)-> impl PinInit<(Self, ProbeEndToken), Error> {
> let sdev= sdev.open()?;
> // After open we can configure and do initial reads / writes.
> sdev.set_baudrate(9600);
>
> // Here we can also write if needed.
>
> for _ in 0..8 {
> // For initial reads we will have iterator. This cannot be
> done after finish().
> let _ = sdev.next_byte(Duration::from_millis(300));
> }
>
> // We do finnish so user cannot store "initial sdev" anywhere. Also get
> // end_token so probe cannot return unless open() and finnish() are called.
> let (sdev, end_token) = sdev.finish();
> Ok(try_pin_init!((Self {sdev}, end_token)))
> }
> ```
>
> then our receive_initial will look something like this. (Not runnable
> on kernel).
> Idea is that we have event for next_byte(). So initial read is little bit slower
> but that should not be used so much that it matters in practice.
>
> ```rust
> extern "C" fn initial_receive_buf(dev: *mut serdev_device, buf: *const
> u8, count: usize) -> usize {
> let abs = unsafe { &mut *((*dev).client_data as *mut Abstraction) };
>
> let mut consumed = 0usize;
>
> loop {
> // Wait until we can either switch or deliver one byte (slot
> must be empty).
> let mut st = abs.st.lock().unwrap();
> while !st.probe_finished && !(st.want_byte && st.slot.is_none()) {
> st = abs.ev.wait(st).unwrap();
> }
>
> if st.probe_finished {
> // TODO: Do we lock write lock so it is really safe to
> change OPS here?
> unsafe { bindings::serdev_device_set_client_ops(dev,
> &RUNTIME_OPS) };
>
> // Forward the *remaining* bytes that we have NOT taken
> out of this buf.
> let rem_ptr = unsafe { buf.add(consumed) };
> let rem = count - consumed;
> return consumed + Self::runtime_receive_buf(dev, rem_ptr, rem);
> }
>
> // Deliver exactly one byte to iterator (slot is empty here).
> let b = unsafe { *buf.add(consumed) };
> st.slot = Some(b);
> // wake next_byte()
> abs.ev.notify_all();
>
> // Now WAIT until next_byte really consumes it (slot becomes None),
> // or probe finishes (in which case we’ll flush it to runtime).
> while !st.probe_finished && st.slot.is_some() {
> st = abs.ev.wait(st).unwrap();
> }
> if st.probe_finished {
> continue;
> }
>
> // Now it is truly consumed by next_byte().
> consumed += 1;
> if consumed == count {
> return count;
> }
> }
> }
> ```
>
> Does anyone have opinions which is better or is there some other way
> for this which
> we have not yet discovered?
I am not sure if using this approach is intended by the serdev
subsystem. Otherwise I assume there already would be a C function in
the subsystem for reading and waiting for data on probe.
There currently is only one serial device bus driver in the kernel,
which needs initial data:
drivers/bluetooth/hci_uart.h
drivers/bluetooth/hci_ldisc.c
drivers/bluetooth/hci_serdev.c
This driver retrieves this initial data after probe (not in the probe)
and then initializes it with a workqueue. Given it is part of the
kernel, I assume this is the intended behaviour.
@Danilo: Can you share your opinion on this?
Thanks
- Markus Probst
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply
* Re: [PATCH 01/19] printk/nbcon: Use an enum to specify the required callback in console_is_usable()
From: Petr Mladek @ 2026-01-13 16:25 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Richard Weinberger, Anton Ivanov, Johannes Berg,
Greg Kroah-Hartman, Jason Wessel, Daniel Thompson,
Douglas Anderson, Steven Rostedt, John Ogness, Sergey Senozhatsky,
Jiri Slaby, Breno Leitao, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Geert Uytterhoeven,
Kees Cook, Tony Luck, Guilherme G. Piccoli, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Andreas Larsson, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Jacky Huang, Shan-Chun Hung, Laurentiu Tudor,
linux-um, linux-kernel, kgdb-bugreport, linux-serial, netdev,
linux-m68k, linux-hardening, linuxppc-dev, sparclinux,
linux-stm32, linux-arm-kernel, linux-fsdevel
In-Reply-To: <20251227-printk-cleanup-part3-v1-1-21a291bcf197@suse.com>
On Sat 2025-12-27 09:16:08, Marcos Paulo de Souza wrote:
> The current usage of console_is_usable() is clumsy. The parameter
> @use_atomic is boolean and thus not self-explanatory. The function is
> called twice in situations when there are no-strict requirements.
>
> Replace it with enum nbcon_write_cb which provides a more descriptive
> values for all 3 situations: atomic, thread or any.
>
> Note that console_is_usable() checks only NBCON_USE_ATOMIC because
> .write_thread() callback is mandatory. But the other two values still
> make sense because they describe the intention of the caller.
>
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -202,6 +202,19 @@ enum cons_flags {
> CON_NBCON_ATOMIC_UNSAFE = BIT(9),
> };
>
> +/**
> + * enum nbcon_write_cb - Defines which nbcon write() callback must be used based
> + * on the caller context.
> + * @NBCON_USE_ATOMIC: Use con->write_atomic().
> + * @NBCON_USE_THREAD: Use con->write_thread().
> + * @NBCON_USE_ANY: The caller does not have any strict requirements.
> + */
> +enum nbcon_write_cb {
> + NBCON_USE_ATOMIC,
> + NBCON_USE_THREAD,
> + NBCON_USE_ANY,
AFAIK, this would define NBCON_USE_ATOMIC as zero. See below.
> +};
> +
> /**
> * struct nbcon_state - console state for nbcon consoles
> * @atom: Compound of the state fields for atomic operations
> @@ -622,7 +635,8 @@ extern void nbcon_kdb_release(struct nbcon_write_context *wctxt);
> * which can also play a role in deciding if @con can be used to print
> * records.
> */
> -static inline bool console_is_usable(struct console *con, short flags, bool use_atomic)
> +static inline bool console_is_usable(struct console *con, short flags,
> + enum nbcon_write_cb nwc)
> {
> if (!(flags & CON_ENABLED))
> return false;
> @@ -631,7 +645,7 @@ static inline bool console_is_usable(struct console *con, short flags, bool use_
> return false;
>
> if (flags & CON_NBCON) {
> - if (use_atomic) {
> + if (nwc & NBCON_USE_ATOMIC) {
This will always be false because NBCON_USE_ATOMIC is zero.
I think that it was defined as "0x1" in the original proposal.
Let's keep it defined by as zero and use here:
if (nwc == NBCON_USE_ATOMIC) {
Note that we do _not_ want to return "false" for "NBCON_USE_ANY"
when con->write_atomic does not exist.
> /* The write_atomic() callback is optional. */
> if (!con->write_atomic)
> return false;
Otherwise, it looks good to me.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v4 8/9] power: sequencing: pcie-m2: Add support for PCIe M.2 Key E connectors
From: Manivannan Sadhasivam @ 2026-01-13 16:34 UTC (permalink / raw)
To: Sean Anderson
Cc: manivannan.sadhasivam, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Nathan Chancellor, Nicolas Schier, Hans de Goede,
Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Krzysztof Kozlowski, Conor Dooley, Marcel Holtmann,
Luiz Augusto von Dentz, Bartosz Golaszewski, Andy Shevchenko,
Bartosz Golaszewski, linux-serial, linux-kernel, linux-kbuild,
platform-driver-x86, linux-pci, devicetree, linux-arm-msm,
linux-bluetooth, linux-pm, Stephan Gerhold, Dmitry Baryshkov,
linux-acpi
In-Reply-To: <2432dafc-4101-4b23-90b2-85ea5459435c@linux.dev>
On Tue, Jan 13, 2026 at 10:26:04AM -0500, Sean Anderson wrote:
> On 1/12/26 11:26, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> >
> > Add support for handling the power sequence of the PCIe M.2 Key E
> > connectors. These connectors are used to attach the Wireless Connectivity
> > devices to the host machine including combinations of WiFi, BT, NFC using
> > interfaces such as PCIe/SDIO for WiFi, USB/UART for BT and I2C for NFC.
> >
> > Currently, this driver supports only the PCIe interface for WiFi and UART
> > interface for BT. The driver also only supports driving the 3.3v/1.8v power
> > supplies and W_DISABLE{1/2}# GPIOs. The optional signals of the Key E
> > connectors are not currently supported.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> > drivers/power/sequencing/Kconfig | 1 +
> > drivers/power/sequencing/pwrseq-pcie-m2.c | 110 ++++++++++++++++++++++++++++--
> > 2 files changed, 104 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/power/sequencing/Kconfig b/drivers/power/sequencing/Kconfig
> > index f5fff84566ba..29bd204319cc 100644
> > --- a/drivers/power/sequencing/Kconfig
> > +++ b/drivers/power/sequencing/Kconfig
> > @@ -38,6 +38,7 @@ config POWER_SEQUENCING_TH1520_GPU
> > config POWER_SEQUENCING_PCIE_M2
> > tristate "PCIe M.2 connector power sequencing driver"
> > depends on OF || COMPILE_TEST
> > + depends on PCI
> > help
> > Say Y here to enable the power sequencing driver for PCIe M.2
> > connectors. This driver handles the power sequencing for the M.2
> > diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> > index e01e19123415..4b85a40d7692 100644
> > --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> > +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> > @@ -4,12 +4,16 @@
> > * Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > */
> >
> > +#include <linux/err.h>
> > #include <linux/device.h>
> > +#include <linux/delay.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/mod_devicetable.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > #include <linux/of_graph.h>
> > #include <linux/of_platform.h>
> > +#include <linux/pci.h>
> > #include <linux/platform_device.h>
> > #include <linux/pwrseq/provider.h>
> > #include <linux/regulator/consumer.h>
> > @@ -25,17 +29,19 @@ struct pwrseq_pcie_m2_ctx {
> > const struct pwrseq_pcie_m2_pdata *pdata;
> > struct regulator_bulk_data *regs;
> > size_t num_vregs;
> > - struct notifier_block nb;
> > + struct gpio_desc *w_disable1_gpio;
> > + struct gpio_desc *w_disable2_gpio;
> > + struct device *dev;
> > };
> >
> > -static int pwrseq_pcie_m2_m_vregs_enable(struct pwrseq_device *pwrseq)
> > +static int pwrseq_pcie_m2_vregs_enable(struct pwrseq_device *pwrseq)
> > {
> > struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> >
> > return regulator_bulk_enable(ctx->num_vregs, ctx->regs);
> > }
> >
> > -static int pwrseq_pcie_m2_m_vregs_disable(struct pwrseq_device *pwrseq)
> > +static int pwrseq_pcie_m2_vregs_disable(struct pwrseq_device *pwrseq)
> > {
> > struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> >
> > @@ -44,18 +50,84 @@ static int pwrseq_pcie_m2_m_vregs_disable(struct pwrseq_device *pwrseq)
> >
> > static const struct pwrseq_unit_data pwrseq_pcie_m2_vregs_unit_data = {
> > .name = "regulators-enable",
> > - .enable = pwrseq_pcie_m2_m_vregs_enable,
> > - .disable = pwrseq_pcie_m2_m_vregs_disable,
> > + .enable = pwrseq_pcie_m2_vregs_enable,
> > + .disable = pwrseq_pcie_m2_vregs_disable,
> > };
> >
> > -static const struct pwrseq_unit_data *pwrseq_pcie_m2_m_unit_deps[] = {
> > +static const struct pwrseq_unit_data *pwrseq_pcie_m2_unit_deps[] = {
> > &pwrseq_pcie_m2_vregs_unit_data,
> > NULL
> > };
> >
> > +static int pwrseq_pci_m2_e_uart_enable(struct pwrseq_device *pwrseq)
> > +{
> > + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> > +
> > + return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 0);
> > +}
> > +
> > +static int pwrseq_pci_m2_e_uart_disable(struct pwrseq_device *pwrseq)
> > +{
> > + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> > +
> > + return gpiod_set_value_cansleep(ctx->w_disable2_gpio, 1);
> > +}
> > +
> > +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_uart_unit_data = {
> > + .name = "uart-enable",
> > + .deps = pwrseq_pcie_m2_unit_deps,
> > + .enable = pwrseq_pci_m2_e_uart_enable,
> > + .disable = pwrseq_pci_m2_e_uart_disable,
> > +};
> > +
> > +static int pwrseq_pci_m2_e_pcie_enable(struct pwrseq_device *pwrseq)
> > +{
> > + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> > +
> > + return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 0);
> > +}
> > +
> > +static int pwrseq_pci_m2_e_pcie_disable(struct pwrseq_device *pwrseq)
> > +{
> > + struct pwrseq_pcie_m2_ctx *ctx = pwrseq_device_get_drvdata(pwrseq);
> > +
> > + return gpiod_set_value_cansleep(ctx->w_disable1_gpio, 1);
> > +}
> > +
> > +static const struct pwrseq_unit_data pwrseq_pcie_m2_e_pcie_unit_data = {
> > + .name = "pcie-enable",
> > + .deps = pwrseq_pcie_m2_unit_deps,
> > + .enable = pwrseq_pci_m2_e_pcie_enable,
> > + .disable = pwrseq_pci_m2_e_pcie_disable,
> > +};
> > +
> > static const struct pwrseq_unit_data pwrseq_pcie_m2_m_pcie_unit_data = {
> > .name = "pcie-enable",
> > - .deps = pwrseq_pcie_m2_m_unit_deps,
> > + .deps = pwrseq_pcie_m2_unit_deps,
> > +};
> > +
> > +static int pwrseq_pcie_m2_e_pwup_delay(struct pwrseq_device *pwrseq)
> > +{
> > + /*
> > + * FIXME: This delay is only required for some Qcom WLAN/BT cards like
> > + * WCN7850 and not for all devices. But currently, there is no way to
> > + * identify the device model before enumeration.
> > + */
> > + msleep(50);
>
> Section 3.1.4 of the M.2 spec says that "Power Valid to PERST# input
> inactive" (T_PVPGL) is "Implementation specific recommended 50 ms." So I
> think we should delay for at least 50 ms for all M.2 cards.
Yes, this pretty much looks like T_PVPGL, but this delay is already accounted
for in pcie-qcom.c as a part of PERST# deassertion (I believe WCN7850 was tested
with Qcom host). I will check it and get back.
> Additionally, the PCIe CEM specifies that "Power stable to PERST#
> inactive" (T_PVPERL) must be at least 100 ms. So I think we should just
> delay for 100 ms regardless of the slot, perhaps making this
> configurable in the devicetree if e.g. the system integrator knows the
> soldered-down M.2 requires less initialization time. This is exactly
> what I proposed in [1].
>
I'd love to do it in the pwrctrl/pwrseq driver, but most of the controller
drivers are already handling this delay as a part of their PERST# deassertion.
This was the only reason I didn't add the T_PVPERL delay here. Also, those
controller drivers handle non-pwrctrl design as well (for backwards
compatibility), so they need the delay anyway and it will make them messy if the
delay is only handled in non-pwrctrl case.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v4 4/9] dt-bindings: serial: Document the graph port
From: Rob Herring (Arm) @ 2026-01-13 16:43 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Nathan Chancellor, devicetree, Mark Pearson, Nicolas Schier,
Bartosz Golaszewski, linux-pm, Manivannan Sadhasivam,
Dmitry Baryshkov, Hans de Goede, Marcel Holtmann, linux-kbuild,
platform-driver-x86, Jiri Slaby, linux-kernel,
Krzysztof Kozlowski, Luiz Augusto von Dentz, Stephan Gerhold,
Conor Dooley, Andy Shevchenko, Bartosz Golaszewski,
Greg Kroah-Hartman, Ilpo Järvinen, linux-bluetooth,
Derek J. Clark, linux-pci, linux-serial, linux-acpi,
linux-arm-msm
In-Reply-To: <20260112-pci-m2-e-v4-4-eff84d2c6d26@oss.qualcomm.com>
On Mon, 12 Jan 2026 21:56:03 +0530, Manivannan Sadhasivam wrote:
> A serial controller could be connected to an external connector like PCIe
> M.2 for controlling the serial interface of the card. Hence, document the
> OF graph port.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/serial/serial.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 5/9] dt-bindings: connector: Add PCIe M.2 Mechanical Key E connector
From: Rob Herring @ 2026-01-13 17:14 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor, Nicolas Schier,
Hans de Goede, Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Manivannan Sadhasivam, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Andy Shevchenko, Bartosz Golaszewski, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi
In-Reply-To: <20260112-pci-m2-e-v4-5-eff84d2c6d26@oss.qualcomm.com>
On Mon, Jan 12, 2026 at 09:56:04PM +0530, Manivannan Sadhasivam wrote:
> Add the devicetree binding for PCIe M.2 Mechanical Key E connector defined
> in the PCI Express M.2 Specification, r4.0, sec 5.1.2. This connector
> provides interfaces like PCIe or SDIO to attach the WiFi devices to the
> host machine, USB or UART+PCM interfaces to attach the Bluetooth (BT)
> devices. Spec also provides an optional interface to connect the UIM card,
> but that is not covered in this binding.
>
> The connector provides a primary power supply of 3.3v, along with an
> optional 1.8v VIO supply for the Adapter I/O buffer circuitry operating at
> 1.8v sideband signaling.
>
> The connector also supplies optional signals in the form of GPIOs for fine
> grained power management.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> .../bindings/connector/pcie-m2-e-connector.yaml | 154 +++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 155 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml b/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml
> new file mode 100644
> index 000000000000..b65b39ddfd19
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/connector/pcie-m2-e-connector.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: PCIe M.2 Mechanical Key E Connector
> +
> +maintainers:
> + - Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> +
> +description:
> + A PCIe M.2 E connector node represents a physical PCIe M.2 Mechanical Key E
> + connector. Mechanical Key E connectors are used to connect Wireless
> + Connectivity devices including combinations of Wi-Fi, BT, NFC to the host
> + machine over interfaces like PCIe/SDIO, USB/UART+PCM, and I2C.
> +
> +properties:
> + compatible:
> + const: pcie-m2-e-connector
> +
> + vpcie3v3-supply:
> + description: A phandle to the regulator for 3.3v supply.
> +
> + vpcie1v8-supply:
> + description: A phandle to the regulator for VIO 1.8v supply.
I don't see any 1.8V supply on the connector. There are 1.8V IOs and you
may need something in DT to ensure those are powered. However, there's
no guarantee that it's a single supply.
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> + description: OF graph bindings modeling the interfaces exposed on the
> + connector. Since a single connector can have multiple interfaces, every
> + interface has an assigned OF graph port number as described below.
> +
> + properties:
> + port@0:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: Connector interfaces for Wi-Fi
> +
> + properties:
> + endpoint@0:
> + $ref: /schemas/graph.yaml#/properties/endpoint
> + description: PCIe interface
> +
> + endpoint@1:
> + $ref: /schemas/graph.yaml#/properties/endpoint
> + description: SDIO interface
I think I already said this, but multiple endpoints are generally for
something that's muxed. Looking at the connector pinout, PCIe and SDIO
are not muxed. So these 2 should be 2 port nodes.
> +
> + anyOf:
> + - required:
> + - endpoint@0
> + - required:
> + - endpoint@1
> +
> + port@1:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: Connector interfaces for BT
> +
> + properties:
> + endpoint@0:
> + $ref: /schemas/graph.yaml#/properties/endpoint
> + description: USB 2.0 interface
> +
> + endpoint@1:
> + $ref: /schemas/graph.yaml#/properties/endpoint
> + description: UART interface
And UART and USB are not muxed either.
> +
> + anyOf:
> + - required:
> + - endpoint@0
> + - required:
> + - endpoint@1
> +
> + port@2:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: PCM/I2S interface
> +
> + i2c-parent:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description: I2C interface
Move out of 'ports'.
> +
> + oneOf:
> + - required:
> + - port@0
> +
> + clocks:
> + description: 32.768 KHz Suspend Clock (SUSCLK) input from the host system to
> + the M.2 card. Refer, PCI Express M.2 Specification r4.0, sec 3.1.12.1 for
> + more details.
> + maxItems: 1
> +
> + w-disable1-gpios:
> + description: GPIO input to W_DISABLE1# signal. This signal is used by the
> + system to disable WiFi radio in the M.2 card. Refer, PCI Express M.2
> + Specification r4.0, sec 3.1.12.3 for more details.
> + maxItems: 1
> +
> + w-disable2-gpios:
> + description: GPIO input to W_DISABLE2# signal. This signal is used by the
> + system to disable WiFi radio in the M.2 card. Refer, PCI Express M.2
> + Specification r4.0, sec 3.1.12.3 for more details.
> + maxItems: 1
> +
> + viocfg-gpios:
> + description: GPIO output to IO voltage configuration (VIO_CFG) signal. This
> + signal is used by the M.2 card to indicate to the host system that the
> + card supports an independent IO voltage domain for the sideband signals.
> + Refer, PCI Express M.2 Specification r4.0, sec 3.1.15.1 for more details.
> + maxItems: 1
What about SDIO and UART WAKE, SDIO RESET, and vendor defined signals?
> +
> +required:
> + - compatible
> + - vpcie3v3-supply
> +
> +additionalProperties: false
> +
> +examples:
> + # PCI M.2 Key E connector for Wi-Fi/BT with PCIe/UART interfaces
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + connector {
> + compatible = "pcie-m2-e-connector";
> + vpcie3v3-supply = <&vreg_wcn_3p3>;
> + vpcie1v8-supply = <&vreg_l15b_1p8>;
> + w-disable1-gpios = <&tlmm 117 GPIO_ACTIVE_LOW>;
> + w-disable2-gpios = <&tlmm 116 GPIO_ACTIVE_LOW>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&pcie4_port0_ep>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <&uart14_ep>;
> + };
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2eb7b6d26573..451c54675b24 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20795,6 +20795,7 @@ PCIE M.2 POWER SEQUENCING
> M: Manivannan Sadhasivam <mani@kernel.org>
> L: linux-pci@vger.kernel.org
> S: Maintained
> +F: Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml
> F: Documentation/devicetree/bindings/connector/pcie-m2-m-connector.yaml
> F: drivers/power/sequencing/pwrseq-pcie-m2.c
>
>
> --
> 2.48.1
>
^ permalink raw reply
* Re: [PATCH v4 5/9] dt-bindings: connector: Add PCIe M.2 Mechanical Key E connector
From: Rob Herring @ 2026-01-13 17:16 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor, Nicolas Schier,
Hans de Goede, Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Manivannan Sadhasivam, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Andy Shevchenko, Bartosz Golaszewski, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi
In-Reply-To: <20260112-pci-m2-e-v4-5-eff84d2c6d26@oss.qualcomm.com>
On Mon, Jan 12, 2026 at 09:56:04PM +0530, Manivannan Sadhasivam wrote:
> Add the devicetree binding for PCIe M.2 Mechanical Key E connector defined
> in the PCI Express M.2 Specification, r4.0, sec 5.1.2. This connector
> provides interfaces like PCIe or SDIO to attach the WiFi devices to the
> host machine, USB or UART+PCM interfaces to attach the Bluetooth (BT)
> devices. Spec also provides an optional interface to connect the UIM card,
> but that is not covered in this binding.
>
> The connector provides a primary power supply of 3.3v, along with an
> optional 1.8v VIO supply for the Adapter I/O buffer circuitry operating at
> 1.8v sideband signaling.
>
> The connector also supplies optional signals in the form of GPIOs for fine
> grained power management.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> .../bindings/connector/pcie-m2-e-connector.yaml | 154 +++++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 155 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml b/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml
> new file mode 100644
> index 000000000000..b65b39ddfd19
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/connector/pcie-m2-e-connector.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: PCIe M.2 Mechanical Key E Connector
> +
> +maintainers:
> + - Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> +
> +description:
> + A PCIe M.2 E connector node represents a physical PCIe M.2 Mechanical Key E
> + connector. Mechanical Key E connectors are used to connect Wireless
> + Connectivity devices including combinations of Wi-Fi, BT, NFC to the host
> + machine over interfaces like PCIe/SDIO, USB/UART+PCM, and I2C.
> +
> +properties:
> + compatible:
> + const: pcie-m2-e-connector
> +
> + vpcie3v3-supply:
> + description: A phandle to the regulator for 3.3v supply.
> +
> + vpcie1v8-supply:
> + description: A phandle to the regulator for VIO 1.8v supply.
> +
> + ports:
Also, nodes go after all properties.
> + $ref: /schemas/graph.yaml#/properties/ports
> + description: OF graph bindings modeling the interfaces exposed on the
> + connector. Since a single connector can have multiple interfaces, every
> + interface has an assigned OF graph port number as described below.
^ permalink raw reply
* Re: [PATCH v4 6/9] dt-bindings: connector: m2: Add M.2 1620 LGA soldered down connector
From: Rob Herring @ 2026-01-13 17:25 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor, Nicolas Schier,
Hans de Goede, Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Manivannan Sadhasivam, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Andy Shevchenko, Bartosz Golaszewski, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi
In-Reply-To: <20260112-pci-m2-e-v4-6-eff84d2c6d26@oss.qualcomm.com>
On Mon, Jan 12, 2026 at 09:56:05PM +0530, Manivannan Sadhasivam wrote:
> Lenovo Thinkpad T14s is found to have a soldered down version of M.2 1620
> LGA connector. Though, there is no 1620 LGA form factor defined in the M.2
> spec, it looks very similar to the M.2 Key M connector. So add the
> "pcie-m2-1620-lga-connector" compatible with "pcie-m2-e-connector" fallback
> to reuse the Key M binding.
Key M or Key E? I'm confused.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> .../devicetree/bindings/connector/pcie-m2-e-connector.yaml | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml b/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml
> index b65b39ddfd19..9757fe92907b 100644
> --- a/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml
> @@ -17,7 +17,14 @@ description:
>
> properties:
> compatible:
> - const: pcie-m2-e-connector
> + oneOf:
> + - items:
> + - enum:
> + - pcie-m2-1620-lga-connector
> + - const: pcie-m2-e-connector
> + - items:
> + - enum:
> + - pcie-m2-e-connector
>
> vpcie3v3-supply:
> description: A phandle to the regulator for 3.3v supply.
>
> --
> 2.48.1
>
^ permalink raw reply
* Re: [PATCH 02/19] printk: Introduce console_is_nbcon
From: Petr Mladek @ 2026-01-13 17:37 UTC (permalink / raw)
To: Marcos Paulo de Souza
Cc: Richard Weinberger, Anton Ivanov, Johannes Berg,
Greg Kroah-Hartman, Jason Wessel, Daniel Thompson,
Douglas Anderson, Steven Rostedt, John Ogness, Sergey Senozhatsky,
Jiri Slaby, Breno Leitao, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Geert Uytterhoeven,
Kees Cook, Tony Luck, Guilherme G. Piccoli, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy,
Andreas Larsson, Alexander Shishkin, Maxime Coquelin,
Alexandre Torgue, Jacky Huang, Shan-Chun Hung, Laurentiu Tudor,
linux-um, linux-kernel, kgdb-bugreport, linux-serial, netdev,
linux-m68k, linux-hardening, linuxppc-dev, sparclinux,
linux-stm32, linux-arm-kernel, linux-fsdevel
In-Reply-To: <20251227-printk-cleanup-part3-v1-2-21a291bcf197@suse.com>
On Sat 2025-12-27 09:16:09, Marcos Paulo de Souza wrote:
> Besides checking if the current console is NBCON or not, console->flags
> is also being read in order to serve as argument of the console_is_usable
> function.
>
> But CON_NBCON flag is unique: it's set just once in the console
> registration and never cleared. In this case it can be possible to read
> the flag when console_srcu_lock is held (which is the case when using
> for_each_console).
>
> This change makes possible to remove the flags argument from
> console_is_usable in the next patches.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> ---
> include/linux/console.h | 27 +++++++++++++++++++++++++++
> kernel/debug/kdb/kdb_io.c | 2 +-
> kernel/printk/nbcon.c | 2 +-
> kernel/printk/printk.c | 15 ++++++---------
> 4 files changed, 35 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 35c03fc4ed51..dd4ec7a5bff9 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -561,6 +561,33 @@ static inline void console_srcu_write_flags(struct console *con, short flags)
> WRITE_ONCE(con->flags, flags);
> }
>
> +/**
> + * console_srcu_is_nbcon - Locklessly check whether the console is nbcon
There is _srcu in the function name, see below.
> + * @con: struct console pointer of console to check
> + *
> + * Requires console_srcu_read_lock to be held, which implies that @con might
> + * be a registered console. The purpose of holding console_srcu_read_lock is
> + * to guarantee that no exit/cleanup routines will run if the console
> + * is currently undergoing unregistration.
> + *
> + * If the caller is holding the console_list_lock or it is _certain_ that
> + * @con is not and will not become registered, the caller may read
> + * @con->flags directly instead.
> + *
> + * Context: Any context.
> + * Return: True when CON_NBCON flag is set.
> + */
> +static inline bool console_is_nbcon(const struct console *con)
And here it is without _srcu.
I would prefer the variant with _srcu to make it clear that it
can be called only under _srcu. Similar to console_srcu_read_flags(con).
> +{
> + WARN_ON_ONCE(!console_srcu_read_lock_is_held());
> +
> + /*
> + * The CON_NBCON flag is statically initialized and is never
> + * set or cleared at runtime.
> + */
> + return data_race(con->flags & CON_NBCON);
> +}
> +
> /* Variant of console_is_registered() when the console_list_lock is held. */
> static inline bool console_is_registered_locked(const struct console *con)
> {
Otherwise, it looks good to me.
With a consistent name, feel free to use:
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH RFC 2/4] rust: add basic serial device bus abstractions
From: Danilo Krummrich @ 2026-01-13 17:37 UTC (permalink / raw)
To: Markus Probst
Cc: Kari Argillander, Rob Herring, Greg Kroah-Hartman, Jiri Slaby,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linux-serial, linux-kernel, rust-for-linux
In-Reply-To: <76491897ad6e0ff2749935c39702b93adc9951d6.camel@posteo.de>
On Tue Jan 13, 2026 at 5:15 PM CET, Markus Probst wrote:
>> > +impl<T: Driver + 'static> Adapter<T> {
>> > + const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
>> > + receive_buf: if T::HAS_RECEIVE {
>> > + Some(Self::receive_buf_callback)
>> > + } else {
>> > + None
>> > + },
>> > + write_wakeup: if T::HAS_WRITE_WAKEUP {
>> > + Some(Self::write_wakeup_callback)
>> > + } else {
>> > + Some(bindings::serdev_device_write_wakeup)
>> > + },
>> > + };
>> > + const INITIAL_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
>> > + receive_buf: Some(Self::initial_receive_buf_callback),
>> > + write_wakeup: if T::HAS_WRITE_WAKEUP_INITIAL {
>> > + Some(Self::initial_write_wakeup_callback)
>> > + } else {
>> > + Some(bindings::serdev_device_write_wakeup)
>> > + },
>> > + };
>> > + const NO_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
>> > + receive_buf: None,
>> > + write_wakeup: Some(bindings::serdev_device_write_wakeup),
>> > + };
>> > +
>> > + extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi::c_int {
>> > + // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer to
>> > + // a `struct serdev_device`.
>> > + //
>> > + // INVARIANT: `sdev` is valid for the duration of `probe_callback()`.
>> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
>> > + let info = <Self as driver::Adapter>::id_info(sdev.as_ref());
>> > +
>> > + from_result(|| {
>> > + let data = try_pin_init!(Drvdata {
>> > + driver <- T::probe(sdev, info),
>> > + initial_data: Some(Default::default()).into(),
>> > + late_probe_data: None.into(),
>> > + });
>> > +
>> > + sdev.as_ref().set_drvdata(data)?;
This does not work, a driver can obtain its device private data with
Device::<Bound>::drvdata() [1].
For this the driver must assert the correct type, but since you use a private
type instead of the type given by the driver, i.e. T, Device::<Bound>::drvdata()
will always fail for the driver.
[1] https://rust.docs.kernel.org/kernel/device/struct.Device.html#method.drvdata
>> > +
>> > + // SAFETY: We just set drvdata to `Drvdata<T>`.
>> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
>> > +
>> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
>> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::INITIAL_OPS) };
>> > +
>> > + // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer
>> > + // to a `struct serdev_device`.
>> > + to_result(unsafe {
>> > + bindings::devm_serdev_device_open(sdev.as_ref().as_raw(), sdev.as_raw())
>> > + })?;
I don't think it is a good idea to hardcode this into the probe() callback and
split it up in multiple stages, we can always solve things like ordering with
new types, type states and guards.
>> > +
>> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
>> > + T::configure(sdev, unsafe { Pin::new_unchecked(&data.driver) }, info)?;
>> > +
>> > + if !T::HAS_RECEIVE_INITIAL {
>> > + // SAFETY:
>> > + // - It is guaranteed that we have exclusive access to `data.initial_data` and
>> > + // `data.late_probe_data`.
How is it ensured that this does not run concurrently with
initial_receive_buf_callback(), etc.?
>> > + // - We just initialized `data.initial_data` with Some.
>> > + unsafe { Self::do_late_probe(sdev, data)? };
>> > + }
It is a bit unclear to me what you try to achieve here.
Do you want to synchronize an initial data transfer? Then something along the
lines of what Kari proposes seems reasonable.
Or is the intention that this will run entirely async? But then distinct
initialization stages as they appear above won't work.
>> > +
>> > + Ok(0)
>> > + })
>> > + }
>> > +
>> > + /// # Safety
>> > + ///
>> > + /// The caller must guarantee, that we have exclusive access to `data.initial_data` and
>> > + /// `data.late_probe_data`. `data.initial_data` must be Some.
>> > + /// (i. e. `late_probe` has not been called yet).
>> > + unsafe fn do_late_probe(sdev: &Device<device::CoreInternal>, data: Pin<&Drvdata<T>>) -> Result {
>> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
>> > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
>> > +
>> > + // SAFETY: The function contract guarantees that we have exclusive access to
>> > + // `data.initial_data`.
>> > + let initial_data = unsafe { &mut *data.initial_data.get() };
>> > +
>> > + // SAFETY: The function contract guarantees that we have exclusive access to
>> > + // `data.late_probe_data`.
>> > + let late_probe_data = unsafe { &mut *data.late_probe_data.get() };
>> > +
>> > + *late_probe_data = Some(KBox::pin_init(
>> > + T::late_probe(
>> > + sdev,
>> > + data_driver,
>> > + // SAFETY: The function contract guarantees that `data.initial_data` is Some.
>> > + unsafe { initial_data.take().unwrap_unchecked() },
>> > + ),
>> > + GFP_KERNEL,
>> > + )?);
>> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
>> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::OPS) };
>> > + Ok(())
>> > + }
<snip>
>> > + extern "C" fn initial_receive_buf_callback(
>> > + sdev: *mut bindings::serdev_device,
>> > + buf: *const u8,
>> > + length: usize,
>> > + ) -> usize {
>> > + if !T::HAS_RECEIVE_INITIAL {
>> > + return 0;
>> > + }
>> > +
>> > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
>> > + // pointer to a `struct serdev_device`.
>> > + //
>> > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
>> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
>> > +
>> > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
>> > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
>> > +
>> > + // SAFETY: `initial_receive_buf_callback` is only ever called after a successful call to
>> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
>> > + // and stored a `Pin<KBox<Drvdata<T>>>`.
>> > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
>> > +
>> > + // SAFETY: `&data.driver` is guaranteed to be pinned.
>> > + let driver_data = unsafe { Pin::new_unchecked(&data.driver) };
>> > +
>> > + // SAFETY:
>> > + // - `data.initial_data` is always Some until `InitialReceiveResult::Ready` is
>> > + // returned below.
>> > + // - It is guaranteed that we have exclusive access to `data.initial_data`.
>> > + let initial_data = unsafe { (*data.initial_data.get()).as_mut().unwrap_unchecked() };
>> > +
>> > + match T::receive_initial(sdev, driver_data, initial_data, buf) {
>> > + Ok(InitialReceiveResult::Pending(size)) => size,
>> > + Ok(InitialReceiveResult::Ready(size)) => {
>> > + // SAFETY:
>> > + // - It is guaranteed that we have exclusive access to `data.initial_data` and
>> > + // `data.late_probe_data`.
>> > + // - We just initialized `data.initial_data` with Some.
>> > + if let Err(err) = unsafe { Self::do_late_probe(sdev, data) } {
We are calling late_probe() again from initial_receive_buf_callback()? Why?
>> > + dev_err!(sdev.as_ref(), "Unable to late probe device: {err:?}\n");
>> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to
>> > + // `serdev_device`.
>> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::NO_OPS) };
>> > + return length;
>> > + }
>> > + size
>> > + }
>> > + Err(err) => {
>> > + dev_err!(
>> > + sdev.as_ref(),
>> > + "Unable to receive initial data for probe: {err:?}.\n"
>> > + );
>> > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
>> > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::NO_OPS) };
>> > + length
>> > + }
>> > + }
>> > + }
<snip>
> There currently is only one serial device bus driver in the kernel,
> which needs initial data:
> drivers/bluetooth/hci_uart.h
> drivers/bluetooth/hci_ldisc.c
> drivers/bluetooth/hci_serdev.c
>
> This driver retrieves this initial data after probe (not in the probe)
> and then initializes it with a workqueue. Given it is part of the
> kernel, I assume this is the intended behaviour.
In this case I assume the driver has a lock protected buffer in its private
data? Which would be entirely different than what you implement above, no?
^ permalink raw reply
* Re: [PATCH RFC 2/4] rust: add basic serial device bus abstractions
From: Markus Probst @ 2026-01-13 17:59 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Kari Argillander, Rob Herring, Greg Kroah-Hartman, Jiri Slaby,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linux-serial, linux-kernel, rust-for-linux
In-Reply-To: <DFNN75KWL8B9.1YHK1ZRV43W7O@kernel.org>
On Tue, 2026-01-13 at 18:37 +0100, Danilo Krummrich wrote:
> On Tue Jan 13, 2026 at 5:15 PM CET, Markus Probst wrote:
> > > > +impl<T: Driver + 'static> Adapter<T> {
> > > > + const OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > > > + receive_buf: if T::HAS_RECEIVE {
> > > > + Some(Self::receive_buf_callback)
> > > > + } else {
> > > > + None
> > > > + },
> > > > + write_wakeup: if T::HAS_WRITE_WAKEUP {
> > > > + Some(Self::write_wakeup_callback)
> > > > + } else {
> > > > + Some(bindings::serdev_device_write_wakeup)
> > > > + },
> > > > + };
> > > > + const INITIAL_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > > > + receive_buf: Some(Self::initial_receive_buf_callback),
> > > > + write_wakeup: if T::HAS_WRITE_WAKEUP_INITIAL {
> > > > + Some(Self::initial_write_wakeup_callback)
> > > > + } else {
> > > > + Some(bindings::serdev_device_write_wakeup)
> > > > + },
> > > > + };
> > > > + const NO_OPS: &'static bindings::serdev_device_ops = &bindings::serdev_device_ops {
> > > > + receive_buf: None,
> > > > + write_wakeup: Some(bindings::serdev_device_write_wakeup),
> > > > + };
> > > > +
> > > > + extern "C" fn probe_callback(sdev: *mut bindings::serdev_device) -> kernel::ffi::c_int {
> > > > + // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer to
> > > > + // a `struct serdev_device`.
> > > > + //
> > > > + // INVARIANT: `sdev` is valid for the duration of `probe_callback()`.
> > > > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > > > + let info = <Self as driver::Adapter>::id_info(sdev.as_ref());
> > > > +
> > > > + from_result(|| {
> > > > + let data = try_pin_init!(Drvdata {
> > > > + driver <- T::probe(sdev, info),
> > > > + initial_data: Some(Default::default()).into(),
> > > > + late_probe_data: None.into(),
> > > > + });
> > > > +
> > > > + sdev.as_ref().set_drvdata(data)?;
>
> This does not work, a driver can obtain its device private data with
> Device::<Bound>::drvdata() [1].
>
> For this the driver must assert the correct type, but since you use a private
> type instead of the type given by the driver, i.e. T, Device::<Bound>::drvdata()
> will always fail for the driver.
>
> [1] https://rust.docs.kernel.org/kernel/device/struct.Device.html#method.drvdata
I need to fix that.
>
> > > > +
> > > > + // SAFETY: We just set drvdata to `Drvdata<T>`.
> > > > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > > > +
> > > > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > > > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::INITIAL_OPS) };
> > > > +
> > > > + // SAFETY: The serial device bus only ever calls the probe callback with a valid pointer
> > > > + // to a `struct serdev_device`.
> > > > + to_result(unsafe {
> > > > + bindings::devm_serdev_device_open(sdev.as_ref().as_raw(), sdev.as_raw())
> > > > + })?;
>
> I don't think it is a good idea to hardcode this into the probe() callback and
> split it up in multiple stages, we can always solve things like ordering with
> new types, type states and guards.
>
> > > > +
> > > > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > > > + T::configure(sdev, unsafe { Pin::new_unchecked(&data.driver) }, info)?;
> > > > +
> > > > + if !T::HAS_RECEIVE_INITIAL {
> > > > + // SAFETY:
> > > > + // - It is guaranteed that we have exclusive access to `data.initial_data` and
> > > > + // `data.late_probe_data`.
>
> How is it ensured that this does not run concurrently with
> initial_receive_buf_callback(), etc.?
```
if !T::HAS_RECEIVE_INITIAL {
return 0;
}
```
at the beginning of the function does.
>
> > > > + // - We just initialized `data.initial_data` with Some.
> > > > + unsafe { Self::do_late_probe(sdev, data)? };
> > > > + }
>
> It is a bit unclear to me what you try to achieve here.
>
> Do you want to synchronize an initial data transfer? Then something along the
> lines of what Kari proposes seems reasonable.
>
> Or is the intention that this will run entirely async? But then distinct
> initialization stages as they appear above won't work.
Async.
The driver gets probed.
late_probe will be called, after all the necessary data for the
initialization is there.
Can you explain what doesn't work? The code has been tested with a
prototype driver before submission.
Kari his approach is synchronize inside the probe function, until all
the necessary data is there.
Which one do you think should be used for the abstraction?
>
> > > > +
> > > > + Ok(0)
> > > > + })
> > > > + }
> > > > +
> > > > + /// # Safety
> > > > + ///
> > > > + /// The caller must guarantee, that we have exclusive access to `data.initial_data` and
> > > > + /// `data.late_probe_data`. `data.initial_data` must be Some.
> > > > + /// (i. e. `late_probe` has not been called yet).
> > > > + unsafe fn do_late_probe(sdev: &Device<device::CoreInternal>, data: Pin<&Drvdata<T>>) -> Result {
> > > > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > > > + let data_driver = unsafe { Pin::new_unchecked(&data.driver) };
> > > > +
> > > > + // SAFETY: The function contract guarantees that we have exclusive access to
> > > > + // `data.initial_data`.
> > > > + let initial_data = unsafe { &mut *data.initial_data.get() };
> > > > +
> > > > + // SAFETY: The function contract guarantees that we have exclusive access to
> > > > + // `data.late_probe_data`.
> > > > + let late_probe_data = unsafe { &mut *data.late_probe_data.get() };
> > > > +
> > > > + *late_probe_data = Some(KBox::pin_init(
> > > > + T::late_probe(
> > > > + sdev,
> > > > + data_driver,
> > > > + // SAFETY: The function contract guarantees that `data.initial_data` is Some.
> > > > + unsafe { initial_data.take().unwrap_unchecked() },
> > > > + ),
> > > > + GFP_KERNEL,
> > > > + )?);
> > > > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > > > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::OPS) };
> > > > + Ok(())
> > > > + }
>
> <snip>
>
> > > > + extern "C" fn initial_receive_buf_callback(
> > > > + sdev: *mut bindings::serdev_device,
> > > > + buf: *const u8,
> > > > + length: usize,
> > > > + ) -> usize {
> > > > + if !T::HAS_RECEIVE_INITIAL {
> > > > + return 0;
> > > > + }
> > > > +
> > > > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > > > + // pointer to a `struct serdev_device`.
> > > > + //
> > > > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > > > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal>>() };
> > > > +
> > > > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > > > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > > > +
> > > > + // SAFETY: `initial_receive_buf_callback` is only ever called after a successful call to
> > > > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > > > + // and stored a `Pin<KBox<Drvdata<T>>>`.
> > > > + let data = unsafe { sdev.as_ref().drvdata_borrow::<Drvdata<T>>() };
> > > > +
> > > > + // SAFETY: `&data.driver` is guaranteed to be pinned.
> > > > + let driver_data = unsafe { Pin::new_unchecked(&data.driver) };
> > > > +
> > > > + // SAFETY:
> > > > + // - `data.initial_data` is always Some until `InitialReceiveResult::Ready` is
> > > > + // returned below.
> > > > + // - It is guaranteed that we have exclusive access to `data.initial_data`.
> > > > + let initial_data = unsafe { (*data.initial_data.get()).as_mut().unwrap_unchecked() };
> > > > +
> > > > + match T::receive_initial(sdev, driver_data, initial_data, buf) {
> > > > + Ok(InitialReceiveResult::Pending(size)) => size,
> > > > + Ok(InitialReceiveResult::Ready(size)) => {
> > > > + // SAFETY:
> > > > + // - It is guaranteed that we have exclusive access to `data.initial_data` and
> > > > + // `data.late_probe_data`.
> > > > + // - We just initialized `data.initial_data` with Some.
> > > > + if let Err(err) = unsafe { Self::do_late_probe(sdev, data) } {
>
> We are calling late_probe() again from initial_receive_buf_callback()? Why?
It only gets called once.
This is gated behind "if T::HAS_RECEIVE_INITIAL". And in probe its
gated behind "if T::HAS_RECEIVE_INITIAL".
If "receive_initial" does not get implemented by the driver, late probe
will be run immediately after the regular probe.
>
> > > > + dev_err!(sdev.as_ref(), "Unable to late probe device: {err:?}\n");
> > > > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to
> > > > + // `serdev_device`.
> > > > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::NO_OPS) };
> > > > + return length;
> > > > + }
> > > > + size
> > > > + }
> > > > + Err(err) => {
> > > > + dev_err!(
> > > > + sdev.as_ref(),
> > > > + "Unable to receive initial data for probe: {err:?}.\n"
> > > > + );
> > > > + // SAFETY: `sdev.as_raw()` is guaranteed to be a valid pointer to `serdev_device`.
> > > > + unsafe { bindings::serdev_device_set_client_ops(sdev.as_raw(), Self::NO_OPS) };
> > > > + length
> > > > + }
> > > > + }
> > > > + }
>
> <snip>
>
> > There currently is only one serial device bus driver in the kernel,
> > which needs initial data:
> > drivers/bluetooth/hci_uart.h
> > drivers/bluetooth/hci_ldisc.c
> > drivers/bluetooth/hci_serdev.c
> >
> > This driver retrieves this initial data after probe (not in the probe)
> > and then initializes it with a workqueue. Given it is part of the
> > kernel, I assume this is the intended behaviour.
>
> In this case I assume the driver has a lock protected buffer in its private
> data? Which would be entirely different than what you implement above, no?
^ permalink raw reply
* Re: [PATCH RFC 2/4] rust: add basic serial device bus abstractions
From: Danilo Krummrich @ 2026-01-13 19:10 UTC (permalink / raw)
To: Markus Probst
Cc: Kari Argillander, Rob Herring, Greg Kroah-Hartman, Jiri Slaby,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
linux-serial, linux-kernel, rust-for-linux
In-Reply-To: <dcab1e61e451aeba27575c8245aef687caf94b23.camel@posteo.de>
On Tue Jan 13, 2026 at 6:59 PM CET, Markus Probst wrote:
> This is gated behind "if T::HAS_RECEIVE_INITIAL". And in probe its
> gated behind "if T::HAS_RECEIVE_INITIAL".
I guess you mean `if !T::HAS_RECEIVE_INITIAL` in probe().
Anyways, I now get what you are trying to do. (I got confused by late_probe(),
which I think is a misleading name for what it actually is and in which context
it might run in. I think a better name would be initial_xfer() or something
along those lines.)
So, what you really want is additional private data that is valid between some
inital xfer happening at some point of time after probe() and unbind(), and
hence is valid to access in all subsequent transfers callbacks.
There are three options:
(1) What you already implement, but with less callbacks.
I think the only additional callback you need is initial_xfer(), where you
return the init private data.
For storing this data you probably want to add void *init_data to
struct serdev_device.
(2) A synchronous inital transfer in probe() as proposed by Kari.
This is much simpler than (1), but still leaves the driver without an
Option for the init data in its device private data.
(3) Leave it to the driver.
The driver can store an Option in its device private data, which is
initialized in the first callback.
> Which one do you think should be used for the abstraction?
I don't know, it depends on the requirements of serdev drivers.
But since you already mentioned that there is only a single driver that needs
this initial xfer data, I'd say don't consider it in the abstraction at all
until we are sure it is a common pattern needed by drivers. We can always add it
later on if needed.
^ permalink raw reply
* Re: [PATCH v4 15/15] arm64: dts: microchip: add EV23X71A board
From: Robert Marko @ 2026-01-13 20:09 UTC (permalink / raw)
To: claudiu beznea
Cc: robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni,
herbert, davem, vkoul, andi.shyti, lee, andrew+netdev, edumazet,
kuba, pabeni, linusw, Steen.Hegelund, daniel.machon,
UNGLinuxDriver, olivia, radu_nicolae.pirea, richard.genoud,
gregkh, jirislaby, broonie, lars.povlsen, devicetree,
linux-arm-kernel, linux-kernel, linux-crypto, dmaengine,
linux-i2c, netdev, linux-gpio, linux-spi, linux-serial, linux-usb,
luka.perkov
In-Reply-To: <858ca139-61c5-45e3-a2c9-d0af414e3592@tuxon.dev>
On Sun, Jan 11, 2026 at 3:42 PM claudiu beznea <claudiu.beznea@tuxon.dev> wrote:
>
> Hi, Robert,
>
> On 12/29/25 20:37, Robert Marko wrote:
> > Microchip EV23X71A is an LAN9696 based evaluation board.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > ---
> > Changes in v2:
> > * Split from SoC DTSI commit
> > * Apply DTS coding style
> > * Enclose array in i2c-mux
> > * Alphanumericaly sort nodes
> > * Change management port mode to RGMII-ID
> >
> > arch/arm64/boot/dts/microchip/Makefile | 1 +
> > .../boot/dts/microchip/lan9696-ev23x71a.dts | 757 ++++++++++++++++++
> > 2 files changed, 758 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/microchip/lan9696-ev23x71a.dts
> >
> > diff --git a/arch/arm64/boot/dts/microchip/Makefile b/arch/arm64/boot/dts/microchip/Makefile
> > index c6e0313eea0f..09d16fc1ce9a 100644
> > --- a/arch/arm64/boot/dts/microchip/Makefile
> > +++ b/arch/arm64/boot/dts/microchip/Makefile
> > @@ -1,4 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0
> > +dtb-$(CONFIG_ARCH_LAN969X) += lan9696-ev23x71a.dtb
> > dtb-$(CONFIG_ARCH_SPARX5) += sparx5_pcb125.dtb
> > dtb-$(CONFIG_ARCH_SPARX5) += sparx5_pcb134.dtb sparx5_pcb134_emmc.dtb
> > dtb-$(CONFIG_ARCH_SPARX5) += sparx5_pcb135.dtb sparx5_pcb135_emmc.dtb
> > diff --git a/arch/arm64/boot/dts/microchip/lan9696-ev23x71a.dts b/arch/arm64/boot/dts/microchip/lan9696-ev23x71a.dts
> > new file mode 100644
> > index 000000000000..435df455b078
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/microchip/lan9696-ev23x71a.dts
>
> [ ...]
>
> > +&gpio {
> > + emmc_sd_pins: emmc-sd-pins {
> > + /* eMMC_SD - CMD, CLK, D0, D1, D2, D3, D4, D5, D6, D7, RSTN */
> > + pins = "GPIO_14", "GPIO_15", "GPIO_16", "GPIO_17",
> > + "GPIO_18", "GPIO_19", "GPIO_20", "GPIO_21",
> > + "GPIO_22", "GPIO_23", "GPIO_24";
> > + function = "emmc_sd";
> > + };
> > +
> > + fan_pins: fan-pins {
> > + pins = "GPIO_25", "GPIO_26";
> > + function = "fan";
> > + };
> > +
> > + fc0_pins: fc0-pins {
> > + pins = "GPIO_3", "GPIO_4";
> > + function = "fc";
> > + };
> > +
> > + fc2_pins: fc2-pins {
> > + pins = "GPIO_64", "GPIO_65", "GPIO_66";
> > + function = "fc";
> > + };
> > +
> > + fc3_pins: fc3-pins {
> > + pins = "GPIO_55", "GPIO_56";
> > + function = "fc";
> > + };
> > +
> > + mdio_pins: mdio-pins {
> > + pins = "GPIO_9", "GPIO_10";
> > + function = "miim";
> > + };
> > +
> > + mdio_irq_pins: mdio-irq-pins {
> > + pins = "GPIO_11";
> > + function = "miim_irq";
> > + };
> > +
> > + sgpio_pins: sgpio-pins {
> > + /* SCK, D0, D1, LD */
> > + pins = "GPIO_5", "GPIO_6", "GPIO_7", "GPIO_8";
> > + function = "sgpio_a";
> > + };
> > +
> > + usb_ulpi_pins: usb-ulpi-pins {
> > + pins = "GPIO_30", "GPIO_31", "GPIO_32", "GPIO_33",
> > + "GPIO_34", "GPIO_35", "GPIO_36", "GPIO_37",
> > + "GPIO_38", "GPIO_39", "GPIO_40", "GPIO_41";
> > + function = "usb_ulpi";
> > + };
> > +
> > + usb_rst_pins: usb-rst-pins {
> > + pins = "GPIO_12";
> > + function = "usb2phy_rst";
> > + };
> > +
> > + usb_over_pins: usb-over-pins {
> > + pins = "GPIO_13";
> > + function = "usb_over_detect";
> > + };
> > +
> > + usb_power_pins: usb-power-pins {
> > + pins = "GPIO_1";
> > + function = "usb_power";
> > + };
> > +
> > + ptp_out_pins: ptp-out-pins {
> > + pins = "GPIO_58";
> > + function = "ptpsync_4";
> > + };
>
> Could you please move this one upper to have all the entries in the gpio
> container alphanumerically sorted?
>
> > +
> > + ptp_ext_pins: ptp-ext-pins {
> > + pins = "GPIO_59";
> > + function = "ptpsync_5";
> > + };
>
> Same here.
Sure, I will make sure that pin nodes are alphabetical (I found some
more that are not) in v5.
>
> [ ...]
>
> > + port29: port@29 {
> > + reg = <29>;
> > + phys = <&serdes 11>;
> > + phy-handle = <&phy3>;
> > + phy-mode = "rgmii-id";
> > + microchip,bandwidth = <1000>;
>
> There are some questions around this node from Andrew in v1 of this series,
> which I don't see an answer for in any of the following versions. Could you
> please clarify?
Sure, as for the RGMII I switched to rgmii-id so the PHY is adding the delays.
Though, I am not sure if its better to add them via MAC as it can add
the delays instead of the PHY,
so I am open to suggestions here.
As for the phys property, yes that is not required here as RGMII ports
are dedicated, there are no
SERDES lanes being used for them.
I have updated the bindings to account for this and it will be part of v5.
Regards,
Robert
>
> The rest looks good to me.
>
> Thank you,
> Claudiu
>
--
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr
^ permalink raw reply
* Re: [PATCH v4 00/11] riscv: spacemit: Add SpacemiT K3 SoC and K3 Pico-ITX board
From: Conor Dooley @ 2026-01-13 22:17 UTC (permalink / raw)
To: Yixun Lan
Cc: Guodong Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Daniel Lezcano, Thomas Gleixner, Samuel Holland, Anup Patel,
Greg Kroah-Hartman, Jiri Slaby, Lubomir Rintel, Yangyu Chen,
Paul Walmsley, Heinrich Schuchardt, Kevin Meng Zhang,
Andrew Jones, devicetree, linux-riscv, linux-kernel, spacemit,
linux-serial, Krzysztof Kozlowski, Heinrich Schuchardt,
Conor Dooley
In-Reply-To: <20260113002123-GYA19926@gentoo.org>
[-- Attachment #1: Type: text/plain, Size: 2733 bytes --]
On Tue, Jan 13, 2026 at 08:21:23AM +0800, Yixun Lan wrote:
> Hi Conor,
>
> On 21:45 Mon 12 Jan , Conor Dooley wrote:
> > On Sat, Jan 10, 2026 at 01:18:12PM +0800, Guodong Xu wrote:
> >
> > > Hi, Conor
> > >
> > > For the binding riscv/extensions.ymal, here's what changed in v3 (no
> > > change in v4):
> > >
> > > 1. Dropped the patch of adding "supm" into extensions.yaml. At the same
> > > time, I will start another patchset which implements the strategy
> > > outlined by Conor in Link [2] and by Samuel in Link [3].
> >
> > Okay, that seems reasonable to separate out.
> >
> > >
> > > 2. Dropped the dependency checks for "sha" on "h", "shcounterenw", and
> > > 6 others. "sha" implies these extensions, and it should be allowed
> > > to be declared independently. Like "a" implies "zaamo" and "zalrsc".
> > >
> > > 3. Enchanced the dependency check of "ziccamoa" on "a". Specifically,
> > > - added the dependency check of "ziccamoa" on "zaamo" or on "a".
> > > - added the dependency check of "za64rs" on "zalrsc" or on "a".
> > > - added the dependency check of "ziccrse" on "zalrsc" or "a".
> > > The commit message of this patch is updated too, to better explain the
> > > relationship between "ziccamoa", "za64rs", "ziccrse" and "a".
> > >
> > > 4. Enhanced checking dependency of "b" and "zba", "zbb", "zbs", making the
> > > dependency check in both directions, as discussed in [4]. Since "b"
> > > was ratified much later than its component extensions (zba/zbb/zbs),
> > > existing software and kernels expect these explicit strings. This
> > > bidirectional check ensures cores declaring "b" remain compatible
> > > with older software that only recognizes zba/zbb/zbs.
> >
> > This I asked about in the relevant patch, I would like to know what your
> > plan for adding the "b"s is.
> >
> ..
> > Spacemit folks, I assume you weren't planning on taking the
> > extensions.yaml stuff via your tree? If you weren't, I'll grab it once
> > the question about b is answered.
>
> sure, please take extension stuff which are patches 6-9, for 1-5, it's
> all about adding support for SpacemiT K3 SoC, to avoid petential conflicts,
> I wouldn't mind if you also taking them? then I can handle the rest 10,11 for DT
Stuff for spacemit is either for you or for the relevant subsystem
maintainers. You're probably safe enough taking the
timer/interrupt-controller stuff if the maintainers don't apply it in a
reasonable period, it's not abnormal for those in particular to go via
the platform maintainer in my experience. Just be clear that you have
done so. I'm only interested in taking 6-9.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: (subset) [PATCH v4 00/11] riscv: spacemit: Add SpacemiT K3 SoC and K3 Pico-ITX board
From: Conor Dooley @ 2026-01-13 22:25 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Yixun Lan,
Daniel Lezcano, Thomas Gleixner, Samuel Holland, Anup Patel,
Greg Kroah-Hartman, Jiri Slaby, Lubomir Rintel, Yangyu Chen,
Guodong Xu
Cc: conor, Conor Dooley, Paul Walmsley, Heinrich Schuchardt,
Kevin Meng Zhang, Andrew Jones, devicetree, linux-riscv,
linux-kernel, spacemit, linux-serial, Krzysztof Kozlowski,
Heinrich Schuchardt
In-Reply-To: <20260110-k3-basic-dt-v4-0-d492f3a30ffa@riscstar.com>
From: Conor Dooley <conor.dooley@microchip.com>
On Sat, 10 Jan 2026 13:18:12 +0800, Guodong Xu wrote:
> This series introduces basic support for the SpacemiT K3 SoC and the
> K3 Pico-ITX evaluation board.
>
> This series (starting from v2) also adds descriptions about ISA extensions
> mandated by the RVA23 Profile Version 1.0 into riscv/extensions.yaml.
> There are extensive discussions about how to handle these new extensions
> in v2. In v3 (now v4), here is my best understading of what I think we have
> reached consensus on.
>
> [...]
Applied 6-9 to riscv-dt-for-next :)
[06/11] dt-bindings: riscv: Add B ISA extension description
https://git.kernel.org/conor/c/0cdb7fc1879b
[07/11] dt-bindings: riscv: Add descriptions for Za64rs, Ziccamoa, Ziccif, and Zicclsm
https://git.kernel.org/conor/c/b321256a4f36
[08/11] dt-bindings: riscv: Add Ssccptr, Sscounterenw, Sstvala, Sstvecd, Ssu64xl
https://git.kernel.org/conor/c/c712413333f8
[09/11] dt-bindings: riscv: Add Sha and its comprised extensions
https://git.kernel.org/conor/c/89febd6a0276
Thanks,
Conor.
^ permalink raw reply
* Re: [PATCH 00/19] printk cleanup - part 3
From: Marcos Paulo de Souza @ 2026-01-14 0:32 UTC (permalink / raw)
To: Daniel Thompson
Cc: Richard Weinberger, Anton Ivanov, Johannes Berg,
Greg Kroah-Hartman, Jason Wessel, Daniel Thompson,
Douglas Anderson, Petr Mladek, Steven Rostedt, John Ogness,
Sergey Senozhatsky, Jiri Slaby, Breno Leitao, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Geert Uytterhoeven, Kees Cook, Tony Luck, Guilherme G. Piccoli,
Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy, Andreas Larsson, Alexander Shishkin,
Maxime Coquelin, Alexandre Torgue, Jacky Huang, Shan-Chun Hung,
Laurentiu Tudor, linux-um, linux-kernel, kgdb-bugreport,
linux-serial, netdev, linux-m68k, linux-hardening, linuxppc-dev,
sparclinux, linux-stm32, linux-arm-kernel, linux-fsdevel
In-Reply-To: <a5d83903fe2d2c2eb21de1527007913ff00847c5.camel@suse.com>
On Tue, 2026-01-13 at 09:41 -0300, Marcos Paulo de Souza wrote:
> On Mon, 2026-01-05 at 14:08 +0000, Daniel Thompson wrote:
> > On Mon, Jan 05, 2026 at 12:52:14PM +0000, Daniel Thompson wrote:
> > > Hi Marcos
> > >
> > > On Sat, Dec 27, 2025 at 09:16:07AM -0300, Marcos Paulo de Souza
> > > wrote:
> > > > The parts 1 and 2 can be found here [1] and here[2].
> > > >
> > > > The changes proposed in this part 3 are mostly to clarify the
> > > > usage of
> > > > the interfaces for NBCON, and use the printk helpers more
> > > > broadly.
> > > > Besides it, it also introduces a new way to register consoles
> > > > and drop thes the CON_ENABLED flag. It seems too much, but in
> > > > reality
> > > > the changes are not complex, and as the title says, it's
> > > > basically a
> > > > cleanup without changing the functional changes.
> > >
> > > I ran this patchset through the kgdb test suite and I'm afraid it
> > > is
> > > reporting functional changes.
> > >
> > > Specifically the earlycon support for kdb has regressed (FWIW the
> > > problem bisects down to the final patch in the series where
> > > CON_ENABLED
> > > is removed).
> > >
> > > Reproduction on x86-64 KVM outside of the test suite should be
> > > easy:
> > >
> > > make defconfig
> > > scripts/config \
> > > --enable DEBUG_INFO \
> > > --enable DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT \
> > > --enable DEBUG_FS \
> > > --enable KALLSYMS_ALL \
> > > --enable MAGIC_SYSRQ \
> > > --enable KGDB \
> > > --enable KGDB_TESTS \
> > > --enable KGDB_KDB \
> > > --enable KDB_KEYBOARD \
> > > --enable LKDTM \
> > > --enable SECURITY_LOCKDOWN_LSM
> > > make olddefconfig
> > > make -j$(nproc)
> > > qemu-system-x86_64 \
> > > -m 1G -smp 2 -nographic \
> > > -kernel arch/x86/boot/bzImage \
> > > -append "console=ttyS0,115200 kgdboc=ttyS0
> > > earlycon=uart8250,io,0x3f8 kgdboc_earlycon kgdbwait"
> >
> > Actually I realized there was a simpler reproduction (hinted at by
> > the
> > missing "printk: legacy bootconsole [uart8250] enabled" in the
> > regressed
> > case). It looks like the earlycon simply doesn't work and that
> > means
> > the
> > reproduction doesn't require anything related to kgdb at all.
> > Simply:
> >
> > make defconfig
> > make -j$(nproc)
> > qemu-system-x86_64 -m 1G -smp 2 -nographic -kernel
> > arch/x86/boot/bzImage \
> > -append "earlycon=uart8250,io,0x3f8"
> >
> > With the part 3 patchset applied I get no output from the earlycon
> > (without the patch set I get the early boot messages which, as
> > expected,
> > stop when tty0 comes up).
>
> Hi Daniel, sorry for the late reply! Lots of things to check lately
> :)
>
> Ok, I reproduced here, thanks a lot for testing kgdboc, it's a quick
> way to check that the new register_console_force is not working. Let
> me
> take a look to find what's wrong. Thanks a lot for finding this
> issue!
Ok, I did a bisect and found out that the issue lies in the last
commit, where CON_ENABLED was removed. After it, I then checked what
was wrong, since everything was being plumbed correctly (tm), and then
I found that it was not:
On _register_console, the function try_enable_default_console is called
when there are not registered consoles, and then it sets CON_ENABLED
for the console. Later on, try_enable_preferred_console it checks if
the console was specified by the user, and at the same time it had
CON_ENABLED set.
It worked by chance, but now, we don't have this flag anymore, and then
we are not _marking_ the console on try_enable_default_console so
try_enable_preferred_console returns ENOENT.
I have added logs for both cases first the case with the patchset
applied but the last one patch, and it works:
$ vng --append "console=ttyS0,115200 earlyprintk=ttyS0,115200
kgdboc=ttyS0 earlycon=uart8250,io,0x3f8 kgdboc_earlycon kgdbwait" --
verbose
Decompressing Linux... Parsing ELF... Performing relocations... done.
Booting the kernel (entry_offset: 0x000000000450d530).
XXX register_console earlyser
XXX try_enable_default_console earlyser enabled
XXX try_enable_preferred_console earlyser user_specified 1 returned -
ENOENT
XXX try_enable_preferred_console earlyser user_specified 0 returned 0
because flags was ENABLED
^^ here, returning 0 means that the console was accepted and will be
registered
XXX __register_console earlyser registered
XXX register_console uart
XXX try_enable_default_console uart enabled
XXX try_enable_preferred_console uart user_specified 1 returned -ENOENT
XXX try_enable_preferred_console uart user_specified 0 returned 0
because flags was ENABLED
XXX __register_console uart registered
^^^^ same here
Going to register kgdb with earlycon 'uart'
Entering kdb (current=0x0000000000000000, pid 0)
Now, the logs of the patchset with the last patch also applied:
Decompressing Linux... Parsing ELF... Performing relocations... done.
Booting the kernel (entry_offset: 0x000000000450d530).
XXX register_console earlyser
XXX try_enable_default_console earlyser enabled
XXX try_enable_preferred_console earlyser user_specified 1 returned -
ENOENT
XXX try_enable_preferred_console earlyser user_specified 0 returned -
ENOENT
XXX register_console uart
XXX try_enable_default_console uart enabled
XXX try_enable_preferred_console uart user_specified 1 returned -ENOENT
XXX try_enable_preferred_console uart user_specified 0 returned -ENOENT
^^^^ here, it should have registered the console
XXX console_setup hvc0
XXX __add_preferred_console hvc added, idx 0 i 0
XXX console_setup ttyS0,115200
XXX __add_preferred_console ttyS added, idx 0 i 1
Poking KASLR using RDRAND RDTSC...
XXX register_console tty
XXX try_enable_preferred_console tty user_specified 1 returned -ENOENT
XXX try_enable_preferred_console tty user_specified 0 returned -ENOENT
^^^ again, it fails because we don't flag the console with CON_ENABLED
as before.
XXX register_console hvc
XXX register_console ttyS
XXX try_enable_preferred_console ttyS user_specified 1 returned 0 with
user specified
XXX __register_console ttyS registered
[ 0.000000] Linux version 6.18.0+ (mpdesouza@daedalus) (clang
version 21.1.7, LLD 21.1.7) #374 SMP PREEMPT_RT Tue J
an 13 21:08:34 -03 2026 reserved
[ 0.000000] earlycon: uart8250 at I/O port 0x3f8 (options '')
[ 0.000000] kgdboc: No suitable earlycon yet, will try later
So, without any console kgdb is activated much later in the boot
process, as you found it.
I talked with Petr Mladek and it would need to rework the way that we
register a console, and he's already working on it. For now I believe
that we could take a look in all the patches besides the last one that
currently breaks the earlycon with kgdb and maybe other usecases.
Sorry for not catching this issue before. I'll use kgdb next time to
make sure that it keeps working :)
^ permalink raw reply
* Re: [PATCH v4 00/11] riscv: spacemit: Add SpacemiT K3 SoC and K3 Pico-ITX board
From: Yixun Lan @ 2026-01-14 2:14 UTC (permalink / raw)
To: Conor Dooley, Paul Walmsley
Cc: Guodong Xu, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Daniel Lezcano,
Thomas Gleixner, Samuel Holland, Anup Patel, Greg Kroah-Hartman,
Jiri Slaby, Lubomir Rintel, Yangyu Chen, Paul Walmsley,
Heinrich Schuchardt, Kevin Meng Zhang, Andrew Jones, devicetree,
linux-riscv, linux-kernel, spacemit, linux-serial,
Krzysztof Kozlowski, Heinrich Schuchardt, Conor Dooley
In-Reply-To: <20260113-swarm-mama-cbd7d0546578@spud>
On 22:17 Tue 13 Jan , Conor Dooley wrote:
> On Tue, Jan 13, 2026 at 08:21:23AM +0800, Yixun Lan wrote:
> > Hi Conor,
> >
> > On 21:45 Mon 12 Jan , Conor Dooley wrote:
> > > On Sat, Jan 10, 2026 at 01:18:12PM +0800, Guodong Xu wrote:
> > >
> > > > Hi, Conor
> > > >
> > > > For the binding riscv/extensions.ymal, here's what changed in v3 (no
> > > > change in v4):
> > > >
> > > > 1. Dropped the patch of adding "supm" into extensions.yaml. At the same
> > > > time, I will start another patchset which implements the strategy
> > > > outlined by Conor in Link [2] and by Samuel in Link [3].
> > >
> > > Okay, that seems reasonable to separate out.
> > >
> > > >
> > > > 2. Dropped the dependency checks for "sha" on "h", "shcounterenw", and
> > > > 6 others. "sha" implies these extensions, and it should be allowed
> > > > to be declared independently. Like "a" implies "zaamo" and "zalrsc".
> > > >
> > > > 3. Enchanced the dependency check of "ziccamoa" on "a". Specifically,
> > > > - added the dependency check of "ziccamoa" on "zaamo" or on "a".
> > > > - added the dependency check of "za64rs" on "zalrsc" or on "a".
> > > > - added the dependency check of "ziccrse" on "zalrsc" or "a".
> > > > The commit message of this patch is updated too, to better explain the
> > > > relationship between "ziccamoa", "za64rs", "ziccrse" and "a".
> > > >
> > > > 4. Enhanced checking dependency of "b" and "zba", "zbb", "zbs", making the
> > > > dependency check in both directions, as discussed in [4]. Since "b"
> > > > was ratified much later than its component extensions (zba/zbb/zbs),
> > > > existing software and kernels expect these explicit strings. This
> > > > bidirectional check ensures cores declaring "b" remain compatible
> > > > with older software that only recognizes zba/zbb/zbs.
> > >
> > > This I asked about in the relevant patch, I would like to know what your
> > > plan for adding the "b"s is.
> > >
> > ..
> > > Spacemit folks, I assume you weren't planning on taking the
> > > extensions.yaml stuff via your tree? If you weren't, I'll grab it once
> > > the question about b is answered.
> >
> > sure, please take extension stuff which are patches 6-9, for 1-5, it's
> > all about adding support for SpacemiT K3 SoC, to avoid petential conflicts,
> > I wouldn't mind if you also taking them? then I can handle the rest 10,11 for DT
>
> Stuff for spacemit is either for you or for the relevant subsystem
> maintainers. You're probably safe enough taking the
> timer/interrupt-controller stuff if the maintainers don't apply it in a
> reasonable period, it's not abnormal for those in particular to go via
> the platform maintainer in my experience. Just be clear that you have
> done so. I'm only interested in taking 6-9.
Hi Conor,
Ok, I got, thank you!
Hi Paul Walmsley,
I assume you're responsible for more general riscv stuff with your
effective maintainer hat, so do you mind if I take patches 1-5 via SpacemiT
SoC tree? I think the potential conflicts should be low and easy to fix.
Or, in the other hand, just let me know which patches you would like
to take, then I will handle the rest. Thanks
--
Yixun Lan (dlan)
^ permalink raw reply
* Re: [PATCH v3 03/14] software node: Implement device_get_match_data fwnode callback
From: Sui Jingfeng @ 2026-01-14 3:40 UTC (permalink / raw)
To: Andy Shevchenko, manivannan.sadhasivam
Cc: Rob Herring, Greg Kroah-Hartman, Jiri Slaby, Nathan Chancellor,
Nicolas Schier, Hans de Goede, Ilpo Järvinen, Mark Pearson,
Derek J. Clark, Manivannan Sadhasivam, Krzysztof Kozlowski,
Conor Dooley, Marcel Holtmann, Luiz Augusto von Dentz,
Bartosz Golaszewski, Daniel Scally, Heikki Krogerus, Sakari Ailus,
Rafael J. Wysocki, Danilo Krummrich, Bartosz Golaszewski,
linux-serial, linux-kernel, linux-kbuild, platform-driver-x86,
linux-pci, devicetree, linux-arm-msm, linux-bluetooth, linux-pm,
Stephan Gerhold, Dmitry Baryshkov, linux-acpi, linux-pci
In-Reply-To: <aWSpFk9z0zpyKjr6@smile.fi.intel.com>
On 2026/1/12 15:56, Andy Shevchenko wrote:
> On Sat, Jan 10, 2026 at 12:26:21PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
>
>> Because the software node backend of the fwnode API framework lacks an
>> implementation for the .device_get_match_data function callback.
>
> Maybe this is done on purpose.
It is a *fact* that the broken swnode lacks an implementation for the
.device_get_match_data stub.
Otherwise, If it is really done *on purpose*, the maintainers of swnode
backend probably shall document it in the source file *explicitly*.
Have you thought about this aspect?
>
If it is sure thing, then it shouldn't start with "Maybe ..."
>> This makes it difficult to use(and/or test) a few drivers that originates
>> from DT world on the non-DT platform.
>
> How difficult?
The emphasis isn't on the 'difficult' word, it means 'inconvenience'
> DSA implementation went to the way of taking DT overlay
> approach. Why that one can't be applied here?
Software node as an complement of ACPI, Therefore should do the same.
DT overlay introduce extra overhead/side effects on the non-DT systems.
Besides, DT overlay requires the OS distribution(such as ubuntu) has the
DT overlay config option selected.
>
>> Implement the .device_get_match_data fwnode callback, which helps to keep
>> the three backends of the fwnode API aligned as much as possible. This is
>> also a fundamental step to make a few drivers OF-independent truely
>> possible.
>>
>> Device drivers or platform setup codes are expected to provide a software
>> node string property, named as "compatible". At this moment, the value of
>> this string property is being used to match against the compatible entries
>> in the of_device_id table. It can be extended in the future though.
>
> I really do not want to see this patch
You can do that by dropping the maintainer-ship.
Your endless, bruth-force ranting on such a straight-forward thing
doesn't make much sense, because that waste everybody's time.
> without very good justification
Justifications has been provided over and over again.
> (note, there were at least two attempts in the past to add this stuff
This exactly saying that the implementation is missing.
> and no-one was merged,
That's the reason why you see it at least the second time.
have you studied those cases?).
>
The first one is not 100% correct.
^ permalink raw reply
* Re: [PATCH v2] serial:xilinx_uartps:fix rs485 delay_rts_after_send
From: Shubhrajyoti Datta @ 2026-01-14 4:38 UTC (permalink / raw)
To: j.turek
Cc: gregkh, jirislaby, michal.simek, namcao, tglx, zack.rusin,
sean.anderson, hshah, linux-serial, linux-arm-kernel,
linux-kernel
In-Reply-To: <20251221103221.1971125-1-jakub.turek@elsta.tech>
On Sun, Dec 21, 2025 at 4:03 PM j.turek <jakub.turek@elsta.tech> wrote:
>
> RTS line control with delay should be triggered when there is no more bytes
> in kfifo and hardware buffer is empty. Without this patch RTS control is
> scheduled right after feeding hardware buffer and this is too early.
> RTS line may change state before hardware buffer is empty.
> With this patch delayed RTS state change is triggered when function
> cdns_uart_handle_tx is called from cdns_uart_isr on CDNS_UART_IXR_TXEMPTY
> exactly when hardware completed transmission
>
> Signed-off-by: Jakub Turek <jakub.turek@elsta.tech>
>
> Fixes: fccc9d9233f9 ("tty: serial: uartps: Add rs485 support to uartps driver")
Reviewed-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
^ 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