[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 | 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: 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: 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: 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 --]

[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