Re: [PATCH v9 4/4] arm64: dts: qcom: aim300: add AIM300 AIoT

[Dmitry Baryshkov]

On Mon, 3 Jun 2024 at 10:38, Tengfei Fan <quic_tengfan@xxxxxxxxxxx> wrote:
>
>
>
> On 5/31/2024 4:38 PM, Dmitry Baryshkov wrote:
> > On Fri, 31 May 2024 at 11:35, Tengfei Fan <quic_tengfan@xxxxxxxxxxx> wrote:
> >>
> >>
> >>
> >> On 5/29/2024 11:18 PM, Dmitry Baryshkov wrote:
> >>> On Wed, May 29, 2024 at 06:09:26PM +0800, Tengfei Fan wrote:
> >>>> Add AIM300 AIoT Carrier board DTS support, including usb, UART, PCIe,
> >>>> I2C functions support.
> >>>> Here is a diagram of AIM300 AIoT Carrie Board and SoM
> >>>>    +--------------------------------------------------+
> >>>>    |             AIM300 AIOT Carrier Board            |
> >>>>    |                                                  |
> >>>>    |           +-----------------+                    |
> >>>>    |power----->| Fixed regulator |---------+          |
> >>>>    |           +-----------------+         |          |
> >>>>    |                                       |          |
> >>>>    |                                       v VPH_PWR  |
> >>>>    | +----------------------------------------------+ |
> >>>>    | |                          AIM300 SOM |        | |
> >>>>    | |                                     |VPH_PWR | |
> >>>>    | |                                     v        | |
> >>>>    | |   +-------+       +--------+     +------+    | |
> >>>>    | |   | UFS   |       | QCS8550|     |PMIC  |    | |
> >>>>    | |   +-------+       +--------+     +------+    | |
> >>>>    | |                                              | |
> >>>>    | +----------------------------------------------+ |
> >>>>    |                                                  |
> >>>>    |                    +----+          +------+      |
> >>>>    |                    |USB |          | UART |      |
> >>>>    |                    +----+          +------+      |
> >>>>    +--------------------------------------------------+
> >>>>
> >>>> Co-developed-by: Qiang Yu <quic_qianyu@xxxxxxxxxxx>
> >>>> Signed-off-by: Qiang Yu <quic_qianyu@xxxxxxxxxxx>
> >>>> Co-developed-by: Ziyue Zhang <quic_ziyuzhan@xxxxxxxxxxx>
> >>>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@xxxxxxxxxxx>
> >>>> Signed-off-by: Tengfei Fan <quic_tengfan@xxxxxxxxxxx>
> >>>> ---
> >>>>    arch/arm64/boot/dts/qcom/Makefile             |   1 +
> >>>>    .../boot/dts/qcom/qcs8550-aim300-aiot.dts     | 322 ++++++++++++++++++
> >>>>    2 files changed, 323 insertions(+)
> >>>>    create mode 100644 arch/arm64/boot/dts/qcom/qcs8550-aim300-aiot.dts
> >>>
> >>> [trimmed]
> >>>
> >>>> +&remoteproc_adsp {
> >>>> +    firmware-name = "qcom/qcs8550/adsp.mbn",
> >>>> +                    "qcom/qcs8550/adsp_dtbs.elf";
> >>>
> >>> Please excuse me, I think I missed those on the previous run.
> >>>
> >>> adsp_dtb.mbn
> >>
> >> Currently, waht we have released is adsp_dtbs.elf. If we modify it to
> >> adsp_dtb.mbn, it may cause the ADSP functionality can not boot normally.
> >
> > Released where? linux-firmware doesn't have such a file. And the modem
> > partition most likely has a different path for it anyway.
>
> Firmware releases can be obtained from
> https://qpm-git.qualcomm.com/home2/git/qualcomm/qualcomm-linux-spf-1-0_test_device_public.git
> after users sign up for free accounts on both
> https://qpm-git.qualcomm.com and https://chipmaster2.qti.qualcomm.com.

I'm getting 403 when accessing qpm-git (both with my Linaro
credentials and with gmail ones).
If I try to git-clone the URL you've provided, I'm getting "Not found"
when using a gmail account and CURL error when using Linaro
createntials.

error: RPC failed; HTTP 302 curl 22 The requested URL returned error: 302

Not to mention that the URL wasn't mentioned anywhere beforehand. So I
can hardly call that 'released'

>
> >
> >>
> >>>
> >>>> +    status = "okay";
> >>>> +};
> >>>> +
> >>>> +&remoteproc_cdsp {
> >>>> +    firmware-name = "qcom/qcs8550/cdsp.mbn",
> >>>> +                    "qcom/qcs8550/cdsp_dtbs.elf";
> >>>
> >>> cdsp_dtb.mbn
> >>
> >> CDSP also as above ADSP.
> >>
> >>>
> >
> >>>> +
> >>>> +    te_active: te-active-state {
> >>>> +            pins = "gpio86";
> >>>> +            function = "mdp_vsync";
> >>>> +            drive-strength = <2>;
> >>>> +            bias-pull-down;
> >>>> +    };
> >>>> +
> >>>> +    te_suspend: te-suspend-state {
> >>>> +            pins = "gpio86"
> >>>> +            function = "mdp_vsync";
> >>>> +            drive-strength = <2>;
> >>>> +            bias-pull-down;
> >>>> +    };
> >>>
> >>> What is the difference between these two?
> >>
> >> TE pin needs to be pulled down for both active and suspend states. There
> >> is no difference.
> >
> > So why do you need two different states for it?
>
> Dividing into two different states can provide a clearer expression of
> whether the corresponging functionality is avtive or suspend.

How?

>
> We can also find similar settings in the other SM8550 and SM8650
> platform dts files, such as sm8550-qrd.dts and sm8650-qrd.dts.

Which means more items to cleanup.

See the discussion starting from
https://lore.kernel.org/linux-arm-msm/36f22383-79a3-427e-bf17-35ce2e1dd620@xxxxxxxxxx/

>
> [1] sm8550-qrd.dts:
> https://elixir.bootlin.com/linux/v6.9.3/source/arch/arm64/boot/dts/qcom/sm8550-qrd.dts#L1052
>
> [2] sm8650-qrd.dts:
> https://elixir.bootlin.com/linux/v6.9.3/source/arch/arm64/boot/dts/qcom/sm8650-qrd.dts#L1098
>
> >
> >
> >
> >
> >
>
> --
> Thx and BRs,
> Tengfei Fan



-- 
With best wishes
Dmitry