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

[Conor Dooley]

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: signature.asc
Description: PGP signature