Messages in this thread | | | Date | Fri, 3 Jun 2022 16:33:12 +0530 | Subject | Re: [PATCH v2 2/2] pinctrl: qcom: sc7280: Add lpi pinctrl variant data for adsp based targets | From | Srinivasa Rao Mandadapu <> |
| |
On 6/3/2022 3:58 PM, Linus Walleij wrote: Thanks for Your time and valuable inputs Linus!!! > On Wed, Jun 1, 2022 at 12:30 PM Srinivasa Rao Mandadapu > <quic_srivasam@quicinc.com> wrote: > > So one way to just use a propert and avoid more compatible strings: > >> Add compatible string and lpi pinctrl variant data structure for adsp >> enabled sc7280 platforms. >> This variant data structure rnd compatible name required for >> distingushing ADSP based platforms and ADSP bypass platforms. >> In case of ADSP enabled platforms, where audio is routed through ADSP >> macro and decodec GDSC Switches are triggered as clocks by pinctrl >> driver and ADSP firmware controls them. So It's mandatory to enable >> them in ADSP based solutions. >> In case of ADSP bypass platforms clock voting is optional as these macro >> and dcodec GDSC switches are maintained as power domains and operated from >> lpass clock drivers. >> >> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com> >> Co-developed-by: Venkata Prasad Potturu <quic_potturu@quicinc.com> >> Signed-off-by: Venkata Prasad Potturu <quic_potturu@quicinc.com> >> --- >> drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c >> index 2add9a4..c9e85d9 100644 >> --- a/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c >> +++ b/drivers/pinctrl/qcom/pinctrl-sc7280-lpass-lpi.c >> @@ -134,6 +134,16 @@ static const struct lpi_function sc7280_functions[] = { >> LPI_FUNCTION(wsa_swr_data), >> }; >> >> +static const struct lpi_pinctrl_variant_data sc7280_adsp_lpi_data = { > Remove static and export this struct in drivers/pinctrl/qcom/pinctrl-lpass-lpi.h We can remove entire data structure if my below approach is okay. > >> + .pins = sc7280_lpi_pins, >> + .npins = ARRAY_SIZE(sc7280_lpi_pins), >> + .groups = sc7280_groups, >> + .ngroups = ARRAY_SIZE(sc7280_groups), >> + .functions = sc7280_functions, >> + .nfunctions = ARRAY_SIZE(sc7280_functions), >> + .is_clk_optional = false, >> +}; >> >> >> static const struct lpi_pinctrl_variant_data sc7280_lpi_data = { >> .pins = sc7280_lpi_pins, >> .npins = ARRAY_SIZE(sc7280_lpi_pins), >> @@ -149,6 +159,10 @@ static const struct of_device_id lpi_pinctrl_of_match[] = { >> .compatible = "qcom,sc7280-lpass-lpi-pinctrl", >> .data = &sc7280_lpi_data, >> }, >> + { >> + .compatible = "qcom,sc7280-lpass-adsp-lpi-pinctrl", >> + .data = &sc7280_adsp_lpi_data, >> + }, > Drop this and instead add some code in the probe() > in drivers/pinctrl/qcom/pinctrl-lpass-lpi.c > lines: > > if (of_device_is_compatible(np, "qcom,sc7280-lpass-lpi-pinctrl") && > of_property_read_bool(np, "qcom,adsp-mode)) > data = &sc7280_adsp_lpi_data;
Here, only diff between ADSP and ADSP bypass variant dats is "is_clk_optional" field.
So we can keep something like this. Kindly suggest, if it's not making sense.
if (of_device_is_compatible(np, "qcom,sc7280-lpass-lpi-pinctrl") && of_property_read_bool(np, "qcom,adsp-mode)) data->is_clk_optional = false;
> > Yours, > Linus Walleij
| |