[PATCH] video: mx3_camera: Allocate camera object via kzalloc

[PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Fabio Estevam]

Align mx3_camera driver with the other soc camera driver implementations
by allocating the camera object via kzalloc.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/media/video/mx3_camera.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index 7452277..cccd574 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -1159,7 +1159,7 @@ static int __devinit mx3_camera_probe(struct platform_device *pdev)
 		goto egetres;
 	}
 
-	mx3_cam = vzalloc(sizeof(*mx3_cam));
+	mx3_cam = kzalloc(sizeof(*mx3_cam), GFP_KERNEL);
 	if (!mx3_cam) {
 		dev_err(&pdev->dev, "Could not allocate mx3 camera object\n");
 		err = -ENOMEM;
-- 
1.7.1

Re: [PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Guennadi Liakhovetski]

On Mon, 20 Feb 2012, Fabio Estevam wrote:

> Align mx3_camera driver with the other soc camera driver implementations
> by allocating the camera object via kzalloc.

Sorry, any specific reason, why you think this "aligning" is so important? 
I personally don't see any.

Thanks
Guennadi

> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  drivers/media/video/mx3_camera.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
> index 7452277..cccd574 100644
> --- a/drivers/media/video/mx3_camera.c
> +++ b/drivers/media/video/mx3_camera.c
> @@ -1159,7 +1159,7 @@ static int __devinit mx3_camera_probe(struct platform_device *pdev)
>  		goto egetres;
>  	}
>  
> -	mx3_cam = vzalloc(sizeof(*mx3_cam));
> +	mx3_cam = kzalloc(sizeof(*mx3_cam), GFP_KERNEL);
>  	if (!mx3_cam) {
>  		dev_err(&pdev->dev, "Could not allocate mx3 camera object\n");
>  		err = -ENOMEM;
> -- 
> 1.7.1
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Fabio Estevam]

On Mon, Feb 20, 2012 at 4:17 PM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> On Mon, 20 Feb 2012, Fabio Estevam wrote:
>
>> Align mx3_camera driver with the other soc camera driver implementations
>> by allocating the camera object via kzalloc.
>
> Sorry, any specific reason, why you think this "aligning" is so important?

Not really.

Just compared it with all other soc camera drivers I found and
mx3_camera was the only one that uses "vzalloc"

Any specific reason that requires mx3_camera to use "vzalloc" instead
of "kzalloc"?

Tested with kzalloc and it worked fine on my mx31pdk.

Regards,

Fabio Estevam

Re: [PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Mauro Carvalho Chehab]

Em 20-02-2012 16:23, Fabio Estevam escreveu:
> On Mon, Feb 20, 2012 at 4:17 PM, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
>> On Mon, 20 Feb 2012, Fabio Estevam wrote:
>>
>>> Align mx3_camera driver with the other soc camera driver implementations
>>> by allocating the camera object via kzalloc.
>>
>> Sorry, any specific reason, why you think this "aligning" is so important?
> 
> Not really.
> 
> Just compared it with all other soc camera drivers I found and
> mx3_camera was the only one that uses "vzalloc"
> 
> Any specific reason that requires mx3_camera to use "vzalloc" instead
> of "kzalloc"?

kzalloc() is more restrictive than vzalloc(). With v*alloc, it will allocate
a virtual memory area, with can be discontinuous, while kzalloc will get
a continuous area.

The DMA logic need to be prepared for virtual memory, if v*alloc() is used
(e. g. using videobuf2-vmalloc).

As it is currently including media/videobuf2-dma-contig.h, I this patch
makes sense on my eyes.

> 
> Tested with kzalloc and it worked fine on my mx31pdk.

If the driver is working with vzalloc, then maybe it is due to some arch-specific
implementation for v*alloc. It shouldn't be working like that.

Regards,
Mauro
> 
> Regards,
> 
> Fabio Estevam
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Guennadi Liakhovetski]

On Mon, 19 Mar 2012, Mauro Carvalho Chehab wrote:

> Em 20-02-2012 16:23, Fabio Estevam escreveu:
> > On Mon, Feb 20, 2012 at 4:17 PM, Guennadi Liakhovetski
> > <g.liakhovetski@gmx.de> wrote:
> >> On Mon, 20 Feb 2012, Fabio Estevam wrote:
> >>
> >>> Align mx3_camera driver with the other soc camera driver implementations
> >>> by allocating the camera object via kzalloc.
> >>
> >> Sorry, any specific reason, why you think this "aligning" is so important?
> > 
> > Not really.
> > 
> > Just compared it with all other soc camera drivers I found and
> > mx3_camera was the only one that uses "vzalloc"
> > 
> > Any specific reason that requires mx3_camera to use "vzalloc" instead
> > of "kzalloc"?
> 
> kzalloc() is more restrictive than vzalloc(). With v*alloc, it will allocate
> a virtual memory area, with can be discontinuous, while kzalloc will get
> a continuous area.
> 
> The DMA logic need to be prepared for virtual memory, if v*alloc() is used
> (e. g. using videobuf2-vmalloc).
> 
> As it is currently including media/videobuf2-dma-contig.h, I this patch
> makes sense on my eyes.

Don't think so. vzalloc() is used in mx3_camera to allocate driver private 
data objects and are never used for DMA, so, it doesn't have any 
restrictions on contiguity, coherency, alignment etc.

One could argue, that since the struct is anyway smaller than 1 page, it 
anyway will be allocated in a physically contiguous memory area (will it?) 
and so, maybe, kmalloc() is less heavy weight than vmalloc() and might 
save a couple of CPU cycles, but I don't think it's anything important, 
that we should be optimising for.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Fabio Estevam]

Hi Guennadi,

On 3/20/12, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> Don't think so. vzalloc() is used in mx3_camera to allocate driver private
> data objects and are never used for DMA, so, it doesn't have any
> restrictions on contiguity, coherency, alignment etc.

Is this valid only for mx3_camera driver?

All other soc camera drivers use kzalloc.

What makes mx3_camera different in this respect?

Re: [PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Guennadi Liakhovetski]

On Tue, 20 Mar 2012, Fabio Estevam wrote:

> Hi Guennadi,
> 
> On 3/20/12, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> 
> > Don't think so. vzalloc() is used in mx3_camera to allocate driver private
> > data objects and are never used for DMA, so, it doesn't have any
> > restrictions on contiguity, coherency, alignment etc.
> 
> Is this valid only for mx3_camera driver?

No

> All other soc camera drivers use kzalloc.
> 
> What makes mx3_camera different in this respect?

Nothing

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Fabio Estevam]

On 3/20/12, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

>> Is this valid only for mx3_camera driver?
>
> No
>
>> All other soc camera drivers use kzalloc.
>>
>> What makes mx3_camera different in this respect?
>
> Nothing

Ok, so isn't my patch correct then?

Re: [PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Guennadi Liakhovetski]

On Tue, 20 Mar 2012, Fabio Estevam wrote:

> On 3/20/12, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> 
> >> Is this valid only for mx3_camera driver?
> >
> > No
> >
> >> All other soc camera drivers use kzalloc.
> >>
> >> What makes mx3_camera different in this respect?
> >
> > Nothing
> 
> Ok, so isn't my patch correct then?

No. It doesn't improve anything.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

AverTV Volar HD PRO : a return case.

[Sril]

Hi,
I acquire this usb stick 07ca:a835 and it's still does not work properly.

Now, with the af9035 patch from 
http://openpli.git.sourceforge.net/git/gitweb.cgi?p=openpli/openembedded;a=blob_plain;f=recipes/linux/linux-etxx00/dvb-usb-af9035.patch;hb=HEAD

the tv interface is recognize but have trouble with kaffeine, tvtime and gnome-dvb-daemon.

Here's a trace from gnome : "af9033: I2C read failed reg:0047".
>From kaffeine : "kaffeine(3817) DvbDevice::frontendEvent: tuning failed".
>From tvtime, nothing card doesn't appear : probably missing conf, it's ok.

This message try to follow Andrej Podzimekand Gianluca Gennari's messages on 02/07/2012.

Does someone got ideas about what to do to correct this ?

kernel : 3.2.11 with patch noticed. No externe v4l at all during construct. Compile fine :

Mar 18 16:09:43 localhost kernel: [  305.726981] dvb-usb: found a 'Avermedia AverTV Volar HD & HD PRO (A835)' in cold state, will try to load a firmware
Mar 18 16:09:43 localhost kernel: [  305.742050] dvb-usb: downloading firmware from file 'dvb-usb-af9035-01.fw'
Mar 18 16:09:43 localhost kernel: [  306.039886] dvb-usb: found a 'Avermedia AverTV Volar HD & HD PRO (A835)' in warm state.
Mar 18 16:09:43 localhost kernel: [  306.040032] dvb-usb: will pass the complete MPEG2 transport stream to the software demuxer.
Mar 18 16:09:43 localhost kernel: [  306.040406] DVB: registering new adapter (Avermedia AverTV Volar HD & HD PRO (A835))
Mar 18 16:09:43 localhost kernel: [  306.078104] af9033: firmware version: LINK:11.15.10.0 OFDM:5.48.10.0
Mar 18 16:09:43 localhost kernel: [  306.080355] DVB: registering adapter 0 frontend 0 (Afatech AF9033 DVB-T)...
Mar 18 16:09:43 localhost kernel: [  306.129981] tda18218: NXP TDA18218HN successfully identified.
Mar 18 16:09:43 localhost kernel: [  306.131483] dvb-usb: Avermedia AverTV Volar HD & HD PRO (A835) successfully initialized and connected.
Mar 18 16:09:43 localhost kernel: [  306.140531] usbcore: registered new interface driver dvb_usb_af9035

Best regards.
See ya.

Re: [PATCH] video: mx3_camera: Allocate camera object via kzalloc

[Mauro Carvalho Chehab]

Em 20-03-2012 04:57, Guennadi Liakhovetski escreveu:
> On Mon, 19 Mar 2012, Mauro Carvalho Chehab wrote:
> 
>> Em 20-02-2012 16:23, Fabio Estevam escreveu:
>>> On Mon, Feb 20, 2012 at 4:17 PM, Guennadi Liakhovetski
>>> <g.liakhovetski@gmx.de> wrote:
>>>> On Mon, 20 Feb 2012, Fabio Estevam wrote:
>>>>
>>>>> Align mx3_camera driver with the other soc camera driver implementations
>>>>> by allocating the camera object via kzalloc.
>>>>
>>>> Sorry, any specific reason, why you think this "aligning" is so important?
>>>
>>> Not really.
>>>
>>> Just compared it with all other soc camera drivers I found and
>>> mx3_camera was the only one that uses "vzalloc"
>>>
>>> Any specific reason that requires mx3_camera to use "vzalloc" instead
>>> of "kzalloc"?
>>
>> kzalloc() is more restrictive than vzalloc(). With v*alloc, it will allocate
>> a virtual memory area, with can be discontinuous, while kzalloc will get
>> a continuous area.
>>
>> The DMA logic need to be prepared for virtual memory, if v*alloc() is used
>> (e. g. using videobuf2-vmalloc).
>>
>> As it is currently including media/videobuf2-dma-contig.h, I this patch
>> makes sense on my eyes.
> 
> Don't think so. vzalloc() is used in mx3_camera to allocate driver private 
> data objects and are never used for DMA, so, it doesn't have any 
> restrictions on contiguity, coherency, alignment etc.

OK. In this case, using v*alloc()/vfree() won't be that different than k*alloc()/k*free().

> One could argue, that since the struct is anyway smaller than 1 page, it 
> anyway will be allocated in a physically contiguous memory area (will it?) 
> and so, maybe, kmalloc() is less heavy weight than vmalloc() and might 
> save a couple of CPU cycles, but I don't think it's anything important, 
> that we should be optimising for.

Yes. 

There's another aspect to consider there: the vmalloc space is limited
(there's a boot time parameter to regulate its size)[1]. I dunno why, nor
what are the consequences of using a bigger value, but I think a big vmalloc
size creates a big table to map between virtual/physical memory space.

Yet, a single page is far below the vmaloc default max size,
except if the system has very severe memory constraints.

[1] On x86, I think that the default is 256MB. Several video adapters
eat a lot of space there. I use a bigger value here, otherwise my 3-head
system won't initialize all screens.

Regards,
Mauro