Messages in this thread | | | Date | Tue, 12 Apr 2022 16:43:57 -0700 | From | Matthias Kaehlcke <> | Subject | Re: [PATCH v7 2/2] arm64: dts: qcom: sc7280: add lpass lpi pin controller node |
| |
On Tue, Apr 12, 2022 at 06:18:33PM +0530, Srinivasa Rao Mandadapu wrote: > > On 4/12/2022 1:02 AM, Matthias Kaehlcke wrote: > Thanks for your time Matthias!!! > > On Mon, Apr 11, 2022 at 07:23:04PM +0530, Srinivasa Rao Mandadapu wrote: > > > Add LPASS LPI pinctrl node required for Audio functionality on sc7280 > > > based platforms. > > > > > > 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> > > > --- > > > arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 84 ++++++++++++++++++++++++ > > > arch/arm64/boot/dts/qcom/sc7280.dtsi | 107 +++++++++++++++++++++++++++++++ > > > 2 files changed, 191 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > > > index 4ba2274..ea751dc 100644 > > > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > > > @@ -238,6 +238,90 @@ > > > modem-init; > > > }; > > > +&dmic01 { > > Shouldn't these nodes be in the PINCTRL section at their respective > > positions in alphabetical order? > > These are not part of tlmm pin control section. These are part of lpass_tlmm > section.
Agreed, these shouldn't be in the tlmm section, but it still seems nice to separate them from device nodes.
Some sc7180-trogdor devices have a section like this:
/* PINCTRL - modifications to sc7180-trogdor.dtsi */
In any case I don't care too much about the IDP, but this might come up again for sc7280-herobrine boards.
> In your previous comment you asked to remove &lpass_tlmm. Hence brought out. > > > > > nit: since you are keeping the groups the group names are a bit generic IMO. > > e.g. it is fairly obvious that 'dmic01_clk' refers to a clock pin, however > > just 'dmic01' is a bit vague. You could consider adding the prefix 'lpass_' > > to the group names for more clarity. > as dmic01 has both clk and data section, I don't think keeping clk is > appropriate here.
Agreed, _clk isn't appropriate as long as it's a group of pins, that would be for a label per pin.
> > From the schematics I interpret that the LPASS GPIOs 0-9 are mapped to the > > SC7280 GPIOs 144-153. Is that correct? > Yes. But we refer with GPIOs 0-9 in driver.
Thanks, just wanted to make sure my understanding is correct.
| |