[PATCH] em28xx: Make a few drxk_config structs static

[PATCH] em28xx: Make a few drxk_config structs static

[Ezequiel Garcia]

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/video/em28xx/em28xx-dvb.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/video/em28xx/em28xx-dvb.c b/drivers/media/video/em28xx/em28xx-dvb.c
index 16410ac..241a71a 100644
--- a/drivers/media/video/em28xx/em28xx-dvb.c
+++ b/drivers/media/video/em28xx/em28xx-dvb.c
@@ -310,14 +310,14 @@ static struct drxd_config em28xx_drxd = {
 	.disable_i2c_gate_ctrl = 1,
 };
 
-struct drxk_config terratec_h5_drxk = {
+static struct drxk_config terratec_h5_drxk = {
 	.adr = 0x29,
 	.single_master = 1,
 	.no_i2c_bridge = 1,
 	.microcode_name = "dvb-usb-terratec-h5-drxk.fw",
 };
 
-struct drxk_config hauppauge_930c_drxk = {
+static struct drxk_config hauppauge_930c_drxk = {
 	.adr = 0x29,
 	.single_master = 1,
 	.no_i2c_bridge = 1,
@@ -325,13 +325,13 @@ struct drxk_config hauppauge_930c_drxk = {
 	.chunk_size = 56,
 };
 
-struct drxk_config maxmedia_ub425_tc_drxk = {
+static struct drxk_config maxmedia_ub425_tc_drxk = {
 	.adr = 0x29,
 	.single_master = 1,
 	.no_i2c_bridge = 1,
 };
 
-struct drxk_config pctv_520e_drxk = {
+static struct drxk_config pctv_520e_drxk = {
 	.adr = 0x29,
 	.single_master = 1,
 	.microcode_name = "dvb-demod-drxk-pctv.fw",
-- 
1.7.3.4

[PATCH] em28xx: Remove useless runtime->private_data usage

[Ezequiel Garcia]

Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/video/em28xx/em28xx-audio.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/em28xx/em28xx-audio.c b/drivers/media/video/em28xx/em28xx-audio.c
index d7e2a3d..2fcddae 100644
--- a/drivers/media/video/em28xx/em28xx-audio.c
+++ b/drivers/media/video/em28xx/em28xx-audio.c
@@ -305,7 +305,6 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream)
 
 	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
 	dev->adev.capture_pcm_substream = substream;
-	runtime->private_data = dev;
 
 	return 0;
 err:
-- 
1.7.3.4

Re: [PATCH] em28xx: Remove useless runtime->private_data usage

[Mauro Carvalho Chehab]

Em 12-06-2012 10:53, Ezequiel Garcia escreveu:
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>   drivers/media/video/em28xx/em28xx-audio.c |    1 -
>   1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/em28xx/em28xx-audio.c b/drivers/media/video/em28xx/em28xx-audio.c
> index d7e2a3d..2fcddae 100644
> --- a/drivers/media/video/em28xx/em28xx-audio.c
> +++ b/drivers/media/video/em28xx/em28xx-audio.c
> @@ -305,7 +305,6 @@ static int snd_em28xx_capture_open(struct snd_pcm_substream *substream)
>   
>   	snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
>   	dev->adev.capture_pcm_substream = substream;
> -	runtime->private_data = dev;


Are you sure that this can be removed? I think this is used internally
by the alsa API, but maybe something has changed and this is not
required anymore.

Had you test em28xx audio with this change?

Regards,
Mauro

Re: [PATCH] em28xx: Remove useless runtime->private_data usage

[Ezequiel Garcia]

Hi Mauro,

On Thu, Jul 5, 2012 at 1:57 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:

>>
>>       snd_pcm_hw_constraint_integer(runtime, SNDRV_PCM_HW_PARAM_PERIODS);
>>       dev->adev.capture_pcm_substream = substream;
>> -     runtime->private_data = dev;
>
>
> Are you sure that this can be removed? I think this is used internally
> by the alsa API, but maybe something has changed and this is not
> required anymore.

Yes, I'm sure.

>
> Had you test em28xx audio with this change?

No, I did not test it.

To make this patch, I've considered two things:

1. Alsa documentation [1]
This is from chapter 5, "Private Data" section.

---
You can allocate a record for the substream and store it in
runtime->private_data. Usually, this is done in the open callback.
Don't mix this with pcm->private_data. The pcm->private_data usually
points to the chip instance assigned statically at the creation of
PCM, while the runtime->private_data points to a dynamic data
structure created at the PCM open callback.

  static int snd_xxx_open(struct snd_pcm_substream *substream)
  {
          struct my_pcm_data *data;
          ....
          data = kmalloc(sizeof(*data), GFP_KERNEL);
          substream->runtime->private_data = data;
          ....
  }
---

I think the part "Don't mix this with pcm->private_data", is the one
related to this case.
Also, what alsa documentation calls "chip instance" is our em28xx
device structure.

2. Regular kernel practice:
Normally, private_data fields are suppose to be (private) data the
driver author wants
the core subsystem to pass him as callback parameter. The core
subsystem is not supposed
to use it in anyway (he wouldn't know how).
So, if you don't use it anywhere else in your code, it's safe to remove it.

If still in doubt, just don't apply it.

I'm not really concerned about one extra line,
rather about drivers doing unnecessary stuff,
and then others take these drivers as example
and spread the bloatness all over the place, so to speak.

[1] http://www.alsa-project.org/~tiwai/writing-an-alsa-driver/ch05s05.html

Re: [PATCH] em28xx: Remove useless runtime->private_data usage

[Ezequiel Garcia]

Mauro,

On Thu, Jul 5, 2012 at 2:22 PM, Ezequiel Garcia >> Are you sure that
this can be removed? I think this is used internally
>> by the alsa API, but maybe something has changed and this is not
>> required anymore.
>
> Yes, I'm sure.
>

This should be: "I'm almost sure" :-)
Anyway, probably the patch should have a more verbose commit
message, right?

Do you want to do drop it entirely?

Regards,
Ezequiel.

Re: [PATCH] em28xx: Remove useless runtime->private_data usage

[Mauro Carvalho Chehab]

Em 06-07-2012 11:33, Ezequiel Garcia escreveu:
> Mauro,
> 
> On Thu, Jul 5, 2012 at 2:22 PM, Ezequiel Garcia >> Are you sure that
> this can be removed? I think this is used internally
>>> by the alsa API, but maybe something has changed and this is not
>>> required anymore.
>>
>> Yes, I'm sure.
>>
> 
> This should be: "I'm almost sure" :-)
> Anyway, probably the patch should have a more verbose commit
> message, right?

Yeah, that would be good.

> Do you want to do drop it entirely?

No, but, as I'm taking a 2-week vacations starting next week, I'll postpone
those "compiled-only" cleanup patches to apply after my return, probably
holding them to be applied on 3.6.

Regards,
Mauro

Re: [PATCH] em28xx: Remove useless runtime->private_data usage

[Ezequiel Garcia]

On Fri, Jul 6, 2012 at 12:12 PM, Mauro Carvalho Chehab
<mchehab@redhat.com> wrote:
> Em 06-07-2012 11:33, Ezequiel Garcia escreveu:
>> Mauro,
>>
>> On Thu, Jul 5, 2012 at 2:22 PM, Ezequiel Garcia >> Are you sure that
>> this can be removed? I think this is used internally
>>>> by the alsa API, but maybe something has changed and this is not
>>>> required anymore.
>>>
>>> Yes, I'm sure.
>>>
>>
>> This should be: "I'm almost sure" :-)
>> Anyway, probably the patch should have a more verbose commit
>> message, right?
>
> Yeah, that would be good.
>
>> Do you want to do drop it entirely?
>
> No, but, as I'm taking a 2-week vacations starting next week, I'll postpone
> those "compiled-only" cleanup patches to apply after my return, probably
> holding them to be applied on 3.6.
>

Okey.

Have a nice holiday!
Ezequiel.