Re: [PATCH v4 1/6] dt-bindings: arm: qcom: Document QCM8550, QCS8550 SoC and board

[Dmitry Baryshkov]

On Mon, 5 Feb 2024 at 12:21, Tengfei Fan <quic_tengfan@xxxxxxxxxxx> wrote:
>
>
>
> On 2/5/2024 12:29 AM, Dmitry Baryshkov wrote:
> > On Fri, 19 Jan 2024 at 11:07, Tengfei Fan <quic_tengfan@xxxxxxxxxxx> wrote:
> >>
> >> Document QCM8550, QCS8550 SoC and the AIM300 AIoT board bindings.
> >> QCS8550 and QCM8550 processor combines powerful computing, extreme edge
> >> AI processing, Wi-Fi 7, and robust video and graphics for a wide range
> >> of use cases for the Internet of Things (IoT). QCS8550 is a QCS version
> >> for QCM8550. Modem RF only in QCM8550 but not in QCS8550.
> >> AIM300 Series is a highly optimized family of modules designed to
> >> support AIoT applications. The module is mounted onto Qualcomm AIoT
> >> carrier board to support verification, evaluation and development. It
> >> integrates QCS8550 SoC, UFS and PMIC chip etc.
> >> AIM stands for Artificial Intelligence Module. AIoT stands for AI IoT.
> >>
> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> >> Signed-off-by: Tengfei Fan <quic_tengfan@xxxxxxxxxxx>
> >> ---
> >>   Documentation/devicetree/bindings/arm/qcom.yaml | 11 +++++++++++
> >>   1 file changed, 11 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> >> index 1a5fb889a444..9cee874a8eae 100644
> >> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> >> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> >> @@ -49,8 +49,10 @@ description: |
> >>           msm8996
> >>           msm8998
> >>           qcs404
> >> +        qcs8550
> >>           qcm2290
> >>           qcm6490
> >> +        qcm8550
> >
> > Drop
>
> we want to introduce qcm8550 here.

What for. It either had to be introduced beforehand, or it should be
introduced when one adds support for an actual qcm8550 device.

> qcm8550.dtsi has been introduced and qcs8550-aim300.dtsi include
> qcm8550.dtsi directly.
>
> qcs8550 is a QCS version for qcm8550. qcs8550 is a sub-series of
> qcm8550. qcm8550 will be a firmware release series from qualcomm.

All three names refer to the different kinds of the same platform. The
base chip name is sm8550, so it is the last one. Other than that,
there is no need to include any SoC compatibles other than the actual
SoC name. See existing qrb devices for an inspiration.

>
> here is the qcm8550/qcs8550 detailed spec:
> https://docs.qualcomm.com/bundle/publicresource/87-61717-1_REV_A_Qualcomm_QCS8550_QCM8550_Processors_Product_Brief.pdf
>
> here is the sm8550 detailed spec:
> https://docs.qualcomm.com/bundle/publicresource/87-71408-1_REV_C_Snapdragon_8_gen_3_Mobile_Platform_Product_Brief.pdf

Can you please summarise the _actual_ difference between qcm8550,
qcs8550 and sm8550? Are they fully soft compatible? Soft compatible
except the modem? Pin compatible?

>
> >
> >>           qdu1000
> >>           qrb2210
> >>           qrb4210
> >> @@ -93,6 +95,7 @@ description: |
> >>     The 'board' element must be one of the following strings:
> >>
> >>           adp
> >> +        aim300-aiot
> >
> > We probably need to drop this list, it doesn't surve its purposes.
>
> I am a little confused, do you expect to just remove this "aim300-aiot"
> or do you want to introduce a new patch and remove the whole list?

If you were following the list, you would have seen the patch
reworking the bindings.

>
> >
> >>           cdp
> >>           dragonboard
> >>           idp
> >> @@ -904,6 +907,14 @@ properties:
> >>             - const: qcom,qcs404-evb
> >>             - const: qcom,qcs404
> >>
> >> +      - items:
> >> +          - enum:
> >> +              - qcom,qcs8550-aim300-aiot
> >> +          - const: qcom,qcs8550-aim300
> >> +          - const: qcom,qcs8550
> >> +          - const: qcom,qcm8550
> >
> > In the review comments for v3 you have been asked to add qcom,sm8550.
> > But not the qcom,qcm8550. I don't think that there is any need to
> > mention qcm8550 here.
>
> qcm8550 and sm8550 are different, they have different firmware release.
>
> AIM300 AIoT board depend on qcs8550, qcs8550 is a QCS version for
> qcm8550. Modem RF only in qcm8550 but not in qcs8550.

There are no 'dependecies' here. The thing is about declaring compatibility.
In my opinion, the qcm8550 is an unnecesary part of the equation. You
declare compatibility with the board itself,  with the SoM, with the
actual SoC and with the base of the series. Anybody caring for the
difference between QCM, QCS and SM will have to check for both
qcom,qcs8550 and qcom,qcm8550 anyway, as there are differences on the
modem side.

> >> +          - const: qcom,sm8550
> >> +
> >>         - items:
> >>             - enum:
> >>                 - qcom,sa8155p-adp
> >> --

-- 
With best wishes
Dmitry