Re: em28xx: msi Digivox ATSC board id [0db0:8810]
Em Sat, 15 Dec 2012 03:12:58 +0200 Antti Palosaari escreveu: > On 12/15/2012 03:03 AM, Mauro Carvalho Chehab wrote: > > Em Sat, 15 Dec 2012 02:56:25 +0200 > > Antti Palosaari escreveu: > > > >> NACK. NEC variant selection logic is broken by design. > > > > If you think so, then feel free to fix it without causing regressions to > > the existing userspace. > > > > While you don't do it, I don't see anything wrong on this patch, as it > > will behave just like any other NEC decoder. > > yes, so true as I mentioned end of the mail. Oh, I didn't saw your comments. Sorry. Please, next time, drop the part of the code that you're not commenting. On long emails like that, it is sometimes hard to see what's out there. I'll reply to your comments there again. > But it is very high > probability there is some non/wrong working keys when 32bit NEC variant > remote is used with that implementation. > > And what happened those patches David sends sometime ago. I remember > there was a patch for the af9015 which removes that kind of logic from > the driver. If not change NEC to 32bit at least heuristic could be moved > to single point - rc-core. There were some problems on his series, and it was breaking the userspace API. David made a new series, with a smaller set of patches that got applied, without those changes. The big issue there is to not break the current NEC userspace tables. Unfortunately, when the NEC software decoder was written, it were taking care only of the real NEC standard (the 24-bits and 32-bits variants aren't part of any spec I'm aware of). When we latter added support for those weird variants (RC-5 variants; Sony variants; NEC variants), it was decided, after long debates at the mailing list, to not create a new protocol for each, but, instead, to add support for them into the existing code. This is OK on most cases, as the variants are real variants, and the decoder can properly distinguish them. Unfortunately, NEC protocol variants don't fill on such case, as the only difference is that they doesn't honour to the checksum bytes. So, again after long discussions, it was decided to implement it the way it is. Changing it right now is not trivial, as it is easy to break existing userspace. Regards, Mauro -- 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: em28xx: msi Digivox ATSC board id [0db0:8810]
Em Sat, 15 Dec 2012 02:56:25 +0200
Antti Palosaari escreveu:
> On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote:
> > Em Fri, 14 Dec 2012 17:39:50 -0200
> > Mauro Carvalho Chehab escreveu:
> >
> >>> Anyway, first we have to GET the bytes from the hardware. That's our
> >>> current problem !
> >>> And the hardware seems to need a different setup for reg 0x50 for the
> >>> different NEC sub protocols.
> >>> Which means that the we need to know the sub protocol BEFORE we get any
> >>> bytes from the device.
> >>
> >> No. All em28xx needs is to make sure that the NEC protocol will return
> >> the full 32 bits scancode.
> >
> > It seems a way easier/quicker to just add the proper support there at the
> > driver than keep answering to this thread ;)
> >
> > Tested here with a Terratec HTC stick, and using two different IR's:
> > - a Terratec IR (address code 0x14 - standard NEC);
> > - a Pixelview IR (address code 0x866b - 24 bits NEC).
> >
> > All tests were done with the very latest version of ir-keytable, found at
> > v4l-utils.git tree (http://git.linuxtv.org/v4l-utils.git).
> >
> > See the results, with the Terratec table loaded (default when the
> > driver is loaded):
> >
> > # ir-keytable -t
> > Testing events. Please, press CTRL-C to abort.
> > # Terratec IR
> > 1355529698.198046: event type EV_MSC(0x04): scancode = 0x1402
> > 1355529698.198046: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
> > 1355529698.198046: event type EV_SYN(0x00).
> > 11355529698.298170: event type EV_MSC(0x04): scancode = 0x1402
> > 1355529698.298170: event type EV_SYN(0x00).
> > 1355529698.547998: event type EV_KEY(0x01) key_up: KEY_1(0x0001)
> > 1355529698.547998: event type EV_SYN(0x00).
> > # Pixelview IR
> > 1355530261.416415: event type EV_MSC(0x04): scancode = 0x866b01
> > 1355530261.416415: event type EV_SYN(0x00).
> > 1355530262.216301: event type EV_MSC(0x04): scancode = 0x866b0b
> > 1355530262.216301: event type EV_SYN(0x00).
> >
> > Replacing the keytable to the Pixelview's one:
> >
> > # ir-keytable -w /etc/rc_keymaps/pixelview_002t -c
> > Read pixelview_002t table
> > Old keytable cleared
> > Wrote 26 keycode(s) to driver
> > Protocols changed to NEC
> >
> > # ir-keytable -t
> > Testing events. Please, press CTRL-C to abort.
> > 1355530569.420398: event type EV_MSC(0x04): scancode = 0x1402
> > 1355530569.420398: event type EV_SYN(0x00).
> > 1355530588.120409: event type EV_MSC(0x04): scancode = 0x866b01
> > 1355530588.120409: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
> > 1355530591.670077: event type EV_SYN(0x00).
> >
> > And, finally, keeping both keytables active at the same time:
> >
> > # ir-keytable -c -w /etc/rc_keymaps/pixelview_002t -w
> > /etc/rc_keymaps/nec_terratec_cinergy_xs
> > Read pixelview_002t table
> > Read nec_terratec_cinergy_xs table
> > Old keytable cleared
> > Wrote 74 keycode(s) to driver
> > Protocols changed to NEC
> >
> > # sudo ir-keytable -t
> > Testing events. Please, press CTRL-C to abort.
> > 1355530856.325201: event type EV_MSC(0x04): scancode = 0x866b01
> > 1355530856.325201: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
> > 1355530856.325201: event type EV_SYN(0x00).
> > 11355530856.575070: event type EV_KEY(0x01) key_up: KEY_1(0x0001)
> > 1355530856.575070: event type EV_SYN(0x00).
> > 1355530869.125226: event type EV_MSC(0x04): scancode = 0x1402
> > 1355530869.125226: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
> > 1355530869.125226: event type EV_SYN(0x00).
> > 11355530869.225216: event type EV_MSC(0x04): scancode = 0x1402
> > 1355530869.225216: event type EV_SYN(0x00).
> > 1355530869.475075: event type EV_KEY(0x01) key_up: KEY_1(0x0001)
> > 1355530869.475075: event type EV_SYN(0x00).
> >
> > -
> >
> > em28xx: add support for 24bits/32 bits NEC variants on em2874 and upper
> >
> > By disabling the NEC parity check, it is possible to handle all 3 NEC
> > protocol variants (32, 24 or 16 bits).
> >
> > Change the driver in order to handle all of them.
> >
> > Signed-off-by: Mauro Carvalho Chehab
>
>
>
>
>
> NACK. NEC variant selection logic is broken by design.
>
>
>
>
>
> >
> > diff --git a/drivers/media/usb/em28xx/em28xx-input.c
> > b/drivers/media/usb/em28xx/em28xx-input.c
> > index 97d36b4..c84e4c8 100644
> > --- a/drivers/media/usb/em28xx/em28xx-input.c
> > +++ b/drivers/media/usb/em28xx/em28xx-input.c
> > @@ -57,8 +57,8 @@ MODULE_PARM_DESC(ir_debug, "enable debug messages [IR]");
> > struct em28xx_ir_poll_result {
> > unsigned int toggle_bit:1;
> > unsigned int read_count:7;
> > - u8 rc_address;
> > - u8 rc_data[4]; /* 1 byte on em2860/2880, 4 on em2874 */
> > +
> > + u32 scancode;
> > };
> >
> > struct em28xx_IR {
> > @@ -72,6 +72,7 @@ struct em28xx_IR {
> > struct delayed_work work;
> > unsigned int full_c
Re: em28xx: msi Digivox ATSC board id [0db0:8810]
On 12/15/2012 03:03 AM, Mauro Carvalho Chehab wrote: Em Sat, 15 Dec 2012 02:56:25 +0200 Antti Palosaari escreveu: NACK. NEC variant selection logic is broken by design. If you think so, then feel free to fix it without causing regressions to the existing userspace. While you don't do it, I don't see anything wrong on this patch, as it will behave just like any other NEC decoder. yes, so true as I mentioned end of the mail. But it is very high probability there is some non/wrong working keys when 32bit NEC variant remote is used with that implementation. And what happened those patches David sends sometime ago. I remember there was a patch for the af9015 which removes that kind of logic from the driver. If not change NEC to 32bit at least heuristic could be moved to single point - rc-core. regards Antti -- http://palosaari.fi/ -- 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: em28xx: msi Digivox ATSC board id [0db0:8810]
Em Sat, 15 Dec 2012 02:56:25 +0200 Antti Palosaari escreveu: > NACK. NEC variant selection logic is broken by design. If you think so, then feel free to fix it without causing regressions to the existing userspace. While you don't do it, I don't see anything wrong on this patch, as it will behave just like any other NEC decoder. Cheers, Mauro -- 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: em28xx: msi Digivox ATSC board id [0db0:8810]
On 12/15/2012 02:26 AM, Mauro Carvalho Chehab wrote:
Em Fri, 14 Dec 2012 17:39:50 -0200
Mauro Carvalho Chehab escreveu:
Anyway, first we have to GET the bytes from the hardware. That's our
current problem !
And the hardware seems to need a different setup for reg 0x50 for the
different NEC sub protocols.
Which means that the we need to know the sub protocol BEFORE we get any
bytes from the device.
No. All em28xx needs is to make sure that the NEC protocol will return
the full 32 bits scancode.
It seems a way easier/quicker to just add the proper support there at the
driver than keep answering to this thread ;)
Tested here with a Terratec HTC stick, and using two different IR's:
- a Terratec IR (address code 0x14 - standard NEC);
- a Pixelview IR (address code 0x866b - 24 bits NEC).
All tests were done with the very latest version of ir-keytable, found at
v4l-utils.git tree (http://git.linuxtv.org/v4l-utils.git).
See the results, with the Terratec table loaded (default when the
driver is loaded):
# ir-keytable -t
Testing events. Please, press CTRL-C to abort.
# Terratec IR
1355529698.198046: event type EV_MSC(0x04): scancode = 0x1402
1355529698.198046: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
1355529698.198046: event type EV_SYN(0x00).
11355529698.298170: event type EV_MSC(0x04): scancode = 0x1402
1355529698.298170: event type EV_SYN(0x00).
1355529698.547998: event type EV_KEY(0x01) key_up: KEY_1(0x0001)
1355529698.547998: event type EV_SYN(0x00).
# Pixelview IR
1355530261.416415: event type EV_MSC(0x04): scancode = 0x866b01
1355530261.416415: event type EV_SYN(0x00).
1355530262.216301: event type EV_MSC(0x04): scancode = 0x866b0b
1355530262.216301: event type EV_SYN(0x00).
Replacing the keytable to the Pixelview's one:
# ir-keytable -w /etc/rc_keymaps/pixelview_002t -c
Read pixelview_002t table
Old keytable cleared
Wrote 26 keycode(s) to driver
Protocols changed to NEC
# ir-keytable -t
Testing events. Please, press CTRL-C to abort.
1355530569.420398: event type EV_MSC(0x04): scancode = 0x1402
1355530569.420398: event type EV_SYN(0x00).
1355530588.120409: event type EV_MSC(0x04): scancode = 0x866b01
1355530588.120409: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
1355530591.670077: event type EV_SYN(0x00).
And, finally, keeping both keytables active at the same time:
# ir-keytable -c -w /etc/rc_keymaps/pixelview_002t -w
/etc/rc_keymaps/nec_terratec_cinergy_xs
Read pixelview_002t table
Read nec_terratec_cinergy_xs table
Old keytable cleared
Wrote 74 keycode(s) to driver
Protocols changed to NEC
# sudo ir-keytable -t
Testing events. Please, press CTRL-C to abort.
1355530856.325201: event type EV_MSC(0x04): scancode = 0x866b01
1355530856.325201: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
1355530856.325201: event type EV_SYN(0x00).
11355530856.575070: event type EV_KEY(0x01) key_up: KEY_1(0x0001)
1355530856.575070: event type EV_SYN(0x00).
1355530869.125226: event type EV_MSC(0x04): scancode = 0x1402
1355530869.125226: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
1355530869.125226: event type EV_SYN(0x00).
11355530869.225216: event type EV_MSC(0x04): scancode = 0x1402
1355530869.225216: event type EV_SYN(0x00).
1355530869.475075: event type EV_KEY(0x01) key_up: KEY_1(0x0001)
1355530869.475075: event type EV_SYN(0x00).
-
em28xx: add support for 24bits/32 bits NEC variants on em2874 and upper
By disabling the NEC parity check, it is possible to handle all 3 NEC
protocol variants (32, 24 or 16 bits).
Change the driver in order to handle all of them.
Signed-off-by: Mauro Carvalho Chehab
NACK. NEC variant selection logic is broken by design.
diff --git a/drivers/media/usb/em28xx/em28xx-input.c
b/drivers/media/usb/em28xx/em28xx-input.c
index 97d36b4..c84e4c8 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -57,8 +57,8 @@ MODULE_PARM_DESC(ir_debug, "enable debug messages [IR]");
struct em28xx_ir_poll_result {
unsigned int toggle_bit:1;
unsigned int read_count:7;
- u8 rc_address;
- u8 rc_data[4]; /* 1 byte on em2860/2880, 4 on em2874 */
+
+ u32 scancode;
};
struct em28xx_IR {
@@ -72,6 +72,7 @@ struct em28xx_IR {
struct delayed_work work;
unsigned int full_code:1;
unsigned int last_readcount;
+ u64 rc_type;
int (*get_key)(struct em28xx_IR *, struct em28xx_ir_poll_result *);
};
@@ -236,11 +237,8 @@ static int default_polling_getkey(struct em28xx_IR *ir,
/* Infrared read count (Reg 0x45[6:0] */
poll_result->read_cou
Re: em28xx: msi Digivox ATSC board id [0db0:8810]
Em Fri, 14 Dec 2012 22:26:31 -0200 Mauro Carvalho Chehab escreveu: > Em Fri, 14 Dec 2012 17:39:50 -0200 > Mauro Carvalho Chehab escreveu: > > > > Anyway, first we have to GET the bytes from the hardware. That's our > > > current problem ! > > > And the hardware seems to need a different setup for reg 0x50 for the > > > different NEC sub protocols. > > > Which means that the we need to know the sub protocol BEFORE we get any > > > bytes from the device. > > > > No. All em28xx needs is to make sure that the NEC protocol will return > > the full 32 bits scancode. > > It seems a way easier/quicker to just add the proper support there at the > driver than keep answering to this thread ;) > > Tested here with a Terratec HTC stick, and using two different IR's: > - a Terratec IR (address code 0x14 - standard NEC); > - a Pixelview IR (address code 0x866b - 24 bits NEC). Just in case, I tested also that RC5 keeps working, by using a Hauppauge grey control: # ir-keytable -c -w /etc/rc_keymaps/hauppauge Read hauppauge table Old keytable cleared Wrote 136 keycode(s) to driver Protocols changed to RC-5 # sudo ir-keytable -t Testing events. Please, press CTRL-C to abort. 1355531445.443208: event type EV_MSC(0x04): scancode = 0x1e02 1355531445.443208: event type EV_KEY(0x01) key_down: KEY_2(0x0001) 1355531445.443208: event type EV_SYN(0x00). 21355531445.543207: event type EV_MSC(0x04): scancode = 0x1e02 1355531445.543207: event type EV_SYN(0x00). 1355531445.793072: event type EV_KEY(0x01) key_up: KEY_2(0x0001) 1355531445.793072: event type EV_SYN(0x00). 1355531446.643224: event type EV_MSC(0x04): scancode = 0x1e02 1355531446.643224: event type EV_KEY(0x01) key_down: KEY_2(0x0001) 1355531446.643224: event type EV_SYN(0x00). 21355531446.743205: event type EV_MSC(0x04): scancode = 0x1e02 1355531446.743205: event type EV_SYN(0x00). Cheers, Mauro -- 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: em28xx: msi Digivox ATSC board id [0db0:8810]
Em Fri, 14 Dec 2012 17:39:50 -0200
Mauro Carvalho Chehab escreveu:
> > Anyway, first we have to GET the bytes from the hardware. That's our
> > current problem !
> > And the hardware seems to need a different setup for reg 0x50 for the
> > different NEC sub protocols.
> > Which means that the we need to know the sub protocol BEFORE we get any
> > bytes from the device.
>
> No. All em28xx needs is to make sure that the NEC protocol will return
> the full 32 bits scancode.
It seems a way easier/quicker to just add the proper support there at the
driver than keep answering to this thread ;)
Tested here with a Terratec HTC stick, and using two different IR's:
- a Terratec IR (address code 0x14 - standard NEC);
- a Pixelview IR (address code 0x866b - 24 bits NEC).
All tests were done with the very latest version of ir-keytable, found at
v4l-utils.git tree (http://git.linuxtv.org/v4l-utils.git).
See the results, with the Terratec table loaded (default when the
driver is loaded):
# ir-keytable -t
Testing events. Please, press CTRL-C to abort.
# Terratec IR
1355529698.198046: event type EV_MSC(0x04): scancode = 0x1402
1355529698.198046: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
1355529698.198046: event type EV_SYN(0x00).
11355529698.298170: event type EV_MSC(0x04): scancode = 0x1402
1355529698.298170: event type EV_SYN(0x00).
1355529698.547998: event type EV_KEY(0x01) key_up: KEY_1(0x0001)
1355529698.547998: event type EV_SYN(0x00).
# Pixelview IR
1355530261.416415: event type EV_MSC(0x04): scancode = 0x866b01
1355530261.416415: event type EV_SYN(0x00).
1355530262.216301: event type EV_MSC(0x04): scancode = 0x866b0b
1355530262.216301: event type EV_SYN(0x00).
Replacing the keytable to the Pixelview's one:
# ir-keytable -w /etc/rc_keymaps/pixelview_002t -c
Read pixelview_002t table
Old keytable cleared
Wrote 26 keycode(s) to driver
Protocols changed to NEC
# ir-keytable -t
Testing events. Please, press CTRL-C to abort.
1355530569.420398: event type EV_MSC(0x04): scancode = 0x1402
1355530569.420398: event type EV_SYN(0x00).
1355530588.120409: event type EV_MSC(0x04): scancode = 0x866b01
1355530588.120409: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
1355530591.670077: event type EV_SYN(0x00).
And, finally, keeping both keytables active at the same time:
# ir-keytable -c -w /etc/rc_keymaps/pixelview_002t -w
/etc/rc_keymaps/nec_terratec_cinergy_xs
Read pixelview_002t table
Read nec_terratec_cinergy_xs table
Old keytable cleared
Wrote 74 keycode(s) to driver
Protocols changed to NEC
# sudo ir-keytable -t
Testing events. Please, press CTRL-C to abort.
1355530856.325201: event type EV_MSC(0x04): scancode = 0x866b01
1355530856.325201: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
1355530856.325201: event type EV_SYN(0x00).
11355530856.575070: event type EV_KEY(0x01) key_up: KEY_1(0x0001)
1355530856.575070: event type EV_SYN(0x00).
1355530869.125226: event type EV_MSC(0x04): scancode = 0x1402
1355530869.125226: event type EV_KEY(0x01) key_down: KEY_1(0x0001)
1355530869.125226: event type EV_SYN(0x00).
11355530869.225216: event type EV_MSC(0x04): scancode = 0x1402
1355530869.225216: event type EV_SYN(0x00).
1355530869.475075: event type EV_KEY(0x01) key_up: KEY_1(0x0001)
1355530869.475075: event type EV_SYN(0x00).
-
em28xx: add support for 24bits/32 bits NEC variants on em2874 and upper
By disabling the NEC parity check, it is possible to handle all 3 NEC
protocol variants (32, 24 or 16 bits).
Change the driver in order to handle all of them.
Signed-off-by: Mauro Carvalho Chehab
diff --git a/drivers/media/usb/em28xx/em28xx-input.c
b/drivers/media/usb/em28xx/em28xx-input.c
index 97d36b4..c84e4c8 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -57,8 +57,8 @@ MODULE_PARM_DESC(ir_debug, "enable debug messages [IR]");
struct em28xx_ir_poll_result {
unsigned int toggle_bit:1;
unsigned int read_count:7;
- u8 rc_address;
- u8 rc_data[4]; /* 1 byte on em2860/2880, 4 on em2874 */
+
+ u32 scancode;
};
struct em28xx_IR {
@@ -72,6 +72,7 @@ struct em28xx_IR {
struct delayed_work work;
unsigned int full_code:1;
unsigned int last_readcount;
+ u64 rc_type;
int (*get_key)(struct em28xx_IR *, struct em28xx_ir_poll_result *);
};
@@ -236,11 +237,8 @@ static int default_polling_getkey(struct em28xx_IR *ir,
/* Infrared read count (Reg 0x45[6:0] */
poll_result->read_count = (msg[0] & 0x7f);
- /* Remote Control Address (Reg 0x46) */
- poll_result
Re: [PATCH] [media] ngene: fix dvb_pll_attach failure
On Fri, Dec 14, 2012 at 6:02 PM, Antti Palosaari wrote: > That one is better solution... > > but it is clearly DRXD driver bug - it should offer working I2C gate control > after attach() :-( I looked quickly DRXD driver and there is clearly some > other places to enhance too. But I suspect there is no maintainer, nor > interest from anyone to start fix things properly, so only reasonable way is > to add that hack to get it working... > > Honestly, I hate this kind of hacks :/ That makes our live hard on long run. > It goes slowly more and more hard to make any core changes as regressions > will happen due to this kind of hacks. > > So send new patch which put demod chip sleeping after tuner attach. Or even > better, find out what is minimal set of commands needed do execute during > attach in order to offer working I2C gate (I suspect firmware load is > needed). Opening the gate at the end of the attach callback should be a trivial exercise. Should just be a call to drxd_config_i2c(fe, enable) at the end of drxd_attach(). Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [media] ngene: fix dvb_pll_attach failure
On 12/15/2012 12:31 AM, Patrice Chotard wrote:
Le 14/12/2012 18:55, Antti Palosaari a écrit :
On 12/14/2012 07:28 PM, Patrice Chotard wrote:
Before dvb_pll_attch call, be sure that drxd demodulator was
initialized, otherwise, dvb_pll_attach() will always failed.
In dvb_pll_attach(), first thing done is to enable the I2C gate
control in order to probe the pll by performing a read access.
As demodulator was not initialized, every i2c access failed.
Reported-by: frederic.mantega...@gbiloba.org
Signed-off-by: Patrice Chotard
---
drivers/media/pci/ngene/ngene-cards.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/pci/ngene/ngene-cards.c
b/drivers/media/pci/ngene/ngene-cards.c
index 96a13ed..e2192db 100644
--- a/drivers/media/pci/ngene/ngene-cards.c
+++ b/drivers/media/pci/ngene/ngene-cards.c
@@ -328,6 +328,8 @@ static int demod_attach_drxd(struct ngene_channel
*chan)
return -ENODEV;
}
+/* initialized the DRXD demodulator */
+chan->fe->ops.init(chan->fe);
if (!dvb_attach(dvb_pll_attach, chan->fe, feconf->pll_address,
&chan->i2c_adapter,
feconf->pll_type)) {
I don't like that as this causes again more deviation against normal
procedures. If gate open is needed (for probe or id check?) then
pll/tuner attach should open it. If that is not easily possible then
calling gate_control() before pll attach is allowed. init() is very,
very, bad as generally starts whole chip => starts eating power etc.
Even better would be to let whole gate-control to responsibility of
DVB-core, but unfortunately current situation is quite mess. Gate is
operated sometimes by DVB-core (like for init/sleep) and for some cases
it is left for responsibility of tuner driver. So on real life there is
mixed solutions and for init/sleep gate is even double controlled.
regards
Antti
Hi Antti,
Unfortunately, opening gate doesn't work with drxd demodulator before
its initialization (drxd_init() call).
Also, in dvb_pll_attach(), first thing done is opening the gate control.
I was aware by calling init() directly in demod_attach_drxd() was not
good at the power consumption point of view, but it was the only
solution to make it work.
To stop the power consumption, it would be possible to call the frontends :
}
+ /* initialized the DRXD demodulator */
+ chan->fe->ops.init(chan->fe);
if (!dvb_attach(dvb_pll_attach, chan->fe, feconf->pll_address,
&chan->i2c_adapter,
feconf->pll_type)) {
pr_err("No pll(%d) found!\n", feconf->pll_type);
return -ENODEV;
}
+ chan->fe->ops.sleep(chan->fe);
return 0;
}
Thanks for your feedback.
That one is better solution...
but it is clearly DRXD driver bug - it should offer working I2C gate
control after attach() :-( I looked quickly DRXD driver and there is
clearly some other places to enhance too. But I suspect there is no
maintainer, nor interest from anyone to start fix things properly, so
only reasonable way is to add that hack to get it working...
Honestly, I hate this kind of hacks :/ That makes our live hard on long
run. It goes slowly more and more hard to make any core changes as
regressions will happen due to this kind of hacks.
So send new patch which put demod chip sleeping after tuner attach. Or
even better, find out what is minimal set of commands needed do execute
during attach in order to offer working I2C gate (I suspect firmware
load is needed).
regards
Antti
--
http://palosaari.fi/
--
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] [media] ngene: fix dvb_pll_attach failure
Le 14/12/2012 18:55, Antti Palosaari a écrit :
> On 12/14/2012 07:28 PM, Patrice Chotard wrote:
>> Before dvb_pll_attch call, be sure that drxd demodulator was
>> initialized, otherwise, dvb_pll_attach() will always failed.
>>
>> In dvb_pll_attach(), first thing done is to enable the I2C gate
>> control in order to probe the pll by performing a read access.
>> As demodulator was not initialized, every i2c access failed.
>>
>> Reported-by: frederic.mantega...@gbiloba.org
>> Signed-off-by: Patrice Chotard
>> ---
>> drivers/media/pci/ngene/ngene-cards.c |2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/pci/ngene/ngene-cards.c
>> b/drivers/media/pci/ngene/ngene-cards.c
>> index 96a13ed..e2192db 100644
>> --- a/drivers/media/pci/ngene/ngene-cards.c
>> +++ b/drivers/media/pci/ngene/ngene-cards.c
>> @@ -328,6 +328,8 @@ static int demod_attach_drxd(struct ngene_channel
>> *chan)
>> return -ENODEV;
>> }
>>
>> +/* initialized the DRXD demodulator */
>> +chan->fe->ops.init(chan->fe);
>> if (!dvb_attach(dvb_pll_attach, chan->fe, feconf->pll_address,
>> &chan->i2c_adapter,
>> feconf->pll_type)) {
>>
>
> I don't like that as this causes again more deviation against normal
> procedures. If gate open is needed (for probe or id check?) then
> pll/tuner attach should open it. If that is not easily possible then
> calling gate_control() before pll attach is allowed. init() is very,
> very, bad as generally starts whole chip => starts eating power etc.
>
>
> Even better would be to let whole gate-control to responsibility of
> DVB-core, but unfortunately current situation is quite mess. Gate is
> operated sometimes by DVB-core (like for init/sleep) and for some cases
> it is left for responsibility of tuner driver. So on real life there is
> mixed solutions and for init/sleep gate is even double controlled.
>
>
> regards
> Antti
>
Hi Antti,
Unfortunately, opening gate doesn't work with drxd demodulator before
its initialization (drxd_init() call).
Also, in dvb_pll_attach(), first thing done is opening the gate control.
I was aware by calling init() directly in demod_attach_drxd() was not
good at the power consumption point of view, but it was the only
solution to make it work.
To stop the power consumption, it would be possible to call the frontends :
}
+ /* initialized the DRXD demodulator */
+ chan->fe->ops.init(chan->fe);
if (!dvb_attach(dvb_pll_attach, chan->fe, feconf->pll_address,
&chan->i2c_adapter,
feconf->pll_type)) {
pr_err("No pll(%d) found!\n", feconf->pll_type);
return -ENODEV;
}
+ chan->fe->ops.sleep(chan->fe);
return 0;
}
Thanks for your feedback.
Regards
Patrice
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
cron job: media_tree daily build: ERRORS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date:Fri Dec 14 19:00:32 CET 2012 git hash:16427faf28674451a7a0485ab0a929402f355ffd gcc version: i686-linux-gcc (GCC) 4.7.1 host hardware:x86_64 host os: 3.4.07-marune linux-git-arm-eabi-davinci: WARNINGS linux-git-arm-eabi-exynos: OK linux-git-arm-eabi-omap: WARNINGS linux-git-i686: WARNINGS linux-git-m32r: OK linux-git-mips: WARNINGS linux-git-powerpc64: OK linux-git-sh: WARNINGS linux-git-x86_64: WARNINGS linux-2.6.31.12-i686: WARNINGS linux-2.6.32.6-i686: WARNINGS linux-2.6.33-i686: WARNINGS linux-2.6.34-i686: WARNINGS linux-2.6.35.3-i686: WARNINGS linux-2.6.36-i686: WARNINGS linux-2.6.37-i686: WARNINGS linux-2.6.38.2-i686: WARNINGS linux-2.6.39.1-i686: WARNINGS linux-3.0-i686: WARNINGS linux-3.1-i686: WARNINGS linux-3.2.1-i686: WARNINGS linux-3.3-i686: WARNINGS linux-3.4-i686: WARNINGS linux-3.5-i686: WARNINGS linux-3.6-i686: WARNINGS linux-3.7-i686: ERRORS linux-2.6.31.12-x86_64: WARNINGS linux-2.6.32.6-x86_64: WARNINGS linux-2.6.33-x86_64: WARNINGS linux-2.6.34-x86_64: WARNINGS linux-2.6.35.3-x86_64: WARNINGS linux-2.6.36-x86_64: WARNINGS linux-2.6.37-x86_64: WARNINGS linux-2.6.38.2-x86_64: WARNINGS linux-2.6.39.1-x86_64: WARNINGS linux-3.0-x86_64: WARNINGS linux-3.1-x86_64: WARNINGS linux-3.2.1-x86_64: WARNINGS linux-3.3-x86_64: WARNINGS linux-3.4-x86_64: WARNINGS linux-3.5-x86_64: WARNINGS linux-3.6-x86_64: WARNINGS linux-3.7-x86_64: ERRORS apps: WARNINGS spec-git: WARNINGS sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2 The V4L-DVB specification from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: em28xx: msi Digivox ATSC board id [0db0:8810]
Em Fri, 14 Dec 2012 16:33:34 +0100 Frank Schäfer escreveu: > Am 13.12.2012 21:23, schrieb Mauro Carvalho Chehab: > > Em Tue, 11 Dec 2012 22:59:06 +0200 > > Antti Palosaari escreveu: > > > >> On 12/11/2012 10:51 PM, Frank Schäfer wrote: > >>> Am 10.12.2012 21:48, schrieb Antti Palosaari: > On 12/10/2012 09:24 PM, Frank Schäfer wrote: > > Am 10.12.2012 18:57, schrieb Antti Palosaari: > > Specification comes here: > > NEC send always 32 bit, 4 bytes. There is 3 different "sub" protocols: > > > > 1) 16bit NEC standard, 1 byte address code, 1 byte key code > > full 4 byte code: AA BB CC DD > > where: > > AA = address code > > BB = ~address code > > CC = key code > > DD = ~key code > > > > checksum: > > AA + BB = 0xff > > CC + DD = 0xff > > > > 2) 24bit NEC extended, 2 byte address code, 1 byte key code > > full 4 byte code: AA BB CC DD > > where: > > AA = address code (MSB) > > BB = address code (LSB) > > CC = key code > > DD = ~key code > > > > 3) 32bit NEC full, 4 byte key code > > full 4 byte code: AA BB CC DD > > where: > > AA = > > BB = > > CC = > > DD = > > > > I am not sure if there is separate parts for address and key code in > > case of 32bit NEC. See some existing remote keytables if there is any > > such table. It is very rare protocol. 1) and 2) are much more common. > > > >>> Many thanks. > >>> So the problem is, that we have only a single RC_TYPE for all 3 protocol > >>> variants and need a method to distinguish between them, right ? > > This is not actually needed, as it is very easy to distinguish them when > > doing the table lookups. Take a look at v4l-utils, at > > /utils/keytable/rc_keymaps: > > > > A 16-bits NEC table: > > # table kworld_315u, type: NEC > > 0x6143 KEY_POWER > > 0x6101 KEY_VIDEO > > ... > > So 0x6143 is not the same as 0x006143 and 0x6143 ??? > > And even when assuming that 00 bytes are unused: I never seen that. > do you really think the > driver should parse the whole rc map and check all scancodes to find out > which sub-protocol is used ? Scancode search can use a b-tree (I think it currently does). In any case, the typical usage is that the IR table will match the IR device in usage. So, a not-found scancode just means that some error happened during the transmission. > > On a 24-bit NEC table: AA is always different than ~BB, otherwise, it would > > be a 16-bit NEC. > > No, if AA != ~BB it can't be 16 bit, but if AA == ~BB, it can still be > 16, 24 or 32bit ! Have you ever seen any remote using something like that??? The hole point of the IR address is to distinguish a given IR device from the others. The specs define specific addresses for VCR, TV Set, DVD, etc. So, no, if AA == ~BB it must be a 16 or 24 bits NEC protocol, as there's just 8 bits (or 16 bits) to differentiate the IR address for a given remote from other NEC IR addresses. > > On a 32-bit NEC table: CC is always different than ~DD, otherwise, it would > > be > > a 24-bit NEC. > > Right, if CC != ~DD it must be 32 bit. > > > So what if we get 52 AD 76 89 from the hardware ? This can be 32, 24 or > 16 bit ! It is 16 bits, except if someone at the manufacturer is completely senseless, and explicitly wants to cause troubles to their customers by using an IR address and IR commands that could be produced by some other vendor's remotes. In any case, the RC core will still support such crappy device, as the IR keytable can have a mix of 16 bits/24 bits/32 bits NEC codes inside it. We do have some tables with multiple IR's inside. For example, the Hauppauge RC5 table contains keycodes for 3 different types of Hauppauge controls. The same happens to one NEC's terratec table, where we're storing there both the codes of different IR models. The rationale is that the same board may be sold with different remotes. > Anyway, first we have to GET the bytes from the hardware. That's our > current problem ! > And the hardware seems to need a different setup for reg 0x50 for the > different NEC sub protocols. > Which means that the we need to know the sub protocol BEFORE we get any > bytes from the device. No. All em28xx needs is to make sure that the NEC protocol will return the full 32 bits scancode. Regards, Mauro -- 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/7] ir-rx51: Handle signals properly
* Timo Kokkonen [121214 11:33]: > On 12/14/12 19:26, Felipe Balbi wrote: > > > > if it's really for PWM, shouldn't we be using drivers/pwm/ ?? > > > > Now that Neil Brown posted the PWM driver for omap, I've been thinking > about whether converting the ir-rx51 into the PWM API would work. Maybe > controlling the PWM itself would be sufficient, but the ir-rx51 uses > also another dmtimer for creating accurate (enough) timing source for > the IR pulse edges. OK. > I haven't tried whether the default 32kHz clock source is enough for > that. Now that I think about it, I don't see why it wouldn't be good > enough. I think it would even be possible to just use the PWM api alone Cool. Regards, Tony -- 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/7] ir-rx51: Handle signals properly
On 12/14/12 19:26, Felipe Balbi wrote:
> Hi,
>
> On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote:
>> * Tony Lindgren [121120 12:00]:
>>> Hi,
>>>
>>> * Timo Kokkonen [121118 07:15]:
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
OMAP_TIMER_TRIGGER_NONE);
}
+static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
+{
+ if (lirc_rx51->wbuf_index < 0)
+ return;
+
+ lirc_rx51_off(lirc_rx51);
+ lirc_rx51->wbuf_index = -1;
+ omap_dm_timer_stop(lirc_rx51->pwm_timer);
+ omap_dm_timer_stop(lirc_rx51->pulse_timer);
+ omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
+ wake_up(&lirc_rx51->wqueue);
+}
+
static int init_timing_params(struct lirc_rx51 *lirc_rx51)
{
u32 load, match;
>>>
>>> Good fixes in general.. But you won't be able to access the
>>> omap_dm_timer functions after we enable ARM multiplatform support
>>> for omap2+. That's for v3.9 probably right after v3.8-rc1.
>>>
>>> We need to find some Linux generic API to use hardware timers
>>> like this, so I've added Thomas Gleixner and linux-arm-kernel
>>> mailing list to cc.
>>>
>>> If no such API is available, then maybe we can export some of
>>> the omap_dm_timer functions if Thomas is OK with that.
>>
>> Just to update the status on this.. It seems that we'll be moving
>> parts of plat/dmtimer into a minimal include/linux/timer-omap.h
>> unless people have better ideas on what to do with custom
>> hardware timers for PWM etc.
>
> if it's really for PWM, shouldn't we be using drivers/pwm/ ??
>
Now that Neil Brown posted the PWM driver for omap, I've been thinking
about whether converting the ir-rx51 into the PWM API would work. Maybe
controlling the PWM itself would be sufficient, but the ir-rx51 uses
also another dmtimer for creating accurate (enough) timing source for
the IR pulse edges.
I haven't tried whether the default 32kHz clock source is enough for
that. Now that I think about it, I don't see why it wouldn't be good
enough. I think it would even be possible to just use the PWM api alone
(plus hr-timers) in order to generate good enough IR pulses.
-Timo
> Meaning that $SUBJECT would just request a PWM device and use it. That
> doesn't solve the whole problem, however, as pwm-omap.c would still need
> access to timer-omap.h.
>
--
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/7] ir-rx51: Handle signals properly
Hi, On Fri, Dec 14, 2012 at 10:06:45AM -0800, Tony Lindgren wrote: > * Felipe Balbi [121214 09:59]: > > On Fri, Dec 14, 2012 at 09:46:29AM -0800, Tony Lindgren wrote: > > > * Felipe Balbi [121214 09:36]: > > > > > > > > if it's really for PWM, shouldn't we be using drivers/pwm/ ?? > > > > > > > > Meaning that $SUBJECT would just request a PWM device and use it. That > > > > doesn't solve the whole problem, however, as pwm-omap.c would still need > > > > access to timer-omap.h. > > > > > > That would only help with omap_dm_timer_set_pwm() I think. > > > > > > The other functions are also needed by the clocksource and clockevent > > > drivers. And tidspbridge too: > > > > well, we _do_ have drivers/clocksource ;-) > > That's where the dmtimer code should live. But still it does not help > with the header. yeah, the header should be where you suggested, no doubts. I was actually criticizing the current timer code. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/7] ir-rx51: Handle signals properly
* Felipe Balbi [121214 09:59]: > On Fri, Dec 14, 2012 at 09:46:29AM -0800, Tony Lindgren wrote: > > * Felipe Balbi [121214 09:36]: > > > > > > if it's really for PWM, shouldn't we be using drivers/pwm/ ?? > > > > > > Meaning that $SUBJECT would just request a PWM device and use it. That > > > doesn't solve the whole problem, however, as pwm-omap.c would still need > > > access to timer-omap.h. > > > > That would only help with omap_dm_timer_set_pwm() I think. > > > > The other functions are also needed by the clocksource and clockevent > > drivers. And tidspbridge too: > > well, we _do_ have drivers/clocksource ;-) That's where the dmtimer code should live. But still it does not help with the header. Thomas, maybe we could use the hrtimer framework for it if there was some way to completely leave out the rb tree for the dedicated hardware timers? There's no queue needed as there's always just one value tied to a specific timer. Regards, Tony -- 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/7] ir-rx51: Handle signals properly
Hi,
On Fri, Dec 14, 2012 at 09:46:29AM -0800, Tony Lindgren wrote:
> * Felipe Balbi [121214 09:36]:
> > Hi,
> >
> > On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote:
> > > * Tony Lindgren [121120 12:00]:
> > > > Hi,
> > > >
> > > > * Timo Kokkonen [121118 07:15]:
> > > > > --- a/drivers/media/rc/ir-rx51.c
> > > > > +++ b/drivers/media/rc/ir-rx51.c
> > > > > @@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51
> > > > > *lirc_rx51)
> > > > > OMAP_TIMER_TRIGGER_NONE);
> > > > > }
> > > > >
> > > > > +static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
> > > > > +{
> > > > > + if (lirc_rx51->wbuf_index < 0)
> > > > > + return;
> > > > > +
> > > > > + lirc_rx51_off(lirc_rx51);
> > > > > + lirc_rx51->wbuf_index = -1;
> > > > > + omap_dm_timer_stop(lirc_rx51->pwm_timer);
> > > > > + omap_dm_timer_stop(lirc_rx51->pulse_timer);
> > > > > + omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
> > > > > + wake_up(&lirc_rx51->wqueue);
> > > > > +}
> > > > > +
> > > > > static int init_timing_params(struct lirc_rx51 *lirc_rx51)
> > > > > {
> > > > > u32 load, match;
> > > >
> > > > Good fixes in general.. But you won't be able to access the
> > > > omap_dm_timer functions after we enable ARM multiplatform support
> > > > for omap2+. That's for v3.9 probably right after v3.8-rc1.
> > > >
> > > > We need to find some Linux generic API to use hardware timers
> > > > like this, so I've added Thomas Gleixner and linux-arm-kernel
> > > > mailing list to cc.
> > > >
> > > > If no such API is available, then maybe we can export some of
> > > > the omap_dm_timer functions if Thomas is OK with that.
> > >
> > > Just to update the status on this.. It seems that we'll be moving
> > > parts of plat/dmtimer into a minimal include/linux/timer-omap.h
> > > unless people have better ideas on what to do with custom
> > > hardware timers for PWM etc.
> >
> > if it's really for PWM, shouldn't we be using drivers/pwm/ ??
> >
> > Meaning that $SUBJECT would just request a PWM device and use it. That
> > doesn't solve the whole problem, however, as pwm-omap.c would still need
> > access to timer-omap.h.
>
> That would only help with omap_dm_timer_set_pwm() I think.
>
> The other functions are also needed by the clocksource and clockevent
> drivers. And tidspbridge too:
well, we _do_ have drivers/clocksource ;-)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] [media] ngene: fix dvb_pll_attach failure
On 12/14/2012 07:28 PM, Patrice Chotard wrote:
Before dvb_pll_attch call, be sure that drxd demodulator was
initialized, otherwise, dvb_pll_attach() will always failed.
In dvb_pll_attach(), first thing done is to enable the I2C gate
control in order to probe the pll by performing a read access.
As demodulator was not initialized, every i2c access failed.
Reported-by: frederic.mantega...@gbiloba.org
Signed-off-by: Patrice Chotard
---
drivers/media/pci/ngene/ngene-cards.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/pci/ngene/ngene-cards.c
b/drivers/media/pci/ngene/ngene-cards.c
index 96a13ed..e2192db 100644
--- a/drivers/media/pci/ngene/ngene-cards.c
+++ b/drivers/media/pci/ngene/ngene-cards.c
@@ -328,6 +328,8 @@ static int demod_attach_drxd(struct ngene_channel *chan)
return -ENODEV;
}
+ /* initialized the DRXD demodulator */
+ chan->fe->ops.init(chan->fe);
if (!dvb_attach(dvb_pll_attach, chan->fe, feconf->pll_address,
&chan->i2c_adapter,
feconf->pll_type)) {
I don't like that as this causes again more deviation against normal
procedures. If gate open is needed (for probe or id check?) then
pll/tuner attach should open it. If that is not easily possible then
calling gate_control() before pll attach is allowed. init() is very,
very, bad as generally starts whole chip => starts eating power etc.
Even better would be to let whole gate-control to responsibility of
DVB-core, but unfortunately current situation is quite mess. Gate is
operated sometimes by DVB-core (like for init/sleep) and for some cases
it is left for responsibility of tuner driver. So on real life there is
mixed solutions and for init/sleep gate is even double controlled.
regards
Antti
--
http://palosaari.fi/
--
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/7] ir-rx51: Handle signals properly
* Felipe Balbi [121214 09:36]:
> Hi,
>
> On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote:
> > * Tony Lindgren [121120 12:00]:
> > > Hi,
> > >
> > > * Timo Kokkonen [121118 07:15]:
> > > > --- a/drivers/media/rc/ir-rx51.c
> > > > +++ b/drivers/media/rc/ir-rx51.c
> > > > @@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51
> > > > *lirc_rx51)
> > > > OMAP_TIMER_TRIGGER_NONE);
> > > > }
> > > >
> > > > +static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
> > > > +{
> > > > + if (lirc_rx51->wbuf_index < 0)
> > > > + return;
> > > > +
> > > > + lirc_rx51_off(lirc_rx51);
> > > > + lirc_rx51->wbuf_index = -1;
> > > > + omap_dm_timer_stop(lirc_rx51->pwm_timer);
> > > > + omap_dm_timer_stop(lirc_rx51->pulse_timer);
> > > > + omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
> > > > + wake_up(&lirc_rx51->wqueue);
> > > > +}
> > > > +
> > > > static int init_timing_params(struct lirc_rx51 *lirc_rx51)
> > > > {
> > > > u32 load, match;
> > >
> > > Good fixes in general.. But you won't be able to access the
> > > omap_dm_timer functions after we enable ARM multiplatform support
> > > for omap2+. That's for v3.9 probably right after v3.8-rc1.
> > >
> > > We need to find some Linux generic API to use hardware timers
> > > like this, so I've added Thomas Gleixner and linux-arm-kernel
> > > mailing list to cc.
> > >
> > > If no such API is available, then maybe we can export some of
> > > the omap_dm_timer functions if Thomas is OK with that.
> >
> > Just to update the status on this.. It seems that we'll be moving
> > parts of plat/dmtimer into a minimal include/linux/timer-omap.h
> > unless people have better ideas on what to do with custom
> > hardware timers for PWM etc.
>
> if it's really for PWM, shouldn't we be using drivers/pwm/ ??
>
> Meaning that $SUBJECT would just request a PWM device and use it. That
> doesn't solve the whole problem, however, as pwm-omap.c would still need
> access to timer-omap.h.
That would only help with omap_dm_timer_set_pwm() I think.
The other functions are also needed by the clocksource and clockevent
drivers. And tidspbridge too:
$ grep -r omap_dm_timer drivers/
...
Regards,
Tony
--
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/7] ir-rx51: Handle signals properly
Hi,
On Fri, Dec 14, 2012 at 09:28:09AM -0800, Tony Lindgren wrote:
> * Tony Lindgren [121120 12:00]:
> > Hi,
> >
> > * Timo Kokkonen [121118 07:15]:
> > > --- a/drivers/media/rc/ir-rx51.c
> > > +++ b/drivers/media/rc/ir-rx51.c
> > > @@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
> > > OMAP_TIMER_TRIGGER_NONE);
> > > }
> > >
> > > +static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
> > > +{
> > > + if (lirc_rx51->wbuf_index < 0)
> > > + return;
> > > +
> > > + lirc_rx51_off(lirc_rx51);
> > > + lirc_rx51->wbuf_index = -1;
> > > + omap_dm_timer_stop(lirc_rx51->pwm_timer);
> > > + omap_dm_timer_stop(lirc_rx51->pulse_timer);
> > > + omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
> > > + wake_up(&lirc_rx51->wqueue);
> > > +}
> > > +
> > > static int init_timing_params(struct lirc_rx51 *lirc_rx51)
> > > {
> > > u32 load, match;
> >
> > Good fixes in general.. But you won't be able to access the
> > omap_dm_timer functions after we enable ARM multiplatform support
> > for omap2+. That's for v3.9 probably right after v3.8-rc1.
> >
> > We need to find some Linux generic API to use hardware timers
> > like this, so I've added Thomas Gleixner and linux-arm-kernel
> > mailing list to cc.
> >
> > If no such API is available, then maybe we can export some of
> > the omap_dm_timer functions if Thomas is OK with that.
>
> Just to update the status on this.. It seems that we'll be moving
> parts of plat/dmtimer into a minimal include/linux/timer-omap.h
> unless people have better ideas on what to do with custom
> hardware timers for PWM etc.
if it's really for PWM, shouldn't we be using drivers/pwm/ ??
Meaning that $SUBJECT would just request a PWM device and use it. That
doesn't solve the whole problem, however, as pwm-omap.c would still need
access to timer-omap.h.
--
balbi
signature.asc
Description: Digital signature
[PATCH] [media] ngene: fix dvb_pll_attach failure
Before dvb_pll_attch call, be sure that drxd demodulator was
initialized, otherwise, dvb_pll_attach() will always failed.
In dvb_pll_attach(), first thing done is to enable the I2C gate
control in order to probe the pll by performing a read access.
As demodulator was not initialized, every i2c access failed.
Reported-by: frederic.mantega...@gbiloba.org
Signed-off-by: Patrice Chotard
---
drivers/media/pci/ngene/ngene-cards.c |2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/media/pci/ngene/ngene-cards.c
b/drivers/media/pci/ngene/ngene-cards.c
index 96a13ed..e2192db 100644
--- a/drivers/media/pci/ngene/ngene-cards.c
+++ b/drivers/media/pci/ngene/ngene-cards.c
@@ -328,6 +328,8 @@ static int demod_attach_drxd(struct ngene_channel *chan)
return -ENODEV;
}
+ /* initialized the DRXD demodulator */
+ chan->fe->ops.init(chan->fe);
if (!dvb_attach(dvb_pll_attach, chan->fe, feconf->pll_address,
&chan->i2c_adapter,
feconf->pll_type)) {
--
1.7.10.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/7] ir-rx51: Handle signals properly
* Tony Lindgren [121120 12:00]:
> Hi,
>
> * Timo Kokkonen [121118 07:15]:
> > --- a/drivers/media/rc/ir-rx51.c
> > +++ b/drivers/media/rc/ir-rx51.c
> > @@ -74,6 +74,19 @@ static void lirc_rx51_off(struct lirc_rx51 *lirc_rx51)
> > OMAP_TIMER_TRIGGER_NONE);
> > }
> >
> > +static void lirc_rx51_stop_tx(struct lirc_rx51 *lirc_rx51)
> > +{
> > + if (lirc_rx51->wbuf_index < 0)
> > + return;
> > +
> > + lirc_rx51_off(lirc_rx51);
> > + lirc_rx51->wbuf_index = -1;
> > + omap_dm_timer_stop(lirc_rx51->pwm_timer);
> > + omap_dm_timer_stop(lirc_rx51->pulse_timer);
> > + omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
> > + wake_up(&lirc_rx51->wqueue);
> > +}
> > +
> > static int init_timing_params(struct lirc_rx51 *lirc_rx51)
> > {
> > u32 load, match;
>
> Good fixes in general.. But you won't be able to access the
> omap_dm_timer functions after we enable ARM multiplatform support
> for omap2+. That's for v3.9 probably right after v3.8-rc1.
>
> We need to find some Linux generic API to use hardware timers
> like this, so I've added Thomas Gleixner and linux-arm-kernel
> mailing list to cc.
>
> If no such API is available, then maybe we can export some of
> the omap_dm_timer functions if Thomas is OK with that.
Just to update the status on this.. It seems that we'll be moving
parts of plat/dmtimer into a minimal include/linux/timer-omap.h
unless people have better ideas on what to do with custom
hardware timers for PWM etc.
> The other alternative is to set them up as platform_data
> function pointers, but that won't work after we make omap2+
> device tree only. And that really just postpones the problem.
>
> Cheers,
>
> Tony
>
>
> > @@ -163,13 +176,7 @@ static irqreturn_t lirc_rx51_interrupt_handler(int
> > irq, void *ptr)
> >
> > return IRQ_HANDLED;
> > end:
> > - /* Stop TX here */
> > - lirc_rx51_off(lirc_rx51);
> > - lirc_rx51->wbuf_index = -1;
> > - omap_dm_timer_stop(lirc_rx51->pwm_timer);
> > - omap_dm_timer_stop(lirc_rx51->pulse_timer);
> > - omap_dm_timer_set_int_enable(lirc_rx51->pulse_timer, 0);
> > - wake_up_interruptible(&lirc_rx51->wqueue);
> > + lirc_rx51_stop_tx(lirc_rx51);
> >
> > return IRQ_HANDLED;
> > }
> > @@ -249,8 +256,9 @@ static ssize_t lirc_rx51_write(struct file *file, const
> > char *buf,
> > if ((count > WBUF_LEN) || (count % 2 == 0))
> > return -EINVAL;
> >
> > - /* Wait any pending transfers to finish */
> > - wait_event_interruptible(lirc_rx51->wqueue, lirc_rx51->wbuf_index < 0);
> > + /* We can have only one transmit at a time */
> > + if (lirc_rx51->wbuf_index >= 0)
> > + return -EBUSY;
> >
> > if (copy_from_user(lirc_rx51->wbuf, buf, n))
> > return -EFAULT;
> > @@ -276,9 +284,18 @@ static ssize_t lirc_rx51_write(struct file *file,
> > const char *buf,
> >
> > /*
> > * Don't return back to the userspace until the transfer has
> > -* finished
> > +* finished. However, we wish to not spend any more than 500ms
> > +* in kernel. No IR code TX should ever take that long.
> > +*/
> > + i = wait_event_timeout(lirc_rx51->wqueue, lirc_rx51->wbuf_index < 0,
> > + HZ / 2);
> > +
> > + /*
> > +* Ensure transmitting has really stopped, even if the timers
> > +* went mad or something else happened that caused it still
> > +* sending out something.
> > */
> > - wait_event_interruptible(lirc_rx51->wqueue, lirc_rx51->wbuf_index < 0);
> > + lirc_rx51_stop_tx(lirc_rx51);
> >
> > /* We can sleep again */
> > lirc_rx51->pdata->set_max_mpu_wakeup_lat(lirc_rx51->dev, -1);
> > --
> > 1.8.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" 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-omap" 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
Re: [PATCH 5/5] em28xx: fix+improve+unify i2c error handling, debug messages and code comments
On 12/14/2012 06:28 PM, Frank Schäfer wrote:
- check i2c slave address range (only 7 bit addresses supported)
- do not pass USB specific error codes to userspace/i2c-subsystem
- unify the returned error codes and make them compliant with
the i2c subsystem spec
- check number of actually transferred bytes (via USB) everywehere
- fix/improve debug messages
- improve code comments
Signed-off-by: Frank Schäfer
@@ -244,16 +294,20 @@ static int em28xx_i2c_xfer(struct i2c_adapter *i2c_adap,
dprintk2(2, "%s %s addr=%x len=%d:",
(msgs[i].flags & I2C_M_RD) ? "read" : "write",
i == num - 1 ? "stop" : "nonstop", addr, msgs[i].len);
+ if (addr > 0xff) {
+ dprintk2(2, " ERROR: 10 bit addresses not supported\n");
+ return -EOPNOTSUPP;
+ }
There is own flag for 10bit I2C address. Use it (and likely not compare
at all addr validly like that). This kind of address validation check is
quite unnecessary - and after all if it is wanted then correct place is
somewhere in I2C routines.
regards
Antti
--
http://palosaari.fi/
--
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 2/5] em28xx: respect the message size constraints for i2c transfers
On 12/14/2012 06:28 PM, Frank Schäfer wrote: The em2800 can transfer up to 4 bytes per i2c message. All other em25xx/em27xx/28xx chips can transfer at least 64 bytes per message. I2C adapters should never split messages transferred via the I2C subsystem into multiple message transfers, because the result will almost always NOT be the same as when the whole data is transferred to the I2C client in a single message. If the message size exceeds the capabilities of the I2C adapter, -EOPNOTSUPP should be returned. Signed-off-by: Frank Schäfer + if (len < 1 || len > 4) + return -EOPNOTSUPP; That patch seem to be good for my eyes, but that check for len < 1 is something I would like to double checked. Generally len = 0 is OK and is used some cases, probing and sometimes when all registers are read for example. Did you test it returns some error for zero len messages? regards Antti -- http://palosaari.fi/ -- 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: em28xx: msi Digivox ATSC board id [0db0:8810]
On 12/14/2012 06:32 PM, Antti Palosaari wrote:
On 12/14/2012 05:33 PM, Frank Schäfer wrote:
Am 13.12.2012 21:23, schrieb Mauro Carvalho Chehab:
Em Tue, 11 Dec 2012 22:59:06 +0200
Antti Palosaari escreveu:
On 12/11/2012 10:51 PM, Frank Schäfer wrote:
Am 10.12.2012 21:48, schrieb Antti Palosaari:
On 12/10/2012 09:24 PM, Frank Schäfer wrote:
Am 10.12.2012 18:57, schrieb Antti Palosaari:
Specification comes here:
NEC send always 32 bit, 4 bytes. There is 3 different "sub"
protocols:
1) 16bit NEC standard, 1 byte address code, 1 byte key code
full 4 byte code: AA BB CC DD
where:
AA = address code
BB = ~address code
CC = key code
DD = ~key code
checksum:
AA + BB = 0xff
CC + DD = 0xff
2) 24bit NEC extended, 2 byte address code, 1 byte key code
full 4 byte code: AA BB CC DD
where:
AA = address code (MSB)
BB = address code (LSB)
CC = key code
DD = ~key code
3) 32bit NEC full, 4 byte key code
full 4 byte code: AA BB CC DD
where:
AA =
BB =
CC =
DD =
I am not sure if there is separate parts for address and key code in
case of 32bit NEC. See some existing remote keytables if there is
any
such table. It is very rare protocol. 1) and 2) are much more
common.
Many thanks.
So the problem is, that we have only a single RC_TYPE for all 3
protocol
variants and need a method to distinguish between them, right ?
This is not actually needed, as it is very easy to distinguish them when
doing the table lookups. Take a look at v4l-utils, at
/utils/keytable/rc_keymaps:
A 16-bits NEC table:
# table kworld_315u, type: NEC
0x6143 KEY_POWER
0x6101 KEY_VIDEO
...
So 0x6143 is not the same as 0x006143 and 0x6143 ???
And even when assuming that 00 bytes are unused: do you really think the
driver should parse the whole rc map and check all scancodes to find out
which sub-protocol is used ?
A 24-bits NEC table:
# table pixelview_002t, type: NEC
0x866b13 KEY_MUTE
0x866b12 KEY_POWER2
...
A 32-bits NEC table:
# table tivo, type: NEC
0xa10c900f KEY_MEDIA
0xa10c0807 KEY_POWER2
...
If you see there, there's no way for the Kernel to handle it wrong, as
there's an implicit rule when dealing with "extended NEC" protocols:
Being the IR code being given by: AA BB CC DD
On a 24-bit NEC table: AA is always different than ~BB, otherwise, it
would
be a 16-bit NEC.
No, if AA != ~BB it can't be 16 bit, but if AA == ~BB, it can still be
16, 24 or 32bit !
On a 32-bit NEC table: CC is always different than ~DD, otherwise, it
would be
a 24-bit NEC.
Right, if CC != ~DD it must be 32 bit.
So what if we get 52 AD 76 89 from the hardware ? This can be 32, 24 or
16 bit !
Anyway, first we have to GET the bytes from the hardware. That's our
current problem !
And the hardware seems to need a different setup for reg 0x50 for the
different NEC sub protocols.
Which means that the we need to know the sub protocol BEFORE we get any
bytes from the device.
I don't understand you. As to prove this possible, I made simple test.
Patch attached.
Tested with two devices, em2884 and em28174. Here are the results:
rc-anysee.c:{ 0x0800, KEY_0 },
em28xx_rc: 81 08 f7 01 fe
em28xx_rc: 01 02 bd 00 ff
rc-terratec-slim.c:{ 0x02bd09, KEY_0 },
Ooops, I copy&pasted wrong buttons, key 1 as key 0 was taken from the
maps. Here is correct debug outputs matching given key maps.
em28xx_rc: 81 08 f7 00 ff
em28xx_rc: 01 02 bd 09 f6
for my eyes the results, output from the hardware, is just what we want.
I will not change my opinion until you prove I made some mistake and it
could not work. This is just similar what goes to af9015 and could be
implemented similarly in order to make it general NEC IR receiver.
RC-core stupid key binding with all the variants is another stupid issue
which should be fixed by converting all the key maps to 32bit format and
use only it. IMHO.
regards
Antti
--
http://palosaari.fi/
--
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: em28xx: msi Digivox ATSC board id [0db0:8810]
On 12/14/2012 05:33 PM, Frank Schäfer wrote:
Am 13.12.2012 21:23, schrieb Mauro Carvalho Chehab:
Em Tue, 11 Dec 2012 22:59:06 +0200
Antti Palosaari escreveu:
On 12/11/2012 10:51 PM, Frank Schäfer wrote:
Am 10.12.2012 21:48, schrieb Antti Palosaari:
On 12/10/2012 09:24 PM, Frank Schäfer wrote:
Am 10.12.2012 18:57, schrieb Antti Palosaari:
Specification comes here:
NEC send always 32 bit, 4 bytes. There is 3 different "sub" protocols:
1) 16bit NEC standard, 1 byte address code, 1 byte key code
full 4 byte code: AA BB CC DD
where:
AA = address code
BB = ~address code
CC = key code
DD = ~key code
checksum:
AA + BB = 0xff
CC + DD = 0xff
2) 24bit NEC extended, 2 byte address code, 1 byte key code
full 4 byte code: AA BB CC DD
where:
AA = address code (MSB)
BB = address code (LSB)
CC = key code
DD = ~key code
3) 32bit NEC full, 4 byte key code
full 4 byte code: AA BB CC DD
where:
AA =
BB =
CC =
DD =
I am not sure if there is separate parts for address and key code in
case of 32bit NEC. See some existing remote keytables if there is any
such table. It is very rare protocol. 1) and 2) are much more common.
Many thanks.
So the problem is, that we have only a single RC_TYPE for all 3 protocol
variants and need a method to distinguish between them, right ?
This is not actually needed, as it is very easy to distinguish them when
doing the table lookups. Take a look at v4l-utils, at
/utils/keytable/rc_keymaps:
A 16-bits NEC table:
# table kworld_315u, type: NEC
0x6143 KEY_POWER
0x6101 KEY_VIDEO
...
So 0x6143 is not the same as 0x006143 and 0x6143 ???
And even when assuming that 00 bytes are unused: do you really think the
driver should parse the whole rc map and check all scancodes to find out
which sub-protocol is used ?
A 24-bits NEC table:
# table pixelview_002t, type: NEC
0x866b13 KEY_MUTE
0x866b12 KEY_POWER2
...
A 32-bits NEC table:
# table tivo, type: NEC
0xa10c900f KEY_MEDIA
0xa10c0807 KEY_POWER2
...
If you see there, there's no way for the Kernel to handle it wrong, as
there's an implicit rule when dealing with "extended NEC" protocols:
Being the IR code being given by: AA BB CC DD
On a 24-bit NEC table: AA is always different than ~BB, otherwise, it would
be a 16-bit NEC.
No, if AA != ~BB it can't be 16 bit, but if AA == ~BB, it can still be
16, 24 or 32bit !
On a 32-bit NEC table: CC is always different than ~DD, otherwise, it would be
a 24-bit NEC.
Right, if CC != ~DD it must be 32 bit.
So what if we get 52 AD 76 89 from the hardware ? This can be 32, 24 or
16 bit !
Anyway, first we have to GET the bytes from the hardware. That's our
current problem !
And the hardware seems to need a different setup for reg 0x50 for the
different NEC sub protocols.
Which means that the we need to know the sub protocol BEFORE we get any
bytes from the device.
I don't understand you. As to prove this possible, I made simple test.
Patch attached.
Tested with two devices, em2884 and em28174. Here are the results:
rc-anysee.c:{ 0x0800, KEY_0 },
em28xx_rc: 81 08 f7 01 fe
em28xx_rc: 01 02 bd 00 ff
rc-terratec-slim.c: { 0x02bd09, KEY_0 },
for my eyes the results, output from the hardware, is just what we want.
I will not change my opinion until you prove I made some mistake and it
could not work. This is just similar what goes to af9015 and could be
implemented similarly in order to make it general NEC IR receiver.
RC-core stupid key binding with all the variants is another stupid issue
which should be fixed by converting all the key maps to 32bit format and
use only it. IMHO.
regards
Antti
--
http://palosaari.fi/
diff --git a/drivers/media/usb/em28xx/em28xx-input.c b/drivers/media/usb/em28xx/em28xx-input.c
index 660bf80..d26c5f7 100644
--- a/drivers/media/usb/em28xx/em28xx-input.c
+++ b/drivers/media/usb/em28xx/em28xx-input.c
@@ -257,6 +257,8 @@ static int em2874_polling_getkey(struct em28xx_IR *ir,
*/
rc = dev->em28xx_read_reg_req_len(dev, 0, EM2874_R51_IR,
msg, sizeof(msg));
+
+ pr_info("%s: %*ph\n", KBUILD_MODNAME, 5, &msg);
if (rc < 0)
return rc;
@@ -352,6 +354,8 @@ static int em28xx_ir_change_protocol(struct rc_dev *rc_dev, u64 *rc_type)
struct em28xx *dev = ir->dev;
u8 ir_config = EM2874_IR_RC5;
+ *rc_type = RC_BIT_NEC;
+
/* Adjust xclk based o IR table for RC5/NEC tables */
if (*rc_type & RC_BIT_RC5) {
@@ -369,6 +373,8 @@ static int em28xx_ir_change_protocol(struct rc_dev *rc_dev, u64 *rc_type)
em28xx_write_reg_bits(dev, EM28XX_R0F_XCLK, dev->board.xclk,
EM28XX_XCLK_IR_RC5_MODE);
+ ir_config = 0x01; // 32bit NEC
+
/* Setup the proper handler based on the chip */
switch (dev->chip_id) {
case CHIP_ID_EM2860:
[PATCH 4/5] em28xx: fix the i2c adapter functionality flags
I2C_FUNC_SMBUS_EMUL includes flag I2C_FUNC_SMBUS_WRITE_BLOCK_DATA which signals
that up to 31 data bytes can be written to the ic2 client.
But the EM2800 supports only i2c messages with max. 4 data bytes.
I2C_FUNC_IC2 should be set if a master_xfer fucntion pointer is provided in
struct i2c_algorithm.
Signed-off-by: Frank Schäfer
---
drivers/media/usb/em28xx/em28xx-i2c.c |6 +-
1 Datei geändert, 5 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 940ff4d..7118535 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -445,7 +445,11 @@ static int em28xx_i2c_eeprom(struct em28xx *dev, unsigned
char *eedata, int len)
*/
static u32 functionality(struct i2c_adapter *adap)
{
- return I2C_FUNC_SMBUS_EMUL;
+ struct em28xx *dev = adap->algo_data;
+ u32 func_flags = I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+ if (dev->board.is_em2800)
+ func_flags &= ~I2C_FUNC_SMBUS_WRITE_BLOCK_DATA;
+ return func_flags;
}
static struct i2c_algorithm em28xx_algo = {
--
1.7.10.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
[PATCH 5/5] em28xx: fix+improve+unify i2c error handling, debug messages and code comments
- check i2c slave address range (only 7 bit addresses supported)
- do not pass USB specific error codes to userspace/i2c-subsystem
- unify the returned error codes and make them compliant with
the i2c subsystem spec
- check number of actually transferred bytes (via USB) everywehere
- fix/improve debug messages
- improve code comments
Signed-off-by: Frank Schäfer
---
drivers/media/usb/em28xx/em28xx-core.c |5 +-
drivers/media/usb/em28xx/em28xx-i2c.c | 116 +++-
2 Dateien geändert, 89 Zeilen hinzugefügt(+), 32 Zeilen entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-core.c
b/drivers/media/usb/em28xx/em28xx-core.c
index c78d38b..cd808bf 100644
--- a/drivers/media/usb/em28xx/em28xx-core.c
+++ b/drivers/media/usb/em28xx/em28xx-core.c
@@ -101,7 +101,7 @@ int em28xx_read_reg_req_len(struct em28xx *dev, u8 req, u16
reg,
if (reg_debug)
printk(" failed!\n");
mutex_unlock(&dev->ctrl_urb_lock);
- return ret;
+ return usb_translate_errors(ret);
}
if (len)
@@ -182,6 +182,9 @@ int em28xx_write_regs_req(struct em28xx *dev, u8 req, u16
reg, char *buf,
0x, reg, dev->urb_buf, len, HZ);
mutex_unlock(&dev->ctrl_urb_lock);
+ if (ret < 0)
+ return usb_translate_errors(ret);
+
if (dev->wait_after_write)
msleep(dev->wait_after_write);
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 7118535..508c3e1 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -76,18 +76,26 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8
addr, u8 *buf, u16 len)
/* trigger write */
ret = dev->em28xx_write_regs(dev, 4 - len, &b2[4 - len], 2 + len);
if (ret != 2 + len) {
- em28xx_warn("writing to i2c device failed (error=%i)\n", ret);
- return -EIO;
+ em28xx_warn("failed to trigger write to i2c address 0x%x "
+ "(error=%i)\n", addr, ret);
+ return (ret < 0) ? ret : -EIO;
}
/* wait for completion */
for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0;
write_timeout -= 5) {
ret = dev->em28xx_read_reg(dev, 0x05);
- if (ret == 0x80 + len - 1)
+ if (ret == 0x80 + len - 1) {
return len;
+ } else if (ret == 0x94 + len - 1) {
+ return -ENODEV;
+ } else if (ret < 0) {
+ em28xx_warn("failed to get i2c transfer status from "
+ "bridge register (error=%i)\n", ret);
+ return ret;
+ }
msleep(5);
}
- em28xx_warn("i2c write timed out\n");
+ em28xx_warn("write to i2c device at 0x%x timed out\n", addr);
return -EIO;
}
@@ -168,24 +176,46 @@ static int em2800_i2c_check_for_device(struct em28xx
*dev, u8 addr)
static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
u16 len, int stop)
{
- int wrcount = 0;
int write_timeout, ret;
if (len < 1 || len > 64)
return -EOPNOTSUPP;
- wrcount = dev->em28xx_write_regs_req(dev, stop ? 2 : 3, addr, buf, len);
+ /* Write to i2c device */
+ ret = dev->em28xx_write_regs_req(dev, stop ? 2 : 3, addr, buf, len);
+ if (ret != len) {
+ if (ret < 0) {
+ em28xx_warn("writing to i2c device at 0x%x failed "
+ "(error=%i)\n", addr, ret);
+ return ret;
+ } else {
+ em28xx_warn("%i bytes write to i2c device at 0x%x "
+ "requested, but %i bytes written\n",
+ len, addr, ret);
+ return -EIO;
+ }
+ }
- /* Seems to be required after a write */
+ /* Check success of the i2c operation */
for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0;
write_timeout -= 5) {
ret = dev->em28xx_read_reg(dev, 0x05);
- if (!ret)
- break;
+ if (ret == 0) { /* success */
+ return len;
+ } else if (ret == 0x10) {
+ return -ENODEV;
+ } else if (ret < 0) {
+ em28xx_warn("failed to read i2c transfer status from "
+ "bridge (error=%i)\n", ret);
+ return ret;
+ }
msleep(5);
+ /* NOTE: do we really have to wait for success ?
+ Never seen anything else than 0x00 or 0x10
+
[PATCH 3/5] em28xx: fix two severe bugs in function em2800_i2c_recv_bytes()
Function em2800_i2c_recv_bytes() has 2 severe bugs:
1) It does not wait for the i2c read to complete before reading the received
message content from the bridge registers
2) Reading more than 1 byte doesn't work
The former can result in data corruption, the latter always does.
The rewritten code also superseds the content of function
em2800_i2c_check_for_device().
Tested with device "Terratec Cinergy 200 USB".
Signed-off-by: Frank Schäfer
---
drivers/media/usb/em28xx/em28xx-i2c.c | 104 ++---
drivers/media/usb/em28xx/em28xx.h |2 +-
2 Dateien geändert, 58 Zeilen hinzugefügt(+), 48 Zeilen entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index c508c12..940ff4d 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -73,12 +73,14 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8
addr, u8 *buf, u16 len)
if (len > 3)
b2[0] = buf[3];
+ /* trigger write */
ret = dev->em28xx_write_regs(dev, 4 - len, &b2[4 - len], 2 + len);
if (ret != 2 + len) {
em28xx_warn("writing to i2c device failed (error=%i)\n", ret);
return -EIO;
}
- for (write_timeout = EM2800_I2C_WRITE_TIMEOUT; write_timeout > 0;
+ /* wait for completion */
+ for (write_timeout = EM2800_I2C_XFER_TIMEOUT; write_timeout > 0;
write_timeout -= 5) {
ret = dev->em28xx_read_reg(dev, 0x05);
if (ret == 0x80 + len - 1)
@@ -90,66 +92,74 @@ static int em2800_i2c_send_bytes(struct em28xx *dev, u8
addr, u8 *buf, u16 len)
}
/*
- * em2800_i2c_check_for_device()
- * check if there is a i2c_device at the supplied address
+ * em2800_i2c_recv_bytes()
+ * read up to 4 bytes from the em2800 i2c device
*/
-static int em2800_i2c_check_for_device(struct em28xx *dev, u8 addr)
+static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
{
- u8 msg;
+ u8 buf2[4];
int ret;
- int write_timeout;
- msg = addr;
- ret = dev->em28xx_write_regs(dev, 0x04, &msg, 1);
- if (ret < 0) {
- em28xx_warn("setting i2c device address failed (error=%i)\n",
- ret);
- return ret;
- }
- msg = 0x84;
- ret = dev->em28xx_write_regs(dev, 0x05, &msg, 1);
- if (ret < 0) {
- em28xx_warn("preparing i2c read failed (error=%i)\n", ret);
- return ret;
+ int read_timeout;
+ int i;
+
+ if (len < 1 || len > 4)
+ return -EOPNOTSUPP;
+
+ /* trigger read */
+ buf2[1] = 0x84 + len - 1;
+ buf2[0] = addr;
+ ret = dev->em28xx_write_regs(dev, 0x04, buf2, 2);
+ if (ret != 2) {
+ em28xx_warn("failed to trigger read from i2c address 0x%x "
+ "(error=%i)\n", addr, ret);
+ return (ret < 0) ? ret : -EIO;
}
- for (write_timeout = EM2800_I2C_WRITE_TIMEOUT; write_timeout > 0;
-write_timeout -= 5) {
- unsigned reg = dev->em28xx_read_reg(dev, 0x5);
- if (reg == 0x94)
+ /* wait for completion */
+ for (read_timeout = EM2800_I2C_XFER_TIMEOUT; read_timeout > 0;
+read_timeout -= 5) {
+ ret = dev->em28xx_read_reg(dev, 0x05);
+ if (ret == 0x84 + len - 1) {
+ break;
+ } else if (ret == 0x94 + len - 1) {
return -ENODEV;
- else if (reg == 0x84)
- return 0;
+ } else if (ret < 0) {
+ em28xx_warn("failed to get i2c transfer status from "
+ "bridge register (error=%i)\n", ret);
+ return ret;
+ }
msleep(5);
}
- return -ENODEV;
+ if (ret != 0x84 + len - 1)
+ em28xx_warn("read from i2c device at 0x%x timed out\n", addr);
+
+ /* get the received message */
+ ret = dev->em28xx_read_reg_req_len(dev, 0x00, 4-len, buf2, len);
+ if (ret != len) {
+ em28xx_warn("reading from i2c device at 0x%x failed: "
+ "couldn't get the received message from the bridge "
+ "(error=%i)\n", addr, ret);
+ return (ret < 0) ? ret : -EIO;
+ }
+ for (i=0; i 4)
- return -EOPNOTSUPP;
-
- /* check for the device and set i2c read address */
- ret = em2800_i2c_check_for_device(dev, addr);
- if (ret) {
- em28xx_warn
- ("preparing read at i2c address 0x%x failed (error=%i)\n",
-addr, ret);
- return ret;
- }
- ret = dev->em28xx_read_reg_req_len(dev, 0x0, 0x3, buf, len);
- if (ret < 0) {
- em28xx_warn("reading from
[PATCH 2/5] em28xx: respect the message size constraints for i2c transfers
The em2800 can transfer up to 4 bytes per i2c message.
All other em25xx/em27xx/28xx chips can transfer at least 64 bytes per message.
I2C adapters should never split messages transferred via the I2C subsystem
into multiple message transfers, because the result will almost always NOT be
the same as when the whole data is transferred to the I2C client in a single
message.
If the message size exceeds the capabilities of the I2C adapter, -EOPNOTSUPP
should be returned.
Signed-off-by: Frank Schäfer
---
drivers/media/usb/em28xx/em28xx-i2c.c | 44 ++---
1 Datei geändert, 18 Zeilen hinzugefügt(+), 26 Zeilen entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 44533e4..c508c12 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -50,14 +50,18 @@ do {
\
} while (0)
/*
- * em2800_i2c_send_max4()
- * send up to 4 bytes to the i2c device
+ * em2800_i2c_send_bytes()
+ * send up to 4 bytes to the em2800 i2c device
*/
-static int em2800_i2c_send_max4(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
+static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
{
int ret;
int write_timeout;
u8 b2[6];
+
+ if (len < 1 || len > 4)
+ return -EOPNOTSUPP;
+
BUG_ON(len < 1 || len > 4);
b2[5] = 0x80 + len - 1;
b2[4] = addr;
@@ -86,29 +90,6 @@ static int em2800_i2c_send_max4(struct em28xx *dev, u8 addr,
u8 *buf, u16 len)
}
/*
- * em2800_i2c_send_bytes()
- */
-static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
-{
- u8 *bufPtr = buf;
- int ret;
- int wrcount = 0;
- int count;
- int maxLen = 4;
- while (len > 0) {
- count = (len > maxLen) ? maxLen : len;
- ret = em2800_i2c_send_max4(dev, addr, bufPtr, count);
- if (ret > 0) {
- len -= count;
- bufPtr += count;
- wrcount += count;
- } else
- return (ret < 0) ? ret : -EFAULT;
- }
- return wrcount;
-}
-
-/*
* em2800_i2c_check_for_device()
* check if there is a i2c_device at the supplied address
*/
@@ -150,6 +131,10 @@ static int em2800_i2c_check_for_device(struct em28xx *dev,
u8 addr)
static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
{
int ret;
+
+ if (len < 1 || len > 4)
+ return -EOPNOTSUPP;
+
/* check for the device and set i2c read address */
ret = em2800_i2c_check_for_device(dev, addr);
if (ret) {
@@ -176,6 +161,9 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16
addr, u8 *buf,
int wrcount = 0;
int write_timeout, ret;
+ if (len < 1 || len > 64)
+ return -EOPNOTSUPP;
+
wrcount = dev->em28xx_write_regs_req(dev, stop ? 2 : 3, addr, buf, len);
/* Seems to be required after a write */
@@ -197,6 +185,10 @@ static int em28xx_i2c_send_bytes(struct em28xx *dev, u16
addr, u8 *buf,
static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16
len)
{
int ret;
+
+ if (len < 1 || len > 64)
+ return -EOPNOTSUPP;
+
ret = dev->em28xx_read_reg_req_len(dev, 2, addr, buf, len);
if (ret < 0) {
em28xx_warn("reading i2c device failed (error=%i)\n", ret);
--
1.7.10.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
[PATCH 1/5] em28xx: clean up the data type mess of the i2c transfer function parameters
Signed-off-by: Frank Schäfer
---
drivers/media/usb/em28xx/em28xx-i2c.c | 28 +++-
1 Datei geändert, 11 Zeilen hinzugefügt(+), 17 Zeilen entfernt(-)
diff --git a/drivers/media/usb/em28xx/em28xx-i2c.c
b/drivers/media/usb/em28xx/em28xx-i2c.c
index 1683bd9..44533e4 100644
--- a/drivers/media/usb/em28xx/em28xx-i2c.c
+++ b/drivers/media/usb/em28xx/em28xx-i2c.c
@@ -53,12 +53,11 @@ do {
\
* em2800_i2c_send_max4()
* send up to 4 bytes to the i2c device
*/
-static int em2800_i2c_send_max4(struct em28xx *dev, unsigned char addr,
- char *buf, int len)
+static int em2800_i2c_send_max4(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
{
int ret;
int write_timeout;
- unsigned char b2[6];
+ u8 b2[6];
BUG_ON(len < 1 || len > 4);
b2[5] = 0x80 + len - 1;
b2[4] = addr;
@@ -89,15 +88,13 @@ static int em2800_i2c_send_max4(struct em28xx *dev,
unsigned char addr,
/*
* em2800_i2c_send_bytes()
*/
-static int em2800_i2c_send_bytes(void *data, unsigned char addr, char *buf,
-short len)
+static int em2800_i2c_send_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
{
- char *bufPtr = buf;
+ u8 *bufPtr = buf;
int ret;
int wrcount = 0;
int count;
int maxLen = 4;
- struct em28xx *dev = (struct em28xx *)data;
while (len > 0) {
count = (len > maxLen) ? maxLen : len;
ret = em2800_i2c_send_max4(dev, addr, bufPtr, count);
@@ -115,9 +112,9 @@ static int em2800_i2c_send_bytes(void *data, unsigned char
addr, char *buf,
* em2800_i2c_check_for_device()
* check if there is a i2c_device at the supplied address
*/
-static int em2800_i2c_check_for_device(struct em28xx *dev, unsigned char addr)
+static int em2800_i2c_check_for_device(struct em28xx *dev, u8 addr)
{
- char msg;
+ u8 msg;
int ret;
int write_timeout;
msg = addr;
@@ -150,8 +147,7 @@ static int em2800_i2c_check_for_device(struct em28xx *dev,
unsigned char addr)
* em2800_i2c_recv_bytes()
* read from the i2c device
*/
-static int em2800_i2c_recv_bytes(struct em28xx *dev, unsigned char addr,
-char *buf, int len)
+static int em2800_i2c_recv_bytes(struct em28xx *dev, u8 addr, u8 *buf, u16 len)
{
int ret;
/* check for the device and set i2c read address */
@@ -174,11 +170,10 @@ static int em2800_i2c_recv_bytes(struct em28xx *dev,
unsigned char addr,
/*
* em28xx_i2c_send_bytes()
*/
-static int em28xx_i2c_send_bytes(void *data, unsigned char addr, char *buf,
-short len, int stop)
+static int em28xx_i2c_send_bytes(struct em28xx *dev, u16 addr, u8 *buf,
+u16 len, int stop)
{
int wrcount = 0;
- struct em28xx *dev = (struct em28xx *)data;
int write_timeout, ret;
wrcount = dev->em28xx_write_regs_req(dev, stop ? 2 : 3, addr, buf, len);
@@ -199,8 +194,7 @@ static int em28xx_i2c_send_bytes(void *data, unsigned char
addr, char *buf,
* em28xx_i2c_recv_bytes()
* read a byte from the i2c device
*/
-static int em28xx_i2c_recv_bytes(struct em28xx *dev, unsigned char addr,
-char *buf, int len)
+static int em28xx_i2c_recv_bytes(struct em28xx *dev, u16 addr, u8 *buf, u16
len)
{
int ret;
ret = dev->em28xx_read_reg_req_len(dev, 2, addr, buf, len);
@@ -217,7 +211,7 @@ static int em28xx_i2c_recv_bytes(struct em28xx *dev,
unsigned char addr,
* em28xx_i2c_check_for_device()
* check if there is a i2c_device at the supplied address
*/
-static int em28xx_i2c_check_for_device(struct em28xx *dev, unsigned char addr)
+static int em28xx_i2c_check_for_device(struct em28xx *dev, u16 addr)
{
int ret;
--
1.7.10.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
[PATCH 0/5] em28xx: i2c bug fixes and cleanups
This patch series contains some I2C bug fixes / cleanups / unifications and improvements for the em28xx driver, which I've made while working on adding support for the em25xx/em276x i2c bus B support and playing with the Terratec Cinergy 200 USB which I've got recently. Patches 1 and 5 are cleanup/unification patches, patches 2, 3, 4 fix some bugs. Patch 3 actually fixes 2 bugs, but separate commits didn't make sense, because more or less the whole function had to be rewritten. Frank Schäfer (5): em28xx: clean up the data type mess of the i2c transfer function parameters em28xx: respect the message size constraints for i2c transfers em28xx: fix two severe bugs in function em2800_i2c_recv_bytes() em28xx: fix the i2c adapter functionality flags em28xx: fix+improve+unify i2c error handling, debug messages and code comments drivers/media/usb/em28xx/em28xx-core.c |5 +- drivers/media/usb/em28xx/em28xx-i2c.c | 280 +++- drivers/media/usb/em28xx/em28xx.h |2 +- 3 Dateien geändert, 172 Zeilen hinzugefügt(+), 115 Zeilen entfernt(-) -- 1.7.10.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 0/9] em28xx: refactor the frame data processing code
Am 14.12.2012 16:29, schrieb Devin Heitmueller: Yes, there will likely be heavy merge conflicts... In which tree are the videobuf2 patches ? >>> It's in a private tree right now, and it doesn't support VBI >>> currently. Once I've setup a public tree with yours and Hans changes, >>> I'll start merging in my changes. >> I suggest to do the conversion on top of my patches, as they should make >> things much easier for you. >> I unified the handling of the VBI and video buffers, leaving just a few >> common functions dealing with the videobuf stuff. > Yup, that's exactly what I had planned. > >> In any case, we should develop against a common tree with a minimum >> number of pending patches. >> And we should coordinate development. >> I don't work on further changes of the frame processing stuff at the moment. >> Some I2C fixes/changes will be next. After that, I will try to fix >> support for remote controls with external IR IC (connected via i2c). >> >>> Obviously it would be great for you to test with your webcam and make >>> sure I didn't break anything along the way. >> Sure, I will be glad to test your changes. >> >>> I've also got changes to support V4L2_FIELD_SEQ_TB, which is needed in >>> order to take the output and feed to certain hardware deinterlacers. >>> In reality this is pretty much just a matter of treating the video >>> data as progressive but changing the field type indicator. >> Ok, so I assume most of the changes will happen in em28xx_copy_video(). > The changes really are all over the tree because it's not just vb2 > support but also support for v4l2_fh, which means every ioctl() has a > change to its arguments, and there is no longer an open/close call > implemented. Also significant impact on the locking model. Ok. Sounds like a lot of fun... ;) If the changes are all over the tree, we will likely get more collisions. So we should both make our changes public as soon as possible. > >> Maybe we can then use a common copy function for video and VBI. Placing >> the field data sequentially in the videobuf is what we already do with >> the VBI data in em28xx_copy_vbi() > Let's get something that works, at which point we can tune/optimize as needed. I agree. Frank > > Devin > > -- > Devin J. Heitmueller - Kernel Labs > http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: em28xx: msi Digivox ATSC board id [0db0:8810]
Am 13.12.2012 21:23, schrieb Mauro Carvalho Chehab: > Em Tue, 11 Dec 2012 22:59:06 +0200 > Antti Palosaari escreveu: > >> On 12/11/2012 10:51 PM, Frank Schäfer wrote: >>> Am 10.12.2012 21:48, schrieb Antti Palosaari: On 12/10/2012 09:24 PM, Frank Schäfer wrote: > Am 10.12.2012 18:57, schrieb Antti Palosaari: > Specification comes here: > NEC send always 32 bit, 4 bytes. There is 3 different "sub" protocols: > > 1) 16bit NEC standard, 1 byte address code, 1 byte key code > full 4 byte code: AA BB CC DD > where: > AA = address code > BB = ~address code > CC = key code > DD = ~key code > > checksum: > AA + BB = 0xff > CC + DD = 0xff > > 2) 24bit NEC extended, 2 byte address code, 1 byte key code > full 4 byte code: AA BB CC DD > where: > AA = address code (MSB) > BB = address code (LSB) > CC = key code > DD = ~key code > > 3) 32bit NEC full, 4 byte key code > full 4 byte code: AA BB CC DD > where: > AA = > BB = > CC = > DD = > > I am not sure if there is separate parts for address and key code in > case of 32bit NEC. See some existing remote keytables if there is any > such table. It is very rare protocol. 1) and 2) are much more common. > >>> Many thanks. >>> So the problem is, that we have only a single RC_TYPE for all 3 protocol >>> variants and need a method to distinguish between them, right ? > This is not actually needed, as it is very easy to distinguish them when > doing the table lookups. Take a look at v4l-utils, at > /utils/keytable/rc_keymaps: > > A 16-bits NEC table: > # table kworld_315u, type: NEC > 0x6143 KEY_POWER > 0x6101 KEY_VIDEO > ... So 0x6143 is not the same as 0x006143 and 0x6143 ??? And even when assuming that 00 bytes are unused: do you really think the driver should parse the whole rc map and check all scancodes to find out which sub-protocol is used ? > A 24-bits NEC table: > # table pixelview_002t, type: NEC > 0x866b13 KEY_MUTE > 0x866b12 KEY_POWER2 > ... > > A 32-bits NEC table: > # table tivo, type: NEC > 0xa10c900f KEY_MEDIA > 0xa10c0807 KEY_POWER2 > ... > > If you see there, there's no way for the Kernel to handle it wrong, as > there's an implicit rule when dealing with "extended NEC" protocols: > > Being the IR code being given by: AA BB CC DD > > On a 24-bit NEC table: AA is always different than ~BB, otherwise, it would > be a 16-bit NEC. No, if AA != ~BB it can't be 16 bit, but if AA == ~BB, it can still be 16, 24 or 32bit ! > On a 32-bit NEC table: CC is always different than ~DD, otherwise, it would be > a 24-bit NEC. Right, if CC != ~DD it must be 32 bit. So what if we get 52 AD 76 89 from the hardware ? This can be 32, 24 or 16 bit ! Anyway, first we have to GET the bytes from the hardware. That's our current problem ! And the hardware seems to need a different setup for reg 0x50 for the different NEC sub protocols. Which means that the we need to know the sub protocol BEFORE we get any bytes from the device. Regards, Frank -- 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 0/9] em28xx: refactor the frame data processing code
>>> Yes, there will likely be heavy merge conflicts... >>> In which tree are the videobuf2 patches ? >> It's in a private tree right now, and it doesn't support VBI >> currently. Once I've setup a public tree with yours and Hans changes, >> I'll start merging in my changes. > > I suggest to do the conversion on top of my patches, as they should make > things much easier for you. > I unified the handling of the VBI and video buffers, leaving just a few > common functions dealing with the videobuf stuff. Yup, that's exactly what I had planned. > In any case, we should develop against a common tree with a minimum > number of pending patches. > And we should coordinate development. > I don't work on further changes of the frame processing stuff at the moment. > Some I2C fixes/changes will be next. After that, I will try to fix > support for remote controls with external IR IC (connected via i2c). > >> Obviously it would be great for you to test with your webcam and make >> sure I didn't break anything along the way. > > Sure, I will be glad to test your changes. > >> I've also got changes to support V4L2_FIELD_SEQ_TB, which is needed in >> order to take the output and feed to certain hardware deinterlacers. >> In reality this is pretty much just a matter of treating the video >> data as progressive but changing the field type indicator. > > Ok, so I assume most of the changes will happen in em28xx_copy_video(). The changes really are all over the tree because it's not just vb2 support but also support for v4l2_fh, which means every ioctl() has a change to its arguments, and there is no longer an open/close call implemented. Also significant impact on the locking model. > Maybe we can then use a common copy function for video and VBI. Placing > the field data sequentially in the videobuf is what we already do with > the VBI data in em28xx_copy_vbi() Let's get something that works, at which point we can tune/optimize as needed. Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/9] em28xx: refactor the frame data processing code
Am 13.12.2012 19:16, schrieb Devin Heitmueller: > On Thu, Dec 13, 2012 at 12:56 PM, Frank Schäfer > wrote: >> Am 13.12.2012 18:36, schrieb Devin Heitmueller: >>> On Sat, Dec 8, 2012 at 10:31 AM, Frank Schäfer >>> wrote: This patch series refactors the frame data processing code in em28xx-video.c to - reduce code duplication - fix a bug in vbi data processing - prepare for adding em25xx/em276x frame data processing support - clean up the code and make it easier to understand >>> Hi Frank, >>> >>> Do you have these patches in a git tree somewhere that I can do a git >>> pull from? If not then that's fine - I'll just save off the patches >>> and apply them by hand. >> No, I have no public git tree. >> >>> I've basically got your patches, fixes Hans did for v4l2 compliance, >>> and I've got a tree that converts the driver to videobuf2 (which >>> obviously heavily conflicts with the URB handler cleanup you did). >>> Plan is to suck them all into a single tree, deal with the merge >>> issues, then issue a pull request to Mauro. >> Ahhh, videobuf2 ! >> Good to know, because I've put this on my TODO list... ;) > It's harder than it looks. There are currently no other devices > ported to vb2 which have VBI and/or radio devices. Hence I have to > refactor the locking a bit (since the URB callback feeds two different > VB2 queues). In other words, there's no other driver to look at as a > model and copy the business logic from. Yeah, that's one of the reasons why I decided to do it later ;) > >> Yes, there will likely be heavy merge conflicts... >> In which tree are the videobuf2 patches ? > It's in a private tree right now, and it doesn't support VBI > currently. Once I've setup a public tree with yours and Hans changes, > I'll start merging in my changes. I suggest to do the conversion on top of my patches, as they should make things much easier for you. I unified the handling of the VBI and video buffers, leaving just a few common functions dealing with the videobuf stuff. In any case, we should develop against a common tree with a minimum number of pending patches. And we should coordinate development. I don't work on further changes of the frame processing stuff at the moment. Some I2C fixes/changes will be next. After that, I will try to fix support for remote controls with external IR IC (connected via i2c). > Obviously it would be great for you to test with your webcam and make > sure I didn't break anything along the way. Sure, I will be glad to test your changes. > I've also got changes to support V4L2_FIELD_SEQ_TB, which is needed in > order to take the output and feed to certain hardware deinterlacers. > In reality this is pretty much just a matter of treating the video > data as progressive but changing the field type indicator. Ok, so I assume most of the changes will happen in em28xx_copy_video(). Maybe we can then use a common copy function for video and VBI. Placing the field data sequentially in the videobuf is what we already do with the VBI data in em28xx_copy_vbi() Regards, Frank > I'm generally pretty easy to find in #linuxtv or #v4l if you want to > discuss further. > > Cheers, > > Devin > > -- > Devin J. Heitmueller - Kernel Labs > http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support
Op 14-12-12 15:11, Rob Clark schreef: > On Fri, Dec 14, 2012 at 5:57 AM, Maarten Lankhorst > wrote: >> Op 14-12-12 10:36, sumit.sem...@ti.com schreef: >>> From: Sumit Semwal >>> >>> Add debugfs support to make it easier to print debug information >>> about the dma-buf buffers. >>> >> I like the idea, I don't know if it could be done in a free manner, but for >> bonus points >> could we also have the dma-buf fd be obtainable that way from a debugfs >> entry? >> >> Doing so would allow me to 'steal' a dma-buf from an existing mapping >> easily, and test against that. >> >> Also I think the name of the device and process that exported the dma-buf >> would be useful >> to have as well, even if in case of the device that would mean changing the >> api slightly to record it. >> >> I was thinking of having a directory structure like this: >> >> /sys/kernel/debug/dma_buf/stats >> >> and then for each dma-buf: >> >> /sys/kernel/debug/dma-buf/exporting_file.c/-fd >> /sys/kernel/debug/dma-buf/exporting_file.c/-attachments >> /sys/kernel/debug/dma-buf/exporting_file.c/-info >> >> Opening the fd file would give you back the original fd, or fail with -EIO >> if refcount was dropped to 0. >> >> Would something like this be doable? I don't know debugfs that well, but I >> don't see why it wouldn't be, > yeah.. but sort of back-door's the security benefits of an anonymous fd.. > > BR, > -R If you have access to debugfs you're root, so what stops you from stealing it through a ptrace? ~Maarten -- 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
RFCv2: Second draft of guidelines for submitting patches to linux-media
Hi all, As discussed in Barcelona I would write a text describing requirements for new drivers and what to expect when submitting patches to linux-media. This is the second rough draft and nothing is fixed yet. I have incorporated all comments I received after I posted the first version, although I've rewritten some of the suggestions. I have a few open questions: 1) Where to put it? One thing I would propose that we improve is to move the dvb and video4linux directories in Documentation/ to Documentation/media to correctly reflect the drivers/media structure. If we do that, then we can put this document in Documentation/media/SubmittingMediaPatches. Alternatively, this is something we can document in our wiki. It was also suggested to add it to media_build.git, but I am opposed to that: media_build.git is only used to backport our code to older kernels, and that is rarely done when working with embedded drivers. So chances are developers will not be aware of media_build.git as they may never need it. 2) The patchwork section is very short at the moment. It should be extended when patchwork gets support to recognize the various tags. 3) Anything else that should be discussed here? Again, remember that this is a rough draft only, so be gentle with me :-) My vacation starts tomorrow, so I won't be very active on the mailinglist for the next three weeks, but don't hesitate to give feedback as I will go through it when I prepare v3 of this document. Regards, Hans --- cut here --- For general information on how to submit patches see: http://linuxtv.org/wiki/index.php/Developer_Section In particular the section 'Submitting Your Work'. This document goes into more detail regarding media specific requirements when submitting patches and what the patch flow looks like in this subsystem. Note 1: there are always exceptions to the rule, so if you believe certain requirements do not apply to your code, then let us know and we can discuss it. Note 2: this list is not exhaustive and will be updated over time, so make sure you always use the latest version of this document. The latest version will always be available here: TBD. Note 3: when submitting a patch use ./scripts/get_maintainer.pl to figure out who is maintaining the sources you touched. Submitting New Media Drivers When submitting new media drivers for inclusion in drivers/staging/media all that is required is that the driver compiles with the latest kernel, that an entry is added to the MAINTAINERS file, and that a TODO file is added with a list of action items that need to be taken before the driver can be moved to drivers/media. It should be noticed, however, that it is expected that the driver will be fixed to fulfill the requirements for upstream addition. If a driver at staging lacks relevant patches fixing it for more than a few kernel cycles, it can be dropped from staging. We will contact you before doing that provided that the email address of the maintainer is still valid. For inclusion as a non-staging driver the requirements are more strict: General requirements: - It must pass checkpatch.pl, but see the note regarding interpreting the output from checkpatch below. - An entry for the driver is added to the MAINTAINERS file. - The kernel internal APIs are used properly. - Don't reinvent the wheel by adding new defines, math logic, etc. for which there are already solutions in the kernel. - Follow the CodingStyle guidelines, paying specific attention to the follow frequently made mistakes: - Errors should be reported as negative numbers using the kernel error codes. See also the CodingStyle document, chapter 16. - Don't use typedefs. See also the CodingStyle document, chapter 5. V4L2 specific requirements: - Use struct v4l2_device for bridge drivers, use struct v4l2_subdev for sub-device drivers. - Each i2c/spi device should be implemented as a separate sub-device driver. - Use the control framework for control handling. - Use struct v4l2_fh if the driver supports events (implied by the use of controls) or priority handling. - Use videobuf2 for buffer handling. Mike Krufky will look into extending vb2 to support DVB buffers. Note: using vb2 for VBI devices has not been tested yet, but it should work. Please contact the mailinglist in case of problems with that. - Must pass the v4l2-compliance tests. - Hybrid tuners should be shared with DVB. DVB specific requirements: - Use the DVB core for both internal and external APIs. - Each I2C-based chip should have its own driver. - Tuners and frontends should be mapped as different drivers. - dvb_frontend_ops should specify the delivery system instead of specifying the frontend type via the dvb_frontend_ops info.type field. - DVB frontends should not implement dummy function handlers; if the function is not implemented, the DVB core should han
Re: Lockup on second streamon with omap3-isp
Hi Jean-Philippe, I have had exactly the same problem and the following workaround has caused no regression on our board yet. I can't explain exactly why it works and I think that it is internal to the isp. In function ccdc_set_stream, don't disable the ccdc_subclk when stopping capture: ret = ccdc_disable(ccdc); if (ccdc->output & CCDC_OUTPUT_MEMORY) omap3isp_sbl_disable(isp, OMAP3_ISP_SBL_CCDC_WRITE); - omap3isp_subclk_disable(isp, OMAP3_ISP_SUBCLK_CCDC); + //omap3isp_subclk_disable(isp, OMAP3_ISP_SUBCLK_CCDC); I know that it is still a workaround but I hope that maybe it will help someone to understand the real cause of this issue. Best Regards, Julien BERAUD Le 13/12/2012 15:14, jean-philippe francois a écrit : Hi, I have news on the "IRQ storm on second streamon" problem. Reminder : Given a perfectly fine HSYNC / VSYNC / PIXELCLOK configuration, the omap3-isp (at least until 3.4) will go into an interrupt storm when streamon is called for the second time, unless you are able to stop the sensor when not streaming. I have reproduced this on three different board, with three different sensor. On board 1, the problem disappeared by itself (in fact not by itself, see below) and the board is not in my possession anymore. On board 2, I implemented a working s_stream operation in the subdev driver, so the problem was solved because the sensor would effectively stop streaming when told to, keeping the ISP + CCDC happy On board 3, I can't stop the streaming, or I did not figure out how to make it stop yet. I tried to disable the HS_VS_IRQ, but it did not help, so I came back looking at the code of board 1, which was running fine with a 3.4 kernel. And I discovered the problem doesn't happen if I break the pipeline between two consecutive streamon. In other word if I do the following : media-ctl -l '16:0->5:0[1], 5:1->6:0[1]' media-ctl -f '16:0 [SBGGR8 2560x800 (0, 0)/2560x800]' yavta yavta ... <- board locks up, soft lockup is fired But if I do this instead : media-ctl -l '16:0->5:0[0], 5:1->6:0[0]' media-ctl -l '16:0->5:0[1], 5:1->6:0[1]' media-ctl -f '16:0 [SBGGR8 2560x800 (0, 0)/2560x800]' yavta media-ctl -l '16:0->5:0[0], 5:1->6:0[0]' media-ctl -l '16:0->5:0[1], 5:1->6:0[1]' media-ctl -f '16:0 [SBGGR8 2560x800 (0, 0)/2560x800]' yavta ... <- image are acquired, board doesn't lock up anymore It would be interesting to go from this workaround to the elimination of the root cause. What can I do / test next to stop this bug from hapenning ? Regards, Jean-Philippe François -- 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
Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support
On Fri, Dec 14, 2012 at 5:57 AM, Maarten Lankhorst wrote: > Op 14-12-12 10:36, sumit.sem...@ti.com schreef: >> From: Sumit Semwal >> >> Add debugfs support to make it easier to print debug information >> about the dma-buf buffers. >> > I like the idea, I don't know if it could be done in a free manner, but for > bonus points > could we also have the dma-buf fd be obtainable that way from a debugfs entry? > > Doing so would allow me to 'steal' a dma-buf from an existing mapping easily, > and test against that. > > Also I think the name of the device and process that exported the dma-buf > would be useful > to have as well, even if in case of the device that would mean changing the > api slightly to record it. > > I was thinking of having a directory structure like this: > > /sys/kernel/debug/dma_buf/stats > > and then for each dma-buf: > > /sys/kernel/debug/dma-buf/exporting_file.c/-fd > /sys/kernel/debug/dma-buf/exporting_file.c/-attachments > /sys/kernel/debug/dma-buf/exporting_file.c/-info > > Opening the fd file would give you back the original fd, or fail with -EIO if > refcount was dropped to 0. > > Would something like this be doable? I don't know debugfs that well, but I > don't see why it wouldn't be, yeah.. but sort of back-door's the security benefits of an anonymous fd.. BR, -R > ~Maarten > > -- > 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
Re: [Linaro-mm-sig] [PATCH] dma-buf: Add debugfs support
Op 14-12-12 10:36, sumit.sem...@ti.com schreef: > From: Sumit Semwal > > Add debugfs support to make it easier to print debug information > about the dma-buf buffers. > I like the idea, I don't know if it could be done in a free manner, but for bonus points could we also have the dma-buf fd be obtainable that way from a debugfs entry? Doing so would allow me to 'steal' a dma-buf from an existing mapping easily, and test against that. Also I think the name of the device and process that exported the dma-buf would be useful to have as well, even if in case of the device that would mean changing the api slightly to record it. I was thinking of having a directory structure like this: /sys/kernel/debug/dma_buf/stats and then for each dma-buf: /sys/kernel/debug/dma-buf/exporting_file.c/-fd /sys/kernel/debug/dma-buf/exporting_file.c/-attachments /sys/kernel/debug/dma-buf/exporting_file.c/-info Opening the fd file would give you back the original fd, or fail with -EIO if refcount was dropped to 0. Would something like this be doable? I don't know debugfs that well, but I don't see why it wouldn't be, ~Maarten -- 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 06/12] media/rc: fix oops on unloading module rc-core
During modiles initialization rc-core schedules work which calls
request_module() several times to load ir-*-decoder modules, but
it does not wait or cancel this work on module unloading.
rc-core should use request_module_nowait() instead, because it
anyway cannot load modules synchronously or cancel/wait pending
work on unloading, because this leads to deadlock on modules_mutex
between several "modprobe" processes.
Signed-off-by: Konstantin Khlebnikov
Cc: Mauro Carvalho Chehab
Cc: linux-media@vger.kernel.org
---
drivers/media/rc/ir-raw.c | 17 +
drivers/media/rc/rc-core-priv.h | 16
2 files changed, 9 insertions(+), 24 deletions(-)
diff --git a/drivers/media/rc/ir-raw.c b/drivers/media/rc/ir-raw.c
index 97dc8d1..17c94be 100644
--- a/drivers/media/rc/ir-raw.c
+++ b/drivers/media/rc/ir-raw.c
@@ -31,11 +31,6 @@ static DEFINE_MUTEX(ir_raw_handler_lock);
static LIST_HEAD(ir_raw_handler_list);
static u64 available_protocols;
-#ifdef MODULE
-/* Used to load the decoders */
-static struct work_struct wq_load;
-#endif
-
static int ir_raw_event_thread(void *data)
{
struct ir_raw_event ev;
@@ -347,8 +342,7 @@ void ir_raw_handler_unregister(struct ir_raw_handler
*ir_raw_handler)
}
EXPORT_SYMBOL(ir_raw_handler_unregister);
-#ifdef MODULE
-static void init_decoders(struct work_struct *work)
+void ir_raw_init(void)
{
/* Load the decoder modules */
@@ -365,12 +359,3 @@ static void init_decoders(struct work_struct *work)
it is needed to change the CONFIG_MODULE test at rc-core.h
*/
}
-#endif
-
-void ir_raw_init(void)
-{
-#ifdef MODULE
- INIT_WORK(&wq_load, init_decoders);
- schedule_work(&wq_load);
-#endif
-}
diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
index 96f0a8b..5d87287 100644
--- a/drivers/media/rc/rc-core-priv.h
+++ b/drivers/media/rc/rc-core-priv.h
@@ -165,56 +165,56 @@ void ir_raw_init(void);
/* from ir-nec-decoder.c */
#ifdef CONFIG_IR_NEC_DECODER_MODULE
-#define load_nec_decode() request_module("ir-nec-decoder")
+#define load_nec_decode() request_module_nowait("ir-nec-decoder")
#else
static inline void load_nec_decode(void) { }
#endif
/* from ir-rc5-decoder.c */
#ifdef CONFIG_IR_RC5_DECODER_MODULE
-#define load_rc5_decode() request_module("ir-rc5-decoder")
+#define load_rc5_decode() request_module_nowait("ir-rc5-decoder")
#else
static inline void load_rc5_decode(void) { }
#endif
/* from ir-rc6-decoder.c */
#ifdef CONFIG_IR_RC6_DECODER_MODULE
-#define load_rc6_decode() request_module("ir-rc6-decoder")
+#define load_rc6_decode() request_module_nowait("ir-rc6-decoder")
#else
static inline void load_rc6_decode(void) { }
#endif
/* from ir-jvc-decoder.c */
#ifdef CONFIG_IR_JVC_DECODER_MODULE
-#define load_jvc_decode() request_module("ir-jvc-decoder")
+#define load_jvc_decode() request_module_nowait("ir-jvc-decoder")
#else
static inline void load_jvc_decode(void) { }
#endif
/* from ir-sony-decoder.c */
#ifdef CONFIG_IR_SONY_DECODER_MODULE
-#define load_sony_decode() request_module("ir-sony-decoder")
+#define load_sony_decode() request_module_nowait("ir-sony-decoder")
#else
static inline void load_sony_decode(void) { }
#endif
/* from ir-sanyo-decoder.c */
#ifdef CONFIG_IR_SANYO_DECODER_MODULE
-#define load_sanyo_decode()request_module("ir-sanyo-decoder")
+#define load_sanyo_decode()request_module_nowait("ir-sanyo-decoder")
#else
static inline void load_sanyo_decode(void) { }
#endif
/* from ir-mce_kbd-decoder.c */
#ifdef CONFIG_IR_MCE_KBD_DECODER_MODULE
-#define load_mce_kbd_decode() request_module("ir-mce_kbd-decoder")
+#define load_mce_kbd_decode() request_module_nowait("ir-mce_kbd-decoder")
#else
static inline void load_mce_kbd_decode(void) { }
#endif
/* from ir-lirc-codec.c */
#ifdef CONFIG_IR_LIRC_CODEC_MODULE
-#define load_lirc_codec() request_module("ir-lirc-codec")
+#define load_lirc_codec() request_module_nowait("ir-lirc-codec")
#else
static inline void load_lirc_codec(void) { }
#endif
--
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] dma-buf: Add debugfs support
Missed one ...
On Fri, Dec 14, 2012 at 10:36 AM, wrote:
> + list_for_each_entry(attach_obj, &buf_obj->attachments, node) {
> + seq_printf(s, "\t\t");
> +
> + seq_printf(s, "%s\n", attach_obj->dev->init_name);
> + attach_count++;
> + }
You need to hold dmabuf->lock while walking the attachment list.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
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] dma-buf: Add debugfs support
On Fri, Dec 14, 2012 at 10:36 AM, wrote:
> From: Sumit Semwal
>
> Add debugfs support to make it easier to print debug information
> about the dma-buf buffers.
>
> Signed-off-by: Sumit Semwal
Looks line, only nitpick is that we have no idea who exported a given
buffer. So what about adding a new dma_buf_export_named which takes a
char * to identify the exporter, and then add a bit of cpp magic like
this:
#define dma_buf_export(args) dma_buf_export_named(args, stringify(__FILE))
At least for drm drivers using the prime helpers this should add the
file of the exporting driver, so would be good enough to identify the
exporter. Or we could add a dma_buf_export_dev which has a struct
device * as additional argument ... Otoh that wouldn't really work for
exporting dma_bufs from ION, so maybe a const char * is better.
-Daniel
> ---
> drivers/base/dma-buf.c | 149
> +++
> include/linux/dma-buf.h |6 +-
> 2 files changed, 154 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index a3f79c4..49fcf0f 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -27,9 +27,18 @@
> #include
> #include
> #include
> +#include
> +#include
>
> static inline int is_dma_buf_file(struct file *);
>
> +struct dma_buf_list {
> + struct list_head head;
> + struct mutex lock;
> +};
> +
> +static struct dma_buf_list db_list;
> +
> static int dma_buf_release(struct inode *inode, struct file *file)
> {
> struct dma_buf *dmabuf;
> @@ -40,6 +49,11 @@ static int dma_buf_release(struct inode *inode, struct
> file *file)
> dmabuf = file->private_data;
>
> dmabuf->ops->release(dmabuf);
> +
> + mutex_lock(&db_list.lock);
> + list_del(&dmabuf->list_node);
> + mutex_unlock(&db_list.lock);
> +
> kfree(dmabuf);
> return 0;
> }
> @@ -120,6 +134,10 @@ struct dma_buf *dma_buf_export(void *priv, const struct
> dma_buf_ops *ops,
> mutex_init(&dmabuf->lock);
> INIT_LIST_HEAD(&dmabuf->attachments);
>
> + mutex_lock(&db_list.lock);
> + list_add(&dmabuf->list_node, &db_list.head);
> + mutex_unlock(&db_list.lock);
> +
> return dmabuf;
> }
> EXPORT_SYMBOL_GPL(dma_buf_export);
> @@ -505,3 +523,134 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
> dmabuf->ops->vunmap(dmabuf, vaddr);
> }
> EXPORT_SYMBOL_GPL(dma_buf_vunmap);
> +
> +static int dma_buf_init_debugfs(void);
> +static void dma_buf_uninit_debugfs(void);
> +
> +static void __init dma_buf_init(void)
> +{
> + mutex_init(&db_list.lock);
> + INIT_LIST_HEAD(&db_list.head);
> + dma_buf_init_debugfs();
> +}
> +
> +static void __exit dma_buf_deinit(void)
> +{
> + dma_buf_uninit_debugfs();
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +static int dma_buf_describe(struct seq_file *s)
> +{
> + int ret;
> + struct dma_buf *buf_obj;
> + struct dma_buf_attachment *attach_obj;
> + int count = 0, attach_count;
> + size_t size = 0;
> +
> + ret = mutex_lock_interruptible(&db_list.lock);
> +
> + if (ret)
> + return ret;
> +
> + seq_printf(s, "\nDma-buf Objects:\n");
> + seq_printf(s, "\tsize\tflags\tmode\tcount\n");
> +
> + list_for_each_entry(buf_obj, &db_list.head, list_node) {
> + seq_printf(s, "\t");
> +
> + seq_printf(s, "%08zu\t%08x\t%08x\t%08d\n",
> + buf_obj->size, buf_obj->file->f_flags,
> + buf_obj->file->f_mode,
> + buf_obj->file->f_count.counter);
> +
> + seq_printf(s, "\t\tAttached Devices:\n");
> + attach_count = 0;
> +
> + list_for_each_entry(attach_obj, &buf_obj->attachments, node) {
> + seq_printf(s, "\t\t");
> +
> + seq_printf(s, "%s\n", attach_obj->dev->init_name);
> + attach_count++;
> + }
> +
> + seq_printf(s, "\n\t\tTotal %d devices attached\n",
> + attach_count);
> +
> + count++;
> + size += buf_obj->size;
> + }
> +
> + seq_printf(s, "\nTotal %d objects, %zu bytes\n", count, size);
> +
> + mutex_unlock(&db_list.lock);
> + return 0;
> +}
> +
> +static int dma_buf_show(struct seq_file *s, void *unused)
> +{
> + void (*func)(struct seq_file *) = s->private;
> + func(s);
> + return 0;
> +}
> +
> +static int dma_buf_debug_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, dma_buf_show, inode->i_private);
> +}
> +
> +static const struct file_operations dma_buf_debug_fops = {
> + .open = dma_buf_debug_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release= single_release,
> +};
> +
> +static stru
[PATCH] dma-buf: Add debugfs support
From: Sumit Semwal
Add debugfs support to make it easier to print debug information
about the dma-buf buffers.
Signed-off-by: Sumit Semwal
---
drivers/base/dma-buf.c | 149 +++
include/linux/dma-buf.h |6 +-
2 files changed, 154 insertions(+), 1 deletion(-)
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index a3f79c4..49fcf0f 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -27,9 +27,18 @@
#include
#include
#include
+#include
+#include
static inline int is_dma_buf_file(struct file *);
+struct dma_buf_list {
+ struct list_head head;
+ struct mutex lock;
+};
+
+static struct dma_buf_list db_list;
+
static int dma_buf_release(struct inode *inode, struct file *file)
{
struct dma_buf *dmabuf;
@@ -40,6 +49,11 @@ static int dma_buf_release(struct inode *inode, struct file
*file)
dmabuf = file->private_data;
dmabuf->ops->release(dmabuf);
+
+ mutex_lock(&db_list.lock);
+ list_del(&dmabuf->list_node);
+ mutex_unlock(&db_list.lock);
+
kfree(dmabuf);
return 0;
}
@@ -120,6 +134,10 @@ struct dma_buf *dma_buf_export(void *priv, const struct
dma_buf_ops *ops,
mutex_init(&dmabuf->lock);
INIT_LIST_HEAD(&dmabuf->attachments);
+ mutex_lock(&db_list.lock);
+ list_add(&dmabuf->list_node, &db_list.head);
+ mutex_unlock(&db_list.lock);
+
return dmabuf;
}
EXPORT_SYMBOL_GPL(dma_buf_export);
@@ -505,3 +523,134 @@ void dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
dmabuf->ops->vunmap(dmabuf, vaddr);
}
EXPORT_SYMBOL_GPL(dma_buf_vunmap);
+
+static int dma_buf_init_debugfs(void);
+static void dma_buf_uninit_debugfs(void);
+
+static void __init dma_buf_init(void)
+{
+ mutex_init(&db_list.lock);
+ INIT_LIST_HEAD(&db_list.head);
+ dma_buf_init_debugfs();
+}
+
+static void __exit dma_buf_deinit(void)
+{
+ dma_buf_uninit_debugfs();
+}
+
+#ifdef CONFIG_DEBUG_FS
+static int dma_buf_describe(struct seq_file *s)
+{
+ int ret;
+ struct dma_buf *buf_obj;
+ struct dma_buf_attachment *attach_obj;
+ int count = 0, attach_count;
+ size_t size = 0;
+
+ ret = mutex_lock_interruptible(&db_list.lock);
+
+ if (ret)
+ return ret;
+
+ seq_printf(s, "\nDma-buf Objects:\n");
+ seq_printf(s, "\tsize\tflags\tmode\tcount\n");
+
+ list_for_each_entry(buf_obj, &db_list.head, list_node) {
+ seq_printf(s, "\t");
+
+ seq_printf(s, "%08zu\t%08x\t%08x\t%08d\n",
+ buf_obj->size, buf_obj->file->f_flags,
+ buf_obj->file->f_mode,
+ buf_obj->file->f_count.counter);
+
+ seq_printf(s, "\t\tAttached Devices:\n");
+ attach_count = 0;
+
+ list_for_each_entry(attach_obj, &buf_obj->attachments, node) {
+ seq_printf(s, "\t\t");
+
+ seq_printf(s, "%s\n", attach_obj->dev->init_name);
+ attach_count++;
+ }
+
+ seq_printf(s, "\n\t\tTotal %d devices attached\n",
+ attach_count);
+
+ count++;
+ size += buf_obj->size;
+ }
+
+ seq_printf(s, "\nTotal %d objects, %zu bytes\n", count, size);
+
+ mutex_unlock(&db_list.lock);
+ return 0;
+}
+
+static int dma_buf_show(struct seq_file *s, void *unused)
+{
+ void (*func)(struct seq_file *) = s->private;
+ func(s);
+ return 0;
+}
+
+static int dma_buf_debug_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, dma_buf_show, inode->i_private);
+}
+
+static const struct file_operations dma_buf_debug_fops = {
+ .open = dma_buf_debug_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release= single_release,
+};
+
+static struct dentry *dma_buf_debugfs_dir;
+
+static int dma_buf_init_debugfs(void)
+{
+ int err = 0;
+ dma_buf_debugfs_dir = debugfs_create_dir("dma_buf", NULL);
+ if (IS_ERR(dma_buf_debugfs_dir)) {
+ err = PTR_ERR(dma_buf_debugfs_dir);
+ dma_buf_debugfs_dir = NULL;
+ return err;
+ }
+
+ err = dma_buf_debugfs_create_file("bufinfo", dma_buf_describe);
+
+ if (err)
+ pr_debug("dma_buf: debugfs: failed to create node bufinfo\n");
+
+ return err;
+}
+
+static void dma_buf_uninit_debugfs(void)
+{
+ if (dma_buf_debugfs_dir)
+ debugfs_remove_recursive(dma_buf_debugfs_dir);
+}
+
+int dma_buf_debugfs_create_file(const char *name,
+ int (*write)(struct seq_file *))
+{
+ struct dentry *d;
+
+ d = debugfs_create_file(name, S_IRUGO, dma_buf_debugfs_dir,
+ write, &dma_buf_debug_fops);
+
+ if (IS_E
