On Mon, May 06, 2024 at 06:14:10PM -0500, Bjorn Andersson wrote:
> On Fri, Apr 26, 2024 at 07:54:42PM GMT, Prahlad Valluru wrote:
> > From: Venkata Prahlad Valluru <quic_vvalluru@xxxxxxxxxxx>
> >
>
> Please don't thread new versions off existing version. b4 helps you with
> getting these things right, please check go/upstream for more details.
My internal gitconfig is not configured correctly. Fixed in v3.
>
> > Enable lt9611uxc bridge for qcs6490 rb3 gen2 platform.
> >
>
> Even if it's clear what this is, I would prefer if you described the
> hardware a little bit in your commit message.
> "Rb3Gen2 has a HDMI connector, connected to DSI via a LT on i2cX.... reset and
> irq pins comes from x and y. Describe this."
>
Agreed. Updated the commit text to include bridge details.
> > Signed-off-by: Prahlad Valluru <quic_vvalluru@xxxxxxxxxxx>
> > ---
> > v2: Addressed dtschema errors
> > - Fixed lt9611-irq
> > - vdd-supply error to be ignored, as it is connected to
> > input supply directly, on rb3gen2
> > ---
> > arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts | 86 ++++++++++++++++++++
> > 1 file changed, 86 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > index a085ff5b5fb2..c14d4a4bb3ce 100644
> > --- a/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > +++ b/arch/arm64/boot/dts/qcom/qcs6490-rb3gen2.dts
> > @@ -51,6 +51,18 @@
> > };
> > };
> > };
> > +
>
> There's a stray tab here, please run checkpatch --strict on your
> patches.
Fixed the checkpatch issues.
>
> > + hdmi-connector {
> > + compatible = "hdmi-connector";
> > + label = "HDMI";
> > + type = "a";
> > +
> > + port {
> > + hdmi_con: endpoint {
> > + remote-endpoint = <<9611_out>;
> > + };
> > + };
> > + };
> >
> > reserved-memory {
> > xbl_mem: xbl@80700000 {
> > @@ -530,6 +542,45 @@
> [..]
> > @@ -602,6 +653,21 @@
> > status = "okay";
> > };
> >
> > +&mdss_dsi {
>
> We want to keep nodes sorted (by address if applicable, otherwise
> alphabetically on node names and on labels). "mdss_dsi" < "mdss_edp".
>
> So please move this up where it belongs.
>
Moved the dsi nodes on top of edp. Didn't realise this.
> > + vdda-supply = <&vreg_l6b_1p2>;
> > + status = "okay";
> > +};
> > +
> > +&mdss_dsi0_out {
> > + remote-endpoint = <<9611_a>;
> > + data-lanes = <0 1 2 3>;
> > +};
> > +
> > +&mdss_dsi_phy {
> > + vdds-supply = <&vreg_l10c_0p88>;
> > + status = "okay";
> > +};
> > +
> > &qupv3_id_0 {
> > status = "okay";
> > };
> > @@ -711,3 +777,23 @@
> > function = "gpio";
> > bias-disable;
> > };
> > +
> > +&pm7250b_gpios {
> > + lt9611_rst_pin: lt9611-rst-state {
> > + pins = "gpio2";
> > + function = "normal";
> > +
> > + output-high;
> > + input-disable;
> > + power-source = <0>;
> > + };
> > +};
> > +
> > +&tlmm {
> > + lt9611_irq_pin: lt9611-irq-state {
> > + pins = "gpio24";
> > + function = "gpio";
> > + drive-strength = <8>;
>
> I'd expect a 2 here, so please document why this is 8.
Accepted. I dont see any reason to have drive strength 8.
>
> Regards,
> Bjorn
>
> > + bias-disable;
> > + };
> > +};
> > --
> > 2.17.1
> >
