From: Richard Acayan <mailingradian@gmail.com>
To: Bjorn Andersson <andersson@kernel.org>
Cc: linux-arm-msm@vger.kernel.org, Andy Gross <agross@kernel.org>,
Konrad Dybcio <konrad.dybcio@somainline.org>,
Linus Walleij <linus.walleij@linaro.org>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
~postmarketos/upstreaming@lists.sr.ht,
Richard Acayan <mailingradian@gmail.com>
Subject: Re: [PATCH 2/2] pinctrl: qcom: add sdm670 pinctrl
Date: Thu, 15 Sep 2022 22:01:17 -0400 [thread overview]
Message-ID: <20220916020117.227480-1-mailingradian@gmail.com> (raw)
In-Reply-To: <20220914023900.z64wugbq7p2gfb32@builder.lan>
> > diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
> > index 2961b5eb8e10..7aba4188110c 100644
> > --- a/drivers/pinctrl/qcom/Kconfig
> > +++ b/drivers/pinctrl/qcom/Kconfig
> > @@ -283,6 +283,15 @@ config PINCTRL_SDM660
> > Qualcomm Technologies Inc TLMM block found on the Qualcomm
> > Technologies Inc SDM660 platform.
> >
> > +config PINCTRL_SDM670
> > + tristate "Qualcomm Technologies Inc SDM670 pin controller driver"
> > + depends on (OF || ACPI)
>
> I believe you can drop ACPI from this?
Yes, I adapted this driver from the SDM845 driver and removed the ACPI
features but forgot to remove the config dependency.
> > + depends on PINCTRL_MSM
> > + help
> > + This is the pinctrl, pinmux, pinconf and gpiolib driver for the
> > + Qualcomm Technologies Inc TLMM block found on the Qualcomm
> > + Technologies Inc SDM670 platform.
> > +
> > config PINCTRL_SDM845
> > tristate "Qualcomm Technologies Inc SDM845 pin controller driver"
> > depends on (OF || ACPI)
> > +/* Every pin is maintained as a single group, and missing or non-existing pin
> > + * would be maintained as dummy group to synchronize pin group index with
> > + * pin descriptor registered with pinctrl core.
> > + * Clients would not be able to request these dummy pin groups.
>
> The client wouldn't be able to define pinmux/pinconf, but I'm not able
> to spot anything that would prevent a client from referencing the gpio?
>
> Perhaps I'm missing something?
No, you're not. I kept this comment because I saw it in other pinctrl
drivers and thought it was standard:
~/linux $ grep dummy -RC1 drivers/pinctrl/qcom/
drivers/pinctrl/qcom/pinctrl-qcs404.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-qcs404.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-qcs404.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-qcs404.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-qcs404.c- */
--
drivers/pinctrl/qcom/pinctrl-sc7180.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sc7180.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sc7180.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sc7180.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sc7180.c- */
--
drivers/pinctrl/qcom/pinctrl-sc7280.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sc7280.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sc7280.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sc7280.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sc7280.c- */
--
drivers/pinctrl/qcom/pinctrl-sdx55.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sdx55.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sdx55.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sdx55.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sdx55.c- */
--
drivers/pinctrl/qcom/pinctrl-sdx65.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sdx65.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sdx65.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sdx65.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sdx65.c- */
--
drivers/pinctrl/qcom/pinctrl-sm6115.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sm6115.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sm6115.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sm6115.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sm6115.c- */
--
drivers/pinctrl/qcom/pinctrl-sm8350.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sm8350.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sm8350.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sm8350.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sm8350.c- */
--
drivers/pinctrl/qcom/pinctrl-qcm2290.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-qcm2290.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-qcm2290.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-qcm2290.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-qcm2290.c- */
--
drivers/pinctrl/qcom/pinctrl-sm6125.c- * Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sm6125.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sm6125.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sm6125.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sm6125.c- */
--
drivers/pinctrl/qcom/pinctrl-sm6350.c- * Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sm6350.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sm6350.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sm6350.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sm6350.c- */
--
drivers/pinctrl/qcom/pinctrl-sm8150.c- * Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sm8150.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sm8150.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sm8150.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sm8150.c- */
--
drivers/pinctrl/qcom/pinctrl-sm8450.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sm8450.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sm8450.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sm8450.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sm8450.c- */
--
drivers/pinctrl/qcom/pinctrl-sdm845.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sdm845.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sdm845.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sdm845.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sdm845.c- */
--
drivers/pinctrl/qcom/pinctrl-sm6375.c- * Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sm6375.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sm6375.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sm6375.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sm6375.c- */
--
drivers/pinctrl/qcom/pinctrl-sm8250.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sm8250.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sm8250.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sm8250.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sm8250.c- */
--
drivers/pinctrl/qcom/pinctrl-sc8180x.c-/* Every pin is maintained as a single group, and missing or non-existing pin
drivers/pinctrl/qcom/pinctrl-sc8180x.c: * would be maintained as dummy group to synchronize pin group index with
drivers/pinctrl/qcom/pinctrl-sc8180x.c- * pin descriptor registered with pinctrl core.
drivers/pinctrl/qcom/pinctrl-sc8180x.c: * Clients would not be able to request these dummy pin groups.
drivers/pinctrl/qcom/pinctrl-sc8180x.c- */
Since this driver has dummy pingroups, it is a bit confusing to see this
inaccurate information because it is relevant. I'll rewrite the comment so
that it makes sense.
> Otherwise, I think you should be able to specify reserved_gpios in
> sdm670_pinctrl and list the dummy items. This would ensure that the gpio
> code as well treat them as absent.
Yes, as long as I can reserve pins 0, 1, 2, 3, 81, 82, 83, and 84 for the
Pixel 3a. However, I think reserved_gpios overrides the DT schema where it
would be sensible to add it:
drivers/pinctrl/qcom/pinctrl-msm.c:690:
/* Driver provided reserved list overrides DT and ACPI */
Perhaps I should omit the dummy pingroups from the driver and try to handle
the gpio numbers discrepency on the DT side, like:
gpio-ranges = <&tlmm 0 0 58>, <&tlmm 65 59 4>, ...
I don't see this being done anywhere else but it should clear up the
debugfs problems I was having.
> > + */
> > +static const struct msm_pingroup sdm670_groups[] = {
> > + PINGROUP(0, SOUTH, qup0, _, _, _, _, _, _, _, _),
> > + PINGROUP(1, SOUTH, qup0, _, _, _, _, _, _, _, _),
prev parent reply other threads:[~2022-09-16 2:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-14 1:44 [PATCH 0/2] SDM670 Pin Control Driver Richard Acayan
2022-09-14 1:44 ` [PATCH 1/2] dt-bindings: pinctrl: qcom: add sdm670 pinctrl Richard Acayan
2022-09-14 2:25 ` Bjorn Andersson
2022-09-14 1:44 ` [PATCH 2/2] " Richard Acayan
2022-09-14 2:39 ` Bjorn Andersson
2022-09-16 2:01 ` Richard Acayan [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=20220916020117.227480-1-mailingradian@gmail.com \
--to=mailingradian@gmail.com \
--cc=agross@kernel.org \
--cc=andersson@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=konrad.dybcio@somainline.org \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=~postmarketos/upstreaming@lists.sr.ht \
/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).