| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGXv+5E7yzwgEnSUaS3YerK_kcO853AJsq=gBbkWruh86mXbgQ@mail.gmail.com>
Date: Tue, 2 Jan 2024 18:25:46 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Eugen Hristev <eugen.hristev@...labora.com>
Cc: Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
devicetree@...r.kernel.org, linux-mediatek@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Conor Dooley <conor.dooley@...rochip.com>
Subject: Re: [PATCH v4 6/9] arm64: dts: mediatek: Add MT8186 Krabby platform
based Tentacruel / Tentacool
Hi,
On Mon, Jan 1, 2024 at 10:09 PM Eugen Hristev
<eugen.hristev@...labora.com> 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@...omium.org>
> > Acked-by: Conor Dooley <conor.dooley@...rochip.com>
> > ---
> > 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@...0f000/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@...81000: usb@...80000:#address-cells:0:0: 1 was expected
> from schema $id: http://devicetree.org/schemas/usb/mediatek,mtu3.yaml#
> usb@...81000: usb@...80000:#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
Powered by blists - more mailing lists