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

[Caleb Connolly <caleb@connolly.tech>]

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
>