Re: [PATCH v4 6/9] arm64: dts: mediatek: Add MT8186 Krabby platform based Tentacruel / Tentacool

[Chen-Yu Tsai]

Hi,

On Mon, Jan 1, 2024 at 10:09 PM Eugen Hristev
<eugen.hristev@xxxxxxxxxxxxx> wrote:
>
> Hello Chen-Yu,
>
> There is still some nonconformity with the bindings, please see below:
>
> On 12/13/23 17:04, Chen-Yu Tsai wrote:
> > Tentacruel and Tentacool are MT8186 based Chromebooks based on the
> > Krabby design.
> >
> > Tentacruel, also known as the ASUS Chromebook CM14 Flip CM1402F, is a
> > convertible device with touchscreen and stylus.
> >
> > Tentacool, also known as the ASUS Chromebook CM14 CM1402C, is a laptop
> > device. It does not have a touchscreen or stylus.
> >
> > 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@xxxxxxxxxxxx>
> > Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > ---
> > Changes since v3:
> > - Reorder some properties to conform better to the newly proposed DT
> >   style guidelines
> > - Drop unused labels
> > - Rename bt-sco node name to bt-sco-codec
> > - Drop i2s*-share properties from afe node
> > - Drop aud_gpio_tdm_{on,off} pinctrl nodes
> > - Replace interrupts with interrupts-extended in tpm node
> > - Enable adsp device
> >
> > Changes since v2:
> > - Picked up Conor's ack
> > - Rename touchpad to trackpad
> > - Drop pinctrl properties from trackpad in tentacruel/tentacool second
> >   source trackpad
> >
> > Changes since v1:
> > - Reorder SKU numbers in descending order.
> > - Fixed pinconfig node names
> > - Moved pinctrl-* properties after interrupts-*
> > - Switched to interrupts-extended for external components
> > - Marked ADSP as explicitly disabled, with a comment explaining that it
> >   stalls the system
> > - Renamed "touchpad" to "trackpad"
> > - Dropped bogus "no-laneswap" property from it6505 node
> > - Moved "realtek,jd-src" property to after all the regulator supplies
> > - Switched to macros for MT6366 regulator "regulator-allowed-modes"
> > - Renamed "vgpu" regulator name to allow coupling, with a comment
> >   containing the name used in the design
> > - Renamed "cr50" node name to "tpm"
> > - Moved trackpad_pins reference up to i2c2; workaround for second source
> >   component resource sharing.
> > - Fix copyright year
> > - Fixed touchscreen supply name
> > ---
>
> [snip]
>
> > +
> > +&i2c3 {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&i2c3_pins>;
> > +     clock-frequency = <100000>;
> > +     status = "okay";
> > +
> > +     it6505dptx: dp-bridge@5c {
> > +             compatible = "ite,it6505";
>
> dp-bridge@5c: '#address-cells', '#size-cells', '#sound-dai-cells' do not match any
> of the regexes: 'pinctrl-[0-9]+'
>         from schema $id: http://devicetree.org/schemas/display/bridge/ite,it6505.yaml#

Will add a patch to update the bindings.

> > +             reg = <0x5c>;
> > +             interrupts-extended = <&pio 8 IRQ_TYPE_LEVEL_LOW>;
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&it6505_pins>;
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
>
> /soc/i2c@1100f000/dp-bridge@5c: unnecessary #address-cells/#size-cells without
> "ranges" or child "reg" property

Dropped.

> > +             #sound-dai-cells = <0>;
> > +             ovdd-supply = <&mt6366_vsim2_reg>;
> > +             pwr18-supply = <&pp1800_dpbrdg_dx>;
> > +             reset-gpios = <&pio 177 GPIO_ACTIVE_HIGH>;
> > +
> > +             ports {
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +
> > +                     port@0 {
> > +                             reg = <0>;
> > +
> > +                             it6505_in: endpoint {
> > +                                     link-frequencies = /bits/ 64 <150000000>;
> > +                                     remote-endpoint = <&dpi_out>;
> > +                             };
> > +                     };
> > +
> > +                     port@1 {
> > +                             reg = <1>;
> > +                     };
> > +             };
> > +     };
> > +};
> > +
>
> [snip]
>
> > +&spi1 {
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&spi1_pins>;
> > +     mediatek,pad-select = <0>;
> > +     status = "okay";
> > +
> > +     cros_ec: ec@0 {
> > +             compatible = "google,cros-ec-spi";
> > +             reg = <0>;
> > +             interrupts-extended = <&pio 13 IRQ_TYPE_LEVEL_LOW>;
> > +             pinctrl-names = "default";
> > +             pinctrl-0 = <&ec_ap_int>;
> > +             spi-max-frequency = <1000000>;
> > +
> > +             i2c_tunnel: i2c-tunnel {
> > +                     compatible = "google,cros-ec-i2c-tunnel";
> > +                     google,remote-bus = <1>;
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +             };
> > +
> > +             typec {
> > +                     compatible = "google,cros-ec-typec";
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +
> > +                     usb_c0: connector@0 {
> > +                             compatible = "usb-c-connector";
> > +                             reg = <0>;
> > +                             label = "left";
> > +                             power-role = "dual";
> > +                             data-role = "host";
> > +                             try-power-role = "source";
> > +
> > +                             ports {
> > +                                     #address-cells = <1>;
> > +                                     #size-cells = <0>;
> > +
> typec:connector@0:ports: 'port@0' is a required property
> > +                                     port@1 {
> > +                                             reg = <1>;
> > +
> > +                                             typec_port0: endpoint { };
> > +                                     };
> > +                             };
> > +                     };
> > +
> > +                     usb_c1: connector@1 {
> > +                             compatible = "usb-c-connector";
> > +                             reg = <1>;
> > +                             label = "right";
> > +                             power-role = "dual";
> > +                             data-role = "host";
> > +                             try-power-role = "source";
> > +
> > +                             ports {
> connector@1: Unevaluated properties are not allowed ('ports' was unexpected)
>         from schema $id: http://devicetree.org/schemas/connector/usb-connector.yaml#

Not sure why this is happening. Maybe because the sub-schema validation
failed?

In any case, I will drop the whole ports section. This can be re-added once
all the type-C mux stuff has been sorted out.

> > +                                     #address-cells = <1>;
> > +                                     #size-cells = <0>;
> > +
> > +                                     port@1 {
> connector@0: ports: 'port@0' is a required property
> > +                                             reg = <1>;
> > +
> > +                                             typec_port1: endpoint { };
> > +                                     };
> > +                             };
> > +                     };
> > +             };
> > +     };
> > +};
> > +
>
> [snip]
>
> > +
> > +&usb_host1 {
> > +     #address-cells = <2>;
> > +     #size-cells = <2>;
>
>  usb@11281000: usb@11280000:#address-cells:0:0: 1 was expected
>         from schema $id: http://devicetree.org/schemas/usb/mediatek,mtu3.yaml#
> usb@11281000: usb@11280000:#size-cells:0:0: 0 was expected

Dropped.

> > +     vbus-supply = <&usb_p1_vbus>;
> > +     status = "okay";
> > +};
> > +
> > +&watchdog {
> > +     mediatek,reset-by-toprgu;
> > +};
> > +
> > +#include <arm/cros-ec-keyboard.dtsi>
> > +#include <arm/cros-ec-sbs.dtsi>
>
>
> Eugen

Thanks for the review. Since the merge window is just around the corner,
I will send a new version later this month.

ChenYu