On 20/12/2024 19:24, Stephan Gerhold wrote: > 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. No, because: 1. There is no such thing as x1e 2. If there was a soc like this, this has nothing to do with SoC. It is not a component inside SoC and that is the only allowed case when you use SoC compatibles. > 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. Unless you have some sort of insights or secret knowledge from Qualcomm (Bjorn or Konrad can chime in here), I think these are pure guesses that this is a Qualcomm product (implied by vendor prefix) or some sort of re-usable generic firmware from Qualcomm present on multiple devices. If the FW across devices is the same, then fallbacks for these are fine with me. Best regards, Krzysztof
