| Message ID | 20240904-qcs8300_initial_dtsi-v1-2-d0ea9afdc007@quicinc.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | Add initial support for QCS8300 | expand |
On 04/09/2024 10:33, Jingyi Wang wrote: > Add support for PIL loading on ADSP, CDSP and GPDSP on QCS8300 > platform. > > Co-developed-by: Xin Liu <quic_liuxin@quicinc.com> > Signed-off-by: Xin Liu <quic_liuxin@quicinc.com> > Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com> > --- > drivers/remoteproc/qcom_q6v5_pas.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c > index ef82835e98a4..f92ccd4921b7 100644 > --- a/drivers/remoteproc/qcom_q6v5_pas.c > +++ b/drivers/remoteproc/qcom_q6v5_pas.c > @@ -1416,6 +1416,9 @@ static const struct of_device_id adsp_of_match[] = { > { .compatible = "qcom,qcs404-adsp-pas", .data = &adsp_resource_init }, > { .compatible = "qcom,qcs404-cdsp-pas", .data = &cdsp_resource_init }, > { .compatible = "qcom,qcs404-wcss-pas", .data = &wcss_resource_init }, > + { .compatible = "qcom,qcs8300-adsp-pas", .data = &sa8775p_adsp_resource}, > + { .compatible = "qcom,qcs8300-cdsp-pas", .data = &sa8775p_cdsp0_resource}, > + { .compatible = "qcom,qcs8300-gpdsp-pas", .data = &sa8775p_gpdsp0_resource}, What's the point of this? You have entire commit msg to explain such weird duplication. Otherwise sorry, don't duplicate unnecessarily. Devices are compatible, aren't they? Best regards, Krzysztof
On 9/4/2024 5:36 PM, Krzysztof Kozlowski wrote: > On 04/09/2024 10:33, Jingyi Wang wrote: >> Add support for PIL loading on ADSP, CDSP and GPDSP on QCS8300 >> platform. >> >> Co-developed-by: Xin Liu <quic_liuxin@quicinc.com> >> Signed-off-by: Xin Liu <quic_liuxin@quicinc.com> >> Signed-off-by: Jingyi Wang <quic_jingyw@quicinc.com> >> --- >> drivers/remoteproc/qcom_q6v5_pas.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c >> index ef82835e98a4..f92ccd4921b7 100644 >> --- a/drivers/remoteproc/qcom_q6v5_pas.c >> +++ b/drivers/remoteproc/qcom_q6v5_pas.c >> @@ -1416,6 +1416,9 @@ static const struct of_device_id adsp_of_match[] = { >> { .compatible = "qcom,qcs404-adsp-pas", .data = &adsp_resource_init }, >> { .compatible = "qcom,qcs404-cdsp-pas", .data = &cdsp_resource_init }, >> { .compatible = "qcom,qcs404-wcss-pas", .data = &wcss_resource_init }, >> + { .compatible = "qcom,qcs8300-adsp-pas", .data = &sa8775p_adsp_resource}, >> + { .compatible = "qcom,qcs8300-cdsp-pas", .data = &sa8775p_cdsp0_resource}, >> + { .compatible = "qcom,qcs8300-gpdsp-pas", .data = &sa8775p_gpdsp0_resource}, > > What's the point of this? You have entire commit msg to explain such > weird duplication. Otherwise sorry, don't duplicate unnecessarily. > Devices are compatible, aren't they? > > Best regards, > Krzysztof > > I will drop this, could you please help us to understand what is the correct way to deal such situation, do we need to update the yaml and add qcs8300 bindings or just reference to sa8775p bindings in the device tree? Thanks, Jingyi
On 05/09/2024 06:30, Jingyi Wang wrote: >>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c >>> index ef82835e98a4..f92ccd4921b7 100644 >>> --- a/drivers/remoteproc/qcom_q6v5_pas.c >>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c >>> @@ -1416,6 +1416,9 @@ static const struct of_device_id adsp_of_match[] = { >>> { .compatible = "qcom,qcs404-adsp-pas", .data = &adsp_resource_init }, >>> { .compatible = "qcom,qcs404-cdsp-pas", .data = &cdsp_resource_init }, >>> { .compatible = "qcom,qcs404-wcss-pas", .data = &wcss_resource_init }, >>> + { .compatible = "qcom,qcs8300-adsp-pas", .data = &sa8775p_adsp_resource}, >>> + { .compatible = "qcom,qcs8300-cdsp-pas", .data = &sa8775p_cdsp0_resource}, >>> + { .compatible = "qcom,qcs8300-gpdsp-pas", .data = &sa8775p_gpdsp0_resource}, >> >> What's the point of this? You have entire commit msg to explain such >> weird duplication. Otherwise sorry, don't duplicate unnecessarily. >> Devices are compatible, aren't they? >> >> Best regards, >> Krzysztof >> >> > I will drop this, could you please help us to understand what is the correct way to > deal such situation, do we need to update the yaml and add qcs8300 bindings or just > reference to sa8775p bindings in the device tree? Above diff hunk suggests that devices are compatible, so should be made compatible in the bindings (use fallback). There are plenty examples of this for all Qualcomm devices. Best regards, Krzysztof
On 9/5/2024 2:24 PM, Krzysztof Kozlowski wrote: > On 05/09/2024 06:30, Jingyi Wang wrote: >>>> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c >>>> index ef82835e98a4..f92ccd4921b7 100644 >>>> --- a/drivers/remoteproc/qcom_q6v5_pas.c >>>> +++ b/drivers/remoteproc/qcom_q6v5_pas.c >>>> @@ -1416,6 +1416,9 @@ static const struct of_device_id adsp_of_match[] = { >>>> { .compatible = "qcom,qcs404-adsp-pas", .data = &adsp_resource_init }, >>>> { .compatible = "qcom,qcs404-cdsp-pas", .data = &cdsp_resource_init }, >>>> { .compatible = "qcom,qcs404-wcss-pas", .data = &wcss_resource_init }, >>>> + { .compatible = "qcom,qcs8300-adsp-pas", .data = &sa8775p_adsp_resource}, >>>> + { .compatible = "qcom,qcs8300-cdsp-pas", .data = &sa8775p_cdsp0_resource}, >>>> + { .compatible = "qcom,qcs8300-gpdsp-pas", .data = &sa8775p_gpdsp0_resource}, >>> >>> What's the point of this? You have entire commit msg to explain such >>> weird duplication. Otherwise sorry, don't duplicate unnecessarily. >>> Devices are compatible, aren't they? >>> >>> Best regards, >>> Krzysztof >>> >>> >> I will drop this, could you please help us to understand what is the correct way to >> deal such situation, do we need to update the yaml and add qcs8300 bindings or just >> reference to sa8775p bindings in the device tree? > > Above diff hunk suggests that devices are compatible, so should be made > compatible in the bindings (use fallback). There are plenty examples of > this for all Qualcomm devices. > > Best regards, > Krzysztof > The usage of binding seems inconsistent across different Qualcomm drivers. Could you please confirm that when you mentioned "use fallback", do you mean binding like this? - items: - enum: - qcom,sm8550-sndcard - qcom,sm8650-sndcard - const: qcom,sm8450-sndcard https://www.kernel.org/doc/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml Thanks, Jingyi
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c index ef82835e98a4..f92ccd4921b7 100644 --- a/drivers/remoteproc/qcom_q6v5_pas.c +++ b/drivers/remoteproc/qcom_q6v5_pas.c @@ -1416,6 +1416,9 @@ static const struct of_device_id adsp_of_match[] = { { .compatible = "qcom,qcs404-adsp-pas", .data = &adsp_resource_init }, { .compatible = "qcom,qcs404-cdsp-pas", .data = &cdsp_resource_init }, { .compatible = "qcom,qcs404-wcss-pas", .data = &wcss_resource_init }, + { .compatible = "qcom,qcs8300-adsp-pas", .data = &sa8775p_adsp_resource}, + { .compatible = "qcom,qcs8300-cdsp-pas", .data = &sa8775p_cdsp0_resource}, + { .compatible = "qcom,qcs8300-gpdsp-pas", .data = &sa8775p_gpdsp0_resource}, { .compatible = "qcom,sa8775p-adsp-pas", .data = &sa8775p_adsp_resource}, { .compatible = "qcom,sa8775p-cdsp0-pas", .data = &sa8775p_cdsp0_resource}, { .compatible = "qcom,sa8775p-cdsp1-pas", .data = &sa8775p_cdsp1_resource},