Re: [PATCH 2/2] arm64: dts: qcom: add Xiaomi Mi 8 (dipper)

On 18/08/2026 11:27, Konrad Dybcio wrote:
> On 8/9/26 6:48 PM, David Heidelberg via B4 Relay wrote:
>> From: David Heidelberg <[email protected]>
>>
>> Initial submission for one member of the prime Xiaomi Mi 8 family.
>> Mi 8 Pro (equuleus) and Mi 8 Explorer (ursa) are nearly identical.
>> Known differences are battery capacity and fingerprint sensor placement.
>> This submission includes basic abstraction for other developers to be
>> able easily extend it to support other variants.
> 
> I had one of those. IIRC the Pro had a 3d IR scanner (vide face id)
> whereas the base one had a normal IR cam
> 
> [...]
> 
>> +	reserved-memory {
>> +		adsp_mem: memory@8c500000 {
> 
> adsp@ etc. and drop labels where unused
> 
> [...]
> 
>> +		linux,cma {
>> +			compatible = "shared-dma-pool";
>> +			alloc-ranges = <0 0x00000000 0 0xffffffff>;
>> +			reusable;
>> +			alignment = <0 0x400000>;
>> +			size = <0 0x2c00000>;
>> +			linux,cma-default;
>> +		};
> 
> Is this necessary?
> 
>> +	};
>> +
>> +	vph_pwr: regulator-vph-pwr {
> 
> This is out of order
> 
> [...]
> 
> 
>> +&adsp_pas {
>> +	firmware-name = "qcom/sdm845/Xiaomi/dipper/adsp.mbn";
> 
> Would you know whether the FWs are common across the SKUs?

I don't, I have only one device (the dipper) available.

I'll see if I quickly figure out, if not, as no-one else having Mi8 family 
volunteer to test, I would move it to dipper specific for now and can be moved 
to common later.

> 
> And more importantly, regulator settings? FWIW it's easiest to compare
> compiled DTBs (or even better, ones pulled from sysfs on a running
> system)
> 
> [...]
> 
>> +&tlmm {
>> +	gpio-reserved-ranges = < 0 4>, /* SPI (Goodix Fingerprint) */
> 
> odd space after <

I call it alignment, but sure :)>
>> +			       <81 4>; /* SPI (Synaptics Fingerprint) */
> 
> No NFC eSE reservation?

It seems that vendor used different combinations of Goodix / Synaptics across 
these ranges. Not even depending on the model. Same model can have different 
wiring, but always honoring position 0:4, 81:4 as far goes my understanding. >
> [...]
> 
>> +&usb_1_dwc3 {
>> +	/* Until we have Type C hooked up we'll force this as peripheral. */
> 
> Should be fairly trivial on 845, no?
> 
>> +	dr_mode = "peripheral";
>> +
>> +	maximum-speed = "high-speed";
>> +	phys = <&usb_1_hsphy>;
>> +	phy-names = "usb2-phy";
> 
> No SS?

Haven't thought about it, for now my goal is upstream basic telnet/ssh + FB. 
Then display and touch - e.g. minimal u-boot and kernel development features and 
then bring the rest.

> 
> [...]
> 
>> +	bluetooth {
>> +		compatible = "qcom,wcn3990-bt";
>> +
>> +		/*
>> +		 * This path is relative to the qca/
>> +		 * subdir under lib/firmware.
>> +		 */
> 
> Let's drop this comment, not very useful

Sure.

Then I'll also drop these from whole sdm845 in follow-up commit.

I'll address also the rest of the comments in the next version :)

Thanks for the review
David>
> Konrad