On Fri, Dec 20, 2024 at 07:16:34PM +0100, Maya Matuszczyk wrote: > Excuse the formatting, I've typed this reply from my phone > > pt., 20 gru 2024, 19:05 użytkownik Stephan Gerhold < > stephan.gerhold@xxxxxxxxxx> napisał: > > > On Thu, Dec 19, 2024 at 09:08:18PM +0100, Maya Matuszczyk wrote: > > > This patch adds bindings for the EC firmware running on IT8987 present > > > on most of X1E80100 devices > > > > > > Signed-off-by: Maya Matuszczyk <maccraft123mc@xxxxxxxxx> > > > --- > > > .../bindings/platform/qcom,x1e-it8987-ec.yaml | 52 +++++++++++++++++++ > > > 1 file changed, 52 insertions(+) > > > create mode 100644 > > Documentation/devicetree/bindings/platform/qcom,x1e-it8987-ec.yaml > > > > > > diff --git > > a/Documentation/devicetree/bindings/platform/qcom,x1e-it8987-ec.yaml > > b/Documentation/devicetree/bindings/platform/qcom,x1e-it8987-ec.yaml > > > new file mode 100644 > > > index 000000000000..4a4f6eb63072 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/platform/qcom,x1e-it8987-ec.yaml > > > @@ -0,0 +1,52 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/platform/qcom,x1e-it8987-ec.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Qualcomm Embedded Controller on IT8987 chip. > > > + > > > +maintainers: > > > + - Maya Matuszczyk <maccraft123mc@xxxxxxxxx> > > > + > > > +description: > > > + Most x1e80100 laptops have an EC running on IT8987 MCU chip. The EC > > controls > > > + minor functions, like fans, power LED, and on some laptops it also > > handles > > > + keyboard hotkeys. > > > + > > > +properties: > > > + compatible: > > > + oneOf: > > > + - const: qcom,x1e-it8987-ec > > > > Given that ECs tend to be somewhat device-specific and many vendors > > might have slightly customized the EC firmware(?), I think it would be > > better to disallow using this generic compatible without a more specific > > one. In other words, I would drop this line and just keep the case > > below: > > > I've looked at DSDT of other devices and they look to be compatible with > what's on the devkit, with differences being extra features on magicbook > art 14 and yoga slim 7x. Though this isn't a hill I'm willing to die on. > I think it's fine to keep qcom,x1e-it8987-ec as second compatible. However, without a more specific compatible, there is a risk we have nothing to match on in case device-specific handling becomes necessary in the driver at some point. It's certainly subjective, but it might be better to play it safe here and have a specific compatible that one can match, even if the behavior is 99% the same. There will often be subtly different behavior across devices, you mentioned the "keyboard backlight turning off and the power LED slowly blinking", who knows what else exists. I suppose worst case we could also use of_machine_is_compatible() to just match the device the EC is running on, but I'm not sure if that would be frowned upon. Thanks, Stephan
