All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-24 10:52 ` Jarkko Nikula
  0 siblings, 0 replies; 30+ messages in thread
From: Jarkko Nikula @ 2015-08-24 10:52 UTC (permalink / raw)
  To: linux-i2c
  Cc: alsa-devel, linux-acpi, lm-sensors, Wolfram Sang, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux, Jarkko Nikula

Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
/sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
code in lm-sensors does not find them anymore:

lib/sysfs.c:665:
if ((!subsys || !strcmp(subsys, "i2c")) &&
    sscanf(dev_name, "%hd-%x", &entry.chip.bus.nr,
	   &entry.chip.addr) == 2) {

This patch fixes this by reverting back the old device naming in i2c-core
but at the same avoids regression to ALSA SoC drivers that depend on stable
device binding.

Reverted I2C slave device naming is handled in ALSA SoC core by using the
ACPI name as a component name if device is ACPI probed. This keeps the
component binding in ALSA SoC stable and requires only minimal changes to
affected machine drivers.

Fixes: 70762abb9f89 i2c: Use stable dev_name for ACPI enumerated I2C slaves
Reported-by: Dustin Byford <dustin@cumulusnetworks.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
This is on top of 4.2.0-rc8. I didn't check exact kernel versions where this
still applies but I know we would need a few different versions for older
stable versions. Mainly because of added and moved drivers in
sound/soc/intel/.

This is for discussion so I didn't cc stable@vger.kernel.org yet. I was
thinking would it work if we'd keep the stable name but have an another
symlink in /sys/bus/i2c/devices/ that uses "x-00yz" name. However this
feels ill-use of devices directory and probably causes more troubles
elsewhere.

I don't know how common ACPI enumerated I2C hwmon devices are but I guess
they exists since Dustin notices this issue and wrote "With that change,
/sys/bus/i2c/devices/i2c-0-004a, for example, became
/sys/bus/i2c/devices/i2c-PNPXXXX:xx".

It could be possible there is use for the new device naming. I found a
comment from Documentation/hid/hid-sensor.txt introduced by commit
b2eafd7282fd ("HID: sensor: Update document for custom sensor") that seems
to indicate current i2c-INTABCD:xy naming may be used from userspace too but
I didn't find examples of that.
---
 drivers/i2c/i2c-core.c                       | 21 ++++-----------------
 sound/soc/intel/boards/broadwell.c           |  6 +++---
 sound/soc/intel/boards/byt-max98090.c        |  2 +-
 sound/soc/intel/boards/byt-rt5640.c          |  2 +-
 sound/soc/intel/boards/bytcr_rt5640.c        |  2 +-
 sound/soc/intel/boards/cht_bsw_max98090_ti.c |  4 ++--
 sound/soc/intel/boards/cht_bsw_rt5645.c      |  4 ++--
 sound/soc/intel/boards/cht_bsw_rt5672.c      |  6 +++---
 sound/soc/intel/boards/haswell.c             |  2 +-
 sound/soc/soc-core.c                         | 10 ++++++++++
 10 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index c83e4d13cfc5..e9c227b9dc92 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -913,22 +913,6 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
 
-static void i2c_dev_set_name(struct i2c_adapter *adap,
-			     struct i2c_client *client)
-{
-	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
-
-	if (adev) {
-		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
-		return;
-	}
-
-	/* For 10-bit clients, add an arbitrary offset to avoid collisions */
-	dev_set_name(&client->dev, "%d-%04x", i2c_adapter_id(adap),
-		     client->addr | ((client->flags & I2C_CLIENT_TEN)
-				     ? 0xa000 : 0));
-}
-
 /**
  * i2c_new_device - instantiate an i2c device
  * @adap: the adapter managing the device
@@ -987,7 +971,10 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->dev.of_node = info->of_node;
 	client->dev.fwnode = info->fwnode;
 
-	i2c_dev_set_name(adap, client);
+	/* For 10-bit clients, add an arbitrary offset to avoid collisions */
+	dev_set_name(&client->dev, "%d-%04x", i2c_adapter_id(adap),
+		     client->addr | ((client->flags & I2C_CLIENT_TEN)
+				     ? 0xa000 : 0));
 	status = device_register(&client->dev);
 	if (status)
 		goto out_err;
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c
index 8bafaf6ceab1..3abcf0d7682e 100644
--- a/sound/soc/intel/boards/broadwell.c
+++ b/sound/soc/intel/boards/broadwell.c
@@ -205,7 +205,7 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = {
 		.cpu_dai_name = "snd-soc-dummy-dai",
 		.platform_name = "snd-soc-dummy",
 		.no_pcm = 1,
-		.codec_name = "i2c-INT343A:00",
+		.codec_name = "INT343A:00",
 		.codec_dai_name = "rt286-aif1",
 		.init = broadwell_rt286_codec_init,
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
@@ -223,7 +223,7 @@ static int broadwell_suspend(struct snd_soc_card *card){
 	struct snd_soc_codec *codec;
 
 	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
-		if (!strcmp(codec->component.name, "i2c-INT343A:00")) {
+		if (!strcmp(codec->component.name, "INT343A:00")) {
 			dev_dbg(codec->dev, "disabling jack detect before going to suspend.\n");
 			rt286_mic_detect(codec, NULL);
 			break;
@@ -236,7 +236,7 @@ static int broadwell_resume(struct snd_soc_card *card){
 	struct snd_soc_codec *codec;
 
 	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
-		if (!strcmp(codec->component.name, "i2c-INT343A:00")) {
+		if (!strcmp(codec->component.name, "INT343A:00")) {
 			dev_dbg(codec->dev, "enabling jack detect for resume.\n");
 			rt286_mic_detect(codec, &broadwell_headset);
 			break;
diff --git a/sound/soc/intel/boards/byt-max98090.c b/sound/soc/intel/boards/byt-max98090.c
index 7ab8cc9fbfd5..ae336b8522a8 100644
--- a/sound/soc/intel/boards/byt-max98090.c
+++ b/sound/soc/intel/boards/byt-max98090.c
@@ -116,7 +116,7 @@ static struct snd_soc_dai_link byt_max98090_dais[] = {
 		.stream_name = "Audio",
 		.cpu_dai_name = "baytrail-pcm-audio",
 		.codec_dai_name = "HiFi",
-		.codec_name = "i2c-193C9890:00",
+		.codec_name = "193C9890:00",
 		.platform_name = "baytrail-pcm-audio",
 		.init = byt_max98090_init,
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
diff --git a/sound/soc/intel/boards/byt-rt5640.c b/sound/soc/intel/boards/byt-rt5640.c
index ae89b9b966d9..275b056c495c 100644
--- a/sound/soc/intel/boards/byt-rt5640.c
+++ b/sound/soc/intel/boards/byt-rt5640.c
@@ -186,7 +186,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = {
 		.stream_name = "Audio",
 		.cpu_dai_name = "baytrail-pcm-audio",
 		.codec_dai_name = "rt5640-aif1",
-		.codec_name = "i2c-10EC5640:00",
+		.codec_name = "10EC5640:00",
 		.platform_name = "baytrail-pcm-audio",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			   SND_SOC_DAIFMT_CBS_CFS,
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 7f55d59024a8..973f07548b08 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -171,7 +171,7 @@ static struct snd_soc_dai_link byt_dailink[] = {
 		.platform_name = "sst-mfld-platform",
 		.no_pcm = 1,
 		.codec_dai_name = "rt5640-aif1",
-		.codec_name = "i2c-10EC5640:00",
+		.codec_name = "10EC5640:00",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
 						| SND_SOC_DAIFMT_CBS_CFS,
 		.be_hw_params_fixup = byt_codec_fixup,
diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
index 70f832114a5a..1a5348771e03 100644
--- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c
+++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
@@ -232,7 +232,7 @@ static struct snd_soc_ops cht_be_ssp2_ops = {
 static struct snd_soc_aux_dev cht_max98090_headset_dev = {
 	.name = "Headset Chip",
 	.init = cht_max98090_headset_init,
-	.codec_name = "i2c-104C227E:00",
+	.codec_name = "104C227E:00",
 };
 
 static struct snd_soc_dai_link cht_dailink[] = {
@@ -265,7 +265,7 @@ static struct snd_soc_dai_link cht_dailink[] = {
 		.platform_name = "sst-mfld-platform",
 		.no_pcm = 1,
 		.codec_dai_name = "HiFi",
-		.codec_name = "i2c-193C9890:00",
+		.codec_name = "193C9890:00",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
 					| SND_SOC_DAIFMT_CBS_CFS,
 		.init = cht_codec_init,
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index bdcaf467842a..c05841264dc7 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -290,7 +290,7 @@ static struct snd_soc_dai_link cht_dailink[] = {
 		.platform_name = "sst-mfld-platform",
 		.no_pcm = 1,
 		.codec_dai_name = "rt5645-aif1",
-		.codec_name = "i2c-10EC5645:00",
+		.codec_name = "10EC5645:00",
 		.dai_fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF
 					| SND_SOC_DAIFMT_CBS_CFS,
 		.init = cht_codec_init,
@@ -365,7 +365,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 		}
 	}
 	card->dev = &pdev->dev;
-	sprintf(codec_name, "i2c-%s:00", drv->acpi_card->codec_id);
+	sprintf(codec_name, "%s:00", drv->acpi_card->codec_id);
 	/* set correct codec name */
 	strcpy((char *)card->dai_link[2].codec_name, codec_name);
 	snd_soc_card_set_drvdata(card, drv);
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
index 2c9cc5be439e..632926d0c595 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5672.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
@@ -280,7 +280,7 @@ static struct snd_soc_dai_link cht_dailink[] = {
 		.no_pcm = 1,
 		.nonatomic = true,
 		.codec_dai_name = "rt5670-aif1",
-		.codec_name = "i2c-10EC5670:00",
+		.codec_name = "10EC5670:00",
 		.dai_fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF
 					| SND_SOC_DAIFMT_CBS_CFS,
 		.init = cht_codec_init,
@@ -296,7 +296,7 @@ static int cht_suspend_pre(struct snd_soc_card *card)
 	struct snd_soc_codec *codec;
 
 	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
-		if (!strcmp(codec->component.name, "i2c-10EC5670:00")) {
+		if (!strcmp(codec->component.name, "10EC5670:00")) {
 			dev_dbg(codec->dev, "disabling jack detect before going to suspend.\n");
 			rt5670_jack_suspend(codec);
 			break;
@@ -310,7 +310,7 @@ static int cht_resume_post(struct snd_soc_card *card)
 	struct snd_soc_codec *codec;
 
 	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
-		if (!strcmp(codec->component.name, "i2c-10EC5670:00")) {
+		if (!strcmp(codec->component.name, "10EC5670:00")) {
 			dev_dbg(codec->dev, "enabling jack detect for resume.\n");
 			rt5670_jack_resume(codec);
 			break;
diff --git a/sound/soc/intel/boards/haswell.c b/sound/soc/intel/boards/haswell.c
index 22558572cb9c..6fd6491e368d 100644
--- a/sound/soc/intel/boards/haswell.c
+++ b/sound/soc/intel/boards/haswell.c
@@ -160,7 +160,7 @@ static struct snd_soc_dai_link haswell_rt5640_dais[] = {
 		.cpu_dai_name = "snd-soc-dummy-dai",
 		.platform_name = "snd-soc-dummy",
 		.no_pcm = 1,
-		.codec_name = "i2c-INT33CA:00",
+		.codec_name = "INT33CA:00",
 		.codec_dai_name = "rt5640-aif1",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			SND_SOC_DAIFMT_CBS_CFS,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 0e1e69c7abd5..811d63e31a7a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -34,6 +34,7 @@
 #include <linux/ctype.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/acpi.h>
 #include <sound/core.h>
 #include <sound/jack.h>
 #include <sound/pcm.h>
@@ -2476,10 +2477,18 @@ static char *fmt_single_name(struct device *dev, int *id)
 {
 	char *found, name[NAME_SIZE];
 	int id1, id2;
+	struct acpi_device *adev;
 
 	if (dev_name(dev) == NULL)
 		return NULL;
 
+	if (ACPI_HANDLE(dev) &&
+	    !acpi_bus_get_device(ACPI_HANDLE(dev), &adev)) {
+		/* use ACPI name as a component name for ACPI probed devices */
+		strlcpy(name, acpi_dev_name(adev), NAME_SIZE);
+		goto out;
+	}
+
 	strlcpy(name, dev_name(dev), NAME_SIZE);
 
 	/* are we a "%s.%d" name (platform and SPI components) */
@@ -2508,6 +2517,7 @@ static char *fmt_single_name(struct device *dev, int *id)
 			*id = 0;
 	}
 
+out:
 	return kstrdup(name, GFP_KERNEL);
 }
 
-- 
2.5.0


^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-24 10:52 ` Jarkko Nikula
  0 siblings, 0 replies; 30+ messages in thread
From: Jarkko Nikula @ 2015-08-24 10:52 UTC (permalink / raw)
  To: linux-i2c
  Cc: alsa-devel, linux-acpi, lm-sensors, Wolfram Sang, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux, Jarkko Nikula

Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
/sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
code in lm-sensors does not find them anymore:

lib/sysfs.c:665:
if ((!subsys || !strcmp(subsys, "i2c")) &&
    sscanf(dev_name, "%hd-%x", &entry.chip.bus.nr,
	   &entry.chip.addr) = 2) {

This patch fixes this by reverting back the old device naming in i2c-core
but at the same avoids regression to ALSA SoC drivers that depend on stable
device binding.

Reverted I2C slave device naming is handled in ALSA SoC core by using the
ACPI name as a component name if device is ACPI probed. This keeps the
component binding in ALSA SoC stable and requires only minimal changes to
affected machine drivers.

Fixes: 70762abb9f89 i2c: Use stable dev_name for ACPI enumerated I2C slaves
Reported-by: Dustin Byford <dustin@cumulusnetworks.com>
Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
This is on top of 4.2.0-rc8. I didn't check exact kernel versions where this
still applies but I know we would need a few different versions for older
stable versions. Mainly because of added and moved drivers in
sound/soc/intel/.

This is for discussion so I didn't cc stable@vger.kernel.org yet. I was
thinking would it work if we'd keep the stable name but have an another
symlink in /sys/bus/i2c/devices/ that uses "x-00yz" name. However this
feels ill-use of devices directory and probably causes more troubles
elsewhere.

I don't know how common ACPI enumerated I2C hwmon devices are but I guess
they exists since Dustin notices this issue and wrote "With that change,
/sys/bus/i2c/devices/i2c-0-004a, for example, became
/sys/bus/i2c/devices/i2c-PNPXXXX:xx".

It could be possible there is use for the new device naming. I found a
comment from Documentation/hid/hid-sensor.txt introduced by commit
b2eafd7282fd ("HID: sensor: Update document for custom sensor") that seems
to indicate current i2c-INTABCD:xy naming may be used from userspace too but
I didn't find examples of that.
---
 drivers/i2c/i2c-core.c                       | 21 ++++-----------------
 sound/soc/intel/boards/broadwell.c           |  6 +++---
 sound/soc/intel/boards/byt-max98090.c        |  2 +-
 sound/soc/intel/boards/byt-rt5640.c          |  2 +-
 sound/soc/intel/boards/bytcr_rt5640.c        |  2 +-
 sound/soc/intel/boards/cht_bsw_max98090_ti.c |  4 ++--
 sound/soc/intel/boards/cht_bsw_rt5645.c      |  4 ++--
 sound/soc/intel/boards/cht_bsw_rt5672.c      |  6 +++---
 sound/soc/intel/boards/haswell.c             |  2 +-
 sound/soc/soc-core.c                         | 10 ++++++++++
 10 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index c83e4d13cfc5..e9c227b9dc92 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -913,22 +913,6 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
 
-static void i2c_dev_set_name(struct i2c_adapter *adap,
-			     struct i2c_client *client)
-{
-	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
-
-	if (adev) {
-		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
-		return;
-	}
-
-	/* For 10-bit clients, add an arbitrary offset to avoid collisions */
-	dev_set_name(&client->dev, "%d-%04x", i2c_adapter_id(adap),
-		     client->addr | ((client->flags & I2C_CLIENT_TEN)
-				     ? 0xa000 : 0));
-}
-
 /**
  * i2c_new_device - instantiate an i2c device
  * @adap: the adapter managing the device
@@ -987,7 +971,10 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->dev.of_node = info->of_node;
 	client->dev.fwnode = info->fwnode;
 
-	i2c_dev_set_name(adap, client);
+	/* For 10-bit clients, add an arbitrary offset to avoid collisions */
+	dev_set_name(&client->dev, "%d-%04x", i2c_adapter_id(adap),
+		     client->addr | ((client->flags & I2C_CLIENT_TEN)
+				     ? 0xa000 : 0));
 	status = device_register(&client->dev);
 	if (status)
 		goto out_err;
diff --git a/sound/soc/intel/boards/broadwell.c b/sound/soc/intel/boards/broadwell.c
index 8bafaf6ceab1..3abcf0d7682e 100644
--- a/sound/soc/intel/boards/broadwell.c
+++ b/sound/soc/intel/boards/broadwell.c
@@ -205,7 +205,7 @@ static struct snd_soc_dai_link broadwell_rt286_dais[] = {
 		.cpu_dai_name = "snd-soc-dummy-dai",
 		.platform_name = "snd-soc-dummy",
 		.no_pcm = 1,
-		.codec_name = "i2c-INT343A:00",
+		.codec_name = "INT343A:00",
 		.codec_dai_name = "rt286-aif1",
 		.init = broadwell_rt286_codec_init,
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
@@ -223,7 +223,7 @@ static int broadwell_suspend(struct snd_soc_card *card){
 	struct snd_soc_codec *codec;
 
 	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
-		if (!strcmp(codec->component.name, "i2c-INT343A:00")) {
+		if (!strcmp(codec->component.name, "INT343A:00")) {
 			dev_dbg(codec->dev, "disabling jack detect before going to suspend.\n");
 			rt286_mic_detect(codec, NULL);
 			break;
@@ -236,7 +236,7 @@ static int broadwell_resume(struct snd_soc_card *card){
 	struct snd_soc_codec *codec;
 
 	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
-		if (!strcmp(codec->component.name, "i2c-INT343A:00")) {
+		if (!strcmp(codec->component.name, "INT343A:00")) {
 			dev_dbg(codec->dev, "enabling jack detect for resume.\n");
 			rt286_mic_detect(codec, &broadwell_headset);
 			break;
diff --git a/sound/soc/intel/boards/byt-max98090.c b/sound/soc/intel/boards/byt-max98090.c
index 7ab8cc9fbfd5..ae336b8522a8 100644
--- a/sound/soc/intel/boards/byt-max98090.c
+++ b/sound/soc/intel/boards/byt-max98090.c
@@ -116,7 +116,7 @@ static struct snd_soc_dai_link byt_max98090_dais[] = {
 		.stream_name = "Audio",
 		.cpu_dai_name = "baytrail-pcm-audio",
 		.codec_dai_name = "HiFi",
-		.codec_name = "i2c-193C9890:00",
+		.codec_name = "193C9890:00",
 		.platform_name = "baytrail-pcm-audio",
 		.init = byt_max98090_init,
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
diff --git a/sound/soc/intel/boards/byt-rt5640.c b/sound/soc/intel/boards/byt-rt5640.c
index ae89b9b966d9..275b056c495c 100644
--- a/sound/soc/intel/boards/byt-rt5640.c
+++ b/sound/soc/intel/boards/byt-rt5640.c
@@ -186,7 +186,7 @@ static struct snd_soc_dai_link byt_rt5640_dais[] = {
 		.stream_name = "Audio",
 		.cpu_dai_name = "baytrail-pcm-audio",
 		.codec_dai_name = "rt5640-aif1",
-		.codec_name = "i2c-10EC5640:00",
+		.codec_name = "10EC5640:00",
 		.platform_name = "baytrail-pcm-audio",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			   SND_SOC_DAIFMT_CBS_CFS,
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 7f55d59024a8..973f07548b08 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -171,7 +171,7 @@ static struct snd_soc_dai_link byt_dailink[] = {
 		.platform_name = "sst-mfld-platform",
 		.no_pcm = 1,
 		.codec_dai_name = "rt5640-aif1",
-		.codec_name = "i2c-10EC5640:00",
+		.codec_name = "10EC5640:00",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
 						| SND_SOC_DAIFMT_CBS_CFS,
 		.be_hw_params_fixup = byt_codec_fixup,
diff --git a/sound/soc/intel/boards/cht_bsw_max98090_ti.c b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
index 70f832114a5a..1a5348771e03 100644
--- a/sound/soc/intel/boards/cht_bsw_max98090_ti.c
+++ b/sound/soc/intel/boards/cht_bsw_max98090_ti.c
@@ -232,7 +232,7 @@ static struct snd_soc_ops cht_be_ssp2_ops = {
 static struct snd_soc_aux_dev cht_max98090_headset_dev = {
 	.name = "Headset Chip",
 	.init = cht_max98090_headset_init,
-	.codec_name = "i2c-104C227E:00",
+	.codec_name = "104C227E:00",
 };
 
 static struct snd_soc_dai_link cht_dailink[] = {
@@ -265,7 +265,7 @@ static struct snd_soc_dai_link cht_dailink[] = {
 		.platform_name = "sst-mfld-platform",
 		.no_pcm = 1,
 		.codec_dai_name = "HiFi",
-		.codec_name = "i2c-193C9890:00",
+		.codec_name = "193C9890:00",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF
 					| SND_SOC_DAIFMT_CBS_CFS,
 		.init = cht_codec_init,
diff --git a/sound/soc/intel/boards/cht_bsw_rt5645.c b/sound/soc/intel/boards/cht_bsw_rt5645.c
index bdcaf467842a..c05841264dc7 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5645.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5645.c
@@ -290,7 +290,7 @@ static struct snd_soc_dai_link cht_dailink[] = {
 		.platform_name = "sst-mfld-platform",
 		.no_pcm = 1,
 		.codec_dai_name = "rt5645-aif1",
-		.codec_name = "i2c-10EC5645:00",
+		.codec_name = "10EC5645:00",
 		.dai_fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF
 					| SND_SOC_DAIFMT_CBS_CFS,
 		.init = cht_codec_init,
@@ -365,7 +365,7 @@ static int snd_cht_mc_probe(struct platform_device *pdev)
 		}
 	}
 	card->dev = &pdev->dev;
-	sprintf(codec_name, "i2c-%s:00", drv->acpi_card->codec_id);
+	sprintf(codec_name, "%s:00", drv->acpi_card->codec_id);
 	/* set correct codec name */
 	strcpy((char *)card->dai_link[2].codec_name, codec_name);
 	snd_soc_card_set_drvdata(card, drv);
diff --git a/sound/soc/intel/boards/cht_bsw_rt5672.c b/sound/soc/intel/boards/cht_bsw_rt5672.c
index 2c9cc5be439e..632926d0c595 100644
--- a/sound/soc/intel/boards/cht_bsw_rt5672.c
+++ b/sound/soc/intel/boards/cht_bsw_rt5672.c
@@ -280,7 +280,7 @@ static struct snd_soc_dai_link cht_dailink[] = {
 		.no_pcm = 1,
 		.nonatomic = true,
 		.codec_dai_name = "rt5670-aif1",
-		.codec_name = "i2c-10EC5670:00",
+		.codec_name = "10EC5670:00",
 		.dai_fmt = SND_SOC_DAIFMT_DSP_B | SND_SOC_DAIFMT_IB_NF
 					| SND_SOC_DAIFMT_CBS_CFS,
 		.init = cht_codec_init,
@@ -296,7 +296,7 @@ static int cht_suspend_pre(struct snd_soc_card *card)
 	struct snd_soc_codec *codec;
 
 	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
-		if (!strcmp(codec->component.name, "i2c-10EC5670:00")) {
+		if (!strcmp(codec->component.name, "10EC5670:00")) {
 			dev_dbg(codec->dev, "disabling jack detect before going to suspend.\n");
 			rt5670_jack_suspend(codec);
 			break;
@@ -310,7 +310,7 @@ static int cht_resume_post(struct snd_soc_card *card)
 	struct snd_soc_codec *codec;
 
 	list_for_each_entry(codec, &card->codec_dev_list, card_list) {
-		if (!strcmp(codec->component.name, "i2c-10EC5670:00")) {
+		if (!strcmp(codec->component.name, "10EC5670:00")) {
 			dev_dbg(codec->dev, "enabling jack detect for resume.\n");
 			rt5670_jack_resume(codec);
 			break;
diff --git a/sound/soc/intel/boards/haswell.c b/sound/soc/intel/boards/haswell.c
index 22558572cb9c..6fd6491e368d 100644
--- a/sound/soc/intel/boards/haswell.c
+++ b/sound/soc/intel/boards/haswell.c
@@ -160,7 +160,7 @@ static struct snd_soc_dai_link haswell_rt5640_dais[] = {
 		.cpu_dai_name = "snd-soc-dummy-dai",
 		.platform_name = "snd-soc-dummy",
 		.no_pcm = 1,
-		.codec_name = "i2c-INT33CA:00",
+		.codec_name = "INT33CA:00",
 		.codec_dai_name = "rt5640-aif1",
 		.dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
 			SND_SOC_DAIFMT_CBS_CFS,
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 0e1e69c7abd5..811d63e31a7a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -34,6 +34,7 @@
 #include <linux/ctype.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/acpi.h>
 #include <sound/core.h>
 #include <sound/jack.h>
 #include <sound/pcm.h>
@@ -2476,10 +2477,18 @@ static char *fmt_single_name(struct device *dev, int *id)
 {
 	char *found, name[NAME_SIZE];
 	int id1, id2;
+	struct acpi_device *adev;
 
 	if (dev_name(dev) = NULL)
 		return NULL;
 
+	if (ACPI_HANDLE(dev) &&
+	    !acpi_bus_get_device(ACPI_HANDLE(dev), &adev)) {
+		/* use ACPI name as a component name for ACPI probed devices */
+		strlcpy(name, acpi_dev_name(adev), NAME_SIZE);
+		goto out;
+	}
+
 	strlcpy(name, dev_name(dev), NAME_SIZE);
 
 	/* are we a "%s.%d" name (platform and SPI components) */
@@ -2508,6 +2517,7 @@ static char *fmt_single_name(struct device *dev, int *id)
 			*id = 0;
 	}
 
+out:
 	return kstrdup(name, GFP_KERNEL);
 }
 
-- 
2.5.0


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
       [not found] ` <1440413522-7855-1-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2015-08-24 13:26     ` Wolfram Sang
  2015-08-25  5:03     ` [lm-sensors] " Dustin Byford
  2015-08-25  5:25     ` [lm-sensors] " Mark Brown
  2 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-24 13:26 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Mark Brown, Rafael J. Wysocki,
	Jean Delvare, Liam Girdwood, Dustin Byford,
	linux-0h96xk9xTtrk1uMJSBkQmQ

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

On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> code in lm-sensors does not find them anymore:
> 
> lib/sysfs.c:665:
> if ((!subsys || !strcmp(subsys, "i2c")) &&
>     sscanf(dev_name, "%hd-%x", &entry.chip.bus.nr,
> 	   &entry.chip.addr) == 2) {
> 
> This patch fixes this by reverting back the old device naming in i2c-core
> but at the same avoids regression to ALSA SoC drivers that depend on stable
> device binding.

This way we fix some userspace by going back. However, this name change
was effective for 18 months, so enough time for some other userspace to
adapt. We would break the latter by reverting.

I tend to create the symlinks to prevent any further breakage. I am open
for other suggestions, though. This ugly situation has surely happened
before somewhere somewhen in the kernel :/

   Wolfram

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-24 13:26     ` Wolfram Sang
  0 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-24 13:26 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Mark Brown, Rafael J. Wysocki,
	Jean Delvare, Liam Girdwood, Dustin Byford,
	linux-0h96xk9xTtrk1uMJSBkQmQ


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

On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> code in lm-sensors does not find them anymore:
> 
> lib/sysfs.c:665:
> if ((!subsys || !strcmp(subsys, "i2c")) &&
>     sscanf(dev_name, "%hd-%x", &entry.chip.bus.nr,
> 	   &entry.chip.addr) == 2) {
> 
> This patch fixes this by reverting back the old device naming in i2c-core
> but at the same avoids regression to ALSA SoC drivers that depend on stable
> device binding.

This way we fix some userspace by going back. However, this name change
was effective for 18 months, so enough time for some other userspace to
adapt. We would break the latter by reverting.

I tend to create the symlinks to prevent any further breakage. I am open
for other suggestions, though. This ugly situation has surely happened
before somewhere somewhen in the kernel :/

   Wolfram

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-08-24 13:26     ` [lm-sensors] " Wolfram Sang
@ 2015-08-25  0:19       ` Rafael J. Wysocki
  -1 siblings, 0 replies; 30+ messages in thread
From: Rafael J. Wysocki @ 2015-08-25  0:19 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi, lm-sensors,
	Mark Brown, Jean Delvare, Liam Girdwood, Dustin Byford, linux

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

On Monday, August 24, 2015 03:26:13 PM Wolfram Sang wrote:
> On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> > Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> > slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> > /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> > devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> > code in lm-sensors does not find them anymore:
> > 
> > lib/sysfs.c:665:
> > if ((!subsys || !strcmp(subsys, "i2c")) &&
> >     sscanf(dev_name, "%hd-%x", &entry.chip.bus.nr,
> > 	   &entry.chip.addr) == 2) {
> > 
> > This patch fixes this by reverting back the old device naming in i2c-core
> > but at the same avoids regression to ALSA SoC drivers that depend on stable
> > device binding.
> 
> This way we fix some userspace by going back. However, this name change
> was effective for 18 months, so enough time for some other userspace to
> adapt. We would break the latter by reverting.

Right.

Generally, reverts of such things need to be made relatively quickly.

> I tend to create the symlinks to prevent any further breakage. I am open
> for other suggestions, though. This ugly situation has surely happened
> before somewhere somewhen in the kernel :/

It happens on a regular basis.

In this particular case it looks like we need a mechanism to use the old
naming scheme for hwmon and the new one for everything else or something
similar.  I'm afraid I am not familiar enough with the i2c core to suggest
anything specific ATM, though.

Thanks,
Rafael

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25  0:19       ` Rafael J. Wysocki
  0 siblings, 0 replies; 30+ messages in thread
From: Rafael J. Wysocki @ 2015-08-25  0:19 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi, lm-sensors,
	Mark Brown, Jean Delvare, Liam Girdwood, Dustin Byford, linux


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

On Monday, August 24, 2015 03:26:13 PM Wolfram Sang wrote:
> On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> > Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> > slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> > /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> > devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> > code in lm-sensors does not find them anymore:
> > 
> > lib/sysfs.c:665:
> > if ((!subsys || !strcmp(subsys, "i2c")) &&
> >     sscanf(dev_name, "%hd-%x", &entry.chip.bus.nr,
> > 	   &entry.chip.addr) == 2) {
> > 
> > This patch fixes this by reverting back the old device naming in i2c-core
> > but at the same avoids regression to ALSA SoC drivers that depend on stable
> > device binding.
> 
> This way we fix some userspace by going back. However, this name change
> was effective for 18 months, so enough time for some other userspace to
> adapt. We would break the latter by reverting.

Right.

Generally, reverts of such things need to be made relatively quickly.

> I tend to create the symlinks to prevent any further breakage. I am open
> for other suggestions, though. This ugly situation has surely happened
> before somewhere somewhen in the kernel :/

It happens on a regular basis.

In this particular case it looks like we need a mechanism to use the old
naming scheme for hwmon and the new one for everything else or something
similar.  I'm afraid I am not familiar enough with the i2c core to suggest
anything specific ATM, though.

Thanks,
Rafael

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
       [not found] ` <1440413522-7855-1-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2015-08-25  5:03     ` Dustin Byford
  2015-08-25  5:03     ` [lm-sensors] " Dustin Byford
  2015-08-25  5:25     ` [lm-sensors] " Mark Brown
  2 siblings, 0 replies; 30+ messages in thread
From: Dustin Byford @ 2015-08-25  5:03 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Wolfram Sang, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood,
	linux-0h96xk9xTtrk1uMJSBkQmQ

On Mon Aug 24 13:52, Jarkko Nikula wrote:

> I don't know how common ACPI enumerated I2C hwmon devices are but I guess
> they exists since Dustin notices this issue and wrote "With that change,
> /sys/bus/i2c/devices/i2c-0-004a, for example, became
> /sys/bus/i2c/devices/i2c-PNPXXXX:xx".

I wouldn't take my particular use case as evidence they are common, or
even they really exist.  I got here because I loaded a hwmon driver
using the _DSD/PRP0001 mechanism in ACPI.  In that case, those devices
have never worked with 'sensors'  Therefore, in that context, this
change is just fixing a bug.

I can't say much about other apps writing against the current sysfs
names.  18 months seems like a long time.

		--Dustin

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25  5:03     ` Dustin Byford
  0 siblings, 0 replies; 30+ messages in thread
From: Dustin Byford @ 2015-08-25  5:03 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Wolfram Sang, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood,
	linux-0h96xk9xTtrk1uMJSBkQmQ

On Mon Aug 24 13:52, Jarkko Nikula wrote:

> I don't know how common ACPI enumerated I2C hwmon devices are but I guess
> they exists since Dustin notices this issue and wrote "With that change,
> /sys/bus/i2c/devices/i2c-0-004a, for example, became
> /sys/bus/i2c/devices/i2c-PNPXXXX:xx".

I wouldn't take my particular use case as evidence they are common, or
even they really exist.  I got here because I loaded a hwmon driver
using the _DSD/PRP0001 mechanism in ACPI.  In that case, those devices
have never worked with 'sensors'  Therefore, in that context, this
change is just fixing a bug.

I can't say much about other apps writing against the current sysfs
names.  18 months seems like a long time.

		--Dustin

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
       [not found] ` <1440413522-7855-1-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
@ 2015-08-25  5:25     ` Mark Brown
  2015-08-25  5:03     ` [lm-sensors] " Dustin Byford
  2015-08-25  5:25     ` [lm-sensors] " Mark Brown
  2 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2015-08-25  5:25 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Wolfram Sang,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux-0h96xk9xTtrk1uMJSBkQmQ

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

On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> code in lm-sensors does not find them anymore:

Acked-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25  5:25     ` Mark Brown
  0 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2015-08-25  5:25 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Wolfram Sang,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux-0h96xk9xTtrk1uMJSBkQmQ


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

On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> code in lm-sensors does not find them anymore:

Acked-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
       [not found]     ` <20150825050306.GB21569-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
@ 2015-08-25 14:50         ` Jarkko Nikula
  0 siblings, 0 replies; 30+ messages in thread
From: Jarkko Nikula @ 2015-08-25 14:50 UTC (permalink / raw)
  To: Dustin Byford
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Wolfram Sang, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood,
	linux-0h96xk9xTtrk1uMJSBkQmQ

On 08/25/2015 08:03 AM, Dustin Byford wrote:
> On Mon Aug 24 13:52, Jarkko Nikula wrote:
>
>> I don't know how common ACPI enumerated I2C hwmon devices are but I guess
>> they exists since Dustin notices this issue and wrote "With that change,
>> /sys/bus/i2c/devices/i2c-0-004a, for example, became
>> /sys/bus/i2c/devices/i2c-PNPXXXX:xx".
>
> I wouldn't take my particular use case as evidence they are common, or
> even they really exist.  I got here because I loaded a hwmon driver
> using the _DSD/PRP0001 mechanism in ACPI.  In that case, those devices
> have never worked with 'sensors'  Therefore, in that context, this
> change is just fixing a bug.
>
So does this then fall more into new use case rather than being a 
regression as such?

What I'm trying to understand what kind of regression we actually do 
have here? Is it about listing the devices or does it also cease to show 
for instance temperature sensor readings?

If it requires to add a match table to existing driver in order to 
trigger this then at least stable tree is not worry here.

-- 
Jarkko

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25 14:50         ` Jarkko Nikula
  0 siblings, 0 replies; 30+ messages in thread
From: Jarkko Nikula @ 2015-08-25 14:50 UTC (permalink / raw)
  To: Dustin Byford
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Wolfram Sang, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood,
	linux-0h96xk9xTtrk1uMJSBkQmQ

On 08/25/2015 08:03 AM, Dustin Byford wrote:
> On Mon Aug 24 13:52, Jarkko Nikula wrote:
>
>> I don't know how common ACPI enumerated I2C hwmon devices are but I guess
>> they exists since Dustin notices this issue and wrote "With that change,
>> /sys/bus/i2c/devices/i2c-0-004a, for example, became
>> /sys/bus/i2c/devices/i2c-PNPXXXX:xx".
>
> I wouldn't take my particular use case as evidence they are common, or
> even they really exist.  I got here because I loaded a hwmon driver
> using the _DSD/PRP0001 mechanism in ACPI.  In that case, those devices
> have never worked with 'sensors'  Therefore, in that context, this
> change is just fixing a bug.
>
So does this then fall more into new use case rather than being a 
regression as such?

What I'm trying to understand what kind of regression we actually do 
have here? Is it about listing the devices or does it also cease to show 
for instance temperature sensor readings?

If it requires to add a match table to existing driver in order to 
trigger this then at least stable tree is not worry here.

-- 
Jarkko

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-08-25  5:25     ` [lm-sensors] " Mark Brown
@ 2015-08-25 14:57       ` Wolfram Sang
  -1 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-25 14:57 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi, lm-sensors,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux

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

On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
> On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> > Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> > slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> > /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> > devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> > code in lm-sensors does not find them anymore:
> 
> Acked-by: Mark Brown <broonie@kernel.org>

Don't you think there will be regressions given that the new naming
scheme was around for 18 months?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25 14:57       ` Wolfram Sang
  0 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-25 14:57 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi, lm-sensors,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux


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

On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
> On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> > Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> > slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> > /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> > devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> > code in lm-sensors does not find them anymore:
> 
> Acked-by: Mark Brown <broonie@kernel.org>

Don't you think there will be regressions given that the new naming
scheme was around for 18 months?


[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-08-25  0:19       ` [lm-sensors] " Rafael J. Wysocki
@ 2015-08-25 14:59         ` Wolfram Sang
  -1 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-25 14:59 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi, lm-sensors,
	Mark Brown, Jean Delvare, Liam Girdwood, Dustin Byford, linux

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


> In this particular case it looks like we need a mechanism to use the old
> naming scheme for hwmon and the new one for everything else or something
> similar.  I'm afraid I am not familiar enough with the i2c core to suggest
> anything specific ATM, though.

Well, it boils down to directory names in sysfs. So, to support both
naming schemes, I'd simply suggest symlinks.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25 14:59         ` Wolfram Sang
  0 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-25 14:59 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi, lm-sensors,
	Mark Brown, Jean Delvare, Liam Girdwood, Dustin Byford, linux


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


> In this particular case it looks like we need a mechanism to use the old
> naming scheme for hwmon and the new one for everything else or something
> similar.  I'm afraid I am not familiar enough with the i2c core to suggest
> anything specific ATM, though.

Well, it boils down to directory names in sysfs. So, to support both
naming schemes, I'd simply suggest symlinks.


[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
       [not found]       ` <20150825145756.GA4066-oo5tB6JMkjKRinMKxDlMNPwbnWRJjS81@public.gmane.org>
@ 2015-08-25 15:18           ` Guenter Roeck
  0 siblings, 0 replies; 30+ messages in thread
From: Guenter Roeck @ 2015-08-25 15:18 UTC (permalink / raw)
  To: Wolfram Sang, Mark Brown
  Cc: Jarkko Nikula, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Rafael J. Wysocki,
	Jean Delvare, Liam Girdwood, Dustin Byford

On 08/25/2015 07:57 AM, Wolfram Sang wrote:
> On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
>> On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
>>> Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
>>> slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
>>> /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
>>> devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
>>> code in lm-sensors does not find them anymore:
>>
>> Acked-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>
> Don't you think there will be regressions given that the new naming
> scheme was around for 18 months?
>

acpi is pretty long term. New bindings don't show up quickly.
So I am not surprised that this only shows up now.

Will there be regressions ? Who knows. What we do know is
that there are regressions today due to the original change.

Guenter

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25 15:18           ` Guenter Roeck
  0 siblings, 0 replies; 30+ messages in thread
From: Guenter Roeck @ 2015-08-25 15:18 UTC (permalink / raw)
  To: Wolfram Sang, Mark Brown
  Cc: Jarkko Nikula, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Rafael J. Wysocki,
	Jean Delvare, Liam Girdwood, Dustin Byford

On 08/25/2015 07:57 AM, Wolfram Sang wrote:
> On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
>> On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
>>> Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
>>> slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
>>> /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
>>> devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
>>> code in lm-sensors does not find them anymore:
>>
>> Acked-by: Mark Brown <broonie@kernel.org>
>
> Don't you think there will be regressions given that the new naming
> scheme was around for 18 months?
>

acpi is pretty long term. New bindings don't show up quickly.
So I am not surprised that this only shows up now.

Will there be regressions ? Who knows. What we do know is
that there are regressions today due to the original change.

Guenter



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-08-25 14:57       ` [lm-sensors] " Wolfram Sang
@ 2015-08-25 16:14         ` Mark Brown
  -1 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2015-08-25 16:14 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: alsa-devel, Jean Delvare, Rafael J. Wysocki, Liam Girdwood,
	lm-sensors, linux-acpi, Jarkko Nikula, linux-i2c, linux,
	Dustin Byford


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

On Tue, Aug 25, 2015 at 04:57:56PM +0200, Wolfram Sang wrote:
> On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
> > On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:

> > > Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> > > slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> > > /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> > > devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> > > code in lm-sensors does not find them anymore:

> > Acked-by: Mark Brown <broonie@kernel.org>

> Don't you think there will be regressions given that the new naming
> scheme was around for 18 months?

That's a "whatever" from the ASoC point of view.  I don't particularly
care about the userspace ABI, on the one hand there aren't that many
ACPI 5 devices that might be affected and there's other devices that
won't work anyway.  On the other hand I guess there's other devices that
would have worked with the old kernel.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25 16:14         ` Mark Brown
  0 siblings, 0 replies; 30+ messages in thread
From: Mark Brown @ 2015-08-25 16:14 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: alsa-devel, Jean Delvare, Rafael J. Wysocki, Liam Girdwood,
	lm-sensors, linux-acpi, Jarkko Nikula, linux-i2c, linux,
	Dustin Byford


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

On Tue, Aug 25, 2015 at 04:57:56PM +0200, Wolfram Sang wrote:
> On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
> > On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:

> > > Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> > > slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> > > /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> > > devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> > > code in lm-sensors does not find them anymore:

> > Acked-by: Mark Brown <broonie@kernel.org>

> Don't you think there will be regressions given that the new naming
> scheme was around for 18 months?

That's a "whatever" from the ASoC point of view.  I don't particularly
care about the userspace ABI, on the one hand there aren't that many
ACPI 5 devices that might be affected and there's other devices that
won't work anyway.  On the other hand I guess there's other devices that
would have worked with the old kernel.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
       [not found]           ` <55DC8746.1060809-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
@ 2015-08-25 16:18               ` Wolfram Sang
  0 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-25 16:18 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Brown, Jarkko Nikula, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Rafael J. Wysocki,
	Jean Delvare, Liam Girdwood, Dustin Byford

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

On Tue, Aug 25, 2015 at 08:18:30AM -0700, Guenter Roeck wrote:
> On 08/25/2015 07:57 AM, Wolfram Sang wrote:
> >On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
> >>On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> >>>Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> >>>slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> >>>/sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> >>>devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> >>>code in lm-sensors does not find them anymore:
> >>
> >>Acked-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >
> >Don't you think there will be regressions given that the new naming
> >scheme was around for 18 months?
> >
> 
> acpi is pretty long term. New bindings don't show up quickly.

The key is, there might be userspace applications that got used to our
new "export" of ACPI bindings. The bindings were already existing, our
"export" did change.

> So I am not surprised that this only shows up now.

I am, to be honest. It shows running lm-sensors with ACPI on a kernel
newer than 18 months. Not a rare scenario, so I thought.

> Will there be regressions ? Who knows. What we do know is
> that there are regressions today due to the original change.

Because we didn't pay enough attention. I wouldn't like to do the same
mistake again. Or?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25 16:18               ` Wolfram Sang
  0 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-25 16:18 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Brown, Jarkko Nikula, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	lm-sensors-GZX6beZjE8VD60Wz+7aTrA, Rafael J. Wysocki,
	Jean Delvare, Liam Girdwood, Dustin Byford


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

On Tue, Aug 25, 2015 at 08:18:30AM -0700, Guenter Roeck wrote:
> On 08/25/2015 07:57 AM, Wolfram Sang wrote:
> >On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
> >>On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
> >>>Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
> >>>slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
> >>>/sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
> >>>devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
> >>>code in lm-sensors does not find them anymore:
> >>
> >>Acked-by: Mark Brown <broonie@kernel.org>
> >
> >Don't you think there will be regressions given that the new naming
> >scheme was around for 18 months?
> >
> 
> acpi is pretty long term. New bindings don't show up quickly.

The key is, there might be userspace applications that got used to our
new "export" of ACPI bindings. The bindings were already existing, our
"export" did change.

> So I am not surprised that this only shows up now.

I am, to be honest. It shows running lm-sensors with ACPI on a kernel
newer than 18 months. Not a rare scenario, so I thought.

> Will there be regressions ? Who knows. What we do know is
> that there are regressions today due to the original change.

Because we didn't pay enough attention. I wouldn't like to do the same
mistake again. Or?


[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-08-25 16:18               ` [lm-sensors] " Wolfram Sang
@ 2015-08-25 16:22                 ` Wolfram Sang
  -1 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-25 16:22 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Brown, Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi,
	lm-sensors, Rafael J. Wysocki, Jean Delvare, Liam Girdwood,
	Dustin Byford

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


> > So I am not surprised that this only shows up now.
> 
> I am, to be honest. It shows running lm-sensors with ACPI on a kernel
> newer than 18 months. Not a rare scenario, so I thought.

And thanks to Mark's recent post I understand now that this is only
relevant for ACPI5 enumerated devices. I lost that detail.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25 16:22                 ` Wolfram Sang
  0 siblings, 0 replies; 30+ messages in thread
From: Wolfram Sang @ 2015-08-25 16:22 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Mark Brown, Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi,
	lm-sensors, Rafael J. Wysocki, Jean Delvare, Liam Girdwood,
	Dustin Byford


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


> > So I am not surprised that this only shows up now.
> 
> I am, to be honest. It shows running lm-sensors with ACPI on a kernel
> newer than 18 months. Not a rare scenario, so I thought.

And thanks to Mark's recent post I understand now that this is only
relevant for ACPI5 enumerated devices. I lost that detail.


[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-08-25 16:18               ` [lm-sensors] " Wolfram Sang
@ 2015-08-25 17:12                 ` Guenter Roeck
  -1 siblings, 0 replies; 30+ messages in thread
From: Guenter Roeck @ 2015-08-25 17:12 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Mark Brown, Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi,
	lm-sensors, Rafael J. Wysocki, Jean Delvare, Liam Girdwood,
	Dustin Byford

On 08/25/2015 09:18 AM, Wolfram Sang wrote:
> On Tue, Aug 25, 2015 at 08:18:30AM -0700, Guenter Roeck wrote:
>> On 08/25/2015 07:57 AM, Wolfram Sang wrote:
>>> On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
>>>> On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
>>>>> Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
>>>>> slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
>>>>> /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
>>>>> devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
>>>>> code in lm-sensors does not find them anymore:
>>>>
>>>> Acked-by: Mark Brown <broonie@kernel.org>
>>>
>>> Don't you think there will be regressions given that the new naming
>>> scheme was around for 18 months?
>>>
>>
>> acpi is pretty long term. New bindings don't show up quickly.
>
> The key is, there might be userspace applications that got used to our
> new "export" of ACPI bindings. The bindings were already existing, our
> "export" did change.
>
>> So I am not surprised that this only shows up now.
>
> I am, to be honest. It shows running lm-sensors with ACPI on a kernel
> newer than 18 months. Not a rare scenario, so I thought.
>
With i2c (sensor) devices enumerated in acpi ? Yes, I think that is quite rare.
This is not about the kernel, it is about such device bindings showing up
in acpi data.

Guenter


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [lm-sensors] [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
@ 2015-08-25 17:12                 ` Guenter Roeck
  0 siblings, 0 replies; 30+ messages in thread
From: Guenter Roeck @ 2015-08-25 17:12 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Mark Brown, Jarkko Nikula, linux-i2c, alsa-devel, linux-acpi,
	lm-sensors, Rafael J. Wysocki, Jean Delvare, Liam Girdwood,
	Dustin Byford

On 08/25/2015 09:18 AM, Wolfram Sang wrote:
> On Tue, Aug 25, 2015 at 08:18:30AM -0700, Guenter Roeck wrote:
>> On 08/25/2015 07:57 AM, Wolfram Sang wrote:
>>> On Tue, Aug 25, 2015 at 06:25:13AM +0100, Mark Brown wrote:
>>>> On Mon, Aug 24, 2015 at 01:52:02PM +0300, Jarkko Nikula wrote:
>>>>> Commit 70762abb9f89 ("i2c: Use stable dev_name for ACPI enumerated I2C
>>>>> slaves") broke the lm-sensors which relies on I2C hwmon slave devices under
>>>>> /sys/bus/i2c/devices/ to be named as "x-00yz". However if those hwmon
>>>>> devices are ACPI 5 enumerated their name became "i2c-INTABCD:ij" and sysfs
>>>>> code in lm-sensors does not find them anymore:
>>>>
>>>> Acked-by: Mark Brown <broonie@kernel.org>
>>>
>>> Don't you think there will be regressions given that the new naming
>>> scheme was around for 18 months?
>>>
>>
>> acpi is pretty long term. New bindings don't show up quickly.
>
> The key is, there might be userspace applications that got used to our
> new "export" of ACPI bindings. The bindings were already existing, our
> "export" did change.
>
>> So I am not surprised that this only shows up now.
>
> I am, to be honest. It shows running lm-sensors with ACPI on a kernel
> newer than 18 months. Not a rare scenario, so I thought.
>
With i2c (sensor) devices enumerated in acpi ? Yes, I think that is quite rare.
This is not about the kernel, it is about such device bindings showing up
in acpi data.

Guenter


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-08-24 10:52 ` [lm-sensors] " Jarkko Nikula
  (?)
  (?)
@ 2015-10-01 20:37 ` Wolfram Sang
  2015-10-02  9:27   ` Jarkko Nikula
  -1 siblings, 1 reply; 30+ messages in thread
From: Wolfram Sang @ 2015-10-01 20:37 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: linux-i2c, alsa-devel, linux-acpi, lm-sensors, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux

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


> This is for discussion so I didn't cc stable@vger.kernel.org yet. I was
> thinking would it work if we'd keep the stable name but have an another
> symlink in /sys/bus/i2c/devices/ that uses "x-00yz" name. However this
> feels ill-use of devices directory and probably causes more troubles
> elsewhere.

Do you foresee troubles already? I am still in favour of a symlink.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-10-01 20:37 ` Wolfram Sang
@ 2015-10-02  9:27   ` Jarkko Nikula
  2015-10-09 21:47     ` Wolfram Sang
  0 siblings, 1 reply; 30+ messages in thread
From: Jarkko Nikula @ 2015-10-02  9:27 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, alsa-devel, linux-acpi, lm-sensors, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux

On 10/01/2015 11:37 PM, Wolfram Sang wrote:
>
>> This is for discussion so I didn't cc stable@vger.kernel.org yet. I was
>> thinking would it work if we'd keep the stable name but have an another
>> symlink in /sys/bus/i2c/devices/ that uses "x-00yz" name. However this
>> feels ill-use of devices directory and probably causes more troubles
>> elsewhere.
>
> Do you foresee troubles already? I am still in favour of a symlink.
>
I haven't looked at this for a while but one problem was that devices/ 
directory belongs to private structure of struct bus_type and in order 
to create a symlink there it needs to done in drivers/base/bus.c: 
bus_add_device() which felt quite hackish to me.

-- 
Jarkko

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-10-02  9:27   ` Jarkko Nikula
@ 2015-10-09 21:47     ` Wolfram Sang
  2015-10-12  8:32       ` Jarkko Nikula
  0 siblings, 1 reply; 30+ messages in thread
From: Wolfram Sang @ 2015-10-09 21:47 UTC (permalink / raw)
  To: Jarkko Nikula
  Cc: linux-i2c, alsa-devel, linux-acpi, lm-sensors, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux

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

On Fri, Oct 02, 2015 at 12:27:16PM +0300, Jarkko Nikula wrote:
> On 10/01/2015 11:37 PM, Wolfram Sang wrote:
> >
> >>This is for discussion so I didn't cc stable@vger.kernel.org yet. I was
> >>thinking would it work if we'd keep the stable name but have an another
> >>symlink in /sys/bus/i2c/devices/ that uses "x-00yz" name. However this
> >>feels ill-use of devices directory and probably causes more troubles
> >>elsewhere.
> >
> >Do you foresee troubles already? I am still in favour of a symlink.
> >
> I haven't looked at this for a while but one problem was that devices/
> directory belongs to private structure of struct bus_type and in order to
> create a symlink there it needs to done in drivers/base/bus.c:
> bus_add_device() which felt quite hackish to me.

This is just a quick prototype and untested; but I did something similar
in the i2c-mux code:

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 5f89f1e3c2f24f..715dca57ba68fd 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -970,13 +970,15 @@ static void i2c_dev_set_name(struct i2c_adapter *adap,
 {
 	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
 
-	if (adev) {
-		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
-		return;
-	}
-
 	dev_set_name(&client->dev, "%d-%04x", i2c_adapter_id(adap),
 		     i2c_encode_flags_to_addr(client));
+
+	if (adev) {
+		char symlink_name[256];
+
+		snprintf(symlink_name, sizeof(symlink_name), "i2c-%s", acpi_dev_name(adev));
+		sysfs_create_link(&client->dev.kobj, &adap->dev.kobj, symlink_name);
+	}
 }
 
 /**

Shouldn't something like this be enough?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves
  2015-10-09 21:47     ` Wolfram Sang
@ 2015-10-12  8:32       ` Jarkko Nikula
  0 siblings, 0 replies; 30+ messages in thread
From: Jarkko Nikula @ 2015-10-12  8:32 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, alsa-devel, linux-acpi, lm-sensors, Mark Brown,
	Rafael J. Wysocki, Jean Delvare, Liam Girdwood, Dustin Byford,
	linux

Hi
On 10/10/2015 12:47 AM, Wolfram Sang wrote:
> On Fri, Oct 02, 2015 at 12:27:16PM +0300, Jarkko Nikula wrote:
>> On 10/01/2015 11:37 PM, Wolfram Sang wrote:
>>> Do you foresee troubles already? I am still in favour of a symlink.
>>>
>> I haven't looked at this for a while but one problem was that devices/
>> directory belongs to private structure of struct bus_type and in order to
>> create a symlink there it needs to done in drivers/base/bus.c:
>> bus_add_device() which felt quite hackish to me.
>
> This is just a quick prototype and untested; but I did something similar
> in the i2c-mux code:
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 5f89f1e3c2f24f..715dca57ba68fd 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -970,13 +970,15 @@ static void i2c_dev_set_name(struct i2c_adapter *adap,
>   {
>   	struct acpi_device *adev = ACPI_COMPANION(&client->dev);
>
> -	if (adev) {
> -		dev_set_name(&client->dev, "i2c-%s", acpi_dev_name(adev));
> -		return;
> -	}
> -
>   	dev_set_name(&client->dev, "%d-%04x", i2c_adapter_id(adap),
>   		     i2c_encode_flags_to_addr(client));
> +
> +	if (adev) {
> +		char symlink_name[256];
> +
> +		snprintf(symlink_name, sizeof(symlink_name), "i2c-%s", acpi_dev_name(adev));
> +		sysfs_create_link(&client->dev.kobj, &adap->dev.kobj, symlink_name);
> +	}
>   }
>
>   /**
>
> Shouldn't something like this be enough?
>
Not really. It would create the symlink under the device not under 
/sys/bus/i2c/devices/. Please note the sysfs_create_link() must be 
called after device_register(). I moved the symlink creation into 
i2c_new_device() for the example below.

Now:

/sys/bus/i2c/devices/i2c-ATML3432:00 -> 
../../../devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-9/i2c-ATML3432:00

After modified patch:

/sys/bus/i2c/devices/9-004c -> 
../../../devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-9/9-004c

which has now the symlink pointing to adapter

/sys/devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-9/9-004c/
i2c-ATML3432:00 -> ../../i2c-9

We would need a following symlink but I didn't figure out how to do it 
without touching drivers/base/bus.c.

/sys/bus/i2c/devices/i2c-ATML3432:00 -> 9-004c

Symlinks there are added/removed by using &bus->p->devices_kset->kobj 
where p is a private for driver core only.

-- 
Jarkko

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2015-10-12  8:32 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-24 10:52 [RFC] i2c: Revert back to old device naming for ACPI enumerated I2C slaves Jarkko Nikula
2015-08-24 10:52 ` [lm-sensors] " Jarkko Nikula
     [not found] ` <1440413522-7855-1-git-send-email-jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2015-08-24 13:26   ` Wolfram Sang
2015-08-24 13:26     ` [lm-sensors] " Wolfram Sang
2015-08-25  0:19     ` Rafael J. Wysocki
2015-08-25  0:19       ` [lm-sensors] " Rafael J. Wysocki
2015-08-25 14:59       ` Wolfram Sang
2015-08-25 14:59         ` [lm-sensors] " Wolfram Sang
2015-08-25  5:03   ` Dustin Byford
2015-08-25  5:03     ` [lm-sensors] " Dustin Byford
     [not found]     ` <20150825050306.GB21569-qUQiAmfTcIp+XZJcv9eMoEEOCMrvLtNR@public.gmane.org>
2015-08-25 14:50       ` Jarkko Nikula
2015-08-25 14:50         ` [lm-sensors] " Jarkko Nikula
2015-08-25  5:25   ` Mark Brown
2015-08-25  5:25     ` [lm-sensors] " Mark Brown
2015-08-25 14:57     ` Wolfram Sang
2015-08-25 14:57       ` [lm-sensors] " Wolfram Sang
     [not found]       ` <20150825145756.GA4066-oo5tB6JMkjKRinMKxDlMNPwbnWRJjS81@public.gmane.org>
2015-08-25 15:18         ` Guenter Roeck
2015-08-25 15:18           ` [lm-sensors] " Guenter Roeck
     [not found]           ` <55DC8746.1060809-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2015-08-25 16:18             ` Wolfram Sang
2015-08-25 16:18               ` [lm-sensors] " Wolfram Sang
2015-08-25 16:22               ` Wolfram Sang
2015-08-25 16:22                 ` [lm-sensors] " Wolfram Sang
2015-08-25 17:12               ` Guenter Roeck
2015-08-25 17:12                 ` [lm-sensors] " Guenter Roeck
2015-08-25 16:14       ` Mark Brown
2015-08-25 16:14         ` [lm-sensors] " Mark Brown
2015-10-01 20:37 ` Wolfram Sang
2015-10-02  9:27   ` Jarkko Nikula
2015-10-09 21:47     ` Wolfram Sang
2015-10-12  8:32       ` Jarkko Nikula

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.