[PATCH 5/7] S3C AUDIO: Add header to pass platform data to device drivers

[PATCH 5/7] S3C AUDIO: Add header to pass platform data to device drivers

[Jassi Brar]

Signed-off-by: Jassi Brar <jassi.brar@samsung.com>
---
 arch/arm/plat-s3c/include/plat/audio.h |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-s3c/include/plat/audio.h

diff --git a/arch/arm/plat-s3c/include/plat/audio.h b/arch/arm/plat-s3c/include/plat/audio.h
new file mode 100644
index 0000000..f22d23b
--- /dev/null
+++ b/arch/arm/plat-s3c/include/plat/audio.h
@@ -0,0 +1,17 @@
+/* arch/arm/plat-s3c/include/plat/audio.h
+ *
+ * Copyright (c) 2009 Samsung Electronics Co. Ltd
+ * Author: Jaswinder Singh <jassi.brar@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/**
+ * struct s3c_audio_pdata - common platform data for audio device drivers
+ * @cfg_gpio: Callback function to setup mux'ed pins in I2S/PCM/AC97 mode
+ */
+struct s3c_audio_pdata {
+	int (*cfg_gpio)(struct platform_device *);
+};
-- 
1.6.2.5

Re: [PATCH 5/7] S3C AUDIO: Add header to pass platform data to device drivers

[Ben Dooks]

On Wed, Nov 11, 2009 at 01:45:22PM +0900, Jassi Brar wrote:

Please put something here! Even if it just a slightly expanded version
of the patch title.

> Signed-off-by: Jassi Brar <jassi.brar@samsung.com>

This could be merged to the alsa tree, we can move it once the other
changes are made.

> ---
>  arch/arm/plat-s3c/include/plat/audio.h |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)
>  create mode 100644 arch/arm/plat-s3c/include/plat/audio.h
> 
> diff --git a/arch/arm/plat-s3c/include/plat/audio.h b/arch/arm/plat-s3c/include/plat/audio.h
> new file mode 100644
> index 0000000..f22d23b
> --- /dev/null
> +++ b/arch/arm/plat-s3c/include/plat/audio.h
> @@ -0,0 +1,17 @@
> +/* arch/arm/plat-s3c/include/plat/audio.h
> + *
> + * Copyright (c) 2009 Samsung Electronics Co. Ltd
> + * Author: Jaswinder Singh <jassi.brar@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +/**
> + * struct s3c_audio_pdata - common platform data for audio device drivers
> + * @cfg_gpio: Callback function to setup mux'ed pins in I2S/PCM/AC97 mode
> + */
> +struct s3c_audio_pdata {
> +	int (*cfg_gpio)(struct platform_device *);
> +};
> -- 
> 1.6.2.5
> 

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [PATCH 5/7] S3C AUDIO: Add header to pass platform data to device drivers

[jassi brar]

On Mon, Nov 16, 2009 at 7:49 PM, Ben Dooks <ben-linux@fluff.org> wrote:
> On Wed, Nov 11, 2009 at 01:45:22PM +0900, Jassi Brar wrote:
>
> Please put something here! Even if it just a slightly expanded version
> of the patch title.
will do. thanks

>> Signed-off-by: Jassi Brar <jassi.brar@samsung.com>
>
> This could be merged to the alsa tree, we can move it once the other
> changes are made.
>
>> ---
>>  arch/arm/plat-s3c/include/plat/audio.h |   17 +++++++++++++++++
>>  1 files changed, 17 insertions(+), 0 deletions(-)
>>  create mode 100644 arch/arm/plat-s3c/include/plat/audio.h
>>
>> diff --git a/arch/arm/plat-s3c/include/plat/audio.h b/arch/arm/plat-s3c/include/plat/audio.h
>> new file mode 100644
>> index 0000000..f22d23b
>> --- /dev/null
>> +++ b/arch/arm/plat-s3c/include/plat/audio.h
>> @@ -0,0 +1,17 @@
>> +/* arch/arm/plat-s3c/include/plat/audio.h
>> + *
>> + * Copyright (c) 2009 Samsung Electronics Co. Ltd
>> + * Author: Jaswinder Singh <jassi.brar@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +/**
>> + * struct s3c_audio_pdata - common platform data for audio device drivers
>> + * @cfg_gpio: Callback function to setup mux'ed pins in I2S/PCM/AC97 mode
>> + */
>> +struct s3c_audio_pdata {
>> +     int (*cfg_gpio)(struct platform_device *);
>> +};
>> --
>> 1.6.2.5
>>
>
> --
> --
> Ben
>
> Q:      What's a light-year?
> A:      One-third less calories than a regular year.
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 5/7] S3C AUDIO: Add header to pass platform data to device drivers.

[Mark Brown]

On Sat, Nov 07, 2009 at 12:31:43PM +0900, jassi brar wrote:

> Well, since the abstraction(dev-audio) is supposed to be common for
> I2S, PCM and AC97 protocol, we need to have another layer which
> communicate protocol specific platform data.
> This pdata is meant to serve that purpose in future.

There's no real abstraction here - it's just a place to put the devices
and whatnot for the various audio related devices.  I would be very
surprised to ever see any need for generic audio platform data, the
devices don't have that much in common.

> > I'd just embed the GPIO configuration callback in a driver-specific
> > platform data.
> If not for pdata, the cfg_gpio wudn't be enough for future SoCs which
> not only configure the MUX'ed pins but also set the drive strength. Some
> might also need configuring the SoC which may not fall into either Clock
> or Power domain.
> So, if I am to remove pdata, maybe cfg_gpio can be renamed config_controller?

I'd be happier if it stayed as GPIO - that'd still cover things like
drive strength.  This is because the platform device code has recently
gained the ability to provide platform specific annotations on the
device itself which would allow this to be done in a way that is
transparent to the edge drivers and avoids code duplication in all the
device drivers.  This is (IIRC) used by the OMAP power domain management
and I'd expect that the Samsung SoCs will end up going down a similar
way in future.  I'd rather try to reduce the amount of device specific
handling for things like that which appears.

Re: [PATCH 5/7] S3C AUDIO: Add header to pass platform data to device drivers.

[jassi brar]

On Sat, Nov 7, 2009 at 1:52 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Nov 05, 2009 at 10:35:00AM +0900, Jassi Brar wrote:
>
>> +/**
>> + * struct s3c_audio_pdata - common platform data for audio device drivers
>> + * @pdata: Pointer to protocol(I2S, PCM or AC97) specific platform data
>> + * @cfg_gpio: Callback function to setup mux'ed pins in I2S/PCM/AC97 mode
>> + */
>> +struct s3c_audio_pdata {
>> +     void *pdata;
>> +
>> +     int (*cfg_gpio)(struct platform_device *);
>> +};
>
> I don't see much benefit in this structure with the pdata embedded in it
> - you just end up with an additional layer of indirection in your
> platform data if you need to add device specific stuff which is annoying
> to specify in machine drivers.
Well, since the abstraction(dev-audio) is supposed to be common for
I2S, PCM and AC97 protocol, we need to have another layer which
communicate protocol specific platform data.
This pdata is meant to serve that purpose in future.

> I'd just embed the GPIO configuration callback in a driver-specific
> platform data.
If not for pdata, the cfg_gpio wudn't be enough for future SoCs which
not only configure the MUX'ed pins but also set the drive strength. Some
might also need configuring the SoC which may not fall into either Clock
or Power domain.
So, if I am to remove pdata, maybe cfg_gpio can be renamed config_controller?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 5/7] S3C AUDIO: Add header to pass platform data to device drivers.

[Mark Brown]

On Thu, Nov 05, 2009 at 10:35:00AM +0900, Jassi Brar wrote:

> +/**
> + * struct s3c_audio_pdata - common platform data for audio device drivers
> + * @pdata: Pointer to protocol(I2S, PCM or AC97) specific platform data
> + * @cfg_gpio: Callback function to setup mux'ed pins in I2S/PCM/AC97 mode
> + */
> +struct s3c_audio_pdata {
> +	void *pdata;
> +
> +	int (*cfg_gpio)(struct platform_device *);
> +};

I don't see much benefit in this structure with the pdata embedded in it
- you just end up with an additional layer of indirection in your
platform data if you need to add device specific stuff which is annoying
to specify in machine drivers.

I'd just embed the GPIO configuration callback in a driver-specific
platform data.

[PATCH 5/7] S3C AUDIO: Add header to pass platform data to device drivers.

[Jassi Brar]

Signed-off-by: Jassi Brar <jassi.brar@samsung.com>
---
 arch/arm/plat-s3c/include/plat/audio.h |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-s3c/include/plat/audio.h

diff --git a/arch/arm/plat-s3c/include/plat/audio.h b/arch/arm/plat-s3c/include/plat/audio.h
new file mode 100644
index 0000000..9b2007c
--- /dev/null
+++ b/arch/arm/plat-s3c/include/plat/audio.h
@@ -0,0 +1,20 @@
+/* arch/arm/plat-s3c/include/plat/audio.h
+ *
+ * Copyright (c) 2009 Samsung Electronics Co. Ltd
+ * Author: Jaswinder Singh <jassi.brar@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/**
+ * struct s3c_audio_pdata - common platform data for audio device drivers
+ * @pdata: Pointer to protocol(I2S, PCM or AC97) specific platform data
+ * @cfg_gpio: Callback function to setup mux'ed pins in I2S/PCM/AC97 mode
+ */
+struct s3c_audio_pdata {
+	void *pdata;
+
+	int (*cfg_gpio)(struct platform_device *);
+};
-- 
1.6.2.5

[PATCH 5/7] S3C AUDIO: Add header to pass platform data to device drivers.

[jassisinghbrar]

From: Jassi Brar <jassi.brar@samsung.com>

Signed-off-by: Jassi Brar <jassi.brar@samsung.com>
---
 arch/arm/plat-s3c/include/plat/audio.h |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-s3c/include/plat/audio.h

diff --git a/arch/arm/plat-s3c/include/plat/audio.h b/arch/arm/plat-s3c/include/plat/audio.h
new file mode 100644
index 0000000..9b2007c
--- /dev/null
+++ b/arch/arm/plat-s3c/include/plat/audio.h
@@ -0,0 +1,20 @@
+/* arch/arm/plat-s3c/include/plat/audio.h
+ *
+ * Copyright (c) 2009 Samsung Electronics Co. Ltd
+ * Author: Jaswinder Singh <jassi.brar@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/**
+ * struct s3c_audio_pdata - common platform data for audio device drivers
+ * @pdata: Pointer to protocol(I2S, PCM or AC97) specific platform data
+ * @cfg_gpio: Callback function to setup mux'ed pins in I2S/PCM/AC97 mode
+ */
+struct s3c_audio_pdata {
+	void *pdata;
+
+	int (*cfg_gpio)(struct platform_device *);
+};
-- 
1.6.2.5