[PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Andy Shevchenko]

There is no platform code that uses i2c module table.
Remove it altogether and adjust ->probe() to be ->probe_new().

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

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index bc1a23dd7c2d..8a0181a2db08 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -5006,13 +5006,6 @@ static const struct regmap_config rt5677_regmap = {
 	.num_ranges = ARRAY_SIZE(rt5677_ranges),
 };
 
-static const struct i2c_device_id rt5677_i2c_id[] = {
-	{ "rt5677", RT5677 },
-	{ "rt5676", RT5676 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id);
-
 static const struct of_device_id rt5677_of_match[] = {
 	{ .compatible = "realtek,rt5677", RT5677 },
 	{ }
@@ -5130,8 +5123,7 @@ static void rt5677_free_irq(struct i2c_client *i2c)
 		regmap_del_irq_chip(i2c->irq, rt5677->irq_data);
 }
 
-static int rt5677_i2c_probe(struct i2c_client *i2c,
-		    const struct i2c_device_id *id)
+static int rt5677_i2c_probe(struct i2c_client *i2c)
 {
 	struct rt5677_priv *rt5677;
 	int ret;
@@ -5278,9 +5270,8 @@ static struct i2c_driver rt5677_i2c_driver = {
 		.of_match_table = rt5677_of_match,
 		.acpi_match_table = ACPI_PTR(rt5677_acpi_match),
 	},
-	.probe = rt5677_i2c_probe,
+	.probe_new = rt5677_i2c_probe,
 	.remove   = rt5677_i2c_remove,
-	.id_table = rt5677_i2c_id,
 };
 module_i2c_driver(rt5677_i2c_driver);
 
-- 
2.17.0

Applied "ASoC: rt5677: Convert I2C driver to ->probe_new()" to the asoc tree

[Mark Brown]

The patch

   ASoC: rt5677: Convert I2C driver to ->probe_new()

has been applied to the asoc tree at

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

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

>From 3a4f4f2963f48a4dcc0e18a03bef66eaf3d46c55 Mon Sep 17 00:00:00 2001
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Fri, 18 May 2018 18:41:51 +0300
Subject: [PATCH] ASoC: rt5677: Convert I2C driver to ->probe_new()

There is no platform code that uses i2c module table.
Remove it altogether and adjust ->probe() to be ->probe_new().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/rt5677.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index bc1a23dd7c2d..8a0181a2db08 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -5006,13 +5006,6 @@ static const struct regmap_config rt5677_regmap = {
 	.num_ranges = ARRAY_SIZE(rt5677_ranges),
 };
 
-static const struct i2c_device_id rt5677_i2c_id[] = {
-	{ "rt5677", RT5677 },
-	{ "rt5676", RT5676 },
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id);
-
 static const struct of_device_id rt5677_of_match[] = {
 	{ .compatible = "realtek,rt5677", RT5677 },
 	{ }
@@ -5130,8 +5123,7 @@ static void rt5677_free_irq(struct i2c_client *i2c)
 		regmap_del_irq_chip(i2c->irq, rt5677->irq_data);
 }
 
-static int rt5677_i2c_probe(struct i2c_client *i2c,
-		    const struct i2c_device_id *id)
+static int rt5677_i2c_probe(struct i2c_client *i2c)
 {
 	struct rt5677_priv *rt5677;
 	int ret;
@@ -5278,9 +5270,8 @@ static struct i2c_driver rt5677_i2c_driver = {
 		.of_match_table = rt5677_of_match,
 		.acpi_match_table = ACPI_PTR(rt5677_acpi_match),
 	},
-	.probe = rt5677_i2c_probe,
+	.probe_new = rt5677_i2c_probe,
 	.remove   = rt5677_i2c_remove,
-	.id_table = rt5677_i2c_id,
 };
 module_i2c_driver(rt5677_i2c_driver);
 
-- 
2.17.0

[PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Andy Shevchenko]

There is no platform code that uses i2c module table.
Remove it altogether and adjust ->probe() to be ->probe_new().

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

diff --git a/sound/soc/codecs/rt5677.c b/sound/soc/codecs/rt5677.c
index c735590c5a25..6448b7a00203 100644
--- a/sound/soc/codecs/rt5677.c
+++ b/sound/soc/codecs/rt5677.c
@@ -5009,11 +5009,6 @@ static const struct regmap_config rt5677_regmap = {
 	.num_ranges = ARRAY_SIZE(rt5677_ranges),
 };
 
-static const struct i2c_device_id rt5677_i2c_id[] = {
-	{ }
-};
-MODULE_DEVICE_TABLE(i2c, rt5677_i2c_id);
-
 static const struct of_device_id rt5677_of_match[] = {
 	{ .compatible = "realtek,rt5677", RT5677 },
 	{ }
@@ -5131,8 +5126,7 @@ static void rt5677_free_irq(struct i2c_client *i2c)
 		regmap_del_irq_chip(i2c->irq, rt5677->irq_data);
 }
 
-static int rt5677_i2c_probe(struct i2c_client *i2c,
-		    const struct i2c_device_id *id)
+static int rt5677_i2c_probe(struct i2c_client *i2c)
 {
 	struct rt5677_priv *rt5677;
 	int ret;
@@ -5279,9 +5273,8 @@ static struct i2c_driver rt5677_i2c_driver = {
 		.of_match_table = rt5677_of_match,
 		.acpi_match_table = ACPI_PTR(rt5677_acpi_match),
 	},
-	.probe = rt5677_i2c_probe,
+	.probe_new = rt5677_i2c_probe,
 	.remove   = rt5677_i2c_remove,
-	.id_table = rt5677_i2c_id,
 };
 module_i2c_driver(rt5677_i2c_driver);
 
-- 
2.14.1

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 344 bytes --]

On Thu, Aug 24, 2017 at 04:41:10PM +0300, Andy Shevchenko wrote:

> There is no platform code that uses i2c module table.
> Remove it altogether and adjust ->probe() to be ->probe_new().

What is the value in doing this?  Removing the ability to instantiate
via board files is just inconveniencing people doing that, it's not a
goal in itself.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Takashi Iwai]

On Thu, 24 Aug 2017 15:49:24 +0200,
Mark Brown wrote:
> 
> On Thu, Aug 24, 2017 at 04:41:10PM +0300, Andy Shevchenko wrote:
> 
> > There is no platform code that uses i2c module table.
> > Remove it altogether and adjust ->probe() to be ->probe_new().
> 
> What is the value in doing this?  Removing the ability to instantiate
> via board files is just inconveniencing people doing that, it's not a
> goal in itself.

But the id table is already empty...?


Takashi

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 549 bytes --]

On Thu, Aug 24, 2017 at 04:33:29PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > On Thu, Aug 24, 2017 at 04:41:10PM +0300, Andy Shevchenko wrote:
> > 
> > > There is no platform code that uses i2c module table.
> > > Remove it altogether and adjust ->probe() to be ->probe_new().

> > What is the value in doing this?  Removing the ability to instantiate
> > via board files is just inconveniencing people doing that, it's not a
> > goal in itself.

> But the id table is already empty...?

Right, that seems like a bug though.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Takashi Iwai]

On Thu, 24 Aug 2017 17:49:47 +0200,
Mark Brown wrote:
> 
> On Thu, Aug 24, 2017 at 04:33:29PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > On Thu, Aug 24, 2017 at 04:41:10PM +0300, Andy Shevchenko wrote:
> > > 
> > > > There is no platform code that uses i2c module table.
> > > > Remove it altogether and adjust ->probe() to be ->probe_new().
> 
> > > What is the value in doing this?  Removing the ability to instantiate
> > > via board files is just inconveniencing people doing that, it's not a
> > > goal in itself.
> 
> > But the id table is already empty...?
> 
> Right, that seems like a bug though.

Well, that's what the commit ddc9e69b9dc2 does after all: it cleans up
the legacy usage, and moves all either OF or ACPI matching.

If we want to take back old i2c ids, better to put the explicit
i2c_match_id() check in rt5677_i2c_probe() instead of sticking with
the old i2c probe callback.


Takashi

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Andy Shevchenko]

On Thu, 2017-08-24 at 18:11 +0200, Takashi Iwai wrote:
> On Thu, 24 Aug 2017 17:49:47 +0200,
> Mark Brown wrote:
> > 
> > On Thu, Aug 24, 2017 at 04:33:29PM +0200, Takashi Iwai wrote:
> > > Mark Brown wrote:
> > > > On Thu, Aug 24, 2017 at 04:41:10PM +0300, Andy Shevchenko wrote:
> > > > 
> > > > > There is no platform code that uses i2c module table.
> > > > > Remove it altogether and adjust ->probe() to be ->probe_new().
> > > > What is the value in doing this?  Removing the ability to
> > > > instantiate
> > > > via board files is just inconveniencing people doing that, it's
> > > > not a
> > > > goal in itself.
> > > But the id table is already empty...?
> > 
> > Right, that seems like a bug though.
> 
> Well, that's what the commit ddc9e69b9dc2 does after all: it cleans up
> the legacy usage, and moves all either OF or ACPI matching.
> 
> If we want to take back old i2c ids, better to put the explicit
> i2c_match_id() check in rt5677_i2c_probe() instead of sticking with
> the old i2c probe callback.

Well, there was never user for rt5676, thus, platform code can use
driver name to register / create platform device to be matched.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 536 bytes --]

On Thu, Aug 24, 2017 at 08:12:12PM +0300, Andy Shevchenko wrote:
> On Thu, 2017-08-24 at 18:11 +0200, Takashi Iwai wrote:

> > If we want to take back old i2c ids, better to put the explicit
> > i2c_match_id() check in rt5677_i2c_probe() instead of sticking with
> > the old i2c probe callback.

> Well, there was never user for rt5676, thus, platform code can use
> driver name to register / create platform device to be matched.

The driver handle both rt5676 and rt5677 using different IDs so it can't
just rely on the driver table.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 554 bytes --]

On Thu, Aug 24, 2017 at 06:11:12PM +0200, Takashi Iwai wrote:
> Mark Brown wrote:

> > > But the id table is already empty...?

> > Right, that seems like a bug though.

> Well, that's what the commit ddc9e69b9dc2 does after all: it cleans up
> the legacy usage, and moves all either OF or ACPI matching.

Which is broken since not all the world is ACPI or DT.

> If we want to take back old i2c ids, better to put the explicit
> i2c_match_id() check in rt5677_i2c_probe() instead of sticking with
> the old i2c probe callback.

That's what I said, yes.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Andy Shevchenko]

On Fri, 2017-08-25 at 14:11 +0100, Mark Brown wrote:
> On Thu, Aug 24, 2017 at 06:11:12PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> > > > But the id table is already empty...?
> > > Right, that seems like a bug though.
> > Well, that's what the commit ddc9e69b9dc2 does after all: it cleans
> > up
> > the legacy usage, and moves all either OF or ACPI matching.
> 
> Which is broken since not all the world is ACPI or DT.

While I agree on the second part, I don't think it's broken per se,
there never was a single user of that, and looking how ASoC board files
are organized it is quite unlikely there will be one...

> > If we want to take back old i2c ids, better to put the explicit
> > i2c_match_id() check in rt5677_i2c_probe() instead of sticking with
> > the old i2c probe callback.
> 
> That's what I said, yes.

...though we can (re)introduce this dead code back for a potential user
in the future.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 959 bytes --]

On Fri, Aug 25, 2017 at 04:33:08PM +0300, Andy Shevchenko wrote:
> On Fri, 2017-08-25 at 14:11 +0100, Mark Brown wrote:
> > On Thu, Aug 24, 2017 at 06:11:12PM +0200, Takashi Iwai wrote:

> > > Well, that's what the commit ddc9e69b9dc2 does after all: it cleans
> > > up
> > > the legacy usage, and moves all either OF or ACPI matching.

> > Which is broken since not all the world is ACPI or DT.

> While I agree on the second part, I don't think it's broken per se,
> there never was a single user of that, and looking how ASoC board files
> are organized it is quite unlikely there will be one...

I have no idea why you say the ASoC machine drivers would prevent
someone adding a platform based user, obviously ASoC predates the use of
both ACPI and DT quite considerably.

> > That's what I said, yes.

> ...though we can (re)introduce this dead code back for a potential user
> in the future.

The IDs are back as a result of the merge resolution I did.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Andy Shevchenko]

On Fri, 2017-08-25 at 15:21 +0100, Mark Brown wrote:
> On Fri, Aug 25, 2017 at 04:33:08PM +0300, Andy Shevchenko wrote:
> > On Fri, 2017-08-25 at 14:11 +0100, Mark Brown wrote:
> > > On Thu, Aug 24, 2017 at 06:11:12PM +0200, Takashi Iwai wrote:
> > > > Well, that's what the commit ddc9e69b9dc2 does after all: it
> > > > cleans
> > > > up
> > > > the legacy usage, and moves all either OF or ACPI matching.
> > > Which is broken since not all the world is ACPI or DT.
> > While I agree on the second part, I don't think it's broken per se,
> > there never was a single user of that, and looking how ASoC board
> > files
> > are organized it is quite unlikely there will be one...
> 
> I have no idea why you say the ASoC machine drivers would prevent
> someone adding a platform based user,

For example, looking into
 sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c
I see a direct use of codec driver without any platform driver
involvement.

Is it exception from the rule, or a common use?

>  obviously ASoC predates the use of
> both ACPI and DT quite considerably.

Yep, the current users of driver in question either DT or ACPI, would it
be pure platform in the future?

> 
> > > That's what I said, yes.
> > ...though we can (re)introduce this dead code back for a potential
> > user
> > in the future.
> 
> The IDs are back as a result of the merge resolution I did.

I'm fine as long as it doesn't include broken 'ACPI instance as I2C ID'.
Thanks!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1] ASoC: rt5677: Convert I2C driver to ->probe_new()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 904 bytes --]

On Fri, Aug 25, 2017 at 06:09:52PM +0300, Andy Shevchenko wrote:
> On Fri, 2017-08-25 at 15:21 +0100, Mark Brown wrote:

> > I have no idea why you say the ASoC machine drivers would prevent
> > someone adding a platform based user,

> For example, looking into
>  sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c
> I see a direct use of codec driver without any platform driver
> involvement.

> Is it exception from the rule, or a common use?

That's an entirely standard driver using normally registered devices?

> >  obviously ASoC predates the use of
> > both ACPI and DT quite considerably.

> Yep, the current users of driver in question either DT or ACPI, would it
> be pure platform in the future?

It could be potentially (well, I2C but registered using a board file).
Most architectures still use board files and neither ACPI nor DT has a
sensible story for instantiating plugin modules yet.

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

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]