[PATCH v2 0/4] ASoC: rt5677: Refactor GPIO and use device_get_match_data()

[PATCH v2 0/4] ASoC: rt5677: Refactor GPIO and use device_get_match_data()

[Andy Shevchenko]

The code can be simplified with refactored GPIO parts and with use of
device_get_match_data(). Besides that couple of additional changes,
one for maintenance and one for making IRQ domain agnostic (not being
pinned to OF).

Changelog v2:
- refactored GPIO code in (a new) patch 1
- fixed compilation error in patch 2 (LKP)

Andy Shevchenko (4):
  ASoC: rt5677: Refactor GPIO support code
  ASoC: rt5677: Use agnostic irq_domain_create_linear()
  ASoC: rt5677: Use device_get_match_data()
  ASoC: rt5677: Sort headers alphabetically

 sound/soc/codecs/rt5677.c | 117 +++++++++++++-------------------------
 sound/soc/codecs/rt5677.h |  92 +++++-------------------------
 2 files changed, 53 insertions(+), 156 deletions(-)

-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 1/4] ASoC: rt5677: Refactor GPIO support code

[Andy Shevchenko]

After compiler complains:

  sound/soc/codecs/rt5677.c:4748:30: warning: dubious: x | !y

I looked into the code and realized that we can refactor it
for better reading and fixing above issue at the same time.
Hence this change. It does not imply any functional changes.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/codecs/rt5677.c | 76 +++++++++++----------------------
 sound/soc/codecs/rt5677.h | 88 ++++++---------------------------------
 2 files changed, 37 insertions(+), 127 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index ad14d18860fc..3a2a6b150cda 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -4717,50 +4717,34 @@ static int rt5677_set_bias_level(struct snd_soc_component *component,
 	return 0;
 }
 
+static int rt5677_update_gpio_bits(struct rt5677_priv *rt5677, unsigned offset, int m, int v)
+{
+	unsigned int bank = offset / 5;
+	unsigned int shift = (offset % 5) * 3;
+	unsigned int reg = bank ? RT5677_GPIO_CTRL3 : RT5677_GPIO_CTRL2;
+
+	return regmap_update_bits(rt5677->regmap, reg, m << shift, v << shift);
+}
+
 #ifdef CONFIG_GPIOLIB
 static void rt5677_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 {
 	struct rt5677_priv *rt5677 = gpiochip_get_data(chip);
+	int level = value ? RT5677_GPIOx_OUT_HI : RT5677_GPIOx_OUT_LO;
+	int m = RT5677_GPIOx_OUT_MASK;
 
-	switch (offset) {
-	case RT5677_GPIO1 ... RT5677_GPIO5:
-		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL2,
-			0x1 << (offset * 3 + 1), !!value << (offset * 3 + 1));
-		break;
-
-	case RT5677_GPIO6:
-		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL3,
-			RT5677_GPIO6_OUT_MASK, !!value << RT5677_GPIO6_OUT_SFT);
-		break;
-
-	default:
-		break;
-	}
+	rt5677_update_gpio_bits(rt5677, offset, m, level);
 }
 
 static int rt5677_gpio_direction_out(struct gpio_chip *chip,
 				     unsigned offset, int value)
 {
 	struct rt5677_priv *rt5677 = gpiochip_get_data(chip);
+	int level = value ? RT5677_GPIOx_OUT_HI : RT5677_GPIOx_OUT_LO;
+	int m = RT5677_GPIOx_DIR_MASK | RT5677_GPIOx_OUT_MASK;
+	int v = RT5677_GPIOx_DIR_OUT | level;
 
-	switch (offset) {
-	case RT5677_GPIO1 ... RT5677_GPIO5:
-		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL2,
-			0x3 << (offset * 3 + 1),
-			(0x2 | !!value) << (offset * 3 + 1));
-		break;
-
-	case RT5677_GPIO6:
-		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL3,
-			RT5677_GPIO6_DIR_MASK | RT5677_GPIO6_OUT_MASK,
-			RT5677_GPIO6_DIR_OUT | !!value << RT5677_GPIO6_OUT_SFT);
-		break;
-
-	default:
-		break;
-	}
-
-	return 0;
+	return rt5677_update_gpio_bits(rt5677, offset, m, v);
 }
 
 static int rt5677_gpio_get(struct gpio_chip *chip, unsigned offset)
@@ -4778,26 +4762,14 @@ static int rt5677_gpio_get(struct gpio_chip *chip, unsigned offset)
 static int rt5677_gpio_direction_in(struct gpio_chip *chip, unsigned offset)
 {
 	struct rt5677_priv *rt5677 = gpiochip_get_data(chip);
+	int m = RT5677_GPIOx_DIR_MASK;
+	int v = RT5677_GPIOx_DIR_IN;
 
-	switch (offset) {
-	case RT5677_GPIO1 ... RT5677_GPIO5:
-		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL2,
-			0x1 << (offset * 3 + 2), 0x0);
-		break;
-
-	case RT5677_GPIO6:
-		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL3,
-			RT5677_GPIO6_DIR_MASK, RT5677_GPIO6_DIR_IN);
-		break;
-
-	default:
-		break;
-	}
-
-	return 0;
+	return rt5677_update_gpio_bits(rt5677, offset, m, v);
 }
 
-/** Configures the gpio as
+/*
+ * Configures the GPIO as
  *   0 - floating
  *   1 - pull down
  *   2 - pull up
@@ -5673,9 +5645,9 @@ static int rt5677_i2c_probe(struct i2c_client *i2c)
 		regmap_update_bits(rt5677->regmap, RT5677_GEN_CTRL2,
 					RT5677_GPIO5_FUNC_MASK,
 					RT5677_GPIO5_FUNC_DMIC);
-		regmap_update_bits(rt5677->regmap, RT5677_GPIO_CTRL2,
-					RT5677_GPIO5_DIR_MASK,
-					RT5677_GPIO5_DIR_OUT);
+		rt5677_update_gpio_bits(rt5677, RT5677_GPIO5,
+					RT5677_GPIOx_DIR_MASK,
+					RT5677_GPIOx_DIR_OUT);
 	}
 
 	if (rt5677->pdata.micbias1_vdd_3v3)
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index 944ae02aafc2..5932b04cf85e 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -1587,81 +1587,19 @@
 #define RT5677_FUNC_MODE_DMIC_GPIO		(0x0 << 13)
 #define RT5677_FUNC_MODE_JTAG			(0x1 << 13)
 
-/* GPIO Control 2 (0xc1) */
-#define RT5677_GPIO5_DIR_MASK			(0x1 << 14)
-#define RT5677_GPIO5_DIR_SFT			14
-#define RT5677_GPIO5_DIR_IN			(0x0 << 14)
-#define RT5677_GPIO5_DIR_OUT			(0x1 << 14)
-#define RT5677_GPIO5_OUT_MASK			(0x1 << 13)
-#define RT5677_GPIO5_OUT_SFT			13
-#define RT5677_GPIO5_OUT_LO			(0x0 << 13)
-#define RT5677_GPIO5_OUT_HI			(0x1 << 13)
-#define RT5677_GPIO5_P_MASK			(0x1 << 12)
-#define RT5677_GPIO5_P_SFT			12
-#define RT5677_GPIO5_P_NOR			(0x0 << 12)
-#define RT5677_GPIO5_P_INV			(0x1 << 12)
-#define RT5677_GPIO4_DIR_MASK			(0x1 << 11)
-#define RT5677_GPIO4_DIR_SFT			11
-#define RT5677_GPIO4_DIR_IN			(0x0 << 11)
-#define RT5677_GPIO4_DIR_OUT			(0x1 << 11)
-#define RT5677_GPIO4_OUT_MASK			(0x1 << 10)
-#define RT5677_GPIO4_OUT_SFT			10
-#define RT5677_GPIO4_OUT_LO			(0x0 << 10)
-#define RT5677_GPIO4_OUT_HI			(0x1 << 10)
-#define RT5677_GPIO4_P_MASK			(0x1 << 9)
-#define RT5677_GPIO4_P_SFT			9
-#define RT5677_GPIO4_P_NOR			(0x0 << 9)
-#define RT5677_GPIO4_P_INV			(0x1 << 9)
-#define RT5677_GPIO3_DIR_MASK			(0x1 << 8)
-#define RT5677_GPIO3_DIR_SFT			8
-#define RT5677_GPIO3_DIR_IN			(0x0 << 8)
-#define RT5677_GPIO3_DIR_OUT			(0x1 << 8)
-#define RT5677_GPIO3_OUT_MASK			(0x1 << 7)
-#define RT5677_GPIO3_OUT_SFT			7
-#define RT5677_GPIO3_OUT_LO			(0x0 << 7)
-#define RT5677_GPIO3_OUT_HI			(0x1 << 7)
-#define RT5677_GPIO3_P_MASK			(0x1 << 6)
-#define RT5677_GPIO3_P_SFT			6
-#define RT5677_GPIO3_P_NOR			(0x0 << 6)
-#define RT5677_GPIO3_P_INV			(0x1 << 6)
-#define RT5677_GPIO2_DIR_MASK			(0x1 << 5)
-#define RT5677_GPIO2_DIR_SFT			5
-#define RT5677_GPIO2_DIR_IN			(0x0 << 5)
-#define RT5677_GPIO2_DIR_OUT			(0x1 << 5)
-#define RT5677_GPIO2_OUT_MASK			(0x1 << 4)
-#define RT5677_GPIO2_OUT_SFT			4
-#define RT5677_GPIO2_OUT_LO			(0x0 << 4)
-#define RT5677_GPIO2_OUT_HI			(0x1 << 4)
-#define RT5677_GPIO2_P_MASK			(0x1 << 3)
-#define RT5677_GPIO2_P_SFT			3
-#define RT5677_GPIO2_P_NOR			(0x0 << 3)
-#define RT5677_GPIO2_P_INV			(0x1 << 3)
-#define RT5677_GPIO1_DIR_MASK			(0x1 << 2)
-#define RT5677_GPIO1_DIR_SFT			2
-#define RT5677_GPIO1_DIR_IN			(0x0 << 2)
-#define RT5677_GPIO1_DIR_OUT			(0x1 << 2)
-#define RT5677_GPIO1_OUT_MASK			(0x1 << 1)
-#define RT5677_GPIO1_OUT_SFT			1
-#define RT5677_GPIO1_OUT_LO			(0x0 << 1)
-#define RT5677_GPIO1_OUT_HI			(0x1 << 1)
-#define RT5677_GPIO1_P_MASK			(0x1 << 0)
-#define RT5677_GPIO1_P_SFT			0
-#define RT5677_GPIO1_P_NOR			(0x0 << 0)
-#define RT5677_GPIO1_P_INV			(0x1 << 0)
-
-/* GPIO Control 3 (0xc2) */
-#define RT5677_GPIO6_DIR_MASK			(0x1 << 2)
-#define RT5677_GPIO6_DIR_SFT			2
-#define RT5677_GPIO6_DIR_IN			(0x0 << 2)
-#define RT5677_GPIO6_DIR_OUT			(0x1 << 2)
-#define RT5677_GPIO6_OUT_MASK			(0x1 << 1)
-#define RT5677_GPIO6_OUT_SFT			1
-#define RT5677_GPIO6_OUT_LO			(0x0 << 1)
-#define RT5677_GPIO6_OUT_HI			(0x1 << 1)
-#define RT5677_GPIO6_P_MASK			(0x1 << 0)
-#define RT5677_GPIO6_P_SFT			0
-#define RT5677_GPIO6_P_NOR			(0x0 << 0)
-#define RT5677_GPIO6_P_INV			(0x1 << 0)
+/* GPIO Control 2 (0xc1) & 3 (0xc2) common bits */
+#define RT5677_GPIOx_DIR_MASK			(0x1 << 2)
+#define RT5677_GPIOx_DIR_SFT			2
+#define RT5677_GPIOx_DIR_IN			(0x0 << 2)
+#define RT5677_GPIOx_DIR_OUT			(0x1 << 2)
+#define RT5677_GPIOx_OUT_MASK			(0x1 << 1)
+#define RT5677_GPIOx_OUT_SFT			1
+#define RT5677_GPIOx_OUT_LO			(0x0 << 1)
+#define RT5677_GPIOx_OUT_HI			(0x1 << 1)
+#define RT5677_GPIOx_P_MASK			(0x1 << 0)
+#define RT5677_GPIOx_P_SFT			0
+#define RT5677_GPIOx_P_NOR			(0x0 << 0)
+#define RT5677_GPIOx_P_INV			(0x1 << 0)
 
 /* General Control (0xfa) */
 #define RT5677_IRQ_DEBOUNCE_SEL_MASK		(0x3 << 3)
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 2/4] ASoC: rt5677: Use agnostic irq_domain_create_linear()

[Andy Shevchenko]

Instead of irq_domain_add_linear() that requires of_node,
use irq_domain_create_linear() that works outside of OF world.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/codecs/rt5677.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 3a2a6b150cda..17d5dd5d2974 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -5511,7 +5511,7 @@ static int rt5677_init_irq(struct i2c_client *i2c)
 			RT5677_GPIO1_PIN_MASK, RT5677_GPIO1_PIN_IRQ);
 
 	/* Ready to listen for interrupts */
-	rt5677->domain = irq_domain_add_linear(i2c->dev.of_node,
+	rt5677->domain = irq_domain_create_linear(dev_fwnode(&i2c->dev),
 			RT5677_IRQ_NUM, &rt5677_domain_ops, rt5677);
 	if (!rt5677->domain) {
 		dev_err(&i2c->dev, "Failed to create IRQ domain\n");
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 3/4] ASoC: rt5677: Use device_get_match_data()

[Andy Shevchenko]

Use device_get_match_data() to simplify the code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/codecs/rt5677.c | 21 ++++-----------------
 sound/soc/codecs/rt5677.h |  4 ++--
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index 17d5dd5d2974..b0c15e27c763 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -6,7 +6,6 @@
  * Author: Oder Chiou <oder_chiou@realtek.com>
  */
 
-#include <linux/acpi.h>
 #include <linux/fs.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
@@ -18,7 +17,6 @@
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/firmware.h>
-#include <linux/of_device.h>
 #include <linux/property.h>
 #include <linux/irq.h>
 #include <linux/interrupt.h>
@@ -5531,6 +5529,7 @@ static int rt5677_init_irq(struct i2c_client *i2c)
 
 static int rt5677_i2c_probe(struct i2c_client *i2c)
 {
+	struct device *dev = &i2c->dev;
 	struct rt5677_priv *rt5677;
 	int ret;
 	unsigned int val;
@@ -5545,21 +5544,9 @@ static int rt5677_i2c_probe(struct i2c_client *i2c)
 	INIT_DELAYED_WORK(&rt5677->dsp_work, rt5677_dsp_work);
 	i2c_set_clientdata(i2c, rt5677);
 
-	if (i2c->dev.of_node) {
-		const struct of_device_id *match_id;
-
-		match_id = of_match_device(rt5677_of_match, &i2c->dev);
-		if (match_id)
-			rt5677->type = (enum rt5677_type)match_id->data;
-	} else if (ACPI_HANDLE(&i2c->dev)) {
-		const struct acpi_device_id *acpi_id;
-
-		acpi_id = acpi_match_device(rt5677_acpi_match, &i2c->dev);
-		if (acpi_id)
-			rt5677->type = (enum rt5677_type)acpi_id->driver_data;
-	} else {
+	rt5677->type = (enum rt5677_type)(uintptr_t)device_get_match_data(dev);
+	if (rt5677->type == 0)
 		return -EINVAL;
-	}
 
 	rt5677_read_device_properties(rt5677, &i2c->dev);
 
@@ -5674,7 +5661,7 @@ static struct i2c_driver rt5677_i2c_driver = {
 	.driver = {
 		.name = RT5677_DRV_NAME,
 		.of_match_table = rt5677_of_match,
-		.acpi_match_table = ACPI_PTR(rt5677_acpi_match),
+		.acpi_match_table = rt5677_acpi_match,
 	},
 	.probe    = rt5677_i2c_probe,
 	.remove   = rt5677_i2c_remove,
diff --git a/sound/soc/codecs/rt5677.h b/sound/soc/codecs/rt5677.h
index 5932b04cf85e..d67ebae067d9 100644
--- a/sound/soc/codecs/rt5677.h
+++ b/sound/soc/codecs/rt5677.h
@@ -1691,8 +1691,8 @@ enum {
 };
 
 enum rt5677_type {
-	RT5677,
-	RT5676,
+	RT5677 = 1,
+	RT5676 = 2,
 };
 
 /* ASRC clock source selection */
-- 
2.40.0.1.gaa8946217a0b

[PATCH v2 4/4] ASoC: rt5677: Sort headers alphabetically

[Andy Shevchenko]

It's hard to see what's included and what's not on the glance.
Sort headers alphabetically to improve maintenance.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 sound/soc/codecs/rt5677.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index b0c15e27c763..0e70a3ab42b5 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -6,21 +6,21 @@
  * Author: Oder Chiou <oder_chiou@realtek.com>
  */
 
+#include <linux/delay.h>
+#include <linux/firmware.h>
 #include <linux/fs.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/irqdomain.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
-#include <linux/init.h>
-#include <linux/delay.h>
+#include <linux/platform_device.h>
 #include <linux/pm.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
-#include <linux/i2c.h>
-#include <linux/platform_device.h>
 #include <linux/spi/spi.h>
-#include <linux/firmware.h>
-#include <linux/property.h>
-#include <linux/irq.h>
-#include <linux/interrupt.h>
-#include <linux/irqdomain.h>
 #include <linux/workqueue.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
-- 
2.40.0.1.gaa8946217a0b

Re: [PATCH v2 0/4] ASoC: rt5677: Refactor GPIO and use device_get_match_data()

[Mark Brown]

On Fri, 30 Jun 2023 20:21:51 +0300, Andy Shevchenko wrote:
> The code can be simplified with refactored GPIO parts and with use of
> device_get_match_data(). Besides that couple of additional changes,
> one for maintenance and one for making IRQ domain agnostic (not being
> pinned to OF).
> 
> Changelog v2:
> - refactored GPIO code in (a new) patch 1
> - fixed compilation error in patch 2 (LKP)
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/4] ASoC: rt5677: Refactor GPIO support code
      commit: 5512ffd9f39832f312b7f903703ac39d6367fe8a
[2/4] ASoC: rt5677: Use agnostic irq_domain_create_linear()
      commit: c3d42d7baf6b4032171270e3df001fb946493452
[3/4] ASoC: rt5677: Use device_get_match_data()
      commit: 043bb9c012ee7d092a477159cc66dbdf62fd2666
[4/4] ASoC: rt5677: Sort headers alphabetically
      commit: ea1c1019a88d88cf0a7d6892f594b72ddb3b8c0b

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH v2 0/4] ASoC: rt5677: Refactor GPIO and use device_get_match_data()

[Andy Shevchenko]

On Wed, Jul 12, 2023 at 12:47:06PM +0100, Mark Brown wrote:
> On Fri, 30 Jun 2023 20:21:51 +0300, Andy Shevchenko wrote:
> > The code can be simplified with refactored GPIO parts and with use of
> > device_get_match_data(). Besides that couple of additional changes,
> > one for maintenance and one for making IRQ domain agnostic (not being
> > pinned to OF).

> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

JFYI: You might need to amend your scripts (?) as in the merge commit
      the Subject is cut (I believe that your scripts doesn't handle
      RFC2822 long lines in the email headers).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/4] ASoC: rt5677: Refactor GPIO and use device_get_match_data()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]

On Wed, Jul 12, 2023 at 04:56:51PM +0300, Andy Shevchenko wrote:

> JFYI: You might need to amend your scripts (?) as in the merge commit
>       the Subject is cut (I believe that your scripts doesn't handle
>       RFC2822 long lines in the email headers).

Report that to Konstantin, this is just b4.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 0/4] ASoC: rt5677: Refactor GPIO and use device_get_match_data()

[Andy Shevchenko]

+Cc: Konstantin.

Konstantin, seems b4 doesn't handle long Subject lines for merge commits
(PR mode).

On Wed, Jul 12, 2023 at 04:23:11PM +0100, Mark Brown wrote:
> On Wed, Jul 12, 2023 at 04:56:51PM +0300, Andy Shevchenko wrote:
> 
> > JFYI: You might need to amend your scripts (?) as in the merge commit
> >       the Subject is cut (I believe that your scripts doesn't handle
> >       RFC2822 long lines in the email headers).
> 
> Report that to Konstantin, this is just b4.



-- 
With Best Regards,
Andy Shevchenko