| 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: <f412c9f4-4813-4e81-babc-eb8850548a63@kernel.org>
Date: Sun, 4 Aug 2024 10:39:45 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Marcus Glocker <marcus@...gul.ch>, Bjorn Andersson
<andersson@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: Marijn Suijten <marijn.suijten@...ainline.org>,
linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, Abel Vesa <abel.vesa@...aro.org>,
Johan Hovold <johan@...nel.org>, Konrad Dybcio <konrad.dybcio@...aro.org>
Subject: Re: [PATCH] arm64: dts: qcom: Add X1E80100 Samsung Galaxy Book4 Edge
On 04/08/2024 10:10, Marcus Glocker wrote:
> Hi,
>
> We recently added initial support in OpenBSD for the Samsung Galaxy
> Book4 Edge by below DTS diff.
Please write commig msgs matching Linux kernel style, see submitting
patches.
That's not a letter but explanation of what and why you are doing.
>
> - x1e80100-samsung-galaxy-book4-edge.dts:
> Is a copy of x1e80100-crd.dts, and then modified to our needs.
>
> - x1e80100.dtsi:
> Includes the UFSHCI peaces, which was basically pulled from
> sc7180.dtsi.
>
> Main stuff working:
>
> - UFSHCI.
> - Keyboard.
> - Touch-pad.
> - USB (as far tested).
>
> Main stuff not working yet:
>
> - Touch-screen: Pin 51, which mostly works on the other X1E80100
> models, is creating an interrupt storm on the Samsung Galaxy Book4
> Edge. Probing the other pins didn't showed success yet. Not sure at
> this point what the problem is.
>
> Regards,
> Marcus
>
>
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-samsung-galaxy-book4-edge.dts b/arch/arm64/boot/dts/qcom/x1e80100-samsung-galaxy-book4-edge.dts
> new file mode 100644
> index 000000000000..83751455211a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-samsung-galaxy-book4-edge.dts
> @@ -0,0 +1,986 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> +
> +#include "x1e80100.dtsi"
> +#include "x1e80100-pmics.dtsi"
> +
> +/ {
> + model = "Samsung Galaxy Book4 Edge";
> + compatible = "samsung,galaxy-book4-edge", "qcom,x1e80100";
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
> + chassis-type = "laptop";
> +
> + pmic-glink {
> + compatible = "qcom,x1e80100-pmic-glink",
> + "qcom,sm8550-pmic-glink",
> + "qcom,pmic-glink";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + orientation-gpios = <&tlmm 121 GPIO_ACTIVE_HIGH>,
> + <&tlmm 123 GPIO_ACTIVE_HIGH>,
> + <&tlmm 125 GPIO_ACTIVE_HIGH>;
> +
> + /* Left-side rear port */
> + connector@0 {
> + compatible = "usb-c-connector";
> + reg = <0>;
> + power-role = "dual";
> + data-role = "dual";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + pmic_glink_ss0_hs_in: endpoint {
> + remote-endpoint = <&usb_1_ss0_dwc3_hs>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + pmic_glink_ss0_ss_in: endpoint {
> + remote-endpoint = <&usb_1_ss0_qmpphy_out>;
> + };
> + };
> + };
> + };
> +
> + /* Left-side front port */
> + connector@1 {
> + compatible = "usb-c-connector";
> + reg = <1>;
> + power-role = "dual";
> + data-role = "dual";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + pmic_glink_ss1_hs_in: endpoint {
> + remote-endpoint = <&usb_1_ss1_dwc3_hs>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + pmic_glink_ss1_ss_in: endpoint {
> + remote-endpoint = <&usb_1_ss1_qmpphy_out>;
> + };
> + };
> + };
> + };
> +
> + /* Right-side port */
> + connector@2 {
> + compatible = "usb-c-connector";
> + reg = <2>;
> + power-role = "dual";
> + data-role = "dual";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + pmic_glink_ss2_hs_in: endpoint {
> + remote-endpoint = <&usb_1_ss2_dwc3_hs>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + pmic_glink_ss2_ss_in: endpoint {
> + remote-endpoint = <&usb_1_ss2_qmpphy_out>;
> + };
> + };
> + };
> + };
> + };
> +
> + reserved-memory {
> + linux,cma {
> + compatible = "shared-dma-pool";
> + size = <0x0 0x8000000>;
> + reusable;
> + linux,cma-default;
> + };
> + };
> +
> + sound {
> + compatible = "qcom,x1e80100-sndcard";
> + model = "X1E80100-CRD";
That's not a CRD board.
> + audio-routing = "WooferLeft IN", "WSA WSA_SPK1 OUT",
> + "TwitterLeft IN", "WSA WSA_SPK2 OUT",
> + "WooferRight IN", "WSA2 WSA_SPK2 OUT",
> + "TwitterRight IN", "WSA2 WSA_SPK2 OUT",
Is this correct?
> + "IN1_HPHL", "HPHL_OUT",
> + "IN2_HPHR", "HPHR_OUT",
> + "AMIC2", "MIC BIAS2",
And this?
> + "VA DMIC0", "MIC BIAS3",
> + "VA DMIC1", "MIC BIAS3",
> + "VA DMIC2", "MIC BIAS1",
> + "VA DMIC3", "MIC BIAS1",
> + "VA DMIC0", "VA MIC BIAS3",
> + "VA DMIC1", "VA MIC BIAS3",
> + "VA DMIC2", "VA MIC BIAS1",
> + "VA DMIC3", "VA MIC BIAS1",
> + "TX SWR_INPUT1", "ADC2_OUTPUT";
> +
> + wsa-dai-link {
> + link-name = "WSA Playback";
> +
> + cpu {
> + sound-dai = <&q6apmbedai WSA_CODEC_DMA_RX_0>;
> + };
> +
> + codec {
> + sound-dai = <&left_woofer>, <&left_tweeter>,
> + <&swr0 0>, <&lpass_wsamacro 0>,
> + <&right_woofer>, <&right_tweeter>,
> + <&swr3 0>, <&lpass_wsa2macro 0>;
> + };
> +
> + platform {
> + sound-dai = <&q6apm>;
> + };
> + };
> +
> + va-dai-link {
> + link-name = "VA Capture";
> +
> + cpu {
> + sound-dai = <&q6apmbedai VA_CODEC_DMA_TX_0>;
> + };
> +
> + codec {
> + sound-dai = <&lpass_vamacro 0>;
> + };
> +
> + platform {
> + sound-dai = <&q6apm>;
> + };
> + };
> + };
> +
> + vph_pwr: vph-pwr-regulator {
Use consistent naming, so either prefix or suffix, not both. E.g.
consistent regualtor-foo.
...
> +&i2c8 {
> + clock-frequency = <400000>;
> +
> + status = "disabled";
> +
> + touchscreen@5d {
> + compatible = "hid-over-i2c";
> + reg = <0x5d>;
> +
> + hid-descr-addr = <0x1>;
> + /*
> + * Pin 51 is creating an interrupt storm. Hence, choosing
> + * some other pin which just does nothing. But obviously,
No, you cannot add incorrect hardware description.
> + * the touchscreen won't work for now.
So disable the device.
> + */
> + //interrupts-extended = <&tlmm 51 IRQ_TYPE_LEVEL_LOW>;
Odd indentation.
...
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index 7bca5fcd7d52..62d8a91dd707 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -2878,6 +2878,77 @@ mmss_noc: interconnect@...0000 {
> #interconnect-cells = <2>;
> };
>
> + ufs_mem_hc: ufshc@...4000 {
ufs@
> + compatible = "qcom,x1e80100-ufshc", "qcom,ufshc",
Nope, pleasr test before.
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
Please run scripts/checkpatch.pl and fix reported warnings. Then please
run `scripts/checkpatch.pl --strict` and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.
Best regards,
Krzysztof
Powered by blists - more mailing lists