From: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
To: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
agross@kernel.org, bjorn.andersson@linaro.org,
lgirdwood@gmail.com, broonie@kernel.org, robh+dt@kernel.org,
plai@codeaurora.org, bgoswami@codeaurora.org, perex@perex.cz,
tiwai@suse.com, rohitkr@codeaurora.org,
linux-arm-msm@vger.kernel.org, alsa-devel@alsa-project.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
swboyd@chromium.org, judyhsiao@chromium.org
Cc: Venkata Prasad Potturu <potturu@codeaurora.org>
Subject: Re: [PATCH v2 3/3] pinctrl: qcom: Add SC7280 lpass pin configuration
Date: Mon, 8 Nov 2021 15:31:13 +0530 [thread overview]
Message-ID: <54ab2532-a0d5-a04f-0f8c-2ce11fdf973e@codeaurora.org> (raw)
In-Reply-To: <3a05fc62-a060-3257-ad54-53e376763fe3@linaro.org>
On 11/3/2021 4:52 PM, Srinivas Kandagatla wrote:
Thanks Srini for your TIme!!!
> Thanks Srinivasa for the patches.
>
> On 27/10/2021 14:41, Srinivasa Rao Mandadapu wrote:
>> Update pin control support for SC7280 LPASS LPI.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <srivasam@codeaurora.org>
>> Co-developed-by: Venkata Prasad Potturu <potturu@codeaurora.org>
>> Signed-off-by: Venkata Prasad Potturu <potturu@codeaurora.org>
>> ---
>> drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 40
>> ++++++++++++++++++++++++++++++++
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> index 0bd0c16..17a05a6 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
>> @@ -122,6 +122,7 @@ static const struct pinctrl_pin_desc
>> lpass_lpi_pins[] = {
>> PINCTRL_PIN(11, "gpio11"),
>> PINCTRL_PIN(12, "gpio12"),
>> PINCTRL_PIN(13, "gpio13"),
>> + PINCTRL_PIN(14, "gpio14"),
>
> I see your point in first patch making these names more generic, but
> this is not really going to work, as the above line just added new pin
> for sm8250 even though it really only has 0-13 pins.
>
> I think it would be more clear if you could just make a dedicated
> structures for sc7280. Simillar comments apply for other changes too.
>
> Other than that the patch looks good to me.
>
> --srini
Okay. agree to your point. As lpass_lpi_pins array is just declaration,
and it will be used in functions, I thought it won't impact as functions
are different for both architectures.
With your feedback understood that it may not work existing archs. Will
change accordingly and re-post it.
>
>> };
>> enum lpass_lpi_functions {
>> @@ -136,6 +137,7 @@ enum lpass_lpi_functions {
>> LPI_MUX_i2s1_ws,
>> LPI_MUX_i2s2_clk,
>> LPI_MUX_i2s2_data,
>> + LPI_MUX_sc7280_i2s2_data,
>> LPI_MUX_i2s2_ws,
>> LPI_MUX_qua_mi2s_data,
>> LPI_MUX_qua_mi2s_sclk,
>> @@ -144,6 +146,7 @@ enum lpass_lpi_functions {
>> LPI_MUX_swr_rx_data,
>> LPI_MUX_swr_tx_clk,
>> LPI_MUX_swr_tx_data,
>> + LPI_MUX_sc7280_swr_tx_data,
>> LPI_MUX_wsa_swr_clk,
>> LPI_MUX_wsa_swr_data,
>> LPI_MUX_gpio,
>> @@ -164,8 +167,11 @@ static const unsigned int gpio10_pins[] = { 10 };
>> static const unsigned int gpio11_pins[] = { 11 };
>> static const unsigned int gpio12_pins[] = { 12 };
>> static const unsigned int gpio13_pins[] = { 13 };
>> +static const unsigned int gpio14_pins[] = { 14 };
>> +
>> static const char * const swr_tx_clk_groups[] = { "gpio0" };
>> static const char * const swr_tx_data_groups[] = { "gpio1",
>> "gpio2", "gpio5" };
>> +static const char * const sc7280_swr_tx_data_groups[] = { "gpio1",
>> "gpio2", "gpio14" };
>> static const char * const swr_rx_clk_groups[] = { "gpio3" };
>> static const char * const swr_rx_data_groups[] = { "gpio4", "gpio5" };
>> static const char * const dmic1_clk_groups[] = { "gpio6" };
>> @@ -185,6 +191,7 @@ static const char * const i2s1_data_groups[] = {
>> "gpio8", "gpio9" };
>> static const char * const wsa_swr_clk_groups[] = { "gpio10" };
>> static const char * const wsa_swr_data_groups[] = { "gpio11" };
>> static const char * const i2s2_data_groups[] = { "gpio12", "gpio12" };
>> +static const char * const sc7280_i2s2_data_groups[] = { "gpio12",
>> "gpio13" };
>> static const struct lpi_pingroup sm8250_groups[] = {
>> LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, _, _),
>> @@ -203,6 +210,24 @@ static const struct lpi_pingroup sm8250_groups[]
>> = {
>> LPI_PINGROUP(13, NO_SLEW, dmic3_data, i2s2_data, _, _),
>> };
>> +static const struct lpi_pingroup sc7280_groups[] = {
>> + LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, _, _),
>> + LPI_PINGROUP(1, 2, swr_tx_data, qua_mi2s_ws, _, _),
>> + LPI_PINGROUP(2, 4, swr_tx_data, qua_mi2s_data, _, _),
>> + LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data, _, _),
>> + LPI_PINGROUP(4, 10, swr_rx_data, qua_mi2s_data, _, _),
>> + LPI_PINGROUP(5, 12, swr_rx_data, _, _, _),
>> + LPI_PINGROUP(6, NO_SLEW, dmic1_clk, i2s1_clk, _, _),
>> + LPI_PINGROUP(7, NO_SLEW, dmic1_data, i2s1_ws, _, _),
>> + LPI_PINGROUP(8, NO_SLEW, dmic2_clk, i2s1_data, _, _),
>> + LPI_PINGROUP(9, NO_SLEW, dmic2_data, i2s1_data, _, _),
>> + LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, _, _),
>> + LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, _, _),
>> + LPI_PINGROUP(12, NO_SLEW, dmic3_clk, sc7280_i2s2_data, _, _),
>> + LPI_PINGROUP(13, NO_SLEW, dmic3_data, sc7280_i2s2_data, _, _),
>> + LPI_PINGROUP(14, 6, sc7280_swr_tx_data, _, _, _),
>> +};
>> +
>> static const struct lpi_function lpass_functions[] = {
>> LPI_FUNCTION(dmic1_clk),
>> LPI_FUNCTION(dmic1_data),
>> @@ -215,6 +240,7 @@ static const struct lpi_function
>> lpass_functions[] = {
>> LPI_FUNCTION(i2s1_ws),
>> LPI_FUNCTION(i2s2_clk),
>> LPI_FUNCTION(i2s2_data),
>> + LPI_FUNCTION(sc7280_i2s2_data),
>> LPI_FUNCTION(i2s2_ws),
>> LPI_FUNCTION(qua_mi2s_data),
>> LPI_FUNCTION(qua_mi2s_sclk),
>> @@ -223,6 +249,7 @@ static const struct lpi_function
>> lpass_functions[] = {
>> LPI_FUNCTION(swr_rx_data),
>> LPI_FUNCTION(swr_tx_clk),
>> LPI_FUNCTION(swr_tx_data),
>> + LPI_FUNCTION(sc7280_swr_tx_data),
>> LPI_FUNCTION(wsa_swr_clk),
>> LPI_FUNCTION(wsa_swr_data),
>> };
>> @@ -236,6 +263,15 @@ static struct lpi_pinctrl_variant_data
>> sm8250_lpi_data = {
>> .nfunctions = ARRAY_SIZE(lpass_functions),
>> };
>> +static const struct lpi_pinctrl_variant_data sc7280_lpi_data = {
>> + .pins = lpass_lpi_pins,
>> + .npins = ARRAY_SIZE(lpass_lpi_pins),
>> + .groups = sc7280_groups,
>> + .ngroups = ARRAY_SIZE(sc7280_groups),
>> + .functions = lpass_functions,
>> + .nfunctions = ARRAY_SIZE(lpass_functions),
>> +};
>> +
>> static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin,
>> unsigned int addr)
>> {
>> @@ -677,6 +713,10 @@ static const struct of_device_id
>> lpi_pinctrl_of_match[] = {
>> .compatible = "qcom,sm8250-lpass-lpi-pinctrl",
>> .data = &sm8250_lpi_data,
>> },
>> + {
>> + .compatible = "qcom,sc7280-lpass-lpi-pinctrl",
>> + .data = &sc7280_lpi_data,
>> + },
>> { }
>> };
>> MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match);
>>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.,
is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
prev parent reply other threads:[~2021-11-08 10:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-27 13:41 [PATCH v2 0/3] Add pin control support for lpass sc7280 Srinivasa Rao Mandadapu
2021-10-27 13:41 ` [PATCH v2 1/3] pinctrl: qcom: Update lpass variant independent functions as generic Srinivasa Rao Mandadapu
2021-11-03 11:22 ` Srinivas Kandagatla
2021-11-19 6:36 ` Srinivasa Rao Mandadapu
2021-10-27 13:41 ` [PATCH v2 2/3] dt-bindings: pinctrl: qcom: Add sc7280 lpass lpi pinctrl compatible Srinivasa Rao Mandadapu
2021-11-01 21:37 ` Rob Herring
2021-10-27 13:41 ` [PATCH v2 3/3] pinctrl: qcom: Add SC7280 lpass pin configuration Srinivasa Rao Mandadapu
2021-11-03 11:22 ` Srinivas Kandagatla
2021-11-08 10:01 ` Srinivasa Rao Mandadapu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54ab2532-a0d5-a04f-0f8c-2ce11fdf973e@codeaurora.org \
--to=srivasam@codeaurora.org \
--cc=agross@kernel.org \
--cc=alsa-devel@alsa-project.org \
--cc=bgoswami@codeaurora.org \
--cc=bjorn.andersson@linaro.org \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=judyhsiao@chromium.org \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=perex@perex.cz \
--cc=plai@codeaurora.org \
--cc=potturu@codeaurora.org \
--cc=robh+dt@kernel.org \
--cc=rohitkr@codeaurora.org \
--cc=srinivas.kandagatla@linaro.org \
--cc=swboyd@chromium.org \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).