On 30.06.2024 8:36 PM, Caleb Connolly wrote:
> Initial support for USB, UFS, touchscreen, panel, wifi, and bluetooth.
>
> Co-developed-by: Frieder Hannenheim <frieder.hannenheim@xxxxxxxxx>
> Signed-off-by: Frieder Hannenheim <frieder.hannenheim@xxxxxxxxx>
> Signed-off-by: Caleb Connolly <caleb@xxxxxxxxxxxxxxxx>
> ---
[...]
> +/delete-node/ &spss_mem;
> +/delete-node/ &cdsp_secure_heap;
> +
odd double newline
> +
> +/ {
[...]
> +
> + framebuffer@9c000000 {
> + reg = <0 0x9c000000 0 0x2300000>;
0x0 for consistency
> + no-map;
> + };
> + };
> +
> + panel_avdd_5p5: regulator-panel-avdd {
> + compatible = "regulator-fixed";
> + regulator-name = "panel_avdd_5p5";
> + regulator-min-microvolt = <5500000>;
> + regulator-max-microvolt = <5500000>;
Please unsqash these properties like in e.g. x1e80100-crd
> + regulator-enable-ramp-delay = <233>;
> + gpio = <&tlmm 61 GPIO_ACTIVE_HIGH>;
> + regulator-boot-on;
> + pinctrl-names = "default";
> + pinctrl-0 = <&panel_avdd_pins>;
property-n
property-names
(theres more occurences in this patch)
> + vreg_l11c_3p3: ldo11 {
> + regulator-name = "vreg_l11c_3p3";
> + regulator-min-microvolt = <2900000>;
> + regulator-max-microvolt = <3304000>;
> + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> + regulator-boot-on;
> + /* FIXME: we don't yet support power cycling the panel */
> + //regulator-always-on;
so should this be in or out?
[...]
> + /*
> + * Pixelworks Iris 5 @ 26 (i3c) or 22 (i2c)
> + * This is a co-processor for the display which needs to be
> + * initialized along with the panel.
> + */
yikes
> +};
> +
> +&i2c15 {
> + status = "okay";
> +
> + typec-mux@42 {
> + compatible = "fcs,fsa4480";
> + reg = <0x42>;
> +
> + vcc-supply = <&vreg_s4a_1p8>;
> +
> + orientation-switch;
> +
> + /* Currently unsupported */
> + status = "disabled";
Any particular problems with it?
[...]
> + /*
> + * FIXME: There is a bug somewhere in the display stack and it isn't
> + * possible to get the panel to a working state after toggling reset.
> + * At best it just shows one or more vertical red lines. So for now
> + * let's skip the reset GPIO.
> + */
> + // reset-gpios = <&tlmm 75 GPIO_ACTIVE_LOW>;
c++-style comments used not to be cool.. not sure what's the current policy
> +
> + pinctrl-0 = <&panel_reset_pins &panel_vsync_pins &panel_vout_pins>;
should panel_vout_pins be modeled as a regulator?
[...]
> +&pm8150_gpios {
> + /*
> + * These are marked as reserved in downstream
> + * with no description, without schematics we
> + * don't know what the deal is here.
> + */
> + gpio-reserved-ranges = <2 1>, <4 2>, <8 1>;
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
/* pm8150 has 10 GPIOs with holes on 2, 5, 7 and 8 */
{ .compatible = "qcom,pm8150-gpio", .data = (void *) 10 },
[...]
> +&pon_resin {
> + status = "okay";
> +
> + linux,code = <KEY_VOLUMEDOWN>;
status should go last
[...]
> +&tlmm {
> + gpio-reserved-ranges = <28 4>, <40 4>;
Any chance you'd know what they're for?
[...]
> +&usb_1_dwc3 {
> + dr_mode = "peripheral";
[...]
> +
> + touchscreen@4b {
> + compatible = "syna,s3908";
> + reg = <0x4B>;
lowercase hex
> +
> + pinctrl-0 = <&touch_irq_active &touch_rst_active>;
> + pinctrl-names = "default";
> +
> + interrupts-extended = <&tlmm 39 0x2008>;
https://lore.kernel.org/linux-arm-msm/20240605160032.150587-1-krzysztof.kozlowski@xxxxxxxxxx/
Konrad
