Re: [PATCH v4 1/2] dt-bindings: embedded-controller: Add synology,microp device

[Markus Probst <markus.probst@posteo.de>]

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

On Thu, 2026-03-26 at 14:36 -0500, Rob Herring wrote:
> On Thu, Mar 26, 2026 at 8:02 AM Markus Probst <markus.probst@posteo.de> wrote:
> > 
> > On Wed, 2026-03-25 at 17:07 -0500, Rob Herring wrote:
> > > On Sat, Mar 21, 2026 at 01:02:22PM +0000, Markus Probst wrote:
> > > > On Sat, 2026-03-21 at 13:32 +0100, Krzysztof Kozlowski wrote:
> > > > > On 21/03/2026 13:17, Markus Probst wrote:
> > > > > > On Sat, 2026-03-21 at 11:21 +0100, Krzysztof Kozlowski wrote:
> > > > > > > On Fri, Mar 20, 2026 at 11:09:53PM +0100, Markus Probst wrote:
> > > > > > > > +
> > > > > > > > +examples:
> > > > > > > > +  - |
> > > > > > > > +    #include <dt-bindings/leds/common.h>
> > > > > > > > +
> > > > > > > > +    embedded-controller {
> > > > > > > > +      compatible = "synology,microp";
> > > > > > > > +
> > > > > > > > +      power-led {
> > > > > > > > +        color = <LED_COLOR_ID_BLUE>;
> > > > > > > > +        function = LED_FUNCTION_POWER;
> > > > > > > > +      };
> > > > > > > > +
> > > > > > > > +      status-led {
> > > > > > > > +        color = <LED_COLOR_ID_MULTI>;
> > > > > > > > +        function = LED_FUNCTION_STATUS;
> > > > > > > > +      };
> > > > > > > 
> > > > > > > Where are other leds? Binding mentions 4.
> > > > > > > 
> > > > > > Status and Power leds exist on every Synology NAS model I am aware of.
> > > > > > But there are models which have additionally a usb or alert led. The
> > > > > > device nodes for those leds should only be present, if they exist
> > > > > > physically on the device.
> > > > > 
> > > > > Then help me to understand - are these different models?
> > > > Yes, even with different CPU architectures.
> > > > How much the "microp" device differs is not clear, but the
> > > > communication protocol is the same.
> > > > > 
> > > > > EC is not a generic purpose component and is tightly coupled with the
> > > > > actual board it is being present on. Unless exactly same board is used
> > > > > in different models (unlikely) then the compatible defines the LEDs and
> > > > > they are not needed in DT.
> > > > So for instance "synology,ds923p-microp", "synology,ds723p-microp" etc.
> > > > ?
> > > > 
> > > > I can do that, but that would be many.
> > > 
> > > How many is many?
> > Estimated 300.
> 
> Okay, that's a lot and probably safe to say there are not 300
> variations of the EC.
> 
> > As a side note: I only have 1 model I can test the driver with.
> > > 
> > > > Having it generic seems more flexible.
> > > 
> > > Is there firmware for these ECs? If so is it the same or different
> > > firmware for each device? If the former or the functionality is really
> > > trivial, then I'd be more comfortable with 1 or a few compatibles.
> > The firmware is not public and the exact differences between them isn't
> > documented. The communication protocol is the same though.
> > 
> > > 
> > > Generic means you'll need to add quirk properties when there is some
> > > difference the OS needs to handle which we'll reject. So stuck with one
> > > compatible and no way to distinguish different ECs is anything but
> > > flexible.
> > Describing the physical leds that are present on the NAS device are not
> > quirk properties, at least in my definition.
> 
> That's not what I mean. I mean things like this other device needs
> some different timing for power-on/reset or delays between accesses or
> some LED control is inverted or some protocol difference... Could be
> about anything. The key thing is you have specific enough information
> (compatible) to start with that you can handle any issue that comes up
> *without* changing the DT.
> 
> As you said, you only have 1 device. Make the binding specific to that
> 1 device. If the next one that comes along can reuse the binding as
> it, then great. Nothing to do. If it can't, then it gets its own new
> compatible. Strictly speaking we would add a new compatible for each
> device, but it's a judgement call that there aren't going to be
> differences to handle. In this case, there likely aren't 300 versions
> of h/w, the functionality is simple enough, and the functionality is
> entirely optional (just a guess). But that's all really your argument
> to make.
I think I will go with different compatible, but instead of 1 I will
compile a small list of devices, which:
- are most similar to my testing device
- have unique functionality (like having additionally a "cpu fan" or
similar). This way I can implement every functionality in the driver,
which makes it easier to add devices in the future.
- I know are owned by people who like to experiment with their device
(including my device)

Thanks
- Markus Probst

> 
> Rob

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