Re: CX23103 Video Grabber seems to be supported by cx231xx driver
Hi Florian,
On 07/12/2013 06:26 PM, Florian Streibelt wrote:
> Hi,
>
> the chip CX23103 that is used in various devices sold e.g. in germany works
> with the cx231xx stock driver.
>
> The author of that driver is not reachable via the email adress stated in the
> source file: srinivasa.de...@conexant.com
> [ host cnxtsmtp1.conexant.com [198.62.9.252]: 550 5.1.1
> : Recipient address rejected: User unknown in
> relay recipient table]
Yeah, I suspect he left Conexant. For all practical purposes that leaves me as
the maintainer for my sins.
>
> In drivers/media/video/cx231xx/cx231xx-cards.c the struct usb_device_id
> cx231xx_id_table[] needs these lines added:
>
>{USB_DEVICE(0x1D19, 0x6109),
>.driver_info = CX231XX_BOARD_PV_XCAPTURE_USB},
That looks OK.
> While the change is minimal due to the fact that no real technical
> documentation is available on the chip the support was guessed - but worked
> for video.
>
> The videostream can pe played using mplayer tv:///0 - proof:
> http://streibelt.de/blog/2013/06/23/kernel-patch-for-cx23103-video-grabber-linux-support/
>
> However when trying to capture audio using audacity while playing the video
> stream in mplayer my system locked (no message in syslog, complete freeze).
I've no idea what is happening here. It has probably to do with the board setup,
although there isn't all that much that you can change there that relates to
audio.
Try using 'arecord' instead of audicity. The arecord tool is more low-level, so
it will be interesting to know if it behaves differently.
Besides that the only thing I can think of is just to try and add printk's to
cx231xx-audio.c and see where things go boom.
A useful trick there is to add a mdelay(5) or so after the printk to give the
system time to write to the kernel log.
Be aware that I consider this driver to be flaky, so I would not at all be
surprised if there are bugs lurking in the code.
Regards,
Hans
> I posted this one month ago to this list without any reaction so I ask if
> this is the correct way to get that grabber really supported.
>
> I am willing to do any tests neccessary and try out patches.
--
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: [media] cx25821 regression from 3.9: BUG: bad unlock balance detected!
Hi Sander, On 07/12/2013 10:56 PM, Sander Eikelenboom wrote: > > Friday, May 17, 2013, 11:52:17 AM, you wrote: > >> On Fri May 17 2013 11:04:50 Sander Eikelenboom wrote: >>> >>> Friday, May 17, 2013, 10:25:24 AM, you wrote: >>> On Thu May 16 2013 19:41:42 Sander Eikelenboom wrote: > Hi Hans / Mauro, > > With 3.10.0-rc1 (including the cx25821 changes from Hans), I get the bug > below which wasn't present with 3.9. >>> How do I reproduce this? I've tried to, but I can't make this happen. >>> Looking at the code I can't see how it could hit this bug anyway. >>> >>> I'm using "motion" to grab and process 6 from the video streams of the card >>> i have (card with 8 inputs). >>> It seems the cx25821 underwent quite some changes between 3.9 and 3.10. > >> It did. > >>> And in the past there have been some more locking issues around mmap and >>> media devices, although they seem to appear as circular locking >>> dependencies and with different devices. >>>- http://www.mail-archive.com/linux-media@vger.kernel.org/msg46217.html >>>- Under kvm: http://www.spinics.net/lists/linux-media/msg63322.html > >> Neither of those are related to this issue. > >>> >>> - Perhaps that running in a VM could have to do with it ? >>>- The driver on 3.9 occasionaly gives this, probably latency related >>> (but continues to work): >>> cx25821: cx25821_video_wakeup: 2 buffers handled (should be 1) >>> >>> Could it be something double unlocking in that path ? >>> >>> - Is there any extra debugging i could enable that could pinpoint the issue >>> ? > >> Try this patch: > >> diff --git a/drivers/media/pci/cx25821/cx25821-core.c >> b/drivers/media/pci/cx25821/cx25821-core.c >> index b762c5b..8f8d0e0 100644 >> --- a/drivers/media/pci/cx25821/cx25821-core.c >> +++ b/drivers/media/pci/cx25821/cx25821-core.c >> @@ -1208,7 +1208,6 @@ void cx25821_free_buffer(struct videobuf_queue *q, >> struct cx25821_buffer *buf) >> struct videobuf_dmabuf *dma = videobuf_to_dma(&buf->vb); >> >> BUG_ON(in_interrupt()); >> - videobuf_waiton(q, &buf->vb, 0, 0); >> videobuf_dma_unmap(q->dev, dma); >> videobuf_dma_free(dma); >> btcx_riscmem_free(to_pci_dev(q->dev), &buf->risc); > >> I don't think the waiton is really needed for this driver. > >> What really should happen is that videobuf is replaced by videobuf2 in this >> driver, but that's a fair amount of work. > > Hi Hans, > > After being busy for quite some time, i do have some spare time now. > > Since i'm still having trouble with this driver, is there a patch series for > a similar driver > that was converted to videobuf2 ? > I don't know if it is entirely in my league, but i could give it a try when i > have a example. The changes done for usb/em28xx might come close. That said, the cx25821 is already in decent shape to convert to vb2. At least the videobuf data structures are already in the correct place (they are often stored in a per-filehandle struct, which is wrong). include/media/videobuf2-core.h gives a reasonable overview of vb2. Like em28xx, you should use the vb2 helper functions (vb2_fop_* and vb2_ioctl_*) which takes a lot of the work off your hands. Converting cx25821-alsa.c may be the most difficult part as it is using some videobuf internal functions which probably won't translate to vb2 as is. I think videobuf is being abused here, but I don't know off-hand what the correct approach will be with vb2. I would ignore the alsa part for the time being (also the audio/video-upstream code, that's been disabled and without datasheets of the cx25821 I'm not sure it can be resurrected). If you can get cx25821-video.c to work with vb2, then we can take a look at the alsa code. Regards, Hans -- 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: CX23103 Video Grabber seems to be supported by cx231xx driver
On Sun, 14 Jul 2013 11:23:46 +0200
Hans Verkuil wrote:
> Hi Florian,
>
> On 07/12/2013 06:26 PM, Florian Streibelt wrote:
> > Hi,
> >
> > the chip CX23103 that is used in various devices sold e.g. in germany works
> > with the cx231xx stock driver.
> >
> > The author of that driver is not reachable via the email adress stated in
> > the source file: srinivasa.de...@conexant.com
> > [ host cnxtsmtp1.conexant.com [198.62.9.252]: 550 5.1.1
> > : Recipient address rejected: User unknown
> > in relay recipient table]
>
> Yeah, I suspect he left Conexant. For all practical purposes that leaves me as
> the maintainer for my sins.
heh - also means a patch should remove the wrong email adress/change the
maintainers in the source?
>
> >
> > In drivers/media/video/cx231xx/cx231xx-cards.c the struct usb_device_id
> > cx231xx_id_table[] needs these lines added:
> >
> >{USB_DEVICE(0x1D19, 0x6109),
> >.driver_info = CX231XX_BOARD_PV_XCAPTURE_USB},
>
> That looks OK.
Only if the board layout is different :/
There is no information from the vendor, they don't even reply to messages via
the contact form, of course.
>
> > While the change is minimal due to the fact that no real technical
> > documentation is available on the chip the support was guessed - but worked
> > for video.
> >
> > The videostream can pe played using mplayer tv:///0 - proof:
> > http://streibelt.de/blog/2013/06/23/kernel-patch-for-cx23103-video-grabber-linux-support/
> >
> > However when trying to capture audio using audacity while playing the video
> > stream in mplayer my system locked (no message in syslog, complete freeze).
>
> I've no idea what is happening here. It has probably to do with the board
> setup,
> although there isn't all that much that you can change there that relates to
> audio.
hm. maybe disable it - currently my time budget is "negative" - so I cannot
really work on this.
>
> Try using 'arecord' instead of audicity. The arecord tool is more low-level,
> so
> it will be interesting to know if it behaves differently.
I'll try - the problem is the complete system freeze - I'll see if I can setup
a system with serial console for the kernel log
>
> Besides that the only thing I can think of is just to try and add printk's to
> cx231xx-audio.c and see where things go boom.
yup. If I had the time.
>
> A useful trick there is to add a mdelay(5) or so after the printk to give the
> system time to write to the kernel log.
ok
>
> Be aware that I consider this driver to be flaky, so I would not at all be
> surprised if there are bugs lurking in the code.
Hum. Because of code quality or due to the missing documentation from the
vendor?
If you have any documents on the chip I would be happy.
/Florian
--
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: [PATCH 00/50] USB: cleanup spin_lock in URB->complete()
On Thu, 2013-07-11 at 17:05 +0800, Ming Lei wrote: > Hi, > > As we are going to run URB->complete() in tasklet context[1][2], Hi, Please pardon my naivete, but why was it decided to use tasklets to defer work, as opposed to some other deferred work mechanism? It seems to me that getting rid of tasklets has been an objective for years: http://lwn.net/Articles/239633/ http://lwn.net/Articles/520076/ http://lwn.net/Articles/240054/ Regards, Andy > and > hard interrupt may be enabled when running URB completion handler[3], > so we might need to disable interrupt when acquiring one lock in > the completion handler for the below reasons: > > - URB->complete() holds a subsystem wide lock which may be acquired > in another hard irq context, and the subsystem wide lock is acquired > by spin_lock()/read_lock()/write_lock() in complete() > > - URB->complete() holds a private lock with > spin_lock()/read_lock()/write_lock() > but driver may export APIs to make other drivers acquire the same private > lock in its interrupt handler. > > For the sake of safety and making the change simple, this patch set > converts all spin_lock()/read_lock()/write_lock() in completion handler > path into their irqsave version mechanically. > > But if you are sure the above two cases do not happen in your driver, > please let me know and I can drop the unnecessary change. > > Also if you find some conversions are missed, also please let me know so > that I can add it in the next round. > > > [1], http://marc.info/?l=linux-usb&m=137286322526312&w=2 > [2], http://marc.info/?l=linux-usb&m=137286326726326&w=2 > [3], http://marc.info/?l=linux-usb&m=137286330626363&w=2 > [snip] > > > Thanks, > -- > Ming Lei -- 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: CX23103 Video Grabber seems to be supported by cx231xx driver
On Sun, Jul 14, 2013 at 9:23 AM, Florian Streibelt wrote: >> Be aware that I consider this driver to be flaky, so I would not at all be >> surprised if there are bugs lurking in the code. > > > Hum. Because of code quality or due to the missing documentation from the > vendor? While this is all too common for vendors, it really isn't a reasonable assertion in this case. Conexant (the chip vendor) wrote the original driver, and they have been very supportive in the past. They provided documentation (under NDA) and reference designs at no cost. They've also answered questions I've had in the past regarding the chip and you'll also note that the email address of the maintainer was a Conexant engineer until he left the company. Regarding the flakiness, there indeed have been some reliability problems - some of them were in the original driver sources, a couple I introduced doing the cleanup work to get it upstream (and long ago fixed), and some were regressions introduced after the driver went upstream. It's tough maintaining a driver on an ongoing basis that supports many different cards from different vendors, in particular since individuals making changes to the driver to make it work with some new device, don't have all of the other products to test with (to ensure regressions aren't introduced). These drivers also tend to suffer from bitrot. If they aren't actively being used by many people and if there isn't a developer who actively maintains the driver, then regressions sneak in there and go unnoticed for months/years. > If you have any documents on the chip I would be happy. I don't think there are any documents that aren't under NDA. That said, you don't need the register docs to debug a system hang. If you don't have the time to jam a few printk() statements into the source, then there's no point in going through the effort to get you docs. Your best bet at this point is probably to wait [indefinitely] for some developer who has a need for the device to work to stumble across the problem and debug it. You get what you pay for. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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: [PATCH 00/50] USB: cleanup spin_lock in URB->complete()
On Sun, Jul 14, 2013 at 9:17 PM, Andy Walls wrote: > On Thu, 2013-07-11 at 17:05 +0800, Ming Lei wrote: >> Hi, >> >> As we are going to run URB->complete() in tasklet context[1][2], > > Hi, > > Please pardon my naivete, but why was it decided to use tasklets to > defer work, as opposed to some other deferred work mechanism? > > It seems to me that getting rid of tasklets has been an objective for > years: > > http://lwn.net/Articles/239633/ > http://lwn.net/Articles/520076/ > http://lwn.net/Articles/240054/ We discussed the problem in the below link previously[1], Steven and Thomas suggested to use threaded irq handler, but which may degrade USB mass storage performance, so we have to take tasklet now until we rewrite transport part of USB mass storage driver. Also the conversion[2] has avoided the tasklet spin lock problem already. [1], http://marc.info/?t=13707911921&r=1&w=2 [2], http://marc.info/?l=linux-usb&m=137286326726326&w=2 Thanks, -- Ming Lei -- 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
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Sun Jul 14 19:00:22 CEST 2013 git branch: test git hash: 1c26190a8d492adadac4711fe5762d46204b18b0 gcc version:i686-linux-gcc (GCC) 4.8.1 sparse version: v0.4.5-rc1 host hardware: x86_64 host os:3.9-7.slh.1-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.31.14-i686: WARNINGS linux-2.6.32.27-i686: WARNINGS linux-2.6.33.7-i686: WARNINGS linux-2.6.34.7-i686: WARNINGS linux-2.6.35.9-i686: WARNINGS linux-2.6.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: OK linux-3.10-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: WARNINGS linux-3.9.2-i686: WARNINGS linux-2.6.31.14-x86_64: WARNINGS linux-2.6.32.27-x86_64: WARNINGS linux-2.6.33.7-x86_64: WARNINGS linux-2.6.34.7-x86_64: WARNINGS linux-2.6.35.9-x86_64: WARNINGS linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: OK linux-3.10-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: WARNINGS linux-3.8-x86_64: WARNINGS linux-3.9.2-x86_64: WARNINGS apps: WARNINGS spec-git: OK sparse version: v0.4.5-rc1 sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Sunday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- 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: [PATCH RFC v3] media: OF: add video sync endpoint property
Hi Prabhakar,
On 07/12/2013 06:29 AM, Prabhakar Lad wrote:
On Fri, Jul 12, 2013 at 2:45 AM, Sylwester Nawrocki
wrote:
On 07/11/2013 01:41 PM, Prabhakar Lad wrote:
[...]
diff --git a/drivers/media/v4l2-core/v4l2-of.c
b/drivers/media/v4l2-core/v4l2-of.c
index aa59639..1a54530 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -100,6 +100,26 @@ static void v4l2_of_parse_parallel_bus(const struct
device_node *node,
if (!of_property_read_u32(node, "data-shift",&v))
bus->data_shift = v;
+ if (!of_property_read_u32(node, "video-sync",&v)) {
+ switch (v) {
+ case V4L2_MBUS_VIDEO_SEPARATE_SYNC:
+ flags |= V4L2_MBUS_VIDEO_SEPARATE_SYNC;
I'm not convinced all those video sync types is something that really
belongs
to the flags field. In my understanding this field is supposed to hold
only
the _signal polarity_ information.
Ok, so there should be a function say v4l2_of_parse_signal_polarity()
to get the polarity alone then.
I don't think this is required, I would just extend
v4l2_of_parse_parallel_bus()
function to also handle sync-on-green-active property.
If that is the case than I have to add a member say 'signal_polarity'
in struct v4l2_of_bus_parallel and assign the polarity to it.
Let me know if you are OK with it.
It probably would have been sensible to do something like this, however
I can't
see any advantage at the moment. struct v4l2_of_bus_parallel::flags
currently
holds all the polarity flags. Let's just add relevant macros for
sync-on-green
and store them in the flags field, as is done with the others.
Those V4L2_MUS_* flags are used by soc-camera to negotiate the
capabilities,
so I would rather not split them further without good reason, even though
struct v4l2_mbus_config::flags is used in those negotiation routines,
rather
than struct v4l2_of_bus_parallel::flags.
--
Thanks,
Sylwester
--
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: [PATCH v2 2/2] media: i2c: tvp7002: add OF support
Hi Prabhakar, On 07/12/2013 06:43 AM, Prabhakar Lad wrote: On Fri, Jul 12, 2013 at 3:34 AM, Sylwester Nawrocki wrote: On 07/11/2013 07:09 PM, Prabhakar Lad wrote: [...] And include/media/tvp70002.h: * fid_polarity: * 0 -> the field ID output is set to logic 1 for an odd * field (field 1) and set to logic 0 for an even * field (field 0). * 1 -> operation with polarity inverted. Do you know if the chip automatically selects video sync source (sync-on-green vs. VSYNC/HSYNC) and there is no need to configure this on the analogue input side ? At least the driver seems to always select the default SOGIN_1 input (TVP7002_IN_MUX_SEL_1 register is set only at initialization time). Yes the driver is selecting the default SOGIN_1 input. Or perhaps it just outputs on SOGOUT, VSOUT, HSOUT lines whatever is fed to its analogue inputs, and any further processing unit need to determine what synchronization signal is present and should be used ? Yes that correct, there is a register (Sync Detect Status) which detects the sync for you. I suspect that we don't need, e.g. another endpoint node to specify the configuration of the TVP7002 analogue input interface, that would contain a property like video-sync. If I understand correctly you mean if there are two tvp7002 devices connected we don’t need to specify video-sync property, but my question how do we specify this property in common then ? No, I thought about two port sub-nodes of a single device node, one for the TVP7002 video input and one for the output. But it seems there is no need for that, i.e. to specify the input configuration statically in the firmware. The chip detects the signals automatically, i.e. it uses whatever is available, and it allows querying the selection status at run time. What would really need to be configured statically in DT in that case then ? Some initial video sync configuration ? I guess it could be well hard coded in the driver, since the hardware does run time detection anyway. Yes the chip detects the signal automatically, What I want to configure in the DT case is say if SOG signal is detected, I want to invert the polarity of it this is what I am trying to set in DT case whether to invert or not. 0 = Normal operation (default) 1 = SOG output polarity inverted Something similar to fid_polarity. Then as I suggested earlier, let's just add 'sync-on-green-active' DT property for that. I wouldn't expect the DT properties to be directly replacing your driver platform_data members. Saying in the binding that this is a normal operation and that is an inverted one is not very useful in general, as you would need to dig in the binding's description what "normal" exactly means. sync-on-green-active = <1>; seems much more explicit than, e.g. sync-on-green-inverted. By looking at the sync-on-green-active property each device driver would determine whether it means normal or inverted operation for its device. -- Thanks, Sylwester -- 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: [PATCH v2] media: i2c: adv7343: add OF support
Hi Prabhakar,
On 07/13/2013 01:12 PM, Prabhakar Lad wrote:
From: "Lad, Prabhakar"
add OF support for the adv7343 driver.
Signed-off-by: Lad, Prabhakar
---
Changes for v2:
1: Fixed naming of properties.
.../devicetree/bindings/media/i2c/adv7343.txt | 54
drivers/media/i2c/adv7343.c| 65 +++-
2 files changed, 118 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/media/i2c/adv7343.txt
diff --git a/Documentation/devicetree/bindings/media/i2c/adv7343.txt
b/Documentation/devicetree/bindings/media/i2c/adv7343.txt
new file mode 100644
index 000..1d2e854
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/adv7343.txt
@@ -0,0 +1,54 @@
+* Analog Devices adv7343 video encoder
+
+The ADV7343 are high speed, digital-to-analog video encoders in a 64-lead LQFP
+package. Six high speed, 3.3 V, 11-bit video DACs provide support for composite
+(CVBS), S-Video (Y-C), and component (YPrPb/RGB) analog outputs in standard
+definition (SD), enhanced definition (ED), or high definition (HD) video
+formats.
+
+Required Properties :
+- compatible: Must be "ad,adv7343"
Please have a look at Documentation/devicetree/bindings/vendor-prefixes.txt.
'ad' is a vendor prefix reserved for "Avionic Design GmbH".
For "Analog Devices, Inc." 'adi' should be used.
If I would have to draft a new DT binding proposal checklist checking
vendor-prefixes.txt would certainly be one of the first steps.
+Optional Properties :
+- ad,adv7343-power-mode-sleep-mode: on enable the current consumption is
+reduced to micro ampere level. All DACs and
+the internal PLL circuit are disabled.
+- ad,adv7343-power-mode-pll-ctrl: PLL and oversampling control. This control
+ allows internal PLL 1 circuit to be powered
+ down and the oversampling to beswitched off.
+- ad,adv7343-power-mode-dac-1: power on/off DAC 1, 0 = OFF and 1 = ON.
+- ad,adv7343-power-mode-dac-2: power on/off DAC 2, 0 = OFF and 1 = ON.
+- ad,adv7343-power-mode-dac-3: power on/off DAC 3, 0 = OFF and 1 = ON.
+- ad,adv7343-power-mode-dac-4: power on/off DAC 4, 0 = OFF and 1 = ON.
+- ad,adv7343-power-mode-dac-5: power on/off DAC 5, 0 = OFF and 1 = ON.
+- ad,adv7343-power-mode-dac-6: power on/off DAC 6, 0 = OFF and 1 = ON.
+- ad,adv7343-sd-config-dac-out-1: Configure SD DAC Output 1.
+- ad,adv7343-sd-config-dac-out-2: Configure SD DAC Output 2.
All these properties look more like hardware configuration, rather than
hardware description. So at first sight I would say none of these properties
is suitable for the device tree.
sleep mode and pll ctrl should likely only have default values in the
driver.
sleep-mode disables all DAC, while power-mode-dac-? does power on/off
(enables / disables?) individual DACs. How those properties interact,
what's
going on here exactly ? :)
That said, how about only leaving the properties indicating which DACs
(including SD DACs) should be enabled ? E.g.
adi,dac-enable - an array indicating which DACs are enabled, in order
DAC1...DAC6, 1 to enable DAC, 0 to disable.
adi,sd-dac-enable - an array indicating which SD DACs are enabled, in order
DAC1...DAC2, 1 to enable SD DAC, 0 to disable.
Please note you don't need ",adv7343-" prefix in each single property
for that device.
+Example:
+
+i2c0@1c22000 {
+ ...
+ ...
+
+ adv7343@2a {
+ compatible = "ad,adv7343";
+ reg =<0x2a>;
+
+ port {
+ adv7343_1: endpoint {
+ ad,adv7343-power-mode-sleep-mode;
+ ad,adv7343-power-mode-pll-ctrl;
+ ad,adv7343-power-mode-dac-1;
+ ad,adv7343-power-mode-dac-2;
+ ad,adv7343-power-mode-dac-3;
+ ad,adv7343-power-mode-dac-4;
+ ad,adv7343-power-mode-dac-5;
+ ad,adv7343-power-mode-dac-6;
Then this would have become:
adi,dac-enable = <1 1 1 1 1 1>;
But I would put some disabled DACs in the example as well:
/* Use DAC1..3, DAC6 */
adi,dac-enable = <1 1 1 0 0 1>;
+ ad,adv7343-sd-config-dac-out-1;
+ ad,adv7343-sd-config-dac-out-2;
And this:
adi,sd-dac-enable = <1 1>;
+ };
+ };
+ };
+ ...
+};
diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c
index 7606218..22ee6f4 100644
--- a/drivers/media/i2
Re: [PATCH v3] ov10635: Add OmniVision ov10635 SoC camera driver
Hi Phil
I'll comment to this version, although the driver has to be updated to the
V4L2 clock API at least, preferably also to asynchronous probing.
On Wed, 5 Jun 2013, Phil Edworthy wrote:
> Signed-off-by: Phil Edworthy
> ---
> v3:
> - Removed dupplicated writes to reg 0x3042
> - Program all the standard registers after checking the ID
>
> v2:
> - Simplified flow in ov10635_s_ctrl.
> - Removed chip ident code - build tested only
>
> drivers/media/i2c/soc_camera/Kconfig |6 +
> drivers/media/i2c/soc_camera/Makefile |1 +
> drivers/media/i2c/soc_camera/ov10635.c | 1134
>
> 3 files changed, 1141 insertions(+)
> create mode 100644 drivers/media/i2c/soc_camera/ov10635.c
>
> diff --git a/drivers/media/i2c/soc_camera/Kconfig
> b/drivers/media/i2c/soc_camera/Kconfig
> index 23d352f..db97ee6 100644
> --- a/drivers/media/i2c/soc_camera/Kconfig
> +++ b/drivers/media/i2c/soc_camera/Kconfig
> @@ -74,6 +74,12 @@ config SOC_CAMERA_OV9740
> help
> This is a ov9740 camera driver
>
> +config SOC_CAMERA_OV10635
> + tristate "ov10635 camera support"
> + depends on SOC_CAMERA && I2C
> + help
> + This is an OmniVision ov10635 camera driver
> +
> config SOC_CAMERA_RJ54N1
> tristate "rj54n1cb0c support"
> depends on SOC_CAMERA && I2C
> diff --git a/drivers/media/i2c/soc_camera/Makefile
> b/drivers/media/i2c/soc_camera/Makefile
> index d0421fe..f3d3403 100644
> --- a/drivers/media/i2c/soc_camera/Makefile
> +++ b/drivers/media/i2c/soc_camera/Makefile
> @@ -10,5 +10,6 @@ obj-$(CONFIG_SOC_CAMERA_OV6650) += ov6650.o
> obj-$(CONFIG_SOC_CAMERA_OV772X) += ov772x.o
> obj-$(CONFIG_SOC_CAMERA_OV9640) += ov9640.o
> obj-$(CONFIG_SOC_CAMERA_OV9740) += ov9740.o
> +obj-$(CONFIG_SOC_CAMERA_OV10635) += ov10635.o
> obj-$(CONFIG_SOC_CAMERA_RJ54N1) += rj54n1cb0c.o
> obj-$(CONFIG_SOC_CAMERA_TW9910) += tw9910.o
> diff --git a/drivers/media/i2c/soc_camera/ov10635.c
> b/drivers/media/i2c/soc_camera/ov10635.c
> new file mode 100644
> index 000..064cc7b
> --- /dev/null
> +++ b/drivers/media/i2c/soc_camera/ov10635.c
> @@ -0,0 +1,1134 @@
> +/*
> + * OmniVision OV10635 Camera Driver
> + *
> + * Copyright (C) 2013 Phil Edworthy
> + * Copyright (C) 2013 Renesas Electronics
> + *
> + * This driver has been tested at QVGA, VGA and 720p, and 1280x800 at up to
> + * 30fps and it should work at any resolution in between and any frame rate
> + * up to 30fps.
> + *
> + * FIXME:
> + * Horizontal flip (mirroring) does not work correctly. The image is
> flipped,
> + * but the colors are wrong.
Then maybe just remove it, if you cannot fix it? You could post an
incremental patch / rfc later just to have it on the ML for the future, in
case someone manages to fix it.
> + *
> + * 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.
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +
> +/* Register definitions */
> +#define OV10635_VFLIP 0x381c
> +#define OV10635_VFLIP_ON (0x3 << 6)
> +#define OV10635_VFLIP_SUBSAMPLE0x1
> +#define OV10635_HMIRROR 0x381d
> +#define OV10635_HMIRROR_ON 0x3
> +#define OV10635_PID 0x300a
> +#define OV10635_VER 0x300b
Please, don't mix spaces and TABs for the same pattern, I'd just use
spaces between "#define" and the macro name above
> +
> +/* IDs */
> +#define OV10635_VERSION_REG 0xa635
> +#define OV10635_VERSION(pid, ver)(((pid) << 8) | ((ver) & 0xff))
> +
> +#define OV10635_SENSOR_WIDTH 1312
> +#define OV10635_SENSOR_HEIGHT814
> +
> +#define OV10635_MAX_WIDTH1280
> +#define OV10635_MAX_HEIGHT 800
> +
> +struct ov10635_color_format {
> + enum v4l2_mbus_pixelcode code;
> + enum v4l2_colorspace colorspace;
> +};
> +
> +struct ov10635_reg {
> + u16 reg;
> + u8 val;
> +};
> +
> +struct ov10635_priv {
> + struct v4l2_subdev subdev;
> + struct v4l2_ctrl_handlerhdl;
> + int xvclk;
> + int fps_numerator;
> + int fps_denominator;
> + const struct ov10635_color_format *cfmt;
> + int width;
> + int height;
> +};
Uhm, may I suggest to either align all the lines, or to leave all
unaligned :) Either variant would look better than the above imho :)
> +
> +/* default register setup */
> +static const struct ov10635_reg ov10635_regs_default[] = {
> + { 0x0103, 0x01 }, { 0x301b, 0xff }, { 0x301c, 0xff }, { 0x301a, 0xff },
> + { 0x3011, 0x02 }, /* drive
Re: [media] cx25821 regression from 3.9: BUG: bad unlock balance detected!
Sunday, July 14, 2013, 11:41:13 AM, you wrote: > Hi Sander, > On 07/12/2013 10:56 PM, Sander Eikelenboom wrote: >> >> Friday, May 17, 2013, 11:52:17 AM, you wrote: >> >>> On Fri May 17 2013 11:04:50 Sander Eikelenboom wrote: Friday, May 17, 2013, 10:25:24 AM, you wrote: > On Thu May 16 2013 19:41:42 Sander Eikelenboom wrote: >> Hi Hans / Mauro, >> >> With 3.10.0-rc1 (including the cx25821 changes from Hans), I get the bug >> below which wasn't present with 3.9. > How do I reproduce this? I've tried to, but I can't make this happen. > Looking at the code I can't see how it could hit this bug anyway. I'm using "motion" to grab and process 6 from the video streams of the card i have (card with 8 inputs). It seems the cx25821 underwent quite some changes between 3.9 and 3.10. >> >>> It did. >> And in the past there have been some more locking issues around mmap and media devices, although they seem to appear as circular locking dependencies and with different devices. - http://www.mail-archive.com/linux-media@vger.kernel.org/msg46217.html - Under kvm: http://www.spinics.net/lists/linux-media/msg63322.html >> >>> Neither of those are related to this issue. >> - Perhaps that running in a VM could have to do with it ? - The driver on 3.9 occasionaly gives this, probably latency related (but continues to work): cx25821: cx25821_video_wakeup: 2 buffers handled (should be 1) Could it be something double unlocking in that path ? - Is there any extra debugging i could enable that could pinpoint the issue ? >> >>> Try this patch: >> >>> diff --git a/drivers/media/pci/cx25821/cx25821-core.c >>> b/drivers/media/pci/cx25821/cx25821-core.c >>> index b762c5b..8f8d0e0 100644 >>> --- a/drivers/media/pci/cx25821/cx25821-core.c >>> +++ b/drivers/media/pci/cx25821/cx25821-core.c >>> @@ -1208,7 +1208,6 @@ void cx25821_free_buffer(struct videobuf_queue *q, >>> struct cx25821_buffer *buf) >>> struct videobuf_dmabuf *dma = videobuf_to_dma(&buf->vb); >>> >>> BUG_ON(in_interrupt()); >>> - videobuf_waiton(q, &buf->vb, 0, 0); >>> videobuf_dma_unmap(q->dev, dma); >>> videobuf_dma_free(dma); >>> btcx_riscmem_free(to_pci_dev(q->dev), &buf->risc); >> >>> I don't think the waiton is really needed for this driver. >> >>> What really should happen is that videobuf is replaced by videobuf2 in this >>> driver, but that's a fair amount of work. >> >> Hi Hans, >> >> After being busy for quite some time, i do have some spare time now. >> >> Since i'm still having trouble with this driver, is there a patch series for >> a similar driver >> that was converted to videobuf2 ? >> I don't know if it is entirely in my league, but i could give it a try when >> i have a example. > The changes done for usb/em28xx might come close. That said, the cx25821 is > already in > decent shape to convert to vb2. At least the videobuf data structures are > already in the > correct place (they are often stored in a per-filehandle struct, which is > wrong). Found the em28xx port to videobuf2 patch from Devin Heitmueller. Unfortunately the patch format isn't very neat as a example (due to reusing/renaming function parts) Apart from that the cx25821 also supports multiple "channels / subdevices". >From what i see one of the major changes is that the handling of the queue is >now internal to and handled by videobuf2 ? > include/media/videobuf2-core.h gives a reasonable overview of vb2. Like > em28xx, you > should use the vb2 helper functions (vb2_fop_* and vb2_ioctl_*) which takes a > lot > of the work off your hands. > Converting cx25821-alsa.c may be the most difficult part as it is using some > videobuf > internal functions which probably won't translate to vb2 as is. I think > videobuf is > being abused here, but I don't know off-hand what the correct approach will > be with > vb2. > I would ignore the alsa part for the time being (also the > audio/video-upstream code, > that's been disabled and without datasheets of the cx25821 I'm not sure it > can be > resurrected). > If you can get cx25821-video.c to work with vb2, then we can take a look at > the alsa > code. Will do. > Regards, > Hans -- Sander -- 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: [media] cx25821 regression from 3.9: BUG: bad unlock balance detected!
On Sunday, July 14, 2013 23:18:42 Sander Eikelenboom wrote: > > Sunday, July 14, 2013, 11:41:13 AM, you wrote: > >> Hi Hans, > >> > >> After being busy for quite some time, i do have some spare time now. > >> > >> Since i'm still having trouble with this driver, is there a patch series > >> for a similar driver > >> that was converted to videobuf2 ? > >> I don't know if it is entirely in my league, but i could give it a try > >> when i have a example. > > > The changes done for usb/em28xx might come close. That said, the cx25821 is > > already in > > decent shape to convert to vb2. At least the videobuf data structures are > > already in the > > correct place (they are often stored in a per-filehandle struct, which is > > wrong). > > Found the em28xx port to videobuf2 patch from Devin Heitmueller. > Unfortunately the patch format isn't very neat as a example (due to > reusing/renaming function parts) > > Apart from that the cx25821 also supports multiple "channels / subdevices". That in it self isn't a problem, each channel has it's own queue, which is true for videobuf as well in this driver. > From what i see one of the major changes is that the handling of the queue is > now internal to and handled by videobuf2 ? Well, that was true for videobuf as well. The whole idea behind videobuf and vb2 is to isolate the driver from the boring buffer manipulation stuff and just implement the DMA engine parts. Unfortunately, videobuf did a very poor job of that, in particular where it came to resource management (when are buffers allocated, when and how are they freed, when should the DMA engine start, when should it stop, etc.) whereas vb2 makes this much more precise and understandable. Due to the differences between videobuf and vb2 it isn't a trivial conversion. It's a 'medium level' job, I would say. A better example of this is probably the staging/media/solo6x10 driver that I did recently. It's also a PCI driver, so that helps. Still, I am not convinced you should look too much at the actual patches, more at the final result. It basically boils down to implementing the vb2_ops. Most are trivial or quite similar to what videobuf did, but the big ones to implement are always start_streaming and stop_streaming. Regards, Hans > > > include/media/videobuf2-core.h gives a reasonable overview of vb2. Like > > em28xx, you > > should use the vb2 helper functions (vb2_fop_* and vb2_ioctl_*) which takes > > a lot > > of the work off your hands. > > > Converting cx25821-alsa.c may be the most difficult part as it is using > > some videobuf > > internal functions which probably won't translate to vb2 as is. I think > > videobuf is > > being abused here, but I don't know off-hand what the correct approach will > > be with > > vb2. > > > I would ignore the alsa part for the time being (also the > > audio/video-upstream code, > > that's been disabled and without datasheets of the cx25821 I'm not sure it > > can be > > resurrected). > > > If you can get cx25821-video.c to work with vb2, then we can take a look at > > the alsa > > code. > > Will do. > > > Regards, > > > Hans > > -- > Sander -- 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: [media] cx25821 regression from 3.9: BUG: bad unlock balance detected!
On Sun, Jul 14, 2013 at 5:39 PM, Hans Verkuil wrote: >> > If you can get cx25821-video.c to work with vb2, then we can take a look >> > at the alsa >> > code. If I can make a suggestion: fix the lock problem first. The last thing you want to do is simultaneously debug a known buffer management problem at the same time you're trying to port to VB2. I panic'd my system enough times during the em28xx conversion where you really want to know whether the source is the VB2 work in progress or some other issue with buffer management. I'm not saying to not do the VB2 port -- just that you would be well served to have a reasonable stable driver before attempting to do the port. That said, I guess it's possible that digging into the code enough to understand what specifically has to be done for a VB2 port might reveal the source of the locking problem, but that's not how I would do it. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- 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: [media] cx25821 regression from 3.9: BUG: bad unlock balance detected!
On Sunday, July 14, 2013 18:44:13 Devin Heitmueller wrote: > On Sun, Jul 14, 2013 at 5:39 PM, Hans Verkuil wrote: > >> > If you can get cx25821-video.c to work with vb2, then we can take a look > >> > at the alsa > >> > code. > > If I can make a suggestion: fix the lock problem first. That's why I propose to move to vb2 :-) I looked at it for a bit and what makes locking a problem is videobuf in the first place. It's the cause of the locking problems and the solution is to get rid of it. In vb2 I understand at least who is locking what, whereas videobuf is locking and unlocking at the weirdest places. From what I remember it was not really solvable without hacking videobuf, which is something you really don't want to do. Don't ask me the details, it's been a while ago that I looked at this particular issue. I did similar vb2 conversions for go7007 and solo6x10 for pretty much the same reasons: fixing an unmaintainable locking spaghetti. In general I agree with you, but in this particular case moving to vb2 *is* the solution for the problem. Regards, Hans > The last > thing you want to do is simultaneously debug a known buffer management > problem at the same time you're trying to port to VB2. I panic'd my > system enough times during the em28xx conversion where you really want > to know whether the source is the VB2 work in progress or some other > issue with buffer management. > > I'm not saying to not do the VB2 port -- just that you would be well > served to have a reasonable stable driver before attempting to do the > port. > > That said, I guess it's possible that digging into the code enough to > understand what specifically has to be done for a VB2 port might > reveal the source of the locking problem, but that's not how I would > do it. > > Devin > > -- 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: CX23103 Video Grabber seems to be supported by cx231xx driver
On Sun, 14 Jul 2013 09:41:40 -0400 Devin Heitmueller wrote: > said, you don't need the register docs to debug a system hang. If you > don't have the time to jam a few printk() statements into the source, > then there's no point in going through the effort to get you docs. heh - yea. The thing is that I don't want to star yet another project which ends in frustration because I have to guess and miss documents. I'll see if I find the time to put some printk's and find a debug system with external logging - but this wil take some time (currently writing my diploma thesis)... /Florian -- 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
[PATCH 1/1] [media] s5p-g2d: Fix registration failure
Commit 1c1d86a1ea ("[media] v4l2: always require v4l2_dev,
rename parent to dev_parent") expects v4l2_dev to be always set.
It converted most of the drivers using the parent field of video_device
to v4l2_dev field. G2D driver did not set the parent field. Hence it got
left out. Without this patch we get the following boot warning and G2D
driver fails to register the video device.
WARNING: CPU: 0 PID: 1 at drivers/media/v4l2-core/v4l2-dev.c:775
__video_register_device+0xfc0/0x1028()
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.11.0-rc1-1-g1c3e372-dirty #9
[] (unwind_backtrace+0x0/0xf4) from []
(show_stack+0x10/0x14)
[] (show_stack+0x10/0x14) from [] (dump_stack+0x7c/0xb0)
[] (dump_stack+0x7c/0xb0) from []
(warn_slowpath_common+0x6c/0x88)
[] (warn_slowpath_common+0x6c/0x88) from []
(warn_slowpath_null+0x1c/0x24)
[] (warn_slowpath_null+0x1c/0x24) from []
(__video_register_device+0xfc0/0x1028)
[] (__video_register_device+0xfc0/0x1028) from []
(g2d_probe+0x1f8/0x398)
[] (g2d_probe+0x1f8/0x398) from []
(platform_drv_probe+0x14/0x18)
[] (platform_drv_probe+0x14/0x18) from []
(driver_probe_device+0x108/0x220)
[] (driver_probe_device+0x108/0x220) from []
(__driver_attach+0x8c/0x90)
[] (__driver_attach+0x8c/0x90) from []
(bus_for_each_dev+0x60/0x94)
[] (bus_for_each_dev+0x60/0x94) from []
(bus_add_driver+0x1c0/0x24c)
[] (bus_add_driver+0x1c0/0x24c) from []
(driver_register+0x78/0x140)
[] (driver_register+0x78/0x140) from []
(do_one_initcall+0xf8/0x144)
[] (do_one_initcall+0xf8/0x144) from []
(kernel_init_freeable+0x13c/0x1d8)
[] (kernel_init_freeable+0x13c/0x1d8) from []
(kernel_init+0xc/0x160)
[] (kernel_init+0xc/0x160) from [] (ret_from_fork+0x14/0x3c)
---[ end trace 4e0ec028b0028e02 ]---
s5p-g2d 1280.g2d: Failed to register video device
s5p-g2d: probe of 1280.g2d failed with error -22
Signed-off-by: Sachin Kamat
Cc: Hans Verkuil
---
drivers/media/platform/s5p-g2d/g2d.c |1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/s5p-g2d/g2d.c
b/drivers/media/platform/s5p-g2d/g2d.c
index 553d87e..fd6289d 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -784,6 +784,7 @@ static int g2d_probe(struct platform_device *pdev)
}
*vfd = g2d_videodev;
vfd->lock = &dev->mutex;
+ vfd->v4l2_dev = &dev->v4l2_dev;
ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
if (ret) {
v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
--
1.7.9.5
--
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
