linux-kernel - Re: [PATCH v3 3/3] pinctrl: qcom: Add SDM660 LPASS LPI TLMM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <xcsx7fxl2wcnd6ocbzlptwkzm433aneaopigp5j2bxqq64mltn@56uq6lflgyio>
Date: Wed, 27 Aug 2025 04:47:43 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Bjorn Andersson <andersson@...nel.org>
Cc: setotau@...dex.ru, Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-arm-msm@...r.kernel.org,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht,
        Richard Acayan <mailingradian@...il.com>
Subject: Re: [PATCH v3 3/3] pinctrl: qcom: Add SDM660 LPASS LPI TLMM

On Mon, Aug 25, 2025 at 10:00:28PM -0500, Bjorn Andersson wrote:
> On Mon, Aug 25, 2025 at 03:32:30PM +0300, Nickolay Goppen via B4 Relay wrote:
> > From: Richard Acayan <mailingradian@...il.com>
> > 
> > The Snapdragon 660 has a Low-Power Island (LPI) TLMM for configuring
> > pins related to audio. Add the driver for this.
> > Also, this driver uses it's own quirky pin_offset function like downstream
> > driver does [1].
> 
> Please describe the quirky behavior in the commit message, rather than
> just referencing the downstream code.
> 
> > 
> > [1] https://git.codelinaro.org/clo/la/kernel/msm-4.4/-/blob/LA.UM.7.2.c27-07400-sdm660.0/drivers/pinctrl/qcom/pinctrl-lpi.c#L107
> > 
> > Signed-off-by: Richard Acayan <mailingradian@...il.com>
> > Co-developed-by: Nickolay Goppen <setotau@...dex.ru>
> > Signed-off-by: Nickolay Goppen <setotau@...dex.ru>
> > ---
> >  drivers/pinctrl/qcom/Kconfig                    |  10 ++
> >  drivers/pinctrl/qcom/Makefile                   |   1 +
> >  drivers/pinctrl/qcom/pinctrl-sdm660-lpass-lpi.c | 196 ++++++++++++++++++++++++
> >  3 files changed, 207 insertions(+)
> > 
> > diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
> > index dd9bbe8f3e11c37418d2143b33c21eeea10d456b..ef42520115f461302098d878cb76c6f25e55b5e4 100644
> > --- a/drivers/pinctrl/qcom/Kconfig
> > +++ b/drivers/pinctrl/qcom/Kconfig
> > @@ -68,6 +68,16 @@ config PINCTRL_SC7280_LPASS_LPI
> >  	  Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
> >  	  (Low Power Island) found on the Qualcomm Technologies Inc SC7280 platform.
> >  
> > +config PINCTRL_SDM660_LPASS_LPI
> > +	tristate "Qualcomm Technologies Inc SDM660 LPASS LPI pin controller driver"
> > +	depends on GPIOLIB
> > +	depends on ARM64 || COMPILE_TEST
> > +	depends on PINCTRL_LPASS_LPI
> > +	help
> > +	  This is the pinctrl, pinmux, pinconf and gpiolib driver for the
> > +	  Qualcomm Technologies Inc LPASS (Low Power Audio SubSystem) LPI
> > +	  (Low Power Island) found on the Qualcomm Technologies Inc SDM660 platform.
> > +
> >  config PINCTRL_SM4250_LPASS_LPI
> >  	tristate "Qualcomm Technologies Inc SM4250 LPASS LPI pin controller driver"
> >  	depends on ARM64 || COMPILE_TEST
> > diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile
> > index 954f5291cc37242baffc021e3c68d850aabd57cd..cea8617ac650ecfc75c2a0c745a53d6a1b829842 100644
> > --- a/drivers/pinctrl/qcom/Makefile
> > +++ b/drivers/pinctrl/qcom/Makefile
> > @@ -43,6 +43,7 @@ obj-$(CONFIG_PINCTRL_SC7280_LPASS_LPI) += pinctrl-sc7280-lpass-lpi.o
> >  obj-$(CONFIG_PINCTRL_SC8180X)	+= pinctrl-sc8180x.o
> >  obj-$(CONFIG_PINCTRL_SC8280XP)	+= pinctrl-sc8280xp.o
> >  obj-$(CONFIG_PINCTRL_SDM660)   += pinctrl-sdm660.o
> > +obj-$(CONFIG_PINCTRL_SDM660_LPASS_LPI) += pinctrl-sdm660-lpass-lpi.o
> >  obj-$(CONFIG_PINCTRL_SDM670) += pinctrl-sdm670.o
> >  obj-$(CONFIG_PINCTRL_SDM845) += pinctrl-sdm845.o
> >  obj-$(CONFIG_PINCTRL_SDX55) += pinctrl-sdx55.o
> > diff --git a/drivers/pinctrl/qcom/pinctrl-sdm660-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-sdm660-lpass-lpi.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..36fba93fda1160ad51a979996f8007393555f222
> > --- /dev/null
> > +++ b/drivers/pinctrl/qcom/pinctrl-sdm660-lpass-lpi.c
> > @@ -0,0 +1,196 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * This driver is solely based on the limited information in downstream code.
> > + * Any verification with schematics would be greatly appreciated.
> > + *
> > + * Copyright (c) 2023, Richard Acayan. All rights reserved.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pinctrl/pinctrl.h>
> > +
> > +#include "pinctrl-lpass-lpi.h"
> > +
> > +enum lpass_lpi_functions {
> > +	LPI_MUX_comp_rx,
> > +	LPI_MUX_dmic12,
> > +	LPI_MUX_dmic34,
> > +	LPI_MUX_mclk0,
> > +	LPI_MUX_pdm_2_gpios,
> > +	LPI_MUX_pdm_clk,
> > +	LPI_MUX_pdm_rx,
> > +	LPI_MUX_pdm_sync,
> > +
> > +	LPI_MUX_gpio,
> > +	LPI_MUX__,
> > +};
> > +
> > +static const u32 sdm660_lpi_offset[] = {
> 
> This should be write only, but I still find it error prone and ugly to
> keep this array separate of the pingroups - and I don't fancy the extra
> indirect jump just to lookup an element in the array.
> 
> Can't we instead extend lpi_pingroup with an "reg_offset", and then use
> lpi_pinctrl_variant_data->flags to indicate when this should be used?
> 
> That consolidates the information in the groups[] and avoids the
> additional function calls.

I R-B'ed too early. This seems to be a good idea.


-- 
With best wishes
Dmitry