[PATCH v2 0/2] pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config register

[PATCH v2 0/2] pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config register

[Krzysztof Kozlowski]

Hi,

Changes in v2
=============
1. Reversed xmas tree
v1: https://lore.kernel.org/all/20230901090224.27770-1-krzysztof.kozlowski@linaro.org/

Description
===========
Prepare LPASS (Low Power Audio SubSystem) pin controller for newer
Qualcomm SoCs.  The patchset does not bring the newer SoCs yet, but only
re-organizes the code for future changes.

I understand that patch #2 (adding flag) makes little sense without
actual user of that flag, but such user I cannot post yet.

Dependency
==========
Context depends on my previous fix:
https://lore.kernel.org/linux-arm-msm/20230815110625.317971-1-krzysztof.kozlowski@linaro.org/

Best regards,
Krzysztof

Krzysztof Kozlowski (2):
  pinctrl: qcom: lpass-lpi: split slew rate set to separate function
  pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config
    register

 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 69 +++++++++++++++---------
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h |  7 +++
 2 files changed, 52 insertions(+), 24 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] pinctrl: qcom: lpass-lpi: split slew rate set to separate function

[Krzysztof Kozlowski]

Setting slew rate for each pin will grow with upcoming Qualcomm SoCs,
so split the code responsible for this into separate function for easier
readability and maintenance.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Changes in v2:
1. None
---
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 53 +++++++++++++++---------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 9651aed048cf..4fb808545f7f 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -186,6 +186,35 @@ static int lpi_config_get(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
+static int lpi_config_set_slew_rate(struct lpi_pinctrl *pctrl,
+				    const struct lpi_pingroup *g,
+				    unsigned int group, unsigned int slew)
+{
+	unsigned long sval;
+	int slew_offset;
+
+	if (slew > LPI_SLEW_RATE_MAX) {
+		dev_err(pctrl->dev, "invalid slew rate %u for pin: %d\n",
+			slew, group);
+		return -EINVAL;
+	}
+
+	slew_offset = g->slew_offset;
+	if (slew_offset == LPI_NO_SLEW)
+		return 0;
+
+	mutex_lock(&pctrl->lock);
+
+	sval = ioread32(pctrl->slew_base + LPI_SLEW_RATE_CTL_REG);
+	sval &= ~(LPI_SLEW_RATE_MASK << slew_offset);
+	sval |= slew << slew_offset;
+	iowrite32(sval, pctrl->slew_base + LPI_SLEW_RATE_CTL_REG);
+
+	mutex_unlock(&pctrl->lock);
+
+	return 0;
+}
+
 static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
 			  unsigned long *configs, unsigned int nconfs)
 {
@@ -193,8 +222,7 @@ static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
 	unsigned int param, arg, pullup = LPI_GPIO_BIAS_DISABLE, strength = 2;
 	bool value, output_enabled = false;
 	const struct lpi_pingroup *g;
-	unsigned long sval;
-	int i, slew_offset;
+	int i, ret;
 	u32 val;
 
 	g = &pctrl->data->groups[group];
@@ -226,24 +254,9 @@ static int lpi_config_set(struct pinctrl_dev *pctldev, unsigned int group,
 			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);
-				return -EINVAL;
-			}
-
-			slew_offset = g->slew_offset;
-			if (slew_offset == LPI_NO_SLEW)
-				break;
-
-			mutex_lock(&pctrl->lock);
-
-			sval = ioread32(pctrl->slew_base + LPI_SLEW_RATE_CTL_REG);
-			sval &= ~(LPI_SLEW_RATE_MASK << slew_offset);
-			sval |= arg << slew_offset;
-			iowrite32(sval, pctrl->slew_base + LPI_SLEW_RATE_CTL_REG);
-
-			mutex_unlock(&pctrl->lock);
+			ret = lpi_config_set_slew_rate(pctrl, g, group, arg);
+			if (ret)
+				return ret;
 			break;
 		default:
 			return -EINVAL;
-- 
2.34.1

Re: [PATCH v2 1/2] pinctrl: qcom: lpass-lpi: split slew rate set to separate function

[Konrad Dybcio]

On 10/13/23 16:59, Krzysztof Kozlowski wrote:
> Setting slew rate for each pin will grow with upcoming Qualcomm SoCs,
> so split the code responsible for this into separate function for easier
> readability and maintenance.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Changes in v2:
> 1. None
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

[PATCH v2 2/2] pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config register

[Krzysztof Kozlowski]

Existing Qualcomm SoCs have the LPASS pin controller slew rate control
in separate register, however this will change with upcoming Qualcomm
SoCs.  The slew rate will be part of the main register for pin
configuration, thus second device IO address space is not needed.

Prepare for supporting new SoCs by adding flag customizing the driver
behavior for slew rate.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Changes in v2:
1. Reversed xmas tree

v1: https://lore.kernel.org/all/20230901090224.27770-1-krzysztof.kozlowski@linaro.org/
---
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.c | 20 ++++++++++++++------
 drivers/pinctrl/qcom/pinctrl-lpass-lpi.h |  7 +++++++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
index 4fb808545f7f..9e410a281bfa 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.c
@@ -191,6 +191,7 @@ static int lpi_config_set_slew_rate(struct lpi_pinctrl *pctrl,
 				    unsigned int group, unsigned int slew)
 {
 	unsigned long sval;
+	void __iomem *reg;
 	int slew_offset;
 
 	if (slew > LPI_SLEW_RATE_MAX) {
@@ -203,12 +204,17 @@ static int lpi_config_set_slew_rate(struct lpi_pinctrl *pctrl,
 	if (slew_offset == LPI_NO_SLEW)
 		return 0;
 
+	if (pctrl->data->flags & LPI_FLAG_SLEW_RATE_SAME_REG)
+		reg = pctrl->tlmm_base + LPI_TLMM_REG_OFFSET * group + LPI_GPIO_CFG_REG;
+	else
+		reg = pctrl->slew_base + LPI_SLEW_RATE_CTL_REG;
+
 	mutex_lock(&pctrl->lock);
 
-	sval = ioread32(pctrl->slew_base + LPI_SLEW_RATE_CTL_REG);
+	sval = ioread32(reg);
 	sval &= ~(LPI_SLEW_RATE_MASK << slew_offset);
 	sval |= slew << slew_offset;
-	iowrite32(sval, pctrl->slew_base + LPI_SLEW_RATE_CTL_REG);
+	iowrite32(sval, reg);
 
 	mutex_unlock(&pctrl->lock);
 
@@ -452,10 +458,12 @@ int lpi_pinctrl_probe(struct platform_device *pdev)
 		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");
+	if (!(data->flags & LPI_FLAG_SLEW_RATE_SAME_REG)) {
+		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_optional(dev, MAX_LPI_NUM_CLKS, pctrl->clks);
 	if (ret)
diff --git a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
index 387d83ee95b5..206b2c0ca828 100644
--- a/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
+++ b/drivers/pinctrl/qcom/pinctrl-lpass-lpi.h
@@ -60,6 +60,12 @@ struct pinctrl_pin_desc;
 		.nfuncs = 5,				\
 	}
 
+/*
+ * Slew rate control is done in the same register as rest of the
+ * pin configuration.
+ */
+#define LPI_FLAG_SLEW_RATE_SAME_REG			BIT(0)
+
 struct lpi_pingroup {
 	struct group_desc group;
 	unsigned int pin;
@@ -82,6 +88,7 @@ struct lpi_pinctrl_variant_data {
 	int ngroups;
 	const struct lpi_function *functions;
 	int nfunctions;
+	unsigned int flags;
 };
 
 int lpi_pinctrl_probe(struct platform_device *pdev);
-- 
2.34.1

Re: [PATCH v2 2/2] pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config register

[Konrad Dybcio]

On 10/13/23 16:59, Krzysztof Kozlowski wrote:
> Existing Qualcomm SoCs have the LPASS pin controller slew rate control
> in separate register, however this will change with upcoming Qualcomm
> SoCs.  The slew rate will be part of the main register for pin
> configuration, thus second device IO address space is not needed.
> 
> Prepare for supporting new SoCs by adding flag customizing the driver
> behavior for slew rate.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> ---
> 
> Changes in v2:
> 1. Reversed xmas tree
> 
> v1: https://lore.kernel.org/all/20230901090224.27770-1-krzysztof.kozlowski@linaro.org/
> ---
Only because I know it'll be used soon:

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad

Re: [PATCH v2 2/2] pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config register

[Linus Walleij]

On Fri, Oct 13, 2023 at 4:59 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:

> Existing Qualcomm SoCs have the LPASS pin controller slew rate control
> in separate register, however this will change with upcoming Qualcomm
> SoCs.  The slew rate will be part of the main register for pin
> configuration, thus second device IO address space is not needed.
>
> Prepare for supporting new SoCs by adding flag customizing the driver
> behavior for slew rate.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Patches applied.

Yours,
Linus Walleij

Re: [PATCH v2 0/2] pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config register

[Linus Walleij]

On Fri, Oct 13, 2023 at 4:59 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:


> Changes in v2

I tried to apply this to the pinctrl devel branch:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel

It doesn't apply, could you rebase it?

Yours,
Linus Walleij

Re: [PATCH v2 0/2] pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config register

[Krzysztof Kozlowski]

On 23/10/2023 10:19, Linus Walleij wrote:
> On Fri, Oct 13, 2023 at 4:59 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> 
> 
>> Changes in v2
> 
> I tried to apply this to the pinctrl devel branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
> 
> It doesn't apply, could you rebase it?

The context depends on my previous fix which you applied and sent for
v6.6 already:
https://lore.kernel.org/linux-arm-msm/CACRpkdaybnYEmkgv7VG4fh5sXQ7uwHm2wH2Khja-P1b6idYr8w@mail.gmail.com/

I can rebase, but I am afraid it will cause conflicts. Is it reasonable
for you to merge v6.6-rc7 into your devel branch?



Best regards,
Krzysztof

Re: [PATCH v2 0/2] pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config register

[Linus Walleij]

On Mon, Oct 23, 2023 at 10:22 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:

> On 23/10/2023 10:19, Linus Walleij wrote:
> > On Fri, Oct 13, 2023 at 4:59 PM Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >
> >
> >> Changes in v2
> >
> > I tried to apply this to the pinctrl devel branch:
> > https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
> >
> > It doesn't apply, could you rebase it?
>
> The context depends on my previous fix which you applied and sent for
> v6.6 already:
> https://lore.kernel.org/linux-arm-msm/CACRpkdaybnYEmkgv7VG4fh5sXQ7uwHm2wH2Khja-P1b6idYr8w@mail.gmail.com/
>
> I can rebase, but I am afraid it will cause conflicts. Is it reasonable
> for you to merge v6.6-rc7 into your devel branch?

Torvalds is usually not super-happy when we do that, especially
this late in the development cycle it gets a bit ugly with all
the stuff that brings in.

Can we wait with this patch set until the next development
cycle or is it urgent?

Yours,
Linus Walleij

Re: [PATCH v2 0/2] pinctrl: qcom: lpass-lpi: allow slew rate bit in main pin config register

[Krzysztof Kozlowski]

On 23/10/2023 10:27, Linus Walleij wrote:
> On Mon, Oct 23, 2023 at 10:22 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 23/10/2023 10:19, Linus Walleij wrote:
>>> On Fri, Oct 13, 2023 at 4:59 PM Krzysztof Kozlowski
>>> <krzysztof.kozlowski@linaro.org> wrote:
>>>
>>>
>>>> Changes in v2
>>>
>>> I tried to apply this to the pinctrl devel branch:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git/log/?h=devel
>>>
>>> It doesn't apply, could you rebase it?
>>
>> The context depends on my previous fix which you applied and sent for
>> v6.6 already:
>> https://lore.kernel.org/linux-arm-msm/CACRpkdaybnYEmkgv7VG4fh5sXQ7uwHm2wH2Khja-P1b6idYr8w@mail.gmail.com/
>>
>> I can rebase, but I am afraid it will cause conflicts. Is it reasonable
>> for you to merge v6.6-rc7 into your devel branch?
> 
> Torvalds is usually not super-happy when we do that, especially
> this late in the development cycle it gets a bit ugly with all
> the stuff that brings in.
> 
> Can we wait with this patch set until the next development
> cycle or is it urgent?

We can wait, no problem with that.

Best regards,
Krzysztof