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