* [PATCH v4 0/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl support @ 2020-11-16 14:34 Srinivas Kandagatla 2020-11-16 14:34 ` [PATCH v4 1/2] dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings Srinivas Kandagatla 2020-11-16 14:34 ` [PATCH v4 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver Srinivas Kandagatla 0 siblings, 2 replies; 10+ messages in thread From: Srinivas Kandagatla @ 2020-11-16 14:34 UTC (permalink / raw) To: linus.walleij, bjorn.andersson, robh+dt Cc: agross, linux-arm-msm, linux-gpio, devicetree, linux-kernel, Srinivas Kandagatla This patch adds support for LPASS (Low Power Audio SubSystem) LPI (Low Power Island) pinctrl on SM8250. This patch has been tested on support to Qualcomm Robotics RB5 Development Kit based on QRB5165 Robotics SoC. This board has 2 WSA881X smart speakers with onboard DMIC connected to internal LPASS codec via WSA and VA macros respectively. Most of the work is derived from downstream Qualcomm kernels. Credits to various Qualcomm authors from Patrick Lai's team who have contributed to this code. Am guessing existing qcom folder should cover maintining this driver too! If not I can send additional patch to consolidate this along with other Audio related drivers in Maintainer file! Changes since v3: - updated bindings as per Rob's review! Srinivas Kandagatla (2): dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver .../pinctrl/qcom,lpass-lpi-pinctrl.yaml | 132 ++++ drivers/pinctrl/qcom/Kconfig | 8 + drivers/pinctrl/qcom/Makefile | 1 + drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 734 ++++++++++++++++++ 4 files changed, 875 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml create mode 100644 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c -- 2.21.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/2] dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings 2020-11-16 14:34 [PATCH v4 0/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl support Srinivas Kandagatla @ 2020-11-16 14:34 ` Srinivas Kandagatla 2020-11-30 23:49 ` Rob Herring 2020-12-01 0:55 ` Bjorn Andersson 2020-11-16 14:34 ` [PATCH v4 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver Srinivas Kandagatla 1 sibling, 2 replies; 10+ messages in thread From: Srinivas Kandagatla @ 2020-11-16 14:34 UTC (permalink / raw) To: linus.walleij, bjorn.andersson, robh+dt Cc: agross, linux-arm-msm, linux-gpio, devicetree, linux-kernel, Srinivas Kandagatla Add device tree binding Documentation details for Qualcomm SM8250 LPASS(Low Power Audio Sub System) LPI(Low Power Island) pinctrl driver. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- .../pinctrl/qcom,lpass-lpi-pinctrl.yaml | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml new file mode 100644 index 000000000000..704e761146ef --- /dev/null +++ b/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml @@ -0,0 +1,132 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/pinctrl/qcom,lpass-lpi-pinctrl.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Technologies, Inc. Low Power Audio SubSystem (LPASS) + Low Power Island (LPI) TLMM block + +maintainers: + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> + +description: | + This binding describes the Top Level Mode Multiplexer block found in the + LPASS LPI IP on most Qualcomm SoCs + +properties: + compatible: + const: qcom,sm8250-lpass-lpi-pinctrl + + reg: + minItems: 2 + maxItems: 2 + + clocks: + items: + - description: LPASS Core voting clock + - description: LPASS Audio voting clock + + clock-names: + items: + - const: core + - const: audio + + gpio-controller: true + + '#gpio-cells': + description: Specifying the pin number and flags, as defined in + include/dt-bindings/gpio/gpio.h + const: 2 + + gpio-ranges: + maxItems: 1 + +#PIN CONFIGURATION NODES +patternProperties: + '-pins$': + type: object + description: + Pinctrl node's client devices use subnodes for desired pin configuration. + Client device subnodes use below standard properties. + $ref: "/schemas/pinctrl/pincfg-node.yaml" + + properties: + pins: + description: + List of gpio pins affected by the properties specified in this + subnode. + items: + oneOf: + - pattern: "^gpio([0-9]|[1-9][0-9])$" + minItems: 1 + maxItems: 14 + + function: + enum: [ gpio, swr_tx_clk, qua_mi2s_sclk, swr_tx_data1, qua_mi2s_ws, + swr_tx_data2, qua_mi2s_data0, swr_rx_clk, qua_mi2s_data1, + swr_rx_data1, qua_mi2s_data2, swr_tx_data3, swr_rx_data2, + dmic1_clk, i2s1_clk, dmic1_data, i2s1_ws, dmic2_clk, + i2s1_data0, dmic2_data, i2s1_data1, i2s2_clk, wsa_swr_clk, + i2s2_ws, wsa_swr_data, dmic3_clk, i2s2_data0, dmic3_data, + i2s2_data1 ] + description: + Specify the alternative function to be configured for the specified + pins. + + drive-strength: + enum: [2, 4, 6, 8, 10, 12, 14, 16] + default: 2 + description: + Selects the drive strength for the specified pins, in mA. + + slew-rate: + enum: [0, 1, 2, 3] + default: 0 + description: | + 0: No adjustments + 1: Higher Slew rate (faster edges) + 2: Lower Slew rate (slower edges) + 3: Reserved (No adjustments) + + bias-pull-down: true + + bias-pull-up: true + + bias-disable: true + + output-high: true + + output-low: true + + required: + - pins + - function + + additionalProperties: false + +required: + - compatible + - reg + - clocks + - clock-names + - gpio-controller + - '#gpio-cells' + - gpio-ranges + +additionalProperties: false + +examples: + - | + #include <dt-bindings/sound/qcom,q6afe.h> + lpi_tlmm: pinctrl@33c0000 { + compatible = "qcom,sm8250-lpass-lpi-pinctrl"; + reg = <0x33c0000 0x20000>, + <0x355a000 0x1000>; + clocks = <&q6afecc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>, + <&q6afecc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>; + clock-names = "core", "audio"; + gpio-controller; + #gpio-cells = <2>; + gpio-ranges = <&lpi_tlmm 0 0 14>; + }; -- 2.21.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings 2020-11-16 14:34 ` [PATCH v4 1/2] dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings Srinivas Kandagatla @ 2020-11-30 23:49 ` Rob Herring 2020-12-01 0:55 ` Bjorn Andersson 1 sibling, 0 replies; 10+ messages in thread From: Rob Herring @ 2020-11-30 23:49 UTC (permalink / raw) To: Srinivas Kandagatla Cc: linus.walleij, robh+dt, linux-gpio, devicetree, bjorn.andersson, linux-arm-msm, linux-kernel, agross On Mon, 16 Nov 2020 14:34:31 +0000, Srinivas Kandagatla wrote: > Add device tree binding Documentation details for Qualcomm SM8250 > LPASS(Low Power Audio Sub System) LPI(Low Power Island) pinctrl driver. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > .../pinctrl/qcom,lpass-lpi-pinctrl.yaml | 132 ++++++++++++++++++ > 1 file changed, 132 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml > Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings 2020-11-16 14:34 ` [PATCH v4 1/2] dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings Srinivas Kandagatla 2020-11-30 23:49 ` Rob Herring @ 2020-12-01 0:55 ` Bjorn Andersson 2020-12-01 10:03 ` Srinivas Kandagatla 1 sibling, 1 reply; 10+ messages in thread From: Bjorn Andersson @ 2020-12-01 0:55 UTC (permalink / raw) To: Srinivas Kandagatla Cc: linus.walleij, robh+dt, agross, linux-arm-msm, linux-gpio, devicetree, linux-kernel On Mon 16 Nov 08:34 CST 2020, Srinivas Kandagatla wrote: > Add device tree binding Documentation details for Qualcomm SM8250 > LPASS(Low Power Audio Sub System) LPI(Low Power Island) pinctrl driver. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > .../pinctrl/qcom,lpass-lpi-pinctrl.yaml | 132 ++++++++++++++++++ > 1 file changed, 132 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml > > diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml > new file mode 100644 > index 000000000000..704e761146ef > --- /dev/null > +++ b/Documentation/devicetree/bindings/pinctrl/qcom,lpass-lpi-pinctrl.yaml > @@ -0,0 +1,132 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/pinctrl/qcom,lpass-lpi-pinctrl.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Technologies, Inc. Low Power Audio SubSystem (LPASS) > + Low Power Island (LPI) TLMM block > + > +maintainers: > + - Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > + > +description: | > + This binding describes the Top Level Mode Multiplexer block found in the > + LPASS LPI IP on most Qualcomm SoCs > + > +properties: > + compatible: > + const: qcom,sm8250-lpass-lpi-pinctrl > + > + reg: > + minItems: 2 > + maxItems: 2 > + > + clocks: > + items: > + - description: LPASS Core voting clock > + - description: LPASS Audio voting clock > + > + clock-names: > + items: > + - const: core > + - const: audio > + > + gpio-controller: true > + > + '#gpio-cells': > + description: Specifying the pin number and flags, as defined in > + include/dt-bindings/gpio/gpio.h > + const: 2 > + > + gpio-ranges: > + maxItems: 1 > + > +#PIN CONFIGURATION NODES > +patternProperties: > + '-pins$': > + type: object > + description: > + Pinctrl node's client devices use subnodes for desired pin configuration. > + Client device subnodes use below standard properties. > + $ref: "/schemas/pinctrl/pincfg-node.yaml" > + > + properties: > + pins: > + description: > + List of gpio pins affected by the properties specified in this > + subnode. > + items: > + oneOf: > + - pattern: "^gpio([0-9]|[1-9][0-9])$" > + minItems: 1 > + maxItems: 14 > + > + function: > + enum: [ gpio, swr_tx_clk, qua_mi2s_sclk, swr_tx_data1, qua_mi2s_ws, > + swr_tx_data2, qua_mi2s_data0, swr_rx_clk, qua_mi2s_data1, > + swr_rx_data1, qua_mi2s_data2, swr_tx_data3, swr_rx_data2, > + dmic1_clk, i2s1_clk, dmic1_data, i2s1_ws, dmic2_clk, > + i2s1_data0, dmic2_data, i2s1_data1, i2s2_clk, wsa_swr_clk, > + i2s2_ws, wsa_swr_data, dmic3_clk, i2s2_data0, dmic3_data, > + i2s2_data1 ] > + description: > + Specify the alternative function to be configured for the specified > + pins. > + > + drive-strength: > + enum: [2, 4, 6, 8, 10, 12, 14, 16] > + default: 2 > + description: > + Selects the drive strength for the specified pins, in mA. > + > + slew-rate: > + enum: [0, 1, 2, 3] > + default: 0 > + description: | > + 0: No adjustments > + 1: Higher Slew rate (faster edges) > + 2: Lower Slew rate (slower edges) > + 3: Reserved (No adjustments) > + > + bias-pull-down: true > + > + bias-pull-up: true > + > + bias-disable: true > + > + output-high: true > + > + output-low: true > + > + required: > + - pins > + - function > + > + additionalProperties: false > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - gpio-controller > + - '#gpio-cells' > + - gpio-ranges > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/sound/qcom,q6afe.h> > + lpi_tlmm: pinctrl@33c0000 { > + compatible = "qcom,sm8250-lpass-lpi-pinctrl"; > + reg = <0x33c0000 0x20000>, > + <0x355a000 0x1000>; We shouldn't reference parts of blocks, so this should be 16KB at 0x35500000 and if we have multiple drivers that needs to poke in that region we'd need to abstract that somehow (e.g. though a syscon). Regards, Bjorn > + clocks = <&q6afecc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>, > + <&q6afecc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>; > + clock-names = "core", "audio"; > + gpio-controller; > + #gpio-cells = <2>; > + gpio-ranges = <&lpi_tlmm 0 0 14>; > + }; > -- > 2.21.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings 2020-12-01 0:55 ` Bjorn Andersson @ 2020-12-01 10:03 ` Srinivas Kandagatla 0 siblings, 0 replies; 10+ messages in thread From: Srinivas Kandagatla @ 2020-12-01 10:03 UTC (permalink / raw) To: Bjorn Andersson Cc: linus.walleij, robh+dt, agross, linux-arm-msm, linux-gpio, devicetree, linux-kernel On 01/12/2020 00:55, Bjorn Andersson wrote: >> + reg = <0x33c0000 0x20000>, >> + <0x355a000 0x1000>; > We shouldn't reference parts of blocks, so this should be 16KB at yes, makes sense! Will change the example accordingly! > 0x35500000 and if we have multiple drivers that needs to poke in that > region we'd need to abstract that somehow (e.g. though a syscon).Yes! > > Regards, > Bjorn > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver 2020-11-16 14:34 [PATCH v4 0/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl support Srinivas Kandagatla 2020-11-16 14:34 ` [PATCH v4 1/2] dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings Srinivas Kandagatla @ 2020-11-16 14:34 ` Srinivas Kandagatla 2020-12-01 0:47 ` Bjorn Andersson 1 sibling, 1 reply; 10+ messages in thread From: Srinivas Kandagatla @ 2020-11-16 14:34 UTC (permalink / raw) To: linus.walleij, bjorn.andersson, robh+dt Cc: agross, linux-arm-msm, linux-gpio, devicetree, linux-kernel, Srinivas Kandagatla Add initial pinctrl driver to support pin configuration for LPASS (Low Power Audio SubSystem) LPI (Low Power Island) pinctrl on SM8250. This IP is an additional pin control block for Audio Pins on top the existing SoC Top level pin-controller. Hardware setup looks like: TLMM GPIO[146 - 159] --> LPASS LPI GPIO [0 - 13] This pin controller has some similarities compared to Top level msm SoC Pin controller like 'each pin belongs to a single group' and so on. However this one is intended to control only audio pins in particular, which can not be configured/touched by the Top level SoC pin controller except setting them as gpios. Apart from this, slew rate is also available in this block for certain pins which are connected to SLIMbus or SoundWire Bus. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/pinctrl/qcom/Kconfig | 8 + drivers/pinctrl/qcom/Makefile | 1 + drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 734 +++++++++++++++++++++++ 3 files changed, 743 insertions(+) create mode 100644 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig index 5fe7b8aaf69d..d3e4e89c2810 100644 --- a/drivers/pinctrl/qcom/Kconfig +++ b/drivers/pinctrl/qcom/Kconfig @@ -236,4 +236,12 @@ config PINCTRL_SM8250 Qualcomm Technologies Inc TLMM block found on the Qualcomm Technologies Inc SM8250 platform. +config PINCTRL_LPASS_LPI + tristate "Qualcomm Technologies Inc LPASS LPI pin controller driver" + depends on GPIOLIB + 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 SoCs. + endif diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile index 9e3d9c91a444..c8520155fb1b 100644 --- a/drivers/pinctrl/qcom/Makefile +++ b/drivers/pinctrl/qcom/Makefile @@ -28,3 +28,4 @@ obj-$(CONFIG_PINCTRL_SDM660) += pinctrl-sdm660.o obj-$(CONFIG_PINCTRL_SDM845) += pinctrl-sdm845.o obj-$(CONFIG_PINCTRL_SM8150) += pinctrl-sm8150.o obj-$(CONFIG_PINCTRL_SM8250) += pinctrl-sm8250.o +obj-$(CONFIG_PINCTRL_LPASS_LPI) += pinctrl-lpass-lpi.o diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c new file mode 100644 index 000000000000..63cfbb2d032a --- /dev/null +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c @@ -0,0 +1,734 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved. + * Copyright (c) 2020 Linaro Ltd. + */ + +#include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/gpio/driver.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of.h> +#include <linux/pinctrl/pinconf-generic.h> +#include <linux/pinctrl/pinconf.h> +#include <linux/pinctrl/pinmux.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/types.h> +#include "../core.h" +#include "../pinctrl-utils.h" + +#define LPI_GPIO_REG_VAL_CTL 0x00 +#define LPI_GPIO_REG_DIR_CTL 0x04 +#define LPI_SLEW_REG_VAL_CTL 0x00 +#define LPI_SLEW_RATE_MAX 0x03 +#define LPI_SLEW_BITS_SIZE 0x02 +#define LPI_GPIO_REG_PULL_SHIFT 0x0 +#define LPI_GPIO_REG_PULL_MASK GENMASK(1, 0) +#define LPI_GPIO_REG_FUNCTION_SHIFT 0x2 +#define LPI_GPIO_REG_FUNCTION_MASK GENMASK(5, 2) +#define LPI_GPIO_REG_OUT_STRENGTH_SHIFT 0x6 +#define LPI_GPIO_REG_OUT_STRENGTH_MASK GENMASK(8, 6) +#define LPI_GPIO_REG_OE_SHIFT 0x9 +#define LPI_GPIO_REG_OE_MASK BIT(9) +#define LPI_GPIO_REG_DIR_SHIFT 0x1 +#define LPI_GPIO_REG_DIR_MASK 0x2 +#define LPI_GPIO_BIAS_DISABLE 0x0 +#define LPI_GPIO_PULL_DOWN 0x1 +#define LPI_GPIO_KEEPER 0x2 +#define LPI_GPIO_PULL_UP 0x3 +#define LPI_GPIO_DS_TO_VAL(v) ((v / 2 - 1) << LPI_GPIO_REG_OUT_STRENGTH_SHIFT) +#define NO_SLEW -1 + +#define LPI_FUNCTION(fname) \ + [LPI_MUX_##fname] = { \ + .name = #fname, \ + .groups = fname##_groups, \ + .ngroups = ARRAY_SIZE(fname##_groups), \ + } + +#define LPI_PINGROUP(id, soff, f1, f2, f3, f4) \ + { \ + .name = "gpio" #id, \ + .pins = gpio##id##_pins, \ + .pin = id, \ + .slew_offset = soff, \ + .npins = ARRAY_SIZE(gpio##id##_pins), \ + .funcs = (int[]){ \ + LPI_MUX_gpio, \ + LPI_MUX_##f1, \ + LPI_MUX_##f2, \ + LPI_MUX_##f3, \ + LPI_MUX_##f4, \ + }, \ + .nfuncs = 5, \ + } + +struct lpi_pingroup { + const char *name; + const unsigned int *pins; + unsigned int npins; + unsigned int pin; + /* Bit offset in slew register for SoundWire pins only */ + unsigned int slew_offset; + unsigned int *funcs; + unsigned int nfuncs; +}; + +struct lpi_function { + const char *name; + const char * const *groups; + unsigned int ngroups; +}; + +struct lpi_pinctrl_variant_data { + int tlmm_reg_offset; + const struct pinctrl_pin_desc *pins; + int npins; + const struct lpi_pingroup *groups; + int ngroups; + const struct lpi_function *functions; + int nfunctions; +}; + +#define MAX_LPI_NUM_CLKS 2 + +struct lpi_pinctrl { + struct device *dev; + struct pinctrl_dev *ctrl; + struct gpio_chip chip; + struct pinctrl_desc desc; + char __iomem *tlmm_base; + char __iomem *slew_base; + struct clk_bulk_data clks[MAX_LPI_NUM_CLKS]; + struct mutex slew_access_lock; + const struct lpi_pinctrl_variant_data *data; +}; + +/* sm8250 variant specific data */ +static const struct pinctrl_pin_desc sm8250_lpi_pins[] = { + PINCTRL_PIN(0, "gpio0"), + PINCTRL_PIN(1, "gpio1"), + PINCTRL_PIN(2, "gpio2"), + PINCTRL_PIN(3, "gpio3"), + PINCTRL_PIN(4, "gpio4"), + PINCTRL_PIN(5, "gpio5"), + PINCTRL_PIN(6, "gpio6"), + PINCTRL_PIN(7, "gpio7"), + PINCTRL_PIN(8, "gpio8"), + PINCTRL_PIN(9, "gpio9"), + PINCTRL_PIN(10, "gpio10"), + PINCTRL_PIN(11, "gpio11"), + PINCTRL_PIN(12, "gpio12"), + PINCTRL_PIN(13, "gpio13"), +}; + +enum sm8250_lpi_functions { + LPI_MUX_swr_tx_clk, + LPI_MUX_qua_mi2s_sclk, + LPI_MUX_swr_tx_data1, + LPI_MUX_qua_mi2s_ws, + LPI_MUX_swr_tx_data2, + LPI_MUX_qua_mi2s_data0, + LPI_MUX_swr_rx_clk, + LPI_MUX_qua_mi2s_data1, + LPI_MUX_swr_rx_data1, + LPI_MUX_qua_mi2s_data2, + LPI_MUX_swr_tx_data3, + LPI_MUX_swr_rx_data2, + LPI_MUX_dmic1_clk, + LPI_MUX_i2s1_clk, + LPI_MUX_dmic1_data, + LPI_MUX_i2s1_ws, + LPI_MUX_dmic2_clk, + LPI_MUX_i2s1_data0, + LPI_MUX_dmic2_data, + LPI_MUX_i2s1_data1, + LPI_MUX_i2s2_clk, + LPI_MUX_wsa_swr_clk, + LPI_MUX_i2s2_ws, + LPI_MUX_wsa_swr_data, + LPI_MUX_dmic3_clk, + LPI_MUX_i2s2_data0, + LPI_MUX_dmic3_data, + LPI_MUX_i2s2_data1, + LPI_MUX_gpio, + LPI_MUX_NA, +}; + +static const unsigned int gpio0_pins[] = { 0 }; +static const unsigned int gpio1_pins[] = { 1 }; +static const unsigned int gpio2_pins[] = { 2 }; +static const unsigned int gpio3_pins[] = { 3 }; +static const unsigned int gpio4_pins[] = { 4 }; +static const unsigned int gpio5_pins[] = { 5 }; +static const unsigned int gpio6_pins[] = { 6 }; +static const unsigned int gpio7_pins[] = { 7 }; +static const unsigned int gpio8_pins[] = { 8 }; +static const unsigned int gpio9_pins[] = { 9 }; +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 char * const swr_tx_clk_groups[] = { "gpio0" }; +static const char * const swr_tx_data1_groups[] = { "gpio1" }; +static const char * const swr_tx_data2_groups[] = { "gpio2" }; +static const char * const swr_rx_clk_groups[] = { "gpio3" }; +static const char * const swr_rx_data1_groups[] = { "gpio4" }; +static const char * const swr_tx_data3_groups[] = { "gpio5" }; +static const char * const dmic1_clk_groups[] = { "gpio6" }; +static const char * const dmic1_data_groups[] = { "gpio7" }; +static const char * const dmic2_clk_groups[] = { "gpio8" }; +static const char * const dmic2_data_groups[] = { "gpio9" }; +static const char * const i2s2_clk_groups[] = { "gpio10" }; +static const char * const i2s2_ws_groups[] = { "gpio11" }; +static const char * const dmic3_clk_groups[] = { "gpio12" }; +static const char * const dmic3_data_groups[] = { "gpio13" }; +static const char * const qua_mi2s_sclk_groups[] = { "gpio0" }; +static const char * const qua_mi2s_ws_groups[] = { "gpio1" }; +static const char * const qua_mi2s_data0_groups[] = { "gpio2" }; +static const char * const qua_mi2s_data1_groups[] = { "gpio3" }; +static const char * const qua_mi2s_data2_groups[] = { "gpio4" }; +static const char * const swr_rx_data2_groups[] = { "gpio5" }; +static const char * const i2s1_clk_groups[] = { "gpio6" }; +static const char * const i2s1_ws_groups[] = { "gpio7" }; +static const char * const i2s1_data0_groups[] = { "gpio8" }; +static const char * const i2s1_data1_groups[] = { "gpio9" }; +static const char * const wsa_swr_clk_groups[] = { "gpio10" }; +static const char * const wsa_swr_data_groups[] = { "gpio11" }; +static const char * const i2s2_data0_groups[] = { "gpio12" }; +static const char * const i2s2_data1_groups[] = { "gpio13" }; + +static const struct lpi_pingroup sm8250_groups[] = { + LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, NA, NA), + LPI_PINGROUP(1, 2, swr_tx_data1, qua_mi2s_ws, NA, NA), + LPI_PINGROUP(2, 4, swr_tx_data2, qua_mi2s_data0, NA, NA), + LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data1, NA, NA), + LPI_PINGROUP(4, 10, swr_rx_data1, qua_mi2s_data2, NA, NA), + LPI_PINGROUP(5, 12, swr_tx_data3, swr_rx_data2, NA, NA), + LPI_PINGROUP(6, NO_SLEW, dmic1_clk, i2s1_clk, NA, NA), + LPI_PINGROUP(7, NO_SLEW, dmic1_data, i2s1_ws, NA, NA), + LPI_PINGROUP(8, NO_SLEW, dmic2_clk, i2s1_data0, NA, NA), + LPI_PINGROUP(9, NO_SLEW, dmic2_data, i2s1_data1, NA, NA), + LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, NA, NA), + LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, NA, NA), + LPI_PINGROUP(12, NO_SLEW, dmic3_clk, i2s2_data0, NA, NA), + LPI_PINGROUP(13, NO_SLEW, dmic3_data, i2s2_data1, NA, NA), +}; + +static const struct lpi_function sm8250_functions[] = { + LPI_FUNCTION(swr_tx_clk), + LPI_FUNCTION(qua_mi2s_sclk), + LPI_FUNCTION(swr_tx_data1), + LPI_FUNCTION(qua_mi2s_ws), + LPI_FUNCTION(swr_tx_data2), + LPI_FUNCTION(qua_mi2s_data0), + LPI_FUNCTION(swr_rx_clk), + LPI_FUNCTION(qua_mi2s_data1), + LPI_FUNCTION(swr_rx_data1), + LPI_FUNCTION(qua_mi2s_data2), + LPI_FUNCTION(swr_tx_data3), + LPI_FUNCTION(swr_rx_data2), + LPI_FUNCTION(dmic1_clk), + LPI_FUNCTION(i2s1_clk), + LPI_FUNCTION(dmic1_data), + LPI_FUNCTION(i2s1_ws), + LPI_FUNCTION(dmic2_clk), + LPI_FUNCTION(i2s1_data0), + LPI_FUNCTION(dmic2_data), + LPI_FUNCTION(i2s1_data1), + LPI_FUNCTION(i2s2_clk), + LPI_FUNCTION(wsa_swr_clk), + LPI_FUNCTION(i2s2_ws), + LPI_FUNCTION(wsa_swr_data), + LPI_FUNCTION(dmic3_clk), + LPI_FUNCTION(i2s2_data0), + LPI_FUNCTION(dmic3_data), + LPI_FUNCTION(i2s2_data1), +}; + +static struct lpi_pinctrl_variant_data sm8250_lpi_data = { + .tlmm_reg_offset = 0x1000, + .pins = sm8250_lpi_pins, + .npins = ARRAY_SIZE(sm8250_lpi_pins), + .groups = sm8250_groups, + .ngroups = ARRAY_SIZE(sm8250_groups), + .functions = sm8250_functions, + .nfunctions = ARRAY_SIZE(sm8250_functions), +}; + +static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin, + unsigned int addr) +{ + return ioread32(state->tlmm_base + + state->data->tlmm_reg_offset * pin + addr); +} + +static int lpi_gpio_write(struct lpi_pinctrl *state, unsigned int pin, + unsigned int addr, unsigned int val) +{ + iowrite32(val, state->tlmm_base + + state->data->tlmm_reg_offset * pin + addr); + + return 0; +} + +static int lpi_gpio_get_groups_count(struct pinctrl_dev *pctldev) +{ + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); + + return pctrl->data->ngroups; +} + +static const char *lpi_gpio_get_group_name(struct pinctrl_dev *pctldev, + unsigned int group) +{ + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); + + return pctrl->data->groups[group].name; +} + +static int lpi_gpio_get_group_pins(struct pinctrl_dev *pctldev, + unsigned int group, + const unsigned int **pins, + unsigned int *num_pins) +{ + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); + + *pins = pctrl->data->groups[group].pins; + *num_pins = pctrl->data->groups[group].npins; + + return 0; +} + +static const struct pinctrl_ops lpi_gpio_pinctrl_ops = { + .get_groups_count = lpi_gpio_get_groups_count, + .get_group_name = lpi_gpio_get_group_name, + .get_group_pins = lpi_gpio_get_group_pins, + .dt_node_to_map = pinconf_generic_dt_node_to_map_group, + .dt_free_map = pinctrl_utils_free_map, +}; + +static int lpi_gpio_get_functions_count(struct pinctrl_dev *pctldev) +{ + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); + + return pctrl->data->nfunctions; +} + +static const char *lpi_gpio_get_function_name(struct pinctrl_dev *pctldev, + unsigned int function) +{ + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); + + return pctrl->data->functions[function].name; +} + +static int lpi_gpio_get_function_groups(struct pinctrl_dev *pctldev, + unsigned int function, + const char *const **groups, + unsigned *const num_qgroups) +{ + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); + + *groups = pctrl->data->functions[function].groups; + *num_qgroups = pctrl->data->functions[function].ngroups; + + return 0; +} + +static int lpi_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function, + unsigned int group_num) +{ + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); + const struct lpi_pingroup *g = &pctrl->data->groups[group_num]; + unsigned int val; + int i, pin = g->pin; + + for (i = 0; i < g->nfuncs; i++) { + if (g->funcs[i] == function) + break; + } + + if (WARN_ON(i == g->nfuncs)) + return -EINVAL; + + val = lpi_gpio_read(pctrl, pin, LPI_GPIO_REG_VAL_CTL); + val &= ~LPI_GPIO_REG_FUNCTION_MASK; + val |= i << LPI_GPIO_REG_FUNCTION_SHIFT; + lpi_gpio_write(pctrl, pin, LPI_GPIO_REG_VAL_CTL, val); + + return 0; +} + +static const struct pinmux_ops lpi_gpio_pinmux_ops = { + .get_functions_count = lpi_gpio_get_functions_count, + .get_function_name = lpi_gpio_get_function_name, + .get_function_groups = lpi_gpio_get_function_groups, + .set_mux = lpi_gpio_set_mux, +}; + +static int lpi_config_get(struct pinctrl_dev *pctldev, + unsigned int pin, unsigned long *config) +{ + unsigned int param = pinconf_to_config_param(*config); + struct lpi_pinctrl *state = dev_get_drvdata(pctldev->dev); + unsigned int arg = 0; + int is_out; + int pull; + u32 ctl_reg; + + ctl_reg = lpi_gpio_read(state, pin, LPI_GPIO_REG_DIR_CTL); + + is_out = (ctl_reg & LPI_GPIO_REG_DIR_MASK) >> LPI_GPIO_REG_DIR_SHIFT; + + ctl_reg = lpi_gpio_read(state, pin, LPI_GPIO_REG_VAL_CTL); + + pull = (ctl_reg & LPI_GPIO_REG_PULL_MASK) >> LPI_GPIO_REG_PULL_SHIFT; + + switch (param) { + case PIN_CONFIG_BIAS_DISABLE: + if (pull == LPI_GPIO_BIAS_DISABLE) + arg = 1; + break; + case PIN_CONFIG_BIAS_PULL_DOWN: + if (pull == LPI_GPIO_PULL_DOWN) + arg = 1; + break; + case PIN_CONFIG_BIAS_BUS_HOLD: + if (pull == LPI_GPIO_KEEPER) + arg = 1; + break; + case PIN_CONFIG_BIAS_PULL_UP: + if (pull == LPI_GPIO_PULL_UP) + arg = 1; + break; + case PIN_CONFIG_INPUT_ENABLE: + case PIN_CONFIG_OUTPUT: + if (is_out) + arg = 1; + break; + default: + return -EINVAL; + } + + *config = pinconf_to_config_packed(param, arg); + return 0; +} + +static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group, + unsigned long *configs, unsigned int nconfs) +{ + struct lpi_pinctrl *pctrl = dev_get_drvdata(pctldev->dev); + unsigned int param, arg, pullup, strength; + const struct lpi_pingroup *g; + bool value, output_enabled = false; + unsigned long val; + int i, slew_offset, ret = 0; + + g = &pctrl->data->groups[group]; + for (i = 0; i < nconfs; i++) { + param = pinconf_to_config_param(configs[i]); + arg = pinconf_to_config_argument(configs[i]); + + switch (param) { + case PIN_CONFIG_BIAS_DISABLE: + pullup = LPI_GPIO_BIAS_DISABLE; + break; + case PIN_CONFIG_BIAS_PULL_DOWN: + pullup = LPI_GPIO_PULL_DOWN; + break; + case PIN_CONFIG_BIAS_BUS_HOLD: + pullup = LPI_GPIO_KEEPER; + break; + case PIN_CONFIG_BIAS_PULL_UP: + pullup = LPI_GPIO_PULL_UP; + break; + case PIN_CONFIG_INPUT_ENABLE: + output_enabled = false; + break; + case PIN_CONFIG_OUTPUT: + output_enabled = true; + lpi_gpio_write(pctrl, group, LPI_GPIO_REG_DIR_CTL, + output_enabled << LPI_GPIO_REG_DIR_SHIFT); + value = arg; + break; + case PIN_CONFIG_DRIVE_STRENGTH: + strength = arg; + break; + case PIN_CONFIG_SLEW_RATE: + if (arg > LPI_SLEW_RATE_MAX) { + dev_err(pctldev->dev, "invalid slew rate %u for pin: %d\n", + arg, group); + goto set_gpio; + } + + slew_offset = g->slew_offset; + if (slew_offset == NO_SLEW) + break; + + mutex_lock(&pctrl->slew_access_lock); + val = ioread32(pctrl->slew_base + LPI_SLEW_REG_VAL_CTL); + + for (i = 0; i < LPI_SLEW_BITS_SIZE; i++) { + assign_bit(slew_offset, &val, arg & 0x01); + slew_offset++; + arg = arg >> 1; + } + + iowrite32(val, pctrl->slew_base + LPI_SLEW_REG_VAL_CTL); + + mutex_unlock(&pctrl->slew_access_lock); + break; + default: + return -EINVAL; + } + } + +set_gpio: + val = lpi_gpio_read(pctrl, group, LPI_GPIO_REG_VAL_CTL); + val &= ~(LPI_GPIO_REG_PULL_MASK | LPI_GPIO_REG_OUT_STRENGTH_MASK | + LPI_GPIO_REG_OE_MASK); + val |= pullup << LPI_GPIO_REG_PULL_SHIFT; + val |= LPI_GPIO_DS_TO_VAL(strength); + if (output_enabled) + val |= value << LPI_GPIO_REG_OE_SHIFT; + + lpi_gpio_write(pctrl, group, LPI_GPIO_REG_VAL_CTL, val); + lpi_gpio_write(pctrl, group, LPI_GPIO_REG_DIR_CTL, + output_enabled << LPI_GPIO_REG_DIR_SHIFT); + + return ret; +} + +static const struct pinconf_ops lpi_gpio_pinconf_ops = { + .is_generic = true, + .pin_config_group_get = lpi_config_get, + .pin_config_group_set = lpi_config_set, +}; + +static int lpi_gpio_direction_input(struct gpio_chip *chip, unsigned int pin) +{ + struct lpi_pinctrl *state = gpiochip_get_data(chip); + unsigned long config; + + config = pinconf_to_config_packed(PIN_CONFIG_INPUT_ENABLE, 1); + + return lpi_config_set(state->ctrl, pin, &config, 1); +} + +static int lpi_gpio_direction_output(struct gpio_chip *chip, + unsigned int pin, int val) +{ + struct lpi_pinctrl *state = gpiochip_get_data(chip); + unsigned long config; + + config = pinconf_to_config_packed(PIN_CONFIG_OUTPUT, val); + + return lpi_config_set(state->ctrl, pin, &config, 1); +} + +static int lpi_gpio_get(struct gpio_chip *chip, unsigned int pin) +{ + struct lpi_pinctrl *state = gpiochip_get_data(chip); + int value; + + value = lpi_gpio_read(state, pin, LPI_GPIO_REG_VAL_CTL); + return value; +} + +static void lpi_gpio_set(struct gpio_chip *chip, unsigned int pin, int value) +{ + struct lpi_pinctrl *state = gpiochip_get_data(chip); + unsigned long config; + + config = pinconf_to_config_packed(PIN_CONFIG_OUTPUT, value); + + lpi_config_set(state->ctrl, pin, &config, 1); +} + +#ifdef CONFIG_DEBUG_FS +#include <linux/seq_file.h> + +static unsigned int lpi_regval_to_drive(u32 val) +{ + return (val + 1) * 2; +} + +static void lpi_gpio_dbg_show_one(struct seq_file *s, + struct pinctrl_dev *pctldev, + struct gpio_chip *chip, + unsigned int offset, + unsigned int gpio) +{ + struct lpi_pinctrl *state = gpiochip_get_data(chip); + struct pinctrl_pin_desc pindesc; + unsigned int func; + int is_out; + int drive; + int pull; + u32 ctl_reg; + + static const char * const pulls[] = { + "no pull", + "pull down", + "keeper", + "pull up" + }; + + pctldev = pctldev ? : state->ctrl; + pindesc = pctldev->desc->pins[offset]; + ctl_reg = lpi_gpio_read(state, offset, LPI_GPIO_REG_DIR_CTL); + is_out = (ctl_reg & LPI_GPIO_REG_DIR_MASK) >> LPI_GPIO_REG_DIR_SHIFT; + ctl_reg = lpi_gpio_read(state, offset, LPI_GPIO_REG_VAL_CTL); + + func = (ctl_reg & LPI_GPIO_REG_FUNCTION_MASK) >> + LPI_GPIO_REG_FUNCTION_SHIFT; + drive = (ctl_reg & LPI_GPIO_REG_OUT_STRENGTH_MASK) >> + LPI_GPIO_REG_OUT_STRENGTH_SHIFT; + pull = (ctl_reg & LPI_GPIO_REG_PULL_MASK) >> LPI_GPIO_REG_PULL_SHIFT; + + seq_printf(s, " %-8s: %-3s %d", + pindesc.name, is_out ? "out" : "in", func); + seq_printf(s, " %dmA", lpi_regval_to_drive(drive)); + seq_printf(s, " %s", pulls[pull]); +} + +static void lpi_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip) +{ + unsigned int gpio = chip->base; + unsigned int i; + + for (i = 0; i < chip->ngpio; i++, gpio++) { + lpi_gpio_dbg_show_one(s, NULL, chip, i, gpio); + seq_puts(s, "\n"); + } +} + +#else +#define lpi_gpio_dbg_show NULL +#endif + +static const struct gpio_chip lpi_gpio_template = { + .direction_input = lpi_gpio_direction_input, + .direction_output = lpi_gpio_direction_output, + .get = lpi_gpio_get, + .set = lpi_gpio_set, + .request = gpiochip_generic_request, + .free = gpiochip_generic_free, + .dbg_show = lpi_gpio_dbg_show, +}; + +static int lpi_pinctrl_probe(struct platform_device *pdev) +{ + const struct lpi_pinctrl_variant_data *data; + struct device *dev = &pdev->dev; + struct lpi_pinctrl *pctrl; + int ret; + + pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL); + if (!pctrl) + return -ENOMEM; + + platform_set_drvdata(pdev, pctrl); + + data = of_device_get_match_data(dev); + if (!data) + return -EINVAL; + + pctrl->data = data; + pctrl->dev = &pdev->dev; + + pctrl->clks[0].id = "core"; + pctrl->clks[1].id = "audio"; + + pctrl->tlmm_base = devm_platform_ioremap_resource(pdev, 0); + if (IS_ERR(pctrl->tlmm_base)) + return dev_err_probe(dev, PTR_ERR(pctrl->tlmm_base), + "TLMM resource not provided\n"); + + pctrl->slew_base = devm_platform_ioremap_resource(pdev, 1); + if (IS_ERR(pctrl->slew_base)) + return dev_err_probe(dev, PTR_ERR(pctrl->slew_base), + "Slew resource not provided\n"); + + ret = devm_clk_bulk_get(dev, MAX_LPI_NUM_CLKS, pctrl->clks); + if (ret) + return dev_err_probe(dev, ret, "Can't get clocks\n"); + + ret = clk_bulk_prepare_enable(MAX_LPI_NUM_CLKS, pctrl->clks); + if (ret) + return dev_err_probe(dev, ret, "Can't enable clocks\n"); + + pctrl->desc.pctlops = &lpi_gpio_pinctrl_ops; + pctrl->desc.pmxops = &lpi_gpio_pinmux_ops; + pctrl->desc.confops = &lpi_gpio_pinconf_ops; + pctrl->desc.owner = THIS_MODULE; + pctrl->desc.name = dev_name(dev); + pctrl->desc.pins = data->pins; + pctrl->desc.npins = data->npins; + pctrl->chip = lpi_gpio_template; + pctrl->chip.parent = dev; + pctrl->chip.base = -1; + pctrl->chip.ngpio = data->npins; + pctrl->chip.label = dev_name(dev); + pctrl->chip.of_gpio_n_cells = 2; + pctrl->chip.can_sleep = false; + + mutex_init(&pctrl->slew_access_lock); + + pctrl->ctrl = devm_pinctrl_register(dev, &pctrl->desc, pctrl); + if (IS_ERR(pctrl->ctrl)) { + ret = PTR_ERR(pctrl->ctrl); + dev_err(dev, "failed to add pin controller\n"); + goto err_pinctrl; + } + + ret = devm_gpiochip_add_data(dev, &pctrl->chip, pctrl); + if (ret) { + dev_err(pctrl->dev, "can't add gpio chip\n"); + goto err_pinctrl; + } + + return 0; + +err_pinctrl: + mutex_destroy(&pctrl->slew_access_lock); + clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks); + + return ret; +} + +static int lpi_pinctrl_remove(struct platform_device *pdev) +{ + struct lpi_pinctrl *pctrl = platform_get_drvdata(pdev); + + mutex_destroy(&pctrl->slew_access_lock); + clk_bulk_disable_unprepare(MAX_LPI_NUM_CLKS, pctrl->clks); + + return 0; +} + +static const struct of_device_id lpi_pinctrl_of_match[] = { + { + .compatible = "qcom,sm8250-lpass-lpi-pinctrl", + .data = &sm8250_lpi_data, + }, + { } +}; +MODULE_DEVICE_TABLE(of, lpi_pinctrl_of_match); + +static struct platform_driver lpi_pinctrl_driver = { + .driver = { + .name = "qcom-lpass-lpi-pinctrl", + .of_match_table = lpi_pinctrl_of_match, + }, + .probe = lpi_pinctrl_probe, + .remove = lpi_pinctrl_remove, +}; + +module_platform_driver(lpi_pinctrl_driver); +MODULE_DESCRIPTION("QTI LPI GPIO pin control driver"); +MODULE_LICENSE("GPL"); -- 2.21.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver 2020-11-16 14:34 ` [PATCH v4 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver Srinivas Kandagatla @ 2020-12-01 0:47 ` Bjorn Andersson 2020-12-01 10:01 ` Srinivas Kandagatla 0 siblings, 1 reply; 10+ messages in thread From: Bjorn Andersson @ 2020-12-01 0:47 UTC (permalink / raw) To: Srinivas Kandagatla Cc: linus.walleij, robh+dt, agross, linux-arm-msm, linux-gpio, devicetree, linux-kernel On Mon 16 Nov 08:34 CST 2020, Srinivas Kandagatla wrote: > Add initial pinctrl driver to support pin configuration for > LPASS (Low Power Audio SubSystem) LPI (Low Power Island) pinctrl > on SM8250. > > This IP is an additional pin control block for Audio Pins on top the > existing SoC Top level pin-controller. > Hardware setup looks like: > > TLMM GPIO[146 - 159] --> LPASS LPI GPIO [0 - 13] > Iiuc the LPI TLMM block is just "another pinmux/pinconf block" found in these SoCs, with the additional magic that the 14 pads are muxed with some of the TLMM pins - to allow the system integrator to choose how many pins the LPI should have access to. I also believe this is what the "egpio" bit in the TLMM registers are used for (i.e. egpio = route to LPI, egpio = 1 route to TLMM), so we should need to add support for toggling this bit in the TLMM as well (which I think we should do as a pinconf in the pinctrl-msm). > This pin controller has some similarities compared to Top level > msm SoC Pin controller like 'each pin belongs to a single group' > and so on. However this one is intended to control only audio > pins in particular, which can not be configured/touched by the > Top level SoC pin controller except setting them as gpios. Seems like this is just a property of what functions they routed in this region of the TLMM - and when egpio = 1 it could have been anything else. > Apart from this, slew rate is also available in this block for > certain pins which are connected to SLIMbus or SoundWire Bus. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/pinctrl/qcom/Kconfig | 8 + > drivers/pinctrl/qcom/Makefile | 1 + > drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 734 +++++++++++++++++++++++ > 3 files changed, 743 insertions(+) > create mode 100644 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c > > diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig > index 5fe7b8aaf69d..d3e4e89c2810 100644 > --- a/drivers/pinctrl/qcom/Kconfig > +++ b/drivers/pinctrl/qcom/Kconfig > @@ -236,4 +236,12 @@ config PINCTRL_SM8250 > Qualcomm Technologies Inc TLMM block found on the Qualcomm > Technologies Inc SM8250 platform. > > +config PINCTRL_LPASS_LPI > + tristate "Qualcomm Technologies Inc LPASS LPI pin controller driver" > + depends on GPIOLIB > + 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 SoCs. > + > endif > diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile > index 9e3d9c91a444..c8520155fb1b 100644 > --- a/drivers/pinctrl/qcom/Makefile > +++ b/drivers/pinctrl/qcom/Makefile > @@ -28,3 +28,4 @@ obj-$(CONFIG_PINCTRL_SDM660) += pinctrl-sdm660.o > obj-$(CONFIG_PINCTRL_SDM845) += pinctrl-sdm845.o > obj-$(CONFIG_PINCTRL_SM8150) += pinctrl-sm8150.o > obj-$(CONFIG_PINCTRL_SM8250) += pinctrl-sm8250.o > +obj-$(CONFIG_PINCTRL_LPASS_LPI) += pinctrl-lpass-lpi.o > diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c > new file mode 100644 > index 000000000000..63cfbb2d032a > --- /dev/null > +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c > @@ -0,0 +1,734 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved. > + * Copyright (c) 2020 Linaro Ltd. > + */ > + > +#include <linux/bitops.h> > +#include <linux/clk.h> > +#include <linux/gpio/driver.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <linux/of.h> > +#include <linux/pinctrl/pinconf-generic.h> > +#include <linux/pinctrl/pinconf.h> > +#include <linux/pinctrl/pinmux.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/types.h> > +#include "../core.h" > +#include "../pinctrl-utils.h" > + > +#define LPI_GPIO_REG_VAL_CTL 0x00 I think LPI_GPIO_CFG_REG would be a better name for this. > +#define LPI_GPIO_REG_DIR_CTL 0x04 Afaict, BIT(9) of LPI_GPIO_CFG_REG controls of the if the pin should be output or not. This register provides the read value as BIT(0) and the output value in BIT(1). So this would rather be the LPI_GPIO_VALUE_REG. > +#define LPI_SLEW_REG_VAL_CTL 0x00 > +#define LPI_SLEW_RATE_MAX 0x03 > +#define LPI_SLEW_BITS_SIZE 0x02 > +#define LPI_GPIO_REG_PULL_SHIFT 0x0 > +#define LPI_GPIO_REG_PULL_MASK GENMASK(1, 0) > +#define LPI_GPIO_REG_FUNCTION_SHIFT 0x2 > +#define LPI_GPIO_REG_FUNCTION_MASK GENMASK(5, 2) > +#define LPI_GPIO_REG_OUT_STRENGTH_SHIFT 0x6 > +#define LPI_GPIO_REG_OUT_STRENGTH_MASK GENMASK(8, 6) > +#define LPI_GPIO_REG_OE_SHIFT 0x9 > +#define LPI_GPIO_REG_OE_MASK BIT(9) > +#define LPI_GPIO_REG_DIR_SHIFT 0x1 > +#define LPI_GPIO_REG_DIR_MASK 0x2 > +#define LPI_GPIO_BIAS_DISABLE 0x0 > +#define LPI_GPIO_PULL_DOWN 0x1 > +#define LPI_GPIO_KEEPER 0x2 > +#define LPI_GPIO_PULL_UP 0x3 > +#define LPI_GPIO_DS_TO_VAL(v) ((v / 2 - 1) << LPI_GPIO_REG_OUT_STRENGTH_SHIFT) > +#define NO_SLEW -1 > + > +#define LPI_FUNCTION(fname) \ > + [LPI_MUX_##fname] = { \ > + .name = #fname, \ > + .groups = fname##_groups, \ > + .ngroups = ARRAY_SIZE(fname##_groups), \ > + } > + > +#define LPI_PINGROUP(id, soff, f1, f2, f3, f4) \ > + { \ > + .name = "gpio" #id, \ > + .pins = gpio##id##_pins, \ > + .pin = id, \ > + .slew_offset = soff, \ > + .npins = ARRAY_SIZE(gpio##id##_pins), \ > + .funcs = (int[]){ \ > + LPI_MUX_gpio, \ > + LPI_MUX_##f1, \ > + LPI_MUX_##f2, \ > + LPI_MUX_##f3, \ > + LPI_MUX_##f4, \ > + }, \ > + .nfuncs = 5, \ > + } > + > +struct lpi_pingroup { > + const char *name; > + const unsigned int *pins; > + unsigned int npins; > + unsigned int pin; > + /* Bit offset in slew register for SoundWire pins only */ > + unsigned int slew_offset; > + unsigned int *funcs; > + unsigned int nfuncs; > +}; > + > +struct lpi_function { > + const char *name; > + const char * const *groups; > + unsigned int ngroups; > +}; > + > +struct lpi_pinctrl_variant_data { > + int tlmm_reg_offset; > + const struct pinctrl_pin_desc *pins; > + int npins; > + const struct lpi_pingroup *groups; > + int ngroups; > + const struct lpi_function *functions; > + int nfunctions; > +}; > + > +#define MAX_LPI_NUM_CLKS 2 > + > +struct lpi_pinctrl { > + struct device *dev; > + struct pinctrl_dev *ctrl; > + struct gpio_chip chip; > + struct pinctrl_desc desc; > + char __iomem *tlmm_base; > + char __iomem *slew_base; > + struct clk_bulk_data clks[MAX_LPI_NUM_CLKS]; > + struct mutex slew_access_lock; > + const struct lpi_pinctrl_variant_data *data; > +}; > + > +/* sm8250 variant specific data */ > +static const struct pinctrl_pin_desc sm8250_lpi_pins[] = { > + PINCTRL_PIN(0, "gpio0"), > + PINCTRL_PIN(1, "gpio1"), > + PINCTRL_PIN(2, "gpio2"), > + PINCTRL_PIN(3, "gpio3"), > + PINCTRL_PIN(4, "gpio4"), > + PINCTRL_PIN(5, "gpio5"), > + PINCTRL_PIN(6, "gpio6"), > + PINCTRL_PIN(7, "gpio7"), > + PINCTRL_PIN(8, "gpio8"), > + PINCTRL_PIN(9, "gpio9"), > + PINCTRL_PIN(10, "gpio10"), > + PINCTRL_PIN(11, "gpio11"), > + PINCTRL_PIN(12, "gpio12"), > + PINCTRL_PIN(13, "gpio13"), > +}; > + > +enum sm8250_lpi_functions { > + LPI_MUX_swr_tx_clk, Please sort these. > + LPI_MUX_qua_mi2s_sclk, > + LPI_MUX_swr_tx_data1, As there's no single pin that can be both data1 and data2 I think you should have a single group for swr_tx_data and use this function for both swr_tx_data pins. Or perhaps even just have one for swr or swr_tx. (This is nice when you're writing DT later on) > + LPI_MUX_qua_mi2s_ws, > + LPI_MUX_swr_tx_data2, > + LPI_MUX_qua_mi2s_data0, > + LPI_MUX_swr_rx_clk, > + LPI_MUX_qua_mi2s_data1, > + LPI_MUX_swr_rx_data1, > + LPI_MUX_qua_mi2s_data2, > + LPI_MUX_swr_tx_data3, > + LPI_MUX_swr_rx_data2, > + LPI_MUX_dmic1_clk, > + LPI_MUX_i2s1_clk, > + LPI_MUX_dmic1_data, > + LPI_MUX_i2s1_ws, > + LPI_MUX_dmic2_clk, > + LPI_MUX_i2s1_data0, > + LPI_MUX_dmic2_data, > + LPI_MUX_i2s1_data1, > + LPI_MUX_i2s2_clk, > + LPI_MUX_wsa_swr_clk, > + LPI_MUX_i2s2_ws, > + LPI_MUX_wsa_swr_data, > + LPI_MUX_dmic3_clk, > + LPI_MUX_i2s2_data0, > + LPI_MUX_dmic3_data, > + LPI_MUX_i2s2_data1, > + LPI_MUX_gpio, > + LPI_MUX_NA, For me replacing "NA" with "_" makes the lpi_pingroup table below easier to read (i.e. this becomes LPI_MUX__). > +}; > + > +static const unsigned int gpio0_pins[] = { 0 }; > +static const unsigned int gpio1_pins[] = { 1 }; > +static const unsigned int gpio2_pins[] = { 2 }; > +static const unsigned int gpio3_pins[] = { 3 }; > +static const unsigned int gpio4_pins[] = { 4 }; > +static const unsigned int gpio5_pins[] = { 5 }; > +static const unsigned int gpio6_pins[] = { 6 }; > +static const unsigned int gpio7_pins[] = { 7 }; > +static const unsigned int gpio8_pins[] = { 8 }; > +static const unsigned int gpio9_pins[] = { 9 }; > +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 char * const swr_tx_clk_groups[] = { "gpio0" }; > +static const char * const swr_tx_data1_groups[] = { "gpio1" }; > +static const char * const swr_tx_data2_groups[] = { "gpio2" }; > +static const char * const swr_rx_clk_groups[] = { "gpio3" }; > +static const char * const swr_rx_data1_groups[] = { "gpio4" }; > +static const char * const swr_tx_data3_groups[] = { "gpio5" }; > +static const char * const dmic1_clk_groups[] = { "gpio6" }; > +static const char * const dmic1_data_groups[] = { "gpio7" }; > +static const char * const dmic2_clk_groups[] = { "gpio8" }; > +static const char * const dmic2_data_groups[] = { "gpio9" }; > +static const char * const i2s2_clk_groups[] = { "gpio10" }; > +static const char * const i2s2_ws_groups[] = { "gpio11" }; > +static const char * const dmic3_clk_groups[] = { "gpio12" }; > +static const char * const dmic3_data_groups[] = { "gpio13" }; > +static const char * const qua_mi2s_sclk_groups[] = { "gpio0" }; > +static const char * const qua_mi2s_ws_groups[] = { "gpio1" }; > +static const char * const qua_mi2s_data0_groups[] = { "gpio2" }; > +static const char * const qua_mi2s_data1_groups[] = { "gpio3" }; > +static const char * const qua_mi2s_data2_groups[] = { "gpio4" }; > +static const char * const swr_rx_data2_groups[] = { "gpio5" }; > +static const char * const i2s1_clk_groups[] = { "gpio6" }; > +static const char * const i2s1_ws_groups[] = { "gpio7" }; > +static const char * const i2s1_data0_groups[] = { "gpio8" }; > +static const char * const i2s1_data1_groups[] = { "gpio9" }; > +static const char * const wsa_swr_clk_groups[] = { "gpio10" }; > +static const char * const wsa_swr_data_groups[] = { "gpio11" }; > +static const char * const i2s2_data0_groups[] = { "gpio12" }; > +static const char * const i2s2_data1_groups[] = { "gpio13" }; > + > +static const struct lpi_pingroup sm8250_groups[] = { > + LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, NA, NA), > + LPI_PINGROUP(1, 2, swr_tx_data1, qua_mi2s_ws, NA, NA), > + LPI_PINGROUP(2, 4, swr_tx_data2, qua_mi2s_data0, NA, NA), > + LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data1, NA, NA), > + LPI_PINGROUP(4, 10, swr_rx_data1, qua_mi2s_data2, NA, NA), > + LPI_PINGROUP(5, 12, swr_tx_data3, swr_rx_data2, NA, NA), > + LPI_PINGROUP(6, NO_SLEW, dmic1_clk, i2s1_clk, NA, NA), > + LPI_PINGROUP(7, NO_SLEW, dmic1_data, i2s1_ws, NA, NA), > + LPI_PINGROUP(8, NO_SLEW, dmic2_clk, i2s1_data0, NA, NA), > + LPI_PINGROUP(9, NO_SLEW, dmic2_data, i2s1_data1, NA, NA), > + LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, NA, NA), > + LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, NA, NA), > + LPI_PINGROUP(12, NO_SLEW, dmic3_clk, i2s2_data0, NA, NA), > + LPI_PINGROUP(13, NO_SLEW, dmic3_data, i2s2_data1, NA, NA), > +}; > + > +static const struct lpi_function sm8250_functions[] = { > + LPI_FUNCTION(swr_tx_clk), Please sort these. > + LPI_FUNCTION(qua_mi2s_sclk), > + LPI_FUNCTION(swr_tx_data1), > + LPI_FUNCTION(qua_mi2s_ws), > + LPI_FUNCTION(swr_tx_data2), > + LPI_FUNCTION(qua_mi2s_data0), > + LPI_FUNCTION(swr_rx_clk), > + LPI_FUNCTION(qua_mi2s_data1), > + LPI_FUNCTION(swr_rx_data1), > + LPI_FUNCTION(qua_mi2s_data2), > + LPI_FUNCTION(swr_tx_data3), > + LPI_FUNCTION(swr_rx_data2), > + LPI_FUNCTION(dmic1_clk), > + LPI_FUNCTION(i2s1_clk), > + LPI_FUNCTION(dmic1_data), > + LPI_FUNCTION(i2s1_ws), > + LPI_FUNCTION(dmic2_clk), > + LPI_FUNCTION(i2s1_data0), > + LPI_FUNCTION(dmic2_data), > + LPI_FUNCTION(i2s1_data1), > + LPI_FUNCTION(i2s2_clk), > + LPI_FUNCTION(wsa_swr_clk), > + LPI_FUNCTION(i2s2_ws), > + LPI_FUNCTION(wsa_swr_data), > + LPI_FUNCTION(dmic3_clk), > + LPI_FUNCTION(i2s2_data0), > + LPI_FUNCTION(dmic3_data), > + LPI_FUNCTION(i2s2_data1), > +}; > + > +static struct lpi_pinctrl_variant_data sm8250_lpi_data = { > + .tlmm_reg_offset = 0x1000, Do we have any platform in sight where this is not 0x1000? Could we just make a define out of it? > + .pins = sm8250_lpi_pins, > + .npins = ARRAY_SIZE(sm8250_lpi_pins), > + .groups = sm8250_groups, > + .ngroups = ARRAY_SIZE(sm8250_groups), > + .functions = sm8250_functions, > + .nfunctions = ARRAY_SIZE(sm8250_functions), > +}; > + > +static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin, > + unsigned int addr) > +{ > + return ioread32(state->tlmm_base + > + state->data->tlmm_reg_offset * pin + addr); > +} > + > +static int lpi_gpio_write(struct lpi_pinctrl *state, unsigned int pin, > + unsigned int addr, unsigned int val) > +{ > + iowrite32(val, state->tlmm_base + > + state->data->tlmm_reg_offset * pin + addr); > + > + return 0; > +} > + > +static int lpi_gpio_get_groups_count(struct pinctrl_dev *pctldev) > +{ > + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > + > + return pctrl->data->ngroups; > +} > + > +static const char *lpi_gpio_get_group_name(struct pinctrl_dev *pctldev, > + unsigned int group) > +{ > + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > + > + return pctrl->data->groups[group].name; > +} > + > +static int lpi_gpio_get_group_pins(struct pinctrl_dev *pctldev, > + unsigned int group, > + const unsigned int **pins, > + unsigned int *num_pins) > +{ > + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > + > + *pins = pctrl->data->groups[group].pins; > + *num_pins = pctrl->data->groups[group].npins; > + > + return 0; > +} > + > +static const struct pinctrl_ops lpi_gpio_pinctrl_ops = { > + .get_groups_count = lpi_gpio_get_groups_count, > + .get_group_name = lpi_gpio_get_group_name, > + .get_group_pins = lpi_gpio_get_group_pins, > + .dt_node_to_map = pinconf_generic_dt_node_to_map_group, > + .dt_free_map = pinctrl_utils_free_map, > +}; > + > +static int lpi_gpio_get_functions_count(struct pinctrl_dev *pctldev) > +{ > + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > + > + return pctrl->data->nfunctions; > +} > + > +static const char *lpi_gpio_get_function_name(struct pinctrl_dev *pctldev, > + unsigned int function) > +{ > + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > + > + return pctrl->data->functions[function].name; > +} > + > +static int lpi_gpio_get_function_groups(struct pinctrl_dev *pctldev, > + unsigned int function, > + const char *const **groups, > + unsigned *const num_qgroups) > +{ > + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > + > + *groups = pctrl->data->functions[function].groups; > + *num_qgroups = pctrl->data->functions[function].ngroups; > + > + return 0; > +} > + > +static int lpi_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function, > + unsigned int group_num) > +{ > + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); > + const struct lpi_pingroup *g = &pctrl->data->groups[group_num]; > + unsigned int val; > + int i, pin = g->pin; > + > + for (i = 0; i < g->nfuncs; i++) { > + if (g->funcs[i] == function) > + break; > + } > + > + if (WARN_ON(i == g->nfuncs)) > + return -EINVAL; > + > + val = lpi_gpio_read(pctrl, pin, LPI_GPIO_REG_VAL_CTL); > + val &= ~LPI_GPIO_REG_FUNCTION_MASK; > + val |= i << LPI_GPIO_REG_FUNCTION_SHIFT; I think this would benefit from FIELD_SET() > + lpi_gpio_write(pctrl, pin, LPI_GPIO_REG_VAL_CTL, val); > + > + return 0; > +} > + > +static const struct pinmux_ops lpi_gpio_pinmux_ops = { > + .get_functions_count = lpi_gpio_get_functions_count, > + .get_function_name = lpi_gpio_get_function_name, > + .get_function_groups = lpi_gpio_get_function_groups, > + .set_mux = lpi_gpio_set_mux, > +}; > + > +static int lpi_config_get(struct pinctrl_dev *pctldev, > + unsigned int pin, unsigned long *config) > +{ > + unsigned int param = pinconf_to_config_param(*config); > + struct lpi_pinctrl *state = dev_get_drvdata(pctldev->dev); > + unsigned int arg = 0; > + int is_out; > + int pull; > + u32 ctl_reg; > + > + ctl_reg = lpi_gpio_read(state, pin, LPI_GPIO_REG_DIR_CTL); > + > + is_out = (ctl_reg & LPI_GPIO_REG_DIR_MASK) >> LPI_GPIO_REG_DIR_SHIFT; This is a single bit, which would be clarified if you replaced _MASK and _SHIFT with a single LPI_GPIO_OUT BIT(1) > + > + ctl_reg = lpi_gpio_read(state, pin, LPI_GPIO_REG_VAL_CTL); > + > + pull = (ctl_reg & LPI_GPIO_REG_PULL_MASK) >> LPI_GPIO_REG_PULL_SHIFT; > + > + switch (param) { > + case PIN_CONFIG_BIAS_DISABLE: > + if (pull == LPI_GPIO_BIAS_DISABLE) > + arg = 1; > + break; > + case PIN_CONFIG_BIAS_PULL_DOWN: > + if (pull == LPI_GPIO_PULL_DOWN) > + arg = 1; > + break; > + case PIN_CONFIG_BIAS_BUS_HOLD: > + if (pull == LPI_GPIO_KEEPER) > + arg = 1; > + break; > + case PIN_CONFIG_BIAS_PULL_UP: > + if (pull == LPI_GPIO_PULL_UP) > + arg = 1; > + break; > + case PIN_CONFIG_INPUT_ENABLE: > + case PIN_CONFIG_OUTPUT: > + if (is_out) > + arg = 1; > + break; > + default: > + return -EINVAL; > + } > + > + *config = pinconf_to_config_packed(param, arg); > + return 0; > +} > + > +static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group, > + unsigned long *configs, unsigned int nconfs) > +{ > + struct lpi_pinctrl *pctrl = dev_get_drvdata(pctldev->dev); > + unsigned int param, arg, pullup, strength; > + const struct lpi_pingroup *g; > + bool value, output_enabled = false; > + unsigned long val; > + int i, slew_offset, ret = 0; > + > + g = &pctrl->data->groups[group]; > + for (i = 0; i < nconfs; i++) { > + param = pinconf_to_config_param(configs[i]); > + arg = pinconf_to_config_argument(configs[i]); > + > + switch (param) { > + case PIN_CONFIG_BIAS_DISABLE: > + pullup = LPI_GPIO_BIAS_DISABLE; > + break; > + case PIN_CONFIG_BIAS_PULL_DOWN: > + pullup = LPI_GPIO_PULL_DOWN; > + break; > + case PIN_CONFIG_BIAS_BUS_HOLD: > + pullup = LPI_GPIO_KEEPER; > + break; > + case PIN_CONFIG_BIAS_PULL_UP: > + pullup = LPI_GPIO_PULL_UP; > + break; > + case PIN_CONFIG_INPUT_ENABLE: > + output_enabled = false; > + break; > + case PIN_CONFIG_OUTPUT: > + output_enabled = true; > + lpi_gpio_write(pctrl, group, LPI_GPIO_REG_DIR_CTL, > + output_enabled << LPI_GPIO_REG_DIR_SHIFT); Is there a reason why you write this both here and at the bottom of the function? > + value = arg; > + break; > + case PIN_CONFIG_DRIVE_STRENGTH: > + strength = arg; > + break; > + case PIN_CONFIG_SLEW_RATE: > + if (arg > LPI_SLEW_RATE_MAX) { > + dev_err(pctldev->dev, "invalid slew rate %u for pin: %d\n", > + arg, group); > + goto set_gpio; So if I set an invalid slew rate the remainder of the pinconf/mux properties will be ignored and the pin configured with whatever came before? Wouldn't either return -EINVAL or perhaps just a break make more sense here? > + } > + > + slew_offset = g->slew_offset; > + if (slew_offset == NO_SLEW) > + break; > + > + mutex_lock(&pctrl->slew_access_lock); > + val = ioread32(pctrl->slew_base + LPI_SLEW_REG_VAL_CTL); > + > + for (i = 0; i < LPI_SLEW_BITS_SIZE; i++) { > + assign_bit(slew_offset, &val, arg & 0x01); > + slew_offset++; > + arg = arg >> 1; > + } > + > + iowrite32(val, pctrl->slew_base + LPI_SLEW_REG_VAL_CTL); > + > + mutex_unlock(&pctrl->slew_access_lock); > + break; > + default: > + return -EINVAL; > + } > + } > + > +set_gpio: > + val = lpi_gpio_read(pctrl, group, LPI_GPIO_REG_VAL_CTL); > + val &= ~(LPI_GPIO_REG_PULL_MASK | LPI_GPIO_REG_OUT_STRENGTH_MASK | > + LPI_GPIO_REG_OE_MASK); > + val |= pullup << LPI_GPIO_REG_PULL_SHIFT; > + val |= LPI_GPIO_DS_TO_VAL(strength); FIELD_SET() would make this prettier. > + if (output_enabled) > + val |= value << LPI_GPIO_REG_OE_SHIFT; > + > + lpi_gpio_write(pctrl, group, LPI_GPIO_REG_VAL_CTL, val); > + lpi_gpio_write(pctrl, group, LPI_GPIO_REG_DIR_CTL, > + output_enabled << LPI_GPIO_REG_DIR_SHIFT); > + > + return ret; ret is untouched since you set it to 0 at the beginning. Regards, Bjorn > +} > + ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver 2020-12-01 0:47 ` Bjorn Andersson @ 2020-12-01 10:01 ` Srinivas Kandagatla 2020-12-01 17:28 ` Bjorn Andersson 0 siblings, 1 reply; 10+ messages in thread From: Srinivas Kandagatla @ 2020-12-01 10:01 UTC (permalink / raw) To: Bjorn Andersson Cc: linus.walleij, robh+dt, agross, linux-arm-msm, linux-gpio, devicetree, linux-kernel Many thanks for review Bjorn, On 01/12/2020 00:47, Bjorn Andersson wrote: > On Mon 16 Nov 08:34 CST 2020, Srinivas Kandagatla wrote: > >> Add initial pinctrl driver to support pin configuration for >> LPASS (Low Power Audio SubSystem) LPI (Low Power Island) pinctrl >> on SM8250. >> >> This IP is an additional pin control block for Audio Pins on top the >> existing SoC Top level pin-controller. >> Hardware setup looks like: >> >> TLMM GPIO[146 - 159] --> LPASS LPI GPIO [0 - 13] >> > > Iiuc the LPI TLMM block is just "another pinmux/pinconf block" found in > these SoCs, with the additional magic that the 14 pads are muxed with > some of the TLMM pins - to allow the system integrator to choose how > many pins the LPI should have access to. > > I also believe this is what the "egpio" bit in the TLMM registers are > used for (i.e. egpio = route to LPI, egpio = 1 route to TLMM), so we > should need to add support for toggling this bit in the TLMM as well > (which I think we should do as a pinconf in the pinctrl-msm). Yes, we should add egpio function to these pins in main TLMM pinctrl! > >> This pin controller has some similarities compared to Top level >> msm SoC Pin controller like 'each pin belongs to a single group' >> and so on. However this one is intended to control only audio >> pins in particular, which can not be configured/touched by the >> Top level SoC pin controller except setting them as gpios. > > Seems like this is just a property of what functions they routed in this > region of the TLMM - and when egpio = 1 it could have been anything > else. > >> Apart from this, slew rate is also available in this block for >> certain pins which are connected to SLIMbus or SoundWire Bus. >> >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> drivers/pinctrl/qcom/Kconfig | 8 + >> drivers/pinctrl/qcom/Makefile | 1 + >> drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 734 +++++++++++++++++++++++ >> 3 files changed, 743 insertions(+) >> create mode 100644 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c >> >> diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig >> index 5fe7b8aaf69d..d3e4e89c2810 100644 >> --- a/drivers/pinctrl/qcom/Kconfig >> +++ b/drivers/pinctrl/qcom/Kconfig >> @@ -236,4 +236,12 @@ config PINCTRL_SM8250 >> Qualcomm Technologies Inc TLMM block found on the Qualcomm >> Technologies Inc SM8250 platform. >> >> +config PINCTRL_LPASS_LPI >> + tristate "Qualcomm Technologies Inc LPASS LPI pin controller driver" >> + depends on GPIOLIB >> + 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 SoCs. >> + >> endif >> diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile >> index 9e3d9c91a444..c8520155fb1b 100644 >> --- a/drivers/pinctrl/qcom/Makefile >> +++ b/drivers/pinctrl/qcom/Makefile >> @@ -28,3 +28,4 @@ obj-$(CONFIG_PINCTRL_SDM660) += pinctrl-sdm660.o >> obj-$(CONFIG_PINCTRL_SDM845) += pinctrl-sdm845.o >> obj-$(CONFIG_PINCTRL_SM8150) += pinctrl-sm8150.o >> obj-$(CONFIG_PINCTRL_SM8250) += pinctrl-sm8250.o >> +obj-$(CONFIG_PINCTRL_LPASS_LPI) += pinctrl-lpass-lpi.o >> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c >> new file mode 100644 >> index 000000000000..63cfbb2d032a >> --- /dev/null >> +++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c >> @@ -0,0 +1,734 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (c) 2016-2019, The Linux Foundation. All rights reserved. >> + * Copyright (c) 2020 Linaro Ltd. >> + */ >> + >> +#include <linux/bitops.h> >> +#include <linux/clk.h> >> +#include <linux/gpio/driver.h> >> +#include <linux/io.h> >> +#include <linux/module.h> >> +#include <linux/of_device.h> >> +#include <linux/of.h> >> +#include <linux/pinctrl/pinconf-generic.h> >> +#include <linux/pinctrl/pinconf.h> >> +#include <linux/pinctrl/pinmux.h> >> +#include <linux/platform_device.h> >> +#include <linux/slab.h> >> +#include <linux/types.h> >> +#include "../core.h" >> +#include "../pinctrl-utils.h" >> + >> +#define LPI_GPIO_REG_VAL_CTL 0x00 > > I think LPI_GPIO_CFG_REG would be a better name for this. > >> +#define LPI_GPIO_REG_DIR_CTL 0x04 > > Afaict, BIT(9) of LPI_GPIO_CFG_REG controls of the if the pin should be > output or not. This register provides the read value as BIT(0) and the > output value in BIT(1). > > So this would rather be the LPI_GPIO_VALUE_REG. Yes, that is true, will change it accordingly! > >> +#define LPI_SLEW_REG_VAL_CTL 0x00 >> +#define LPI_SLEW_RATE_MAX 0x03 >> +#define LPI_SLEW_BITS_SIZE 0x02 >> +#define LPI_GPIO_REG_PULL_SHIFT 0x0 >> +#define LPI_GPIO_REG_PULL_MASK GENMASK(1, 0) >> +#define LPI_GPIO_REG_FUNCTION_SHIFT 0x2 >> +#define LPI_GPIO_REG_FUNCTION_MASK GENMASK(5, 2) >> +#define LPI_GPIO_REG_OUT_STRENGTH_SHIFT 0x6 >> +#define LPI_GPIO_REG_OUT_STRENGTH_MASK GENMASK(8, 6) >> +#define LPI_GPIO_REG_OE_SHIFT 0x9 >> +#define LPI_GPIO_REG_OE_MASK BIT(9) >> +#define LPI_GPIO_REG_DIR_SHIFT 0x1 >> +#define LPI_GPIO_REG_DIR_MASK 0x2 >> +#define LPI_GPIO_BIAS_DISABLE 0x0 >> +#define LPI_GPIO_PULL_DOWN 0x1 >> +#define LPI_GPIO_KEEPER 0x2 >> +#define LPI_GPIO_PULL_UP 0x3 >> +#define LPI_GPIO_DS_TO_VAL(v) ((v / 2 - 1) << LPI_GPIO_REG_OUT_STRENGTH_SHIFT) >> +#define NO_SLEW -1 >> + >> +#define LPI_FUNCTION(fname) \ >> + [LPI_MUX_##fname] = { \ >> + .name = #fname, \ >> + .groups = fname##_groups, \ >> + .ngroups = ARRAY_SIZE(fname##_groups), \ >> + } >> + >> +#define LPI_PINGROUP(id, soff, f1, f2, f3, f4) \ >> + { \ >> + .name = "gpio" #id, \ >> + .pins = gpio##id##_pins, \ >> + .pin = id, \ >> + .slew_offset = soff, \ >> + .npins = ARRAY_SIZE(gpio##id##_pins), \ >> + .funcs = (int[]){ \ >> + LPI_MUX_gpio, \ >> + LPI_MUX_##f1, \ >> + LPI_MUX_##f2, \ >> + LPI_MUX_##f3, \ >> + LPI_MUX_##f4, \ >> + }, \ >> + .nfuncs = 5, \ >> + } >> + >> +struct lpi_pingroup { >> + const char *name; >> + const unsigned int *pins; >> + unsigned int npins; >> + unsigned int pin; >> + /* Bit offset in slew register for SoundWire pins only */ >> + unsigned int slew_offset; >> + unsigned int *funcs; >> + unsigned int nfuncs; >> +}; >> + >> +struct lpi_function { >> + const char *name; >> + const char * const *groups; >> + unsigned int ngroups; >> +}; >> + >> +struct lpi_pinctrl_variant_data { >> + int tlmm_reg_offset; >> + const struct pinctrl_pin_desc *pins; >> + int npins; >> + const struct lpi_pingroup *groups; >> + int ngroups; >> + const struct lpi_function *functions; >> + int nfunctions; >> +}; >> + >> +#define MAX_LPI_NUM_CLKS 2 >> + >> +struct lpi_pinctrl { >> + struct device *dev; >> + struct pinctrl_dev *ctrl; >> + struct gpio_chip chip; >> + struct pinctrl_desc desc; >> + char __iomem *tlmm_base; >> + char __iomem *slew_base; >> + struct clk_bulk_data clks[MAX_LPI_NUM_CLKS]; >> + struct mutex slew_access_lock; >> + const struct lpi_pinctrl_variant_data *data; >> +}; >> + >> +/* sm8250 variant specific data */ >> +static const struct pinctrl_pin_desc sm8250_lpi_pins[] = { >> + PINCTRL_PIN(0, "gpio0"), >> + PINCTRL_PIN(1, "gpio1"), >> + PINCTRL_PIN(2, "gpio2"), >> + PINCTRL_PIN(3, "gpio3"), >> + PINCTRL_PIN(4, "gpio4"), >> + PINCTRL_PIN(5, "gpio5"), >> + PINCTRL_PIN(6, "gpio6"), >> + PINCTRL_PIN(7, "gpio7"), >> + PINCTRL_PIN(8, "gpio8"), >> + PINCTRL_PIN(9, "gpio9"), >> + PINCTRL_PIN(10, "gpio10"), >> + PINCTRL_PIN(11, "gpio11"), >> + PINCTRL_PIN(12, "gpio12"), >> + PINCTRL_PIN(13, "gpio13"), >> +}; >> + >> +enum sm8250_lpi_functions { >> + LPI_MUX_swr_tx_clk, > > Please sort these. > >> + LPI_MUX_qua_mi2s_sclk, >> + LPI_MUX_swr_tx_data1, > > As there's no single pin that can be both data1 and data2 I think you > should have a single group for swr_tx_data and use this function for > both swr_tx_data pins. Or perhaps even just have one for swr or swr_tx. > > (This is nice when you're writing DT later on) I did think about this, but we have a rx_data2 pin in different function compared to other rx data pins. The reason to keep it as it is : 1> as this will bring in an additional complexity to the code 2> we have these represented exactly as what hw data sheet mentions it! > >> + LPI_MUX_qua_mi2s_ws, >> + LPI_MUX_swr_tx_data2, >> + LPI_MUX_qua_mi2s_data0, >> + LPI_MUX_swr_rx_clk, >> + LPI_MUX_qua_mi2s_data1, >> + LPI_MUX_swr_rx_data1, >> + LPI_MUX_qua_mi2s_data2, >> + LPI_MUX_swr_tx_data3, >> + LPI_MUX_swr_rx_data2, >> + LPI_MUX_dmic1_clk, >> + LPI_MUX_i2s1_clk, >> + LPI_MUX_dmic1_data, >> + LPI_MUX_i2s1_ws, >> + LPI_MUX_dmic2_clk, >> + LPI_MUX_i2s1_data0, >> + LPI_MUX_dmic2_data, >> + LPI_MUX_i2s1_data1, >> + LPI_MUX_i2s2_clk, >> + LPI_MUX_wsa_swr_clk, >> + LPI_MUX_i2s2_ws, >> + LPI_MUX_wsa_swr_data, >> + LPI_MUX_dmic3_clk, >> + LPI_MUX_i2s2_data0, >> + LPI_MUX_dmic3_data, >> + LPI_MUX_i2s2_data1, >> + LPI_MUX_gpio, >> + LPI_MUX_NA, > > For me replacing "NA" with "_" makes the lpi_pingroup table below easier > to read (i.e. this becomes LPI_MUX__). > Okay sure, will do that! >> +}; >> + >> +static const unsigned int gpio0_pins[] = { 0 }; >> +static const unsigned int gpio1_pins[] = { 1 }; >> +static const unsigned int gpio2_pins[] = { 2 }; >> +static const unsigned int gpio3_pins[] = { 3 }; >> +static const unsigned int gpio4_pins[] = { 4 }; >> +static const unsigned int gpio5_pins[] = { 5 }; >> +static const unsigned int gpio6_pins[] = { 6 }; >> +static const unsigned int gpio7_pins[] = { 7 }; >> +static const unsigned int gpio8_pins[] = { 8 }; >> +static const unsigned int gpio9_pins[] = { 9 }; >> +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 char * const swr_tx_clk_groups[] = { "gpio0" }; >> +static const char * const swr_tx_data1_groups[] = { "gpio1" }; >> +static const char * const swr_tx_data2_groups[] = { "gpio2" }; >> +static const char * const swr_rx_clk_groups[] = { "gpio3" }; >> +static const char * const swr_rx_data1_groups[] = { "gpio4" }; >> +static const char * const swr_tx_data3_groups[] = { "gpio5" }; >> +static const char * const dmic1_clk_groups[] = { "gpio6" }; >> +static const char * const dmic1_data_groups[] = { "gpio7" }; >> +static const char * const dmic2_clk_groups[] = { "gpio8" }; >> +static const char * const dmic2_data_groups[] = { "gpio9" }; >> +static const char * const i2s2_clk_groups[] = { "gpio10" }; >> +static const char * const i2s2_ws_groups[] = { "gpio11" }; >> +static const char * const dmic3_clk_groups[] = { "gpio12" }; >> +static const char * const dmic3_data_groups[] = { "gpio13" }; >> +static const char * const qua_mi2s_sclk_groups[] = { "gpio0" }; >> +static const char * const qua_mi2s_ws_groups[] = { "gpio1" }; >> +static const char * const qua_mi2s_data0_groups[] = { "gpio2" }; >> +static const char * const qua_mi2s_data1_groups[] = { "gpio3" }; >> +static const char * const qua_mi2s_data2_groups[] = { "gpio4" }; >> +static const char * const swr_rx_data2_groups[] = { "gpio5" }; >> +static const char * const i2s1_clk_groups[] = { "gpio6" }; >> +static const char * const i2s1_ws_groups[] = { "gpio7" }; >> +static const char * const i2s1_data0_groups[] = { "gpio8" }; >> +static const char * const i2s1_data1_groups[] = { "gpio9" }; >> +static const char * const wsa_swr_clk_groups[] = { "gpio10" }; >> +static const char * const wsa_swr_data_groups[] = { "gpio11" }; >> +static const char * const i2s2_data0_groups[] = { "gpio12" }; >> +static const char * const i2s2_data1_groups[] = { "gpio13" }; >> + >> +static const struct lpi_pingroup sm8250_groups[] = { >> + LPI_PINGROUP(0, 0, swr_tx_clk, qua_mi2s_sclk, NA, NA), >> + LPI_PINGROUP(1, 2, swr_tx_data1, qua_mi2s_ws, NA, NA), >> + LPI_PINGROUP(2, 4, swr_tx_data2, qua_mi2s_data0, NA, NA), >> + LPI_PINGROUP(3, 8, swr_rx_clk, qua_mi2s_data1, NA, NA), >> + LPI_PINGROUP(4, 10, swr_rx_data1, qua_mi2s_data2, NA, NA), >> + LPI_PINGROUP(5, 12, swr_tx_data3, swr_rx_data2, NA, NA), >> + LPI_PINGROUP(6, NO_SLEW, dmic1_clk, i2s1_clk, NA, NA), >> + LPI_PINGROUP(7, NO_SLEW, dmic1_data, i2s1_ws, NA, NA), >> + LPI_PINGROUP(8, NO_SLEW, dmic2_clk, i2s1_data0, NA, NA), >> + LPI_PINGROUP(9, NO_SLEW, dmic2_data, i2s1_data1, NA, NA), >> + LPI_PINGROUP(10, 16, i2s2_clk, wsa_swr_clk, NA, NA), >> + LPI_PINGROUP(11, 18, i2s2_ws, wsa_swr_data, NA, NA), >> + LPI_PINGROUP(12, NO_SLEW, dmic3_clk, i2s2_data0, NA, NA), >> + LPI_PINGROUP(13, NO_SLEW, dmic3_data, i2s2_data1, NA, NA), >> +}; >> + >> +static const struct lpi_function sm8250_functions[] = { >> + LPI_FUNCTION(swr_tx_clk), > > Please sort these. > >> + LPI_FUNCTION(qua_mi2s_sclk), >> + LPI_FUNCTION(swr_tx_data1), >> + LPI_FUNCTION(qua_mi2s_ws), >> + LPI_FUNCTION(swr_tx_data2), >> + LPI_FUNCTION(qua_mi2s_data0), >> + LPI_FUNCTION(swr_rx_clk), >> + LPI_FUNCTION(qua_mi2s_data1), >> + LPI_FUNCTION(swr_rx_data1), >> + LPI_FUNCTION(qua_mi2s_data2), >> + LPI_FUNCTION(swr_tx_data3), >> + LPI_FUNCTION(swr_rx_data2), >> + LPI_FUNCTION(dmic1_clk), >> + LPI_FUNCTION(i2s1_clk), >> + LPI_FUNCTION(dmic1_data), >> + LPI_FUNCTION(i2s1_ws), >> + LPI_FUNCTION(dmic2_clk), >> + LPI_FUNCTION(i2s1_data0), >> + LPI_FUNCTION(dmic2_data), >> + LPI_FUNCTION(i2s1_data1), >> + LPI_FUNCTION(i2s2_clk), >> + LPI_FUNCTION(wsa_swr_clk), >> + LPI_FUNCTION(i2s2_ws), >> + LPI_FUNCTION(wsa_swr_data), >> + LPI_FUNCTION(dmic3_clk), >> + LPI_FUNCTION(i2s2_data0), >> + LPI_FUNCTION(dmic3_data), >> + LPI_FUNCTION(i2s2_data1), >> +}; >> + >> +static struct lpi_pinctrl_variant_data sm8250_lpi_data = { >> + .tlmm_reg_offset = 0x1000, > > Do we have any platform in sight where this is not 0x1000? Could we just > make a define out of it? Am not 100% sure ATM, But I wanted to keep this flexible as these offsets in downstream were part of device tree for some reason, so having offset here for particular compatible made more sense for me! > >> + .pins = sm8250_lpi_pins, >> + .npins = ARRAY_SIZE(sm8250_lpi_pins), >> + .groups = sm8250_groups, >> + .ngroups = ARRAY_SIZE(sm8250_groups), >> + .functions = sm8250_functions, >> + .nfunctions = ARRAY_SIZE(sm8250_functions), >> +}; >> + >> +static int lpi_gpio_read(struct lpi_pinctrl *state, unsigned int pin, >> + unsigned int addr) >> +{ >> + return ioread32(state->tlmm_base + >> + state->data->tlmm_reg_offset * pin + addr); >> +} >> + >> +static int lpi_gpio_write(struct lpi_pinctrl *state, unsigned int pin, >> + unsigned int addr, unsigned int val) >> +{ >> + iowrite32(val, state->tlmm_base + >> + state->data->tlmm_reg_offset * pin + addr); >> + >> + return 0; >> +} >> + >> +static int lpi_gpio_get_groups_count(struct pinctrl_dev *pctldev) >> +{ >> + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> + >> + return pctrl->data->ngroups; >> +} >> + >> +static const char *lpi_gpio_get_group_name(struct pinctrl_dev *pctldev, >> + unsigned int group) >> +{ >> + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> + >> + return pctrl->data->groups[group].name; >> +} >> + >> +static int lpi_gpio_get_group_pins(struct pinctrl_dev *pctldev, >> + unsigned int group, >> + const unsigned int **pins, >> + unsigned int *num_pins) >> +{ >> + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> + >> + *pins = pctrl->data->groups[group].pins; >> + *num_pins = pctrl->data->groups[group].npins; >> + >> + return 0; >> +} >> + >> +static const struct pinctrl_ops lpi_gpio_pinctrl_ops = { >> + .get_groups_count = lpi_gpio_get_groups_count, >> + .get_group_name = lpi_gpio_get_group_name, >> + .get_group_pins = lpi_gpio_get_group_pins, >> + .dt_node_to_map = pinconf_generic_dt_node_to_map_group, >> + .dt_free_map = pinctrl_utils_free_map, >> +}; >> + >> +static int lpi_gpio_get_functions_count(struct pinctrl_dev *pctldev) >> +{ >> + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> + >> + return pctrl->data->nfunctions; >> +} >> + >> +static const char *lpi_gpio_get_function_name(struct pinctrl_dev *pctldev, >> + unsigned int function) >> +{ >> + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> + >> + return pctrl->data->functions[function].name; >> +} >> + >> +static int lpi_gpio_get_function_groups(struct pinctrl_dev *pctldev, >> + unsigned int function, >> + const char *const **groups, >> + unsigned *const num_qgroups) >> +{ >> + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> + >> + *groups = pctrl->data->functions[function].groups; >> + *num_qgroups = pctrl->data->functions[function].ngroups; >> + >> + return 0; >> +} >> + >> +static int lpi_gpio_set_mux(struct pinctrl_dev *pctldev, unsigned int function, >> + unsigned int group_num) >> +{ >> + struct lpi_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); >> + const struct lpi_pingroup *g = &pctrl->data->groups[group_num]; >> + unsigned int val; >> + int i, pin = g->pin; >> + >> + for (i = 0; i < g->nfuncs; i++) { >> + if (g->funcs[i] == function) >> + break; >> + } >> + >> + if (WARN_ON(i == g->nfuncs)) >> + return -EINVAL; >> + >> + val = lpi_gpio_read(pctrl, pin, LPI_GPIO_REG_VAL_CTL); >> + val &= ~LPI_GPIO_REG_FUNCTION_MASK; >> + val |= i << LPI_GPIO_REG_FUNCTION_SHIFT; > > I think this would benefit from FIELD_SET() I can give that a go! > >> + lpi_gpio_write(pctrl, pin, LPI_GPIO_REG_VAL_CTL, val); >> + >> + return 0; >> +} >> + >> +static const struct pinmux_ops lpi_gpio_pinmux_ops = { >> + .get_functions_count = lpi_gpio_get_functions_count, >> + .get_function_name = lpi_gpio_get_function_name, >> + .get_function_groups = lpi_gpio_get_function_groups, >> + .set_mux = lpi_gpio_set_mux, >> +}; >> + >> +static int lpi_config_get(struct pinctrl_dev *pctldev, >> + unsigned int pin, unsigned long *config) >> +{ >> + unsigned int param = pinconf_to_config_param(*config); >> + struct lpi_pinctrl *state = dev_get_drvdata(pctldev->dev); >> + unsigned int arg = 0; >> + int is_out; >> + int pull; >> + u32 ctl_reg; >> + >> + ctl_reg = lpi_gpio_read(state, pin, LPI_GPIO_REG_DIR_CTL); >> + >> + is_out = (ctl_reg & LPI_GPIO_REG_DIR_MASK) >> LPI_GPIO_REG_DIR_SHIFT; > > This is a single bit, which would be clarified if you replaced _MASK and > _SHIFT with a single LPI_GPIO_OUT BIT(1) yep! > >> + >> + ctl_reg = lpi_gpio_read(state, pin, LPI_GPIO_REG_VAL_CTL); >> + >> + pull = (ctl_reg & LPI_GPIO_REG_PULL_MASK) >> LPI_GPIO_REG_PULL_SHIFT; >> + >> + switch (param) { >> + case PIN_CONFIG_BIAS_DISABLE: >> + if (pull == LPI_GPIO_BIAS_DISABLE) >> + arg = 1; >> + break; >> + case PIN_CONFIG_BIAS_PULL_DOWN: >> + if (pull == LPI_GPIO_PULL_DOWN) >> + arg = 1; >> + break; >> + case PIN_CONFIG_BIAS_BUS_HOLD: >> + if (pull == LPI_GPIO_KEEPER) >> + arg = 1; >> + break; >> + case PIN_CONFIG_BIAS_PULL_UP: >> + if (pull == LPI_GPIO_PULL_UP) >> + arg = 1; >> + break; >> + case PIN_CONFIG_INPUT_ENABLE: >> + case PIN_CONFIG_OUTPUT: >> + if (is_out) >> + arg = 1; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + *config = pinconf_to_config_packed(param, arg); >> + return 0; >> +} >> + >> +static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group, >> + unsigned long *configs, unsigned int nconfs) >> +{ >> + struct lpi_pinctrl *pctrl = dev_get_drvdata(pctldev->dev); >> + unsigned int param, arg, pullup, strength; >> + const struct lpi_pingroup *g; >> + bool value, output_enabled = false; >> + unsigned long val; >> + int i, slew_offset, ret = 0; >> + >> + g = &pctrl->data->groups[group]; >> + for (i = 0; i < nconfs; i++) { >> + param = pinconf_to_config_param(configs[i]); >> + arg = pinconf_to_config_argument(configs[i]); >> + >> + switch (param) { >> + case PIN_CONFIG_BIAS_DISABLE: >> + pullup = LPI_GPIO_BIAS_DISABLE; >> + break; >> + case PIN_CONFIG_BIAS_PULL_DOWN: >> + pullup = LPI_GPIO_PULL_DOWN; >> + break; >> + case PIN_CONFIG_BIAS_BUS_HOLD: >> + pullup = LPI_GPIO_KEEPER; >> + break; >> + case PIN_CONFIG_BIAS_PULL_UP: >> + pullup = LPI_GPIO_PULL_UP; >> + break; >> + case PIN_CONFIG_INPUT_ENABLE: >> + output_enabled = false; >> + break; >> + case PIN_CONFIG_OUTPUT: >> + output_enabled = true; >> + lpi_gpio_write(pctrl, group, LPI_GPIO_REG_DIR_CTL, >> + output_enabled << LPI_GPIO_REG_DIR_SHIFT); > > Is there a reason why you write this both here and at the bottom of the > function? > >> + value = arg; >> + break; >> + case PIN_CONFIG_DRIVE_STRENGTH: >> + strength = arg; >> + break; >> + case PIN_CONFIG_SLEW_RATE: >> + if (arg > LPI_SLEW_RATE_MAX) { >> + dev_err(pctldev->dev, "invalid slew rate %u for pin: %d\n", >> + arg, group); >> + goto set_gpio; > > So if I set an invalid slew rate the remainder of the pinconf/mux > properties will be ignored and the pin configured with whatever came > before? > > Wouldn't either return -EINVAL or perhaps just a break make more sense > here? May be EINVAL should be good error here! Will fix that! > >> + } >> + >> + slew_offset = g->slew_offset; >> + if (slew_offset == NO_SLEW) >> + break; >> + >> + mutex_lock(&pctrl->slew_access_lock); >> + val = ioread32(pctrl->slew_base + LPI_SLEW_REG_VAL_CTL); >> + >> + for (i = 0; i < LPI_SLEW_BITS_SIZE; i++) { >> + assign_bit(slew_offset, &val, arg & 0x01); >> + slew_offset++; >> + arg = arg >> 1; >> + } >> + >> + iowrite32(val, pctrl->slew_base + LPI_SLEW_REG_VAL_CTL); >> + >> + mutex_unlock(&pctrl->slew_access_lock); >> + break; >> + default: >> + return -EINVAL; >> + } >> + } >> + >> +set_gpio: >> + val = lpi_gpio_read(pctrl, group, LPI_GPIO_REG_VAL_CTL); >> + val &= ~(LPI_GPIO_REG_PULL_MASK | LPI_GPIO_REG_OUT_STRENGTH_MASK | >> + LPI_GPIO_REG_OE_MASK); >> + val |= pullup << LPI_GPIO_REG_PULL_SHIFT; >> + val |= LPI_GPIO_DS_TO_VAL(strength); > > FIELD_SET() would make this prettier. sure will give that a try! > >> + if (output_enabled) >> + val |= value << LPI_GPIO_REG_OE_SHIFT; >> + >> + lpi_gpio_write(pctrl, group, LPI_GPIO_REG_VAL_CTL, val); >> + lpi_gpio_write(pctrl, group, LPI_GPIO_REG_DIR_CTL, >> + output_enabled << LPI_GPIO_REG_DIR_SHIFT); >> + >> + return ret; > > ret is untouched since you set it to 0 at the beginning. will clean this one as well > > Regards, > Bjorn > >> +} >> + ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver 2020-12-01 10:01 ` Srinivas Kandagatla @ 2020-12-01 17:28 ` Bjorn Andersson 2020-12-02 15:18 ` Srinivas Kandagatla 0 siblings, 1 reply; 10+ messages in thread From: Bjorn Andersson @ 2020-12-01 17:28 UTC (permalink / raw) To: Srinivas Kandagatla Cc: linus.walleij, robh+dt, agross, linux-arm-msm, linux-gpio, devicetree, linux-kernel On Tue 01 Dec 04:01 CST 2020, Srinivas Kandagatla wrote: > Many thanks for review Bjorn, > > > On 01/12/2020 00:47, Bjorn Andersson wrote: > > On Mon 16 Nov 08:34 CST 2020, Srinivas Kandagatla wrote: > > > > > Add initial pinctrl driver to support pin configuration for > > > LPASS (Low Power Audio SubSystem) LPI (Low Power Island) pinctrl > > > on SM8250. > > > > > > This IP is an additional pin control block for Audio Pins on top the > > > existing SoC Top level pin-controller. > > > Hardware setup looks like: > > > > > > TLMM GPIO[146 - 159] --> LPASS LPI GPIO [0 - 13] > > > > > > > Iiuc the LPI TLMM block is just "another pinmux/pinconf block" found in > > these SoCs, with the additional magic that the 14 pads are muxed with > > some of the TLMM pins - to allow the system integrator to choose how > > many pins the LPI should have access to. > > > > I also believe this is what the "egpio" bit in the TLMM registers are > > used for (i.e. egpio = route to LPI, egpio = 1 route to TLMM), so we > > should need to add support for toggling this bit in the TLMM as well > > (which I think we should do as a pinconf in the pinctrl-msm). > > Yes, we should add egpio function to these pins in main TLMM pinctrl! > I was thinking about abusing the pinconf system, but reading you sentence makes me feel that expressing it as a "function" and adding a special case handling in msm_pinmux_set_mux() would actually make things much cleaner to the outside. i.e. we would then end up with something in DT like: pin-is-normal-tlmm-pin { pins = "gpio146"; function = "gpio"; }; and pin-routed-to-lpi-pin { pins = "gpio146"; function = "egpio"; }; Only "drawback" I can see is that we're inverting the chip's meaning of "egpio" (i.e. active means route-to-tlmm in the hardware). > > > > > This pin controller has some similarities compared to Top level > > > msm SoC Pin controller like 'each pin belongs to a single group' > > > and so on. However this one is intended to control only audio > > > pins in particular, which can not be configured/touched by the > > > Top level SoC pin controller except setting them as gpios. [..] > > > diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c [..] > > > + LPI_MUX_qua_mi2s_sclk, > > > + LPI_MUX_swr_tx_data1, > > > > As there's no single pin that can be both data1 and data2 I think you > > should have a single group for swr_tx_data and use this function for > > both swr_tx_data pins. Or perhaps even just have one for swr or swr_tx. > > > > (This is nice when you're writing DT later on) > > I did think about this, but we have a rx_data2 pin in different function > compared to other rx data pins. > > The reason to keep it as it is : > 1> as this will bring in an additional complexity to the code For each pin lpi_gpio_set_mux() will be invoked and you'd be searching for the index (i) among that pins .funcs. So it doesn't matter that looking up a particular function results in different register values for different pins, it's already dealt with. > 2> we have these represented exactly as what hw data sheet mentions it! > That is true, but the result is that you have to write 2 states in the DT to get your 2 pins to switch to the particular function. By grouping them you could do: data-pins { pins = "gpio1", "gpio2"; function = "swr_tx_data"; }; We do this quite extensively for the TLMM (pinctrl-msm) because it results in cleaner DT. > > > > > + LPI_MUX_qua_mi2s_ws, [..] > > > +static struct lpi_pinctrl_variant_data sm8250_lpi_data = { > > > + .tlmm_reg_offset = 0x1000, > > > > Do we have any platform in sight where this is not 0x1000? Could we just > > make a define out of it? > Am not 100% sure ATM, But I wanted to keep this flexible as these offsets in > downstream were part of device tree for some reason, so having offset here > for particular compatible made more sense for me! > Downtream does indeed favor "flexible" code. I tend to prefer a #define until we actually need the flexibility... Regards, Bjorn ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver 2020-12-01 17:28 ` Bjorn Andersson @ 2020-12-02 15:18 ` Srinivas Kandagatla 0 siblings, 0 replies; 10+ messages in thread From: Srinivas Kandagatla @ 2020-12-02 15:18 UTC (permalink / raw) To: Bjorn Andersson Cc: linus.walleij, robh+dt, agross, linux-arm-msm, linux-gpio, devicetree, linux-kernel On 01/12/2020 17:28, Bjorn Andersson wrote: > On Tue 01 Dec 04:01 CST 2020, Srinivas Kandagatla wrote: > >> Many thanks for review Bjorn, >> >> >> On 01/12/2020 00:47, Bjorn Andersson wrote: >>> On Mon 16 Nov 08:34 CST 2020, Srinivas Kandagatla wrote: >>> >>>> Add initial pinctrl driver to support pin configuration for >>>> LPASS (Low Power Audio SubSystem) LPI (Low Power Island) pinctrl >>>> on SM8250. >>>> >>>> This IP is an additional pin control block for Audio Pins on top the >>>> existing SoC Top level pin-controller. >>>> Hardware setup looks like: >>>> >>>> TLMM GPIO[146 - 159] --> LPASS LPI GPIO [0 - 13] >>>> >>> >>> Iiuc the LPI TLMM block is just "another pinmux/pinconf block" found in >>> these SoCs, with the additional magic that the 14 pads are muxed with >>> some of the TLMM pins - to allow the system integrator to choose how >>> many pins the LPI should have access to. >>> >>> I also believe this is what the "egpio" bit in the TLMM registers are >>> used for (i.e. egpio = route to LPI, egpio = 1 route to TLMM), so we >>> should need to add support for toggling this bit in the TLMM as well >>> (which I think we should do as a pinconf in the pinctrl-msm). >> >> Yes, we should add egpio function to these pins in main TLMM pinctrl! >> > > I was thinking about abusing the pinconf system, but reading you > sentence makes me feel that expressing it as a "function" and adding a > special case handling in msm_pinmux_set_mux() would actually make things > much cleaner to the outside. > > i.e. we would then end up with something in DT like: > > pin-is-normal-tlmm-pin { > pins = "gpio146"; > function = "gpio"; > }; > > and > > pin-routed-to-lpi-pin { > pins = "gpio146"; > function = "egpio"; > }; That is what I was thinking of. > > Only "drawback" I can see is that we're inverting the chip's meaning of > "egpio" (i.e. active means route-to-tlmm in the hardware). > At somepoint we need to start defining what egpio really means w.r.t pinctrl setup! >>> >>>> This pin controller has some similarities compared to Top level >>>> msm SoC Pin controller like 'each pin belongs to a single group' >>>> and so on. However this one is intended to control only audio >>>> pins in particular, which can not be configured/touched by the >>>> Top level SoC pin controller except setting them as gpios. > [..] >>>> diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c > [..] >>>> + LPI_MUX_qua_mi2s_sclk, >>>> + LPI_MUX_swr_tx_data1, >>> >>> As there's no single pin that can be both data1 and data2 I think you >>> should have a single group for swr_tx_data and use this function for >>> both swr_tx_data pins. Or perhaps even just have one for swr or swr_tx. >>> >>> (This is nice when you're writing DT later on) >> >> I did think about this, but we have a rx_data2 pin in different function >> compared to other rx data pins. >> >> The reason to keep it as it is : >> 1> as this will bring in an additional complexity to the code > > For each pin lpi_gpio_set_mux() will be invoked and you'd be searching > for the index (i) among that pins .funcs. So it doesn't matter that > looking up a particular function results in different register values > for different pins, it's already dealt with. > >> 2> we have these represented exactly as what hw data sheet mentions it! >> > > That is true, but the result is that you have to write 2 states in the > DT to get your 2 pins to switch to the particular function. By grouping > them you could do: > > data-pins { > pins = "gpio1", "gpio2"; > function = "swr_tx_data"; > }; > > > We do this quite extensively for the TLMM (pinctrl-msm) because it > results in cleaner DT. These are now changed as requested! > >>> >>>> + LPI_MUX_qua_mi2s_ws, > [..] >>>> +static struct lpi_pinctrl_variant_data sm8250_lpi_data = { >>>> + .tlmm_reg_offset = 0x1000, >>> >>> Do we have any platform in sight where this is not 0x1000? Could we just >>> make a define out of it? >> Am not 100% sure ATM, But I wanted to keep this flexible as these offsets in >> downstream were part of device tree for some reason, so having offset here >> for particular compatible made more sense for me! >> > > Downtream does indeed favor "flexible" code. I tend to prefer a #define > until we actually need the flexibility... Done! --srini > > Regards, > Bjorn > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-12-02 15:19 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-16 14:34 [PATCH v4 0/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl support Srinivas Kandagatla 2020-11-16 14:34 ` [PATCH v4 1/2] dt-bindings: pinctrl: qcom: Add sm8250 lpass lpi pinctrl bindings Srinivas Kandagatla 2020-11-30 23:49 ` Rob Herring 2020-12-01 0:55 ` Bjorn Andersson 2020-12-01 10:03 ` Srinivas Kandagatla 2020-11-16 14:34 ` [PATCH v4 2/2] pinctrl: qcom: Add sm8250 lpass lpi pinctrl driver Srinivas Kandagatla 2020-12-01 0:47 ` Bjorn Andersson 2020-12-01 10:01 ` Srinivas Kandagatla 2020-12-01 17:28 ` Bjorn Andersson 2020-12-02 15:18 ` Srinivas Kandagatla
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).