| Message ID | 20230601-topic-sm8550-upstream-type-c-v3-5-22c9973012b6@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 7daada8630eb49cd841c0fd022fadd52da100bce |
| Headers | show |
| Series | arm64: qcom: add Type-C Altmode support | expand |
On 13.06.2023 09:55, Neil Armstrong wrote: > Altmode is also supported for SM8550, allow it. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > drivers/soc/qcom/pmic_glink.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c > index c87056769ebd..8af06bdc6f5a 100644 > --- a/drivers/soc/qcom/pmic_glink.c > +++ b/drivers/soc/qcom/pmic_glink.c > @@ -342,13 +342,9 @@ static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT > BIT(PMIC_GLINK_CLIENT_ALTMODE) | > BIT(PMIC_GLINK_CLIENT_UCSI); > > -/* Do not handle altmode for now on those platforms */ > -static const unsigned long pmic_glink_sm8550_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) | > - BIT(PMIC_GLINK_CLIENT_UCSI); > - > static const struct of_device_id pmic_glink_of_match[] = { > { .compatible = "qcom,sm8450-pmic-glink", .data = &pmic_glink_sm8450_client_mask }, > - { .compatible = "qcom,sm8550-pmic-glink", .data = &pmic_glink_sm8550_client_mask }, > + { .compatible = "qcom,sm8550-pmic-glink", .data = &pmic_glink_sm8450_client_mask }, > { .compatible = "qcom,pmic-glink" }, > {} > }; >
On 13/06/2023 09:55, Neil Armstrong wrote: > Altmode is also supported for SM8550, allow it. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/soc/qcom/pmic_glink.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c > index c87056769ebd..8af06bdc6f5a 100644 > --- a/drivers/soc/qcom/pmic_glink.c > +++ b/drivers/soc/qcom/pmic_glink.c > @@ -342,13 +342,9 @@ static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT > BIT(PMIC_GLINK_CLIENT_ALTMODE) | > BIT(PMIC_GLINK_CLIENT_UCSI); > > -/* Do not handle altmode for now on those platforms */ > -static const unsigned long pmic_glink_sm8550_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) | > - BIT(PMIC_GLINK_CLIENT_UCSI); > - > static const struct of_device_id pmic_glink_of_match[] = { > { .compatible = "qcom,sm8450-pmic-glink", .data = &pmic_glink_sm8450_client_mask }, Orientation and maybe all of the USB-related properties do not look like sm8450 specific, but PM8350B. That's where CC_OUT pin is. I don't think we represented this correctly, but rather copy-pasted downstream solution... Best regards, Krzysztof
On 13/06/2023 13:54, Krzysztof Kozlowski wrote: > On 13/06/2023 09:55, Neil Armstrong wrote: >> Altmode is also supported for SM8550, allow it. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- >> drivers/soc/qcom/pmic_glink.c | 6 +----- >> 1 file changed, 1 insertion(+), 5 deletions(-) >> >> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c >> index c87056769ebd..8af06bdc6f5a 100644 >> --- a/drivers/soc/qcom/pmic_glink.c >> +++ b/drivers/soc/qcom/pmic_glink.c >> @@ -342,13 +342,9 @@ static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT >> BIT(PMIC_GLINK_CLIENT_ALTMODE) | >> BIT(PMIC_GLINK_CLIENT_UCSI); >> >> -/* Do not handle altmode for now on those platforms */ >> -static const unsigned long pmic_glink_sm8550_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) | >> - BIT(PMIC_GLINK_CLIENT_UCSI); >> - >> static const struct of_device_id pmic_glink_of_match[] = { >> { .compatible = "qcom,sm8450-pmic-glink", .data = &pmic_glink_sm8450_client_mask }, > > Orientation and maybe all of the USB-related properties do not look like > sm8450 specific, but PM8350B. That's where CC_OUT pin is. I don't think > we represented this correctly, but rather copy-pasted downstream solution... We do not interact directly with PM8350B or PM8550B but with pmic_glink who does the work work for use, and this is platform specific. Neil > > Best regards, > Krzysztof >
On 13.06.2023 15:43, Neil Armstrong wrote: > On 13/06/2023 13:54, Krzysztof Kozlowski wrote: >> On 13/06/2023 09:55, Neil Armstrong wrote: >>> Altmode is also supported for SM8550, allow it. >>> >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>> --- >>> drivers/soc/qcom/pmic_glink.c | 6 +----- >>> 1 file changed, 1 insertion(+), 5 deletions(-) >>> >>> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c >>> index c87056769ebd..8af06bdc6f5a 100644 >>> --- a/drivers/soc/qcom/pmic_glink.c >>> +++ b/drivers/soc/qcom/pmic_glink.c >>> @@ -342,13 +342,9 @@ static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT >>> BIT(PMIC_GLINK_CLIENT_ALTMODE) | >>> BIT(PMIC_GLINK_CLIENT_UCSI); >>> -/* Do not handle altmode for now on those platforms */ >>> -static const unsigned long pmic_glink_sm8550_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) | >>> - BIT(PMIC_GLINK_CLIENT_UCSI); >>> - >>> static const struct of_device_id pmic_glink_of_match[] = { >>> { .compatible = "qcom,sm8450-pmic-glink", .data = &pmic_glink_sm8450_client_mask }, >> >> Orientation and maybe all of the USB-related properties do not look like >> sm8450 specific, but PM8350B. That's where CC_OUT pin is. I don't think >> we represented this correctly, but rather copy-pasted downstream solution... > > We do not interact directly with PM8350B or PM8550B but with pmic_glink who > does the work work for use, and this is platform specific. Yep, pmic_glink is totally a firmware construct and not all platforms with pm8350b or so implement it. Konrad > > Neil > >> >> Best regards, >> Krzysztof >> >
On 13/06/2023 15:58, Konrad Dybcio wrote: > > > On 13.06.2023 15:43, Neil Armstrong wrote: >> On 13/06/2023 13:54, Krzysztof Kozlowski wrote: >>> On 13/06/2023 09:55, Neil Armstrong wrote: >>>> Altmode is also supported for SM8550, allow it. >>>> >>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>> --- >>>> drivers/soc/qcom/pmic_glink.c | 6 +----- >>>> 1 file changed, 1 insertion(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c >>>> index c87056769ebd..8af06bdc6f5a 100644 >>>> --- a/drivers/soc/qcom/pmic_glink.c >>>> +++ b/drivers/soc/qcom/pmic_glink.c >>>> @@ -342,13 +342,9 @@ static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT >>>> BIT(PMIC_GLINK_CLIENT_ALTMODE) | >>>> BIT(PMIC_GLINK_CLIENT_UCSI); >>>> -/* Do not handle altmode for now on those platforms */ >>>> -static const unsigned long pmic_glink_sm8550_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) | >>>> - BIT(PMIC_GLINK_CLIENT_UCSI); >>>> - >>>> static const struct of_device_id pmic_glink_of_match[] = { >>>> { .compatible = "qcom,sm8450-pmic-glink", .data = &pmic_glink_sm8450_client_mask }, >>> >>> Orientation and maybe all of the USB-related properties do not look like >>> sm8450 specific, but PM8350B. That's where CC_OUT pin is. I don't think >>> we represented this correctly, but rather copy-pasted downstream solution... >> >> We do not interact directly with PM8350B or PM8550B but with pmic_glink who >> does the work work for use, and this is platform specific. > Yep, pmic_glink is totally a firmware construct and not all platforms with > pm8350b or so implement it. The drivers interact like this, but there is no pmic glink on schematics but PM8350B and PM8550 and DT represents here the hardware, thus the schematics, not your chosen way of interface. This looks like a copy of downstream choice which is in most cases just wrong choice. Best regards, Krzysztof
diff --git a/drivers/soc/qcom/pmic_glink.c b/drivers/soc/qcom/pmic_glink.c index c87056769ebd..8af06bdc6f5a 100644 --- a/drivers/soc/qcom/pmic_glink.c +++ b/drivers/soc/qcom/pmic_glink.c @@ -342,13 +342,9 @@ static const unsigned long pmic_glink_sm8450_client_mask = BIT(PMIC_GLINK_CLIENT BIT(PMIC_GLINK_CLIENT_ALTMODE) | BIT(PMIC_GLINK_CLIENT_UCSI); -/* Do not handle altmode for now on those platforms */ -static const unsigned long pmic_glink_sm8550_client_mask = BIT(PMIC_GLINK_CLIENT_BATT) | - BIT(PMIC_GLINK_CLIENT_UCSI); - static const struct of_device_id pmic_glink_of_match[] = { { .compatible = "qcom,sm8450-pmic-glink", .data = &pmic_glink_sm8450_client_mask }, - { .compatible = "qcom,sm8550-pmic-glink", .data = &pmic_glink_sm8550_client_mask }, + { .compatible = "qcom,sm8550-pmic-glink", .data = &pmic_glink_sm8450_client_mask }, { .compatible = "qcom,pmic-glink" }, {} };
Altmode is also supported for SM8550, allow it. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- drivers/soc/qcom/pmic_glink.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)