[PATCH v5 0/4] Introduce Synology Microp driver

[PATCH v5 0/4] Introduce Synology Microp driver

[Markus Probst via B4 Relay]

Synology uses a microcontroller in their NAS devices connected to a
serial port to control certain LEDs, fan speeds, a beeper, to handle
proper shutdown and restart, buttons and fan failures.

This patch series depends on the rust led abstraction [1] and the rust
serdev abstraction [2].

This is only a initial version of the driver able to control LEDs.
The following rust abstractions would be required, to implement the
remaining features:
- hwmon (include/linux/hwmon.h)
- input (include/linux/input.h)
- sysoff handler + hardware protection shutdown (include/linux/reboot.h)

[1] https://lore.kernel.org/rust-for-linux/20260329-rust_leds-v13-0-21a599c5b2d1@posteo.de/
[2] https://lore.kernel.org/rust-for-linux/20260313-rust_serdev-v3-0-c9a3af214f7f@posteo.de/

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Changes in v5:
- add esata led support
- use different compatible for each model
- add visibility modifier to of_device_table macro
- fix match data missing when using PRP0001
- Link to v4: https://lore.kernel.org/r/20260320-synology_microp_initial-v4-0-0423ddb83ca4@posteo.de

Changes in v4:
- convert to monolithic driver and moved it into drivers/platform
- removed mfd rust abstraction
- moved dt-bindings to embedded-controller
- Link to v3: https://lore.kernel.org/r/20260313-synology_microp_initial-v3-0-ad6ac463a201@posteo.de

Changes in v3:
- remove `default n` from Kconfig entry, as n is the default already.
- select RUST_SERIAL_DEV_BUS_ABSTRACTIONS in Kconfig
- add mfd rust abstraction
- split core and led parts into their own driver. It should now be considered a
  MFD device.
- split led part of dt binding into its own file
- Link to v2: https://lore.kernel.org/r/20260308-synology_microp_initial-v2-0-9389963f31c5@posteo.de

Changes in v2:
- fix missing tabs in MAINTAINERS file
- remove word binding from patch subject
- add missing signed-off-by
- add missing help entry in Kconfig
- add missing spdx license headers
- remove no-check{,-cpu}-fan properties from the dt-bindings and replace
  them with the check_fan module parameter
- use patternProperties for leds in dt-bindings
- license dt-binding as GPL-2.0-only OR BSD-2-Clause
- move driver from staging tree into mfd tree and mark it as work in
  progress inside Kconfig
- only register alert and usb led if fwnode is present
- Link to v1: https://lore.kernel.org/r/20260306-synology_microp_initial-v1-0-fcffede6448c@posteo.de

---
Markus Probst (4):
      dt-bindings: embedded-controller: Add synology microp devices
      ACPI: of: match PRP0001 in of_match_device
      rust: add visibility to of_device_table macro
      platform: Add initial synology microp driver

 .../synology,ds923p-microp.yaml                    |  65 +++++
 MAINTAINERS                                        |   6 +
 drivers/acpi/bus.c                                 |   7 +-
 drivers/of/device.c                                |   9 +-
 drivers/platform/Kconfig                           |   2 +
 drivers/platform/Makefile                          |   1 +
 drivers/platform/synology_microp/Kconfig           |  13 +
 drivers/platform/synology_microp/Makefile          |   3 +
 drivers/platform/synology_microp/TODO              |   7 +
 drivers/platform/synology_microp/command.rs        |  55 ++++
 drivers/platform/synology_microp/led.rs            | 276 +++++++++++++++++++++
 drivers/platform/synology_microp/model.rs          | 171 +++++++++++++
 .../platform/synology_microp/synology_microp.rs    |  54 ++++
 include/linux/acpi.h                               |  11 +
 rust/kernel/of.rs                                  |   5 +-
 15 files changed, 678 insertions(+), 7 deletions(-)
---
base-commit: d1d81e9d1a4dd846aee9ae77ff9ecc2800d72148
change-id: 20260306-synology_microp_initial-0f7dac7b7496
prerequisite-change-id: 20251217-rust_serdev-ee5481e9085c:v3
prerequisite-patch-id: 52b17274481cc770c257d8f95335293eca32a2c5
prerequisite-patch-id: eec47e5051640d08bcd34a9670b98804449cad52
prerequisite-patch-id: f24b68c71c3f69371e8ac0251efca0a023b31cc4
prerequisite-patch-id: 3dfc1f7e5ecd3e0dd65d676aeb16f55260847b25
prerequisite-change-id: 20251114-rust_leds-a959f7c2f7f9:v13
prerequisite-patch-id: 818700f22dcb9676157c985f82762d7c607b861e
prerequisite-patch-id: b15ffa7d95d9260151bfb116b259c4473f721c82
prerequisite-patch-id: 8c47e0d107530f577a1be0b79f8ee791f95d3cbe

[PATCH v5 1/4] dt-bindings: embedded-controller: Add synology microp devices

[Markus Probst via B4 Relay]

From: Markus Probst <markus.probst@posteo.de>

Add the Synology Microp devicetree bindings. Those devices are
microcontrollers found on Synology NAS devices. They are connected to a
serial port on the host device.

Those devices are used to control certain LEDs, fan speeds, a beeper, to
handle buttons, fan failures and to properly shutdown and reboot the
device.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 .../synology,ds923p-microp.yaml                    | 65 ++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml b/Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml
new file mode 100644
index 000000000000..599d32ce2be9
--- /dev/null
+++ b/Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/embedded-controller/synology,ds923p-microp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synology NAS on-board Microcontroller
+
+maintainers:
+  - Markus Probst <markus.probst@posteo.de>
+
+description: |
+  Synology Microp is a microcontroller found in Synology NAS devices.
+  It is connected to a serial port on the host device.
+
+  It is necessary to properly shutdown and reboot the NAS device and
+  provides additional functionality such as led control, fan speed control,
+  a beeper and buttons on the NAS device.
+
+properties:
+  compatible:
+    enum:
+      - synology,ds923p-microp
+      - synology,ds918p-microp
+      - synology,ds214play-microp
+      - synology,ds225p-microp
+      - synology,ds425p-microp
+      - synology,ds710p-microp
+      - synology,ds1010p-microp
+      - synology,ds723p-microp
+      - synology,ds1522p-microp
+      - synology,rs422p-microp
+      - synology,ds725p-microp
+      - synology,ds118-microp
+      - synology,ds124-microp
+      - synology,ds223-microp
+      - synology,ds223j-microp
+      - synology,ds1823xsp-microp
+      - synology,rs822p-microp
+      - synology,rs1221p-microp
+      - synology,rs1221rpp-microp
+      - synology,ds925p-microp
+      - synology,ds1525p-microp
+      - synology,ds1825p-microp
+
+  fan-failure-gpios:
+    description: GPIOs needed to determine which fans stopped working on a fan failure event.
+    minItems: 2
+    maxItems: 3
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    embedded-controller {
+      compatible = "synology,ds923p-microp";
+
+      fan-failure-gpios = <&gpio 68 GPIO_ACTIVE_HIGH>, <&gpio 69 GPIO_ACTIVE_HIGH>;
+    };

-- 
2.52.0

Re: [PATCH v5 1/4] dt-bindings: embedded-controller: Add synology microp devices

[Krzysztof Kozlowski]

On Sun, Mar 29, 2026 at 08:02:15PM +0200, Markus Probst wrote:
> +properties:
> +  compatible:
> +    enum:
> +      - synology,ds923p-microp
> +      - synology,ds918p-microp
> +      - synology,ds214play-microp
> +      - synology,ds225p-microp
> +      - synology,ds425p-microp
> +      - synology,ds710p-microp
> +      - synology,ds1010p-microp
> +      - synology,ds723p-microp
> +      - synology,ds1522p-microp
> +      - synology,rs422p-microp
> +      - synology,ds725p-microp
> +      - synology,ds118-microp
> +      - synology,ds124-microp
> +      - synology,ds223-microp
> +      - synology,ds223j-microp
> +      - synology,ds1823xsp-microp
> +      - synology,rs822p-microp
> +      - synology,rs1221p-microp
> +      - synology,rs1221rpp-microp
> +      - synology,ds925p-microp
> +      - synology,ds1525p-microp
> +      - synology,ds1825p-microp

Last time you had one compatible and implied they are all compatible.
Now none of them are compatible, which might be accurate, but nothing
explains WHY they are not compatible in the commit msg.

> +
> +  fan-failure-gpios:
> +    description: GPIOs needed to determine which fans stopped working on a fan failure event.
> +    minItems: 2
> +    maxItems: 3

Constraints cannot be flexible. You need allOf:if:then: block to narrow
them per variant.

Best regards,
Krzysztof

Re: [PATCH v5 1/4] dt-bindings: embedded-controller: Add synology microp devices

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]

On Mon, 2026-03-30 at 08:51 +0200, Krzysztof Kozlowski wrote:
> On Sun, Mar 29, 2026 at 08:02:15PM +0200, Markus Probst wrote:
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - synology,ds923p-microp
> > +      - synology,ds918p-microp
> > +      - synology,ds214play-microp
> > +      - synology,ds225p-microp
> > +      - synology,ds425p-microp
> > +      - synology,ds710p-microp
> > +      - synology,ds1010p-microp
> > +      - synology,ds723p-microp
> > +      - synology,ds1522p-microp
> > +      - synology,rs422p-microp
> > +      - synology,ds725p-microp
> > +      - synology,ds118-microp
> > +      - synology,ds124-microp
> > +      - synology,ds223-microp
> > +      - synology,ds223j-microp
> > +      - synology,ds1823xsp-microp
> > +      - synology,rs822p-microp
> > +      - synology,rs1221p-microp
> > +      - synology,rs1221rpp-microp
> > +      - synology,ds925p-microp
> > +      - synology,ds1525p-microp
> > +      - synology,ds1825p-microp
> 
> Last time you had one compatible and implied they are all compatible.
> Now none of them are compatible, which might be accurate,

As you mentioned earlier:

Unless exactly same board is used
in different models (unlikely) then the compatible defines the LEDs and
they are not needed in DT.

> 
> but nothing
> explains WHY they are not compatible in the commit msg.
Shall all 22 compatible be in the commit msg?

Also, might be worth documenting this requirement [1].

> 
> > +
> > +  fan-failure-gpios:
> > +    description: GPIOs needed to determine which fans stopped working on a fan failure event.
> > +    minItems: 2
> > +    maxItems: 3
> 
> Constraints cannot be flexible. You need allOf:if:then: block to narrow
> them per variant.
I can disable the property or force the property based on compatible
with it.
But it seems it won't let me modify the minItems and maxItems
constraint.

Thanks
- Markus Probst


[1] https://docs.kernel.org/devicetree/bindings/submitting-patches.html


> 
> Best regards,
> Krzysztof

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

[PATCH v5 2/4] ACPI: of: match PRP0001 in of_match_device

[Markus Probst via B4 Relay]

From: Markus Probst <markus.probst@posteo.de>

Export `acpi_of_match_device` function and use it to match for PRP0001
in `of_match_device`, if the device does not have a device node.

This fixes the match data being NULL when using ACPI PRP0001, even though
the device was matched against an of device table.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 drivers/acpi/bus.c   |  7 ++++---
 drivers/of/device.c  |  9 +++++++--
 include/linux/acpi.h | 11 +++++++++++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 2ec095e2009e..cd02f04cf685 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -831,9 +831,9 @@ const struct acpi_device *acpi_companion_match(const struct device *dev)
  * identifiers and a _DSD object with the "compatible" property, use that
  * property to match against the given list of identifiers.
  */
-static bool acpi_of_match_device(const struct acpi_device *adev,
-				 const struct of_device_id *of_match_table,
-				 const struct of_device_id **of_id)
+bool acpi_of_match_device(const struct acpi_device *adev,
+			  const struct of_device_id *of_match_table,
+			  const struct of_device_id **of_id)
 {
 	const union acpi_object *of_compatible, *obj;
 	int i, nval;
@@ -866,6 +866,7 @@ static bool acpi_of_match_device(const struct acpi_device *adev,
 
 	return false;
 }
+EXPORT_SYMBOL_GPL(acpi_of_match_device);
 
 static bool acpi_of_modalias(struct acpi_device *adev,
 			     char *modalias, size_t len)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index f7e75e527667..128682390058 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -11,6 +11,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
+#include <linux/acpi.h>
 
 #include <asm/errno.h>
 #include "of_private.h"
@@ -26,8 +27,12 @@
 const struct of_device_id *of_match_device(const struct of_device_id *matches,
 					   const struct device *dev)
 {
-	if (!matches || !dev->of_node || dev->of_node_reused)
-		return NULL;
+	if (!matches || !dev->of_node || dev->of_node_reused) {
+		const struct of_device_id *id = NULL;
+
+		acpi_of_match_device(ACPI_COMPANION(dev), matches, &id);
+		return id;
+	}
 	return of_match_node(matches, dev->of_node);
 }
 EXPORT_SYMBOL(of_match_device);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4d2f0bed7a06..1cf23edcbfbb 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -736,6 +736,10 @@ const struct acpi_device_id *acpi_match_acpi_device(const struct acpi_device_id
 const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids,
 					       const struct device *dev);
 
+bool acpi_of_match_device(const struct acpi_device *adev,
+			  const struct of_device_id *of_match_table,
+			  const struct of_device_id **of_id);
+
 const void *acpi_device_get_match_data(const struct device *dev);
 extern bool acpi_driver_match_device(struct device *dev,
 				     const struct device_driver *drv);
@@ -965,6 +969,13 @@ static inline const struct acpi_device_id *acpi_match_device(
 	return NULL;
 }
 
+static inline bool acpi_of_match_device(const struct acpi_device *adev,
+					const struct of_device_id *of_match_table,
+					const struct of_device_id **of_id)
+{
+	return false;
+}
+
 static inline const void *acpi_device_get_match_data(const struct device *dev)
 {
 	return NULL;

-- 
2.52.0

Re: [PATCH v5 2/4] ACPI: of: match PRP0001 in of_match_device

[Krzysztof Kozlowski]

On Sun, Mar 29, 2026 at 08:02:16PM +0200, Markus Probst wrote:
> Export `acpi_of_match_device` function and use it to match for PRP0001
> in `of_match_device`, if the device does not have a device node.
> 
> This fixes the match data being NULL when using ACPI PRP0001, even though
> the device was matched against an of device table.

Fixes tag?

I don't see how this is going to fix !ACPI case - the
acpi_of_match_device() will just return false.


> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  drivers/acpi/bus.c   |  7 ++++---
>  drivers/of/device.c  |  9 +++++++--
>  include/linux/acpi.h | 11 +++++++++++
>  3 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 2ec095e2009e..cd02f04cf685 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -831,9 +831,9 @@ const struct acpi_device *acpi_companion_match(const struct device *dev)
>   * identifiers and a _DSD object with the "compatible" property, use that
>   * property to match against the given list of identifiers.
>   */
> -static bool acpi_of_match_device(const struct acpi_device *adev,
> -				 const struct of_device_id *of_match_table,
> -				 const struct of_device_id **of_id)
> +bool acpi_of_match_device(const struct acpi_device *adev,
> +			  const struct of_device_id *of_match_table,
> +			  const struct of_device_id **of_id)
>  {
>  	const union acpi_object *of_compatible, *obj;
>  	int i, nval;
> @@ -866,6 +866,7 @@ static bool acpi_of_match_device(const struct acpi_device *adev,
>  
>  	return false;
>  }
> +EXPORT_SYMBOL_GPL(acpi_of_match_device);
>  
>  static bool acpi_of_modalias(struct acpi_device *adev,
>  			     char *modalias, size_t len)
> diff --git a/drivers/of/device.c b/drivers/of/device.c
> index f7e75e527667..128682390058 100644
> --- a/drivers/of/device.c
> +++ b/drivers/of/device.c
> @@ -11,6 +11,7 @@
>  #include <linux/mod_devicetable.h>
>  #include <linux/slab.h>
>  #include <linux/platform_device.h>
> +#include <linux/acpi.h>
>  
>  #include <asm/errno.h>
>  #include "of_private.h"
> @@ -26,8 +27,12 @@
>  const struct of_device_id *of_match_device(const struct of_device_id *matches,
>  					   const struct device *dev)
>  {
> -	if (!matches || !dev->of_node || dev->of_node_reused)
> -		return NULL;
> +	if (!matches || !dev->of_node || dev->of_node_reused) {
> +		const struct of_device_id *id = NULL;
> +
> +		acpi_of_match_device(ACPI_COMPANION(dev), matches, &id);

I don't think this should be done from of_match_device. Yuo will have
soon recursive calls, because acpi_of_match_device() will call other
match, that will call of_match_device() and so on...

of_match_device() is supposed to match only against OF. Not ACPI. There
should be no ACPI header or code in this unit file.

Best regards,
Krzysztof

Re: [PATCH v5 2/4] ACPI: of: match PRP0001 in of_match_device

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 3606 bytes --]

On Mon, 2026-03-30 at 09:00 +0200, Krzysztof Kozlowski wrote:
> On Sun, Mar 29, 2026 at 08:02:16PM +0200, Markus Probst wrote:
> > Export `acpi_of_match_device` function and use it to match for PRP0001
> > in `of_match_device`, if the device does not have a device node.
> > 
> > This fixes the match data being NULL when using ACPI PRP0001, even though
> > the device was matched against an of device table.
> 
> Fixes tag?
> 
> I don't see how this is going to fix !ACPI case - the
> acpi_of_match_device() will just return false.
While trying to argue I found out that there already is
`device_get_match_data`, which takes PRP0001 into account.

I will now instead make a patch, which will make rust use this function
instead of calling `of_match_device` and `acpi_match_device`
individually, which ignores PRP0001.

There are still a lot of drivers only using `of_match_device`, which
makes it impossible to use PRP0001 with them. But this is not relevant
for this driver.

Thanks
- Markus Probst

> 
> 
> > 
> > Signed-off-by: Markus Probst <markus.probst@posteo.de>
> > ---
> >  drivers/acpi/bus.c   |  7 ++++---
> >  drivers/of/device.c  |  9 +++++++--
> >  include/linux/acpi.h | 11 +++++++++++
> >  3 files changed, 22 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 2ec095e2009e..cd02f04cf685 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -831,9 +831,9 @@ const struct acpi_device *acpi_companion_match(const struct device *dev)
> >   * identifiers and a _DSD object with the "compatible" property, use that
> >   * property to match against the given list of identifiers.
> >   */
> > -static bool acpi_of_match_device(const struct acpi_device *adev,
> > -				 const struct of_device_id *of_match_table,
> > -				 const struct of_device_id **of_id)
> > +bool acpi_of_match_device(const struct acpi_device *adev,
> > +			  const struct of_device_id *of_match_table,
> > +			  const struct of_device_id **of_id)
> >  {
> >  	const union acpi_object *of_compatible, *obj;
> >  	int i, nval;
> > @@ -866,6 +866,7 @@ static bool acpi_of_match_device(const struct acpi_device *adev,
> >  
> >  	return false;
> >  }
> > +EXPORT_SYMBOL_GPL(acpi_of_match_device);
> >  
> >  static bool acpi_of_modalias(struct acpi_device *adev,
> >  			     char *modalias, size_t len)
> > diff --git a/drivers/of/device.c b/drivers/of/device.c
> > index f7e75e527667..128682390058 100644
> > --- a/drivers/of/device.c
> > +++ b/drivers/of/device.c
> > @@ -11,6 +11,7 @@
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/slab.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/acpi.h>
> >  
> >  #include <asm/errno.h>
> >  #include "of_private.h"
> > @@ -26,8 +27,12 @@
> >  const struct of_device_id *of_match_device(const struct of_device_id *matches,
> >  					   const struct device *dev)
> >  {
> > -	if (!matches || !dev->of_node || dev->of_node_reused)
> > -		return NULL;
> > +	if (!matches || !dev->of_node || dev->of_node_reused) {
> > +		const struct of_device_id *id = NULL;
> > +
> > +		acpi_of_match_device(ACPI_COMPANION(dev), matches, &id);
> 
> I don't think this should be done from of_match_device. Yuo will have
> soon recursive calls, because acpi_of_match_device() will call other
> match, that will call of_match_device() and so on...
> 
> of_match_device() is supposed to match only against OF. Not ACPI. There
> should be no ACPI header or code in this unit file.
> 
> Best regards,
> Krzysztof

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Re: [PATCH v5 2/4] ACPI: of: match PRP0001 in of_match_device

[Rob Herring]

On Mon, Mar 30, 2026 at 07:04:21PM +0000, Markus Probst wrote:
> On Mon, 2026-03-30 at 09:00 +0200, Krzysztof Kozlowski wrote:
> > On Sun, Mar 29, 2026 at 08:02:16PM +0200, Markus Probst wrote:
> > > Export `acpi_of_match_device` function and use it to match for PRP0001
> > > in `of_match_device`, if the device does not have a device node.
> > > 
> > > This fixes the match data being NULL when using ACPI PRP0001, even though
> > > the device was matched against an of device table.
> > 
> > Fixes tag?
> > 
> > I don't see how this is going to fix !ACPI case - the
> > acpi_of_match_device() will just return false.
> While trying to argue I found out that there already is
> `device_get_match_data`, which takes PRP0001 into account.
> 
> I will now instead make a patch, which will make rust use this function
> instead of calling `of_match_device` and `acpi_match_device`
> individually, which ignores PRP0001.

IIRC, the rust binding already gives you the data pointer in probe.

> There are still a lot of drivers only using `of_match_device`, which
> makes it impossible to use PRP0001 with them. But this is not relevant
> for this driver.

Usually using of_match_device() in drivers is wrong. You generally just 
want the data pointer. There's a whole bunch of drivers still doing the 
old way.

Rob

Re: [PATCH v5 2/4] ACPI: of: match PRP0001 in of_match_device

[Markus Probst]

[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]

On Mon, 2026-03-30 at 14:22 -0500, Rob Herring wrote:
> On Mon, Mar 30, 2026 at 07:04:21PM +0000, Markus Probst wrote:
> > On Mon, 2026-03-30 at 09:00 +0200, Krzysztof Kozlowski wrote:
> > > On Sun, Mar 29, 2026 at 08:02:16PM +0200, Markus Probst wrote:
> > > > Export `acpi_of_match_device` function and use it to match for PRP0001
> > > > in `of_match_device`, if the device does not have a device node.
> > > > 
> > > > This fixes the match data being NULL when using ACPI PRP0001, even though
> > > > the device was matched against an of device table.
> > > 
> > > Fixes tag?
> > > 
> > > I don't see how this is going to fix !ACPI case - the
> > > acpi_of_match_device() will just return false.
> > While trying to argue I found out that there already is
> > `device_get_match_data`, which takes PRP0001 into account.
> > 
> > I will now instead make a patch, which will make rust use this function
> > instead of calling `of_match_device` and `acpi_match_device`
> > individually, which ignores PRP0001.
> 
> IIRC, the rust binding already gives you the data pointer in probe.
Yes, but that pointer is obtained by calling `acpi_match_device` and
`of_match_device` [1].

Thanks
- Markus Probst

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/driver.rs?id=7aaa8047eafd0bd628065b15757d9b48c5f9c07d

> 
> > There are still a lot of drivers only using `of_match_device`, which
> > makes it impossible to use PRP0001 with them. But this is not relevant
> > for this driver.
> 
> Usually using of_match_device() in drivers is wrong. You generally just 
> want the data pointer. There's a whole bunch of drivers still doing the 
> old way.
> 
> Rob

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

[PATCH v5 3/4] rust: add visibility to of_device_table macro

[Markus Probst via B4 Relay]

From: Markus Probst <markus.probst@posteo.de>

Add visibility argument to macro to allow defining an of device table in
a different module than the driver, which can improve readability.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/of.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 58b20c367f99..a0de98a23416 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -53,8 +53,9 @@ pub const fn new(compatible: &'static CStr) -> Self {
 /// Create an OF `IdTable` with an "alias" for modpost.
 #[macro_export]
 macro_rules! of_device_table {
-    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
-        const $table_name: $crate::device_id::IdArray<
+    ($table_vis:vis $table_name:ident, $module_table_name:ident, $id_info_type: ty,
+        $table_data: expr) => {
+        $table_vis const $table_name: $crate::device_id::IdArray<
             $crate::of::DeviceId,
             $id_info_type,
             { $table_data.len() },

-- 
2.52.0

Re: [PATCH v5 3/4] rust: add visibility to of_device_table macro

[Krzysztof Kozlowski]

On Sun, Mar 29, 2026 at 08:02:17PM +0200, Markus Probst wrote:
> Add visibility argument to macro to allow defining an of device table in
> a different module than the driver, which can improve readability.

No. The of_device_id table must be always placed next to the foo_driver.
You keep introducing completely inconsisting styles in this patchset,
like completely broken usage of compatibles in the driver.

Best regards,
Krzysztof

[PATCH v5 4/4] platform: Add initial synology microp driver

[Markus Probst via B4 Relay]

From: Markus Probst <markus.probst@posteo.de>

Add a initial synology microp driver, written in Rust.
The driver targets a microcontroller found in Synology NAS devices. It
currently only supports controlling of the power led, status led, alert
led and usb led. Other components such as fan control or handling
on-device buttons will be added once the required rust abstractions are
there.

This driver can be used both on arm and x86, thus it goes into the root
directory of drivers/platform.

Tested successfully on a Synology DS923+.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 MAINTAINERS                                        |   6 +
 drivers/platform/Kconfig                           |   2 +
 drivers/platform/Makefile                          |   1 +
 drivers/platform/synology_microp/Kconfig           |  13 +
 drivers/platform/synology_microp/Makefile          |   3 +
 drivers/platform/synology_microp/TODO              |   7 +
 drivers/platform/synology_microp/command.rs        |  55 ++++
 drivers/platform/synology_microp/led.rs            | 276 +++++++++++++++++++++
 drivers/platform/synology_microp/model.rs          | 171 +++++++++++++
 .../platform/synology_microp/synology_microp.rs    |  54 ++++
 10 files changed, 588 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 83b5a45de729..24cc4f63cce6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25544,6 +25544,12 @@ F:	drivers/dma-buf/sync_*
 F:	include/linux/sync_file.h
 F:	include/uapi/linux/sync_file.h
 
+SYNOLOGY MICROP DRIVER
+M:	Markus Probst <markus.probst@posteo.de>
+S:	Maintained
+F:	Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml
+F:	drivers/platform/synology_microp/
+
 SYNOPSYS ARC ARCHITECTURE
 M:	Vineet Gupta <vgupta@kernel.org>
 L:	linux-snps-arc@lists.infradead.org
diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index 312788f249c9..996050566a4a 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -22,3 +22,5 @@ source "drivers/platform/arm64/Kconfig"
 source "drivers/platform/raspberrypi/Kconfig"
 
 source "drivers/platform/wmi/Kconfig"
+
+source "drivers/platform/synology_microp/Kconfig"
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index fa322e7f8716..2381872e9133 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_SURFACE_PLATFORMS)	+= surface/
 obj-$(CONFIG_ARM64_PLATFORM_DEVICES)	+= arm64/
 obj-$(CONFIG_BCM2835_VCHIQ)	+= raspberrypi/
 obj-$(CONFIG_ACPI_WMI)		+= wmi/
+obj-$(CONFIG_SYNOLOGY_MICROP)	+= synology_microp/
diff --git a/drivers/platform/synology_microp/Kconfig b/drivers/platform/synology_microp/Kconfig
new file mode 100644
index 000000000000..0c145a5b7174
--- /dev/null
+++ b/drivers/platform/synology_microp/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config SYNOLOGY_MICROP
+	tristate "Synology Microp driver"
+	depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
+	depends on RUST_SERIAL_DEV_BUS_ABSTRACTIONS
+	help
+	  Enable support for the MCU found in Synology NAS devices.
+
+	  This is needed to properly shutdown and reboot the device, as well as
+	  additional functionality like fan and LED control.
+
+	  This driver is work in progress and may not be fully functional.
diff --git a/drivers/platform/synology_microp/Makefile b/drivers/platform/synology_microp/Makefile
new file mode 100644
index 000000000000..63585ccf76e4
--- /dev/null
+++ b/drivers/platform/synology_microp/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-y += synology_microp.o
diff --git a/drivers/platform/synology_microp/TODO b/drivers/platform/synology_microp/TODO
new file mode 100644
index 000000000000..1961a33115db
--- /dev/null
+++ b/drivers/platform/synology_microp/TODO
@@ -0,0 +1,7 @@
+TODO:
+- add missing components:
+  - handle on-device buttons (Power, Factory reset, "USB Copy")
+  - handle fan failure
+  - beeper
+  - fan speed control
+  - correctly perform device power-off and restart on Synology devices
diff --git a/drivers/platform/synology_microp/command.rs b/drivers/platform/synology_microp/command.rs
new file mode 100644
index 000000000000..5b3dd715afac
--- /dev/null
+++ b/drivers/platform/synology_microp/command.rs
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+    device::Bound,
+    error::Result,
+    serdev, //
+};
+
+use crate::led;
+
+#[derive(Copy, Clone)]
+#[expect(
+    clippy::enum_variant_names,
+    reason = "future variants will not end with Led"
+)]
+pub(crate) enum Command {
+    PowerLed(led::State),
+    StatusLed(led::StatusLedColor, led::State),
+    AlertLed(led::State),
+    UsbLed(led::State),
+    EsataLed(led::State),
+}
+
+impl Command {
+    pub(crate) fn write(self, dev: &serdev::Device<Bound>) -> Result {
+        dev.write_all(
+            match self {
+                Self::PowerLed(led::State::On) => &[0x34],
+                Self::PowerLed(led::State::Blink) => &[0x35],
+                Self::PowerLed(led::State::Off) => &[0x36],
+
+                Self::StatusLed(_, led::State::Off) => &[0x37],
+                Self::StatusLed(led::StatusLedColor::Green, led::State::On) => &[0x38],
+                Self::StatusLed(led::StatusLedColor::Green, led::State::Blink) => &[0x39],
+                Self::StatusLed(led::StatusLedColor::Orange, led::State::On) => &[0x3A],
+                Self::StatusLed(led::StatusLedColor::Orange, led::State::Blink) => &[0x3B],
+
+                Self::AlertLed(led::State::On) => &[0x4C, 0x41, 0x31],
+                Self::AlertLed(led::State::Blink) => &[0x4C, 0x41, 0x32],
+                Self::AlertLed(led::State::Off) => &[0x4C, 0x41, 0x33],
+
+                Self::UsbLed(led::State::On) => &[0x40],
+                Self::UsbLed(led::State::Blink) => &[0x41],
+                Self::UsbLed(led::State::Off) => &[0x42],
+
+                Self::EsataLed(led::State::On) => &[0x4C, 0x45, 0x31],
+                Self::EsataLed(led::State::Blink) => &[0x4C, 0x45, 0x32],
+                Self::EsataLed(led::State::Off) => &[0x4C, 0x45, 0x33],
+            },
+            serdev::Timeout::Max,
+        )?;
+        dev.wait_until_sent(serdev::Timeout::Max);
+        Ok(())
+    }
+}
diff --git a/drivers/platform/synology_microp/led.rs b/drivers/platform/synology_microp/led.rs
new file mode 100644
index 000000000000..a78a95588456
--- /dev/null
+++ b/drivers/platform/synology_microp/led.rs
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+    device::Bound,
+    devres::{
+        self,
+        Devres, //
+    },
+    led::{
+        self,
+        LedOps,
+        MultiColorSubLed, //
+    },
+    new_mutex,
+    prelude::*,
+    serdev,
+    str::CString,
+    sync::Mutex, //
+};
+use pin_init::pin_init_scope;
+
+use crate::{
+    command::Command,
+    model::Model, //
+};
+
+#[pin_data]
+pub(crate) struct Data {
+    #[pin]
+    status: Devres<led::MultiColorDevice<StatusLedHandler>>,
+    power_name: CString,
+    #[pin]
+    power: Devres<led::Device<LedHandler>>,
+}
+
+impl Data {
+    pub(super) fn register<'a>(
+        dev: &'a serdev::Device<Bound>,
+        model: &'a Model,
+    ) -> impl PinInit<Self, Error> + 'a {
+        pin_init_scope(move || {
+            if let Some(color) = model.led_alert {
+                let name = CString::try_from_fmt(fmt!("{}:alarm", color.as_c_str().to_str()?))?;
+                devres::register(
+                    dev.as_ref(),
+                    led::DeviceBuilder::new().color(color).name(&name).build(
+                        dev,
+                        try_pin_init!(LedHandler {
+                            blink <- new_mutex!(false),
+                            command: Command::AlertLed,
+                        }),
+                    ),
+                    GFP_KERNEL,
+                )?;
+            }
+
+            if model.led_usb_copy {
+                devres::register(
+                    dev.as_ref(),
+                    led::DeviceBuilder::new()
+                        .color(led::Color::Green)
+                        .name(c"green:usb")
+                        .build(
+                            dev,
+                            try_pin_init!(LedHandler {
+                                blink <- new_mutex!(false),
+                                command: Command::UsbLed,
+                            }),
+                        ),
+                    GFP_KERNEL,
+                )?;
+            }
+
+            if model.led_esata {
+                devres::register(
+                    dev.as_ref(),
+                    led::DeviceBuilder::new()
+                        .color(led::Color::Green)
+                        .name(c"green:esata")
+                        .build(
+                            dev,
+                            try_pin_init!(LedHandler {
+                                blink <- new_mutex!(false),
+                                command: Command::EsataLed,
+                            }),
+                        ),
+                    GFP_KERNEL,
+                )?;
+            }
+
+            Ok(try_pin_init!(Self {
+                status <- led::DeviceBuilder::new()
+                    .color(led::Color::Multi)
+                    .name(c"multicolor:status")
+                    .build_multicolor(
+                        dev,
+                        try_pin_init!(StatusLedHandler {
+                            blink <- new_mutex!(false),
+                        }),
+                        StatusLedHandler::SUBLEDS,
+                    ),
+                power_name: CString::try_from_fmt(fmt!(
+                    "{}:power",
+                    model.led_power.as_c_str().to_str()?
+                ))?,
+                power <- led::DeviceBuilder::new()
+                    .color(model.led_power)
+                    .name(power_name)
+                    .build(
+                        dev,
+                        try_pin_init!(LedHandler {
+                            blink <- new_mutex!(true),
+                            command: Command::PowerLed,
+                        }),
+                    ),
+            }))
+        })
+    }
+}
+
+#[derive(Copy, Clone)]
+pub(crate) enum StatusLedColor {
+    Green,
+    Orange,
+}
+
+#[derive(Copy, Clone)]
+pub(crate) enum State {
+    On,
+    Blink,
+    Off,
+}
+
+#[pin_data]
+struct LedHandler {
+    #[pin]
+    blink: Mutex<bool>,
+    command: fn(State) -> Command,
+}
+
+#[vtable]
+impl LedOps for LedHandler {
+    type Bus = serdev::Device<Bound>;
+    type Mode = led::Normal;
+    const BLOCKING: bool = true;
+    const MAX_BRIGHTNESS: u32 = 1;
+
+    fn brightness_set(
+        &self,
+        dev: &Self::Bus,
+        _classdev: &led::Device<Self>,
+        brightness: u32,
+    ) -> Result<()> {
+        let mut blink = self.blink.lock();
+        (self.command)(if brightness == 0 {
+            *blink = false;
+            State::Off
+        } else if *blink {
+            State::Blink
+        } else {
+            State::On
+        })
+        .write(dev)?;
+
+        Ok(())
+    }
+
+    fn blink_set(
+        &self,
+        dev: &Self::Bus,
+        _classdev: &led::Device<Self>,
+        delay_on: &mut usize,
+        delay_off: &mut usize,
+    ) -> Result<()> {
+        let mut blink = self.blink.lock();
+
+        (self.command)(if *delay_on == 0 && *delay_off != 0 {
+            State::Off
+        } else if *delay_on != 0 && *delay_off == 0 {
+            State::On
+        } else {
+            *blink = true;
+            *delay_on = 167;
+            *delay_off = 167;
+
+            State::Blink
+        })
+        .write(dev)
+    }
+}
+
+#[pin_data]
+struct StatusLedHandler {
+    #[pin]
+    blink: Mutex<bool>,
+}
+
+impl StatusLedHandler {
+    const SUBLEDS: &[MultiColorSubLed] = &[
+        MultiColorSubLed::new(led::Color::Green).initial_intensity(1),
+        MultiColorSubLed::new(led::Color::Orange),
+    ];
+}
+
+#[vtable]
+impl LedOps for StatusLedHandler {
+    type Bus = serdev::Device<Bound>;
+    type Mode = led::MultiColor;
+    const BLOCKING: bool = true;
+    const MAX_BRIGHTNESS: u32 = 1;
+
+    fn brightness_set(
+        &self,
+        dev: &Self::Bus,
+        classdev: &led::MultiColorDevice<Self>,
+        brightness: u32,
+    ) -> Result<()> {
+        let mut blink = self.blink.lock();
+        if brightness == 0 {
+            *blink = false;
+        }
+
+        let (color, subled_brightness) = if classdev.subleds()[1].intensity == 0 {
+            (StatusLedColor::Green, classdev.subleds()[0].brightness)
+        } else {
+            (StatusLedColor::Orange, classdev.subleds()[1].brightness)
+        };
+
+        Command::StatusLed(
+            color,
+            if subled_brightness == 0 {
+                State::Off
+            } else if *blink {
+                State::Blink
+            } else {
+                State::On
+            },
+        )
+        .write(dev)
+    }
+
+    fn blink_set(
+        &self,
+        dev: &Self::Bus,
+        classdev: &led::MultiColorDevice<Self>,
+        delay_on: &mut usize,
+        delay_off: &mut usize,
+    ) -> Result<()> {
+        let mut blink = self.blink.lock();
+        *blink = true;
+
+        let (color, subled_intensity) = if classdev.subleds()[1].intensity == 0 {
+            (StatusLedColor::Green, classdev.subleds()[0].intensity)
+        } else {
+            (StatusLedColor::Orange, classdev.subleds()[1].intensity)
+        };
+        Command::StatusLed(
+            color,
+            if *delay_on == 0 && *delay_off != 0 {
+                *blink = false;
+                State::Off
+            } else if subled_intensity == 0 {
+                State::Off
+            } else if *delay_on != 0 && *delay_off == 0 {
+                *blink = false;
+                State::On
+            } else {
+                *delay_on = 167;
+                *delay_off = 167;
+
+                State::Blink
+            },
+        )
+        .write(dev)
+    }
+}
diff --git a/drivers/platform/synology_microp/model.rs b/drivers/platform/synology_microp/model.rs
new file mode 100644
index 000000000000..b972aae2b805
--- /dev/null
+++ b/drivers/platform/synology_microp/model.rs
@@ -0,0 +1,171 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+    led::{
+        self,
+        Color, //
+    },
+    of::DeviceId, //
+};
+
+pub(crate) struct Architecture;
+
+impl Architecture {
+    const fn new() -> Self {
+        Self
+    }
+}
+
+pub(crate) struct Model {
+    #[expect(
+        dead_code,
+        reason = "needed later for architecture specific properties, like poweroff behaviour"
+    )]
+    pub(crate) arch: Architecture,
+    pub(crate) led_power: led::Color,
+    pub(crate) led_alert: Option<led::Color>,
+    pub(crate) led_usb_copy: bool,
+    pub(crate) led_esata: bool,
+}
+
+impl Model {
+    const fn new(arch: Architecture) -> Self {
+        Self {
+            arch,
+            led_power: led::Color::Blue,
+            led_alert: None,
+            led_usb_copy: false,
+            led_esata: false,
+        }
+    }
+
+    const fn led_power(self, color: led::Color) -> Self {
+        Self {
+            led_power: color,
+            ..self
+        }
+    }
+
+    const fn led_alert(self, color: led::Color) -> Self {
+        Self {
+            led_alert: Some(color),
+            ..self
+        }
+    }
+
+    const fn led_esata(self) -> Self {
+        Self {
+            led_esata: true,
+            ..self
+        }
+    }
+
+    const fn led_usb_copy(self) -> Self {
+        Self {
+            led_usb_copy: true,
+            ..self
+        }
+    }
+}
+
+macro_rules! models {
+    [
+        $($arch:ident $(.$arch_func:ident( $($arch_arg:tt)* ))*
+            @ [
+                $($model:ident $(.$func:ident( $($arg:tt)* ))*, )*
+            ],
+        )*
+    ] => {
+        models![
+            $(
+                {
+                    Architecture::new()
+                    $(
+                        .$arch_func($($arch_arg)*)
+                    )*
+                }
+                @
+                [
+                    $(
+                        $model $(.$func($($arg)*))*,
+                    )*
+                ],
+            )*
+        ]
+    };
+    [
+        $($arch:block
+            @ [
+                $($model:ident $(.$func:ident( $($arg:tt)* ))*, )*
+            ],
+        )*
+    ] => {
+        [
+            $(
+                $((
+                    DeviceId::new(::kernel::c_str!(
+                        ::core::concat!(
+                            "synology,",
+                            ::core::stringify!($model),
+                            "-microp",
+                        )
+                    )),
+                    Model::new($arch)
+                    $(
+                        .$func($($arg)*)
+                    )*
+                ),)*
+            )*
+        ]
+    };
+}
+
+kernel::of_device_table!(
+    pub(crate) OF_TABLE,
+    MODULE_OF_TABLE,
+    Model,
+    models![
+        apollolake @ [
+            ds918p,
+        ],
+        evansport @ [
+            ds214play,
+        ],
+        geminilakenk @ [
+            ds225p.led_usb_copy(),
+            ds425p,
+        ],
+        pineview @ [
+            ds710p.led_esata(),
+            ds1010p.led_alert(Color::Orange),
+        ],
+        r1000 @ [
+            ds923p,
+            ds723p,
+            ds1522p,
+            rs422p.led_power(Color::Green),
+        ],
+        r1000nk @ [
+            ds725p,
+        ],
+        rtd1296 @ [
+            ds118,
+        ],
+        rtd1619b @ [
+            ds124,
+            ds223.led_usb_copy(),
+            ds223j,
+        ],
+        v1000 @ [
+            ds1823xsp,
+            rs822p.led_power(Color::Green),
+            rs1221p.led_power(Color::Green),
+            rs1221rpp.led_power(Color::Green),
+        ],
+        v1000nk @ [
+            ds925p,
+            ds1525p,
+            ds1825p,
+        ],
+    ]
+);
diff --git a/drivers/platform/synology_microp/synology_microp.rs b/drivers/platform/synology_microp/synology_microp.rs
new file mode 100644
index 000000000000..51152cc14b1e
--- /dev/null
+++ b/drivers/platform/synology_microp/synology_microp.rs
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Synology Microp driver
+
+use kernel::{
+    device,
+    of,
+    prelude::*,
+    serdev, //
+};
+use pin_init::pin_init_scope;
+
+use crate::model::Model;
+
+pub(crate) mod command;
+mod led;
+mod model;
+
+kernel::module_serdev_device_driver! {
+    type: SynologyMicropDriver,
+    name: "synology_microp",
+    authors: ["Markus Probst <markus.probst@posteo.de>"],
+    description: "Synology Microp driver",
+    license: "GPL v2",
+}
+
+#[pin_data]
+struct SynologyMicropDriver {
+    #[pin]
+    led: led::Data,
+}
+
+#[vtable]
+impl serdev::Driver for SynologyMicropDriver {
+    type IdInfo = Model;
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&model::OF_TABLE);
+
+    fn probe(
+        dev: &serdev::Device<device::Core>,
+        model: Option<&Model>,
+    ) -> impl PinInit<Self, kernel::error::Error> {
+        pin_init_scope(move || {
+            let model = model.ok_or(EINVAL)?;
+
+            dev.set_baudrate(9600).map_err(|_| EINVAL)?;
+            dev.set_flow_control(false);
+            dev.set_parity(serdev::Parity::None)?;
+
+            Ok(try_pin_init!(Self {
+                led <- led::Data::register(dev, model),
+            }))
+        })
+    }
+}

-- 
2.52.0

Re: [PATCH v5 4/4] platform: Add initial synology microp driver

[Krzysztof Kozlowski]

On 29/03/2026 20:02, Markus Probst via B4 Relay wrote:
> +
> +kernel::of_device_table!(
> +    pub(crate) OF_TABLE,
> +    MODULE_OF_TABLE,
> +    Model,
> +    models![
> +        apollolake @ [
> +            ds918p,
> +        ],
> +        evansport @ [
> +            ds214play,
> +        ],
> +        geminilakenk @ [
> +            ds225p.led_usb_copy(),
> +            ds425p,
> +        ],
> +        pineview @ [
> +            ds710p.led_esata(),
> +            ds1010p.led_alert(Color::Orange),
> +        ],
> +        r1000 @ [
> +            ds923p,
> +            ds723p,
> +            ds1522p,
> +            rs422p.led_power(Color::Green),
> +        ],
> +        r1000nk @ [
> +            ds725p,
> +        ],
> +        rtd1296 @ [
> +            ds118,
> +        ],
> +        rtd1619b @ [
> +            ds124,
> +            ds223.led_usb_copy(),
> +            ds223j,
> +        ],
> +        v1000 @ [
> +            ds1823xsp,
> +            rs822p.led_power(Color::Green),
> +            rs1221p.led_power(Color::Green),
> +            rs1221rpp.led_power(Color::Green),
> +        ],
> +        v1000nk @ [
> +            ds925p,
> +            ds1525p,
> +            ds1825p,
I don't see any compatible strings here. Actually, nowhere in the
driver. If that's how you write Rust drivers then NAK. Compatibles must
be greppable. Not only for humans but also for ABI check.

Best regards,
Krzysztof

Re: [PATCH v5 4/4] platform: Add initial synology microp driver

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3373 bytes --]

On Mon, Mar 30, 2026 at 08:51:14AM +0200, Krzysztof Kozlowski wrote:
> On 29/03/2026 20:02, Markus Probst via B4 Relay wrote:
> > +
> > +kernel::of_device_table!(
> > +    pub(crate) OF_TABLE,
> > +    MODULE_OF_TABLE,
> > +    Model,
> > +    models![
> > +        apollolake @ [
> > +            ds918p,
> > +        ],
> > +        evansport @ [
> > +            ds214play,
> > +        ],
> > +        geminilakenk @ [
> > +            ds225p.led_usb_copy(),
> > +            ds425p,
> > +        ],
> > +        pineview @ [
> > +            ds710p.led_esata(),
> > +            ds1010p.led_alert(Color::Orange),
> > +        ],
> > +        r1000 @ [
> > +            ds923p,
> > +            ds723p,
> > +            ds1522p,
> > +            rs422p.led_power(Color::Green),
> > +        ],
> > +        r1000nk @ [
> > +            ds725p,
> > +        ],
> > +        rtd1296 @ [
> > +            ds118,
> > +        ],
> > +        rtd1619b @ [
> > +            ds124,
> > +            ds223.led_usb_copy(),
> > +            ds223j,
> > +        ],
> > +        v1000 @ [
> > +            ds1823xsp,
> > +            rs822p.led_power(Color::Green),
> > +            rs1221p.led_power(Color::Green),
> > +            rs1221rpp.led_power(Color::Green),
> > +        ],
> > +        v1000nk @ [
> > +            ds925p,
> > +            ds1525p,
> > +            ds1825p,
> I don't see any compatible strings here. Actually, nowhere in the
> driver. If that's how you write Rust drivers then NAK. Compatibles must
> be greppable. Not only for humans but also for ABI check.

The code immediately prior creates a macro, which is called here to
produce these. This macro is barely grokkable to begin with IMO, but
you can see the DeviceID::new() call down there that creates the
compatible using string concatenation.

Definitely on the same page as you about compatibles being greppable.
It's not as if it is difficult to create the list using vim or whatever
code generator llm you wanna use. Probably making the macro was more
effort than writing them out!

+macro_rules! models {
+    [
+        $($arch:ident $(.$arch_func:ident( $($arch_arg:tt)* ))*
+            @ [
+                $($model:ident $(.$func:ident( $($arg:tt)* ))*, )*
+            ],
+        )*
+    ] => {
+        models![
+            $(
+                {
+                    Architecture::new()
+                    $(
+                        .$arch_func($($arch_arg)*)
+                    )*
+                }
+                @
+                [
+                    $(
+                        $model $(.$func($($arg)*))*,
+                    )*
+                ],
+            )*
+        ]
+    };
+    [
+        $($arch:block
+            @ [
+                $($model:ident $(.$func:ident( $($arg:tt)* ))*, )*
+            ],
+        )*
+    ] => {
+        [
+            $(
+                $((
+                    DeviceId::new(::kernel::c_str!(
+                        ::core::concat!(
+                            "synology,",
+                            ::core::stringify!($model),
+                            "-microp",
+                        )
+                    )),
+                    Model::new($arch)
+                    $(
+                        .$func($($arg)*)
+                    )*
+                ),)*
+            )*
+        ]
+    };
+}


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]