do the editing
Want to follow up the email sent last week. Do you have needs for photo editing? We can edit 400 images within 24 hours. We are working on all kinds of ecommerce photos, jewelry photos, and the portrait images. We do cutting out and clipping path and others, and also we provide retouching for your photos, You can throw us a photo and we will do testing for you to check our quality. Thanks, Jason
for editing the photos
Want to follow up the email sent last week. Do you have needs for photo editing? We can edit 400 images within 24 hours. We are working on all kinds of ecommerce photos, jewelry photos, and the portrait images. We do cutting out and clipping path and others, and also we provide retouching for your photos, You can throw us a photo and we will do testing for you to check our quality. Thanks, Jason
for editing the photos
Want to follow up the email sent last week. Do you have needs for photo editing? We can edit 400 images within 24 hours. We are working on all kinds of ecommerce photos, jewelry photos, and the portrait images. We do cutting out and clipping path and others, and also we provide retouching for your photos, You can throw us a photo and we will do testing for you to check our quality. Thanks, Jason
Re: [PATCH 1/7] i2c: i2c-gpio: move header to platform_data
On Mon, May 14, 2018 at 11:37:20PM +0200, Wolfram Sang wrote: > > diff --git a/arch/mips/alchemy/board-gpr.c b/arch/mips/alchemy/board-gpr.c > > index 4e79dbd54a33..fa75d75b5ba9 100644 > > --- a/arch/mips/alchemy/board-gpr.c > > +++ b/arch/mips/alchemy/board-gpr.c > > @@ -29,7 +29,7 @@ > > #include > > #include > > #include > > -#include > > +#include > > #include > > #include > > #include Acked-by: James Hogan <jho...@kernel.org> Cheers James signature.asc Description: PGP signature
,Your urgent confirmation
Attn: Beneficiary, We have contacted the Federal Ministry of Finance on your Behalf and they have brought a solution to your problem by coordinating your payment in total (10,000,000.00) Ten Million Dollars in an atm card which you can use to withdraw money from any ATM MACHINE CENTER anywhere in the world with a maximum of 1 Dollars daily. You now have the lawful right to claim your fund in an atm card. Since the Federal Bureau of Investigation is involved in this transaction, you have to be rest assured for this is 100% risk free it is our duty to protect the American Citizens, European Citizens, Asian Citizen. All I want you to do is to contact the atm card CENTER Via email or call the office telephone number one of the Consultant will assist you for their requirements to proceed and procure your Approval Slip on your behalf. CONTACT INFORMATION NAME: James Williams EMAIL: paymasterofficed...@gmail.com Do contact us with your details: Full name// Address// Age// Telephone Numbers// Occupation// Your Country// Bank Details// So your files would be updated after which the Delivery of your atm card will be affected to your designated home Address without any further delay and the bank will transfer your funds in total (10,000,000.00) Ten Million Dollars to your Bank account. We will reply you with the secret code (1600 atm card). We advice you get back to the payment office after you have contacted the ATM SWIFT CARD CENTER and we do await your response so we can move on with our Investigation and make sure your ATM SWIFT CARD gets to you. Best Regards James Williams Paymaster General Federal Republic Of Nigeri
[GIT PULL] Remove metag architecture
Hi Arnd, On Fri, Feb 23, 2018 at 01:26:09PM +0100, Arnd Bergmann wrote: > On Fri, Feb 23, 2018 at 12:02 PM, James Hogan <jho...@kernel.org> wrote: > > I'm happy to put v2 in linux-next now (only patch 4 has changed, I just > > sent an updated version), and send you a pull request early next week so > > you can take it from there. The patches can't be directly applied with > > git-am anyway thanks to the -D option to make them more concise. > > > > Sound okay? > > Yes, sounds good, thanks! As discussed, here is a tagged branch to remove arch/metag and dependent drivers. Its basically v2 with some acks added. Cheers James The following changes since commit 91ab883eb21325ad80f3473633f794c78ac87f51: Linux 4.16-rc2 (2018-02-18 17:29:42 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/jhogan/metag.git tags/metag_remove for you to fetch changes up to ef9fb83815db7d7e03da9a0904b4ef352e633922: i2c: img-scb: Drop METAG dependency (2018-02-26 14:58:09 +) Remove metag architecture These patches remove the metag architecture and tightly dependent drivers from the kernel. With the 4.16 kernel the ancient gcc 4.2.4 based metag toolchain we have been using is hitting compiler bugs, so now seems a good time to drop it altogether. ---- James Hogan (13): metag: Remove arch/metag/ docs: Remove metag docs docs: Remove remaining references to metag Drop a bunch of metag references irqchip: Remove metag irqchip drivers clocksource: Remove metag generic timer driver tty: Remove metag DA TTY and console driver MAINTAINERS/CREDITS: Drop METAG ARCHITECTURE pinctrl: Drop TZ1090 drivers gpio: Drop TZ1090 drivers watchdog: imgpdc: Drop METAG dependency media: img-ir: Drop METAG dependency i2c: img-scb: Drop METAG dependency CREDITS|5 + Documentation/00-INDEX |2 - Documentation/admin-guide/kernel-parameters.txt|4 - Documentation/dev-tools/kmemleak.rst |2 +- .../devicetree/bindings/gpio/gpio-tz1090-pdc.txt | 45 - .../devicetree/bindings/gpio/gpio-tz1090.txt | 88 - Documentation/devicetree/bindings/metag/meta.txt | 30 - .../bindings/pinctrl/img,tz1090-pdc-pinctrl.txt| 127 -- .../bindings/pinctrl/img,tz1090-pinctrl.txt| 227 --- .../features/core/BPF-JIT/arch-support.txt |1 - .../core/generic-idle-thread/arch-support.txt |1 - .../features/core/jump-labels/arch-support.txt |1 - .../features/core/tracehook/arch-support.txt |1 - .../features/debug/KASAN/arch-support.txt |1 - .../debug/gcov-profile-all/arch-support.txt|1 - Documentation/features/debug/kgdb/arch-support.txt |1 - .../debug/kprobes-on-ftrace/arch-support.txt |1 - .../features/debug/kprobes/arch-support.txt|1 - .../features/debug/kretprobes/arch-support.txt |1 - .../features/debug/optprobes/arch-support.txt |1 - .../features/debug/stackprotector/arch-support.txt |1 - .../features/debug/uprobes/arch-support.txt|1 - .../debug/user-ret-profiler/arch-support.txt |1 - .../features/io/dma-api-debug/arch-support.txt |1 - .../features/io/dma-contiguous/arch-support.txt|1 - .../features/io/sg-chain/arch-support.txt |1 - .../features/lib/strncasecmp/arch-support.txt |1 - .../locking/cmpxchg-local/arch-support.txt |1 - .../features/locking/lockdep/arch-support.txt |1 - .../locking/queued-rwlocks/arch-support.txt|1 - .../locking/queued-spinlocks/arch-support.txt |1 - .../locking/rwsem-optimized/arch-support.txt |1 - .../features/perf/kprobes-event/arch-support.txt |1 - .../features/perf/perf-regs/arch-support.txt |1 - .../features/perf/perf-stackdump/arch-support.txt |1 - .../sched/membarrier-sync-core/arch-support.txt|1 - .../features/sched/numa-balancing/arch-support.txt |1 - .../seccomp/seccomp-filter/arch-support.txt|1 - .../time/arch-tick-broadcast/arch-support.txt |1 - .../features/time/clockevents/arch-support.txt |1 - .../time/context-tracking/arch-support.txt |1 - .../features/time/irq-time-acct/arch-support.txt |1 - .../time/modern-timekeeping/arch-support.txt |1 - .../features/time/virt-cpuacct/arch-support.txt|1 - .../features/vm/ELF-ASLR/arch-support.txt |1 - .../features/vm/PG_uncached/arch-support.txt |1 - Documentation/features/vm/THP/arch-support.txt |1 - Documentation/features/vm/TLB/arch-support.txt |1 - .../fe
Re: [PATCH 00/13] Remove metag architecture
On Fri, Feb 23, 2018 at 11:26:58AM +0100, Arnd Bergmann wrote: > On Thu, Feb 22, 2018 at 12:38 AM, James Hogan <jho...@kernel.org> wrote: > > So lets call it a day and drop the Meta architecture port from the > > kernel. RIP Meta. > > Since I brought up the architecture removal independently, I could > pick this up into a git tree that also has the removal of some of the > other architectures. > > I see your tree is part of linux-next, so you could also just put it > in there and send a pull request at the merge window if you prefer. > > The only real reason I see for a shared git tree would be to avoid > conflicts when we touch the same Kconfig files or #ifdefs in driver, > but Meta only appears in > > config FRAME_POINTER > bool "Compile the kernel with frame pointers" > depends on DEBUG_KERNEL && \ > (CRIS || M68K || FRV || UML || \ > SUPERH || BLACKFIN || MN10300 || METAG) || \ > ARCH_WANT_FRAME_POINTERS > > and > > include/trace/events/mmflags.h:#elif defined(CONFIG_PARISC) || > defined(CONFIG_METAG) || defined(CONFIG_IA64) > > so there is little risk. I'm happy to put v2 in linux-next now (only patch 4 has changed, I just sent an updated version), and send you a pull request early next week so you can take it from there. The patches can't be directly applied with git-am anyway thanks to the -D option to make them more concise. Sound okay? Thanks James signature.asc Description: Digital signature
Re: [PATCH 00/13] Remove metag architecture
On Thu, Feb 22, 2018 at 10:26:54AM +0100, Peter Zijlstra wrote: > On Wed, Feb 21, 2018 at 11:38:12PM +0000, James Hogan wrote: > > So lets call it a day and drop the Meta architecture port from the > > kernel. RIP Meta. > > So long, and thanks for all the fish! > > Nice cleanup though, most welcome :-) I thought you might like it ;-) > Acked-by: Peter Zijlstra (Intel) <pet...@infradead.org> Thanks James signature.asc Description: Digital signature
[PATCH 00/13] Remove metag architecture
These patches remove the metag architecture and tightly dependent drivers from the kernel. With the 4.16 kernel the ancient gcc 4.2.4 based metag toolchain we have been using is hitting compiler bugs, so now seems a good time to drop it altogether. Quoting from patch 1: The earliest Meta architecture port of Linux I have a record of was an import of a Meta port of Linux v2.4.1 in February 2004, which was worked on significantly over the next few years by Graham Whaley, Will Newton, Matt Fleming, myself and others. Eventually the port was merged into mainline in v3.9 in March 2013, not long after Imagination Technologies bought MIPS Technologies and shifted its CPU focus over to the MIPS architecture. As a result, though the port was maintained for a while, kept on life support for a while longer, and useful for testing a few specific drivers for which I don't have ready access to the equivalent MIPS hardware, it is now essentially dead with no users. It is also stuck using an out-of-tree toolchain based on GCC 4.2.4 which is no longer maintained, now struggles to build modern kernels due to toolchain bugs, and doesn't itself build with a modern GCC. The latest buildroot port is still using an old uClibc snapshot which is no longer served, and the latest uClibc doesn't build with GCC 4.2.4. So lets call it a day and drop the Meta architecture port from the kernel. RIP Meta. Cc: Guenter Roeck <li...@roeck-us.net> Cc: Jonathan Corbet <cor...@lwn.net> Cc: Steven Rostedt <rost...@goodmis.org> Cc: Ingo Molnar <mi...@redhat.com> Cc: Peter Zijlstra <pet...@infradead.org> Cc: Arnaldo Carvalho de Melo <a...@kernel.org> Cc: Alexander Shishkin <alexander.shish...@linux.intel.com> Cc: Jiri Olsa <jo...@redhat.com> Cc: Namhyung Kim <namhy...@kernel.org> Cc: Thomas Gleixner <t...@linutronix.de> Cc: Jason Cooper <ja...@lakedaemon.net> Cc: Marc Zyngier <marc.zyng...@arm.com> Cc: Daniel Lezcano <daniel.lezc...@linaro.org> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org> Cc: Jiri Slaby <jsl...@suse.com> Cc: Linus Walleij <linus.wall...@linaro.org> Cc: Wim Van Sebroeck <w...@linux-watchdog.org> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Cc: Mauro Carvalho Chehab <mche...@kernel.org> Cc: Wolfram Sang <w...@the-dreams.de> Cc: linux-me...@vger.kernel.org Cc: linux-...@vger.kernel.org Cc: linux...@kvack.org Cc: linux-g...@vger.kernel.org Cc: linux-watch...@vger.kernel.org Cc: linux-media@vger.kernel.org Cc: linux-...@vger.kernel.org James Hogan (13): metag: Remove arch/metag/ docs: Remove metag docs docs: Remove remaining references to metag Drop a bunch of metag references irqchip: Remove metag irqchip drivers clocksource: Remove metag generic timer driver tty: Remove metag DA TTY and console driver MAINTAINERS/CREDITS: Drop METAG ARCHITECTURE pinctrl: Drop TZ1090 drivers gpio: Drop TZ1090 drivers watchdog: imgpdc: Drop METAG dependency media: img-ir: Drop METAG dependency i2c: img-scb: Drop METAG dependency CREDITS|5 + Documentation/00-INDEX |2 - Documentation/admin-guide/kernel-parameters.txt|4 - Documentation/dev-tools/kmemleak.rst |2 +- .../devicetree/bindings/gpio/gpio-tz1090-pdc.txt | 45 - .../devicetree/bindings/gpio/gpio-tz1090.txt | 88 - Documentation/devicetree/bindings/metag/meta.txt | 30 - .../bindings/pinctrl/img,tz1090-pdc-pinctrl.txt| 127 -- .../bindings/pinctrl/img,tz1090-pinctrl.txt| 227 --- .../features/core/BPF-JIT/arch-support.txt |1 - .../core/generic-idle-thread/arch-support.txt |1 - .../features/core/jump-labels/arch-support.txt |1 - .../features/core/tracehook/arch-support.txt |1 - .../features/debug/KASAN/arch-support.txt |1 - .../debug/gcov-profile-all/arch-support.txt|1 - Documentation/features/debug/kgdb/arch-support.txt |1 - .../debug/kprobes-on-ftrace/arch-support.txt |1 - .../features/debug/kprobes/arch-support.txt|1 - .../features/debug/kretprobes/arch-support.txt |1 - .../features/debug/optprobes/arch-support.txt |1 - .../features/debug/stackprotector/arch-support.txt |1 - .../features/debug/uprobes/arch-support.txt|1 - .../debug/user-ret-profiler/arch-support.txt |1 - .../features/io/dma-api-debug/arch-support.txt |1 - .../features/io/dma-contiguous/arch-support.txt|1 - .../features/io/sg-chain/arch-support.txt |1 - .../features/lib/strncasecmp/arch-support.txt |1 - .../locking/cmpxchg-local/arch-support.txt |1 - .../features/locking/lockdep/arch-support.txt |1 - .../locking/queued-rwlocks/arch-support.txt|1 - .../locking/queued-spinlocks/arch-support.txt
[PATCH 12/13] media: img-ir: Drop METAG dependency
Now that arch/metag/ has been removed, remove the METAG dependency from the IMG IR device driver. The hardware is also present on MIPS SoCs so the driver still has value. Signed-off-by: James Hogan <jho...@kernel.org> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com> Cc: Mauro Carvalho Chehab <mche...@kernel.org> Cc: linux-media@vger.kernel.org Cc: linux-me...@vger.kernel.org --- drivers/media/rc/img-ir/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig index a896d3c83a1c..d2c6617d468e 100644 --- a/drivers/media/rc/img-ir/Kconfig +++ b/drivers/media/rc/img-ir/Kconfig @@ -1,7 +1,7 @@ config IR_IMG tristate "ImgTec IR Decoder" depends on RC_CORE - depends on METAG || MIPS || COMPILE_TEST + depends on MIPS || COMPILE_TEST select IR_IMG_HW if !IR_IMG_RAW help Say Y or M here if you want to use the ImgTec infrared decoder -- 2.13.6
Re: [PATCH 04/12] media: img-ir-hw: fix one kernel-doc comment
On Wed, Nov 29, 2017 at 05:46:25AM -0500, Mauro Carvalho Chehab wrote: > Needed to suppress the following warnings: > drivers/media/rc/img-ir/img-ir-hw.c:351: warning: No description found > for parameter 'reg_timings' > drivers/media/rc/img-ir/img-ir-hw.c:351: warning: Excess function > parameter 'timings' description in 'img_ir_decoder_convert' > > Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com> Very true. Acked-by: James Hogan <jho...@kernel.org> Thanks James > --- > drivers/media/rc/img-ir/img-ir-hw.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/rc/img-ir/img-ir-hw.c > b/drivers/media/rc/img-ir/img-ir-hw.c > index f54bc5d23893..ec4ded84cd17 100644 > --- a/drivers/media/rc/img-ir/img-ir-hw.c > +++ b/drivers/media/rc/img-ir/img-ir-hw.c > @@ -339,7 +339,7 @@ static void img_ir_decoder_preprocess(struct > img_ir_decoder *decoder) > /** > * img_ir_decoder_convert() - Generate internal timings in decoder. > * @decoder: Decoder to be converted to internal timings. > - * @timings: Timing register values. > + * @reg_timings: Timing register values. > * @clock_hz:IR clock rate in Hz. > * > * Fills out the repeat timings and timing register values for a specific > clock > -- > 2.14.3 > signature.asc Description: Digital signature
Re: [PATCH 08/29] drivers, md: convert mddev.active from atomic_t to refcount_t
On Tue, 2017-03-14 at 12:29 +, Reshetova, Elena wrote: > > Elena Reshetova <elena.reshet...@intel.com> writes: > > > > > refcount_t type and corresponding API should be > > > used instead of atomic_t when the variable is used as > > > a reference counter. This allows to avoid accidental > > > refcounter overflows that might lead to use-after-free > > > situations. > > > > > > Signed-off-by: Elena Reshetova <elena.reshet...@intel.com> > > > Signed-off-by: Hans Liljestrand <ishkam...@gmail.com> > > > Signed-off-by: Kees Cook <keesc...@chromium.org> > > > Signed-off-by: David Windsor <dwind...@gmail.com> > > > --- > > > drivers/md/md.c | 6 +++--- > > > drivers/md/md.h | 3 ++- > > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > When booting linux-next (specifically 5be4921c9958ec) I'm seeing > > the > > backtrace below. I suspect this patch is just exposing an existing > > issue? > > Yes, we have actually been following this issue in the another > thread. > It looks like the object is re-used somehow, but I can't quite > understand how just by reading the code. > This was what I put into the previous thread: > > "The log below indicates that you are using your refcounter in a bit > weird way in mddev_find(). > However, I can't find the place (just by reading the code) where you > would increment refcounter from zero (vs. setting it to one). > It looks like you either iterate over existing nodes (and increment > their counters, which should be >= 1 at the time of increment) or > create a new node, but then mddev_init() sets the counter to 1. " > > If you can help to understand what is going on with the object > creation/destruction, would be appreciated! > > Also Shaohua Li stopped this patch coming from his tree since the > issue was caught at that time, so we are not going to merge this > until we figure it out. Asking on the correct list (dm-devel) would have got you the easy answer: The refcount behind mddev->active is a genuine atomic. It has refcount properties but only if the array fails to initialise (in that case, final put kills it). Once it's added to the system as a gendisk, it cannot be freed until md_free(). Thus its ->active count can go to zero (when it becomes inactive; usually because of an unmount). On a simple allocation regardless of outcome, the last executed statement in md_alloc is mddev_put(): that destroys the device if we didn't manage to create it or returns 0 and adds an inactive device to the system which the user can get with mddev_find(). James
Re: [PATCH v5 02/18] [media] img-ir: use new wakeup_protocols sysfs mechanism
Hi Sean, On Tue, Dec 13, 2016 at 07:54:16AM +, Sean Young wrote: > So that leaves the question open of whether we want to guess the protocol > variant from the scancode for img-ir or if we can live with having to > select this using wakeup_protocols. Having to do this does solve the issue > of the driver guessing the wrong protocol if the higher bits happen to be > 0 in the scancode. I've received confirmation that pistachio doesn't yet support suspend in mainline, in which case there can never be any real users of the old semantics on current/old mainline kernel versions. So I'm fine with it changing. Cheers James signature.asc Description: Digital signature
Re: [PATCH v5 02/18] [media] img-ir: use new wakeup_protocols sysfs mechanism
Hi Sean (and Sifan),
On Mon, Dec 12, 2016 at 09:13:43PM +, Sean Young wrote:
> Rather than guessing what variant a scancode is from its length,
> use the new wakeup_protocol.
>
> Signed-off-by: Sean Young <s...@mess.org>
> Cc: James Hogan <james.ho...@imgtec.com>
> Cc: Sifan Naeem <sifan.na...@imgtec.com>
> ---
> drivers/media/rc/img-ir/img-ir-hw.c| 2 +-
> drivers/media/rc/img-ir/img-ir-hw.h| 2 +-
> drivers/media/rc/img-ir/img-ir-jvc.c | 2 +-
> drivers/media/rc/img-ir/img-ir-nec.c | 6 +++---
> drivers/media/rc/img-ir/img-ir-rc5.c | 2 +-
> drivers/media/rc/img-ir/img-ir-rc6.c | 2 +-
> drivers/media/rc/img-ir/img-ir-sanyo.c | 2 +-
> drivers/media/rc/img-ir/img-ir-sharp.c | 2 +-
> drivers/media/rc/img-ir/img-ir-sony.c | 11 +++
> 9 files changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
> b/drivers/media/rc/img-ir/img-ir-hw.c
> index 1a0811d..841d9d7 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.c
> +++ b/drivers/media/rc/img-ir/img-ir-hw.c
> @@ -488,7 +488,7 @@ static int img_ir_set_filter(struct rc_dev *dev, enum
> rc_filter_type type,
> /* convert scancode filter to raw filter */
> filter.minlen = 0;
> filter.maxlen = ~0;
> - ret = hw->decoder->filter(sc_filter, , hw->enabled_protocols);
> + ret = hw->decoder->filter(sc_filter, , dev->wakeup_protocol);
According to patch 1, wakeup_protocol can always be set to
RC_TYPE_UNKNOWN using the protocol "none", but this function is used for
the normal filter too. AFAICT that would make it impossible to set a
normal filter without first setting the (new) wakeup protocol too.
Technically when type == RC_FILTER_NORMAL, the protocol should be based
on enabled_protocols, which should be set to a single protocol group.
I'll also note that enforcing that a wakeup protocol is set before
setting the wakeup filter (in patch 1 which I'm not Cc'd on) is an
incompatible API change. The old API basically meant that a mask of 0
disabled the wakeup filter, and there was no wakeup_protocol to set.
If wakeup filters can be changed to still be writable when wakeup
protocol is not set, then I suppose this driver could do something like:
if (type == RC_TYPE_NORMAL) {
use hw->enabled_protocols;
} else if (type == RC_TYPE_WAKEUP) {
if (dev->wakeup_protocol == RC_TYPE_UNKNOWN)
use hw->enabled_protocols;
else
use 1 << dev->wakeup_protocol;
}
Clearly allowing a wakeup filter with no protocol is not ideal though.
It probably isn't a big deal from the img-ir point of view for those
semantics to change slightly. The TZ1090 SoC I originally wrote the
driver for is practically obsolete from my point of view, and the common
clk and power management drivers to make this driver/feature functional
was never merged into mainline.
Sifan: Does the MIPS pistachio SoC support wake up using img-ir, and
does it even support suspend to RAM in mainline yet? If not then its
impossible to utilise the wake filters in current kernels and changing
the semantics is probably fine.
Cheers
James
> if (ret)
> goto unlock;
> dev_dbg(priv->dev, "IR raw %sfilter=%016llx & %016llx\n",
> diff --git a/drivers/media/rc/img-ir/img-ir-hw.h
> b/drivers/media/rc/img-ir/img-ir-hw.h
> index 91a2977..e1959ddc 100644
> --- a/drivers/media/rc/img-ir/img-ir-hw.h
> +++ b/drivers/media/rc/img-ir/img-ir-hw.h
> @@ -179,7 +179,7 @@ struct img_ir_decoder {
> int (*scancode)(int len, u64 raw, u64 enabled_protocols,
> struct img_ir_scancode_req *request);
> int (*filter)(const struct rc_scancode_filter *in,
> - struct img_ir_filter *out, u64 protocols);
> + struct img_ir_filter *out, enum rc_type protocol);
> };
>
> extern struct img_ir_decoder img_ir_nec;
> diff --git a/drivers/media/rc/img-ir/img-ir-jvc.c
> b/drivers/media/rc/img-ir/img-ir-jvc.c
> index d3e2fc0..10b302c 100644
> --- a/drivers/media/rc/img-ir/img-ir-jvc.c
> +++ b/drivers/media/rc/img-ir/img-ir-jvc.c
> @@ -30,7 +30,7 @@ static int img_ir_jvc_scancode(int len, u64 raw, u64
> enabled_protocols,
>
> /* Convert JVC scancode to JVC data filter */
> static int img_ir_jvc_filter(const struct rc_scancode_filter *in,
> - struct img_ir_filter *out, u64 protocols)
> + struct img_ir_filter *out, enum rc_type protocol)
> {
> unsigned int cust, data;
> unsigned int cust_m, data_m;
> diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
> b/drivers/media/rc/img-ir/img-ir-nec.c
> index
Re: [Ksummit-discuss] Including images on Sphinx documents
On Mon, 2016-11-21 at 12:06 -0200, Mauro Carvalho Chehab wrote: > Em Mon, 21 Nov 2016 11:39:41 +0100 > Johannes Berg <johan...@sipsolutions.net> escreveu: > > On Sat, 2016-11-19 at 10:15 -0700, Jonathan Corbet wrote: > > > > > Rather than beating our heads against the wall trying to convert > > > between various image formats, maybe we need to take a step > > > back. We're trying to build better documentation, and there is > > > certainly a place for diagrams and such in that > > > documentation. Johannes was asking about it for the 802.11 docs, > > > and I know Paul has run into these issues with the RCU docs as > > > well. Might there be a tool or an extension out there that would > > > allow us to express these diagrams in a text-friendly, editable > > > form? > > > > > > With some effort, I bet we could get rid of a number of the > > > images, and perhaps end up with something that makes sense when > > > read in the .rst source files as an extra benefit. > > > > I tend to agree, and I think that having this readable in the text > > would be good. > > > > You had pointed me to this plugin before > > https://pythonhosted.org/sphinxcontrib-aafig/ > > > > but I don't think it can actually represent any of the pictures. > > No, but there are some ascii art images inside some txt/rst files > and inside some kernel-doc comments. We could either use the above > extension for them or to convert into some image. The ascii art > images I saw seem to be diagrams, so Graphviz would allow replacing > most of them, if not all. Please don't replace ASCII art that effectively conveys conceptual diagrams. If you do, we'll wind up in situations where someone hasn't built the docs and doesn't possess the tools to see a diagram that was previously shown by every text editor (or can't be bothered to dig out the now separate file). In the name of creating "prettier" diagrams (and final doc), we'll have damaged capacity to understand stuff by just reading the source if this diagram is in kernel doc comments. I think this is a good application of "if it ain't broke, don't fix it". James -- 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: [Ksummit-discuss] Including images on Sphinx documents
On Thu, 2016-11-17 at 13:16 -0200, Mauro Carvalho Chehab wrote: > Hi Ted, > > Em Thu, 17 Nov 2016 09:52:44 -0500 > Theodore Ts'o <ty...@mit.edu> escreveu: > > > On Thu, Nov 17, 2016 at 12:07:15PM +0100, Arnd Bergmann wrote: > > > [adding Linus for clarification] > > > > > > I understood the concern as being about binary files that you > > > cannot > > > modify with classic 'patch', which is a separate issue. > > > > I think the other complaint is that the image files aren't "source" > > in > > the proper term, since they are *not* the preferred form for > > modification --- that's the svg files. Beyond the license > > compliance > > issues (which are satisified because the .svg files are included in > > the git tree), there is the SCM cleaniless argument of not > > including > > generated files in the distribution, since this increases the > > opportunites for the "real" source file and the generated source > > file > > to get out of sync. (As just one example, if the patch can't > > represent the change to binary file.) > > > > I do check in generated files on occasion --- usually because I > > don't > > trust autoconf to be a stable in terms of generating a correct > > configure file from a configure.in across different versions of > > autoconf and different macro libraries that might be installed on > > the > > system. So this isn't a hard and fast rule by any means (although > > Linus may be more strict than I on that issue). > > > > I don't understand why it's so terrible to have generate the image > > file from the .svg file in a Makefile rule, and then copy it > > somewhere > > else if Sphinx is too dumb to fetch it from the normal location? > > The images whose source are in .svg are now generated via Makefile > for the PDF output (after my patches, already applied to the docs > -next > tree). > > So, the problem that remains is for those images whose source > is a bitmap. If we want to stick with the Sphinx supported formats, > we have only two options for bitmaps: jpg or png. We could eventually > use uuencode or base64 to make sure that the patches won't use > git binary diff extension, or, as Arnd proposed, use a portable > bitmap format, in ascii, converting via Makefile, but losing > the alpha channel with makes the background transparent. If it can use svg, why not use that? SVG files can be a simple xml wrapper around a wide variety of graphic image formats which are embedded in the svg using the data-uri format, you know ... Anything that handles SVGs should be able to handle all the embeddable image formats, which should give you a way around image restrictions whatever it is would otherwise have. James -- 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] subsystem:linux-media CVE-2016-5400
This patch addresses CVE-2016-5400, a local DOS vulnerability caused by a
memory leak in the airspy usb device driver. The vulnerability is triggered
when more than 64 usb devices register with v4l2 of type VFL_TYPE_SDR or
VFL_TYPE_SUBDEV.A badusb device can emulate 64 of these devices then
through continual emulated connect/disconnect of the 65th device, cause the
kernel to run out of RAM and crash the kernel. The vulnerability exists in
kernel versions from 3.17 to current 4.7.
The memory leak is caused by the probe function of the airspy driver
mishandeling errors and not freeing the corresponding control structures
when an error occours registering the device to v4l2 core.
Signed-off-by: James Patrick-Evans <ja...@jmp-e.com>
---
drivers/media/usb/airspy/airspy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/airspy/airspy.c
b/drivers/media/usb/airspy/airspy.c
index 87c1293..6c3ac8b 100644
--- a/drivers/media/usb/airspy/airspy.c
+++ b/drivers/media/usb/airspy/airspy.c
@@ -1072,7 +1072,7 @@ static int airspy_probe(struct usb_interface *intf,
if (ret) {
dev_err(s->dev, "Failed to register as video device (%d)\n",
ret);
- goto err_unregister_v4l2_dev;
+ goto err_free_controls;
}
dev_info(s->dev, "Registered as %s\n",
video_device_node_name(>vdev));
--
1.9.1
--
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: IR remote stopped working in kernels 4.5 and 4.6
On Mon, 2016-07-04 at 23:08 +0200, Heiner Kallweit wrote: > Am 04.07.2016 um 22:36 schrieb James Bottomley: > > This looks to be a problem with the rc subsystem. The IR > > controller in question is part of a cx8800 atsc card. In the 4.4 > > kernel, where it works, this is what ir-keytable says: > > > > Found /sys/class/rc/rc0/ (/dev/input/event12) with: > > Driver cx88xx, table rc-hauppauge > > Supported protocols: other lirc rc-5 jvc sony nec sanyo mce-kbd > > rc-6 sharp xmp > > Enabled protocols: lirc nec > > Name: cx88 IR (pcHDTV HD3000 HDTV) > > bus: 1, vendor/product: 7063:3000, version: 0x0001 > > Repeat delay = 500 ms, repeat period = 125 ms > > > > And in 4.6, where it doesn't work: > > > > Found /sys/class/rc/rc0/ (/dev/input/event12) with: > > Driver cx88xx, table rc-hauppauge > > Supported protocols: lirc > > Enabled protocols: lirc > > Name: cx88 IR (pcHDTV HD3000 HDTV) > > bus: 1, vendor/product: 7063:3000, version: 0x0001 > > Repeat delay = 500 ms, repeat period = 125 ms > > > > The particular remote in question seems to require the nec protocol > > to work and the failure in 4.5 and 4.6 is having any supported > > protocols at all. I can get the remote to start working again by > > adding the nec protocol: > > > > echo nec > /sys/class/rc/rc0/protocols > > > > But it would be nice to have this happen by default rather than > > having to add yet another work around init script. > > > Meanwhile decoder modules are loaded on demand only. This can be done > automatically w/o the need for additional init scripts. > > If /etc/rc_maps.cfg includes a keymap with type NEC then the nec > decoder module is loaded automatically. > > My rc_maps.cfg looks like this (and causes the SONY decoder module to > be loaded automatically): > > #driver tablefile > * *sony-rm-sx800 > > And the keymap: > > # table sony-rm-sx800, type SONY > 0x110030KEY_PREVIOUS > 0x110031KEY_NEXT > 0x110033KEY_BACK > .. > .. Well, to work in the 4.4 kernel, the rc_maps.cfg names a table with the nec controller, so it seems that whatever is supposed to trigger autoloading isn't. Is ir-keytable supposed to do this? The current debian testing one is Package: ir-keytable Source: v4l-utils Version: 1.10.1-1 Which seems to be the most current one. James -- 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
IR remote stopped working in kernels 4.5 and 4.6
This looks to be a problem with the rc subsystem. The IR controller in question is part of a cx8800 atsc card. In the 4.4 kernel, where it works, this is what ir-keytable says: Found /sys/class/rc/rc0/ (/dev/input/event12) with: Driver cx88xx, table rc-hauppauge Supported protocols: other lirc rc-5 jvc sony nec sanyo mce-kbd rc-6 sharp xmp Enabled protocols: lirc nec Name: cx88 IR (pcHDTV HD3000 HDTV) bus: 1, vendor/product: 7063:3000, version: 0x0001 Repeat delay = 500 ms, repeat period = 125 ms And in 4.6, where it doesn't work: Found /sys/class/rc/rc0/ (/dev/input/event12) with: Driver cx88xx, table rc-hauppauge Supported protocols: lirc Enabled protocols: lirc Name: cx88 IR (pcHDTV HD3000 HDTV) bus: 1, vendor/product: 7063:3000, version: 0x0001 Repeat delay = 500 ms, repeat period = 125 ms The particular remote in question seems to require the nec protocol to work and the failure in 4.5 and 4.6 is having any supported protocols at all. I can get the remote to start working again by adding the nec protocol: echo nec > /sys/class/rc/rc0/protocols But it would be nice to have this happen by default rather than having to add yet another work around init script. James -- 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: Build regressions/improvements in v4.5-rc7
Hi,
On Mon, Mar 07, 2016 at 10:01:09AM +0100, Geert Uytterhoeven wrote:
> On Mon, Mar 7, 2016 at 9:55 AM, Geert Uytterhoeven <ge...@linux-m68k.org>
> wrote:
> > JFYI, when comparing v4.5-rc7[1] to v4.5-rc6[3], the summaries are:
> > - build errors: +8/-7
> + error: debugfs.c: undefined reference to `clk_round_rate': =>
> .text+0x11b9e0)
>
> arm-randconfig
>
> While looking for more context, I noticed another regression that fell through
> the cracks of my script:
>
> arch/arm/kernel/head.o: In function `stext':
> (.head.text+0x40): undefined reference to `CONFIG_PHYS_OFFSET'
> drivers/built-in.o: In function `v4l2_clk_set_rate':
> debugfs.c:(.text+0x11b9e0): undefined reference to `clk_round_rate'
>
> + error: misc.c: undefined reference to `ftrace_likely_update': =>
> .text+0x714), .text+0x94c), .text+0x3b8), .text+0xc10)
>
> sh-randconfig
>
> arch/sh/boot/compressed/misc.o: In function `lzo1x_decompress_safe':
> misc.c:(.text+0x3b8): undefined reference to `ftrace_likely_update'
> misc.c:(.text+0x714): undefined reference to `ftrace_likely_update'
> misc.c:(.text+0x94c): undefined reference to `ftrace_likely_update'
> arch/sh/boot/compressed/misc.o: In function `unlzo.constprop.2':
> misc.c:(.text+0xc10): undefined reference to `ftrace_likely_update'
>
> + /tmp/cc52LvuK.s: Error: can't resolve `_start' {*UND* section} -
> `L0^A' {.text section}: => 41, 403
> + /tmp/ccHfoDA4.s: Error: can't resolve `_start' {*UND* section} -
> `L0^A' {.text section}: => 43
> + /tmp/cch1r0UQ.s: Error: can't resolve `_start' {*UND* section} -
> `L0^A' {.text section}: => 49, 378
> + /tmp/ccoHdFI8.s: Error: can't resolve `_start' {*UND* section} -
> `L0^A' {.text section}: => 43
>
> mips-allnoconfig
> bigsur_defconfig
> malta_defconfig
> cavium_octeon_defconfig
>
> Not really new, but it would be great if the MIPS people could get this
> fixed for the final release.
This would appear to be related to the ld version check for VDSO
failing.
awk: /home/kisskb/slave/src/scripts/ld-version.sh: line 4: regular expression
compile failed (missing '(')
.*)
/bin/sh: 1: [: -lt: unexpected operator
I.e. this line:
gsub(".*)", "");
appears to be trying to turn e.g. "GNU ld (Gentoo 2.25.1 p1.1) 2.25.1"
into " 2.25.1", so perhaps the bracket should be escaped. What version
of awk is it using? (GNU Awk 4.0.2 works for me).
Can somebody experiencing this please try:
${CROSS_COMPILE}ld --version | ./scripts/ld-version.sh
with the following patch, to see if it helps.
Thanks
James
diff --git a/scripts/ld-version.sh b/scripts/ld-version.sh
index 198580d245e0..1659b409ef10 100755
--- a/scripts/ld-version.sh
+++ b/scripts/ld-version.sh
@@ -1,7 +1,7 @@
#!/usr/bin/awk -f
# extract linker version number from stdin and turn into single number
{
- gsub(".*)", "");
+ gsub(".*\\)", "");
split($1,a, ".");
print a[1]*1000 + a[2]*10 + a[3]*1 + a[4]*100 + a[5];
exit
signature.asc
Description: Digital signature
Re: [PATCH v5 1/3] create SMAF module
On Wed, 21 Oct 2015, Benjamin Gaignard wrote: > Secure Memory Allocation Framework goal is to be able > to allocate memory that can be securing. > There is so much ways to allocate and securing memory that SMAF > doesn't do it by itself but need help of additional modules. > To be sure to use the correct allocation method SMAF implement > deferred allocation (i.e. allocate memory when only really needed) > > Allocation modules (smaf-alloctor.h): > SMAF could manage with multiple allocation modules at same time. > To select the good one SMAF call match() to be sure that a module > can allocate memory for a given list of devices. It is to the module > to check if the devices are compatible or not with it allocation > method. > > Securing module (smaf-secure.h): > The way of how securing memory it is done is platform specific. > Secure module is responsible of grant/revoke memory access. > This documentation is highly inadequate. What does "allocate memory that can be securing" mean? -- James Morris <jmor...@namei.org> -- 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: uvc-1.5
On 09/30/15 19:13, Mauro Carvalho Chehab wrote: Em Tue, 29 Sep 2015 19:06:18 -0400 James <bjloc...@lockie.ca> escreveu: When is the Linux kernel going to support uvc-1.5? It was made a standard on June 6, 2012 When someone writes patches adding support for it, together with the code that adds support to at least one device that uses UVC 1.5 ;) I dunno if anyone is working on that ATM. Regards, Mauro Apparently there was a patch in 2013. https://www.mail-archive.com/linux-media@vger.kernel.org/msg66199.html If the patch is in the kernel, I don't think it works any more. If the patch never made it into the kernel then I don't know why (my guess is it has something to do with QP controls). -- 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
uvc-1.5
When is the Linux kernel going to support uvc-1.5? It was made a standard on June 6, 2012 -- 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 29/31] parisc: handle page-less SG entries
On Thu, 2015-08-13 at 20:30 -0700, Dan Williams wrote: On Thu, Aug 13, 2015 at 7:31 AM, Christoph Hellwig h...@lst.de wrote: On Wed, Aug 12, 2015 at 09:01:02AM -0700, Linus Torvalds wrote: I'm assuming that anybody who wants to use the page-less scatter-gather lists always does so on memory that isn't actually virtually mapped at all, or only does so on sane architectures that are cache coherent at a physical level, but I'd like that assumption *documented* somewhere. It's temporarily mapped by kmap-like helpers. That code isn't in this series. The most recent version of it is here: https://git.kernel.org/cgit/linux/kernel/git/djbw/nvdimm.git/commit/?h=pfnid=de8237c99fdb4352be2193f3a7610e902b9bb2f0 note that it's not doing the cache flushing it would have to do yet, but it's also only enabled for x86 at the moment. For virtually tagged caches I assume we would temporarily map with kmap_atomic_pfn_t(), similar to how drm_clflush_pages() implements powerpc support. However with DAX we could end up with multiple virtual aliases for a page-less pfn. At least on some PA architectures, you have to be very careful. Improperly managed, multiple aliases will cause the system to crash (actually a machine check in the cache chequerboard). For the most temperamental systems, we need the cache line flushed and the alias mapping ejected from the TLB cache before we access the same page at an inequivalent alias. James -- 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: RFC: prepare for struct scatterlist entries without page backing
On Wed, 2015-08-12 at 09:05 +0200, Christoph Hellwig wrote: Dan Williams started to look into addressing I/O to and from Persistent Memory in his series from June: http://thread.gmane.org/gmane.linux.kernel.cross-arch/27944 I've started looking into DMA mapping of these SGLs specifically instead of the map_pfn method in there. In addition to supporting NVDIMM backed I/O I also suspect this would be highly useful for media drivers that go through nasty hoops to be able to DMA from/to their ioremapped regions, with vb2_dc_get_userptr in drivers/media/v4l2-core/videobuf2-dma-contig.c being a prime example for the unsafe hacks currently used. It turns out most DMA mapping implementation can handle SGLs without page structures with some fairly simple mechanical work. Most of it is just about consistently using sg_phys. For implementations that need to flush caches we need a new helper that skips these cache flushes if a entry doesn't have a kernel virtual address. However the ccio (parisc) and sba_iommu (parisc ia64) IOMMUs seem to be operate mostly on virtual addresses. It's a fairly odd concept that I don't fully grasp, so I'll need some help with those if we want to bring this forward. I can explain that. I think this doesn't apply to ia64 because it's cache is PIPT, but on parisc, we have a VIPT cache. On normal physically indexed architectures, when the iommu sees a DMA transfer to/from physical memory, it also notifies the CPU to flush the internal CPU caches of those lines. This is usually an interlocking step of the transfer to make sure the page is coherent before transfer to/from the device (it's why the ia32 for instance is a coherent architecture). Because the system is physically indexed, there's no need to worry about aliases. On Virtually Indexed systems, like parisc, there is an aliasing problem. The CCIO iommu unit (and all other iommu systems on parisc) have what's called a local coherence index (LCI). You program it as part of the IOMMU page table and it tells the system which Virtual line in the cache to flush as part of the IO transaction, thus still ensuring cache coherence. That's why we have to know the virtual as well as physical addresses for the page. The problem we have in Linux is that we have two virtual addresses, which are often incoherent aliases: the user virtual address and a kernel virtual address but we can only make the page coherent with a single alias (only one LCI). The way I/O on Linux currently works is that get_user_pages actually flushes the user virtual address, so that's expected to be coherent, so the address we program into the VCI is the kernel virtual address. Usually nothing in the kernel has ever touched the page, so there's nothing to flush, but we do it just in case. In theory, for these non kernel page backed SG entries, we can make the process more efficient by not flushing in gup and instead programming the user virtual address into the local coherence index. However, simply zeroing the LCI will also work (except that poor VI zero line will get flushed repeatedly, so it's probably best to pick a known untouched line in the kernel). James -- 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: [Xen-devel] RIP MTRR - status update for upcoming v4.2
On Fri, 2015-06-12 at 16:15 -0700, Andy Lutomirski wrote: On Jun 12, 2015 12:59 AM, Jan Beulich jbeul...@suse.com wrote: On 12.06.15 at 01:23, toshi.k...@hp.com wrote: There are two usages on MTRRs: 1) MTRR entries set by firmware 2) MTRR entries set by OS drivers We can obsolete 2), but we have no control over 1). As UEFI firmwares also set this up, this usage will continue to stay. So, we should not get rid of the MTRR code that looks up the MTRR entries, while we have no need to modify them. Such MTRR entries provide safe guard to /dev/mem, which allows privileged user to access a range that may require UC mapping while the /dev/mem driver blindly maps it with WB. MTRRs converts WB to UC in such a case. But it wouldn't be impossible to simply read the MTRRs upon boot, store the information, disable MTRRs, and correctly use PAT to achieve the same effect (i.e. the blindly maps part of course would need fixing). This may crash and burn badly when we call a UEFI function or an SMI happens. I think we should just leave the MTRRs alone. Wholeheartedly agree: PAT only works when the given memory map is in operation but MTRRs function everywhere. Anything that goes into real mode or installs its own memory map won't see the Linux page attributes. James -- 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] rc: img-ir: fix error in parameters passed to irq_free()
On Tue, Feb 10, 2015 at 10:41:56AM +, Sifan Naeem wrote:
img_ir_remove() passes a pointer to the ISR function as the 2nd
parameter to irq_free() instead of a pointer to the device data
structure.
This issue causes unloading img-ir module to fail with the below
warning after building and loading img-ir as a module.
WARNING: CPU: 2 PID: 155 at ../kernel/irq/manage.c:1278
__free_irq+0xb4/0x214() Trying to free already-free IRQ 58
Modules linked in: img_ir(-)
CPU: 2 PID: 155 Comm: rmmod Not tainted 3.14.0 #55 ...
Call Trace:
...
[8048d420] __free_irq+0xb4/0x214
[8048d6b4] free_irq+0xac/0xf4
[c009b130] img_ir_remove+0x54/0xd4 [img_ir] [8073ded0]
platform_drv_remove+0x30/0x54 ...
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
Fixes: 160a8f8aec4d ([media] rc: img-ir: add base driver)
Cc: sta...@vger.kernel.org # 3.15+
Thanks for catching this Sifan. It appears to have been introduced while
getting the driver ready for upstream (it used to use the devm_* API to
request the IRQ, but I changed it to avoid the ISR racing with module
removal).
Acked-by: James Hogan james.ho...@imgtec.com
Cheers
James
---
drivers/media/rc/img-ir/img-ir-core.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/rc/img-ir/img-ir-core.c
b/drivers/media/rc/img-ir/img-ir-core.c
index 77c78de..7020659 100644
--- a/drivers/media/rc/img-ir/img-ir-core.c
+++ b/drivers/media/rc/img-ir/img-ir-core.c
@@ -146,7 +146,7 @@ static int img_ir_remove(struct platform_device *pdev)
{
struct img_ir_priv *priv = platform_get_drvdata(pdev);
- free_irq(priv-irq, img_ir_isr);
+ free_irq(priv-irq, priv);
img_ir_remove_hw(priv);
img_ir_remove_raw(priv);
--
1.7.9.5
signature.asc
Description: Digital signature
Re: [PATCH v2] rc: img-ir: Add and enable sys clock for img-ir
On 04/02/15 16:48, Sifan Naeem wrote:
Gets a handle to the system clock, already described in the binding
document, and calls the appropriate common clock framework functions
to mark it prepared/enabled, the common clock framework initially
enables the clock and doesn't disable it at least until the
device/driver is removed.
It's important the systen clock is enabled before register interface is
accessed by the driver.
The system clock to IR is needed for the driver to communicate with the
IR hardware via MMIO accesses on the system bus, so it must not be
disabled during use or the driver will malfunction.
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
Thanks Sifan, looks good to me, doesn't break tz1090, and seems to still
cope with no clocks provided in DT.
Acked-by: James Hogan james.ho...@imgtec.com
Cheers
James
---
Changes from v1:
System clock enabled in probe function before any hardware is accessed.
Error handling in probe function ensures ISR doesn't get called with
system clock disabled.
drivers/media/rc/img-ir/img-ir-core.c | 29 +
drivers/media/rc/img-ir/img-ir.h |2 ++
2 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-core.c
b/drivers/media/rc/img-ir/img-ir-core.c
index 77c78de..a10d666 100644
--- a/drivers/media/rc/img-ir/img-ir-core.c
+++ b/drivers/media/rc/img-ir/img-ir-core.c
@@ -110,16 +110,32 @@ static int img_ir_probe(struct platform_device *pdev)
priv-clk = devm_clk_get(pdev-dev, core);
if (IS_ERR(priv-clk))
dev_warn(pdev-dev, cannot get core clock resource\n);
+
+ /* Get sys clock */
+ priv-sys_clk = devm_clk_get(pdev-dev, sys);
+ if (IS_ERR(priv-sys_clk))
+ dev_warn(pdev-dev, cannot get sys clock resource\n);
/*
- * The driver doesn't need to know about the system (sys) or power
- * modulation (mod) clocks yet
+ * Enabling the system clock before the register interface is
+ * accessed. ISR shouldn't get called with Sys Clock disabled,
+ * hence exiting probe with an error.
*/
+ if (!IS_ERR(priv-sys_clk)) {
+ error = clk_prepare_enable(priv-sys_clk);
+ if (error) {
+ dev_err(pdev-dev, cannot enable sys clock\n);
+ return error;
+ }
+ }
/* Set up raw hw decoder */
error = img_ir_probe_raw(priv);
error2 = img_ir_probe_hw(priv);
- if (error error2)
- return (error == -ENODEV) ? error2 : error;
+ if (error error2) {
+ if (error == -ENODEV)
+ error = error2;
+ goto err_probe;
+ }
/* Get the IRQ */
priv-irq = irq;
@@ -139,6 +155,9 @@ static int img_ir_probe(struct platform_device *pdev)
err_irq:
img_ir_remove_hw(priv);
img_ir_remove_raw(priv);
+err_probe:
+ if (!IS_ERR(priv-sys_clk))
+ clk_disable_unprepare(priv-sys_clk);
return error;
}
@@ -152,6 +171,8 @@ static int img_ir_remove(struct platform_device *pdev)
if (!IS_ERR(priv-clk))
clk_disable_unprepare(priv-clk);
+ if (!IS_ERR(priv-sys_clk))
+ clk_disable_unprepare(priv-sys_clk);
return 0;
}
diff --git a/drivers/media/rc/img-ir/img-ir.h
b/drivers/media/rc/img-ir/img-ir.h
index 2ddf560..f1387c0 100644
--- a/drivers/media/rc/img-ir/img-ir.h
+++ b/drivers/media/rc/img-ir/img-ir.h
@@ -138,6 +138,7 @@ struct clk;
* @dev: Platform device.
* @irq: IRQ number.
* @clk: Input clock.
+ * @sys_clk: System clock.
* @reg_base:Iomem base address of IR register block.
* @lock:Protects IR registers and variables in this struct.
* @raw: Driver data for raw decoder.
@@ -147,6 +148,7 @@ struct img_ir_priv {
struct device *dev;
int irq;
struct clk *clk;
+ struct clk *sys_clk;
void __iomem*reg_base;
spinlock_t lock;
signature.asc
Description: OpenPGP digital signature
Re: [PATCH] rc: img-ir: Add and enable sys clock for IR
Hi Sifan,
On 03/02/15 17:30, Sifan Naeem wrote:
Gets a handle to the system clock, already described in the binding
document, and calls the appropriate common clock
framework functions to mark it prepared/enabled, the common clock
framework initially enables the clock and doesn't disable it at least
until the device/driver is removed.
The system clock to IR is needed for the driver to communicate with the
IR hardware via MMIO accesses on the system bus, so it must not be
disabled during use or the driver will malfunction.
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
---
drivers/media/rc/img-ir/img-ir-core.c | 13 +
drivers/media/rc/img-ir/img-ir.h |2 ++
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-core.c
b/drivers/media/rc/img-ir/img-ir-core.c
index 77c78de..783dd21 100644
--- a/drivers/media/rc/img-ir/img-ir-core.c
+++ b/drivers/media/rc/img-ir/img-ir-core.c
@@ -60,6 +60,8 @@ static void img_ir_setup(struct img_ir_priv *priv)
if (!IS_ERR(priv-clk))
clk_prepare_enable(priv-clk);
+ if (!IS_ERR(priv-sys_clk))
+ clk_prepare_enable(priv-sys_clk);
The patch mostly looks good, however I just realised this only enables
the system clock after it has already used the register interface. To be
safe it should really be enabled before any hardware accesses, so before
img_ir_ident() is called (and certainly before the img_ir_setup_raw()
and img_ir_setup_hw() functions are called since they set up the default
timings / irq mask etc).
Currently img_ir_probe_raw() and img_ir_probe_hw() don't appear to
access the hardware, but I can imagine they may want to access hardware
in future to read the revision register, so I think its worth doing it
from img_ir_probe() before they get called too, which should also ensure
the ISR doesn't get called with sysclock disabled (obviously error
handling in probe function would need to take it into account).
I suspect you would see this causing a problem when the driver is built
as a module and loaded after unused clocks have been automatically
disabled by the common clock framework at the end of the init sequence.
Thanks
James
}
static void img_ir_ident(struct img_ir_priv *priv)
@@ -110,10 +112,11 @@ static int img_ir_probe(struct platform_device *pdev)
priv-clk = devm_clk_get(pdev-dev, core);
if (IS_ERR(priv-clk))
dev_warn(pdev-dev, cannot get core clock resource\n);
- /*
- * The driver doesn't need to know about the system (sys) or power
- * modulation (mod) clocks yet
- */
+
+ /* Get sys clock */
+ priv-sys_clk = devm_clk_get(pdev-dev, sys);
+ if (IS_ERR(priv-sys_clk))
+ dev_warn(pdev-dev, cannot get sys clock resource\n);
/* Set up raw hw decoder */
error = img_ir_probe_raw(priv);
@@ -152,6 +155,8 @@ static int img_ir_remove(struct platform_device *pdev)
if (!IS_ERR(priv-clk))
clk_disable_unprepare(priv-clk);
+ if (!IS_ERR(priv-sys_clk))
+ clk_disable_unprepare(priv-sys_clk);
return 0;
}
diff --git a/drivers/media/rc/img-ir/img-ir.h
b/drivers/media/rc/img-ir/img-ir.h
index 2ddf560..f1387c0 100644
--- a/drivers/media/rc/img-ir/img-ir.h
+++ b/drivers/media/rc/img-ir/img-ir.h
@@ -138,6 +138,7 @@ struct clk;
* @dev: Platform device.
* @irq: IRQ number.
* @clk: Input clock.
+ * @sys_clk: System clock.
* @reg_base:Iomem base address of IR register block.
* @lock:Protects IR registers and variables in this struct.
* @raw: Driver data for raw decoder.
@@ -147,6 +148,7 @@ struct img_ir_priv {
struct device *dev;
int irq;
struct clk *clk;
+ struct clk *sys_clk;
void __iomem*reg_base;
spinlock_t lock;
signature.asc
Description: OpenPGP digital signature
RE: [possible BUG, cx23885] Dual tuner TV card, works using one tuner only, doesn't work if both tuners are used
Hi, James. After searching for somebody posting some issues similar to mine, I think this one you posted to the mailing list can be related: https://www.mail-archive.com/linux- media%40vger.kernel.org/msg80078.html I'm having problems using both tuners in a dual tuner card (Terratec Cinergy T PCIe Dual), also based on cx23885, but it uses different frontends/tuners than yours. I'm pretty sure mine was an actual hardware fault. At first it worked perfectly. Then after a bit the server would occasionally lock up hard or reboot, then more often, then every time. I spent ages thinking it was a driver problem and did all sorts of traces etc and found nothing. In the end I just got mythtv to not use the second tuner (or the first tuner - as long as only one was used it was fine). Then about a month ago it started locking up again occasionally, then more often, then every time, only using the one tuner. Strange though that if it booted up without locking up it would be okay. I bought a usb tuner and haven't used the cx23885 card since. I will be seeking a replacement under warranty though, as the signal quality is quite a bit better. Good luck in your quest though! James
Re: [PATCH 3/5] rc: img-ir: biphase enabled with workaround
Hi Sifan,
On 11/12/14 18:54, Sifan Naeem wrote:
+/*
+ * Timer function to re-enable the current protocol after it had been
+ * cleared when invalid interrupts were generated due to a quirk in
+the
+ * img-ir decoder.
+ */
+static void img_ir_suspend_timer(unsigned long arg) {
+ struct img_ir_priv *priv = (struct img_ir_priv *)arg;
+
+ img_ir_write(priv, IMG_IR_IRQ_CLEAR,
+ IMG_IR_IRQ_ALL ~IMG_IR_IRQ_EDGE);
+
+ /* Don't set IRQ if it has changed in a different context. */
Should you even be clearing IRQs in that case? Maybe safer to just treat that
case as a return immediately without touching anything sort of situation.
don't have to clear it for this work around to work, so will remove.
+ if ((priv-hw.suspend_irqen IMG_IR_IRQ_EDGE) ==
+ img_ir_read(priv, IMG_IR_IRQ_ENABLE))
+ img_ir_write(priv, IMG_IR_IRQ_ENABLE, priv-
hw.suspend_irqen);
+ /* enable */
+ img_ir_write(priv, IMG_IR_CONTROL, priv-hw.reg_timings.ctrl); }
To clarify, I was only referring to the case where the irq mask has
changed unexpectedly. If it hasn't changed then it would seem to make
sense to clear pending interrupts (i.e. the ones we've been
intentionally ignoring) before re-enabling them.
When you say it works without, do you mean there never are pending
interrupts (if you don't press any other buttons on the remote)?
Cheers
James
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v2 3/5] rc: img-ir: biphase enabled with workaround
Hi Sifan,
On 11/12/14 20:06, Sifan Naeem wrote:
Biphase decoding in the current img-ir has got a quirk, where multiple
Interrupts are generated when an incomplete IR code is received by the
decoder.
Patch adds a work around for the quirk and enables biphase decoding.
Changes from v1:
* rebased due to conflict with img-ir/hw: Fix potential deadlock stopping
timer
* spinlock taken in img_ir_suspend_timer
* check for hw-stopping before handling quirks in img_ir_isr_hw
* new memeber added to img_ir_priv_hw to save irq status over suspend
For future reference, the list of changes between patchset versions is
usually put after a --- so that it doesn't get included in the final
git commit message. You can also add any Acked-by/Reviewed-by tags
you've been given to new versions of patchset, assuming nothing
significant has changed in that patch (maintainers generally add
relevant tags for you, that are sent in response to the patches being
applied).
Anyway, the whole patchset looks okay to me, aside from the one question
I just asked on patch 3 of v1, which I'm not so sure about. I'll let you
decide whether that needs changing since you have the hardware to verify it.
So for the whole patchset feel free to add my:
Acked-by: James Hogan james.ho...@imgtec.com
Thanks
James
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
---
drivers/media/rc/img-ir/img-ir-hw.c | 60
+--
drivers/media/rc/img-ir/img-ir-hw.h |4 +++
2 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index 9cecda7..5c32f05 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -52,6 +52,11 @@ static struct img_ir_decoder *img_ir_decoders[] = {
#define IMG_IR_QUIRK_CODE_BROKEN 0x1 /* Decode is broken */
#define IMG_IR_QUIRK_CODE_LEN_INCR 0x2 /* Bit length needs increment */
+/*
+ * The decoder generates rapid interrupts without actually having
+ * received any new data after an incomplete IR code is decoded.
+ */
+#define IMG_IR_QUIRK_CODE_IRQ0x4
/* functions for preprocessing timings, ensuring max is set */
@@ -542,6 +547,7 @@ static void img_ir_set_decoder(struct img_ir_priv *priv,
*/
spin_unlock_irq(priv-lock);
del_timer_sync(hw-end_timer);
+ del_timer_sync(hw-suspend_timer);
spin_lock_irq(priv-lock);
hw-stopping = false;
@@ -861,6 +867,29 @@ static void img_ir_end_timer(unsigned long arg)
spin_unlock_irq(priv-lock);
}
+/*
+ * Timer function to re-enable the current protocol after it had been
+ * cleared when invalid interrupts were generated due to a quirk in the
+ * img-ir decoder.
+ */
+static void img_ir_suspend_timer(unsigned long arg)
+{
+ struct img_ir_priv *priv = (struct img_ir_priv *)arg;
+
+ spin_lock_irq(priv-lock);
+ /*
+ * Don't overwrite enabled valid/match IRQs if they have already been
+ * changed by e.g. a filter change.
+ */
+ if ((priv-hw.quirk_suspend_irq IMG_IR_IRQ_EDGE) ==
+ img_ir_read(priv, IMG_IR_IRQ_ENABLE))
+ img_ir_write(priv, IMG_IR_IRQ_ENABLE,
+ priv-hw.quirk_suspend_irq);
+ /* enable */
+ img_ir_write(priv, IMG_IR_CONTROL, priv-hw.reg_timings.ctrl);
+ spin_unlock_irq(priv-lock);
+}
+
#ifdef CONFIG_COMMON_CLK
static void img_ir_change_frequency(struct img_ir_priv *priv,
struct clk_notifier_data *change)
@@ -926,15 +955,38 @@ void img_ir_isr_hw(struct img_ir_priv *priv, u32
irq_status)
if (!hw-decoder)
return;
+ ct = hw-decoder-control.code_type;
+
ir_status = img_ir_read(priv, IMG_IR_STATUS);
- if (!(ir_status (IMG_IR_RXDVAL | IMG_IR_RXDVALD2)))
+ if (!(ir_status (IMG_IR_RXDVAL | IMG_IR_RXDVALD2))) {
+ if (!(priv-hw.ct_quirks[ct] IMG_IR_QUIRK_CODE_IRQ) ||
+ hw-stopping)
+ return;
+ /*
+ * The below functionality is added as a work around to stop
+ * multiple Interrupts generated when an incomplete IR code is
+ * received by the decoder.
+ * The decoder generates rapid interrupts without actually
+ * having received any new data. After a single interrupt it's
+ * expected to clear up, but instead multiple interrupts are
+ * rapidly generated. only way to get out of this loop is to
+ * reset the control register after a short delay.
+ */
+ img_ir_write(priv, IMG_IR_CONTROL, 0);
+ hw-quirk_suspend_irq = img_ir_read(priv, IMG_IR_IRQ_ENABLE);
+ img_ir_write(priv, IMG_IR_IRQ_ENABLE,
+ hw-quirk_suspend_irq IMG_IR_IRQ_EDGE
Re: [PATCH v2 3/5] rc: img-ir: biphase enabled with workaround
On 12/12/14 12:07, James Hogan wrote: Hi Sifan, On 11/12/14 20:06, Sifan Naeem wrote: Biphase decoding in the current img-ir has got a quirk, where multiple Interrupts are generated when an incomplete IR code is received by the decoder. Patch adds a work around for the quirk and enables biphase decoding. Changes from v1: * rebased due to conflict with img-ir/hw: Fix potential deadlock stopping timer * spinlock taken in img_ir_suspend_timer * check for hw-stopping before handling quirks in img_ir_isr_hw * new memeber added to img_ir_priv_hw to save irq status over suspend For future reference, the list of changes between patchset versions is usually put after a --- so that it doesn't get included in the final git commit message. You can also add any Acked-by/Reviewed-by tags you've been given to new versions of patchset, assuming nothing significant has changed in that patch (maintainers generally add relevant tags for you, that are sent in response to the patches being applied). Anyway, the whole patchset looks okay to me, aside from the one question I just asked on patch 3 of v1, which I'm not so sure about. I'll let you decide whether that needs changing since you have the hardware to verify it. So for the whole patchset feel free to add my: Acked-by: James Hogan james.ho...@imgtec.com Mauro: Assuming no other changes are requested in this patchset, do you want these resent with the moving of changelogs out of the main commit messages? Cheers James signature.asc Description: OpenPGP digital signature
Re: [REVIEW PATCH 1/2] img-ir/hw: Avoid clearing filter for no-op protocol change
Hi Mauro, On 04/12/14 17:38, Mauro Carvalho Chehab wrote: Em Mon, 1 Dec 2014 12:55:09 + James Hogan james.ho...@imgtec.com escreveu: When the img-ir driver is asked to change protocol, if the chosen decoder is already loaded then don't call img_ir_set_decoder(), so as not to clear the current filter. This is important because store_protocol() does not refresh the scancode filter with the new protocol if the set of enabled protocols hasn't actually changed, but it will still call the change_protocol() callback, resulting in the filter being disabled in the hardware. The problem can be reproduced by setting a filter, and then setting the protocol to the same protocol that is already set: $ echo nec protocols $ echo 0x filter_mask $ echo nec protocols After this, messages which don't match the filter still get received. This should be fixed at the RC core, as this is not driver-specific. Yes, you're right. I've fixed there and attempted backporting, and the problem appears to have actually been introduced in commit da6e162d6a46 ([media] rc-core: simplify sysfs code) which went into v3.17. I'll send a v2. Thanks James Regards, Mauro signature.asc Description: OpenPGP digital signature
[REVIEW PATCH v2] rc-main: Re-apply filter for no-op protocol change
Since commit da6e162d6a46 ([media] rc-core: simplify sysfs code), when
the IR protocol is set using the sysfs interface to the same set of
protocols that are already set, store_protocols() does not refresh the
scancode filter with the new protocol, even if it has already called the
change_protocol() callback successfully. This results in the filter
being disabled in the hardware and not re-enabled until the filter is
set again using sysfs.
Fix in store_protocols() by still re-applying the filter whenever the
change_protocol() driver callback succeeded.
The problem can be reproduced with the img-ir driver by setting a
filter, and then setting the protocol to the same protocol that is
already set:
$ echo nec protocols
$ echo 0x filter_mask
$ echo nec protocols
After this, messages which don't match the filter were still being
received.
Fixes: da6e162d6a46 ([media] rc-core: simplify sysfs code)
Reported-by: Sifan Naeem sifan.na...@imgtec.com
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Mauro Carvalho Chehab m.che...@samsung.com
Cc: David Härdeman da...@hardeman.nu
Cc: sta...@vger.kernel.org # v3.17+
Cc: linux-media@vger.kernel.org
---
Changes in v2:
- Move fix to store_protocols(). Still set filter again even if protocol
mask hasn't been changed as a result of the protocol change (Mauro).
---
drivers/media/rc/rc-main.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 8d3b74c5a717..fc369b033484 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1021,16 +1021,16 @@ static ssize_t store_protocols(struct device *device,
goto out;
}
- if (new_protocols == old_protocols) {
- rc = len;
- goto out;
+ if (new_protocols != old_protocols) {
+ *current_protocols = new_protocols;
+ IR_dprintk(1, Protocols changed to 0x%llx\n,
+ (long long)new_protocols);
}
- *current_protocols = new_protocols;
- IR_dprintk(1, Protocols changed to 0x%llx\n, (long
long)new_protocols);
-
/*
-* If the protocol is changed the filter needs updating.
+* If a protocol change was attempted the filter may need updating, even
+* if the actual protocol mask hasn't changed (since the driver may have
+* cleared the filter).
* Try setting the same filter with the new protocol (if any).
* Fall back to clearing the filter.
*/
--
2.0.4
--
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 1/5] rc: img-ir: add scancode requests to a struct
On 04/12/14 15:38, Sifan Naeem wrote:
The information being requested of hardware decode callbacks through
the img-ir-hw scancode API is mounting up, so combine it into a struct
which can be passed in with a single pointer rather than multiple
pointer arguments. This allows it to be extended more easily without
touching all the hardware decode callbacks.
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
Acked-by: James Hogan james.ho...@imgtec.com
Cheers
James
---
drivers/media/rc/img-ir/img-ir-hw.c| 16 +---
drivers/media/rc/img-ir/img-ir-hw.h| 16 ++--
drivers/media/rc/img-ir/img-ir-jvc.c |8
drivers/media/rc/img-ir/img-ir-nec.c | 24
drivers/media/rc/img-ir/img-ir-sanyo.c |8
drivers/media/rc/img-ir/img-ir-sharp.c |8
drivers/media/rc/img-ir/img-ir-sony.c | 12 ++--
7 files changed, 53 insertions(+), 39 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index ec49f94..61850a6 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -789,20 +789,22 @@ static void img_ir_handle_data(struct img_ir_priv
*priv, u32 len, u64 raw)
struct img_ir_priv_hw *hw = priv-hw;
const struct img_ir_decoder *dec = hw-decoder;
int ret = IMG_IR_SCANCODE;
- u32 scancode;
- enum rc_type protocol = RC_TYPE_UNKNOWN;
+ struct img_ir_scancode_req request;
+
+ request.protocol = RC_TYPE_UNKNOWN;
if (dec-scancode)
- ret = dec-scancode(len, raw, protocol, scancode,
hw-enabled_protocols);
+ ret = dec-scancode(len, raw, hw-enabled_protocols, request);
else if (len = 32)
- scancode = (u32)raw;
+ request.scancode = (u32)raw;
else if (len 32)
- scancode = (u32)raw ((1 len)-1);
+ request.scancode = (u32)raw ((1 len)-1);
dev_dbg(priv-dev, data (%u bits) = %#llx\n,
len, (unsigned long long)raw);
if (ret == IMG_IR_SCANCODE) {
- dev_dbg(priv-dev, decoded scan code %#x\n, scancode);
- rc_keydown(hw-rdev, protocol, scancode, 0);
+ dev_dbg(priv-dev, decoded scan code %#x\n,
+ request.scancode);
+ rc_keydown(hw-rdev, request.protocol, request.scancode, 0);
img_ir_end_repeat(priv);
} else if (ret == IMG_IR_REPEATCODE) {
if (hw-mode == IMG_IR_M_REPEATING) {
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h
b/drivers/media/rc/img-ir/img-ir-hw.h
index 8fcc16c..1fc9583 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -133,6 +133,18 @@ struct img_ir_timing_regvals {
#define IMG_IR_REPEATCODE1 /* repeat the previous code */
/**
+ * struct img_ir_scancode_req - Scancode request data.
+ * @protocol:Protocol code of received message (defaults to
+ * RC_TYPE_UNKNOWN).
+ * @scancode:Scan code of received message (must be written by
+ * handler if IMG_IR_SCANCODE is returned).
+ */
+struct img_ir_scancode_req {
+ enum rc_type protocol;
+ u32 scancode;
+};
+
+/**
* struct img_ir_decoder - Decoder settings for an IR protocol.
* @type:Protocol types bitmap.
* @tolerance: Timing tolerance as a percentage (default 10%).
@@ -162,8 +174,8 @@ struct img_ir_decoder {
struct img_ir_control control;
/* scancode logic */
- int (*scancode)(int len, u64 raw, enum rc_type *protocol,
- u32 *scancode, u64 enabled_protocols);
+ int (*scancode)(int len, u64 raw, u64 enabled_protocols,
+ struct img_ir_scancode_req *request);
int (*filter)(const struct rc_scancode_filter *in,
struct img_ir_filter *out, u64 protocols);
};
diff --git a/drivers/media/rc/img-ir/img-ir-jvc.c
b/drivers/media/rc/img-ir/img-ir-jvc.c
index a60dda8..d3e2fc0 100644
--- a/drivers/media/rc/img-ir/img-ir-jvc.c
+++ b/drivers/media/rc/img-ir/img-ir-jvc.c
@@ -12,8 +12,8 @@
#include img-ir-hw.h
/* Convert JVC data to a scancode */
-static int img_ir_jvc_scancode(int len, u64 raw, enum rc_type *protocol,
-u32 *scancode, u64 enabled_protocols)
+static int img_ir_jvc_scancode(int len, u64 raw, u64 enabled_protocols,
+struct img_ir_scancode_req *request)
{
unsigned int cust, data;
@@ -23,8 +23,8 @@ static int img_ir_jvc_scancode(int len, u64 raw, enum
rc_type *protocol,
cust = (raw 0) 0xff;
data = (raw 8) 0xff;
- *protocol = RC_TYPE_JVC;
- *scancode = cust 8 | data;
+ request-protocol = RC_TYPE_JVC;
+ request-scancode = cust 8 | data;
return IMG_IR_SCANCODE;
}
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
b
Re: [PATCH 2/5] rc: img-ir: pass toggle bit to the rc driver
On 04/12/14 15:38, Sifan Naeem wrote:
Add toggle bit to struct img_ir_scancode_req so that protocols can
provide it to img_ir_handle_data(), and pass that toggle bit up to
rc_keydown instead of 0.
This is nedded for the upcoming rc-5 and rc-6 patches.
Typo (nedded).
Otherwise:
Acked-by: James Hogan james.ho...@imgtec.com
Cheers
James
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
---
drivers/media/rc/img-ir/img-ir-hw.c |8 +---
drivers/media/rc/img-ir/img-ir-hw.h |2 ++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index 61850a6..4a1407b 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -792,6 +792,7 @@ static void img_ir_handle_data(struct img_ir_priv *priv,
u32 len, u64 raw)
struct img_ir_scancode_req request;
request.protocol = RC_TYPE_UNKNOWN;
+ request.toggle = 0;
if (dec-scancode)
ret = dec-scancode(len, raw, hw-enabled_protocols, request);
@@ -802,9 +803,10 @@ static void img_ir_handle_data(struct img_ir_priv *priv,
u32 len, u64 raw)
dev_dbg(priv-dev, data (%u bits) = %#llx\n,
len, (unsigned long long)raw);
if (ret == IMG_IR_SCANCODE) {
- dev_dbg(priv-dev, decoded scan code %#x\n,
- request.scancode);
- rc_keydown(hw-rdev, request.protocol, request.scancode, 0);
+ dev_dbg(priv-dev, decoded scan code %#x, toggle %u\n,
+ request.scancode, request.toggle);
+ rc_keydown(hw-rdev, request.protocol, request.scancode,
+request.toggle);
img_ir_end_repeat(priv);
} else if (ret == IMG_IR_REPEATCODE) {
if (hw-mode == IMG_IR_M_REPEATING) {
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h
b/drivers/media/rc/img-ir/img-ir-hw.h
index 1fc9583..5e59e8e 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -138,10 +138,12 @@ struct img_ir_timing_regvals {
* RC_TYPE_UNKNOWN).
* @scancode:Scan code of received message (must be written by
* handler if IMG_IR_SCANCODE is returned).
+ * @toggle: Toggle bit (defaults to 0).
*/
struct img_ir_scancode_req {
enum rc_type protocol;
u32 scancode;
+ u8 toggle;
};
/**
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 3/5] rc: img-ir: biphase enabled with workaround
On 04/12/14 15:38, Sifan Naeem wrote:
Biphase decoding in the current img-ir has got a quirk, where multiple
Interrupts are generated when an incomplete IR code is received by the
decoder.
Patch adds a work around for the quirk and enables biphase decoding.
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
---
drivers/media/rc/img-ir/img-ir-hw.c | 56
+--
drivers/media/rc/img-ir/img-ir-hw.h |2 ++
2 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index 4a1407b..a977467 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -52,6 +52,11 @@ static struct img_ir_decoder *img_ir_decoders[] = {
#define IMG_IR_QUIRK_CODE_BROKEN 0x1 /* Decode is broken */
#define IMG_IR_QUIRK_CODE_LEN_INCR 0x2 /* Bit length needs increment */
+/*
+ * The decoder generates rapid interrupts without actually having
+ * received any new data after an incomplete IR code is decoded.
+ */
+#define IMG_IR_QUIRK_CODE_IRQ0x4
/* functions for preprocessing timings, ensuring max is set */
@@ -547,6 +552,7 @@ static void img_ir_set_decoder(struct img_ir_priv *priv,
/* stop the end timer and switch back to normal mode */
del_timer_sync(hw-end_timer);
+ del_timer_sync(hw-suspend_timer);
FYI, this'll need rebasing due to conflicting with img-ir/hw: Fix
potential deadlock stopping timer. The new del_timer_sync will need to
be when spin lock isn't held, i.e. still next to the other one, and
don't forget to ensure that suspend_timer doesn't get started if
hw-stopping.
hw-mode = IMG_IR_M_NORMAL;
/* clear the wakeup scancode filter */
@@ -843,6 +849,26 @@ static void img_ir_end_timer(unsigned long arg)
spin_unlock_irq(priv-lock);
}
+/*
+ * Timer function to re-enable the current protocol after it had been
+ * cleared when invalid interrupts were generated due to a quirk in the
+ * img-ir decoder.
+ */
+static void img_ir_suspend_timer(unsigned long arg)
+{
+ struct img_ir_priv *priv = (struct img_ir_priv *)arg;
+
You should take the spin lock for most of this function now that
img-ir/hw: Fix potential deadlock stopping timer is applied and it is
safe to do so.
+ img_ir_write(priv, IMG_IR_IRQ_CLEAR,
+ IMG_IR_IRQ_ALL ~IMG_IR_IRQ_EDGE);
+
+ /* Don't set IRQ if it has changed in a different context. */
Wouldn't hurt to clarify this while you're at it (it confused me for a
moment thinking it was concerned about the enabled raw event IRQs
(IMG_IR_IRQ_EDGE) changing).
Maybe Don't overwrite enabled valid/match IRQs if they have already
been changed by e.g. a filter change.
Should you even be clearing IRQs in that case? Maybe safer to just treat
that case as a return immediately without touching anything sort of
situation.
+ if ((priv-hw.suspend_irqen IMG_IR_IRQ_EDGE) ==
+ img_ir_read(priv, IMG_IR_IRQ_ENABLE))
+ img_ir_write(priv, IMG_IR_IRQ_ENABLE, priv-hw.suspend_irqen);
+ /* enable */
+ img_ir_write(priv, IMG_IR_CONTROL, priv-hw.reg_timings.ctrl);
+}
+
#ifdef CONFIG_COMMON_CLK
static void img_ir_change_frequency(struct img_ir_priv *priv,
struct clk_notifier_data *change)
@@ -908,15 +934,37 @@ void img_ir_isr_hw(struct img_ir_priv *priv, u32
irq_status)
if (!hw-decoder)
return;
+ ct = hw-decoder-control.code_type;
+
ir_status = img_ir_read(priv, IMG_IR_STATUS);
- if (!(ir_status (IMG_IR_RXDVAL | IMG_IR_RXDVALD2)))
+ if (!(ir_status (IMG_IR_RXDVAL | IMG_IR_RXDVALD2))) {
+ if (!(priv-hw.ct_quirks[ct] IMG_IR_QUIRK_CODE_IRQ))
(I suggest adding || hw-stopping to this case)
+ return;
+ /*
+ * The below functionality is added as a work around to stop
+ * multiple Interrupts generated when an incomplete IR code is
+ * received by the decoder.
+ * The decoder generates rapid interrupts without actually
+ * having received any new data. After a single interrupt it's
+ * expected to clear up, but instead multiple interrupts are
+ * rapidly generated. only way to get out of this loop is to
+ * reset the control register after a short delay.
+ */
+ img_ir_write(priv, IMG_IR_CONTROL, 0);
+ hw-suspend_irqen = img_ir_read(priv, IMG_IR_IRQ_ENABLE);
You're reusing hw-suspend_irqen. What if you get this workaround being
activated between img_ir_enable_wake() and img_ir_disable_wake()? I
suggest just using a new img_ir_priv_hw member.
The rest looks reasonable to me, even if unfortunate that it is
necessary in the first place.
Thanks for the hard work!
Cheers
James
Re: [PATCH 4/5] rc: img-ir: add philips rc5 decoder module
On 04/12/14 15:38, Sifan Naeem wrote:
Add img-ir module for decoding Philips rc5 protocol.
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
---
drivers/media/rc/img-ir/Kconfig |7 +++
drivers/media/rc/img-ir/Makefile |1 +
drivers/media/rc/img-ir/img-ir-hw.c |3 ++
drivers/media/rc/img-ir/img-ir-hw.h |1 +
drivers/media/rc/img-ir/img-ir-rc5.c | 88
++
5 files changed, 100 insertions(+)
create mode 100644 drivers/media/rc/img-ir/img-ir-rc5.c
diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig
index 03ba9fc..b5b114f 100644
--- a/drivers/media/rc/img-ir/Kconfig
+++ b/drivers/media/rc/img-ir/Kconfig
@@ -59,3 +59,10 @@ config IR_IMG_SANYO
help
Say Y here to enable support for the Sanyo protocol (used by Sanyo,
Aiwa, Chinon remotes) in the ImgTec infrared decoder block.
+
+config IR_IMG_RC5
+ bool Phillips RC5 protocol support
I think that should be Philips (if wikipedia is anything to go by).
Same elsewhere in this patch and patch 5.
Other than that,
Acked-by: James Hogan james.ho...@imgtec.com
(Note, I don't have RC-5/RC-6 capable hardware yet so can't test this
support)
Thanks
James
+ depends on IR_IMG_HW
+ help
+Say Y here to enable support for the RC5 protocol in the ImgTec
+infrared decoder block.
diff --git a/drivers/media/rc/img-ir/Makefile
b/drivers/media/rc/img-ir/Makefile
index 92a459d..898b1b8 100644
--- a/drivers/media/rc/img-ir/Makefile
+++ b/drivers/media/rc/img-ir/Makefile
@@ -6,6 +6,7 @@ img-ir-$(CONFIG_IR_IMG_JVC) += img-ir-jvc.o
img-ir-$(CONFIG_IR_IMG_SONY) += img-ir-sony.o
img-ir-$(CONFIG_IR_IMG_SHARP)+= img-ir-sharp.o
img-ir-$(CONFIG_IR_IMG_SANYO)+= img-ir-sanyo.o
+img-ir-$(CONFIG_IR_IMG_RC5) += img-ir-rc5.o
img-ir-objs := $(img-ir-y)
obj-$(CONFIG_IR_IMG) += img-ir.o
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index a977467..322cdf8 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -42,6 +42,9 @@ static struct img_ir_decoder *img_ir_decoders[] = {
#ifdef CONFIG_IR_IMG_SANYO
img_ir_sanyo,
#endif
+#ifdef CONFIG_IR_IMG_RC5
+ img_ir_rc5,
+#endif
NULL
};
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h
b/drivers/media/rc/img-ir/img-ir-hw.h
index 8578aa7..f124ec5 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -187,6 +187,7 @@ extern struct img_ir_decoder img_ir_jvc;
extern struct img_ir_decoder img_ir_sony;
extern struct img_ir_decoder img_ir_sharp;
extern struct img_ir_decoder img_ir_sanyo;
+extern struct img_ir_decoder img_ir_rc5;
/**
* struct img_ir_reg_timings - Reg values for decoder timings at clock rate.
diff --git a/drivers/media/rc/img-ir/img-ir-rc5.c
b/drivers/media/rc/img-ir/img-ir-rc5.c
new file mode 100644
index 000..e1a0829
--- /dev/null
+++ b/drivers/media/rc/img-ir/img-ir-rc5.c
@@ -0,0 +1,88 @@
+/*
+ * ImgTec IR Decoder setup for Phillips RC-5 protocol.
+ *
+ * Copyright 2012-2014 Imagination Technologies Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include img-ir-hw.h
+
+/* Convert RC5 data to a scancode */
+static int img_ir_rc5_scancode(int len, u64 raw, u64 enabled_protocols,
+ struct img_ir_scancode_req *request)
+{
+ unsigned int addr, cmd, tgl, start;
+
+ /* Quirk in the decoder shifts everything by 2 to the left. */
+ raw = 2;
+
+ start = (raw 13) 0x01;
+ tgl = (raw 11) 0x01;
+ addr= (raw 6) 0x1f;
+ cmd = raw 0x3f;
+ /*
+ * 12th bit is used to extend the command in extended RC5 and has
+ * no effect on standard RC5.
+ */
+ cmd += ((raw 12) 0x01) ? 0 : 0x40;
+
+ if (!start)
+ return -EINVAL;
+
+ request-protocol = RC_TYPE_RC5;
+ request-scancode = addr 8 | cmd;
+ request-toggle = tgl;
+ return IMG_IR_SCANCODE;
+}
+
+/* Convert RC5 scancode to RC5 data filter */
+static int img_ir_rc5_filter(const struct rc_scancode_filter *in,
+ struct img_ir_filter *out, u64 protocols)
+{
+ /* Not supported by the hw. */
+ return -EINVAL;
+}
+
+/*
+ * RC-5 decoder
+ * see http://www.sbprojects.com/knowledge/ir/rc5.php
+ */
+struct img_ir_decoder img_ir_rc5 = {
+ .type = RC_BIT_RC5,
+ .control = {
+ .bitoriend2 = 1,
+ .code_type = IMG_IR_CODETYPE_BIPHASE,
+ .decodend2 = 1,
+ },
+ /* main timings
Re: [PATCH 5/5] rc: img-ir: add philips rc6 decoder module
On 04/12/14 15:38, Sifan Naeem wrote:
Add img-ir module for decoding Philips rc6 protocol.
Signed-off-by: Sifan Naeem sifan.na...@imgtec.com
Aside from the Philips thing:
Acked-by: James Hogan james.ho...@imgtec.com
(It's unpleasant having unexplained timings for RC-6, but it's better
than no RC-6 support, and hopefully in the future it can be improved).
Cheers
James
---
drivers/media/rc/img-ir/Kconfig |8 +++
drivers/media/rc/img-ir/Makefile |1 +
drivers/media/rc/img-ir/img-ir-hw.c |3 +
drivers/media/rc/img-ir/img-ir-hw.h |1 +
drivers/media/rc/img-ir/img-ir-rc6.c | 117
++
5 files changed, 130 insertions(+)
create mode 100644 drivers/media/rc/img-ir/img-ir-rc6.c
diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig
index b5b114f..4d3fca9 100644
--- a/drivers/media/rc/img-ir/Kconfig
+++ b/drivers/media/rc/img-ir/Kconfig
@@ -66,3 +66,11 @@ config IR_IMG_RC5
help
Say Y here to enable support for the RC5 protocol in the ImgTec
infrared decoder block.
+
+config IR_IMG_RC6
+ bool Phillips RC6 protocol support
+ depends on IR_IMG_HW
+ help
+Say Y here to enable support for the RC6 protocol in the ImgTec
+infrared decoder block.
+Note: This version only supports mode 0.
diff --git a/drivers/media/rc/img-ir/Makefile
b/drivers/media/rc/img-ir/Makefile
index 898b1b8..8e6d458 100644
--- a/drivers/media/rc/img-ir/Makefile
+++ b/drivers/media/rc/img-ir/Makefile
@@ -7,6 +7,7 @@ img-ir-$(CONFIG_IR_IMG_SONY) += img-ir-sony.o
img-ir-$(CONFIG_IR_IMG_SHARP)+= img-ir-sharp.o
img-ir-$(CONFIG_IR_IMG_SANYO)+= img-ir-sanyo.o
img-ir-$(CONFIG_IR_IMG_RC5) += img-ir-rc5.o
+img-ir-$(CONFIG_IR_IMG_RC6) += img-ir-rc6.o
img-ir-objs := $(img-ir-y)
obj-$(CONFIG_IR_IMG) += img-ir.o
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index 322cdf8..3b70dc2 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -45,6 +45,9 @@ static struct img_ir_decoder *img_ir_decoders[] = {
#ifdef CONFIG_IR_IMG_RC5
img_ir_rc5,
#endif
+#ifdef CONFIG_IR_IMG_RC6
+ img_ir_rc6,
+#endif
NULL
};
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h
b/drivers/media/rc/img-ir/img-ir-hw.h
index f124ec5..c7b6e1a 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -188,6 +188,7 @@ extern struct img_ir_decoder img_ir_sony;
extern struct img_ir_decoder img_ir_sharp;
extern struct img_ir_decoder img_ir_sanyo;
extern struct img_ir_decoder img_ir_rc5;
+extern struct img_ir_decoder img_ir_rc6;
/**
* struct img_ir_reg_timings - Reg values for decoder timings at clock rate.
diff --git a/drivers/media/rc/img-ir/img-ir-rc6.c
b/drivers/media/rc/img-ir/img-ir-rc6.c
new file mode 100644
index 000..bcd0822
--- /dev/null
+++ b/drivers/media/rc/img-ir/img-ir-rc6.c
@@ -0,0 +1,117 @@
+/*
+ * ImgTec IR Decoder setup for Phillips RC-6 protocol.
+ *
+ * Copyright 2012-2014 Imagination Technologies Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include img-ir-hw.h
+
+/* Convert RC6 data to a scancode */
+static int img_ir_rc6_scancode(int len, u64 raw, u64 enabled_protocols,
+ struct img_ir_scancode_req *request)
+{
+ unsigned int addr, cmd, mode, trl1, trl2;
+
+ /*
+ * Due to a side effect of the decoder handling the double length
+ * Trailer bit, the header information is a bit scrambled, and the
+ * raw data is shifted incorrectly.
+ * This workaround effectively recovers the header bits.
+ *
+ * The Header field should look like this:
+ *
+ * StartBit ModeBit2 ModeBit1 ModeBit0 TrailerBit
+ *
+ * But what we get is:
+ *
+ * ModeBit2 ModeBit1 ModeBit0 TrailerBit1 TrailerBit2
+ *
+ * The start bit is not important to recover the scancode.
+ */
+
+ raw = 27;
+
+ trl1= (raw 17) 0x01;
+ trl2= (raw 16) 0x01;
+
+ mode= (raw 18) 0x07;
+ addr= (raw8) 0xff;
+ cmd = raw 0xff;
+
+ /*
+ * Due to the above explained irregularity the trailer bits cannot
+ * have the same value.
+ */
+ if (trl1 == trl2)
+ return -EINVAL;
+
+ /* Only mode 0 supported for now */
+ if (mode)
+ return -EINVAL;
+
+ request-protocol = RC_TYPE_RC6_0;
+ request-scancode = addr 8 | cmd;
+ request-toggle = trl2;
+ return IMG_IR_SCANCODE;
+}
+
+/* Convert RC6
[REVIEW PATCH 0/2] img-ir: Some more fixes
A few more fixes for the img-ir RC driver in addition to the ones I posted a couple of weeks ago. The first patch fixes some broken behaviour when the same protocol is set twice and the effective scancode filter gets cleared as a result. The second patch fixes a potential deadlock and lockdep splat due to the repeat end timer being del_timer_sync'd with the spin lock held when changing protocols. The second patch here depends on img-ir/hw: Always read data to clear buffer (patch 1 in the previous img-ir patchset) to avoid conflicts, so that patch should be applied first. I've tagged both for stable (v3.15+). Cc: Mauro Carvalho Chehab m.che...@samsung.com Cc: Sifan Naeem sifan.na...@imgtec.com Cc: linux-media@vger.kernel.org James Hogan (2): img-ir/hw: Avoid clearing filter for no-op protocol change img-ir/hw: Fix potential deadlock stopping timer drivers/media/rc/img-ir/img-ir-hw.c | 28 +--- drivers/media/rc/img-ir/img-ir-hw.h | 3 +++ 2 files changed, 28 insertions(+), 3 deletions(-) -- 2.0.4 -- 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
[REVIEW PATCH 1/2] img-ir/hw: Avoid clearing filter for no-op protocol change
When the img-ir driver is asked to change protocol, if the chosen
decoder is already loaded then don't call img_ir_set_decoder(), so as
not to clear the current filter.
This is important because store_protocol() does not refresh the scancode
filter with the new protocol if the set of enabled protocols hasn't
actually changed, but it will still call the change_protocol() callback,
resulting in the filter being disabled in the hardware.
The problem can be reproduced by setting a filter, and then setting the
protocol to the same protocol that is already set:
$ echo nec protocols
$ echo 0x filter_mask
$ echo nec protocols
After this, messages which don't match the filter still get received.
Reported-by: Sifan Naeem sifan.na...@imgtec.com
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Mauro Carvalho Chehab m.che...@samsung.com
Cc: sta...@vger.kernel.org # v3.15+
Cc: linux-media@vger.kernel.org
---
drivers/media/rc/img-ir/img-ir-hw.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index 9db065344b41..1566337c1059 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -643,6 +643,12 @@ static int img_ir_change_protocol(struct rc_dev *dev, u64
*ir_type)
continue;
if (*ir_type dec-type) {
*ir_type = dec-type;
+ /*
+* We don't want to clear the filter if nothing is
+* changing as it won't get set again.
+*/
+ if (dec == hw-decoder)
+ return 0;
img_ir_set_decoder(priv, dec, *ir_type);
goto success;
}
--
2.0.4
--
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
[REVIEW PATCH 2/2] img-ir/hw: Fix potential deadlock stopping timer
The end timer is used for switching back from repeat code timings when
no repeat codes have been received for a certain amount of time. When
the protocol is changed, the end timer is deleted synchronously with
del_timer_sync(), however this takes place while holding the main spin
lock, and the timer handler also needs to acquire the spin lock.
This opens the possibility of a deadlock on an SMP system if the
protocol is changed just as the repeat timer is expiring. One CPU could
end up in img_ir_set_decoder() holding the lock and waiting for the end
timer to complete, while the other CPU is stuck in the timer handler
spinning on the lock held by the first CPU.
Lockdep also spots a possible lock inversion in the same code, since
img_ir_set_decoder() acquires the img-ir lock before the timer lock, but
the timer handler will try and acquire them the other way around:
=
[ INFO: possible irq lock inversion dependency detected ]
3.18.0-rc5+ #957 Not tainted
-
swapper/0/0 just changed the state of lock:
(((hw-end_timer))){+.-...}, at: [4006ae5c] _call_timer_fn+0x0/0xfc
but this lock was taken by another, HARDIRQ-safe lock in the past:
((priv-lock)-rlock#2){-.}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this:
Possible interrupt unsafe locking scenario:
CPU0CPU1
lock(((hw-end_timer)));
local_irq_disable();
lock((priv-lock)-rlock#2);
lock(((hw-end_timer)));
Interrupt
lock((priv-lock)-rlock#2);
*** DEADLOCK ***
This is fixed by releasing the main spin lock while performing the
del_timer_sync() call. The timer is prevented from restarting before the
lock is reacquired by a new stopping flag which img_ir_handle_data()
checks before updating the timer.
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Mauro Carvalho Chehab m.che...@samsung.com
Cc: Sifan Naeem sifan.na...@imgtec.com
Cc: sta...@vger.kernel.org # v3.15+
Cc: linux-media@vger.kernel.org
---
This patch depends on img-ir/hw: Always read data to clear buffer from
my recent img-ir patchset to avoid a conflict.
---
drivers/media/rc/img-ir/img-ir-hw.c | 22 +++---
drivers/media/rc/img-ir/img-ir-hw.h | 3 +++
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index 1566337c1059..76eaa323ae51 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -530,6 +530,22 @@ static void img_ir_set_decoder(struct img_ir_priv *priv,
u32 ir_status, irq_en;
spin_lock_irq(priv-lock);
+ /*
+* First record that the protocol is being stopped so that the end timer
+* isn't restarted while we're trying to stop it.
+*/
+ hw-stopping = true;
+
+ /*
+* Release the lock to stop the end timer, since the end timer handler
+* acquires the lock and we don't want to deadlock waiting for it.
+*/
+ spin_unlock_irq(priv-lock);
+ del_timer_sync(hw-end_timer);
+ spin_lock_irq(priv-lock);
+
+ hw-stopping = false;
+
/* switch off and disable interrupts */
img_ir_write(priv, IMG_IR_CONTROL, 0);
irq_en = img_ir_read(priv, IMG_IR_IRQ_ENABLE);
@@ -547,8 +563,7 @@ static void img_ir_set_decoder(struct img_ir_priv *priv,
img_ir_read(priv, IMG_IR_DATA_LW);
img_ir_read(priv, IMG_IR_DATA_UP);
- /* stop the end timer and switch back to normal mode */
- del_timer_sync(hw-end_timer);
+ /* switch back to normal mode */
hw-mode = IMG_IR_M_NORMAL;
/* clear the wakeup scancode filter */
@@ -825,7 +840,8 @@ static void img_ir_handle_data(struct img_ir_priv *priv,
u32 len, u64 raw)
}
- if (dec-repeat) {
+ /* we mustn't update the end timer while trying to stop it */
+ if (dec-repeat !hw-stopping) {
unsigned long interval;
img_ir_begin_repeat(priv);
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h
b/drivers/media/rc/img-ir/img-ir-hw.h
index a8c6a8d40206..5c2b216c5fe3 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -211,6 +211,8 @@ enum img_ir_mode {
* @flags: IMG_IR_F_*.
* @filters: HW filters (derived from scancode filters).
* @mode: Current decode mode.
+ * @stopping: Indicates that decoder is being taken down and timers
+ * should not be restarted.
* @suspend_irqen: Saved IRQ enable mask over suspend.
*/
struct img_ir_priv_hw {
@@ -226,6 +228,7 @@ struct img_ir_priv_hw {
struct img_ir_filterfilters[RC_FILTER_MAX];
enum
Re: [REVIEW] Submitting Media Patches
On 22 October 2014 15:12, Hans Verkuil hverk...@xs4all.nl wrote: How to submit patches for a stable kernel = The standard method is to add this tag: Cc: sta...@vger.kernel.org possibly with a comment saying to which versions it should be applied, like: Cc: sta...@vger.kernel.org # for v3.5 and up Maybe put angled brackets around the email address. Some versions of git-send-email get confused by the comment otherwise and try sending to e.g. sta...@vger.kernel.org #3.11. Cheers James -- 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
[REVIEW PATCH 0/5] img-ir: Some fixes
Here are a few fixes for the img-ir RC driver. Patch 1 is the important one. I've tagged it for stable. The other 4 are minor fixes/improvements that don't need backporting to stable. Dylan Rajaratnam (1): img-ir/hw: Always read data to clear buffer James Hogan (4): img-ir/hw: Drop [un]register_decoder declarations img-ir: Depend on METAG or MIPS or COMPILE_TEST img-ir: Don't set driver's module owner MAINTAINERS: Add myself as img-ir maintainer MAINTAINERS | 5 + drivers/media/rc/img-ir/Kconfig | 1 + drivers/media/rc/img-ir/img-ir-core.c | 1 - drivers/media/rc/img-ir/img-ir-hw.c | 6 -- drivers/media/rc/img-ir/img-ir-hw.h | 3 --- 5 files changed, 10 insertions(+), 6 deletions(-) -- 2.0.4 -- 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
[REVIEW PATCH 4/5] img-ir: Don't set driver's module owner
Don't bother setting .owner = THIS_MODULE, since it's already handled by
the platform_driver_register macro.
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Mauro Carvalho Chehab m.che...@samsung.com
Cc: linux-media@vger.kernel.org
---
drivers/media/rc/img-ir/img-ir-core.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/media/rc/img-ir/img-ir-core.c
b/drivers/media/rc/img-ir/img-ir-core.c
index a0cac2f09109..77c78de4f5bf 100644
--- a/drivers/media/rc/img-ir/img-ir-core.c
+++ b/drivers/media/rc/img-ir/img-ir-core.c
@@ -166,7 +166,6 @@ MODULE_DEVICE_TABLE(of, img_ir_match);
static struct platform_driver img_ir_driver = {
.driver = {
.name = img-ir,
- .owner = THIS_MODULE,
.of_match_table = img_ir_match,
.pm = img_ir_pmops,
},
--
2.0.4
--
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
[REVIEW PATCH 3/5] img-ir: Depend on METAG or MIPS or COMPILE_TEST
The ImgTec Infrared decoder block which img-ir drives is only used in IMGWorks SoCs so far, such as the TZ1090 (Meta based) and the upcoming Pistachio (MIPS based). Therefore make the driver depend on METAG (for TZ1090) or MIPS (for Pistachio) or COMPILE_TEST (so that it is included in x86 allmodconfig builds), to avoid cluttering the Kconfig menu with drivers for hardware that isn't yet available on other platforms. Signed-off-by: James Hogan james.ho...@imgtec.com Cc: Mauro Carvalho Chehab m.che...@samsung.com Cc: linux-media@vger.kernel.org --- drivers/media/rc/img-ir/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/rc/img-ir/Kconfig b/drivers/media/rc/img-ir/Kconfig index 03ba9fc170fb..580715c7fc5e 100644 --- a/drivers/media/rc/img-ir/Kconfig +++ b/drivers/media/rc/img-ir/Kconfig @@ -1,6 +1,7 @@ config IR_IMG tristate ImgTec IR Decoder depends on RC_CORE + depends on METAG || MIPS || COMPILE_TEST select IR_IMG_HW if !IR_IMG_RAW help Say Y or M here if you want to use the ImgTec infrared decoder -- 2.0.4 -- 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
[REVIEW PATCH 2/5] img-ir/hw: Drop [un]register_decoder declarations
The img_ir_register_decoder() and img_ir_unregister_decoder() functions
were dropped prior to the img-ir driver being applied to simplify the
protocol decoder setup. However the declarations of these functions in
img-ir-hw.h were still included. Delete them since they're completely
unused.
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Mauro Carvalho Chehab m.che...@samsung.com
Cc: linux-media@vger.kernel.org
---
drivers/media/rc/img-ir/img-ir-hw.h | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.h
b/drivers/media/rc/img-ir/img-ir-hw.h
index 8fcc16c32c5b..a8c6a8d40206 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.h
+++ b/drivers/media/rc/img-ir/img-ir-hw.h
@@ -186,9 +186,6 @@ struct img_ir_reg_timings {
struct img_ir_timing_regvalsrtimings;
};
-int img_ir_register_decoder(struct img_ir_decoder *dec);
-void img_ir_unregister_decoder(struct img_ir_decoder *dec);
-
struct img_ir_priv;
#ifdef CONFIG_IR_IMG_HW
--
2.0.4
--
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
[REVIEW PATCH 5/5] MAINTAINERS: Add myself as img-ir maintainer
Add myself as the maintainer for the Imagination Technologies Infrared Decoder driver. Signed-off-by: James Hogan james.ho...@imgtec.com Cc: Mauro Carvalho Chehab m.che...@samsung.com Cc: linux-media@vger.kernel.org --- MAINTAINERS | 5 + 1 file changed, 5 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index ea4d0058fd1b..814cf15448ad 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4757,6 +4757,11 @@ L: linux-security-mod...@vger.kernel.org S: Supported F: security/integrity/ima/ +IMGTEC IR DECODER DRIVER +M: James Hogan james.ho...@imgtec.com +S: Maintained +F: drivers/media/rc/img-ir/ + IMS TWINTURBO FRAMEBUFFER DRIVER L: linux-fb...@vger.kernel.org S: Orphan -- 2.0.4 -- 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
[REVIEW PATCH FOR v3.18 1/5] img-ir/hw: Always read data to clear buffer
From: Dylan Rajaratnam dylan.rajarat...@imgtec.com
A problem was found on Polaris where if the unit it booted via the power
button on the infrared remote then the next button press on the remote
would return the key code used to power on the unit.
The sequence is:
- The polaris powered off but with the powerdown controller (PDC) block
still powered.
- Press power key on remote, IR block receives the key.
- Kernel starts, IR code is in IMG_IR_DATA_x but neither IMG_IR_RXDVAL
or IMG_IR_RXDVALD2 are set.
- Wait any amount of time.
- Press any key.
- IMG_IR_RXDVAL or IMG_IR_RXDVALD2 is set but IMG_IR_DATA_x is
unchanged since the powerup key data was never read.
This is worked around by always reading the IMG_IR_DATA_x in
img_ir_set_decoder(), rather than only when the IMG_IR_RXDVAL or
IMG_IR_RXDVALD2 bit is set.
Signed-off-by: Dylan Rajaratnam dylan.rajarat...@imgtec.com
Signed-off-by: James Hogan james.ho...@imgtec.com
Cc: Mauro Carvalho Chehab m.che...@samsung.com
Cc: linux-media@vger.kernel.org
Cc: sta...@vger.kernel.org # v3.15+
---
drivers/media/rc/img-ir/img-ir-hw.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index ec49f94425fc..9db065344b41 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -541,10 +541,12 @@ static void img_ir_set_decoder(struct img_ir_priv *priv,
if (ir_status (IMG_IR_RXDVAL | IMG_IR_RXDVALD2)) {
ir_status = ~(IMG_IR_RXDVAL | IMG_IR_RXDVALD2);
img_ir_write(priv, IMG_IR_STATUS, ir_status);
- img_ir_read(priv, IMG_IR_DATA_LW);
- img_ir_read(priv, IMG_IR_DATA_UP);
}
+ /* always read data to clear buffer if IR wakes the device */
+ img_ir_read(priv, IMG_IR_DATA_LW);
+ img_ir_read(priv, IMG_IR_DATA_UP);
+
/* stop the end timer and switch back to normal mode */
del_timer_sync(hw-end_timer);
hw-mode = IMG_IR_M_NORMAL;
--
2.0.4
--
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
panic in cx23885
I'm still not having any luck getting reliable operation of the second tuner on my cx23885 based card. Incidental to this, I'm getting a crash in cx23885_video_wakeup at the line: buf-vb.v4l2_buf.sequence = q-count++; (full log follows this email) I'm not sure exactly why, but I do know that the hardware is well and truly messed up at this point. IRQ's are reading 0x from a lot of the hardware registers etc. Is it worth putting a test in cx23885_video_wakeup to stop it following bad pointers and recovering from a potential crash, or are things so bad by this point that it isn't worth it? I'm getting things like this too: Uhhuh. NMI received for unknown reason 21 on CPU 0. Do you have a strange power saving mode enabled? Dazed and confused, but trying to continue Everything works perfectly with only one tuner enabled so I still think the hardware is okay. Thanks James [ 346.183389] cx23885 driver version 0.0.4 loaded [ 346.188612] Already setup the GSI :16 [ 346.192970] CORE cx23885[0]: subsystem: 18ac:db98, board: DViCO FusionHDTV DVB-T Dual Express2 [card=44,autodetected] [ 346.344149] Registered IR keymap rc-fusionhdtv-mce [ 346.349632] input: i2c IR (FusionHDTV) as /devices/virtual/rc/rc0/input6 [ 346.357248] rc0: i2c IR (FusionHDTV) as /devices/virtual/rc/rc0 [ 346.363937] ir-kbd-i2c: i2c IR (FusionHDTV) detected at i2c-6/6-006b/ir0 [cx23885[0]] [ 346.402161] cx23885_dvb_register() allocating 1 frontend(s) [ 346.408458] cx23885[0]: cx23885 based dvb card [ 346.618609] DiB0070: successfully identified [ 346.623478] DVB: registering new adapter (cx23885[0]) [ 346.629218] cx23885 :10:00.0: DVB: registering adapter 0 frontend 0 (DiBcom 7000PC)... [ 346.639696] cx23885_dvb_register() allocating 1 frontend(s) [ 346.646024] cx23885[0]: cx23885 based dvb card [ 346.854595] DiB0070: successfully identified [ 346.859460] DVB: registering new adapter (cx23885[0]) [ 346.865196] cx23885 :10:00.0: DVB: registering adapter 1 frontend 0 (DiBcom 7000PC)... [ 346.874967] cx23885_dev_checkrevision() Hardware revision = 0xa5 [ 346.881785] cx23885[0]/0: found at :10:00.0, rev: 4, irq: 16, latency: 0, mmio: 0xdfa0 [ 364.229718] cx23885[0]: TS1 B - dma channel status dump [ 364.235665] cx23885[0]: cmds: init risc lo : 0x0804 [ 364.241989] cx23885[0]: cmds: init risc hi : 0x [ 364.248306] cx23885[0]: cmds: cdt base : 0x00010580 [ 364.254622] cx23885[0]: cmds: cdt size : 0x000a [ 364.260946] cx23885[0]: cmds: iq base: 0x00010400 [ 364.267283] cx23885[0]: cmds: iq size: 0x0010 [ 364.273605] cx23885[0]: cmds: risc pc lo : 0x [ 364.279923] cx23885[0]: cmds: risc pc hi : 0x [ 364.286254] cx23885[0]: cmds: iq wr ptr : 0x [ 364.292591] cx23885[0]: cmds: iq rd ptr : 0x [ 364.298923] cx23885[0]: cmds: cdt current: 0x [ 364.305255] cx23885[0]: cmds: pci target lo : 0x [ 364.311590] cx23885[0]: cmds: pci target hi : 0x [ 364.317926] cx23885[0]: cmds: line / byte: 0x [ 364.324257] cx23885[0]: risc0: 0x [ INVALID count=0 ] [ 364.331104] cx23885[0]: risc1: 0x [ INVALID count=0 ] [ 364.337938] cx23885[0]: risc2: 0x [ INVALID count=0 ] [ 364.344762] cx23885[0]: risc3: 0x [ INVALID count=0 ] [ 364.351629] cx23885[0]: (0x00010400) iq 0: 0xb6335993 [ writerm eol irq2 21 20 cnt1 cnt0 14 12 count=2451 ] [ 364.363430] cx23885[0]: iq 1: 0x806ea57e [ arg #1 ] [ 364.369166] cx23885[0]: iq 2: 0xf09414e3 [ arg #2 ] [ 364.374901] cx23885[0]: (0x0001040c) iq 3: [ 364.379946] 0xb5f47d7e [ writerm eol irq1 23 22 21 20 18 14 13 12 [ 364.387525] count=3454 ] [ 364.390567] cx23885[0]: iq 4: 0x2167057e [ arg #1 ] [ 364.396300] cx23885[0]: iq 5: 0xf45bc3a9 [ arg #2 ] [ 364.402035] cx23885[0]: (0x00010418) iq 6: 0x932db884 [ read irq2 irq1 21 19 18 cnt0 resync 13 12 count=2180 ] [ 364.414183] cx23885[0]: (0x0001041c) iq 7: 0x79e72545 [ jump sol irq1 23 22 21 18 cnt1 cnt0 13 count=1349 ] [ 364.426019] cx23885[0]: iq 8: 0xb5cf52c3 [ arg #1 ] [ 364.431756] cx23885[0]: iq 9: 0x9ad81dad [ arg #2 ] [ 364.437490] cx23885[0]: (0x00010428) iq a: 0x0b6a2271 [ INVALID sol irq2 irq1 22 21 19 cnt1 13 count=625 ] [ 364.449193] cx23885[0]: (0x0001042c) iq b: 0x75349ce0 [ jump eol irq1 21 20 18 resync 12 count=3296 ] [ 364.460369] cx23885[0]: iq c: 0x41bfb24c [ arg #1 ] [ 364.466106] cx23885[0]: iq d: 0x796c6b1e [ arg #2 ] [ 364.471840] cx23885[0]: (0x00010438) iq e: 0x5319b595 [ writec irq2 irq1 20 19 cnt0 resync 13 12 count=1429 ] [ 364.483863] cx23885[0]: (0x0001043c) iq f: 0x1d9345c2 [ write sol eol irq1 23 20 cnt1 cnt0 14 count=1474 ] [ 364.495564] cx23885[0]: iq 10: 0x0234dd60 [ arg #1 ] [ 364.501413] cx23885[0]: iq 11: 0x [ arg #2 ] [ 364.507323] cx23885[0]: fifo: 0x5000 - 0x6000 [ 364.512816] cx23885[0
Reply
Sequel to your non-response to my previous email, I am re-sending this to you again thus; A deceased client of mine who died of a heart-related ailment about 3 years ago left behind some funds which I want you to assist in retriving and distributing. Reply so I can give you details. Regards, James. -- 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: buffer delivery stops with cx23885
041ad449683bb2d54a7f082d78ec15bbc958a175 introduced stats gathering into the dib7000p code which seems to generate a lot more i2c traffic which would exacerbate the problem, if one existed. The timing (May/June) very roughly matches what I remember too. When my recording stops, so do the Next all layers stats available in... messages, although that could be for other reasons. Anyway, I'll comment out the call to dib7000p_get_stats and see if it makes a difference. That didn't help. Having chmod 'd /dev/dvb/adapter1 so that mythtv can't open the second tuner, things have been absolutely solid for a week. As soon as I let mythtv access the second tuner it hangs almost immediately. Based on a bunch of printk's I added, the tuner loses sync/lock which would explain why buffer delivery stops. I had been looking in the wrong place. I can't see anything happening when this happens though, it doesn't seem to coincide with gathering stats or anything but I can't be completely sure. Other things I have observed when things stop working are: DiB7000P: i2c read error (often) DiB0070 I2C read failed (less often) DiB0070 I2C write failed (less often) NMI: PCI system error (SERR) for reason b1 on CPU 0. (rarely) Where could the two tuners be treading on each other? Thanks James
RE: buffer delivery stops with cx23885
I'll test out the downgrade to 73d8102298719863d54264f62521362487f84256 just to be sure, then see if I can take it further back. 73d8102298719863d54264f62521362487f84256 still breaks for me, so it's definitely not related to the conversion. Any hints on what I can do to figure out what layer it's stalling at? Thanks James
RE: buffer delivery stops with cx23885
Any hints on what I can do to figure out what layer it's stalling at? I have no idea. I would do a git bisect to try and narrow it down. Problem with the bisect is that I can't be sure what version it actually worked on. It certainly predates when my patch for my card was committed. I have a hunch that it might be the two tuners stomping on each other, possibly in the i2c code. Sometimes recording just stops, other times I get i2c errors. With one tuner disabled in MythTV I can't seem to reproduce the problem anymore. I haven't let it run for long enough to be sure, but it normally would have crashed by now. James
RE: buffer delivery stops with cx23885
Any hints on what I can do to figure out what layer it's stalling at?
I have no idea. I would do a git bisect to try and narrow it down.
Problem with the bisect is that I can't be sure what version it actually
worked on. It certainly predates when my patch for my card was committed.
I have a hunch that it might be the two tuners stomping on each other,
possibly in the i2c code. Sometimes recording just stops, other times I get
i2c
errors.
Is that two tuners in the same device, or two tuners in different devices?
DViCO FusionHDTV DVB-T Dual Express2
2 tuners on one card
James
N�r��yb�X��ǧv�^�){.n�+{���bj)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
RE: buffer delivery stops with cx23885
Is that two tuners in the same device, or two tuners in different devices?
DViCO FusionHDTV DVB-T Dual Express2
2 tuners on one card
Any idea if there were changes or are issues with i2c access when there are
two tuners in the same device?
That's not really my expertise but you might know something about that.
041ad449683bb2d54a7f082d78ec15bbc958a175 introduced stats gathering into the
dib7000p code which seems to generate a lot more i2c traffic which would
exacerbate the problem, if one existed. The timing (May/June) very roughly
matches what I remember too. When my recording stops, so do the Next all
layers stats available in... messages, although that could be for other
reasons.
Anyway, I'll comment out the call to dib7000p_get_stats and see if it makes a
difference.
James
N�r��yb�X��ǧv�^�){.n�+{���bj)w* jg��� �ݢj/���z�ޖ��2�ޙ�)ߡ�a���� �G���h��j:+v���w��٥
RE: buffer delivery stops with cx23885
And important for me, because if it IS related to the vb2 conversion then I
need to know asap.
Might take a few days to be completely sure. I've wondered if it's something
to do with the signal too (maybe some bug when signal error occurs?). So
many variables :(
I can reasonably reliably reproduce the problem using dvbstream. strace output
below. It looks pretty much as you'd expect - poll returning normally up until
it doesn't anymore.
I'll test out the downgrade to 73d8102298719863d54264f62521362487f84256 just to
be sure, then see if I can take it further back.
James
poll([{fd=7, events=POLLIN|POLLPRI}], 1, 500) = 1 ([{fd=7,
revents=POLLIN|POLLPRI}])
accept(0, 0x712e20, [0])= -1 ENOTSOCK (Socket operation on
non-socket)
read(7,
G\v\30\37\252V\317\373\21\377\35\265\6\200Z\200\34\24\342\337bw\237\265\314,:!`\n\370\4...,
188) = 188
write(3,
G\v\30\37\252V\317\373\21\377\35\265\6\200Z\200\34\24\342\337bw\237\265\314,:!`\n\370\4...,
188) = 188
poll([{fd=7, events=POLLIN|POLLPRI}], 1, 500) = 1 ([{fd=7,
revents=POLLIN|POLLPRI}])
accept(0, 0x712e20, [0])= -1 ENOTSOCK (Socket operation on
non-socket)
read(7,
G\v\30\20\f0n!\200\301\10)@),\261\241\330\242\213\376\f\345\223\21\202@N\2b\224i...,
188) = 188
write(3,
G\v\30\20\f0n!\200\301\10)@),\261\241\330\242\213\376\f\345\223\21\202@N\2b\224i...,
188) = 188
poll([{fd=7, events=POLLIN|POLLPRI}], 1, 500) = 1 ([{fd=7,
revents=POLLIN|POLLPRI}])
accept(0, 0x712e20, [0])= -1 ENOTSOCK (Socket operation on
non-socket)
read(7,
G\v\30\21%\35]\356\rv\372+\0222\34\233\312v,\230\371\363\204s\205RQ\334\243\21\337\223...,
188) = 188
write(3,
G\v\30\21%\35]\356\rv\372+\0222\34\233\312v,\230\371\363\204s\205RQ\334\243\21\337\223...,
188) = 188
poll([{fd=7, events=POLLIN|POLLPRI}], 1, 500) = 0 (Timeout)
accept(0, 0x712e20, [0])= -1 ENOTSOCK (Socket operation on
non-socket)
read(7, 0x77145120, 188)= -1 EAGAIN (Resource temporarily
unavailable)
poll([{fd=7, events=POLLIN|POLLPRI}], 1, 500) = 0 (Timeout)
accept(0, 0x712e20, [0])= -1 ENOTSOCK (Socket operation on
non-socket)
read(7, 0x77145120, 188)= -1 EAGAIN (Resource temporarily
unavailable)
poll([{fd=7, events=POLLIN|POLLPRI}], 1, 500) = 0 (Timeout)
accept(0, 0x712e20, [0])= -1 ENOTSOCK (Socket operation on
non-socket)
RE: [PATCH 1/3] vb2: fix VBI/poll regression
The recent conversion of saa7134 to vb2 unconvered a poll() bug that broke the teletext applications alevt and mtt. These applications expect that calling poll() without having called VIDIOC_STREAMON will cause poll() to return POLLERR. That did not happen in vb2. This patch fixes that behavior. It also fixes what should happen when poll() is called when STREAMON is called but no buffers have been queued. In that case poll() will also return POLLERR, but only for capture queues since output queues will always return POLLOUT anyway in that situation. This brings the vb2 behavior in line with the old videobuf behavior. What (mis)behaviour would this cause in userspace application? Thanks James -- 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: buffer delivery stops with cx23885
On 09/20/2014 05:32 AM, James Harper wrote: My cx23885 based DViCO FusionHDTV DVB-T Dual Express2 (I submitted a patch for this a little while ago) has been working great but over the last few months it has started playing up. Nothing has really changed that I can put my finger on. Basically mythtv stops recording after a few minutes sometimes. Rarely when this happens I see some i2c errors but mostly not. With cx23885 debug options turned on (debug=9 vbi_debug=9 v4l_debug=9 video_debug=9 irq_debug=9 ci_dbg=9) it seems like the card just stops delivering buffers (see dmesg output following). If I stop mythtv, all the buffers are cancelled (cx23885_stop_dma()) etc, and then restarting mythtv will get the recording going again, for a short time (minutes). Any suggestions to where I could start looking? Is it possible that my card itself is broken? (apart from this it's flawless). I see nothing wrong in the log, but you can try to use the current media_tree code. The cx23885's DMA engine has effectively been rewritten there, simplifying the control flow. Oops I should have mentioned that. I'm using Debian Jessie with 3.16 kernel and already using the latest v4l as per link you sent (my DViCO FusionHDTV DVB-T Dual Express2 patch is in 3.17 I think, but that's not in Debian yet). I think it only broke since the rewrite. Before that it seemed to be bulletproof. That was why I asked about the patch just before - I can't tell yet if the driver stops supplying data or if mythtv stops asking for data. If there was something funny about the poll loop then that could cause it. I suppose I can try and go back to an older version of the code and see what happens? Would the bug fixed by your fix VBI/poll regression patch cause intermittent stalls, or would the application that relied on the missing behaviour simply not work at all? In any case I've just applied the patch and about to reboot. Thanks James
RE: buffer delivery stops with cx23885
Oops I should have mentioned that. I'm using Debian Jessie with 3.16 kernel and already using the latest v4l as per link you sent (my DViCO FusionHDTV DVB-T Dual Express2 patch is in 3.17 I think, but that's not in Debian yet). Ah, yes, that's rather important information :-) I'll try to reproduce it. How often does it happen? I've setup a test where I just keep streaming to see if I can reproduce it. Happens within 5 minutes some nights (and reliably within 1 minute when I was testing with all the printk turned on), then not for hours other nights. Right now it hasn't crashed since I applied the VBI/poll regression patch (my kids are recording a few movies so I think I'd be in trouble if I tinkered with it any more tonight :) That's a fairly common pattern though - plays up when the kids are recording their afternoon shows when they get home from school, but then is often fairly stable after around 8pm when movies start. I can't really make sense of it. Someone mentioned seeing a few oddities when mythtv was busy downloading EIT guide but I can see when that happens and there isn't a correlation. I think it only broke since the rewrite. Before that it seemed to be bulletproof. That was why I asked about the patch just before - I can't tell yet if the driver stops supplying data or if mythtv stops asking for data. If there was something funny about the poll loop then that could cause it. I suppose I can try and go back to an older version of the code and see what happens? Can you test with the media_tree.git master branch, but going back to commit 73d8102298719863d54264f62521362487f84256? That has the cx23885 that has not yet been converted to vb2. Test with that for a while to see if that works without problems. Then go back to the HEAD of the master branch and try again. If it breaks, then I may have to revert the cx23885 vb2 changes until I figure out what's wrong. I was looking through the patches and saw a date of August 14 on the cx23885 to vb2 patch and thought that could have been around when it started breaking, but then the 73d8102298719863d54264f62521362487f84256 is dated September 3 and I'm pretty sure it had started playing up before then. About what date would I have seen the 453afdd9ce33293f640e84dc17e5f366701516e8 cx23885: convert to vb2 patch? In any case it should be easy enough to revert and build so I'll do that tomorrow once I can prove it still fails with the current regression patch applied. Thanks for your time! James
RE: buffer delivery stops with cx23885
I was looking through the patches and saw a date of August 14 on the cx23885 to vb2 patch and thought that could have been around when it started breaking, but then the 73d8102298719863d54264f62521362487f84256 is dated September 3 and I'm pretty sure it had started playing up before then. About what date would I have seen the 453afdd9ce33293f640e84dc17e5f366701516e8 cx23885: convert to vb2 patch? That patch was merged in the master branch September 8. If you've seen it earlier, then it may not be related to vb2 after all. I'd say not. If it is polling related, then it might be commit 9241650d62f79a3da01f1d5e8ebd195083330b75 (Don't return POLLERR during transient buffer underruns) which was added to the master branch on July 17th and was merged for 3.17. Or it could be something entirely different. You could try reverting that commit and see if that helps. That sounds plausible wrt timeframe, but if cx23885 only started using vb2 after Sept 8 then it couldn't have affected me before then right? In any case it should be easy enough to revert and build so I'll do that tomorrow once I can prove it still fails with the current regression patch applied. Which patch are you using? There have been several versions posted. This is the one you should use: https://patchwork.linuxtv.org/patch/25992/ That's the one I applied - you can even see my questions below in that link :) Based on what you have said I think that's not going to solve anything for me though. So I guess my plan is: . Revert to 73d8102298719863d54264f62521362487f84256 and test (not likely to fix but easy to test) . Revert to sometime around June when I submitted my patch for Fusion Dual Express 2 driver when I know it was reliable and test Other possibilities are: . MythTV bug . Defective card Time to google a command line dvb stream to rule out mythtv I guess... Thanks James
RE: buffer delivery stops with cx23885
That sounds plausible wrt timeframe, but if cx23885 only started using vb2 after Sept 8 then it couldn't have affected me before then right? You are right about that. You are using DVB right? Not analog video? (Just to be 100% certain). Yes. Australia mostly has no analog anymore. That's the one I applied - you can even see my questions below in that link :) Based on what you have said I think that's not going to solve anything for me though. If you are streaming DVB, then that patch has no effect since the vb2_poll call will never be called for DVB anyway, so that can be ruled out. Ok So I guess my plan is: . Revert to 73d8102298719863d54264f62521362487f84256 and test (not likely to fix but easy to test) And important for me, because if it IS related to the vb2 conversion then I need to know asap. Might take a few days to be completely sure. I've wondered if it's something to do with the signal too (maybe some bug when signal error occurs?). So many variables :( James
regression - missing fence.h
I'm building v4l against Debian Jessie (3.14.15) and I am getting this: In file included from /usr/local/src/media_build/v4l/../linux/include/media/videobuf2-core.h:19:0, from /usr/local/src/media_build/v4l/mcam-core.h:13, from /usr/local/src/media_build/v4l/cafe-driver.c:35: /usr/local/src/media_build/v4l/../linux/include/linux/dma-buf.h:33:25: fatal error: linux/fence.h: No such file or directory #include linux/fence.h Any suggestions before I go digging? It worked previously. Thanks James -- 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 01/29] img-ir: fix sparse warnings
On Thursday 21 August 2014 00:59:00 Hans Verkuil wrote: From: Hans Verkuil hans.verk...@cisco.com drivers/media/rc/img-ir/img-ir-nec.c:111:23: warning: symbol 'img_ir_nec' was not declared. Should it be static? drivers/media/rc/img-ir/img-ir-jvc.c:54:23: warning: symbol 'img_ir_jvc' was not declared. Should it be static? drivers/media/rc/img-ir/img-ir-sony.c:120:23: warning: symbol 'img_ir_sony' was not declared. Should it be static? drivers/media/rc/img-ir/img-ir-sharp.c:75:23: warning: symbol 'img_ir_sharp' was not declared. Should it be static? drivers/media/rc/img-ir/img-ir-sanyo.c:82:23: warning: symbol 'img_ir_sanyo' was not declared. Should it be static? Signed-off-by: Hans Verkuil hans.verk...@cisco.com Acked-by: James Hogan james.ho...@imgtec.com Thanks James -- 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 0/9] rc: Add IR encode based wakeup filtering
Hi Mauro, On Wednesday 23 July 2014 16:39:36 Mauro Carvalho Chehab wrote: Em Fri, 14 Mar 2014 23:04:10 + James Hogan ja...@albanarts.com escreveu: A recent discussion about proposed interfaces for setting up the hardware wakeup filter lead to the conclusion that it could help to have the generic capability to encode and modulate scancodes into raw IR events so that drivers for hardware with a low level wake filter (on the level of pulse/space durations) can still easily implement the higher level scancode interface that is proposed. I posted an RFC patchset showing how this could work, and Antti Seppälä posted additional patches to support rc5-sz and nuvoton-cir. This patchset improves the original RFC patches and combines updates Antti's patches. I'm happy these patches are a good start at tackling the problem, as long as Antti is happy with them and they work for him of course. Future work could include: - Encoders for more protocols. - Carrier signal events (no use unless a driver makes use of it). Patch 1 adds the new encode API. Patches 2-3 adds some modulation helpers. Patches 4-6 adds some raw encode implementations. Patch 7 adds some rc-core support for encode based wakeup filtering. Patch 8 adds debug loopback of encoded scancode when filter set. Patch 9 (untested) adds encode based wakeup filtering to nuvoton-cir. Changes in v2: Any news about this patch series? There are some comments about them, so I'll be tagging it as changes requested at patchwork, waiting for a v3 (or is it already there in the middle of the 49 patches from David?). This patch series seems to have been forgotten. I do have a few changes on top of v2 to address the review comments, so as you say I should probably rebase and do a v3 at some point. Cheers James -- 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: [REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)
On Thu, 2014-06-19 at 11:19 -0700, Greg KH wrote:
On Thu, Jun 19, 2014 at 01:45:30PM -0400, Rob Clark wrote:
On Thu, Jun 19, 2014 at 1:00 PM, Greg KH gre...@linuxfoundation.org wrote:
On Thu, Jun 19, 2014 at 10:00:18AM -0400, Rob Clark wrote:
On Wed, Jun 18, 2014 at 9:13 PM, Greg KH gre...@linuxfoundation.org
wrote:
On Wed, Jun 18, 2014 at 12:36:54PM +0200, Maarten Lankhorst wrote:
+#define CREATE_TRACE_POINTS
+#include trace/events/fence.h
+
+EXPORT_TRACEPOINT_SYMBOL(fence_annotate_wait_on);
+EXPORT_TRACEPOINT_SYMBOL(fence_emit);
Are you really willing to live with these as tracepoints for forever?
What is the use of them in debugging? Was it just for debugging the
fence code, or for something else?
+/**
+ * fence_context_alloc - allocate an array of fence contexts
+ * @num: [in]amount of contexts to allocate
+ *
+ * This function will return the first index of the number of fences
allocated.
+ * The fence context is used for setting fence-context to a unique
number.
+ */
+unsigned fence_context_alloc(unsigned num)
+{
+ BUG_ON(!num);
+ return atomic_add_return(num, fence_context_counter) - num;
+}
+EXPORT_SYMBOL(fence_context_alloc);
EXPORT_SYMBOL_GPL()? Same goes for all of the exports in here.
Traditionally all of the driver core exports have been with this
marking, any objection to making that change here as well?
tbh, I prefer EXPORT_SYMBOL().. well, I'd prefer even more if there
wasn't even a need for EXPORT_SYMBOL_GPL(), but sadly it is a fact of
life. We already went through this debate once with dma-buf. We
aren't going to change $evil_vendor's mind about non-gpl modules. The
only result will be a more flugly convoluted solution (ie. use syncpt
EXPORT_SYMBOL() on top of fence EXPORT_SYMBOL_GPL()) just as a
workaround, with the result that no-one benefits.
It has been proven that using _GPL() exports have caused companies to
release their code properly over the years, so as these really are
Linux-only apis, please change them to be marked this way, it helps
everyone out in the end.
Well, maybe that is the true in some cases. But it certainly didn't
work out that way for dma-buf. And I think the end result is worse.
I don't really like coming down on the side of EXPORT_SYMBOL() instead
of EXPORT_SYMBOL_GPL(), but if we do use EXPORT_SYMBOL_GPL() then the
result will only be creative workarounds using the _GPL symbols
indirectly by whatever is available via EXPORT_SYMBOL(). I don't
really see how that will be better.
You are saying that you _know_ companies will violate our license, so
you should just give up? And how do you know people aren't working on
preventing those indirect usages as well? :)
Sorry, I'm not going to give up here, again, it has proven to work in
the past in changing the ways of _very_ large companies, why stop now?
When you try to train a dog, you have to be consistent about it. We're
fantastically inconsistent in symbol exports.
For instance, the mutex primitives are all EXPORT_SYMBOL(), so we're
telling proprietary modules they can use them. However, when the kernel
is built with CONFIG_DEBUG_MUTEX, they all become
EXPORT_SYMBOL_GPL() ... what type of crazy message is that supposed to
send? It's OK to use mutexes but it's potentially a GPL violation to
debug them?
We either need to decide that we have a defined and consistent part of
our API that's GPL only or make the bold statement that we don't have
any part of our API that's usable by non-GPL modules. Right at the
minute we do neither and it confuses people no end about what is and
isn't allowed.
James
--
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: [REPOST PATCH 1/8] fence: dma-buf cross-device synchronization (v17)
On Thu, 2014-06-19 at 15:39 -0700, H. Peter Anvin wrote: On 06/19/2014 01:01 PM, Greg KH wrote: On Thu, Jun 19, 2014 at 09:15:36PM +0200, Daniel Vetter wrote: On Thu, Jun 19, 2014 at 7:00 PM, Greg KH gre...@linuxfoundation.org wrote: + BUG_ON(f1-context != f2-context); Nice, you just crashed the kernel, making it impossible to debug or recover :( agreed, that should probably be 'if (WARN_ON(...)) return NULL;' (but at least I wouldn't expect to hit that under console_lock so you should at least see the last N lines of the backtrace on the screen ;-)) Lots of devices don't have console screens :) Aside: This is a pet peeve of mine and recently I've switched to rejecting all patch that have a BUG_ON, period. Please do, I have been for a few years now as well for the same reasons you cite. I'm actually concerned about this trend. Downgrading things to WARN_ON can allow a security bug in the kernel to continue to exist, for example, or make the error message disappear. Me too. We use BUG_ON in the I/O subsystem where we're forced to violate a guarantee. When the choice is corrupt something or panic the system, I prefer the latter every time. I am wondering if the right thing here isn't to have a user (command line?) settable policy as to how to proceed on an assert violation, instead of hardcoding it at compile time. I'd say it depends on the consequence of the assertion violation. We have assertions that are largely theoretical, ones that govern process internal state (so killing the process mostly sanitizes the system) and a few that imply data loss or data corruption. James -- 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] vmalloc_sg: make sure all pages in vmalloc area are really DMA-ready
Patch originally written by Konrad. Rebased on current linux media tree.
Under Xen, vmalloc_32() isn't guaranteed to return pages which are really
under 4G in machine physical addresses (only in virtual pseudo-physical
addresses). To work around this, implement a vmalloc variant which
allocates each page with dma_alloc_coherent() to guarantee that each
page is suitable for the device in question.
Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Signed-off-by: James Harper james.har...@ejbdigital.com.au
---
drivers/media/v4l2-core/videobuf-dma-sg.c | 62 +--
include/media/videobuf-dma-sg.h | 3 ++
2 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index 828e7c1..3c8cc02 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -211,13 +211,36 @@ EXPORT_SYMBOL_GPL(videobuf_dma_init_user);
int videobuf_dma_init_kernel(struct videobuf_dmabuf *dma, int direction,
int nr_pages)
{
+ int i;
+
dprintk(1, init kernel [%d pages]\n, nr_pages);
dma-direction = direction;
- dma-vaddr = vmalloc_32(nr_pages PAGE_SHIFT);
+ dma-vaddr_pages = kcalloc(nr_pages, sizeof(*dma-vaddr_pages),
+ GFP_KERNEL);
+ if (!dma-vaddr_pages)
+ return -ENOMEM;
+
+ dma-dma_addr = kcalloc(nr_pages, sizeof(*dma-dma_addr), GFP_KERNEL);
+ if (!dma-dma_addr) {
+ kfree(dma-vaddr_pages);
+ return -ENOMEM;
+ }
+ for (i = 0; i nr_pages; i++) {
+ void *addr;
+
+ addr = dma_alloc_coherent(dma-dev, PAGE_SIZE,
+ (dma-dma_addr[i]), GFP_KERNEL);
+ if (addr == NULL)
+ goto out_free_pages;
+
+ dma-vaddr_pages[i] = virt_to_page(addr);
+ }
+ dma-vaddr = vmap(dma-vaddr_pages, nr_pages, VM_MAP | VM_IOREMAP,
+ PAGE_KERNEL);
if (NULL == dma-vaddr) {
dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
- return -ENOMEM;
+ goto out_free_pages;
}
dprintk(1, vmalloc is at addr 0x%08lx, size=%d\n,
@@ -228,6 +251,19 @@ int videobuf_dma_init_kernel(struct videobuf_dmabuf *dma,
int direction,
dma-nr_pages = nr_pages;
return 0;
+out_free_pages:
+ while (i 0) {
+ void *addr = page_address(dma-vaddr_pages[i]);
+ dma_free_coherent(dma-dev, PAGE_SIZE, addr, dma-dma_addr[i]);
+ i--;
+ }
+ kfree(dma-dma_addr);
+ dma-dma_addr = NULL;
+ kfree(dma-vaddr_pages);
+ dma-vaddr_pages = NULL;
+
+ return -ENOMEM;
+
}
EXPORT_SYMBOL_GPL(videobuf_dma_init_kernel);
@@ -322,8 +358,21 @@ int videobuf_dma_free(struct videobuf_dmabuf *dma)
dma-pages = NULL;
}
- vfree(dma-vaddr);
- dma-vaddr = NULL;
+ if (dma-dma_addr) {
+ for (i = 0; i dma-nr_pages; i++) {
+ void *addr;
+
+ addr = page_address(dma-vaddr_pages[i]);
+ dma_free_coherent(dma-dev, PAGE_SIZE, addr,
+ dma-dma_addr[i]);
+ }
+ kfree(dma-dma_addr);
+ dma-dma_addr = NULL;
+ kfree(dma-vaddr_pages);
+ dma-vaddr_pages = NULL;
+ vunmap(dma-vaddr);
+ dma-vaddr = NULL;
+ }
if (dma-bus_addr)
dma-bus_addr = 0;
@@ -461,6 +510,11 @@ static int __videobuf_iolock(struct videobuf_queue *q,
MAGIC_CHECK(mem-magic, MAGIC_SG_MEM);
+ if (!mem-dma.dev)
+ mem-dma.dev = q-dev;
+ else
+ WARN_ON(mem-dma.dev != q-dev);
+
switch (vb-memory) {
case V4L2_MEMORY_MMAP:
case V4L2_MEMORY_USERPTR:
diff --git a/include/media/videobuf-dma-sg.h b/include/media/videobuf-dma-sg.h
index d8fb601..fb6fd4d8 100644
--- a/include/media/videobuf-dma-sg.h
+++ b/include/media/videobuf-dma-sg.h
@@ -53,6 +53,9 @@ struct videobuf_dmabuf {
/* for kernel buffers */
void*vaddr;
+ struct page **vaddr_pages;
+ dma_addr_t *dma_addr;
+ struct device *dev;
/* for overlay buffers (pci-pci dma) */
dma_addr_t bus_addr;
--
2.0.0
--
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] vmalloc_sg: make sure all pages in vmalloc area are really DMA-ready
Patch originally written by Konrad. Rebased on current linux media tree.
Under Xen, vmalloc_32() isn't guaranteed to return pages which are really
under 4G in machine physical addresses (only in virtual pseudo-physical
addresses). To work around this, implement a vmalloc variant which
allocates each page with dma_alloc_coherent() to guarantee that each
page is suitable for the device in question.
Signed-off-by: Konrad Rzeszutek Wilk konrad.w...@oracle.com
Signed-off-by: James Harper james.har...@ejbdigital.com.au
---
drivers/media/v4l2-core/videobuf-dma-sg.c | 62 +--
include/media/videobuf-dma-sg.h | 3 ++
2 files changed, 61 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index 828e7c1..3c8cc02 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -211,13 +211,36 @@ EXPORT_SYMBOL_GPL(videobuf_dma_init_user);
int videobuf_dma_init_kernel(struct videobuf_dmabuf *dma, int direction,
int nr_pages)
{
+ int i;
+
dprintk(1, init kernel [%d pages]\n, nr_pages);
dma-direction = direction;
- dma-vaddr = vmalloc_32(nr_pages PAGE_SHIFT);
+ dma-vaddr_pages = kcalloc(nr_pages, sizeof(*dma-vaddr_pages),
+ GFP_KERNEL);
+ if (!dma-vaddr_pages)
+ return -ENOMEM;
+
+ dma-dma_addr = kcalloc(nr_pages, sizeof(*dma-dma_addr), GFP_KERNEL);
+ if (!dma-dma_addr) {
+ kfree(dma-vaddr_pages);
+ return -ENOMEM;
+ }
+ for (i = 0; i nr_pages; i++) {
+ void *addr;
+
+ addr = dma_alloc_coherent(dma-dev, PAGE_SIZE,
+ (dma-dma_addr[i]), GFP_KERNEL);
+ if (addr == NULL)
+ goto out_free_pages;
+
+ dma-vaddr_pages[i] = virt_to_page(addr);
+ }
+ dma-vaddr = vmap(dma-vaddr_pages, nr_pages, VM_MAP | VM_IOREMAP,
+ PAGE_KERNEL);
if (NULL == dma-vaddr) {
dprintk(1, vmalloc_32(%d pages) failed\n, nr_pages);
- return -ENOMEM;
+ goto out_free_pages;
}
dprintk(1, vmalloc is at addr 0x%08lx, size=%d\n,
@@ -228,6 +251,19 @@ int videobuf_dma_init_kernel(struct videobuf_dmabuf *dma,
int direction,
dma-nr_pages = nr_pages;
return 0;
+out_free_pages:
+ while (i 0) {
+ void *addr = page_address(dma-vaddr_pages[i]);
+ dma_free_coherent(dma-dev, PAGE_SIZE, addr, dma-dma_addr[i]);
+ i--;
+ }
+ kfree(dma-dma_addr);
+ dma-dma_addr = NULL;
+ kfree(dma-vaddr_pages);
+ dma-vaddr_pages = NULL;
+
+ return -ENOMEM;
+
}
EXPORT_SYMBOL_GPL(videobuf_dma_init_kernel);
@@ -322,8 +358,21 @@ int videobuf_dma_free(struct videobuf_dmabuf *dma)
dma-pages = NULL;
}
- vfree(dma-vaddr);
- dma-vaddr = NULL;
+ if (dma-dma_addr) {
+ for (i = 0; i dma-nr_pages; i++) {
+ void *addr;
+
+ addr = page_address(dma-vaddr_pages[i]);
+ dma_free_coherent(dma-dev, PAGE_SIZE, addr,
+ dma-dma_addr[i]);
+ }
+ kfree(dma-dma_addr);
+ dma-dma_addr = NULL;
+ kfree(dma-vaddr_pages);
+ dma-vaddr_pages = NULL;
+ vunmap(dma-vaddr);
+ dma-vaddr = NULL;
+ }
if (dma-bus_addr)
dma-bus_addr = 0;
@@ -461,6 +510,11 @@ static int __videobuf_iolock(struct videobuf_queue *q,
MAGIC_CHECK(mem-magic, MAGIC_SG_MEM);
+ if (!mem-dma.dev)
+ mem-dma.dev = q-dev;
+ else
+ WARN_ON(mem-dma.dev != q-dev);
+
switch (vb-memory) {
case V4L2_MEMORY_MMAP:
case V4L2_MEMORY_USERPTR:
diff --git a/include/media/videobuf-dma-sg.h b/include/media/videobuf-dma-sg.h
index d8fb601..fb6fd4d8 100644
--- a/include/media/videobuf-dma-sg.h
+++ b/include/media/videobuf-dma-sg.h
@@ -53,6 +53,9 @@ struct videobuf_dmabuf {
/* for kernel buffers */
void*vaddr;
+ struct page **vaddr_pages;
+ dma_addr_t *dma_addr;
+ struct device *dev;
/* for overlay buffers (pci-pci dma) */
dma_addr_t bus_addr;
--
2.0.0
--
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] Add support for DViCO FusionHDTV DVB-T Dual Express2
DViCO FusionHDTV DVB-T Dual Express2 is cx23885 + dib7070
Signed-off-by: James Harper james.har...@ejbdigital.com.au
---
drivers/media/pci/cx23885/cx23885-cards.c | 15 +++-
drivers/media/pci/cx23885/cx23885-dvb.c | 123 ++
drivers/media/pci/cx23885/cx23885.h | 1 +
3 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/drivers/media/pci/cx23885/cx23885-cards.c
b/drivers/media/pci/cx23885/cx23885-cards.c
index 79f20c8..9ca8855 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -649,7 +649,12 @@ struct cx23885_board cx23885_boards[] = {
CX25840_NONE1_CH3,
.amux = CX25840_AUDIO6,
} },
- }
+ },
+ [CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2] = {
+ .name = DViCO FusionHDTV DVB-T Dual Express2,
+ .portb = CX23885_MPEG_DVB,
+ .portc = CX23885_MPEG_DVB,
+ },
};
const unsigned int cx23885_bcount = ARRAY_SIZE(cx23885_boards);
@@ -897,6 +902,10 @@ struct cx23885_subid cx23885_subids[] = {
.subvendor = 0x1461,
.subdevice = 0xd939,
.card = CX23885_BOARD_AVERMEDIA_HC81R,
+ }, {
+ .subvendor = 0x18ac,
+ .subdevice = 0xdb98,
+ .card = CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2,
},
};
const unsigned int cx23885_idcount = ARRAY_SIZE(cx23885_subids);
@@ -1137,6 +1146,7 @@ int cx23885_tuner_callback(void *priv, int component, int
command, int arg)
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_7_DUAL_EXP:
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
/* Two identical tuners on two different i2c buses,
* we need to reset the correct gpio. */
if (port-nr == 1)
@@ -1280,6 +1290,7 @@ void cx23885_gpio_setup(struct cx23885_dev *dev)
cx_set(GP0_IO, 0x000f000f);
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
/* GPIO-0 portb xc3028 reset */
/* GPIO-1 portb zl10353 reset */
/* GPIO-2 portc xc3028 reset */
@@ -1585,6 +1596,7 @@ int cx23885_ir_init(struct cx23885_dev *dev)
ir_rxtx_pin_cfg_count, ir_rxtx_pin_cfg);
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
request_module(ir-kbd-i2c);
break;
}
@@ -1720,6 +1732,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_7_DUAL_EXP:
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
ts2-gen_ctrl_val = 0xc; /* Serial bus + punctured clock */
ts2-ts_clk_en_val = 0x1; /* Enable TS_CLK */
ts2-src_sel_val = CX23885_SRC_SEL_PARALLEL_MPEG_VIDEO;
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c
b/drivers/media/pci/cx23885/cx23885-dvb.c
index d037459..5c627e6 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -44,6 +44,7 @@
#include tuner-xc2028.h
#include tuner-simple.h
#include dib7000p.h
+#include dib0070.h
#include dibx000_common.h
#include zl10353.h
#include stv0900.h
@@ -746,6 +747,104 @@ static int netup_altera_fpga_rw(void *device, int flag,
int data, int read)
return 0;
};
+static int dib7070_tuner_reset(struct dvb_frontend *fe, int onoff) {
+ struct dib7000p_ops *dib7000p_ops = fe-sec_priv;
+ return dib7000p_ops-set_gpio(fe, 8, 0, !onoff);
+}
+
+static int
+dib7070_tuner_sleep(struct dvb_frontend *fe, int onoff) {
+ return 0;
+}
+
+static struct dib0070_config dib7070p_dib0070_config = {
+ .i2c_address = DEFAULT_DIB0070_I2C_ADDRESS,
+ .reset = dib7070_tuner_reset,
+ .sleep = dib7070_tuner_sleep,
+ .clock_khz = 12000,
+ .freq_offset_khz_vhf = 950,
+ .freq_offset_khz_vhf = 550,
+ //.flip_chip = 1,
+};
+
+/* DIB7070 generic */
+static struct dibx000_agc_config dib7070_agc_config = {
+ .band_caps = BAND_UHF | BAND_VHF | BAND_LBAND | BAND_SBAND,
+
+ /*
+* P_agc_use_sd_mod1=0, P_agc_use_sd_mod2=0, P_agc_freq_pwm_div=5,
+* P_agc_inv_pwm1=0, P_agc_inv_pwm2=0, P_agc_inh_dc_rv_est=0,
+* P_agc_time_est=3, P_agc_freeze=0, P_agc_nb_est=5, P_agc_write=0
+*/
+ .setup = (0 15) | (0 14) | (5 11) | (0 10) | (0 9) |
+(0 8) | (3 5) | (0 4) | (5 1) | (0 0),
+ .inv_gain = 600,
+ .time_stabiliz = 10,
+ .alpha_level = 0,
+ .thlock = 118
RE: fusion hdtv dual express 2 (working, kind of)
One thing I just noticed, it's always the same pages in each buffer that is 'lost' (in find_next_packet). If I printk some details, the 'lost' parts are always some multiple of 4k in size (with allowance for 188 byte packet). printk(find_next_packet() lost %p %d-%d (%d)\n, buf, start, pos, lost); and then formatting the output through sort -u to eliminate duplicates produces output like: c90002966000 0-24064 (24064) c90002987000 0-4136 (4136) c9000299 0-4136 (4136) c9000299 20492-24064 (3572) c90002999000 0-4136 (4136) c900029a2000 0-4136 (4136) c900029ab000 12408-20492 (8084) c900029b4000 4136-12408 (8272) c900029bd000 12408-20492 (8084) c900029c6000 0-4136 (4136) c900029c6000 20492-24064 (3572) c900029cf000 20492-24064 (3572) c900029d8000 16544-20492 (3948) c900029d8000 4136-12408 (8272) c900029ea000 0-4136 (4136) so taking c900029d8000 as an example, the second and third pages have no data in them, and neither does the fifth page. This happens every time c900029d8000 is accessed. So I'm guessing that the sg list hasn't been programmed correctly or something... does that sound right? I guess I'll print out the pfn's to see if there is a pattern there next... James -- 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: fusion hdtv dual express 2 (working, kind of)
Turns out if I turn off xen (machine is running as a dom0 (xen host, not inside a vm)), it all works perfectly. I assume this issue isn't limited to my new dual express 2 card?? Any suggestions where to look? James -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of James Harper Sent: Sunday, 8 June 2014 8:36 PM To: René; linux-media@vger.kernel.org Subject: RE: fusion hdtv dual express 2 (working, kind of) One thing I just noticed, it's always the same pages in each buffer that is 'lost' (in find_next_packet). If I printk some details, the 'lost' parts are always some multiple of 4k in size (with allowance for 188 byte packet). printk(find_next_packet() lost %p %d-%d (%d)\n, buf, start, pos, lost); and then formatting the output through sort -u to eliminate duplicates produces output like: c90002966000 0-24064 (24064) c90002987000 0-4136 (4136) c9000299 0-4136 (4136) c9000299 20492-24064 (3572) c90002999000 0-4136 (4136) c900029a2000 0-4136 (4136) c900029ab000 12408-20492 (8084) c900029b4000 4136-12408 (8272) c900029bd000 12408-20492 (8084) c900029c6000 0-4136 (4136) c900029c6000 20492-24064 (3572) c900029cf000 20492-24064 (3572) c900029d8000 16544-20492 (3948) c900029d8000 4136-12408 (8272) c900029ea000 0-4136 (4136) so taking c900029d8000 as an example, the second and third pages have no data in them, and neither does the fifth page. This happens every time c900029d8000 is accessed. So I'm guessing that the sg list hasn't been programmed correctly or something... does that sound right? I guess I'll print out the pfn's to see if there is a pattern there next... James -- 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 -- 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
regression in dib0700
Somewhere along the way there's been a regression in dib0700 for my Leadtek
Winfast DTV Dongle (STK7700P based)
One is the addition of dvb_detach(state-dib7000p_ops);
The other is a missing .size_of_priv
The following is required to get it working again, although obviously
commenting out dvb_detach isn't really correct. dvb_detach looks like it is
supposed to take a function as an argument...
James
diff --git a/drivers/media/usb/dvb-usb/dib0700_devices.c
b/drivers/media/usb/dvb-usb/dib0700_devices.c
index d067bb7..4c80151 100644
--- a/drivers/media/usb/dvb-usb/dib0700_devices.c
+++ b/drivers/media/usb/dvb-usb/dib0700_devices.c
@@ -721,7 +721,7 @@ static int stk7700p_frontend_attach(struct dvb_usb_adapter
*adap)
adap-fe_adap[0].fe =
state-dib7000p_ops.init(adap-dev-i2c_adap, 18, stk7700p_dib7000p_config);
st-is_dib7000pc = 1;
} else {
- dvb_detach(state-dib7000p_ops);
+ //dvb_detach(state-dib7000p_ops);
memset(state-dib7000p_ops, 0, sizeof(state-dib7000p_ops));
adap-fe_adap[0].fe = dvb_attach(dib7000m_attach,
adap-dev-i2c_adap, 18, stk7700p_dib7000m_config);
}
@@ -3788,6 +3788,7 @@ struct dvb_usb_device_properties dib0700_devices[] = {
DIB0700_DEFAULT_STREAMING_CONFIG(0x02),
}},
+ .size_of_priv = sizeof(struct
dib0700_adapter_state),
},
},
--
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 support DViCO FusionHDTV DVB-T Dual Express2
The following patch implements support for the cx23885 based DViCO FusionHDTV
DVB-T Dual Express2. It uses the dib7070 chipset, which was already supported
by an existing usb adapter. Working on my pc but not extensively tested yet.
There is obviously a bit of code duplication due to the cut and paste from the
usb adapter, and i'm not sure about the use of sec_priv.
Any pointers for refinement?
James
diff --git a/drivers/media/pci/cx23885/cx23885-cards.c
b/drivers/media/pci/cx23885/cx23885-cards.c
index 79f20c8..9ca8855 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -649,7 +649,12 @@ struct cx23885_board cx23885_boards[] = {
CX25840_NONE1_CH3,
.amux = CX25840_AUDIO6,
} },
- }
+ },
+ [CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2] = {
+ .name = DViCO FusionHDTV DVB-T Dual Express2,
+ .portb = CX23885_MPEG_DVB,
+ .portc = CX23885_MPEG_DVB,
+ },
};
const unsigned int cx23885_bcount = ARRAY_SIZE(cx23885_boards);
@@ -897,6 +902,10 @@ struct cx23885_subid cx23885_subids[] = {
.subvendor = 0x1461,
.subdevice = 0xd939,
.card = CX23885_BOARD_AVERMEDIA_HC81R,
+ }, {
+ .subvendor = 0x18ac,
+ .subdevice = 0xdb98,
+ .card = CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2,
},
};
const unsigned int cx23885_idcount = ARRAY_SIZE(cx23885_subids);
@@ -1137,6 +1146,7 @@ int cx23885_tuner_callback(void *priv, int component, int
command, int arg)
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_7_DUAL_EXP:
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
/* Two identical tuners on two different i2c buses,
* we need to reset the correct gpio. */
if (port-nr == 1)
@@ -1280,6 +1290,7 @@ void cx23885_gpio_setup(struct cx23885_dev *dev)
cx_set(GP0_IO, 0x000f000f);
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
/* GPIO-0 portb xc3028 reset */
/* GPIO-1 portb zl10353 reset */
/* GPIO-2 portc xc3028 reset */
@@ -1585,6 +1596,7 @@ int cx23885_ir_init(struct cx23885_dev *dev)
ir_rxtx_pin_cfg_count, ir_rxtx_pin_cfg);
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
request_module(ir-kbd-i2c);
break;
}
@@ -1720,6 +1732,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_7_DUAL_EXP:
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
ts2-gen_ctrl_val = 0xc; /* Serial bus + punctured clock */
ts2-ts_clk_en_val = 0x1; /* Enable TS_CLK */
ts2-src_sel_val = CX23885_SRC_SEL_PARALLEL_MPEG_VIDEO;
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c
b/drivers/media/pci/cx23885/cx23885-dvb.c
index d037459..4b55303 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -44,6 +44,7 @@
#include tuner-xc2028.h
#include tuner-simple.h
#include dib7000p.h
+#include dib0070.h
#include dibx000_common.h
#include zl10353.h
#include stv0900.h
@@ -746,6 +747,106 @@ static int netup_altera_fpga_rw(void *device, int flag,
int data, int read)
return 0;
};
+static int dib7070_tuner_reset(struct dvb_frontend *fe, int onoff) {
+ struct dib7000p_ops *dib7000p_ops = fe-sec_priv;
+ return dib7000p_ops-set_gpio(fe, 8, 0, !onoff);
+}
+
+static int
+dib7070_tuner_sleep(struct dvb_frontend *fe, int onoff) {
+ return 0;
+}
+
+static struct dib0070_config dib7070p_dib0070_config = {
+ .i2c_address = DEFAULT_DIB0070_I2C_ADDRESS,
+ .reset = dib7070_tuner_reset,
+ .sleep = dib7070_tuner_sleep,
+ .clock_khz = 12000,
+ .freq_offset_khz_vhf = 950,
+ .freq_offset_khz_vhf = 550,
+ //.clock_pad_drive = 0,
+ .flip_chip = 1,
+ //.charge_pump = 2,
+};
+
+/* DIB7070 generic */
+static struct dibx000_agc_config dib7070_agc_config = {
+ .band_caps = BAND_UHF | BAND_VHF | BAND_LBAND | BAND_SBAND,
+
+ /*
+* P_agc_use_sd_mod1=0, P_agc_use_sd_mod2=0, P_agc_freq_pwm_div=5,
+* P_agc_inv_pwm1=0, P_agc_inv_pwm2=0, P_agc_inh_dc_rv_est=0,
+* P_agc_time_est=3, P_agc_freeze=0, P_agc_nb_est=5, P_agc_write=0
+*/
+ .setup = (0 15) | (0 14) | (5 11) | (0 10) | (0 9) |
+(0 8) | (3 5) | (0 4) | (5 1) | (0
[PATCH] Fix regression in some dib0700 based devices.
Fix regression in some dib0700 based devices.
Set size_of_priv, and don't call dvb_detach unnecessarily.
This resolves the oops(s) for my Leadtek Winfast DTV Dongle (STK7700P based)
Signed-off-by: James Harper james.har...@ejbdigital.com.au
---
drivers/media/usb/dvb-usb/dib0700_devices.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/usb/dvb-usb/dib0700_devices.c
b/drivers/media/usb/dvb-usb/dib0700_devices.c
index d067bb7..25355fa 100644
--- a/drivers/media/usb/dvb-usb/dib0700_devices.c
+++ b/drivers/media/usb/dvb-usb/dib0700_devices.c
@@ -721,7 +721,6 @@ static int stk7700p_frontend_attach(struct dvb_usb_adapter
*adap)
adap-fe_adap[0].fe =
state-dib7000p_ops.init(adap-dev-i2c_adap, 18, stk7700p_dib7000p_config);
st-is_dib7000pc = 1;
} else {
- dvb_detach(state-dib7000p_ops);
memset(state-dib7000p_ops, 0, sizeof(state-dib7000p_ops));
adap-fe_adap[0].fe = dvb_attach(dib7000m_attach,
adap-dev-i2c_adap, 18, stk7700p_dib7000m_config);
}
@@ -3788,6 +3787,7 @@ struct dvb_usb_device_properties dib0700_devices[] = {
DIB0700_DEFAULT_STREAMING_CONFIG(0x02),
}},
+ .size_of_priv = sizeof(struct
dib0700_adapter_state),
},
},
--
2.0.0
--
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: fusion hdtv dual express 2 (working, kind of)
Hi, I don't use mythtv myself so I can't help you troubleshooting using this program. But the symptoms you describe (high pitch sound, degraded video) make me think of missing packets in the received stream. May be you might use vlc in maximum verbosity mode to get information about the stream as it is really received by the device. I suggest you store the output of stderr in a file to examine it afterwards because your system my have difficulties to cope with the output rate. Given your tuning parameters the command line shall look like: vlc -vvv -I dummy dvb://frequency=55050 --dvb-adapter=2 --dvb-bandwidth=7 --dvb- transmission=8 --dvb-guard=1/16 --dvb-code-rate-hp=3/4 --no-video --no-audio 2 dttv.log Wait about one minute, ^C and watch the log. You may, before issue grep discontinuity ddtv.log to see if there are a lot of missing packets. However, continuity counters are modulo 16 so is the number of missing packets ... [0x7f239c003c68] ts demux warning: discontinuity received 0x2 instead of 0x1 (pid=2841) [0x7f239c003c68] ts demux warning: discontinuity received 0x3 instead of 0x2 (pid=2840) [0x7f239c003c68] ts demux warning: discontinuity received 0xc instead of 0xb (pid=2845) [0x7f239c003c68] ts demux error: libdvbpsi (PSI decoder): TS discontinuity (received 4, expected 3) for PID 18 [0x7f239c003c68] ts demux warning: discontinuity received 0x4 instead of 0x3 (pid=2841) [0x7f239c003c68] ts demux warning: discontinuity received 0xe instead of 0x5 (pid=2840) [0x7f239c003c68] ts demux warning: discontinuity received 0x6 instead of 0x2 (pid=2840) [0x7f239c003c68] ts demux error: libdvbpsi (PSI decoder): TS discontinuity (received 8, expected 6) for PID 18 [0x7f239c003c68] ts demux warning: discontinuity received 0x8 instead of 0x7 (pid=2841) [0x7f239c003c68] ts demux warning: discontinuity received 0x5 instead of 0x8 (pid=2840) [0x7f239c003c68] ts demux warning: discontinuity received 0xa instead of 0x9 (pid=2841) Looks like a problem! So if I'm getting signal around 5, Verror=0, and BlockError=0 then would I be right in thinking the tuner and everything like that is hooked up correctly, and that my problem is not related to signal quality but to something a bit further along? Next I guess I'll crank up the debug in the kernel modules. I'll probably figure out something on my own but if you had any tips on which would be best to enable it would be much appreciated! PS: I am not a developer but I can help you with dvb if you have specific questions in that domain, just reply to me, I'll do my best to help. Your help so far is greatly appreciated! Thanks James -- 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
fusion hdtv dual express 2
After confirming that it was supported I just bought a fusion hdtv dual express PCI adapter, only to find that I'd bought the 'dual express 2' version, which isn't supported (not the first time I've made such a mistake). This page http://www.linuxtv.org/wiki/index.php/DViCO_FusionHDTV_DVB-T_Dual_Express2 says that the new v2 card is still using CX23885 chipset but also uses DIB7070 (dib7000 + dib0070) which appears to be supported already but only via a few USB adapters. With a bit of cut and paste I have added some support for the card, to the point that dvbtune now says: # dvbtune -f 55050 -bw 7 -c 2 -cr 3_4 -gi 16 -tm 8 -I 1 -m -i Using DVB card DiBcom 7000PC tuning DVB-T (in United Kingdom) to 55050 Hz polling Getting frontend event FE_STATUS: polling Getting frontend event FE_STATUS: FE_HAS_SIGNAL FE_HAS_LOCK FE_HAS_CARRIER FE_HAS_VITERBI FE_HAS_SYNC Bit error rate: 0 Signal strength: 49390 SNR: 233 FE_STATUS: FE_HAS_SIGNAL FE_HAS_LOCK FE_HAS_CARRIER FE_HAS_VITERBI FE_HAS_SYNC transponder type=T freq=55050 Nothing to read from fd_pat Nothing to read from fd_sdt /transponder Signal=49402, Verror=0, SNR=228dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49397, Verror=0, SNR=226dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49377, Verror=0, SNR=235dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49336, Verror=0, SNR=230dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49345, Verror=0, SNR=235dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49374, Verror=0, SNR=232dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49352, Verror=0, SNR=254dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49396, Verror=0, SNR=241dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49344, Verror=0, SNR=243dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49353, Verror=0, SNR=258dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49309, Verror=0, SNR=248dB, BlockErrors=0, (S|L|C|V|SY|) Signal=49334, Verror=0, SNR=243dB, BlockErrors=0, (S|L|C|V|SY|) ^C Which all looks correct at the start, but obviously not as it can't actually read the information in the dvb streams. Is it likely that getting it working only requires a small amount of further tinkering, or am I likely wasting my time? I know next to nothing about dvb and how it all hangs together. Thanks James -- 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: fusion hdtv dual express 2
Hi James, The first basic thing you should look at is if the dvb device has got all its pieces. A dvb adapter has, sort of, 4 sub-devices: [me@home ~]$ ll /dev/dvb/adapter2 total 0 crw-rw+ 1 root video 212, 12 Jun 5 12:31 demux0 crw-rw+ 1 root video 212, 13 Jun 5 12:31 dvr0 crw-rw+ 1 root video 212, 15 Jun 5 12:31 frontend0 crw-rw+ 1 root video 212, 14 Jun 5 12:31 net0 From your post you might miss the demux while the front-end is working. ls -al /dev/dvb/adapter1/ total 0 drwxr-xr-x 2 root root 120 Jun 7 10:08 . drwxr-xr-x 5 root root 100 Jun 7 10:08 .. crw-rw---T 1 root video 212, 5 Jun 7 10:08 demux0 crw-rw---T 1 root video 212, 6 Jun 7 10:08 dvr0 crw-rw---T 1 root video 212, 4 Jun 7 10:08 frontend0 crw-rw---T 1 root video 212, 7 Jun 7 10:08 net0 same for adapter2 (adapter0 is an existing usb dvb-t adapter) I think all the pieces are there, they just aren't connected up internally correctly. If I deliberately put in a wrong i2c address for the tuner (starts of at 0x12 and is reprogrammed to 0x80 in the usb version, which is what I've copied) then I get an error, so it can definitely see the tuner. And if I tune an incorrect frequency I never get lock or anything so I think that much is working. And my original post shows it can see a stream with no errors, and if I put the incorrect code rate in (eg 2_3 instead of 3_4) then it sees lots of errors. So suppose that the demux is what's wrong, how could I debug that further? Is there a block diagram somewhere that explains how the various dvb components feed into each other? Thanks James -- 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: fusion hdtv dual express 2 (working, kind of)
OK I have picture in mythtv now, but it's very glitchy (lines in video, bursts
of high pitched tone in audio). In fact it is behaving much like the dib0700
based adapter that I replaced with the express2 adapter because I thought it
had died. Could there been a regression somewhere? I'll check the git archives
but I don't know when it broke - somewhere between kernel 3.2.x and 3.14.x.
Alternatively the dib0700 adapter could actually be dead and maybe this is
something different...
The change I made was to set the output mode to OUTMODE_MPEG2_SERIAL after
copying the code from the USB dib7070 based adapter.
The Leadtek Research, Inc. WinFast DTV Dongle Gold (0413:6029) that I am
still using works fine so I don't think it's a physical reception problem
unless the signal has gotten suddenly really poor.
Any suggestions?
My current patch follows this email. It's a bit messy because the dib7070 stuff
is tied fairly closely to the usb code so there is a bit of duplication. And
there is some state required and I don't know that I've put it in the right
place (sec_priv).
Thanks
James
diff --git a/drivers/media/dvb-frontends/dib7000p.h
b/drivers/media/dvb-frontends/dib7000p.h
index 1fea0e9..4d1dbca 100644
--- a/drivers/media/dvb-frontends/dib7000p.h
+++ b/drivers/media/dvb-frontends/dib7000p.h
@@ -64,6 +64,7 @@ struct dib7000p_ops {
int (*get_adc_power)(struct dvb_frontend *fe);
int (*slave_reset)(struct dvb_frontend *fe);
struct dvb_frontend *(*init)(struct i2c_adapter *i2c_adap, u8 i2c_addr,
struct dib7000p_config *cfg);
+ int (*set_param_save)(struct dvb_frontend *fe);
};
#if IS_ENABLED(CONFIG_DVB_DIB7000P)
diff --git a/drivers/media/pci/cx23885/cx23885-cards.c
b/drivers/media/pci/cx23885/cx23885-cards.c
index 79f20c8..9ca8855 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -649,7 +649,12 @@ struct cx23885_board cx23885_boards[] = {
CX25840_NONE1_CH3,
.amux = CX25840_AUDIO6,
} },
- }
+ },
+ [CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2] = {
+ .name = DViCO FusionHDTV DVB-T Dual Express2,
+ .portb = CX23885_MPEG_DVB,
+ .portc = CX23885_MPEG_DVB,
+ },
};
const unsigned int cx23885_bcount = ARRAY_SIZE(cx23885_boards);
@@ -897,6 +902,10 @@ struct cx23885_subid cx23885_subids[] = {
.subvendor = 0x1461,
.subdevice = 0xd939,
.card = CX23885_BOARD_AVERMEDIA_HC81R,
+ }, {
+ .subvendor = 0x18ac,
+ .subdevice = 0xdb98,
+ .card = CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2,
},
};
const unsigned int cx23885_idcount = ARRAY_SIZE(cx23885_subids);
@@ -1137,6 +1146,7 @@ int cx23885_tuner_callback(void *priv, int component, int
command, int arg)
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_7_DUAL_EXP:
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
/* Two identical tuners on two different i2c buses,
* we need to reset the correct gpio. */
if (port-nr == 1)
@@ -1280,6 +1290,7 @@ void cx23885_gpio_setup(struct cx23885_dev *dev)
cx_set(GP0_IO, 0x000f000f);
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
/* GPIO-0 portb xc3028 reset */
/* GPIO-1 portb zl10353 reset */
/* GPIO-2 portc xc3028 reset */
@@ -1585,6 +1596,7 @@ int cx23885_ir_init(struct cx23885_dev *dev)
ir_rxtx_pin_cfg_count, ir_rxtx_pin_cfg);
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
request_module(ir-kbd-i2c);
break;
}
@@ -1720,6 +1732,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
break;
case CX23885_BOARD_DVICO_FUSIONHDTV_7_DUAL_EXP:
case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
+ case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP2:
ts2-gen_ctrl_val = 0xc; /* Serial bus + punctured clock */
ts2-ts_clk_en_val = 0x1; /* Enable TS_CLK */
ts2-src_sel_val = CX23885_SRC_SEL_PARALLEL_MPEG_VIDEO;
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c
b/drivers/media/pci/cx23885/cx23885-dvb.c
index d037459..89d6c54 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -44,6 +44,7 @@
#include tuner-xc2028.h
#include tuner-simple.h
#include dib7000p.h
+#include dib0070.h
#include dibx000_common.h
#include zl10353.h
#include stv0900.h
@@ -746,6 +747,119
Re: [PATCH v6 2/3] ARM: sunxi: Add driver for sunxi IR controller
Hi Alexander,
Just a few probe error handling suggestions...
On 13/05/14 19:39, Alexander Bersenev wrote:
+static int sunxi_ir_probe(struct platform_device *pdev)
+{
+ int ret = 0;
+ unsigned long tmp = 0;
+
+ struct device *dev = pdev-dev;
+ struct device_node *dn = dev-of_node;
+ struct resource *res;
+ struct sunxi_ir *ir;
+
+ ir = devm_kzalloc(dev, sizeof(struct sunxi_ir), GFP_KERNEL);
+ if (!ir)
+ return -ENOMEM;
+
+ /* Clock */
+ ir-apb_clk = devm_clk_get(dev, apb);
+ if (IS_ERR(ir-apb_clk)) {
+ dev_err(dev, failed to get a apb clock.\n);
+ return -EINVAL;
Does it make sense to return PTR_ERR(ir-apb_clk) here?
+ }
+ ir-clk = devm_clk_get(dev, ir);
+ if (IS_ERR(ir-clk)) {
+ dev_err(dev, failed to get a ir clock.\n);
+ return -EINVAL;
and here
+ }
+
+ ret = clk_set_rate(ir-clk, SUNXI_IR_BASE_CLK);
+ if (ret) {
+ dev_err(dev, set ir base clock failed!\n);
+ return -EINVAL;
return ret?
+ }
+
+ if (clk_prepare_enable(ir-apb_clk)) {
+ dev_err(dev, try to enable apb_ir_clk failed\n);
+ return -EINVAL;
+ }
+
+ if (clk_prepare_enable(ir-clk)) {
+ dev_err(dev, try to enable ir_clk failed\n);
+ ret = -EINVAL;
+ goto exit_clkdisable_apb_clk;
+ }
+
+ /* IO */
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+ ir-base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(ir-base)) {
+ dev_err(dev, failed to map registers\n);
+ ret = -ENOMEM;
PTR_ERR again?
+ goto exit_clkdisable_clk;
+ }
+
+ /* IRQ */
+ ir-irq = platform_get_irq(pdev, 0);
+ if (ir-irq 0) {
+ dev_err(dev, no irq resource\n);
+ ret = -EINVAL;
ret = ir-irq?
+ goto exit_clkdisable_clk;
+ }
+
+ ret = devm_request_irq(dev, ir-irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);
+ if (ret) {
+ dev_err(dev, failed request irq\n);
+ ret = -EINVAL;
necessary?
+ goto exit_clkdisable_clk;
+ }
+
+ ir-rc = rc_allocate_device();
+
+ if (!ir-rc) {
+ dev_err(dev, failed to allocate device\n);
+ ret = -ENOMEM;
+ goto exit_clkdisable_clk;
+ }
+
+ ir-rc-priv = ir;
+ ir-rc-input_name = SUNXI_IR_DEV;
+ ir-rc-input_phys = sunxi-ir/input0;
+ ir-rc-input_id.bustype = BUS_HOST;
+ ir-rc-input_id.vendor = 0x0001;
+ ir-rc-input_id.product = 0x0001;
+ ir-rc-input_id.version = 0x0100;
+ ir-map_name = of_get_property(dn, linux,rc-map-name, NULL);
+ ir-rc-map_name = ir-map_name ?: RC_MAP_EMPTY;
+ ir-rc-dev.parent = dev;
+ ir-rc-driver_type = RC_DRIVER_IR_RAW;
+ rc_set_allowed_protocols(ir-rc, RC_BIT_ALL);
+ ir-rc-rx_resolution = SUNXI_IR_SAMPLE;
+ ir-rc-timeout = MS_TO_NS(SUNXI_IR_TIMEOUT);
+ ir-rc-driver_name = SUNXI_IR_DEV;
+
+ ret = rc_register_device(ir-rc);
+ if (ret) {
+ dev_err(dev, failed to register rc device\n);
+ ret = -EINVAL;
same again
+ goto exit_free_dev;
+ }
+
Cheers
James
--
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 41/49] rc-core: rename mutex
On Friday 04 April 2014 01:34:43 David Härdeman wrote:
Having a mutex named lock is a bit misleading.
Why? A mutex is a type of lock so what's the problem?
A little grep'ing and sed'ing reveals that out of the 1578 unique mutex names
in the kernel source I have to hand, 540 contain lock, and 921 contain
mutex.
Cheers
James
Signed-off-by: David Härdeman da...@hardeman.nu
---
drivers/media/rc/img-ir/img-ir-hw.c |4 ++-
drivers/media/rc/rc-main.c | 42
++- include/media/rc-core.h |
5 ++--
3 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c index 5bc7903..a9abbb4 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -666,11 +666,11 @@ static void img_ir_set_protocol(struct img_ir_priv
*priv, u64 proto) {
struct rc_dev *rdev = priv-hw.rdev;
- mutex_lock(rdev-lock);
+ mutex_lock(rdev-mutex);
rdev-enabled_protocols = proto;
rdev-allowed_wakeup_protocols = proto;
rdev-enabled_wakeup_protocols = proto;
- mutex_unlock(rdev-lock);
+ mutex_unlock(rdev-mutex);
}
/* Set up IR decoders */
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 7caca4f..bd4dfab 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -109,7 +109,7 @@ int rc_open(struct rc_dev *dev)
{
int err = 0;
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
if (dev-dead)
err = -ENODEV;
@@ -119,7 +119,7 @@ int rc_open(struct rc_dev *dev)
dev-users--;
}
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
return err;
}
@@ -127,12 +127,12 @@ EXPORT_SYMBOL_GPL(rc_open);
void rc_close(struct rc_dev *dev)
{
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
if (!dev-dead !--dev-users dev-close)
dev-close(dev);
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
}
EXPORT_SYMBOL_GPL(rc_close);
@@ -322,7 +322,7 @@ struct rc_filter_attribute {
* It returns the protocol names of supported protocols.
* Enabled protocols are printed in brackets.
*
- * dev-lock is taken to guard against races between store_protocols and
+ * dev-mutex is taken to guard against races between store_protocols and
* show_protocols.
*/
static ssize_t show_protocols(struct device *device,
@@ -339,7 +339,7 @@ static ssize_t show_protocols(struct device *device,
return -EINVAL;
rc_event(dev, RC_KEY, RC_KEY_REPEAT, 1);
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
if (fattr-type == RC_FILTER_NORMAL) {
enabled = dev-enabled_protocols;
@@ -349,7 +349,7 @@ static ssize_t show_protocols(struct device *device,
allowed = dev-allowed_wakeup_protocols;
}
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
IR_dprintk(1, %s: allowed - 0x%llx, enabled - 0x%llx\n,
__func__, (long long)allowed, (long long)enabled);
@@ -449,7 +449,7 @@ static int parse_protocol_change(u64 *protocols, const
char *buf) * See parse_protocol_change() for the valid commands.
* Returns @len on success or a negative error code.
*
- * dev-lock is taken to guard against races between store_protocols and
+ * dev-mutex is taken to guard against races between store_protocols and
* show_protocols.
*/
static ssize_t store_protocols(struct device *device,
@@ -488,7 +488,7 @@ static ssize_t store_protocols(struct device *device,
return -EINVAL;
}
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
old_protocols = *current_protocols;
new_protocols = old_protocols;
@@ -532,7 +532,7 @@ static ssize_t store_protocols(struct device *device,
rc = len;
out:
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
return rc;
}
@@ -550,7 +550,7 @@ out:
* Bits of the filter value corresponding to set bits in the filter mask
are * compared against input scancodes and non-matching scancodes are
discarded. *
- * dev-lock is taken to guard against races between store_filter and
+ * dev-mutex is taken to guard against races between store_filter and
* show_filter.
*/
static ssize_t show_filter(struct device *device,
@@ -571,12 +571,12 @@ static ssize_t show_filter(struct device *device,
else
filter = dev-scancode_wakeup_filter;
- mutex_lock(dev-lock);
+ mutex_lock(dev-mutex);
if (fattr-mask)
val = filter-mask;
else
val = filter-data;
- mutex_unlock(dev-lock);
+ mutex_unlock(dev-mutex);
return sprintf(buf, %#x\n, val);
}
@@ -597,7 +597,7 @@ static ssize_t show_filter(struct device *device,
* Bits of the filter value corresponding to set
Re: [PATCH 05/49] rc-core: split dev-s_filter
Hi David,
On 4 April 2014 00:31, David Härdeman da...@hardeman.nu wrote:
Overloading dev-s_filter to do two different functions (set wakeup filters
and generic hardware filters) makes it impossible to tell what the
hardware actually supports, so create a separate dev-s_wakeup_filter and
make the distinction explicit.
Signed-off-by: David Härdeman da...@hardeman.nu
---
drivers/media/rc/img-ir/img-ir-hw.c | 15 ++-
I think we crossed emails. My comments on your earlier submission of
this patch about the removal of generic scancode filter code that has
crept in to the patch apply here too (sorry I didn't spot that when I
first looked at it!).
But if you fix that you're welcome to my:
Acked-by: James Hogan james.ho...@imgtec.com
Cheers
James
drivers/media/rc/rc-main.c | 31 +++
include/media/rc-core.h |6 --
3 files changed, 37 insertions(+), 15 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c
index aec79f7..871a9b3 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -504,6 +504,18 @@ unlock:
return ret;
}
+static int img_ir_set_normal_filter(struct rc_dev *dev,
+ struct rc_scancode_filter *sc_filter)
+{
+ return img_ir_set_filter(dev, RC_FILTER_NORMAL, sc_filter);
+}
+
+static int img_ir_set_wakeup_filter(struct rc_dev *dev,
+ struct rc_scancode_filter *sc_filter)
+{
+ return img_ir_set_filter(dev, RC_FILTER_WAKEUP, sc_filter);
+}
+
/**
* img_ir_set_decoder() - Set the current decoder.
* @priv: IR private data.
@@ -988,7 +1000,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
rdev-map_name = RC_MAP_EMPTY;
rc_set_allowed_protocols(rdev, img_ir_allowed_protos(priv));
rdev-input_name = IMG Infrared Decoder;
- rdev-s_filter = img_ir_set_filter;
+ rdev-s_filter = img_ir_set_normal_filter;
+ rdev-s_wakeup_filter = img_ir_set_wakeup_filter;
/* Register hardware decoder */
error = rc_register_device(rdev);
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index c0bfd50..ba955ac 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -929,6 +929,7 @@ static ssize_t store_protocols(struct device *device,
int rc, i, count = 0;
ssize_t ret;
int (*change_protocol)(struct rc_dev *dev, u64 *rc_type);
+ int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter
*filter);
struct rc_scancode_filter local_filter, *filter;
/* Device is being removed */
@@ -1013,24 +1014,27 @@ static ssize_t store_protocols(struct device *device,
* Fall back to clearing the filter.
*/
filter = dev-scancode_filters[fattr-type];
+ set_filter = (fattr-type == RC_FILTER_NORMAL)
+ ? dev-s_filter : dev-s_wakeup_filter;
+
if (old_type != type filter-mask) {
local_filter = *filter;
if (!type) {
/* no protocol = clear filter */
ret = -1;
- } else if (!dev-s_filter) {
+ } else if (!set_filter) {
/* generic filtering = accept any filter */
ret = 0;
} else {
/* hardware filtering = try setting, otherwise clear
*/
- ret = dev-s_filter(dev, fattr-type, local_filter);
+ ret = set_filter(dev, local_filter);
}
if (ret 0) {
/* clear the filter */
local_filter.data = 0;
local_filter.mask = 0;
- if (dev-s_filter)
- dev-s_filter(dev, fattr-type,
local_filter);
+ if (set_filter)
+ set_filter(dev, local_filter);
}
/* commit the new filter */
@@ -1112,6 +1116,7 @@ static ssize_t store_filter(struct device *device,
struct rc_scancode_filter local_filter, *filter;
int ret;
unsigned long val;
+ int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter
*filter);
/* Device is being removed */
if (!dev)
@@ -1121,9 +1126,11 @@ static ssize_t store_filter(struct device *device,
if (ret 0)
return ret;
- /* Scancode filter not supported (but still accept 0) */
- if (!dev-s_filter fattr-type != RC_FILTER_NORMAL)
- return val ? -EINVAL : count;
+ /* Can the scancode filter be set? */
+ set_filter = (fattr-type == RC_FILTER_NORMAL)
+ ? dev-s_filter : dev-s_wakeup_filter;
+ if (!set_filter
Re: [PATCH 04/49] rc-core: do not change 32bit NEC scancode format for now
Hi David,
On 4 April 2014 00:31, David Härdeman da...@hardeman.nu wrote:
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
b/drivers/media/rc/img-ir/img-ir-nec.c
index c0111d6..ee45795 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
/* Convert NEC data to a scancode */
static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
@@ -23,11 +24,11 @@ static int img_ir_nec_scancode(int len, u64 raw, enum
rc_type *protocol,
data_inv = (raw 24) 0xff;
if ((data_inv ^ data) != 0xff) {
/* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- *scancode = addr_inv 24 |
- addr 16 |
- data_inv 8 |
- data;
+ /* scan encoding: AAaaDDdd (LSBit first) */
This scan encoding of NEC32 interprets the raw data MSBit first (i.e.
the MSBit of scancode is the first bit received), so this comment is
wrong.
+ *scancode = bitrev8(addr) 24 |
+ bitrev8(addr_inv) 16 |
+ bitrev8(data) 8 |
+ bitrev8(data_inv);
} else if ((addr_inv ^ addr) != 0xff) {
/* Extended NEC */
/* scan encoding: AAaaDD */
@@ -56,13 +57,15 @@ static int img_ir_nec_filter(const struct
rc_scancode_filter *in,
if ((in-data | in-mask) 0xff00) {
/* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- addr_inv = (in-data 24) 0xff;
- addr_inv_m = (in-mask 24) 0xff;
- addr = (in-data 16) 0xff;
- addr_m = (in-mask 16) 0xff;
- data_inv = (in-data 8) 0xff;
- data_inv_m = (in-mask 8) 0xff;
+ /* scan encoding: AAaaDDdd (LSBit first) */
same here
The actual code looks fine now though. If you fix those two comments:
Acked-by: James Hogan james.ho...@imgtec.com
Cheers
James
+ addr = bitrev8(in-data 24);
+ addr_m = bitrev8(in-mask 24);
+ addr_inv = bitrev8(in-data 16);
+ addr_inv_m = bitrev8(in-mask 16);
+ data = bitrev8(in-data 8);
+ data_m = bitrev8(in-mask 8);
+ data_inv = bitrev8(in-data 0);
+ data_inv_m = bitrev8(in-mask 0);
} else if ((in-data | in-mask) 0x00ff) {
/* Extended NEC */
/* scan encoding AAaaDD */
--
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 06/49] rc-core: remove generic scancode filter
Hi David,
On 4 April 2014 00:31, David Härdeman da...@hardeman.nu wrote:
The generic scancode filtering has questionable value and makes it
impossible to determine from userspace if there is an actual
scancode hw filter present or not.
So revert the generic parts.
I've already mentioned in a different email that reverting the last
two hunks of b8c7d915087c97a21fa415fa0e860e59739da202 should be in
this patch rather than combined into patch 5.
But if you fix that you're welcome to my:
Reviewed-by: James Hogan james.ho...@imgtec.com
Thanks
James
Based on a patch from James Hogan james.ho...@imgtec.com, but this
version also makes sure that only the valid sysfs files are created
in the first place.
v2: correct dev-s_filter check
Signed-off-by: David Härdeman da...@hardeman.nu
---
drivers/media/rc/rc-main.c | 67
+---
include/media/rc-core.h|2 +
2 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index ba955ac..26c266b 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -634,7 +634,6 @@ EXPORT_SYMBOL_GPL(rc_repeat);
static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
u32 scancode, u32 keycode, u8 toggle)
{
- struct rc_scancode_filter *filter;
bool new_event = (!dev-keypressed ||
dev-last_protocol != protocol ||
dev-last_scancode != scancode ||
@@ -643,11 +642,6 @@ static void ir_do_keydown(struct rc_dev *dev, enum
rc_type protocol,
if (new_event dev-keypressed)
ir_do_keyup(dev, false);
- /* Generic scancode filtering */
- filter = dev-scancode_filters[RC_FILTER_NORMAL];
- if (filter-mask ((scancode ^ filter-data) filter-mask))
- return;
-
input_event(dev-input_dev, EV_MSC, MSC_SCAN, scancode);
if (new_event keycode != KEY_RESERVED) {
@@ -1017,14 +1011,11 @@ static ssize_t store_protocols(struct device *device,
set_filter = (fattr-type == RC_FILTER_NORMAL)
? dev-s_filter : dev-s_wakeup_filter;
- if (old_type != type filter-mask) {
+ if (set_filter old_type != type filter-mask) {
local_filter = *filter;
if (!type) {
/* no protocol = clear filter */
ret = -1;
- } else if (!set_filter) {
- /* generic filtering = accept any filter */
- ret = 0;
} else {
/* hardware filtering = try setting, otherwise clear
*/
ret = set_filter(dev, local_filter);
@@ -1033,8 +1024,7 @@ static ssize_t store_protocols(struct device *device,
/* clear the filter */
local_filter.data = 0;
local_filter.mask = 0;
- if (set_filter)
- set_filter(dev, local_filter);
+ set_filter(dev, local_filter);
}
/* commit the new filter */
@@ -1078,7 +1068,10 @@ static ssize_t show_filter(struct device *device,
return -EINVAL;
mutex_lock(dev-lock);
- if (fattr-mask)
+ if ((fattr-type == RC_FILTER_NORMAL !dev-s_filter) ||
+ (fattr-type == RC_FILTER_WAKEUP !dev-s_wakeup_filter))
+ val = 0;
+ else if (fattr-mask)
val = dev-scancode_filters[fattr-type].mask;
else
val = dev-scancode_filters[fattr-type].data;
@@ -1202,27 +1195,45 @@ static RC_FILTER_ATTR(wakeup_filter, S_IRUGO|S_IWUSR,
static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR,
show_filter, store_filter, RC_FILTER_WAKEUP, true);
-static struct attribute *rc_dev_attrs[] = {
+static struct attribute *rc_dev_protocol_attrs[] = {
dev_attr_protocols.attr.attr,
+ NULL,
+};
+
+static struct attribute_group rc_dev_protocol_attr_grp = {
+ .attrs = rc_dev_protocol_attrs,
+};
+
+static struct attribute *rc_dev_wakeup_protocol_attrs[] = {
dev_attr_wakeup_protocols.attr.attr,
+ NULL,
+};
+
+static struct attribute_group rc_dev_wakeup_protocol_attr_grp = {
+ .attrs = rc_dev_wakeup_protocol_attrs,
+};
+
+static struct attribute *rc_dev_filter_attrs[] = {
dev_attr_filter.attr.attr,
dev_attr_filter_mask.attr.attr,
- dev_attr_wakeup_filter.attr.attr,
- dev_attr_wakeup_filter_mask.attr.attr,
NULL,
};
-static struct attribute_group rc_dev_attr_grp = {
- .attrs = rc_dev_attrs,
+static struct attribute_group rc_dev_filter_attr_grp = {
+ .attrs = rc_dev_filter_attrs,
+};
+
+static struct attribute
Re: [PATCH 1/3] rc-core: do not change 32bit NEC scancode format for now
On Saturday 05 April 2014 00:05:56 David Härdeman wrote:
This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2
The patch ignores the fact that NEC32 scancodes are generated not only in
the NEC raw decoder but also directly in some drivers. Whichever approach
is chosen it should be consistent across drivers and this patch needs more
discussion.
Furthermore, I'm convinced that we have to stop playing games trying to
decipher the meaning of NEC scancodes (what's the customer/vendor/address,
which byte is the MSB, etc).
This patch is in preparation for the next few patches in this series.
v2: make sure img-ir scancodes are bitrev8():ed as well
v3: update comments
Signed-off-by: David Härdeman da...@hardeman.nu
Acked-by: James Hogan james.ho...@imgtec.com
Thanks
James
---
drivers/media/rc/img-ir/img-ir-nec.c | 27 ++-
drivers/media/rc/ir-nec-decoder.c|5 --
drivers/media/rc/keymaps/rc-tivo.c | 86
+- 3 files changed, 59 insertions(+), 59
deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
b/drivers/media/rc/img-ir/img-ir-nec.c index e7a731b..751d9d9 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -5,6 +5,7 @@
*/
#include img-ir-hw.h
+#include linux/bitrev.h
/* Convert NEC data to a scancode */
static int img_ir_nec_scancode(int len, u64 raw, int *scancode, u64
protocols) @@ -22,11 +23,11 @@ static int img_ir_nec_scancode(int len, u64
raw, int *scancode, u64 protocols) data_inv = (raw 24) 0xff;
if ((data_inv ^ data) != 0xff) {
/* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- *scancode = addr_inv 24 |
- addr 16 |
- data_inv 8 |
- data;
+ /* scan encoding: as transmitted, MSBit = first received bit */
+ *scancode = bitrev8(addr) 24 |
+ bitrev8(addr_inv) 16 |
+ bitrev8(data) 8 |
+ bitrev8(data_inv);
} else if ((addr_inv ^ addr) != 0xff) {
/* Extended NEC */
/* scan encoding: AAaaDD */
@@ -54,13 +55,15 @@ static int img_ir_nec_filter(const struct
rc_scancode_filter *in,
if ((in-data | in-mask) 0xff00) {
/* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- addr_inv = (in-data 24) 0xff;
- addr_inv_m = (in-mask 24) 0xff;
- addr = (in-data 16) 0xff;
- addr_m = (in-mask 16) 0xff;
- data_inv = (in-data 8) 0xff;
- data_inv_m = (in-mask 8) 0xff;
+ /* scan encoding: as transmitted, MSBit = first received bit */
+ addr = bitrev8(in-data 24);
+ addr_m = bitrev8(in-mask 24);
+ addr_inv = bitrev8(in-data 16);
+ addr_inv_m = bitrev8(in-mask 16);
+ data = bitrev8(in-data 8);
+ data_m = bitrev8(in-mask 8);
+ data_inv = bitrev8(in-data 0);
+ data_inv_m = bitrev8(in-mask 0);
} else if ((in-data | in-mask) 0x00ff) {
/* Extended NEC */
/* scan encoding AAaaDD */
diff --git a/drivers/media/rc/ir-nec-decoder.c
b/drivers/media/rc/ir-nec-decoder.c index 9de1791..35c42e5 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -172,10 +172,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct
ir_raw_event ev) if (send_32bits) {
/* NEC transport, but modified protocol, used by at
* least Apple and TiVo remotes */
- scancode = not_address 24 |
-address 16 |
-not_command 8 |
-command;
+ scancode = data-bits;
IR_dprintk(1, NEC (modified) scancode 0x%08x\n,
scancode);
} else if ((address ^ not_address) != 0xff) {
/* Extended NEC */
diff --git a/drivers/media/rc/keymaps/rc-tivo.c
b/drivers/media/rc/keymaps/rc-tivo.c index 5cc1b45..454e062 100644
--- a/drivers/media/rc/keymaps/rc-tivo.c
+++ b/drivers/media/rc/keymaps/rc-tivo.c
@@ -15,62 +15,62 @@
* Initial mapping is for the TiVo remote included in the Nero LiquidTV
bundle, * which also ships with a TiVo-branded IR transceiver, supported by
the mceusb * driver. Note that the remote uses an NEC-ish protocol, but
instead of having - * a command/not_command pair, it has a vendor ID of
0x3085, but some keys, the + * a command/not_command pair, it has a vendor
ID of 0xa10c, but some keys, the * NEC extended checksums do pass, so the
table
Re: [PATCH 2/3] rc-core: split dev-s_filter
On Saturday 05 April 2014 00:06:01 David Härdeman wrote:
Overloading dev-s_filter to do two different functions (set wakeup filters
and generic hardware filters) makes it impossible to tell what the
hardware actually supports, so create a separate dev-s_wakeup_filter and
make the distinction explicit.
v2: hopefully address James' comments on what should be moved from this to
the next patch.
Signed-off-by: David Härdeman da...@hardeman.nu
Acked-by: James Hogan james.ho...@imgtec.com
Thanks
James
---
drivers/media/rc/img-ir/img-ir-hw.c | 15 ++-
drivers/media/rc/rc-main.c | 24 +---
include/media/rc-core.h |6 --
3 files changed, 35 insertions(+), 10 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-hw.c
b/drivers/media/rc/img-ir/img-ir-hw.c index 579a52b..0127dd2 100644
--- a/drivers/media/rc/img-ir/img-ir-hw.c
+++ b/drivers/media/rc/img-ir/img-ir-hw.c
@@ -504,6 +504,18 @@ unlock:
return ret;
}
+static int img_ir_set_normal_filter(struct rc_dev *dev,
+ struct rc_scancode_filter *sc_filter)
+{
+ return img_ir_set_filter(dev, RC_FILTER_NORMAL, sc_filter);
+}
+
+static int img_ir_set_wakeup_filter(struct rc_dev *dev,
+ struct rc_scancode_filter *sc_filter)
+{
+ return img_ir_set_filter(dev, RC_FILTER_WAKEUP, sc_filter);
+}
+
/**
* img_ir_set_decoder() - Set the current decoder.
* @priv:IR private data.
@@ -986,7 +998,8 @@ int img_ir_probe_hw(struct img_ir_priv *priv)
rdev-map_name = RC_MAP_EMPTY;
rc_set_allowed_protocols(rdev, img_ir_allowed_protos(priv));
rdev-input_name = IMG Infrared Decoder;
- rdev-s_filter = img_ir_set_filter;
+ rdev-s_filter = img_ir_set_normal_filter;
+ rdev-s_wakeup_filter = img_ir_set_wakeup_filter;
/* Register hardware decoder */
error = rc_register_device(rdev);
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 99697aa..ecbc20c 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -923,6 +923,7 @@ static ssize_t store_protocols(struct device *device,
int rc, i, count = 0;
ssize_t ret;
int (*change_protocol)(struct rc_dev *dev, u64 *rc_type);
+ int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter
*filter);
struct rc_scancode_filter local_filter, *filter;
/* Device is being removed */
@@ -1007,24 +1008,27 @@ static ssize_t store_protocols(struct device
*device, * Fall back to clearing the filter.
*/
filter = dev-scancode_filters[fattr-type];
+ set_filter = (fattr-type == RC_FILTER_NORMAL)
+ ? dev-s_filter : dev-s_wakeup_filter;
+
if (old_type != type filter-mask) {
local_filter = *filter;
if (!type) {
/* no protocol = clear filter */
ret = -1;
- } else if (!dev-s_filter) {
+ } else if (!set_filter) {
/* generic filtering = accept any filter */
ret = 0;
} else {
/* hardware filtering = try setting, otherwise clear */
- ret = dev-s_filter(dev, fattr-type, local_filter);
+ ret = set_filter(dev, local_filter);
}
if (ret 0) {
/* clear the filter */
local_filter.data = 0;
local_filter.mask = 0;
- if (dev-s_filter)
- dev-s_filter(dev, fattr-type, local_filter);
+ if (set_filter)
+ set_filter(dev, local_filter);
}
/* commit the new filter */
@@ -1106,6 +1110,7 @@ static ssize_t store_filter(struct device *device,
struct rc_scancode_filter local_filter, *filter;
int ret;
unsigned long val;
+ int (*set_filter)(struct rc_dev *dev, struct rc_scancode_filter
*filter);
/* Device is being removed */
if (!dev)
@@ -1115,8 +1120,11 @@ static ssize_t store_filter(struct device *device,
if (ret 0)
return ret;
+ set_filter = (fattr-type == RC_FILTER_NORMAL) ? dev-s_filter :
+ dev-s_wakeup_filter;
+
/* Scancode filter not supported (but still accept 0) */
- if (!dev-s_filter fattr-type != RC_FILTER_NORMAL)
+ if (!set_filter fattr-type == RC_FILTER_WAKEUP)
return val ? -EINVAL : count;
mutex_lock(dev-lock);
@@ -1128,13 +1136,15 @@ static ssize_t store_filter(struct device *device,
local_filter.mask = val;
else
local_filter.data = val;
+
if (!dev-enabled_protocols[fattr-type] local_filter.mask) {
/* refuse to set a filter
Re: [PATCH 3/3] rc-core: remove generic scancode filter
On Saturday 05 April 2014 00:06:06 David Härdeman wrote:
The generic scancode filtering has questionable value and makes it
impossible to determine from userspace if there is an actual
scancode hw filter present or not.
So revert the generic parts.
Based on a patch from James Hogan james.ho...@imgtec.com, but this
version also makes sure that only the valid sysfs files are created
in the first place.
v2: correct dev-s_filter check
v3: move some parts over from the previous patch
Signed-off-by: David Härdeman da...@hardeman.nu
Acked-by: James Hogan james.ho...@imgtec.com
Thanks
James
---
drivers/media/rc/rc-main.c | 88
+++- include/media/rc-core.h|
2 +
2 files changed, 55 insertions(+), 35 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index ecbc20c..970b93d 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -633,19 +633,13 @@ EXPORT_SYMBOL_GPL(rc_repeat);
static void ir_do_keydown(struct rc_dev *dev, int scancode,
u32 keycode, u8 toggle)
{
- struct rc_scancode_filter *filter;
- bool new_event = !dev-keypressed ||
- dev-last_scancode != scancode ||
- dev-last_toggle != toggle;
+ bool new_event = (!dev-keypressed ||
+ dev-last_scancode != scancode ||
+ dev-last_toggle != toggle);
if (new_event dev-keypressed)
ir_do_keyup(dev, false);
- /* Generic scancode filtering */
- filter = dev-scancode_filters[RC_FILTER_NORMAL];
- if (filter-mask ((scancode ^ filter-data) filter-mask))
- return;
-
input_event(dev-input_dev, EV_MSC, MSC_SCAN, scancode);
if (new_event keycode != KEY_RESERVED) {
@@ -1011,14 +1005,11 @@ static ssize_t store_protocols(struct device
*device, set_filter = (fattr-type == RC_FILTER_NORMAL)
? dev-s_filter : dev-s_wakeup_filter;
- if (old_type != type filter-mask) {
+ if (set_filter old_type != type filter-mask) {
local_filter = *filter;
if (!type) {
/* no protocol = clear filter */
ret = -1;
- } else if (!set_filter) {
- /* generic filtering = accept any filter */
- ret = 0;
} else {
/* hardware filtering = try setting, otherwise clear */
ret = set_filter(dev, local_filter);
@@ -1027,8 +1018,7 @@ static ssize_t store_protocols(struct device *device,
/* clear the filter */
local_filter.data = 0;
local_filter.mask = 0;
- if (set_filter)
- set_filter(dev, local_filter);
+ set_filter(dev, local_filter);
}
/* commit the new filter */
@@ -1072,7 +1062,10 @@ static ssize_t show_filter(struct device *device,
return -EINVAL;
mutex_lock(dev-lock);
- if (fattr-mask)
+ if ((fattr-type == RC_FILTER_NORMAL !dev-s_filter) ||
+ (fattr-type == RC_FILTER_WAKEUP !dev-s_wakeup_filter))
+ val = 0;
+ else if (fattr-mask)
val = dev-scancode_filters[fattr-type].mask;
else
val = dev-scancode_filters[fattr-type].data;
@@ -1120,12 +1113,11 @@ static ssize_t store_filter(struct device *device,
if (ret 0)
return ret;
+ /* Can the scancode filter be set? */
set_filter = (fattr-type == RC_FILTER_NORMAL) ? dev-s_filter :
dev-s_wakeup_filter;
-
- /* Scancode filter not supported (but still accept 0) */
- if (!set_filter fattr-type == RC_FILTER_WAKEUP)
- return val ? -EINVAL : count;
+ if (!set_filter)
+ return -EINVAL;
mutex_lock(dev-lock);
@@ -1143,11 +1135,9 @@ static ssize_t store_filter(struct device *device,
goto unlock;
}
- if (set_filter) {
- ret = set_filter(dev, local_filter);
- if (ret 0)
- goto unlock;
- }
+ ret = set_filter(dev, local_filter);
+ if (ret 0)
+ goto unlock;
/* Success, commit the new filter */
*filter = local_filter;
@@ -1199,27 +1189,45 @@ static RC_FILTER_ATTR(wakeup_filter,
S_IRUGO|S_IWUSR, static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR,
show_filter, store_filter, RC_FILTER_WAKEUP, true);
-static struct attribute *rc_dev_attrs[] = {
+static struct attribute *rc_dev_protocol_attrs[] = {
dev_attr_protocols.attr.attr,
+ NULL,
+};
+
+static struct attribute_group rc_dev_protocol_attr_grp = {
+ .attrs = rc_dev_protocol_attrs
Re: [PATCH 05/11] rc-core: split dev-s_filter
Hi David,
On Saturday 29 March 2014 17:11:11 David Härdeman wrote:
Overloading dev-s_filter to do two different functions (set wakeup filters
and generic hardware filters) makes it impossible to tell what the
hardware actually supports, so create a separate dev-s_wakeup_filter and
make the distinction explicit.
Signed-off-by: David Härdeman da...@hardeman.nu
---
@@ -1121,9 +1126,11 @@ static ssize_t store_filter(struct device *device,
if (ret 0)
return ret;
- /* Scancode filter not supported (but still accept 0) */
- if (!dev-s_filter fattr-type != RC_FILTER_NORMAL)
- return val ? -EINVAL : count;
+ /* Can the scancode filter be set? */
+ set_filter = (fattr-type == RC_FILTER_NORMAL)
+ ? dev-s_filter : dev-s_wakeup_filter;
+ if (!set_filter)
+ return -EINVAL;
Technically the removal of the fattr-type != RC_FILTER_NORMAL condition and
returning -EINVAL rather than val ? -EINVAL : count should be in patch 6
since it's for generic scancode filter support.
- if (dev-s_filter) {
- ret = dev-s_filter(dev, fattr-type, local_filter);
- if (ret 0)
- goto unlock;
- }
+
+ ret = set_filter(dev, local_filter);
+ if (ret 0)
+ goto unlock;
same here for removing the if condition.
Otherwise this patch looks okay to me.
Cheers
James
signature.asc
Description: This is a digitally signed message part.
Re: [PATCH 04/11] rc-core: do not change 32bit NEC scancode format for now
On 29/03/14 16:11, David Härdeman wrote:
This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2
The patch ignores the fact that NEC32 scancodes are generated not only in the
NEC raw decoder but also directly in some drivers. Whichever approach is
chosen
it should be consistent across drivers and this patch needs more discussion.
Furthermore, I'm convinced that we have to stop playing games trying to
decipher the meaning of NEC scancodes (what's the customer/vendor/address,
which byte is the MSB, etc).
This patch is in preparation for the next few patches in this series.
Signed-off-by: David Härdeman da...@hardeman.nu
---
drivers/media/rc/img-ir/img-ir-nec.c | 27 ++-
drivers/media/rc/ir-nec-decoder.c|5 --
drivers/media/rc/keymaps/rc-tivo.c | 86
+-
3 files changed, 59 insertions(+), 59 deletions(-)
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
b/drivers/media/rc/img-ir/img-ir-nec.c
index c0111d6..40ee844 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -5,6 +5,7 @@
*/
#include img-ir-hw.h
+#include linux/bitrev.h
/* Convert NEC data to a scancode */
static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
@@ -23,11 +24,11 @@ static int img_ir_nec_scancode(int len, u64 raw, enum
rc_type *protocol,
data_inv = (raw 24) 0xff;
if ((data_inv ^ data) != 0xff) {
/* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- *scancode = addr_inv 24 |
- addr 16 |
- data_inv 8 |
- data;
+ /* scan encoding: AAaaDDdd (LSBit first) */
+ *scancode = bitrev8(addr) 24 |
+ bitrev8(addr_inv) 16 |
+ bitrev8(data) 8 |
+ bitrev8(data_inv);
} else if ((addr_inv ^ addr) != 0xff) {
/* Extended NEC */
/* scan encoding: AAaaDD */
@@ -56,13 +57,15 @@ static int img_ir_nec_filter(const struct
rc_scancode_filter *in,
if ((in-data | in-mask) 0xff00) {
/* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- addr_inv = (in-data 24) 0xff;
- addr_inv_m = (in-mask 24) 0xff;
- addr = (in-data 16) 0xff;
- addr_m = (in-mask 16) 0xff;
- data_inv = (in-data 8) 0xff;
- data_inv_m = (in-mask 8) 0xff;
+ /* scan encoding: AAaaDDdd (LSBit first) */
+ addr = bitrev8((in-data 24) 0xff);
+ addr_m = (in-mask 24) 0xff;
+ addr_inv = bitrev8((in-data 16) 0xff);
+ addr_inv_m = (in-mask 16) 0xff;
+ data = bitrev8((in-data 8) 0xff);
+ data_m = (in-mask 8) 0xff;
+ data_inv = bitrev8((in-data 0) 0xff);
+ data_inv_m = (in-mask 0) 0xff;
I think the masks need bit reversing too, otherwise the mask bits won't
line up with the data as intended.
Otherwise this patch looks okay to me.
Cheers
James
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 06/11] rc-core: remove generic scancode filter
On 29/03/14 16:11, David Härdeman wrote:
The generic scancode filtering has questionable value and makes it
impossible to determine from userspace if there is an actual
scancode hw filter present or not.
So revert the generic parts.
Based on a patch from James Hogan james.ho...@imgtec.com, but this
version also makes sure that only the valid sysfs files are created
in the first place.
Signed-off-by: David Härdeman da...@hardeman.nu
---
drivers/media/rc/rc-main.c | 66
+---
include/media/rc-core.h|2 +
2 files changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index ba955ac..8675e07 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -634,7 +634,6 @@ EXPORT_SYMBOL_GPL(rc_repeat);
static void ir_do_keydown(struct rc_dev *dev, enum rc_type protocol,
u32 scancode, u32 keycode, u8 toggle)
{
- struct rc_scancode_filter *filter;
bool new_event = (!dev-keypressed ||
dev-last_protocol != protocol ||
dev-last_scancode != scancode ||
@@ -643,11 +642,6 @@ static void ir_do_keydown(struct rc_dev *dev, enum
rc_type protocol,
if (new_event dev-keypressed)
ir_do_keyup(dev, false);
- /* Generic scancode filtering */
- filter = dev-scancode_filters[RC_FILTER_NORMAL];
- if (filter-mask ((scancode ^ filter-data) filter-mask))
- return;
-
input_event(dev-input_dev, EV_MSC, MSC_SCAN, scancode);
if (new_event keycode != KEY_RESERVED) {
@@ -1017,14 +1011,11 @@ static ssize_t store_protocols(struct device *device,
set_filter = (fattr-type == RC_FILTER_NORMAL)
? dev-s_filter : dev-s_wakeup_filter;
- if (old_type != type filter-mask) {
+ if (set_filter old_type != type filter-mask) {
local_filter = *filter;
if (!type) {
/* no protocol = clear filter */
ret = -1;
- } else if (!set_filter) {
- /* generic filtering = accept any filter */
- ret = 0;
} else {
/* hardware filtering = try setting, otherwise clear */
ret = set_filter(dev, local_filter);
@@ -1033,8 +1024,7 @@ static ssize_t store_protocols(struct device *device,
/* clear the filter */
local_filter.data = 0;
local_filter.mask = 0;
- if (set_filter)
- set_filter(dev, local_filter);
+ set_filter(dev, local_filter);
}
/* commit the new filter */
@@ -1078,7 +1068,9 @@ static ssize_t show_filter(struct device *device,
return -EINVAL;
mutex_lock(dev-lock);
- if (fattr-mask)
+ if (!dev-s_filter)
+ val = 0;
I suspect this should take s_wakeup_filter into account depending on
fattr-type. It's probably quite common to have a wakeup filter but no
normal filter.
The rest looks reasonable, though it could easily have been a separate
patch (at least as long as the show/store callbacks don't assume the
presence of the callbacks they use).
Cheers
James
+ else if (fattr-mask)
val = dev-scancode_filters[fattr-type].mask;
else
val = dev-scancode_filters[fattr-type].data;
@@ -1202,27 +1194,45 @@ static RC_FILTER_ATTR(wakeup_filter, S_IRUGO|S_IWUSR,
static RC_FILTER_ATTR(wakeup_filter_mask, S_IRUGO|S_IWUSR,
show_filter, store_filter, RC_FILTER_WAKEUP, true);
-static struct attribute *rc_dev_attrs[] = {
+static struct attribute *rc_dev_protocol_attrs[] = {
dev_attr_protocols.attr.attr,
+ NULL,
+};
+
+static struct attribute_group rc_dev_protocol_attr_grp = {
+ .attrs = rc_dev_protocol_attrs,
+};
+
+static struct attribute *rc_dev_wakeup_protocol_attrs[] = {
dev_attr_wakeup_protocols.attr.attr,
+ NULL,
+};
+
+static struct attribute_group rc_dev_wakeup_protocol_attr_grp = {
+ .attrs = rc_dev_wakeup_protocol_attrs,
+};
+
+static struct attribute *rc_dev_filter_attrs[] = {
dev_attr_filter.attr.attr,
dev_attr_filter_mask.attr.attr,
- dev_attr_wakeup_filter.attr.attr,
- dev_attr_wakeup_filter_mask.attr.attr,
NULL,
};
-static struct attribute_group rc_dev_attr_grp = {
- .attrs = rc_dev_attrs,
+static struct attribute_group rc_dev_filter_attr_grp = {
+ .attrs = rc_dev_filter_attrs,
+};
+
+static struct attribute *rc_dev_wakeup_filter_attrs[] = {
+ dev_attr_wakeup_filter.attr.attr,
+ dev_attr_wakeup_filter_mask.attr.attr,
+ NULL,
};
-static const struct attribute_group *rc_dev_attr_groups[] = {
- rc_dev_attr_grp
Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
On 29/03/14 16:11, David Härdeman wrote:
Using the full 32 bits for all kinds of NEC scancodes simplifies rc-core
and the nec decoder without any loss of functionality.
In order to maintain backwards compatibility, some heuristics are added
in rc-main.c to convert scancodes to NEC32 as necessary.
I plan to introduce a different ioctl later which makes the protocol
explicit (and which expects all NEC scancodes to be 32 bit, thereby
removing the need for guesswork).
Signed-off-by: David Härdeman da...@hardeman.nu
---
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
b/drivers/media/rc/img-ir/img-ir-nec.c
index 40ee844..133ea45 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -5,42 +5,20 @@
*/
#include img-ir-hw.h
-#include linux/bitrev.h
/* Convert NEC data to a scancode */
static int img_ir_nec_scancode(int len, u64 raw, enum rc_type *protocol,
u32 *scancode, u64 enabled_protocols)
{
- unsigned int addr, addr_inv, data, data_inv;
/* a repeat code has no data */
if (!len)
return IMG_IR_REPEATCODE;
+
if (len != 32)
return -EINVAL;
- /* raw encoding: ddDDaaAA */
- addr = (raw 0) 0xff;
- addr_inv = (raw 8) 0xff;
- data = (raw 16) 0xff;
- data_inv = (raw 24) 0xff;
- if ((data_inv ^ data) != 0xff) {
- /* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: AAaaDDdd (LSBit first) */
- *scancode = bitrev8(addr) 24 |
- bitrev8(addr_inv) 16 |
- bitrev8(data) 8 |
- bitrev8(data_inv);
- } else if ((addr_inv ^ addr) != 0xff) {
- /* Extended NEC */
- /* scan encoding: AAaaDD */
- *scancode = addr 16 |
- addr_inv 8 |
- data;
- } else {
- /* Normal NEC */
- /* scan encoding: AADD */
- *scancode = addr 8 |
- data;
- }
+
+ /* raw encoding : ddDDaaAA - scan encoding: AAaaDDdd */
+ *scancode = swab32((u32)raw);
What's the point of the byte swapping?
Surely the most natural NEC encoding would just treat it as a single
32-bit (LSBit first) field rather than 4 8-bit fields that needs swapping.
Cheers
James
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 03/11] rc-core: document the protocol type
On 29/03/14 16:11, David Härdeman wrote: Right now the protocol information is not preserved, rc-core gets handed a scancode but has no idea which protocol it corresponds to. This patch (which required reading through the source/keymap for all drivers, not fun) makes the protocol information explicit which is important documentation and makes it easier to e.g. support multiple protocols with one decoder (think rc5 and rc-streamzap). The information isn't used yet so there should be no functional changes. Signed-off-by: David Härdeman da...@hardeman.nu Good stuff. I very much approve of the concept, and had considered doing the same thing myself. Cheers James signature.asc Description: OpenPGP digital signature
Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
On 31/03/14 11:19, David Härdeman wrote:
On 2014-03-31 11:44, James Hogan wrote:
On 29/03/14 16:11, David Härdeman wrote:
Using the full 32 bits for all kinds of NEC scancodes simplifies rc-core
and the nec decoder without any loss of functionality.
In order to maintain backwards compatibility, some heuristics are added
in rc-main.c to convert scancodes to NEC32 as necessary.
I plan to introduce a different ioctl later which makes the protocol
explicit (and which expects all NEC scancodes to be 32 bit, thereby
removing the need for guesswork).
Signed-off-by: David Härdeman da...@hardeman.nu
---
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
b/drivers/media/rc/img-ir/img-ir-nec.c
index 40ee844..133ea45 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -5,42 +5,20 @@
*/
#include img-ir-hw.h
-#include linux/bitrev.h
/* Convert NEC data to a scancode */
static int img_ir_nec_scancode(int len, u64 raw, enum rc_type
*protocol,
u32 *scancode, u64 enabled_protocols)
{
-unsigned int addr, addr_inv, data, data_inv;
/* a repeat code has no data */
if (!len)
return IMG_IR_REPEATCODE;
+
if (len != 32)
return -EINVAL;
-/* raw encoding: ddDDaaAA */
-addr = (raw 0) 0xff;
-addr_inv = (raw 8) 0xff;
-data = (raw 16) 0xff;
-data_inv = (raw 24) 0xff;
-if ((data_inv ^ data) != 0xff) {
-/* 32-bit NEC (used by Apple and TiVo remotes) */
-/* scan encoding: AAaaDDdd (LSBit first) */
-*scancode = bitrev8(addr) 24 |
-bitrev8(addr_inv) 16 |
-bitrev8(data) 8 |
-bitrev8(data_inv);
-} else if ((addr_inv ^ addr) != 0xff) {
-/* Extended NEC */
-/* scan encoding: AAaaDD */
-*scancode = addr 16 |
-addr_inv 8 |
-data;
-} else {
-/* Normal NEC */
-/* scan encoding: AADD */
-*scancode = addr 8 |
-data;
-}
+
+/* raw encoding : ddDDaaAA - scan encoding: AAaaDDdd */
+*scancode = swab32((u32)raw);
What's the point of the byte swapping?
Surely the most natural NEC encoding would just treat it as a single
32-bit (LSBit first) field rather than 4 8-bit fields that needs
swapping.
Thanks for having a look at the patches, I agree with your comments on
the other patches (and I have to respin some of them because I missed
two drivers), but the comments to this patch confuses me a bit.
That the NEC data is transmitted as 32 bits encoded with LSB bit order
within each byte is AFAIK just about the only thing that all
sources/documentation of the protocal can agree on (so bitrev:ing the
bits within each byte makes sense, unless the hardware has done it
already).
Agreed (in the case of img-ir there's a bit orientation setting which
ensures that the u64 raw has the correct bit order, in the case of NEC
the first bit received goes in the lowest order bit of the raw data).
As for the byte order, AAaaDDdd corresponds to the transmission order
and seems to be what most drivers expect/use for their RX data.
AAaaDDdd is big endian rendering, no? (like %08x)
If it should be interpreted as LSBit first, then the first bits received
should go in the low bits of the scancode, and by extension the first
bytes received in the low bytes of the scancode, i.e. at the end of the
inherently big-endian hexadecimal rendering of the scancode.
Are you suggesting that rc-core should standardize on ddDDaaAA order?
Yes (where ddDDaaAA means something like scancode
0x(~cmd)(cmd)(~addr)(addr))
This would mean that if the data is put in the right bit order (first
bit received in BIT(0), last bit received in BIT(31)), then the scancode
= raw, and if the data is received in the reverse bit order (like the
raw decoder, shifting the data left and inserting the last bit in
BIT(0)) then the scancode = bitrev32(raw).
Have I missed something?
Cheers
James
signature.asc
Description: OpenPGP digital signature
Re: [PATCH 10/11] [RFC] rc-core: use the full 32 bits for NEC scancodes
On 31/03/14 14:22, David Härdeman wrote: On 2014-03-31 12:56, James Hogan wrote: This would mean that if the data is put in the right bit order (first bit received in BIT(0), last bit received in BIT(31)), then the scancode = raw, and if the data is received in the reverse bit order (like the raw decoder, shifting the data left and inserting the last bit in BIT(0)) then the scancode = bitrev32(raw). Have I missed something? I just think we have to agree to disagree :) For me, storing/presenting the scancode as 0xAAaaDDdd is obviously the clearest and least confusing interpretation. But I might have spent too long time using that notation in code and mentally to be able to find anything else intuitive :) 0xAAaaDDdd means that you read/parse/print it left to right, just as you would if you drew a pulse-space chart showing the received IR pulse (time normally progresses to the right...modulo the per-byte bitrev). Sure, but the NEC bit order is little endian, and the scancode is a 32bit value not an array of 4 bytes, so it's artificial to expect it to make any sense when read as big endian. E.g. if you extended the transmission to 48 bits you'd expect the hex printed scancode to extend to the left not the right. The bits in the 32-bit word also become discontinuous for no good reason, especially considering the cases we're trying to take into account (NEC-32 and NEC-24) both effectively have 16-bit fields. It kind of matches the other protocol scancodes as well (the address bits high, cmd bits low, the high bits tend to remain constant for one given remote, the low bits change, although it's not a hard rule) and it Very true, but you still have the low byte of the command in the 2nd lowest byte, which is why my original suggestion was: 0xaaAAddDD I.e. swap 16bit halves, each 16bit field intact. matches most software I've ever seen (AFAIK, LIRC represents NEC32 scancodes this way, as does e.g. the Pronto software and protocol). That said...I think we at least agree that we need *a* representation and that it should be used consistently in all drivers, right? Yes, that would be nice. Cheers James signature.asc Description: OpenPGP digital signature
Re: [PATCH 06/11] rc-core: remove generic scancode filter
On Monday 31 March 2014 21:38:13 David Härdeman wrote: The rest looks reasonable, though it could easily have been a separate patch (at least as long as the show/store callbacks don't assume the presence of the callbacks they use). Yes, I wanted to avoid there being more intermediary states than necessary (i.e. first a read/writable sysfs file, then one that can't be read/written, then the file disappears...). Fair enough Can still respin it on top of your patch if you prefer. It doesn't particularly bother me tbh, so do what you think is best. Cheers James signature.asc Description: This is a digitally signed message part.
Re: [PATCH] rc-core: do not change 32bit NEC scancode format for now
On Friday 28 March 2014 01:08:56 David Härdeman wrote: On Thu, Mar 27, 2014 at 11:21:23PM +, James Hogan wrote: Hi David, On Thursday 27 March 2014 22:00:37 David Härdeman wrote: This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2 The patch ignores the fact that NEC32 scancodes are generated not only in the NEC raw decoder but also directly in some drivers. Whichever approach is chosen it should be consistent across drivers and this patch needs more discussion. Fair enough. For reference which drivers are you referring to? The ones I'm aware of right now are: Thanks, I hadn't looked properly outside of drivers/media/rc/ :( drivers/media/usb/dvb-usb/dib0700_core.c AFAICT this only seems to support 16bit and 24bit NEC, so NEC-32 doesn't affect it. I may have missed something subtle. drivers/media/usb/dvb-usb-v2/az6007.c drivers/media/usb/dvb-usb-v2/af9035.c drivers/media/usb/dvb-usb-v2/rtl28xxu.c drivers/media/usb/dvb-usb-v2/af9015.c drivers/media/usb/em28xx/em28xx-input.c Note, it appears none of these do any bit reversing for the 32bit case compared to 16/24 bit, so they're already different to the NEC32 scancode encoding that the raw nec decoder and tivo keymap were using, which used a different bitorder (!!) between the 32-bit and the 24/16-bit cases. Furthermore, I'm convinced that we have to stop playing games trying to decipher the meaning of NEC scancodes (what's the customer/vendor/address, which byte is the MSB, etc). Well when all the buttons on a remote have the same address, and the numeric buttons are sequential commands only in a certain bit/byte order, then I think the word decipher is probably a bit of a stretch. I think you misunderstood me. decipher is a bit of a stretch when talking of one remote control (I'm guessing you're referring to the Tivo remote). It's not that much of a stretch if we're referring to trying to derive a common meaning from the encoding used for *all* remote controls out there. The discussion about the 24-bit version of NEC and whether the address bytes were in MSB or LSB order was a good example. Andy Walls cited a NEC manual which stated one thing and people also referred to http://www.sbprojects.com/knowledge/ir/nec.php which stated the opposite (while referring to an unnamed VCR service manual). As a third example...I've read a Samsung service manual which happily stated that the remote (which used the NEC protocol) sent IR commands starting with the address x 2 (and looking at the raw NEC command, it did start with something like 0x07 0x07). So don't get me wrong, I wasn't referring to your analysis of the Tivo remote but more the general approach that has been taken until now wrt. the NEC protocol in the kernel drivers. Okay, thanks for the clarification. Nevertheless I don't have any attachment to 32-bit NEC. If it's likely to change again I'd prefer img-ir-nec just not support it for now, so please could you add the following hunks to your patch (or if the original patch is to be dropped this could be squashed into the img-ir-nec patch): I'd rather show you my complete proposal first before doing something radical with your driver. But it was a good reminder that I need to keep the NEC32 parsing in your driver in mind as well. Okay no problem. I had assumed you were aiming for a short term fix to prevent the encoding change hitting mainline or an actual release (v3.15). Cheers James I'll post separate proposals to that effect later. Great, please do Cc me (I have a work in progress branch to unify NEC scancodes, but I'm not sure I'd have time to complete it any time soon anyway) That is what I'm working on as well at the moment. It's actually to solve two problems...both to unify NEC scancodes (by simply using 32 bit scancodes everywhere and some fallback code...I'm not 100% sure it's doable but I hope so since it's the only sane solution I can think of in the long run)...and to make sure that protocol information actually gets used in keymaps, etc. I hope to post patches soon that'll make it clearer. Regards, David -- 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 signature.asc Description: This is a digitally signed message part.
Re: [PATCH] rc-core: do not change 32bit NEC scancode format for now
Hi David,
On Thursday 27 March 2014 22:00:37 David Härdeman wrote:
This reverts 18bc17448147e93f31cc9b1a83be49f1224657b2
The patch ignores the fact that NEC32 scancodes are generated not only in
the NEC raw decoder but also directly in some drivers. Whichever approach
is chosen it should be consistent across drivers and this patch needs more
discussion.
Fair enough. For reference which drivers are you referring to?
Furthermore, I'm convinced that we have to stop playing games trying to
decipher the meaning of NEC scancodes (what's the customer/vendor/address,
which byte is the MSB, etc).
Well when all the buttons on a remote have the same address, and the numeric
buttons are sequential commands only in a certain bit/byte order, then I think
the word decipher is probably a bit of a stretch.
Nevertheless I don't have any attachment to 32-bit NEC. If it's likely to
change again I'd prefer img-ir-nec just not support it for now, so please
could you add the following hunks to your patch (or if the original patch is
to be dropped this could be squashed into the img-ir-nec patch):
diff --git a/drivers/media/rc/img-ir/img-ir-nec.c
b/drivers/media/rc/img-ir/img-ir-nec.c
index e7a731b..419d087 100644
--- a/drivers/media/rc/img-ir/img-ir-nec.c
+++ b/drivers/media/rc/img-ir/img-ir-nec.c
@@ -21,12 +21,7 @@ static int img_ir_nec_scancode(int len, u64 raw, int
*scancode, u64 protocols)
data = (raw 16) 0xff;
data_inv = (raw 24) 0xff;
if ((data_inv ^ data) != 0xff) {
- /* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- *scancode = addr_inv 24 |
- addr 16 |
- data_inv 8 |
- data;
+ return -EINVAL;
} else if ((addr_inv ^ addr) != 0xff) {
/* Extended NEC */
/* scan encoding: AAaaDD */
@@ -53,14 +48,7 @@ static int img_ir_nec_filter(const struct rc_scancode_filter
*in,
data_m = in-mask 0xff;
if ((in-data | in-mask) 0xff00) {
- /* 32-bit NEC (used by Apple and TiVo remotes) */
- /* scan encoding: aaAAddDD */
- addr_inv = (in-data 24) 0xff;
- addr_inv_m = (in-mask 24) 0xff;
- addr = (in-data 16) 0xff;
- addr_m = (in-mask 16) 0xff;
- data_inv = (in-data 8) 0xff;
- data_inv_m = (in-mask 8) 0xff;
+ return -EINVAL;
} else if ((in-data | in-mask) 0x00ff) {
/* Extended NEC */
/* scan encoding AAaaDD */
I'll post separate proposals to that effect later.
Great, please do Cc me
(I have a work in progress branch to unify NEC scancodes, but I'm not sure
I'd have time to complete it any time soon anyway)
Thanks
James
Signed-off-by: David Härdeman da...@hardeman.nu
---
drivers/media/rc/ir-nec-decoder.c |5 --
drivers/media/rc/keymaps/rc-tivo.c | 86
++-- 2 files changed, 44 insertions(+), 47
deletions(-)
diff --git a/drivers/media/rc/ir-nec-decoder.c
b/drivers/media/rc/ir-nec-decoder.c index 735a509..c4333d5 100644
--- a/drivers/media/rc/ir-nec-decoder.c
+++ b/drivers/media/rc/ir-nec-decoder.c
@@ -172,10 +172,7 @@ static int ir_nec_decode(struct rc_dev *dev, struct
ir_raw_event ev) if (send_32bits) {
/* NEC transport, but modified protocol, used by at
* least Apple and TiVo remotes */
- scancode = not_address 24 |
-address 16 |
-not_command 8 |
-command;
+ scancode = data-bits;
IR_dprintk(1, NEC (modified) scancode 0x%08x\n,
scancode);
} else if ((address ^ not_address) != 0xff) {
/* Extended NEC */
diff --git a/drivers/media/rc/keymaps/rc-tivo.c
b/drivers/media/rc/keymaps/rc-tivo.c index 5cc1b45..454e062 100644
--- a/drivers/media/rc/keymaps/rc-tivo.c
+++ b/drivers/media/rc/keymaps/rc-tivo.c
@@ -15,62 +15,62 @@
* Initial mapping is for the TiVo remote included in the Nero LiquidTV
bundle, * which also ships with a TiVo-branded IR transceiver, supported by
the mceusb * driver. Note that the remote uses an NEC-ish protocol, but
instead of having - * a command/not_command pair, it has a vendor ID of
0x3085, but some keys, the + * a command/not_command pair, it has a vendor
ID of 0xa10c, but some keys, the * NEC extended checksums do pass, so the
table presently has the intended * values and the checksum-passed versions
for those keys.
*/
static struct rc_map_table tivo[] = {
- { 0x3085f009, KEY_MEDIA }, /* TiVo Button */
- { 0x3085e010, KEY_POWER2 }, /* TV Power */
- { 0x3085e011, KEY_TV
Re: [PATCH v4 00/10] media: rc: ImgTec IR decoder driver
On 25/03/14 23:53, David Härdeman wrote: On Fri, Feb 28, 2014 at 11:28:50PM +, James Hogan wrote: Add a driver for the ImgTec Infrared decoder block. Two separate rc input devices are exposed depending on kernel configuration. One uses the hardware decoder which is set up with timings for a specific protocol and supports mask/value filtering and wake events. The other uses raw edge interrupts and the generic software protocol decoders to allow multiple protocols to be supported, including those not supported by the hardware decoder. One thing I just noticed...your copyright headers throughout the driver seems a bit...sparse? :) True, I can add the basic: * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or * (at your option) any later version. to each of the files if you think it's necessary. Cheers James signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/5] rc-main: add generic scancode filtering
On 26/03/14 13:44, David Härdeman wrote: On 2014-03-26 08:08, Antti Seppälä wrote: On 26 March 2014 01:21, David Härdeman da...@hardeman.nu wrote: On Tue, Mar 25, 2014 at 09:12:11AM +, James Hogan wrote: On Tuesday 25 March 2014 00:51:46 David Härdeman wrote: What's the purpose of providing the sw scancode filtering in the case where there's no hardware filtering support at all? Consistency is probably the main reason, but I'll admit it's not perfectly consistent between generic/hardware filtering (mostly thanks to NEC scancode complexities), and I have no particular objection to dropping it if that isn't considered a good enough reason. I'm kind of sceptical...and given how difficult it is to remove functionality that is in a released kernel...I think that particular part (i.e. the software filtering) should be removed until it has had further discussion... ... I don't understand. What's the purpose of a software fallback for scancode filtering? Antti? Well since the ImgTec patches will create a new sysfs interface for the HW scancode filtering I figured that it would be nice for it to also function on devices which lack the hardware filtering capabilities. Especially since it's only three lines of code. :) Therefore I suggested the software fallback. At the time I had no clue that there might be added complexities with nec scancodes. It's not only NEC scancodes, the sw scancode filter is state that is changeable from user-space and which will require reader/writer synchronization during the RX path (which is the hottest path in rc-core). I've posted patches before which make the RX path lockless, this change makes complicates such changes. Additionally, the provision of the sw fallback means that userspace has no idea if there is an actual hardware filter present or not, meaning that a userspace program that is aware of the scancode filter will always enable it. So, I still think the SW part should be reverted, at least for now (i.e. the sysfs file should only be present if there is hardware support). I've prepared a revert patch (which also reverts a part of a later patch which takes SW filtering into account when updating the filter on a protocol change). I'll double check it works right later and submit. Note that this still leaves the sysfs nodes there though, but if !s_filter then they read as 0 and only accept the value 0 to be written (mask == 0 represents no filtering). Cheers James signature.asc Description: OpenPGP digital signature
