[RFC 0/2] I2C and SPI dev_name change for ACPI enumerated slaves

[RFC 0/2] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

Hi

We've run into problem of changing I2C device names while developing
ALSA SoC drivers for x86 based systems. Changing device names makes
more difficult to match devices by their name. Which is what we use
within ASoC subsystem.

These changing names comes from changing adapter/bus numbering which
could occur due variable amount of bus controllers, probe order, add-on
cards or different BIOS settings.

Patches here try to solve the problem on ACPI 5 based systems by
using stable ACPI device name with a "i2c-"/"spi-" prefix for I2C/SPI
slave device names.

Both patches are independent from each other and can go through their
own subsystems.

Jarkko Nikula (2):
  i2c: Use stable dev_name for ACPI enumerated I2C slaves
  spi: Use stable dev_name for ACPI enumerated SPI slaves

 drivers/i2c/i2c-core.c | 24 ++++++++++++++++++++----
 drivers/spi/spi.c      | 21 ++++++++++++++++++---
 2 files changed, 38 insertions(+), 7 deletions(-)

-- 
1.8.4.rc3

[RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Current I2C adapter id - client address "x-00yy" based device naming scheme
is not always stable enough to be used in name based matching, for instance
within ALSA SoC subsystem.

This is problematic in PC kind of platforms where I2C adapter numbers can
change due variable amount of bus controllers, probe order, add-on cards or
just because of BIOS settings.

This patch addresses the problem by using the ACPI device name with
"i2c-" prefix for ACPI enumerated I2C slaves. For them device name
"x-00yz" becomes "i2c-INTABCD:ij" after this patch.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
I'm not sure when would acpi_bus_get_device fail and how realistic is that here.
I put minimalistic error handling here which just falls back to old
adapter-addr naming scheme.
---
 drivers/i2c/i2c-core.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 111b2c6..8d6f3e5 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -613,6 +613,25 @@ 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)
+{
+#if IS_ENABLED(CONFIG_ACPI)
+	if (ACPI_HANDLE(&client->dev)) {
+		struct acpi_device *adev;
+		if (!acpi_bus_get_device(ACPI_HANDLE(&client->dev), &adev)) {
+			dev_set_name(&client->dev, "i2c-%s",
+				     dev_name(&adev->dev));
+			return;
+		}
+	}
+#endif
+	/* 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
@@ -671,10 +690,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->dev.of_node = info->of_node;
 	ACPI_HANDLE_SET(&client->dev, info->acpi_node.handle);
 
-	/* 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_dev_set_name(adap, client);
 	status = device_register(&client->dev);
 	if (status)
 		goto out_err;
-- 
1.8.4.rc3

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Rafael J. Wysocki]

On Friday, October 25, 2013 03:18:59 PM Jarkko Nikula wrote:
> Current I2C adapter id - client address "x-00yy" based device naming scheme
> is not always stable enough to be used in name based matching, for instance
> within ALSA SoC subsystem.
> 
> This is problematic in PC kind of platforms where I2C adapter numbers can
> change due variable amount of bus controllers, probe order, add-on cards or
> just because of BIOS settings.
> 
> This patch addresses the problem by using the ACPI device name with
> "i2c-" prefix for ACPI enumerated I2C slaves. For them device name
> "x-00yz" becomes "i2c-INTABCD:ij" after this patch.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
> I'm not sure when would acpi_bus_get_device fail and how realistic is that here.
> I put minimalistic error handling here which just falls back to old
> adapter-addr naming scheme.
> ---
>  drivers/i2c/i2c-core.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 111b2c6..8d6f3e5 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -613,6 +613,25 @@ 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)
> +{
> +#if IS_ENABLED(CONFIG_ACPI)
> +	if (ACPI_HANDLE(&client->dev)) {

ACPI_HANDLE() already contains an "is CONFIG_ACPI enabled?" check, so the #if
around the if (ACPI_HANDLE()) {} is redundant.

Similarly for the SPI patch.

> +		struct acpi_device *adev;
> +		if (!acpi_bus_get_device(ACPI_HANDLE(&client->dev), &adev)) {
> +			dev_set_name(&client->dev, "i2c-%s",
> +				     dev_name(&adev->dev));
> +			return;
> +		}
> +	}
> +#endif
> +	/* 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
> @@ -671,10 +690,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
>  	client->dev.of_node = info->of_node;
>  	ACPI_HANDLE_SET(&client->dev, info->acpi_node.handle);
>  
> -	/* 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_dev_set_name(adap, client);
>  	status = device_register(&client->dev);
>  	if (status)
>  		goto out_err;
> 

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Hi

On 10/25/2013 03:52 PM, Rafael J. Wysocki wrote:
>   
> +static void i2c_dev_set_name(struct i2c_adapter *adap,
> +			     struct i2c_client *client)
> +{
> +#if IS_ENABLED(CONFIG_ACPI)
> +	if (ACPI_HANDLE(&client->dev)) {
> ACPI_HANDLE() already contains an "is CONFIG_ACPI enabled?" check, so the #if
> around the if (ACPI_HANDLE()) {} is redundant.
>
> Similarly for the SPI patch.
>
Well, acpi_bus_get_device() is not available for non-ACPI builds and at 
least the gcc I used for test build didn't optimize that block away.

-- 
Jarkko

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Rafael J. Wysocki]

On Friday, October 25, 2013 03:55:29 PM Jarkko Nikula wrote:
> Hi
> 
> On 10/25/2013 03:52 PM, Rafael J. Wysocki wrote:
> >   
> > +static void i2c_dev_set_name(struct i2c_adapter *adap,
> > +			     struct i2c_client *client)
> > +{
> > +#if IS_ENABLED(CONFIG_ACPI)
> > +	if (ACPI_HANDLE(&client->dev)) {
> > ACPI_HANDLE() already contains an "is CONFIG_ACPI enabled?" check, so the #if
> > around the if (ACPI_HANDLE()) {} is redundant.
> >
> > Similarly for the SPI patch.
> >
> Well, acpi_bus_get_device() is not available for non-ACPI builds and at 
> least the gcc I used for test build didn't optimize that block away.

Well, it should.  ACPI_HANDLE() translates to (NULL) if CONFIG_ACPI
is not defined and that causes the check to become "if (NULL)" which
should always be dropped by the compiler.

Does providing a stub acpi_bus_get_device() for !CONFIG_ACPI actually help?

Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

On 10/25/2013 04:18 PM, Rafael J. Wysocki wrote:
> On Friday, October 25, 2013 03:55:29 PM Jarkko Nikula wrote:
>>
>> Well, acpi_bus_get_device() is not available for non-ACPI builds and at
>> least the gcc I used for test build didn't optimize that block away.
> Well, it should.  ACPI_HANDLE() translates to (NULL) if CONFIG_ACPI
> is not defined and that causes the check to become "if (NULL)" which
> should always be dropped by the compiler.
>
My very vague memory says the same. I don't know is this gcc version or 
flag dependent behavior. I got the build error from both i386 build 
using gcc 4.8.1 and arm build by using make ARCH=arm omap2plus_defconfig 
and gcc-4.7-arm-linux-gnueabi 4.7.2-4.
> Does providing a stub acpi_bus_get_device() for !CONFIG_ACPI actually help?
>
>
Hmm, not only. Referencing to dev field in struct acpi_device by 
dev_name(&adev->dev) here will fail too.

-- 
Jarkko

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Rafael J. Wysocki]

On Friday, October 25, 2013 04:30:23 PM Jarkko Nikula wrote:
> On 10/25/2013 04:18 PM, Rafael J. Wysocki wrote:
> > On Friday, October 25, 2013 03:55:29 PM Jarkko Nikula wrote:
> >>
> >> Well, acpi_bus_get_device() is not available for non-ACPI builds and at
> >> least the gcc I used for test build didn't optimize that block away.
> > Well, it should.  ACPI_HANDLE() translates to (NULL) if CONFIG_ACPI
> > is not defined and that causes the check to become "if (NULL)" which
> > should always be dropped by the compiler.
> >
> My very vague memory says the same. I don't know is this gcc version or 
> flag dependent behavior. I got the build error from both i386 build 
> using gcc 4.8.1 and arm build by using make ARCH=arm omap2plus_defconfig 
> and gcc-4.7-arm-linux-gnueabi 4.7.2-4.
> > Does providing a stub acpi_bus_get_device() for !CONFIG_ACPI actually help?
> >
> >
> Hmm, not only. Referencing to dev field in struct acpi_device by 
> dev_name(&adev->dev) here will fail too.

Well, something is not quite right here.

One of the *reasons* for having ACPI_HANDLE() defined this way is to avoid
using explicit CONFIG_ACPI checks, so if that doesn't work, then all of that
becomes a bit pointless.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Hi Rafael

On 10/25/2013 05:08 PM, Rafael J. Wysocki wrote:
> On Friday, October 25, 2013  04:30:23 PM Jarkko Nikula wrote:
 >>>
 >> Hmm, not only. Referencing to dev field in struct acpi_device by
 >> dev_name(&adev->dev) here will fail too.
 >
 > Well, something is not quite right here.
 >
 > One of the *reasons* for having ACPI_HANDLE() defined this way is to
 > avoid using explicit CONFIG_ACPI checks, so if that doesn't work,
 > then all of that becomes a bit pointless.
 >
One possible thing to do is to let structure definitions to be available 
for non-ACPI builds. Then compiler won't fail on structure access which 
will be anyway optimized away by later compiler stages.

With a quick test below vmlinux section sizes don't change for couple 
non-ACPI build tests and allow to get rid off IS_ENABLED(CONFIG_ACPI) 
test in this patch. It's very minimal as it only moves the CONFIG_ACPI 
test just after the struct acpi_device definition and defines 
acpi_bus_get_device for non-ACPI case.

What I don't know how consistent it is as there are still couple 
structure definitions under CONFIG_ACPI, doesn't touch other acpi 
headers and requires to include acpi_bus.h in driver (or move acpi_bus.h 
include in linux/acpi.h currently under CONFIG_ACPI).

-- 
Jarkko

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index d901982..7ab7870 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -66,8 +66,6 @@ bool acpi_ata_match(acpi_handle handle);
  bool acpi_bay_match(acpi_handle handle);
  bool acpi_dock_match(acpi_handle handle);

-#ifdef CONFIG_ACPI
-
  #include <linux/proc_fs.h>

  #define ACPI_BUS_FILE_ROOT    "acpi"
@@ -314,6 +312,8 @@ struct acpi_device {
      void (*remove)(struct acpi_device *);
  };

+#if IS_ENABLED(CONFIG_ACPI)
+
  static inline void *acpi_driver_data(struct acpi_device *d)
  {
      return d->driver_data;
@@ -531,6 +531,8 @@ static inline bool acpi_device_can_poweroff(struct 
acpi_device *adev)

  static inline int register_acpi_bus_type(void *bus) { return 0; }
  static inline int unregister_acpi_bus_type(void *bus) { return 0; }
+static inline int acpi_bus_get_device(acpi_handle handle,
+                      struct acpi_device **device) { return 0; }

  #endif                /* CONFIG_ACPI */

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Mark Brown]

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

On Mon, Oct 28, 2013 at 03:15:25PM +0200, Jarkko Nikula wrote:

> One possible thing to do is to let structure definitions to be
> available for non-ACPI builds. Then compiler won't fail on structure
> access which will be anyway optimized away by later compiler stages.

You could also have inline accessor functions which can be stubbed out
when not needed.

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

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Rafael J. Wysocki]

On Monday, October 28, 2013 03:15:25 PM Jarkko Nikula wrote:
> Hi Rafael
> 
> On 10/25/2013 05:08 PM, Rafael J. Wysocki wrote:
> > On Friday, October 25, 2013  04:30:23 PM Jarkko Nikula wrote:
>  >>>
>  >> Hmm, not only. Referencing to dev field in struct acpi_device by
>  >> dev_name(&adev->dev) here will fail too.
>  >
>  > Well, something is not quite right here.
>  >
>  > One of the *reasons* for having ACPI_HANDLE() defined this way is to
>  > avoid using explicit CONFIG_ACPI checks, so if that doesn't work,
>  > then all of that becomes a bit pointless.
>  >

Can you please send patches inline instead of using inline attachments,
so that people don't have to paste your patches back to comment them?

> One possible thing to do is to let structure definitions to be available 
> for non-ACPI builds. Then compiler won't fail on structure access which 
> will be anyway optimized away by later compiler stages.

Yes, we can do that, but as I said that means we're giving up on the "why
ACPI_HANDLE() doesn't work as expected" front.  For now, I'd like to
understand what's up before making that change.  Moreover ->

> With a quick test below vmlinux section sizes don't change for couple 
> non-ACPI build tests and allow to get rid off IS_ENABLED(CONFIG_ACPI) 
> test in this patch. It's very minimal as it only moves the CONFIG_ACPI 
> test just after the struct acpi_device definition and defines 
> acpi_bus_get_device for non-ACPI case.
> 
> What I don't know how consistent it is as there are still couple 
> structure definitions under CONFIG_ACPI, doesn't touch other acpi 
> headers and requires to include acpi_bus.h in driver (or move acpi_bus.h 
> include in linux/acpi.h currently under CONFIG_ACPI).
>
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index d901982..7ab7870 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -66,8 +66,6 @@ bool acpi_ata_match(acpi_handle handle);
> 
>   bool acpi_bay_match(acpi_handle handle);
>   bool acpi_dock_match(acpi_handle handle);
> 
> -#ifdef CONFIG_ACPI
> -
> 
>   #include <linux/proc_fs.h>
>   
>   #define ACPI_BUS_FILE_ROOT    "acpi"
> 
> @@ -314,6 +312,8 @@ struct acpi_device {
> 
>       void (*remove)(struct acpi_device *);
>   
>   };
> 
> +#if IS_ENABLED(CONFIG_ACPI)
> +
> 
>   static inline void *acpi_driver_data(struct acpi_device *d)
>   {
>   
>       return d->driver_data;
> 
> @@ -531,6 +531,8 @@ static inline bool acpi_device_can_poweroff(struct
> acpi_device *adev)
> 
>   static inline int register_acpi_bus_type(void *bus) { return 0; }
>   static inline int unregister_acpi_bus_type(void *bus) { return 0; }
> 
> +static inline int acpi_bus_get_device(acpi_handle handle,
> +                      struct acpi_device **device) { return 0; }

This has to return an error code, because otherwise the caller can assume
*device to be a valid pointer after it returns which may not be the case.

> 
>   #endif                /* CONFIG_ACPI */

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/2] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Rafael J. Wysocki]

On Friday, November 01, 2013 01:08:47 AM Rafael J. Wysocki wrote:
> On Monday, October 28, 2013 03:15:25 PM Jarkko Nikula wrote:
> > Hi Rafael
> > 
> > On 10/25/2013 05:08 PM, Rafael J. Wysocki wrote:
> > > On Friday, October 25, 2013  04:30:23 PM Jarkko Nikula wrote:
> >  >>>
> >  >> Hmm, not only. Referencing to dev field in struct acpi_device by
> >  >> dev_name(&adev->dev) here will fail too.
> >  >
> >  > Well, something is not quite right here.
> >  >
> >  > One of the *reasons* for having ACPI_HANDLE() defined this way is to
> >  > avoid using explicit CONFIG_ACPI checks, so if that doesn't work,
> >  > then all of that becomes a bit pointless.
> >  >
> 
> Can you please send patches inline instead of using inline attachments,
> so that people don't have to paste your patches back to comment them?
> 
> > One possible thing to do is to let structure definitions to be available 
> > for non-ACPI builds. Then compiler won't fail on structure access which 
> > will be anyway optimized away by later compiler stages.
> 
> Yes, we can do that, but as I said that means we're giving up on the "why
> ACPI_HANDLE() doesn't work as expected" front.  For now, I'd like to
> understand what's up before making that change.  Moreover ->

OK, so I *think* what happens is that the compiler checks if the particular
symbols make sense before trying to optimize out the whole block.

So the patch below modulo the return value of the acpi_bus_get_device() stub
would be fine by me.

Thanks!

> > With a quick test below vmlinux section sizes don't change for couple 
> > non-ACPI build tests and allow to get rid off IS_ENABLED(CONFIG_ACPI) 
> > test in this patch. It's very minimal as it only moves the CONFIG_ACPI 
> > test just after the struct acpi_device definition and defines 
> > acpi_bus_get_device for non-ACPI case.
> > 
> > What I don't know how consistent it is as there are still couple 
> > structure definitions under CONFIG_ACPI, doesn't touch other acpi 
> > headers and requires to include acpi_bus.h in driver (or move acpi_bus.h 
> > include in linux/acpi.h currently under CONFIG_ACPI).
> >
> > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> > index d901982..7ab7870 100644
> > --- a/include/acpi/acpi_bus.h
> > +++ b/include/acpi/acpi_bus.h
> > @@ -66,8 +66,6 @@ bool acpi_ata_match(acpi_handle handle);
> > 
> >   bool acpi_bay_match(acpi_handle handle);
> >   bool acpi_dock_match(acpi_handle handle);
> > 
> > -#ifdef CONFIG_ACPI
> > -
> > 
> >   #include <linux/proc_fs.h>
> >   
> >   #define ACPI_BUS_FILE_ROOT    "acpi"
> > 
> > @@ -314,6 +312,8 @@ struct acpi_device {
> > 
> >       void (*remove)(struct acpi_device *);
> >   
> >   };
> > 
> > +#if IS_ENABLED(CONFIG_ACPI)
> > +
> > 
> >   static inline void *acpi_driver_data(struct acpi_device *d)
> >   {
> >   
> >       return d->driver_data;
> > 
> > @@ -531,6 +531,8 @@ static inline bool acpi_device_can_poweroff(struct
> > acpi_device *adev)
> > 
> >   static inline int register_acpi_bus_type(void *bus) { return 0; }
> >   static inline int unregister_acpi_bus_type(void *bus) { return 0; }
> > 
> > +static inline int acpi_bus_get_device(acpi_handle handle,
> > +                      struct acpi_device **device) { return 0; }
> 
> This has to return an error code, because otherwise the caller can assume
> *device to be a valid pointer after it returns which may not be the case.
> 
> > 
> >   #endif                /* CONFIG_ACPI */

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 2/2] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Jarkko Nikula]

Current spi bus_num.chip_select "spix.y" based device naming scheme may not
be stable enough to be used in name based matching, for instance within
ALSA SoC subsystem.

This can be problem in PC kind of platforms if there are changes in SPI bus
configuration, amount of busses or probe order.

This patch addresses the problem by using the ACPI device name with
"spi-" prefix for ACPI enumerated SPI slave. For them device name
"spix.y" becomes "spi-INTABCD:ij".

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
"may not be" and "This can be problem" since I don't know how likely is to
have changes in SPI bus configuration in PC kind of devices. I assume yes as
this is already problem with I2C devices.
---
 drivers/spi/spi.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 8d05acc..2ca997f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -345,6 +345,23 @@ struct spi_device *spi_alloc_device(struct spi_master *master)
 }
 EXPORT_SYMBOL_GPL(spi_alloc_device);
 
+void spi_dev_set_name(struct spi_device *spi)
+{
+#if IS_ENABLED(CONFIG_ACPI)
+	if (ACPI_HANDLE(&spi->dev)) {
+		struct acpi_device *adev;
+		if (!acpi_bus_get_device(ACPI_HANDLE(&spi->dev), &adev)) {
+			dev_set_name(&spi->dev, "spi-%s",
+				     dev_name(&adev->dev));
+			return;
+		}
+	}
+#endif
+
+	dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->master->dev),
+		     spi->chip_select);
+}
+
 /**
  * spi_add_device - Add spi_device allocated with spi_alloc_device
  * @spi: spi_device to register
@@ -371,9 +388,7 @@ int spi_add_device(struct spi_device *spi)
 	}
 
 	/* Set the bus ID string */
-	dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->master->dev),
-			spi->chip_select);
-
+	spi_dev_set_name(spi);
 
 	/* We need to make sure there's no other device with this
 	 * chipselect **BEFORE** we call setup(), else we'll trash
-- 
1.8.4.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 2/2] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Mark Brown]

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

On Fri, Oct 25, 2013 at 03:19:00PM +0300, Jarkko Nikula wrote:
> Current spi bus_num.chip_select "spix.y" based device naming scheme may not
> be stable enough to be used in name based matching, for instance within
> ALSA SoC subsystem.

I'm happy with this from the SPI side modulo Raphael's comments - are
folks happy from the ACPI side?

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

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

Re: [RFC 2/2] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Rafael J. Wysocki]

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

On Friday, October 25, 2013 01:59:09 PM Mark Brown wrote:
> On Fri, Oct 25, 2013 at 03:19:00PM +0300, Jarkko Nikula wrote:
> > Current spi bus_num.chip_select "spix.y" based device naming scheme may not
> > be stable enough to be used in name based matching, for instance within
> > ALSA SoC subsystem.
> 
> I'm happy with this from the SPI side modulo Raphael's comments - are
> folks happy from the ACPI side?

I think yes in general, but I'd like to understand why the ACPI_HANDLE()
thing doesn't work as expected.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

[PATCHv2 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

Hi

Second version of the set that changes I2C and SPI slave device names
to be generated from stable ACPI device names on ACPI 5 based systems
instead of using bus numbers which could change.

Slave device name change goes as

	"x-00yz" -> "i2c-INTABCD:ij"
	"spix.y" -> "spi-INTABCD:ij"

This version adds patch to include/acpi/acpi_bus.h that allow us to
remove #if IS_ENABLED(CONFIG_ACPI) checks that were added in the
first version.

Set goes on top linux-pm/linux-next commit e56b4d2.

First version here:
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-October/067737.html

Jarkko Nikula (3):
  ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI
    builds
  i2c: Use stable dev_name for ACPI enumerated I2C slaves
  spi: Use stable dev_name for ACPI enumerated SPI slaves

 drivers/i2c/i2c-core.c  | 24 ++++++++++++++++++++----
 drivers/spi/spi.c       | 20 +++++++++++++++++---
 include/acpi/acpi_bus.h |  9 +++++++--
 3 files changed, 44 insertions(+), 9 deletions(-)

-- 
1.8.4.rc3

[PATCHv2 2/3] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Current I2C adapter id - client address "x-00yy" based device naming scheme
is not always stable enough to be used in name based matching, for instance
within ALSA SoC subsystem.

This is problematic in PC kind of platforms where I2C adapter numbers can
change due variable amount of bus controllers, probe order, add-on cards or
just because of BIOS settings.

This patch addresses the problem by using the ACPI device name with
"i2c-" prefix for ACPI enumerated I2C slaves. For them device name
"x-00yz" becomes "i2c-INTABCD:ij" after this patch.

Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
 drivers/i2c/i2c-core.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 75ba860..9126c2e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -48,6 +48,7 @@
 #include <linux/rwsem.h>
 #include <linux/pm_runtime.h>
 #include <linux/acpi.h>
+#include <acpi/acpi_bus.h>
 #include <asm/uaccess.h>
 
 #include "i2c-core.h"
@@ -618,6 +619,24 @@ 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)
+{
+	if (ACPI_HANDLE(&client->dev)) {
+		struct acpi_device *adev;
+		if (!acpi_bus_get_device(ACPI_HANDLE(&client->dev), &adev)) {
+			dev_set_name(&client->dev, "i2c-%s",
+				     dev_name(&adev->dev));
+			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
@@ -676,10 +695,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->dev.of_node = info->of_node;
 	ACPI_HANDLE_SET(&client->dev, info->acpi_node.handle);
 
-	/* 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_dev_set_name(adap, client);
 	status = device_register(&client->dev);
 	if (status)
 		goto out_err;
-- 
1.8.4.rc3

Re: [PATCHv2 2/3] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Wolfram Sang]

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

On Fri, Nov 01, 2013 at 02:35:55PM +0200, Jarkko Nikula wrote:
> Current I2C adapter id - client address "x-00yy" based device naming scheme
> is not always stable enough to be used in name based matching, for instance
> within ALSA SoC subsystem.
> 
> This is problematic in PC kind of platforms where I2C adapter numbers can
> change due variable amount of bus controllers, probe order, add-on cards or
> just because of BIOS settings.
> 
> This patch addresses the problem by using the ACPI device name with
> "i2c-" prefix for ACPI enumerated I2C slaves. For them device name
> "x-00yz" becomes "i2c-INTABCD:ij" after this patch.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Acked-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>


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

[PATCHv2 1/3] ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI builds

[Jarkko Nikula]

We may find use for struct acpi_device and acpi_bus_get_device() in generic
code without wanting to add #if IS_ENABLED(CONFIG_ACPI) churn there.

Code can use ACPI_HANDLE() test to let compiler optimize out code blocks
that are not used in !CONFIG_ACPI builds but structure definitions and
function stubs must be available.

This patch moves CONFIG_ACPI test so that struct acpi_device definition
is available for all builds that include acpi_bus.h and provides a stub for
acpi_bus_get_device().

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 include/acpi/acpi_bus.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 15100f6..232b37d3 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -66,8 +66,6 @@ bool acpi_ata_match(acpi_handle handle);
 bool acpi_bay_match(acpi_handle handle);
 bool acpi_dock_match(acpi_handle handle);
 
-#ifdef CONFIG_ACPI
-
 #include <linux/proc_fs.h>
 
 #define ACPI_BUS_FILE_ROOT	"acpi"
@@ -315,6 +313,8 @@ struct acpi_device {
 	void (*remove)(struct acpi_device *);
 };
 
+#if IS_ENABLED(CONFIG_ACPI)
+
 static inline void *acpi_driver_data(struct acpi_device *d)
 {
 	return d->driver_data;
@@ -532,6 +532,11 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
 
 static inline int register_acpi_bus_type(void *bus) { return 0; }
 static inline int unregister_acpi_bus_type(void *bus) { return 0; }
+static inline int acpi_bus_get_device(acpi_handle handle,
+				      struct acpi_device **device)
+{
+	return -ENODEV;
+}
 
 #endif				/* CONFIG_ACPI */
 
-- 
1.8.4.rc3

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 1/3] ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI builds

[Rafael J. Wysocki]

On Friday, November 01, 2013 02:35:54 PM Jarkko Nikula wrote:
> We may find use for struct acpi_device and acpi_bus_get_device() in generic
> code without wanting to add #if IS_ENABLED(CONFIG_ACPI) churn there.
> 
> Code can use ACPI_HANDLE() test to let compiler optimize out code blocks
> that are not used in !CONFIG_ACPI builds but structure definitions and
> function stubs must be available.
> 
> This patch moves CONFIG_ACPI test so that struct acpi_device definition
> is available for all builds that include acpi_bus.h and provides a stub for
> acpi_bus_get_device().
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

This turns out the cause build problems to happen on some architectures.

I guess it's sufficient to simply define a stub struct acpi_device as

struct acpi_device {
	struct device dev;
};

for !CONFIG_ACPI instead.

> ---
>  include/acpi/acpi_bus.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 15100f6..232b37d3 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -66,8 +66,6 @@ bool acpi_ata_match(acpi_handle handle);
>  bool acpi_bay_match(acpi_handle handle);
>  bool acpi_dock_match(acpi_handle handle);
>  
> -#ifdef CONFIG_ACPI
> -
>  #include <linux/proc_fs.h>
>  
>  #define ACPI_BUS_FILE_ROOT	"acpi"
> @@ -315,6 +313,8 @@ struct acpi_device {
>  	void (*remove)(struct acpi_device *);
>  };
>  
> +#if IS_ENABLED(CONFIG_ACPI)
> +
>  static inline void *acpi_driver_data(struct acpi_device *d)
>  {
>  	return d->driver_data;
> @@ -532,6 +532,11 @@ static inline bool acpi_device_can_poweroff(struct acpi_device *adev)
>  
>  static inline int register_acpi_bus_type(void *bus) { return 0; }
>  static inline int unregister_acpi_bus_type(void *bus) { return 0; }
> +static inline int acpi_bus_get_device(acpi_handle handle,
> +				      struct acpi_device **device)
> +{
> +	return -ENODEV;
> +}
>  
>  #endif				/* CONFIG_ACPI */
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCHv2 1/3] ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI builds

[Rafael J. Wysocki]

On Saturday, November 02, 2013 11:18:31 PM Rafael J. Wysocki wrote:
> On Friday, November 01, 2013 02:35:54 PM Jarkko Nikula wrote:
> > We may find use for struct acpi_device and acpi_bus_get_device() in generic
> > code without wanting to add #if IS_ENABLED(CONFIG_ACPI) churn there.
> > 
> > Code can use ACPI_HANDLE() test to let compiler optimize out code blocks
> > that are not used in !CONFIG_ACPI builds but structure definitions and
> > function stubs must be available.
> > 
> > This patch moves CONFIG_ACPI test so that struct acpi_device definition
> > is available for all builds that include acpi_bus.h and provides a stub for
> > acpi_bus_get_device().
> > 
> > Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> This turns out the cause build problems to happen on some architectures.
> 
> I guess it's sufficient to simply define a stub struct acpi_device as
> 
> struct acpi_device {
> 	struct device dev;
> };
> 
> for !CONFIG_ACPI instead.

Generally, it is a bad idea to #include acpi_bus.h for !CONFIG_ACPI.

The appended patch works for me, sorry for stealing part of your changelog.

Thanks,
Rafael


---
From: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Subject: ACPI: Define struct acpi_device for CONFIG_ACPI unset

We may find use for struct acpi_device and acpi_bus_get_device() in
generic code without wanting to add #if IS_ENABLED(CONFIG_ACPI) churn
there.

Code can use ACPI_HANDLE() test to let compiler optimize out code
blocks that are not used in !CONFIG_ACPI builds but structure
definitions and function stubs must be available.

Define a stub struct acpi_device for CONFIG_ACPI unset containing dev
as the only member and a stub acpi_bus_get_device() always returning
an error code.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 include/linux/acpi.h |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -472,7 +472,17 @@ static inline bool acpi_driver_match_dev
 }
 
 #define ACPI_PTR(_ptr)	(NULL)
+typedef void * acpi_handle;
 
+struct acpi_device {
+	struct device dev;
+};
+
+static inline int acpi_bus_get_device(acpi_handle handle,
+				      struct acpi_device **device)
+{
+	return -ENODEV;
+}
 #endif	/* !CONFIG_ACPI */
 
 #ifdef CONFIG_ACPI

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 1/3] ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI builds

[Jarkko Nikula]

On Mon, Nov 04, 2013 at 02:00:45AM +0100, Rafael J. Wysocki wrote:
> On Saturday, November 02, 2013 11:18:31 PM Rafael J. Wysocki wrote:
> > This turns out the cause build problems to happen on some architectures.
> > 
> > I guess it's sufficient to simply define a stub struct acpi_device as
> > 
> > struct acpi_device {
> > 	struct device dev;
> > };
> > 
> > for !CONFIG_ACPI instead.
> 
> Generally, it is a bad idea to #include acpi_bus.h for !CONFIG_ACPI.
> 
Ouch, indeed.

> The appended patch works for me, sorry for stealing part of your changelog.
> 
No problem. Thanks for fixing this up.

-- 
Jarkko

Re: [PATCHv2 1/3] ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI builds

[Rafael J. Wysocki]

On Monday, November 04, 2013 11:15:59 PM Jarkko Nikula wrote:
> On Mon, Nov 04, 2013 at 02:00:45AM +0100, Rafael J. Wysocki wrote:
> > On Saturday, November 02, 2013 11:18:31 PM Rafael J. Wysocki wrote:
> > > This turns out the cause build problems to happen on some architectures.
> > > 
> > > I guess it's sufficient to simply define a stub struct acpi_device as
> > > 
> > > struct acpi_device {
> > > 	struct device dev;
> > > };
> > > 
> > > for !CONFIG_ACPI instead.
> > 
> > Generally, it is a bad idea to #include acpi_bus.h for !CONFIG_ACPI.
> > 
> Ouch, indeed.
> 
> > The appended patch works for me, sorry for stealing part of your changelog.
> > 
> No problem. Thanks for fixing this up.

Well, unfortunately, this breaks builds for allnoconfig, because <acpi/acpi.h>
is included directly from some header files (not under a CONFIG_ACPI ifdef) and
duplicates the acpi_handle typedef.

So, we'll need to go through the headers, see which ones do that and
replace <acpi/acpi.h> with <linux/acpi.h> (along with adding the stub
definitions for !CONFIG_ACPI).

I'm deferring the patchset for after 3.13-rc1 for this reason.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

[PATCHv2 3/3] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Jarkko Nikula]

Current spi bus_num.chip_select "spix.y" based device naming scheme may not
be stable enough to be used in name based matching, for instance within
ALSA SoC subsystem.

This can be problem in PC kind of platforms if there are changes in SPI bus
configuration, amount of busses or probe order.

This patch addresses the problem by using the ACPI device name with
"spi-" prefix for ACPI enumerated SPI slave. For them device name
"spix.y" becomes "spi-INTABCD:ij".

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/spi/spi.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 740f9dd..4c0c801 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -38,6 +38,7 @@
 #include <linux/kthread.h>
 #include <linux/ioport.h>
 #include <linux/acpi.h>
+#include <acpi/acpi_bus.h>
 
 static void spidev_release(struct device *dev)
 {
@@ -352,6 +353,21 @@ struct spi_device *spi_alloc_device(struct spi_master *master)
 }
 EXPORT_SYMBOL_GPL(spi_alloc_device);
 
+void spi_dev_set_name(struct spi_device *spi)
+{
+	if (ACPI_HANDLE(&spi->dev)) {
+		struct acpi_device *adev;
+		if (!acpi_bus_get_device(ACPI_HANDLE(&spi->dev), &adev)) {
+			dev_set_name(&spi->dev, "spi-%s",
+				     dev_name(&adev->dev));
+			return;
+		}
+	}
+
+	dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->master->dev),
+		     spi->chip_select);
+}
+
 /**
  * spi_add_device - Add spi_device allocated with spi_alloc_device
  * @spi: spi_device to register
@@ -378,9 +394,7 @@ int spi_add_device(struct spi_device *spi)
 	}
 
 	/* Set the bus ID string */
-	dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->master->dev),
-			spi->chip_select);
-
+	spi_dev_set_name(spi);
 
 	/* We need to make sure there's no other device with this
 	 * chipselect **BEFORE** we call setup(), else we'll trash
-- 
1.8.4.rc3

Re: [PATCHv2 3/3] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Mark Brown]

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

On Fri, Nov 01, 2013 at 02:35:56PM +0200, Jarkko Nikula wrote:
> Current spi bus_num.chip_select "spix.y" based device naming scheme may not
> be stable enough to be used in name based matching, for instance within
> ALSA SoC subsystem.

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

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

Re: [PATCHv2 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Rafael J. Wysocki]

On Friday, November 01, 2013 02:35:53 PM Jarkko Nikula wrote:
> Hi
> 
> Second version of the set that changes I2C and SPI slave device names
> to be generated from stable ACPI device names on ACPI 5 based systems
> instead of using bus numbers which could change.
> 
> Slave device name change goes as
> 
> 	"x-00yz" -> "i2c-INTABCD:ij"
> 	"spix.y" -> "spi-INTABCD:ij"
> 
> This version adds patch to include/acpi/acpi_bus.h that allow us to
> remove #if IS_ENABLED(CONFIG_ACPI) checks that were added in the
> first version.
> 
> Set goes on top linux-pm/linux-next commit e56b4d2.
> 
> First version here:
> http://mailman.alsa-project.org/pipermail/alsa-devel/2013-October/067737.html
> 
> Jarkko Nikula (3):
>   ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI
>     builds
>   i2c: Use stable dev_name for ACPI enumerated I2C slaves
>   spi: Use stable dev_name for ACPI enumerated SPI slaves
> 
>  drivers/i2c/i2c-core.c  | 24 ++++++++++++++++++++----
>  drivers/spi/spi.c       | 20 +++++++++++++++++---
>  include/acpi/acpi_bus.h |  9 +++++++--
>  3 files changed, 44 insertions(+), 9 deletions(-)

Looks good to me.  If there are no objections, I can merge these through my tree.

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Wolfram Sang]

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

On Fri, Nov 01, 2013 at 02:18:06PM +0100, Rafael J. Wysocki wrote:
> On Friday, November 01, 2013 02:35:53 PM Jarkko Nikula wrote:
> > Hi
> > 
> > Second version of the set that changes I2C and SPI slave device names
> > to be generated from stable ACPI device names on ACPI 5 based systems
> > instead of using bus numbers which could change.
> > 
> > Slave device name change goes as
> > 
> > 	"x-00yz" -> "i2c-INTABCD:ij"
> > 	"spix.y" -> "spi-INTABCD:ij"
> > 
> > This version adds patch to include/acpi/acpi_bus.h that allow us to
> > remove #if IS_ENABLED(CONFIG_ACPI) checks that were added in the
> > first version.
> > 
> > Set goes on top linux-pm/linux-next commit e56b4d2.
> > 
> > First version here:
> > http://mailman.alsa-project.org/pipermail/alsa-devel/2013-October/067737.html
> > 
> > Jarkko Nikula (3):
> >   ACPI: Expose struct acpi_device and acpi_bus_get_device() to non-ACPI
> >     builds
> >   i2c: Use stable dev_name for ACPI enumerated I2C slaves
> >   spi: Use stable dev_name for ACPI enumerated SPI slaves
> > 
> >  drivers/i2c/i2c-core.c  | 24 ++++++++++++++++++++----
> >  drivers/spi/spi.c       | 20 +++++++++++++++++---
> >  include/acpi/acpi_bus.h |  9 +++++++--
> >  3 files changed, 44 insertions(+), 9 deletions(-)
> 
> Looks good to me.  If there are no objections, I can merge these through my tree.

Which is basically fine with me. Do you want to have it in 3.13 already?
I mean renaming the devices could lead to regressions, so I'd rather be
conservative and aim for 3.14.


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

Re: [PATCHv2 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

On 11/01/2013 03:20 PM, Wolfram Sang wrote:
> On Fri, Nov 01, 2013 at 02:18:06PM +0100, Rafael J. Wysocki wrote:
>>
>> Looks good to me.  If there are no objections, I can merge these through my tree.
> Which is basically fine with me. Do you want to have it in 3.13 already?
> I mean renaming the devices could lead to regressions, so I'd rather be
> conservative and aim for 3.14.
>
Valid concern. Quick grep below doesn't reveal any obvious device name 
matching outside of sound/soc/ but of course it doesn't prove it to be 
impossible.

git grep '[0-9]\-00' |grep name
git grep 'spi[0-9].[0-9]' |grep name

-- 
Jarkko

Re: [PATCHv2 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Rafael J. Wysocki]

On Friday, November 01, 2013 03:44:38 PM Jarkko Nikula wrote:
> On 11/01/2013 03:20 PM, Wolfram Sang wrote:
> > On Fri, Nov 01, 2013 at 02:18:06PM +0100, Rafael J. Wysocki wrote:
> >>
> >> Looks good to me.  If there are no objections, I can merge these through my tree.
> > Which is basically fine with me. Do you want to have it in 3.13 already?
> > I mean renaming the devices could lead to regressions, so I'd rather be
> > conservative and aim for 3.14.
> >
> Valid concern. Quick grep below doesn't reveal any obvious device name 
> matching outside of sound/soc/ but of course it doesn't prove it to be 
> impossible.
> 
> git grep '[0-9]\-00' |grep name
> git grep 'spi[0-9].[0-9]' |grep name

Well, if any breakage is caught in 3.13-rc, it should be easy enough to revert
these changes and try again during the next cycle.  I honestly don't see any
benefit from waiting for the next cycle "just in case".

Thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Re: [PATCHv2 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Wolfram Sang]

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


> Well, if any breakage is caught in 3.13-rc, it should be easy enough to revert
> these changes and try again during the next cycle.  I honestly don't see any
> benefit from waiting for the next cycle "just in case".

OK.


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

[PATCHv3 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

Hi

Third version of the set that changes I2C and SPI slave device names
to be generated from stable ACPI device names on ACPI 5 based systems
instead of using bus numbers which could change.

Slave device name change goes as

	"x-00yz" -> "i2c-INTABCD:ij"
	"spix.y" -> "spi-INTABCD:ij"

This version is actually a bit cleaner thanks to Rafael's
"ACPI / driver core: Store an ACPI device pointer in struct acpi_dev_node"
in git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm

Set goes on top of linux-pm/linux-next due dependency to 1/3. As we are now
in merge-window I'm fine if 1/3 goes in first and I queue 2-3/3 later
to i2c and spi trees.

Jarkko Nikula (3):
  ACPI: Provide struct acpi_device stub for !CONFIG_ACPI builds
  i2c: Use stable dev_name for ACPI enumerated I2C slaves
  spi: Use stable dev_name for ACPI enumerated SPI slaves

 drivers/i2c/i2c-core.c | 21 +++++++++++++++++----
 drivers/spi/spi.c      | 17 ++++++++++++++---
 include/linux/acpi.h   |  4 ++++
 3 files changed, 35 insertions(+), 7 deletions(-)

-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv3 1/3] ACPI: Provide struct acpi_device stub for !CONFIG_ACPI builds

[Jarkko Nikula]

We have a few cases where we want to access struct device dev field in
struct acpi_device from generic code that is build also without CONFIG_ACPI.

Provide here a minimal struct acpi_device stub that allows to build such a
code without adding new #ifdef CONFIG_ACPI churn. This should not increase
section sizes if code is protected by ACPI_COMPANION() runtime checks as
those will be optimized out by later compiler stages in case CONFIG_ACPI is
not set.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
Rafael, instead of this we could also have an accessor but adev->dev looked
better in actual code and size vmlinux didn't change for x86_64_defconfig
with CONFIG_ACPI is not set nor for omap2plus_defconfig.
---
 include/linux/acpi.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0da49ed..df444e1 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -414,6 +414,10 @@ static inline bool acpi_driver_match_device(struct device *dev,
 
 #else	/* !CONFIG_ACPI */
 
+struct acpi_device {
+	struct device dev;
+};
+
 #define acpi_disabled 1
 
 #define ACPI_COMPANION(dev)		(NULL)
-- 
1.8.4.2

Re: [alsa-devel] [PATCHv3 1/3] ACPI: Provide struct acpi_device stub for !CONFIG_ACPI builds

[Takashi Iwai]

At Thu, 14 Nov 2013 11:00:59 +0200,
Jarkko Nikula wrote:
> 
> We have a few cases where we want to access struct device dev field in
> struct acpi_device from generic code that is build also without CONFIG_ACPI.
> 
> Provide here a minimal struct acpi_device stub that allows to build such a
> code without adding new #ifdef CONFIG_ACPI churn. This should not increase
> section sizes if code is protected by ACPI_COMPANION() runtime checks as
> those will be optimized out by later compiler stages in case CONFIG_ACPI is
> not set.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
> Rafael, instead of this we could also have an accessor but adev->dev looked
> better in actual code and size vmlinux didn't change for x86_64_defconfig
> with CONFIG_ACPI is not set nor for omap2plus_defconfig.

This looks too hackish, IMO.  Defining a different dummy struct often
gives more headaches.  Thinking of the potential risk by misuses, it'd
be simpler and safer to define a macro like acpi_dev_name() instead.

BTW, is linux/device.h already included in !CONFIG_ACPI case?


Takashi

> ---
>  include/linux/acpi.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 0da49ed..df444e1 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -414,6 +414,10 @@ static inline bool acpi_driver_match_device(struct device *dev,
>  
>  #else	/* !CONFIG_ACPI */
>  
> +struct acpi_device {
> +	struct device dev;
> +};
> +
>  #define acpi_disabled 1
>  
>  #define ACPI_COMPANION(dev)		(NULL)
> -- 
> 1.8.4.2
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [alsa-devel] [PATCHv3 1/3] ACPI: Provide struct acpi_device stub for !CONFIG_ACPI builds

[Jarkko Nikula]

On 11/14/2013 12:03 PM, Takashi Iwai wrote:
> At Thu, 14 Nov 2013 11:00:59 +0200,
> Jarkko Nikula wrote:
>> We have a few cases where we want to access struct device dev field in
>> struct acpi_device from generic code that is build also without CONFIG_ACPI.
>>
>> Provide here a minimal struct acpi_device stub that allows to build such a
>> code without adding new #ifdef CONFIG_ACPI churn. This should not increase
>> section sizes if code is protected by ACPI_COMPANION() runtime checks as
>> those will be optimized out by later compiler stages in case CONFIG_ACPI is
>> not set.
>>
>> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>> ---
>> Rafael, instead of this we could also have an accessor but adev->dev looked
>> better in actual code and size vmlinux didn't change for x86_64_defconfig
>> with CONFIG_ACPI is not set nor for omap2plus_defconfig.
> This looks too hackish, IMO.  Defining a different dummy struct often
> gives more headaches.  Thinking of the potential risk by misuses, it'd
> be simpler and safer to define a macro like acpi_dev_name() instead.
>
> BTW, is linux/device.h already included in !CONFIG_ACPI case?
>
It is included unconditionally in include/linux/acpi.h.

Your acpi_dev_name() proposal may be good enough for now as adev->dev 
access in generic code (now under CONFIG_ACPI) seems to be anyway around 
dev_name, using only pointer to struct acpi_device or has more things 
that prevents immediate #ifdef CONFIG_ACPI removal there.

But one problem at time. I'll cook a version with acpi_dev_name.

-- 
Jarkko

[PATCHv3 2/3] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Current I2C adapter id - client address "x-00yy" based device naming scheme
is not always stable enough to be used in name based matching, for instance
within ALSA SoC subsystem.

This is problematic in PC kind of platforms where I2C adapter numbers can
change due variable amount of bus controllers, probe order, add-on cards or
just because of BIOS settings.

This patch addresses the problem by using the ACPI device name with
"i2c-" prefix for ACPI enumerated I2C slaves. For them device name
"x-00yz" becomes "i2c-INTABCD:ij" after this patch.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/i2c-core.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index f74af33..b8f2c32 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -618,6 +618,22 @@ 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", dev_name(&adev->dev));
+		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
@@ -676,10 +692,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->dev.of_node = info->of_node;
 	ACPI_COMPANION_SET(&client->dev, info->acpi_node.companion);
 
-	/* 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_dev_set_name(adap, client);
 	status = device_register(&client->dev);
 	if (status)
 		goto out_err;
-- 
1.8.4.2

[PATCHv3 3/3] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Jarkko Nikula]

Current spi bus_num.chip_select "spix.y" based device naming scheme may not
be stable enough to be used in name based matching, for instance within
ALSA SoC subsystem.

This can be problem in PC kind of platforms if there are changes in SPI bus
configuration, amount of busses or probe order.

This patch addresses the problem by using the ACPI device name with
"spi-" prefix for ACPI enumerated SPI slave. For them device name
"spix.y" becomes "spi-INTABCD:ij".

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/spi/spi.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index af4f971..d9140bb 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -352,6 +352,19 @@ struct spi_device *spi_alloc_device(struct spi_master *master)
 }
 EXPORT_SYMBOL_GPL(spi_alloc_device);
 
+static void spi_dev_set_name(struct spi_device *spi)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&spi->dev);
+
+	if (adev) {
+		dev_set_name(&spi->dev, "spi-%s", dev_name(&adev->dev));
+		return;
+	}
+
+	dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->master->dev),
+		     spi->chip_select);
+}
+
 /**
  * spi_add_device - Add spi_device allocated with spi_alloc_device
  * @spi: spi_device to register
@@ -378,9 +391,7 @@ int spi_add_device(struct spi_device *spi)
 	}
 
 	/* Set the bus ID string */
-	dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->master->dev),
-			spi->chip_select);
-
+	spi_dev_set_name(spi);
 
 	/* We need to make sure there's no other device with this
 	 * chipselect **BEFORE** we call setup(), else we'll trash
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv3 3/3] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Mark Brown]

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

On Thu, Nov 14, 2013 at 11:01:01AM +0200, Jarkko Nikula wrote:
> Current spi bus_num.chip_select "spix.y" based device naming scheme may not
> be stable enough to be used in name based matching, for instance within
> ALSA SoC subsystem.

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

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

[PATCHv4 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Jarkko Nikula]

v4:
This version adds and uses acpi_dev_name() accessor instead defining
struct acpi_device stub for !CONFIG_ACPI 

v3:
Third version of the set that changes I2C and SPI slave device names
to be generated from stable ACPI device names on ACPI 5 based systems
instead of using bus numbers which could change.

Slave device name change goes as

	"x-00yz" -> "i2c-INTABCD:ij"
	"spix.y" -> "spi-INTABCD:ij"

This version is actually a bit cleaner thanks to Rafael's
"ACPI / driver core: Store an ACPI device pointer in struct acpi_dev_node"
in git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm

Set goes on top of linux-pm/linux-next due dependency to 1/3. As we are now
in merge-window I'm fine if 1/3 goes in first and I queue 2-3/3 later
to i2c and spi trees.

Jarkko Nikula (3):
  ACPI: Provide acpi_dev_name accessor for struct acpi_device device
    name
  i2c: Use stable dev_name for ACPI enumerated I2C slaves
  spi: Use stable dev_name for ACPI enumerated SPI slaves

 drivers/i2c/i2c-core.c | 21 +++++++++++++++++----
 drivers/spi/spi.c      | 17 ++++++++++++++---
 include/linux/acpi.h   | 10 ++++++++++
 3 files changed, 41 insertions(+), 7 deletions(-)

-- 
1.8.4.2

[PATCHv4 1/3] ACPI: Provide acpi_dev_name accessor for struct acpi_device device name

[Jarkko Nikula]

struct acpi_device fields are only available when CONFIG_ACPI is set. We may
find use for dev_name(&adev->dev) in generic code that is build also without
CONFIG_ACPI is set but currently this requires #ifdef CONFIG_ACPI churn.

Provide here an accessor that returns dev_name of embedded struct device dev
in struct acpi_device or NULL depending on CONFIG_ACPI setting.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 include/linux/acpi.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0da49ed..844e7ff 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -53,6 +53,11 @@ static inline acpi_handle acpi_device_handle(struct acpi_device *adev)
 #define ACPI_COMPANION_SET(dev, adev)	ACPI_COMPANION(dev) = (adev)
 #define ACPI_HANDLE(dev)		acpi_device_handle(ACPI_COMPANION(dev))
 
+static inline const char *acpi_dev_name(struct acpi_device *adev)
+{
+	return dev_name(&adev->dev);
+}
+
 enum acpi_irq_model_id {
 	ACPI_IRQ_MODEL_PIC = 0,
 	ACPI_IRQ_MODEL_IOAPIC,
@@ -420,6 +425,11 @@ static inline bool acpi_driver_match_device(struct device *dev,
 #define ACPI_COMPANION_SET(dev, adev)	do { } while (0)
 #define ACPI_HANDLE(dev)		(NULL)
 
+static inline const char *acpi_dev_name(struct acpi_device *adev)
+{
+	return NULL;
+}
+
 static inline void acpi_early_init(void) { }
 
 static inline int early_acpi_boot_init(void)
-- 
1.8.4.2

[PATCHv4 2/3] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Jarkko Nikula]

Current I2C adapter id - client address "x-00yy" based device naming scheme
is not always stable enough to be used in name based matching, for instance
within ALSA SoC subsystem.

This is problematic in PC kind of platforms where I2C adapter numbers can
change due variable amount of bus controllers, probe order, add-on cards or
just because of BIOS settings.

This patch addresses the problem by using the ACPI device name with
"i2c-" prefix for ACPI enumerated I2C slaves. For them device name
"x-00yz" becomes "i2c-INTABCD:ij" after this patch.

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/i2c/i2c-core.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index f74af33..bc9a294 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -618,6 +618,22 @@ 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
@@ -676,10 +692,7 @@ i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->dev.of_node = info->of_node;
 	ACPI_COMPANION_SET(&client->dev, info->acpi_node.companion);
 
-	/* 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_dev_set_name(adap, client);
 	status = device_register(&client->dev);
 	if (status)
 		goto out_err;
-- 
1.8.4.2

Re: [PATCHv4 2/3] i2c: Use stable dev_name for ACPI enumerated I2C slaves

[Wolfram Sang]

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

On Thu, Nov 14, 2013 at 02:03:52PM +0200, Jarkko Nikula wrote:
> Current I2C adapter id - client address "x-00yy" based device naming scheme
> is not always stable enough to be used in name based matching, for instance
> within ALSA SoC subsystem.
> 
> This is problematic in PC kind of platforms where I2C adapter numbers can
> change due variable amount of bus controllers, probe order, add-on cards or
> just because of BIOS settings.
> 
> This patch addresses the problem by using the ACPI device name with
> "i2c-" prefix for ACPI enumerated I2C slaves. For them device name
> "x-00yz" becomes "i2c-INTABCD:ij" after this patch.
> 
> Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Acked-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>


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

[PATCHv4 3/3] spi: Use stable dev_name for ACPI enumerated SPI slaves

[Jarkko Nikula]

Current spi bus_num.chip_select "spix.y" based device naming scheme may not
be stable enough to be used in name based matching, for instance within
ALSA SoC subsystem.

This can be problem in PC kind of platforms if there are changes in SPI bus
configuration, amount of busses or probe order.

This patch addresses the problem by using the ACPI device name with
"spi-" prefix for ACPI enumerated SPI slave. For them device name
"spix.y" becomes "spi-INTABCD:ij".

Signed-off-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Acked-by: Mark Brown <broonie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Mark's ack added as diff between v3 and v4 is just:
-		dev_set_name(&spi->dev, "spi-%s", dev_name(&adev->dev));
+		dev_set_name(&spi->dev, "spi-%s", acpi_dev_name(adev));
---
 drivers/spi/spi.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index af4f971..be619e0 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -352,6 +352,19 @@ struct spi_device *spi_alloc_device(struct spi_master *master)
 }
 EXPORT_SYMBOL_GPL(spi_alloc_device);
 
+static void spi_dev_set_name(struct spi_device *spi)
+{
+	struct acpi_device *adev = ACPI_COMPANION(&spi->dev);
+
+	if (adev) {
+		dev_set_name(&spi->dev, "spi-%s", acpi_dev_name(adev));
+		return;
+	}
+
+	dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->master->dev),
+		     spi->chip_select);
+}
+
 /**
  * spi_add_device - Add spi_device allocated with spi_alloc_device
  * @spi: spi_device to register
@@ -378,9 +391,7 @@ int spi_add_device(struct spi_device *spi)
 	}
 
 	/* Set the bus ID string */
-	dev_set_name(&spi->dev, "%s.%u", dev_name(&spi->master->dev),
-			spi->chip_select);
-
+	spi_dev_set_name(spi);
 
 	/* We need to make sure there's no other device with this
 	 * chipselect **BEFORE** we call setup(), else we'll trash
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 0/3] I2C and SPI dev_name change for ACPI enumerated slaves

[Rafael J. Wysocki]

On Thursday, November 14, 2013 02:03:50 PM Jarkko Nikula wrote:
> v4:
> This version adds and uses acpi_dev_name() accessor instead defining
> struct acpi_device stub for !CONFIG_ACPI 
> 
> v3:
> Third version of the set that changes I2C and SPI slave device names
> to be generated from stable ACPI device names on ACPI 5 based systems
> instead of using bus numbers which could change.
> 
> Slave device name change goes as
> 
> 	"x-00yz" -> "i2c-INTABCD:ij"
> 	"spix.y" -> "spi-INTABCD:ij"
> 
> This version is actually a bit cleaner thanks to Rafael's
> "ACPI / driver core: Store an ACPI device pointer in struct acpi_dev_node"
> in git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
> 
> Set goes on top of linux-pm/linux-next due dependency to 1/3. As we are now
> in merge-window I'm fine if 1/3 goes in first and I queue 2-3/3 later
> to i2c and spi trees.
> 
> Jarkko Nikula (3):
>   ACPI: Provide acpi_dev_name accessor for struct acpi_device device
>     name
>   i2c: Use stable dev_name for ACPI enumerated I2C slaves
>   spi: Use stable dev_name for ACPI enumerated SPI slaves

All three queued up for the next ACPI pull request, thanks!

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.