Re: [PATCH v2 2/2] arm64: dts: qcom: oneplus-kebab: new device

[Frieder Hannenheim]

Hi Konrad, thanks for the review comments. Replies inline.

On 16.10.26 10:23, Konrad Dybcio wrote:
> On 10/15/26 2:32 PM, Frieder Hannenheim wrote:
> > Add initial device-tree for the OnePlus 8T. Downstream reference is [1].
> >
> > [1] https://github.com/LineageOS/android_kernel_oneplus_sm8250/blob/lineage-23.2/arch/arm64/boot/dts/vendor/oplus/kona-kebab-overlay.dts
> >
> > Signed-off-by: Frieder Hannenheim <git@xxxxxxxxxxxxxxx>
> > Co-developed-by: Robert Oscilowski <drgitx@xxxxxxxxx>
> > Signed-off-by: Robert Oscilowski <drgitx@xxxxxxxxx>
> > ---
> >   Documentation/devicetree/bindings/arm/qcom.yaml   |   1 +
> >   arch/arm64/boot/dts/qcom/Makefile                 |   1 +
> >   arch/arm64/boot/dts/qcom/sm8250-oneplus-kebab.dts | 129 ++++++++++++++++++++++
>
> Better. Please split off the dt-bindings changes to a separate patch,
> and put it first in the series.
>
> [...]
>
>
> > +&framebuffer {
> > +	width = <1080>;
> > +	height = <2400>;
> > +	stride = <(1080 * 4)>;
>
> Does the resolution differ between the two devices?

There's 4 devices: 8 (instantnoodle), 8T (kebab), 8 Pro (instantnoodlep)
and 9R (lemonades).

All except the 8 Pro have a 2400x1080 screen, the 8 Pro has a 3168x1440 
screen.

> > +};
> > +
> > +&i2c2 {
> > +	status = "okay";
> > +
> > +	pm8008: pmic@8 {
> > +		compatible = "qcom,pm8008";
>
> Is it not present on the other one?

The OnePlus 8 doesn't have this according to the downstream dt. >> +	 
reg = <0x8>;
> > +
> > +		interrupt-parent = <&tlmm>;
> > +		interrupts = <112 IRQ_TYPE_EDGE_RISING>;
> > +
> > +		reset-gpios = <&tlmm 93 GPIO_ACTIVE_LOW>;
> > +
> > +		vdd-l1-l2-supply = <&vreg_s8c_1p3>;
> > +		vdd-l3-l4-supply = <&vreg_bob>;
> > +		vdd-l5-supply = <&vreg_bob>;
> > +		vdd-l6-supply = <&vreg_bob>;
> > +		vdd-l7-supply = <&vreg_bob>;
> > +
> > +		pinctrl-names = "default";
> > +		pinctrl-0 = <&pm8008_default>, <&pm8008_interrupt>;
>
> nit:
>
> property-n
> property-names
>
> in this order, file-wide, please

Will fix

> [...]
>
> > +&i2c16 {
> > +	status = "okay";
> > +
> > +	fuel-gauge@55 {
> > +		compatible = "ti,bq28z610";
> > +		reg = <0x55>;
> > +		monitored-battery = <&battery>;
> > +	};
>
> Likewise, is this absent on the other phone?

Not absent but different. The 8T and 9R use two-cell batteries while the 
others only have one cell and use a different fuel gauge.

> > +};
> > +
> > +&tlmm {
> > +	pm8008_default: pm8008-default-state {
> > +		pins = "gpio93";
> > +		function = "gpio";
> > +		bias-pull-up;
> > +		output-high;
>
> Drop the output-high, the driver will set it for you

Will do.

Frieder