linux-kernel - Re: [PATCH v3 1/2] arm64: dts: sdm845: add oneplus6/6t devices

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4db0807e-c33e-5913-1818-1fc055e35acb@connolly.tech>
Date:   Thu, 14 Jan 2021 20:10:39 +0000
From:   Caleb Connolly <caleb@...nolly.tech>
To:     Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>
Cc:     ~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] arm64: dts: sdm845: add oneplus6/6t devices


On 14/01/2021 7:19 pm, Konrad Dybcio wrote:
> Hi!
>
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-db845c.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-mtp.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-xiaomi-beryllium.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-oneplus-enchilada.dtb
>> +dtb-$(CONFIG_ARCH_QCOM)	+= sdm845-oneplus-fajita.dtb
>>   dtb-$(CONFIG_ARCH_QCOM)	+= sdm850-lenovo-yoga-c630.dtb
> Please sort.
>
>> +/ {
>> +
> Redundant newline.
>> +	gpio-keys {
>> +		compatible = "gpio-keys";
>> +		label = "Volume keys";
>> +		autorepeat;
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&volume_down_gpio &volume_up_gpio>;
>> +
>> +		vol-down {
>> +			label = "Volume down";
>> +			linux,code = <KEY_VOLUMEDOWN>;
>> +			gpios = <&pm8998_gpio 5 GPIO_ACTIVE_LOW>;
>> +			debounce-interval = <15>;
>> +		};
>> +
>> +		vol-up {
>> +			label = "Volume up";
>> +			linux,code = <KEY_VOLUMEUP>;
>> +			gpios = <&pm8998_gpio 6 GPIO_ACTIVE_LOW>;
>> +			debounce-interval = <15>;
>> +		};
>> +	};
> Perhaps gpio-keys could come before the fixed regulators to keep it a bit more sane/sorted?
>> +&adsp_pas {
> This should come before apps_rsc alphabetically.
>> +// Prevent garbage data on bluetooth UART lines
> Please use C-style comments (/* thing */).
>> +&usb_1 {
>> +	status = "okay";
>> +
>> +	/*
>> +	 * disable USB3 clock requirement as the device only supports
>> +	 * USB2.
>> +	 */
>> +	qcom,select-utmi-as-pipe-clk;
>> +};
>> +
>> +&usb_1_dwc3 {
>> +	/*
>> +	 * We don't have the capability to switch modes yet.
>> +	 */
>> +	dr_mode = "peripheral";
>> +
>> +	/* fastest mode for USB 2 */
>> +	maximum-speed = "high-speed";
>> +
>> +	/* Remove USB3 phy as it's unused on this device. */
>> +	phys = <&usb_1_hsphy>;
>> +	phy-names = "usb2-phy";
>> +};
> Are you sure you can't get USB3 in host mode? Perhaps it's not worth discarding it upfront, as some boards can do that, definitely do check!
The device definitely doesn't support USB3, although downstream does 
leave the USB3 phy enabled the hardware doesn't support it. So it made 
sense to disable it here.
>> \ No newline at end of file
> Please add a newline.
>
>
> The DTS files look correct, but don't you need at least one of msm-id, board-id, pmic-id? I'm not exactly sure what OnePlus does, but I can imagine they would require such properties..

OnePlus' bootloader doesn't seem to care about these values so I left 
them out for simplicity.

Cheers,

Caleb

>
> Konrad
>