Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two
are based on the same board design: the former is a convertible device
with a touchscreen, stylus, and some extra buttons; the latter is a
clamshell device and lacks these additional features.
The two devices both have two variants. The difference is a second
source touchpad controller that shares the same address as the original,
but is incompatible.
The extra SKU IDs for the Tentacruel devices map to different sensor
components attached to the Embedded Controller. These are not visible
to the main processor.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
.../devicetree/bindings/arm/mediatek.yaml | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
index 60337b439744..aa7e6734b336 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
@@ -206,6 +206,32 @@ properties:
- enum:
- mediatek,mt8183-pumpkin
- const: mediatek,mt8183
+ - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F)
+ items:
+ - const: google,tentacruel-sku262144
+ - const: google,tentacruel-sku262145
+ - const: google,tentacruel-sku262146
+ - const: google,tentacruel-sku262147
+ - const: google,tentacruel
+ - const: mediatek,mt8186
+ - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F)
+ items:
+ - const: google,tentacruel-sku262148
+ - const: google,tentacruel-sku262149
+ - const: google,tentacruel-sku262150
+ - const: google,tentacruel-sku262151
+ - const: google,tentacruel
+ - const: mediatek,mt8186
+ - description: Google Tentacool (ASUS Chromebook CM14 CM1402C)
+ items:
+ - const: google,tentacruel-sku327681
+ - const: google,tentacruel
+ - const: mediatek,mt8186
+ - description: Google Tentacool (ASUS Chromebook CM14 CM1402C)
+ items:
+ - const: google,tentacruel-sku327683
+ - const: google,tentacruel
+ - const: mediatek,mt8186
- items:
- enum:
- mediatek,mt8186-evb
--
2.42.0.655.g421f12c284-googOn Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote: > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two > are based on the same board design: the former is a convertible device > with a touchscreen, stylus, and some extra buttons; the latter is a > clamshell device and lacks these additional features. > > The two devices both have two variants. The difference is a second > source touchpad controller that shares the same address as the original, > but is incompatible. > > The extra SKU IDs for the Tentacruel devices map to different sensor > components attached to the Embedded Controller. These are not visible > to the main processor. > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> I had a chat with Doug at Plumbers about the limitations of your firmware. As a result, I am fine with acking this if you switch the order of the sku compatibles to be in descending order. The firmware can handle that, right? Cheers, Conor. > --- > .../devicetree/bindings/arm/mediatek.yaml | 26 +++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml > index 60337b439744..aa7e6734b336 100644 > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml > @@ -206,6 +206,32 @@ properties: > - enum: > - mediatek,mt8183-pumpkin > - const: mediatek,mt8183 > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F) > + items: > + - const: google,tentacruel-sku262144 > + - const: google,tentacruel-sku262145 > + - const: google,tentacruel-sku262146 > + - const: google,tentacruel-sku262147 > + - const: google,tentacruel > + - const: mediatek,mt8186 > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F) > + items: > + - const: google,tentacruel-sku262148 > + - const: google,tentacruel-sku262149 > + - const: google,tentacruel-sku262150 > + - const: google,tentacruel-sku262151 > + - const: google,tentacruel > + - const: mediatek,mt8186 > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C) > + items: > + - const: google,tentacruel-sku327681 > + - const: google,tentacruel > + - const: mediatek,mt8186 > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C) > + items: > + - const: google,tentacruel-sku327683 > + - const: google,tentacruel > + - const: mediatek,mt8186 > - items: > - enum: > - mediatek,mt8186-evb > -- > 2.42.0.655.g421f12c284-goog >
On Fri, Nov 24, 2023 at 10:09 PM Conor Dooley <conor@kernel.org> wrote: > > On Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote: > > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two > > are based on the same board design: the former is a convertible device > > with a touchscreen, stylus, and some extra buttons; the latter is a > > clamshell device and lacks these additional features. > > > > The two devices both have two variants. The difference is a second > > source touchpad controller that shares the same address as the original, > > but is incompatible. > > > > The extra SKU IDs for the Tentacruel devices map to different sensor > > components attached to the Embedded Controller. These are not visible > > to the main processor. > > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > > I had a chat with Doug at Plumbers about the limitations of your > firmware. As a result, I am fine with acking this if you switch the > order of the sku compatibles to be in descending order. Thank you for taking the effort to understand our weird firmware implementation. And a thank you to Doug for discussing this. I'll send out the next version shortly. > The firmware can handle that, right? Yes. The firmware basically takes a list of machine compatibles, and runs them against all the DTs it has with of_machine_is_compatible(). Thanks ChenYu > > Cheers, > Conor. > > > --- > > .../devicetree/bindings/arm/mediatek.yaml | 26 +++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml > > index 60337b439744..aa7e6734b336 100644 > > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml > > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml > > @@ -206,6 +206,32 @@ properties: > > - enum: > > - mediatek,mt8183-pumpkin > > - const: mediatek,mt8183 > > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F) > > + items: > > + - const: google,tentacruel-sku262144 > > + - const: google,tentacruel-sku262145 > > + - const: google,tentacruel-sku262146 > > + - const: google,tentacruel-sku262147 > > + - const: google,tentacruel > > + - const: mediatek,mt8186 > > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F) > > + items: > > + - const: google,tentacruel-sku262148 > > + - const: google,tentacruel-sku262149 > > + - const: google,tentacruel-sku262150 > > + - const: google,tentacruel-sku262151 > > + - const: google,tentacruel > > + - const: mediatek,mt8186 > > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C) > > + items: > > + - const: google,tentacruel-sku327681 > > + - const: google,tentacruel > > + - const: mediatek,mt8186 > > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C) > > + items: > > + - const: google,tentacruel-sku327683 > > + - const: google,tentacruel > > + - const: mediatek,mt8186 > > - items: > > - enum: > > - mediatek,mt8186-evb > > -- > > 2.42.0.655.g421f12c284-goog > >
On Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote: > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two > are based on the same board design: the former is a convertible device > with a touchscreen, stylus, and some extra buttons; the latter is a > clamshell device and lacks these additional features. > > The two devices both have two variants. The difference is a second > source touchpad controller that shares the same address as the original, > but is incompatible. > The extra SKU IDs for the Tentacruel devices map to different sensor > components attached to the Embedded Controller. These are not visible > to the main processor. Wha? Given your ordering, is a "google,tentacruel-sku262144" a super-set of "google,tentacruel-sku262145"? If not, this compatible ordering doesn't make sense. I can't tell from your description, and the absence of a items: - const: google,tentacruel-sku262145 - const: google,tentacruel-sku262146 - const: google,tentacruel-sku262147 - const: google,tentacruel - const: mediatek,mt8186 suggests that there is no google,tentacruel-sku262145 device? Cheers, Conor. > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > --- > .../devicetree/bindings/arm/mediatek.yaml | 26 +++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml > index 60337b439744..aa7e6734b336 100644 > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml > @@ -206,6 +206,32 @@ properties: > - enum: > - mediatek,mt8183-pumpkin > - const: mediatek,mt8183 > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F) > + items: > + - const: google,tentacruel-sku262144 > + - const: google,tentacruel-sku262145 > + - const: google,tentacruel-sku262146 > + - const: google,tentacruel-sku262147 > + - const: google,tentacruel > + - const: mediatek,mt8186 > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F) > + items: > + - const: google,tentacruel-sku262148 > + - const: google,tentacruel-sku262149 > + - const: google,tentacruel-sku262150 > + - const: google,tentacruel-sku262151 > + - const: google,tentacruel > + - const: mediatek,mt8186 > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C) > + items: > + - const: google,tentacruel-sku327681 > + - const: google,tentacruel > + - const: mediatek,mt8186 > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C) > + items: > + - const: google,tentacruel-sku327683 > + - const: google,tentacruel > + - const: mediatek,mt8186 > - items: > - enum: > - mediatek,mt8186-evb > -- > 2.42.0.655.g421f12c284-goog >
On Fri, Oct 13, 2023 at 8:11 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote:
> > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two
> > are based on the same board design: the former is a convertible device
> > with a touchscreen, stylus, and some extra buttons; the latter is a
> > clamshell device and lacks these additional features.
> >
> > The two devices both have two variants. The difference is a second
> > source touchpad controller that shares the same address as the original,
> > but is incompatible.
>
> > The extra SKU IDs for the Tentacruel devices map to different sensor
> > components attached to the Embedded Controller. These are not visible
> > to the main processor.
>
> Wha? Given your ordering, is a "google,tentacruel-sku262144" a super-set
> of "google,tentacruel-sku262145"? If not, this compatible ordering
> doesn't make sense. I can't tell from your description, and the
> absence of a
> items:
> - const: google,tentacruel-sku262145
> - const: google,tentacruel-sku262146
> - const: google,tentacruel-sku262147
> - const: google,tentacruel
> - const: mediatek,mt8186
> suggests that there is no google,tentacruel-sku262145
> device?
AFAIK all four SKUs exist. And as far as the main processor is concerned,
they look completely identical, so they should share the same device tree.
As mentioned in the commit message, the differences are only visible to
the embedded controller, which fuses the sensor inputs.
Writing it this way avoids having four identical device tree files.
We also do this for many other device families, though those cover
different revisions, such as:
- description: Google Hana (Lenovo Chromebook N23 Yoga, C330, 300e,...)
items:
- const: google,hana-rev6
- const: google,hana-rev5
- const: google,hana-rev4
- const: google,hana-rev3
- const: google,hana
- const: mediatek,mt8173
ChenYu
> Cheers,
> Conor.
>
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> > .../devicetree/bindings/arm/mediatek.yaml | 26 +++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
> > index 60337b439744..aa7e6734b336 100644
> > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml
> > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
> > @@ -206,6 +206,32 @@ properties:
> > - enum:
> > - mediatek,mt8183-pumpkin
> > - const: mediatek,mt8183
> > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F)
> > + items:
> > + - const: google,tentacruel-sku262144
> > + - const: google,tentacruel-sku262145
> > + - const: google,tentacruel-sku262146
> > + - const: google,tentacruel-sku262147
> > + - const: google,tentacruel
> > + - const: mediatek,mt8186
> > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F)
> > + items:
> > + - const: google,tentacruel-sku262148
> > + - const: google,tentacruel-sku262149
> > + - const: google,tentacruel-sku262150
> > + - const: google,tentacruel-sku262151
> > + - const: google,tentacruel
> > + - const: mediatek,mt8186
> > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C)
> > + items:
> > + - const: google,tentacruel-sku327681
> > + - const: google,tentacruel
> > + - const: mediatek,mt8186
> > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C)
> > + items:
> > + - const: google,tentacruel-sku327683
> > + - const: google,tentacruel
> > + - const: mediatek,mt8186
> > - items:
> > - enum:
> > - mediatek,mt8186-evb
> > --
> > 2.42.0.655.g421f12c284-goog
> >
On Fri, Oct 13, 2023 at 10:29:25AM -0700, Chen-Yu Tsai wrote: > On Fri, Oct 13, 2023 at 8:11 AM Conor Dooley <conor@kernel.org> wrote: > > > > On Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote: > > > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two > > > are based on the same board design: the former is a convertible device > > > with a touchscreen, stylus, and some extra buttons; the latter is a > > > clamshell device and lacks these additional features. > > > > > > The two devices both have two variants. The difference is a second > > > source touchpad controller that shares the same address as the original, > > > but is incompatible. > > > > > The extra SKU IDs for the Tentacruel devices map to different sensor > > > components attached to the Embedded Controller. These are not visible > > > to the main processor. > > > > Wha? Given your ordering, is a "google,tentacruel-sku262144" a super-set > > of "google,tentacruel-sku262145"? If not, this compatible ordering > > doesn't make sense. I can't tell from your description, and the > > absence of a > > items: > > - const: google,tentacruel-sku262145 > > - const: google,tentacruel-sku262146 > > - const: google,tentacruel-sku262147 > > - const: google,tentacruel > > - const: mediatek,mt8186 > > suggests that there is no google,tentacruel-sku262145 > > device? > > AFAIK all four SKUs exist. And as far as the main processor is concerned, > they look completely identical, so they should share the same device tree. > As mentioned in the commit message, the differences are only visible to > the embedded controller, which fuses the sensor inputs. Then it makes very little sense to write a binding like this. If this was just for the 252144 SKU, this would be fine. For the other SKUs, there is no way to uniquely identify them, as all four of google,tentacruel-sku262144, google,tentacruel-sku262145, google,tentacruel-sku262146 and google,tentacruel-sku262147 must be present. Given that, why even bother including the SKUs in the first place, since no information can be derived from them that cannot be derived from google,tentacruel? There's something that I am clearly missing here... Also, why is the order inverted, with the lower SKUs being super-sets of the higher ones? The Hana one you show below makes a little more sense in that regard. > Writing it this way avoids having four identical device tree files. > > We also do this for many other device families, though those cover > different revisions, such as: > > - description: Google Hana (Lenovo Chromebook N23 Yoga, C330, 300e,...) > items: > - const: google,hana-rev6 > - const: google,hana-rev5 > - const: google,hana-rev4 > - const: google,hana-rev3 > - const: google,hana > - const: mediatek,mt8173 > > > ChenYu > > > Cheers, > > Conor. > > > > > > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> > > > --- > > > .../devicetree/bindings/arm/mediatek.yaml | 26 +++++++++++++++++++ > > > 1 file changed, 26 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml > > > index 60337b439744..aa7e6734b336 100644 > > > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml > > > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml > > > @@ -206,6 +206,32 @@ properties: > > > - enum: > > > - mediatek,mt8183-pumpkin > > > - const: mediatek,mt8183 > > > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F) > > > + items: > > > + - const: google,tentacruel-sku262144 > > > + - const: google,tentacruel-sku262145 > > > + - const: google,tentacruel-sku262146 > > > + - const: google,tentacruel-sku262147 > > > + - const: google,tentacruel > > > + - const: mediatek,mt8186 > > > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F) > > > + items: > > > + - const: google,tentacruel-sku262148 > > > + - const: google,tentacruel-sku262149 > > > + - const: google,tentacruel-sku262150 > > > + - const: google,tentacruel-sku262151 > > > + - const: google,tentacruel > > > + - const: mediatek,mt8186 > > > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C) > > > + items: > > > + - const: google,tentacruel-sku327681 > > > + - const: google,tentacruel > > > + - const: mediatek,mt8186 > > > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C) > > > + items: > > > + - const: google,tentacruel-sku327683 > > > + - const: google,tentacruel > > > + - const: mediatek,mt8186 > > > - items: > > > - enum: > > > - mediatek,mt8186-evb > > > -- > > > 2.42.0.655.g421f12c284-goog > > >
On Fri, Oct 13, 2023 at 10:55 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Oct 13, 2023 at 10:29:25AM -0700, Chen-Yu Tsai wrote:
> > On Fri, Oct 13, 2023 at 8:11 AM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote:
> > > > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two
> > > > are based on the same board design: the former is a convertible device
> > > > with a touchscreen, stylus, and some extra buttons; the latter is a
> > > > clamshell device and lacks these additional features.
> > > >
> > > > The two devices both have two variants. The difference is a second
> > > > source touchpad controller that shares the same address as the original,
> > > > but is incompatible.
> > >
> > > > The extra SKU IDs for the Tentacruel devices map to different sensor
> > > > components attached to the Embedded Controller. These are not visible
> > > > to the main processor.
> > >
> > > Wha? Given your ordering, is a "google,tentacruel-sku262144" a super-set
> > > of "google,tentacruel-sku262145"? If not, this compatible ordering
> > > doesn't make sense. I can't tell from your description, and the
> > > absence of a
> > > items:
> > > - const: google,tentacruel-sku262145
> > > - const: google,tentacruel-sku262146
> > > - const: google,tentacruel-sku262147
> > > - const: google,tentacruel
> > > - const: mediatek,mt8186
> > > suggests that there is no google,tentacruel-sku262145
> > > device?
> >
> > AFAIK all four SKUs exist. And as far as the main processor is concerned,
> > they look completely identical, so they should share the same device tree.
> > As mentioned in the commit message, the differences are only visible to
> > the embedded controller, which fuses the sensor inputs.
>
> Then it makes very little sense to write a binding like this.
> If this was just for the 252144 SKU, this would be fine.
> For the other SKUs, there is no way to uniquely identify them, as
> all four of google,tentacruel-sku262144, google,tentacruel-sku262145,
> google,tentacruel-sku262146 and google,tentacruel-sku262147 must be
> present.
> Given that, why even bother including the SKUs in the first place,
> since no information can be derived from them that cannot be derived
> from google,tentacruel?
> There's something that I am clearly missing here...
There are incompatible variants of google,tentacruel. This is why this
patch has four google,tentacruel based entries. Of them, two are Tentacool,
which are clamshell laptops, and two of them are Tentacruel, which are
convertibles.
Within each group there are two variants: the second variant swaps out
the I2C touchpad controller. These two controllers use the same I2C
address but use different compatible strings, so it's not possible to
have them coexist within the same device tree file like we do for many
other second source components.
So the relationship looks like the following:
google,tentacruel --- Tentacruel --- google,tentacruel-sku26214[4567]
| |
| -- google,tentacruel-sku2621{48,49,50,51}
|
-- Tentacool ---- google,tentacruel-sku327681
|
--- google,tentacruel-sku327683
Also, the devices themselves only know their own SKU ID. The firmware
will generate a list of compatible strings like:
google,tentacruel-rev4-sku262144
google,tentacruel-rev4
google,tentacruel-sku262144
google,tentacruel
and try to find a match in the kernel FIT image. The method we currently
use is to include all the applicable board compatible strings.
> Also, why is the order inverted, with the lower SKUs being super-sets of
> the higher ones? The Hana one you show below makes a little more sense
> in that regard.
Either way works. The SKU IDs have no particular order. Nor do the
individual bits in the SKU ID have meaning. They are just arbitrarily
assigned by the manufacturer. They just have to all be present so any
of the SKUs will match.
Hope that explains things better.
Regards
ChenYu
> > Writing it this way avoids having four identical device tree files.
> >
> > We also do this for many other device families, though those cover
> > different revisions, such as:
> >
> > - description: Google Hana (Lenovo Chromebook N23 Yoga, C330, 300e,...)
> > items:
> > - const: google,hana-rev6
> > - const: google,hana-rev5
> > - const: google,hana-rev4
> > - const: google,hana-rev3
> > - const: google,hana
> > - const: mediatek,mt8173
> >
> >
> > ChenYu
> >
> > > Cheers,
> > > Conor.
> > >
> > > >
> > > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > > > ---
> > > > .../devicetree/bindings/arm/mediatek.yaml | 26 +++++++++++++++++++
> > > > 1 file changed, 26 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
> > > > index 60337b439744..aa7e6734b336 100644
> > > > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml
> > > > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
> > > > @@ -206,6 +206,32 @@ properties:
> > > > - enum:
> > > > - mediatek,mt8183-pumpkin
> > > > - const: mediatek,mt8183
> > > > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F)
> > > > + items:
> > > > + - const: google,tentacruel-sku262144
> > > > + - const: google,tentacruel-sku262145
> > > > + - const: google,tentacruel-sku262146
> > > > + - const: google,tentacruel-sku262147
> > > > + - const: google,tentacruel
> > > > + - const: mediatek,mt8186
> > > > + - description: Google Tentacruel (ASUS Chromebook CM14 Flip CM1402F)
> > > > + items:
> > > > + - const: google,tentacruel-sku262148
> > > > + - const: google,tentacruel-sku262149
> > > > + - const: google,tentacruel-sku262150
> > > > + - const: google,tentacruel-sku262151
> > > > + - const: google,tentacruel
> > > > + - const: mediatek,mt8186
> > > > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C)
> > > > + items:
> > > > + - const: google,tentacruel-sku327681
> > > > + - const: google,tentacruel
> > > > + - const: mediatek,mt8186
> > > > + - description: Google Tentacool (ASUS Chromebook CM14 CM1402C)
> > > > + items:
> > > > + - const: google,tentacruel-sku327683
> > > > + - const: google,tentacruel
> > > > + - const: mediatek,mt8186
> > > > - items:
> > > > - enum:
> > > > - mediatek,mt8186-evb
> > > > --
> > > > 2.42.0.655.g421f12c284-goog
> > > >
On Fri, Oct 13, 2023 at 11:19:16AM -0700, Chen-Yu Tsai wrote:
> On Fri, Oct 13, 2023 at 10:55 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Fri, Oct 13, 2023 at 10:29:25AM -0700, Chen-Yu Tsai wrote:
> > > On Fri, Oct 13, 2023 at 8:11 AM Conor Dooley <conor@kernel.org> wrote:
> > > >
> > > > On Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote:
> > > > > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two
> > > > > are based on the same board design: the former is a convertible device
> > > > > with a touchscreen, stylus, and some extra buttons; the latter is a
> > > > > clamshell device and lacks these additional features.
> > > > >
> > > > > The two devices both have two variants. The difference is a second
> > > > > source touchpad controller that shares the same address as the original,
> > > > > but is incompatible.
> > > >
> > > > > The extra SKU IDs for the Tentacruel devices map to different sensor
> > > > > components attached to the Embedded Controller. These are not visible
> > > > > to the main processor.
> > > >
> > > > Wha? Given your ordering, is a "google,tentacruel-sku262144" a super-set
> > > > of "google,tentacruel-sku262145"? If not, this compatible ordering
> > > > doesn't make sense. I can't tell from your description, and the
> > > > absence of a
> > > > items:
> > > > - const: google,tentacruel-sku262145
> > > > - const: google,tentacruel-sku262146
> > > > - const: google,tentacruel-sku262147
> > > > - const: google,tentacruel
> > > > - const: mediatek,mt8186
> > > > suggests that there is no google,tentacruel-sku262145
> > > > device?
> > >
> > > AFAIK all four SKUs exist. And as far as the main processor is concerned,
> > > they look completely identical, so they should share the same device tree.
> > > As mentioned in the commit message, the differences are only visible to
> > > the embedded controller, which fuses the sensor inputs.
> >
> > Then it makes very little sense to write a binding like this.
> > If this was just for the 252144 SKU, this would be fine.
> > For the other SKUs, there is no way to uniquely identify them, as
> > all four of google,tentacruel-sku262144, google,tentacruel-sku262145,
> > google,tentacruel-sku262146 and google,tentacruel-sku262147 must be
> > present.
> > Given that, why even bother including the SKUs in the first place,
> > since no information can be derived from them that cannot be derived
> > from google,tentacruel?
> > There's something that I am clearly missing here...
>
> There are incompatible variants of google,tentacruel. This is why this
> patch has four google,tentacruel based entries. Of them, two are Tentacool,
> which are clamshell laptops, and two of them are Tentacruel, which are
> convertibles.
>
> Within each group there are two variants: the second variant swaps out
> the I2C touchpad controller. These two controllers use the same I2C
> address but use different compatible strings, so it's not possible to
> have them coexist within the same device tree file like we do for many
> other second source components.
>
> So the relationship looks like the following:
>
> google,tentacruel --- Tentacruel --- google,tentacruel-sku26214[4567]
> | |
> | -- google,tentacruel-sku2621{48,49,50,51}
> |
> -- Tentacool ---- google,tentacruel-sku327681
> |
> --- google,tentacruel-sku327683
>
> Also, the devices themselves only know their own SKU ID. The firmware
> will generate a list of compatible strings like:
>
> google,tentacruel-rev4-sku262144
> google,tentacruel-rev4
> google,tentacruel-sku262144
> google,tentacruel
>
> and try to find a match in the kernel FIT image. The method we currently
> use is to include all the applicable board compatible strings.
Then it seems like what you need is something like
oneOf:
- items:
- const: google,tentacruel-sku262144
- const: google,tentacruel
- const: mediatek,mt8186
- items:
- enum:
- google,tentacruel-sku262145
- google,tentacruel-sku262146
- google,tentacruel-sku262147
- const: google,tentacruel-sku262144
- const: google,tentacruel
- const: mediatek,mt8186
What you have at the moment just seems like a hack because you want to
stuff all of these compatible strings into a single dts.
On Sat, Oct 14, 2023 at 6:40 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Oct 13, 2023 at 11:19:16AM -0700, Chen-Yu Tsai wrote:
> > On Fri, Oct 13, 2023 at 10:55 AM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Fri, Oct 13, 2023 at 10:29:25AM -0700, Chen-Yu Tsai wrote:
> > > > On Fri, Oct 13, 2023 at 8:11 AM Conor Dooley <conor@kernel.org> wrote:
> > > > >
> > > > > On Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote:
> > > > > > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two
> > > > > > are based on the same board design: the former is a convertible device
> > > > > > with a touchscreen, stylus, and some extra buttons; the latter is a
> > > > > > clamshell device and lacks these additional features.
> > > > > >
> > > > > > The two devices both have two variants. The difference is a second
> > > > > > source touchpad controller that shares the same address as the original,
> > > > > > but is incompatible.
> > > > >
> > > > > > The extra SKU IDs for the Tentacruel devices map to different sensor
> > > > > > components attached to the Embedded Controller. These are not visible
> > > > > > to the main processor.
> > > > >
> > > > > Wha? Given your ordering, is a "google,tentacruel-sku262144" a super-set
> > > > > of "google,tentacruel-sku262145"? If not, this compatible ordering
> > > > > doesn't make sense. I can't tell from your description, and the
> > > > > absence of a
> > > > > items:
> > > > > - const: google,tentacruel-sku262145
> > > > > - const: google,tentacruel-sku262146
> > > > > - const: google,tentacruel-sku262147
> > > > > - const: google,tentacruel
> > > > > - const: mediatek,mt8186
> > > > > suggests that there is no google,tentacruel-sku262145
> > > > > device?
> > > >
> > > > AFAIK all four SKUs exist. And as far as the main processor is concerned,
> > > > they look completely identical, so they should share the same device tree.
> > > > As mentioned in the commit message, the differences are only visible to
> > > > the embedded controller, which fuses the sensor inputs.
> > >
> > > Then it makes very little sense to write a binding like this.
> > > If this was just for the 252144 SKU, this would be fine.
> > > For the other SKUs, there is no way to uniquely identify them, as
> > > all four of google,tentacruel-sku262144, google,tentacruel-sku262145,
> > > google,tentacruel-sku262146 and google,tentacruel-sku262147 must be
> > > present.
> > > Given that, why even bother including the SKUs in the first place,
> > > since no information can be derived from them that cannot be derived
> > > from google,tentacruel?
> > > There's something that I am clearly missing here...
> >
> > There are incompatible variants of google,tentacruel. This is why this
> > patch has four google,tentacruel based entries. Of them, two are Tentacool,
> > which are clamshell laptops, and two of them are Tentacruel, which are
> > convertibles.
> >
> > Within each group there are two variants: the second variant swaps out
> > the I2C touchpad controller. These two controllers use the same I2C
> > address but use different compatible strings, so it's not possible to
> > have them coexist within the same device tree file like we do for many
> > other second source components.
> >
> > So the relationship looks like the following:
> >
> > google,tentacruel --- Tentacruel --- google,tentacruel-sku26214[4567]
> > | |
> > | -- google,tentacruel-sku2621{48,49,50,51}
> > |
> > -- Tentacool ---- google,tentacruel-sku327681
> > |
> > --- google,tentacruel-sku327683
> >
> > Also, the devices themselves only know their own SKU ID. The firmware
> > will generate a list of compatible strings like:
> >
> > google,tentacruel-rev4-sku262144
> > google,tentacruel-rev4
> > google,tentacruel-sku262144
> > google,tentacruel
> >
> > and try to find a match in the kernel FIT image. The method we currently
> > use is to include all the applicable board compatible strings.
>
> Then it seems like what you need is something like
> oneOf:
> - items:
> - const: google,tentacruel-sku262144
> - const: google,tentacruel
> - const: mediatek,mt8186
> - items:
> - enum:
> - google,tentacruel-sku262145
> - google,tentacruel-sku262146
> - google,tentacruel-sku262147
> - const: google,tentacruel-sku262144
> - const: google,tentacruel
> - const: mediatek,mt8186
>
> What you have at the moment just seems like a hack because you want to
> stuff all of these compatible strings into a single dts.
It is. And it works OK downstream. The reason we want to stuff them in
one dts is because the firmware will not generate the fallback to
sku262144 as the scheme above suggests.
Having three or four identical device trees just with different board
compatible string sequences to me seems a bit redundant, and it does
end up bloating our FIT image a bit, which impacts boot time.
Regards
ChenYu
On Sun, Oct 15, 2023 at 11:15:22PM -0700, Chen-Yu Tsai wrote:
> On Sat, Oct 14, 2023 at 6:40 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Fri, Oct 13, 2023 at 11:19:16AM -0700, Chen-Yu Tsai wrote:
> > > On Fri, Oct 13, 2023 at 10:55 AM Conor Dooley <conor@kernel.org> wrote:
> > > >
> > > > On Fri, Oct 13, 2023 at 10:29:25AM -0700, Chen-Yu Tsai wrote:
> > > > > On Fri, Oct 13, 2023 at 8:11 AM Conor Dooley <conor@kernel.org> wrote:
> > > > > >
> > > > > > On Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote:
> > > > > > > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two
> > > > > > > are based on the same board design: the former is a convertible device
> > > > > > > with a touchscreen, stylus, and some extra buttons; the latter is a
> > > > > > > clamshell device and lacks these additional features.
> > > > > > >
> > > > > > > The two devices both have two variants. The difference is a second
> > > > > > > source touchpad controller that shares the same address as the original,
> > > > > > > but is incompatible.
> > > > > >
> > > > > > > The extra SKU IDs for the Tentacruel devices map to different sensor
> > > > > > > components attached to the Embedded Controller. These are not visible
> > > > > > > to the main processor.
> > > > > >
> > > > > > Wha? Given your ordering, is a "google,tentacruel-sku262144" a super-set
> > > > > > of "google,tentacruel-sku262145"? If not, this compatible ordering
> > > > > > doesn't make sense. I can't tell from your description, and the
> > > > > > absence of a
> > > > > > items:
> > > > > > - const: google,tentacruel-sku262145
> > > > > > - const: google,tentacruel-sku262146
> > > > > > - const: google,tentacruel-sku262147
> > > > > > - const: google,tentacruel
> > > > > > - const: mediatek,mt8186
> > > > > > suggests that there is no google,tentacruel-sku262145
> > > > > > device?
> > > > >
> > > > > AFAIK all four SKUs exist. And as far as the main processor is concerned,
> > > > > they look completely identical, so they should share the same device tree.
> > > > > As mentioned in the commit message, the differences are only visible to
> > > > > the embedded controller, which fuses the sensor inputs.
> > > >
> > > > Then it makes very little sense to write a binding like this.
> > > > If this was just for the 252144 SKU, this would be fine.
> > > > For the other SKUs, there is no way to uniquely identify them, as
> > > > all four of google,tentacruel-sku262144, google,tentacruel-sku262145,
> > > > google,tentacruel-sku262146 and google,tentacruel-sku262147 must be
> > > > present.
> > > > Given that, why even bother including the SKUs in the first place,
> > > > since no information can be derived from them that cannot be derived
> > > > from google,tentacruel?
> > > > There's something that I am clearly missing here...
> > >
> > > There are incompatible variants of google,tentacruel. This is why this
> > > patch has four google,tentacruel based entries. Of them, two are Tentacool,
> > > which are clamshell laptops, and two of them are Tentacruel, which are
> > > convertibles.
> > >
> > > Within each group there are two variants: the second variant swaps out
> > > the I2C touchpad controller. These two controllers use the same I2C
> > > address but use different compatible strings, so it's not possible to
> > > have them coexist within the same device tree file like we do for many
> > > other second source components.
> > >
> > > So the relationship looks like the following:
> > >
> > > google,tentacruel --- Tentacruel --- google,tentacruel-sku26214[4567]
> > > | |
> > > | -- google,tentacruel-sku2621{48,49,50,51}
> > > |
> > > -- Tentacool ---- google,tentacruel-sku327681
> > > |
> > > --- google,tentacruel-sku327683
> > >
> > > Also, the devices themselves only know their own SKU ID. The firmware
> > > will generate a list of compatible strings like:
> > >
> > > google,tentacruel-rev4-sku262144
> > > google,tentacruel-rev4
> > > google,tentacruel-sku262144
> > > google,tentacruel
> > >
> > > and try to find a match in the kernel FIT image. The method we currently
> > > use is to include all the applicable board compatible strings.
> >
> > Then it seems like what you need is something like
> > oneOf:
> > - items:
> > - const: google,tentacruel-sku262144
> > - const: google,tentacruel
> > - const: mediatek,mt8186
> > - items:
> > - enum:
> > - google,tentacruel-sku262145
> > - google,tentacruel-sku262146
> > - google,tentacruel-sku262147
> > - const: google,tentacruel-sku262144
> > - const: google,tentacruel
> > - const: mediatek,mt8186
> >
> > What you have at the moment just seems like a hack because you want to
> > stuff all of these compatible strings into a single dts.
>
> It is. And it works OK downstream. The reason we want to stuff them in
> one dts is because the firmware will not generate the fallback to
> sku262144 as the scheme above suggests.
I'm not going to ack the hack that you have here, sorry. Maybe Rob or
Krzysztof will. The list your firmware generates above doesn't even
match the contents of this patch, with the extra "rev-4" compatibles.
> Having three or four identical device trees just with different board
> compatible string sequences to me seems a bit redundant, and it does
> end up bloating our FIT image a bit, which impacts boot time.
I'm not aware of the capabilities of your bootloader when it comes to
setting properties before passing them to the kernel, but that is what I
would be doing to cut down on having 4 different dtbs.
On Wed, Oct 18, 2023 at 11:07 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Sun, Oct 15, 2023 at 11:15:22PM -0700, Chen-Yu Tsai wrote:
> > On Sat, Oct 14, 2023 at 6:40 AM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Fri, Oct 13, 2023 at 11:19:16AM -0700, Chen-Yu Tsai wrote:
> > > > On Fri, Oct 13, 2023 at 10:55 AM Conor Dooley <conor@kernel.org> wrote:
> > > > >
> > > > > On Fri, Oct 13, 2023 at 10:29:25AM -0700, Chen-Yu Tsai wrote:
> > > > > > On Fri, Oct 13, 2023 at 8:11 AM Conor Dooley <conor@kernel.org> wrote:
> > > > > > >
> > > > > > > On Fri, Oct 13, 2023 at 07:02:28AM +0800, Chen-Yu Tsai wrote:
> > > > > > > > Add entries for MT8186 based Tentacruel / Tentacool Chromebooks. The two
> > > > > > > > are based on the same board design: the former is a convertible device
> > > > > > > > with a touchscreen, stylus, and some extra buttons; the latter is a
> > > > > > > > clamshell device and lacks these additional features.
> > > > > > > >
> > > > > > > > The two devices both have two variants. The difference is a second
> > > > > > > > source touchpad controller that shares the same address as the original,
> > > > > > > > but is incompatible.
> > > > > > >
> > > > > > > > The extra SKU IDs for the Tentacruel devices map to different sensor
> > > > > > > > components attached to the Embedded Controller. These are not visible
> > > > > > > > to the main processor.
> > > > > > >
> > > > > > > Wha? Given your ordering, is a "google,tentacruel-sku262144" a super-set
> > > > > > > of "google,tentacruel-sku262145"? If not, this compatible ordering
> > > > > > > doesn't make sense. I can't tell from your description, and the
> > > > > > > absence of a
> > > > > > > items:
> > > > > > > - const: google,tentacruel-sku262145
> > > > > > > - const: google,tentacruel-sku262146
> > > > > > > - const: google,tentacruel-sku262147
> > > > > > > - const: google,tentacruel
> > > > > > > - const: mediatek,mt8186
> > > > > > > suggests that there is no google,tentacruel-sku262145
> > > > > > > device?
> > > > > >
> > > > > > AFAIK all four SKUs exist. And as far as the main processor is concerned,
> > > > > > they look completely identical, so they should share the same device tree.
> > > > > > As mentioned in the commit message, the differences are only visible to
> > > > > > the embedded controller, which fuses the sensor inputs.
> > > > >
> > > > > Then it makes very little sense to write a binding like this.
> > > > > If this was just for the 252144 SKU, this would be fine.
> > > > > For the other SKUs, there is no way to uniquely identify them, as
> > > > > all four of google,tentacruel-sku262144, google,tentacruel-sku262145,
> > > > > google,tentacruel-sku262146 and google,tentacruel-sku262147 must be
> > > > > present.
> > > > > Given that, why even bother including the SKUs in the first place,
> > > > > since no information can be derived from them that cannot be derived
> > > > > from google,tentacruel?
> > > > > There's something that I am clearly missing here...
> > > >
> > > > There are incompatible variants of google,tentacruel. This is why this
> > > > patch has four google,tentacruel based entries. Of them, two are Tentacool,
> > > > which are clamshell laptops, and two of them are Tentacruel, which are
> > > > convertibles.
> > > >
> > > > Within each group there are two variants: the second variant swaps out
> > > > the I2C touchpad controller. These two controllers use the same I2C
> > > > address but use different compatible strings, so it's not possible to
> > > > have them coexist within the same device tree file like we do for many
> > > > other second source components.
> > > >
> > > > So the relationship looks like the following:
> > > >
> > > > google,tentacruel --- Tentacruel --- google,tentacruel-sku26214[4567]
> > > > | |
> > > > | -- google,tentacruel-sku2621{48,49,50,51}
> > > > |
> > > > -- Tentacool ---- google,tentacruel-sku327681
> > > > |
> > > > --- google,tentacruel-sku327683
> > > >
> > > > Also, the devices themselves only know their own SKU ID. The firmware
> > > > will generate a list of compatible strings like:
> > > >
> > > > google,tentacruel-rev4-sku262144
> > > > google,tentacruel-rev4
> > > > google,tentacruel-sku262144
> > > > google,tentacruel
> > > >
> > > > and try to find a match in the kernel FIT image. The method we currently
> > > > use is to include all the applicable board compatible strings.
> > >
> > > Then it seems like what you need is something like
> > > oneOf:
> > > - items:
> > > - const: google,tentacruel-sku262144
> > > - const: google,tentacruel
> > > - const: mediatek,mt8186
> > > - items:
> > > - enum:
> > > - google,tentacruel-sku262145
> > > - google,tentacruel-sku262146
> > > - google,tentacruel-sku262147
> > > - const: google,tentacruel-sku262144
> > > - const: google,tentacruel
> > > - const: mediatek,mt8186
> > >
> > > What you have at the moment just seems like a hack because you want to
> > > stuff all of these compatible strings into a single dts.
> >
> > It is. And it works OK downstream. The reason we want to stuff them in
> > one dts is because the firmware will not generate the fallback to
> > sku262144 as the scheme above suggests.
>
> I'm not going to ack the hack that you have here, sorry. Maybe Rob or
I understand.
> Krzysztof will. The list your firmware generates above doesn't even
> match the contents of this patch, with the extra "rev-4" compatibles.
The firmware generates a list of compatibles that it will go through and
try to find a matching compatible with. I believe the intent was not to
use it as "a list of compatibles" in the usual sense. The kernel image
comes bundled with a whole bunch of device trees, and the firmware just
tries to find the best or most specific match. The firmware only knows
about itself, and does not know about adjacent or similar SKUs or
revisions.
The process is described in detail in the kernel tree in
Documentation/arch/arm/google/chromebook-boot-flow.rst
or https://docs.kernel.org/arch/arm/google/chromebook-boot-flow.html
> > Having three or four identical device trees just with different board
> > compatible string sequences to me seems a bit redundant, and it does
> > end up bloating our FIT image a bit, which impacts boot time.
>
> I'm not aware of the capabilities of your bootloader when it comes to
> setting properties before passing them to the kernel, but that is what I
> would be doing to cut down on having 4 different dtbs.
That could be a solution for future devices, but the devices this series
covers are already on the market. The firmware on these devices is very
hard to update, and there is no guarantee that the device ever gets
updated. Sometimes these extra SKUs or component changes happen after
the device is in production, due to inventory issues or EOL of components.
This is what happened here.
Then there's sometimes the problem where we are originally shipping with
downstream patches / bindings that may change as they are upstreamed.
This is something we are trying to fix by having our vendors upstream
earlier, but sometimes we do end up carrying some stuff downstream
regardless.
Also, doing the customization in firmware would be somewhat fragile as
we end up trying to keep two separate software packages in sync.
I talked to Doug Anderson about this, and his "of: device: Support 2nd
sources of probeable but undiscoverable devices" proposal (specifically,
the second proposal [1] after Rob nacked the first one) could help solve
this as well. Part of what we have with the Tentacruel series is second
source drop-in components, which is the same issue that the proposal
wants to solve.
Specifically, instead of the firmware solution you proposed, there would
be a hardware prober driver running early in the kernel that deals with
the board specifics, either by checking the SKU ID passed in by the
firmware (in inserted device tree nodes), or by manually probing the
hardware in some board specific way and tweaking the device tree to
match. It would know what needs to be handled for each device series.
With this scheme, we would be able to merge some of the device trees
together, though I'm not sure we would want to merge all of
"google,tentacruel", as that would also cover variants of different
form factors. I guess it depends on how much editing or probing we
want to allow in the board manager.
Thanks
ChenYu
[1] https://lore.kernel.org/linux-devicetree/CAD=FV=UjVAgT-febtj4=UZ2GQp01D-ern2Ff9+ODcHeQBOsdTQ@mail.gmail.com/
On 18/10/2023 17:07, Conor Dooley wrote: >>> Then it seems like what you need is something like >>> oneOf: >>> - items: >>> - const: google,tentacruel-sku262144 >>> - const: google,tentacruel >>> - const: mediatek,mt8186 >>> - items: >>> - enum: >>> - google,tentacruel-sku262145 >>> - google,tentacruel-sku262146 >>> - google,tentacruel-sku262147 >>> - const: google,tentacruel-sku262144 >>> - const: google,tentacruel >>> - const: mediatek,mt8186 >>> >>> What you have at the moment just seems like a hack because you want to >>> stuff all of these compatible strings into a single dts. >> >> It is. And it works OK downstream. The reason we want to stuff them in >> one dts is because the firmware will not generate the fallback to >> sku262144 as the scheme above suggests. > > I'm not going to ack the hack that you have here, sorry. Maybe Rob or > Krzysztof will. The list your firmware generates above doesn't even > match the contents of this patch, with the extra "rev-4" compatibles. No Acks from me either... Chromebooks board compatibles are a long standing confusion for me and every time I forget why this is so counter-intuitively. My comments for changing this were rather receiving feedback "but our firmware likes it that way". OTOH, board compatibles just have to be unique, so we could accept some weird, counter-intuitive combinations... but it does not mean I should be happy with them. Best regards, Krzysztof
© 2016 - 2024 Red Hat, Inc.