Re: saa7134 and μPD61151 MPEG2 coder

2010-02-15 Thread Dmitri Belimov
Hi

Some fixes

> On Tuesday 09 February 2010 06:41:50 Dmitri Belimov wrote:
> > Hi Hans
> > 
> > This is my last state for review.
> > After small time I'll finish process of initialize the encoder.
> > Configure some register, upload two firmware for video and for
> > audio. Configure the frontends.
> > 
> > I have the questions.
> > For configuring audio frontend need know samplerate of audio.
> > saa7134 can only 32kHz
> > saa7131/3/5 on I2S 32кГц from SIF source and 32/44.1/48 from
> > external i.e. RCA stereo audio input. 
> > 
> > Hardcode 32kHz or need a function for determine mode of audio??
> 
> See struct v4l2_subdev_audio_ops: it has a s_clock_freq op for
> precisely that purpose. The saa7134 should call that whenever it sets
> a new samplerate.

This op is not used in saa7134 sources. Now hardcoded 32kHz.

> > 
> > Other question. For configure VideoFrontend need know 50 or 60Hz
> > Now I use videomode from h structure. I think more correct detect it
> > on saa7134.
> 
> Whether it is 50 or 60 Hz depends on the video standard that you
> receive via the s_std core op. Just implement that and when you get a
> new standard you can use something like this: is_60hz = (std &
> V4L2_STD_525_60) ? 1 : 0;

I do it, if(std & V4L2_STD_625_50) then 

This data is not real information about input signal. It's what we set from 
end-user programm.
But OK, I used it.

> Some more review comments:
> 
> linux/drivers/media/video/saa7134/saa7134.h:
> 
> @@ -355,6 +377,10 @@
> unsigned char   empress_addr;
> unsigned char   rds_addr;
>  
> +   /* SPI info */
> +   struct saa7134_software_spi spi;
> +   struct spi_board_info   spi_conf;
> 
> Make this a struct spi_board_info *. This struct is too large: it is
> only used in one board but all elements of the board array will
> suddenly get this whole struct increasing the memory footprint
> substantially. In this case you can just make it a pointer, that will
> work just as well.

Yes. I don't know what is more good. Now all config data in saa7134-cards 
description.
In your case need move spi_board_info to SPI part of code. We will mix code and 
data.
For add new cards and devices need edit some files for add configs.
May be it's good in my case.

> +
> unsigned inttda9887_conf;
> unsigned inttuner_config;
> 
> linux/drivers/media/video/v4l2-common.c, in v4l2_spi_subdev_init():
> 
> +   /* initialize name */
> +   snprintf(sd->name, sizeof(sd->name), "%s",
> +   spi->dev.driver->name);
> 
> Use strlcpy here.

Done

> saa7134-spi.c:
> 
> 
> static inline u32 getmiso(struct spi_device *dev)
> {
> struct saa7134_spi_gpio *sb = to_sb(dev);
> unsigned long status;
> 
> status = saa7134_get_gpio(sb->controller_data);
> if ( status & (1 << sb->controller_data->spi.miso))
> return 1;
> else
> return 0;
> }
> 
> Simplify to:
> 
> static inline u32 getmiso(struct spi_device *dev)
> {
> struct saa7134_spi_gpio *sb = to_sb(dev);
> u32 status;
> 
> status = saa7134_get_gpio(sb->controller_data);
> return !!(status & (1 << sb->controller_data->spi.miso));
> }

Thank you.

> Also note that saa7134_get_gpio should return an u32 since unsigned
> long is 64 bits when compiled on a 64-bit kernel, which is probably
> not what you want.

Done

> saa7134_spi_unregister can be a void function as the result code is
> always 0.

Done
 
> There seems to be some old stuff in upd61151.h. Please remove what is
> not needed.

Done

> In upd61151.c I highly recommend that all functions will use struct
> v4l2_subdev *sd as argument. Only at the lowest level should you go
> from sd to spi. Among others this allows you to use the standard
> v4l2_info/dbg etc. logging functions.

Done

> Don't use RESULT_SUCCESS. Just return 0.

Done
 
> Remove upd61151_init. The init op is rarely needed and should in
> general not be used.

Done 

> Remove those emacs editor comments at the end of the files. That's
> bad practice.

Done
 
With my best regards, Dmitry.
diff -r b6b82258cf5e linux/drivers/media/video/saa7134/Makefile
--- a/linux/drivers/media/video/saa7134/Makefile	Thu Dec 31 19:14:54 2009 -0200
+++ b/linux/drivers/media/video/saa7134/Makefile	Tue Feb 16 10:23:52 2010 +0900
@@ -1,9 +1,9 @@
 
 saa7134-objs :=	saa7134-cards.o saa7134-core.o saa7134-i2c.o	\
 		saa7134-ts.o saa7134-tvaudio.o saa7134-vbi.o\
-		saa7134-video.o saa7134-input.o
+		saa7134-video.o saa7134-input.o saa7134-spi.o
 
-obj-$(CONFIG_VIDEO_SAA7134) +=  saa6752hs.o saa7134.o saa7134-empress.o
+obj-$(CONFIG_VIDEO_SAA7134) +=  saa6752hs.o saa7134.o saa7134-empress.o upd61151.o
 
 obj-$(CONFIG_VIDEO_SAA7134_ALSA) += saa7134-alsa.o
 
diff -r b6b82258cf5e linux/drivers/media/video/saa7134/saa7134-cards.c
--- a/linux/drivers/media/video/saa7134/saa7134-cards.c	Thu Dec 31 19:14:54 2009 -0200
+++ b/linux/drivers/media/video/sa

Re: saa7134 and μPD61151 MPEG2 coder

2010-02-12 Thread Hans Verkuil
On Tuesday 09 February 2010 06:41:50 Dmitri Belimov wrote:
> Hi Hans
> 
> This is my last state for review.
> After small time I'll finish process of initialize the encoder.
> Configure some register, upload two firmware for video and for audio.
> Configure the frontends.
> 
> I have the questions.
> For configuring audio frontend need know samplerate of audio.
> saa7134 can only 32kHz
> saa7131/3/5 on I2S 32кГц from SIF source and 32/44.1/48 from external i.e.
> RCA stereo audio input. 
> 
> Hardcode 32kHz or need a function for determine mode of audio??

See struct v4l2_subdev_audio_ops: it has a s_clock_freq op for precisely that
purpose. The saa7134 should call that whenever it sets a new samplerate.

> 
> Other question. For configure VideoFrontend need know 50 or 60Hz
> Now I use videomode from h structure. I think more correct detect it
> on saa7134.

Whether it is 50 or 60 Hz depends on the video standard that you receive via
the s_std core op. Just implement that and when you get a new standard you
can use something like this: is_60hz = (std & V4L2_STD_525_60) ? 1 : 0;

Some more review comments:

linux/drivers/media/video/saa7134/saa7134.h:

@@ -355,6 +377,10 @@
unsigned char   empress_addr;
unsigned char   rds_addr;
 
+   /* SPI info */
+   struct saa7134_software_spi spi;
+   struct spi_board_info   spi_conf;

Make this a struct spi_board_info *. This struct is too large: it is only used
in one board but all elements of the board array will suddenly get this whole
struct increasing the memory footprint substantially. In this case you can just
make it a pointer, that will work just as well.

+
unsigned inttda9887_conf;
unsigned inttuner_config;

linux/drivers/media/video/v4l2-common.c, in v4l2_spi_subdev_init():

+   /* initialize name */
+   snprintf(sd->name, sizeof(sd->name), "%s",
+   spi->dev.driver->name);

Use strlcpy here.

saa7134-spi.c:


static inline u32 getmiso(struct spi_device *dev)
{
struct saa7134_spi_gpio *sb = to_sb(dev);
unsigned long status;

status = saa7134_get_gpio(sb->controller_data);
if ( status & (1 << sb->controller_data->spi.miso))
return 1;
else
return 0;
}

Simplify to:

static inline u32 getmiso(struct spi_device *dev)
{
struct saa7134_spi_gpio *sb = to_sb(dev);
u32 status;

status = saa7134_get_gpio(sb->controller_data);
return !!(status & (1 << sb->controller_data->spi.miso));
}

Also note that saa7134_get_gpio should return an u32 since unsigned long is
64 bits when compiled on a 64-bit kernel, which is probably not what you want.

saa7134_spi_unregister can be a void function as the result code is always 0.

There seems to be some old stuff in upd61151.h. Please remove what is not
needed.

In upd61151.c I highly recommend that all functions will use struct v4l2_subdev 
*sd
as argument. Only at the lowest level should you go from sd to spi. Among
others this allows you to use the standard v4l2_info/dbg etc. logging functions.

Don't use RESULT_SUCCESS. Just return 0.

Remove upd61151_init. The init op is rarely needed and should in general not
be used.

Remove those emacs editor comments at the end of the files. That's bad practice.

Regards,

Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: saa7134 and μPD61151 MPEG2 coder

2010-01-28 Thread Dmitri Belimov
Hi Hans

> > > On Wednesday 27 January 2010 06:36:37 Dmitri Belimov wrote:
> > > > Hi Hans.
> > > > 
> > > > I finished saa7134 part of SPI. Please review saa7134-spi.c and
> > > > diff saa7134-core and etc. I wrote config of SPI to board
> > > > structure. Use this config for register master and slave
> > > > devices.
> > > > 
> > > > SPI other then I2C, do not need call request_module. Udev do
> > > > it. I spend 10 days for understanding :(  
> > > 
> > > I'm almost certain that spi works the same way as i2c and that
> > > means that you must call request_module. Yes, udev will load it
> > > for you, but that is a delayed load: i.e. the module may not be
> > > loaded when we need it. The idea behind this is that usually i2c
> > > or spi modules are standalone, but in the context of v4l such
> > > modules are required to be present before the bridge can properly
> > > configure itself.
> > > 
> > > The easiest way to ensure the correct load sequence is to do a
> > > request_module at the start.
> > > 
> > > Now, I haven't compiled this, but I think this will work:
> > > 
> > > struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device
> > > *v4l2_dev, struct spi_master *master, struct spi_board_info *info)
> > > {
> > >   struct v4l2_subdev *sd = NULL;
> > > struct spi_device *spi;
> > >   
> > >   BUG_ON(!v4l2_dev);
> > > 
> > >   if (module_name)
> > >   request_module(module_name);
> 
> There is one thing missing here: module_name should be passed in as
> argument to v4l2_spi_new_subdev. Does this code actually compile? If
> so, then I suspect module_name must be some global variable with some
> bogus value which causes request_module to time out.
> 
> > [  240.476082]  [] ? v4l2_spi_new_subdev_board+0x2e/0x35
> > [v4l2_common] [  240.476086]  [] ?
> > v4l2_spi_new_subdev+0x64/0x6c [v4l2_common]
> 
> Remove v4l2_spi_new_subdev_board. Just have a v4l2_spi_new_subdev as
> in my code.

I found my error. Calling request_module befor spi_new_device is good.

As I see new v4l2_subdev ops hooked and calling
[   52.796008] DEBUG uPD61151: upd61151_s_std

My next task move our MPEG2 code to upd61151.c
Please review my changes for v4l2_spi_new_subdev.

[4.743008] Linux video capture interface: v2.00
[4.788866] saa7130/34: v4l2 driver version 0.2.15 loaded
[4.788946] saa7134 :04:01.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
[4.788996] saa7133[0]: found at :04:01.0, rev: 209, irq: 19, latency: 
32, mmio: 0xe510
[4.789055] saa7133[0]: subsystem: 5ace:7595, board: Beholder BeholdTV X7 
[card=171,autodetected]
[4.789122] saa7133[0]: board init: gpio is 20
[4.789172] IRQ 19/saa7133[0]: IRQF_DISABLED is not guaranteed on shared IRQs
[4.868459] HDA Intel :00:1b.0: PCI INT A -> GSI 16 (level, low) -> IRQ 
16
[4.868533] HDA Intel :00:1b.0: setting latency timer to 64
[4.952004] saa7133[0]: i2c eeprom 00: ce 5a 95 75 54 20 00 00 00 00 00 00 
00 00 00 01
[4.952604] saa7133[0]: i2c eeprom 10: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.953202] saa7133[0]: i2c eeprom 20: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.953799] saa7133[0]: i2c eeprom 30: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.954397] saa7133[0]: i2c eeprom 40: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.954994] saa7133[0]: i2c eeprom 50: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.955592] saa7133[0]: i2c eeprom 60: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.956200] saa7133[0]: i2c eeprom 70: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.956798] saa7133[0]: i2c eeprom 80: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.957395] saa7133[0]: i2c eeprom 90: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.957993] saa7133[0]: i2c eeprom a0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.958591] saa7133[0]: i2c eeprom b0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.959189] saa7133[0]: i2c eeprom c0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.959786] saa7133[0]: i2c eeprom d0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.960394] saa7133[0]: i2c eeprom e0: 00 00 00 00 ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.960991] saa7133[0]: i2c eeprom f0: 42 54 56 30 30 30 30 ff ff ff ff ff 
ff ff ff ff
[5.012044] tuner 1-0061: chip found @ 0xc2 (saa7133[0])
[5.128672] xc5000 1-0061: creating new instance
[5.136014] xc5000: Successfully identified at address 0x61
[5.136067] xc5000: Firmware has not been loaded previously

[   33.425823] saa7134 :04:01.0: spi master registered: bus_num=32766 
num_chipselect=1
[   33.425883] saa7133[0]: found muPD61151 MPEG encoder
[   33.425928] befor request_module upd61151
[   33.435609] upd61151_probe function
[   33.435654] Read test REG 0xD8 :
[   33.437463] REG = 0x0
[   33.437505] Write test 0x03 to REG 0xD8 :
[   33.439310] Next read test REG 0xD8 :
[   33.441123] REG = 0x3
[   33.441167] bef

Re: saa7134 and μPD61151 MPEG2 coder

2010-01-28 Thread Hans Verkuil
On Thursday 28 January 2010 03:09:41 Dmitri Belimov wrote:
> HIi Hans
> 
> > Hi Dmitri,
> > 
> > Just a quick note: the video4linux mailinglist is obsolete, just use
> > linux-media.
> 
> OK
> 
> > On Wednesday 27 January 2010 06:36:37 Dmitri Belimov wrote:
> > > Hi Hans.
> > > 
> > > I finished saa7134 part of SPI. Please review saa7134-spi.c and
> > > diff saa7134-core and etc. I wrote config of SPI to board
> > > structure. Use this config for register master and slave devices.
> > > 
> > > SPI other then I2C, do not need call request_module. Udev do it. 
> > > I spend 10 days for understanding :(  
> > 
> > I'm almost certain that spi works the same way as i2c and that means
> > that you must call request_module. Yes, udev will load it for you,
> > but that is a delayed load: i.e. the module may not be loaded when we
> > need it. The idea behind this is that usually i2c or spi modules are
> > standalone, but in the context of v4l such modules are required to be
> > present before the bridge can properly configure itself.
> > 
> > The easiest way to ensure the correct load sequence is to do a
> > request_module at the start.
> > 
> > Now, I haven't compiled this, but I think this will work:
> > 
> > struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
> >struct spi_master *master, struct spi_board_info *info)
> > {
> > struct v4l2_subdev *sd = NULL;
> > struct spi_device *spi;
> > 
> > BUG_ON(!v4l2_dev);
> > 
> > if (module_name)
> > request_module(module_name);

There is one thing missing here: module_name should be passed in as argument
to v4l2_spi_new_subdev. Does this code actually compile? If so, then I suspect
module_name must be some global variable with some bogus value which causes
request_module to time out.

> [  240.476082]  [] ? v4l2_spi_new_subdev_board+0x2e/0x35 
> [v4l2_common]
> [  240.476086]  [] ? v4l2_spi_new_subdev+0x64/0x6c [v4l2_common]

Remove v4l2_spi_new_subdev_board. Just have a v4l2_spi_new_subdev as in my code.

Regards,

Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: saa7134 and μPD61151 MPEG2 coder

2010-01-27 Thread Dmitri Belimov
HIi Hans

> Hi Dmitri,
> 
> Just a quick note: the video4linux mailinglist is obsolete, just use
> linux-media.

OK

> On Wednesday 27 January 2010 06:36:37 Dmitri Belimov wrote:
> > Hi Hans.
> > 
> > I finished saa7134 part of SPI. Please review saa7134-spi.c and
> > diff saa7134-core and etc. I wrote config of SPI to board
> > structure. Use this config for register master and slave devices.
> > 
> > SPI other then I2C, do not need call request_module. Udev do it. 
> > I spend 10 days for understanding :(  
> 
> I'm almost certain that spi works the same way as i2c and that means
> that you must call request_module. Yes, udev will load it for you,
> but that is a delayed load: i.e. the module may not be loaded when we
> need it. The idea behind this is that usually i2c or spi modules are
> standalone, but in the context of v4l such modules are required to be
> present before the bridge can properly configure itself.
> 
> The easiest way to ensure the correct load sequence is to do a
> request_module at the start.
> 
> Now, I haven't compiled this, but I think this will work:
> 
> struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
>struct spi_master *master, struct spi_board_info *info)
> {
>   struct v4l2_subdev *sd = NULL;
> struct spi_device *spi;
>   
>   BUG_ON(!v4l2_dev);
> 
>   if (module_name)
>   request_module(module_name);
> 
>   spi = spi_new_device(master, info);
> 
>   if (spi == NULL || spi->dev.driver == NULL)
>   goto error;
> 
>if (!try_module_get(spi->dev.driver->owner))
>goto error;
> 
>sd = spi_get_drvdata(spi);
> 
>/* Register with the v4l2_device which increases the module's
>   use count as well. */
> 
>if (v4l2_device_register_subdev(v4l2_dev, sd))
>sd = NULL;
> 
>/* Decrease the module use count to match the first
> try_module_get. */ module_put(spi->dev.driver->owner);
> 
> error:
>/* If we have a client but no subdev, then something went
> wrong and we must unregister the client. */
> 
>if (spi && sd == NULL)
>spi_unregister_device(spi);
> 
>return sd;
> }
> EXPORT_SYMBOL_GPL(v4l2_spi_new_subdev);

Not work

[6.048195] Linux video capture interface: v2.00
[6.112987] saa7130/34: v4l2 driver version 0.2.15 loaded
[6.113067] saa7134 :04:01.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
[6.113117] saa7133[0]: found at :04:01.0, rev: 209, irq: 19, latency: 
32, mmio: 0xe510
[6.113176] saa7133[0]: subsystem: 5ace:7595, board: Beholder BeholdTV X7 
[card=171,autodetected]
[6.113241] saa7133[0]: board init: gpio is 20
[6.113292] IRQ 19/saa7133[0]: IRQF_DISABLED is not guaranteed on shared IRQs
[6.264512] saa7133[0]: i2c eeprom 00: ce 5a 95 75 54 20 00 00 00 00 00 00 
00 00 00 01
[6.265136] saa7133[0]: i2c eeprom 10: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.265731] saa7133[0]: i2c eeprom 20: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.266327] saa7133[0]: i2c eeprom 30: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.266922] saa7133[0]: i2c eeprom 40: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.267517] saa7133[0]: i2c eeprom 50: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.268113] saa7133[0]: i2c eeprom 60: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.268718] saa7133[0]: i2c eeprom 70: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.269313] saa7133[0]: i2c eeprom 80: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.269908] saa7133[0]: i2c eeprom 90: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.270503] saa7133[0]: i2c eeprom a0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.271098] saa7133[0]: i2c eeprom b0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.271693] saa7133[0]: i2c eeprom c0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.272289] saa7133[0]: i2c eeprom d0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.272895] saa7133[0]: i2c eeprom e0: 00 00 00 00 ff ff ff ff ff ff ff ff 
ff ff ff ff
[6.273490] saa7133[0]: i2c eeprom f0: 42 54 56 30 30 30 30 ff ff ff ff ff 
ff ff ff ff
[6.360023] tuner 1-0061: chip found @ 0xc2 (saa7133[0])
[6.401952] xc5000 1-0061: creating new instance
[6.412005] xc5000: Successfully identified at address 0x61
[6.412054] xc5000: Firmware has not been loaded previously
[6.477742] HDA Intel :00:1b.0: PCI INT A -> GSI 16 (level, low) -> IRQ 
16
[6.477816] HDA Intel :00:1b.0: setting latency timer to 64

[   34.752763] saa7134 :04:01.0: spi master registered: bus_num=32766 
num_chipselect=1
[   34.752823] saa7133[0]: found muPD61151 MPEG encoder
[   34.752883] befor request_module

[  240.476013] INFO: task modprobe:1404 blocked for more than 120 seconds.
[  240.476016] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 

Re: saa7134 and μPD61151 MPEG2 coder

2010-01-27 Thread Hans Verkuil
Hi Dmitri,

Just a quick note: the video4linux mailinglist is obsolete, just use 
linux-media.

On Wednesday 27 January 2010 06:36:37 Dmitri Belimov wrote:
> Hi Hans.
> 
> I finished saa7134 part of SPI. Please review saa7134-spi.c and diff 
> saa7134-core and etc.
> I wrote config of SPI to board structure. Use this config for register master 
> and slave devices.
> 
> SPI other then I2C, do not need call request_module. Udev do it. 
> I spend 10 days for understanding :(  

I'm almost certain that spi works the same way as i2c and that means that you
must call request_module. Yes, udev will load it for you, but that is a delayed
load: i.e. the module may not be loaded when we need it. The idea behind this
is that usually i2c or spi modules are standalone, but in the context of v4l
such modules are required to be present before the bridge can properly configure
itself.

The easiest way to ensure the correct load sequence is to do a request_module
at the start.

Now, I haven't compiled this, but I think this will work:

struct v4l2_subdev *v4l2_spi_new_subdev(struct v4l2_device *v4l2_dev,
   struct spi_master *master, struct spi_board_info *info)
{
struct v4l2_subdev *sd = NULL;
struct spi_device *spi;

BUG_ON(!v4l2_dev);

if (module_name)
request_module(module_name);

spi = spi_new_device(master, info);

if (spi == NULL || spi->dev.driver == NULL)
goto error;

   if (!try_module_get(spi->dev.driver->owner))
   goto error;

   sd = spi_get_drvdata(spi);

   /* Register with the v4l2_device which increases the module's
  use count as well. */

   if (v4l2_device_register_subdev(v4l2_dev, sd))
   sd = NULL;

   /* Decrease the module use count to match the first try_module_get. */
   module_put(spi->dev.driver->owner);

error:
   /* If we have a client but no subdev, then something went wrong and
  we must unregister the client. */

   if (spi && sd == NULL)
   spi_unregister_device(spi);

   return sd;
}
EXPORT_SYMBOL_GPL(v4l2_spi_new_subdev);

Note that you mixed up the spi master and spi client in your original code. So
it is no wonder you experienced crashes.

Also note that there is no need for a separate v4l2_spi_new_subdev_board 
function.

And in v4l2_spi_subdev_init() you should use spi_set_drvdata instead of
dev_set_drvdata.

I hope this helps.

Regards,

Hans

> 
> I need help with v4l2-common.c -> function v4l2_spi_new_subdev_board
> The module of SPI slave loading after some time and spi device hasn't 
> v4l2_subdev structure
> for v4l2_device_register_subdev.
> 
> Now I get kernel crash when call v4l2_device_register_subdev.
> 
> Need use a callback metod or other. I don't know.
> 
> Copy to Mauro Carvalho Chehab and mailing lists for review my code too.
> 
> Dmesg log without call v4l2_device_register_subdev.
> [4.742279] Linux video capture interface: v2.00
> [4.816171] saa7130/34: v4l2 driver version 0.2.15 loaded
> [4.816253] saa7134 :04:01.0: PCI INT A -> GSI 19 (level, low) -> IRQ 
> 19
> [4.816304] saa7133[0]: found at :04:01.0, rev: 209, irq: 19, latency: 
> 32, mmio: 0xe510
> [4.816363] saa7133[0]: subsystem: 5ace:7595, board: Beholder BeholdTV X7 
> [card=171,autodetected]
> [4.816430] saa7133[0]: board init: gpio is 20
> [4.816481] IRQ 19/saa7133[0]: IRQF_DISABLED is not guaranteed on shared 
> IRQs
> [4.976010] saa7133[0]: i2c eeprom 00: ce 5a 95 75 54 20 00 00 00 00 00 00 
> 00 00 00 01
> [4.976635] saa7133[0]: i2c eeprom 10: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.977231] saa7133[0]: i2c eeprom 20: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.977827] saa7133[0]: i2c eeprom 30: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.978423] saa7133[0]: i2c eeprom 40: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.979019] saa7133[0]: i2c eeprom 50: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.979615] saa7133[0]: i2c eeprom 60: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.980223] saa7133[0]: i2c eeprom 70: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.980820] saa7133[0]: i2c eeprom 80: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.981416] saa7133[0]: i2c eeprom 90: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.982012] saa7133[0]: i2c eeprom a0: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.982608] saa7133[0]: i2c eeprom b0: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.983204] saa7133[0]: i2c eeprom c0: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.983800] saa7133[0]: i2c eeprom d0: ff ff ff ff ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.984437] saa7133[0]: i2c eeprom e0: 00 00 00 00 ff ff ff ff ff ff ff ff 
> ff ff ff ff
> [4.985033] saa7133[0]: i2c eeprom f0: 42 54 56

Re: saa7134 and μPD61151 MPEG2 coder

2010-01-26 Thread Dmitri Belimov
Hi Hans.

I finished saa7134 part of SPI. Please review saa7134-spi.c and diff 
saa7134-core and etc.
I wrote config of SPI to board structure. Use this config for register master 
and slave devices.

SPI other then I2C, do not need call request_module. Udev do it. 
I spend 10 days for understanding :(  

I need help with v4l2-common.c -> function v4l2_spi_new_subdev_board
The module of SPI slave loading after some time and spi device hasn't 
v4l2_subdev structure
for v4l2_device_register_subdev.

Now I get kernel crash when call v4l2_device_register_subdev.

Need use a callback metod or other. I don't know.

Copy to Mauro Carvalho Chehab and mailing lists for review my code too.

Dmesg log without call v4l2_device_register_subdev.
[4.742279] Linux video capture interface: v2.00
[4.816171] saa7130/34: v4l2 driver version 0.2.15 loaded
[4.816253] saa7134 :04:01.0: PCI INT A -> GSI 19 (level, low) -> IRQ 19
[4.816304] saa7133[0]: found at :04:01.0, rev: 209, irq: 19, latency: 
32, mmio: 0xe510
[4.816363] saa7133[0]: subsystem: 5ace:7595, board: Beholder BeholdTV X7 
[card=171,autodetected]
[4.816430] saa7133[0]: board init: gpio is 20
[4.816481] IRQ 19/saa7133[0]: IRQF_DISABLED is not guaranteed on shared IRQs
[4.976010] saa7133[0]: i2c eeprom 00: ce 5a 95 75 54 20 00 00 00 00 00 00 
00 00 00 01
[4.976635] saa7133[0]: i2c eeprom 10: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.977231] saa7133[0]: i2c eeprom 20: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.977827] saa7133[0]: i2c eeprom 30: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.978423] saa7133[0]: i2c eeprom 40: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.979019] saa7133[0]: i2c eeprom 50: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.979615] saa7133[0]: i2c eeprom 60: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.980223] saa7133[0]: i2c eeprom 70: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.980820] saa7133[0]: i2c eeprom 80: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.981416] saa7133[0]: i2c eeprom 90: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.982012] saa7133[0]: i2c eeprom a0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.982608] saa7133[0]: i2c eeprom b0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.983204] saa7133[0]: i2c eeprom c0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.983800] saa7133[0]: i2c eeprom d0: ff ff ff ff ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.984437] saa7133[0]: i2c eeprom e0: 00 00 00 00 ff ff ff ff ff ff ff ff 
ff ff ff ff
[4.985033] saa7133[0]: i2c eeprom f0: 42 54 56 30 30 30 30 ff ff ff ff ff 
ff ff ff ff
[5.008041] tuner 1-0061: chip found @ 0xc2 (saa7133[0])
[5.024036] HDA Intel :00:1b.0: PCI INT A -> GSI 16 (level, low) -> IRQ 
16
[5.024109] HDA Intel :00:1b.0: setting latency timer to 64
[5.066725] xc5000 1-0061: creating new instance
[5.076009] xc5000: Successfully identified at address 0x61
[5.076060] xc5000: Firmware has not been loaded previously

[   33.381216] saa7134 :04:01.0: spi master registered: bus_num=32766 
num_chipselect=1
[   33.381274] saa7133[0]: found muPD61151 MPEG encoder
[   33.381430] saa7133[0]: registered device video0 [v4l2]
[   33.381491] saa7133[0]: registered device vbi0
[   33.381551] saa7133[0]: registered device radio0
[   33.406256] saa7133[0]: registered device video1 [mpeg]
[   33.407628] upd61151_probe function
[   33.407672] Read test REG 0xD8 :
[   33.409502] REG = 0x0
[   33.409547] Write test 0x03 to REG 0xD8 :
[   33.411353] Next read test REG 0xD8 :
[   33.413176] REG = 0x3
[   33.431308] saa7134 ALSA driver for DMA sound loaded
[   33.431363] IRQ 19/saa7133[0]: IRQF_DISABLED is not guaranteed on shared IRQs
[   33.431425] saa7133[0]/alsa: saa7133[0] at 0xe510 irq 19 registered as 
card -1

[   48.657067] xc5000: I2C write failed (len=4)
[   48.760018] xc5000: I2C write failed (len=4)
[   48.762960] xc5000: I2C read failed
[   48.763011] xc5000: I2C read failed
[   48.763054] xc5000: waiting for firmware upload (dvb-fe-xc5000-1.6.114.fw)...
[   48.763102] saa7134 :04:01.0: firmware: requesting 
dvb-fe-xc5000-1.6.114.fw
[   48.802473] xc5000: firmware read 12401 bytes.
[   48.802477] xc5000: firmware uploading...
[   49.328504] eth0: no IPv6 routers present
[   52.132007] xc5000: firmware upload complete...
[   53.366772] [ cut here ]
[   53.366820] kernel BUG at lib/kernel_lock.c:126!
[   53.366865] invalid opcode:  [#1] SMP 
[   53.366973] last sysfs file: /sys/class/firmware/:04:01.0/loading
[   53.367019] Modules linked in: ipv6 dm_snapshot dm_mirror dm_region_hash 
dm_log dm_mod loop saa7134_alsa upd61151 saa7134_empress ir_kbd_i2c 
snd_hda_codec_realtek xc5000 snd_hda_intel snd_hda_codec tuner snd_pcm_oss 
snd_mixer_oss snd_pcm snd_seq_dummy snd_seq_oss snd_seq_midi snd_rawmidi 
snd_seq_midi_event snd_seq sa