linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
@ 2014-10-31 13:06 Dan Carpenter
  2014-10-31 14:26 ` Aya Mahfouz
  0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2014-10-31 13:06 UTC (permalink / raw)
  To: mahfouz.saif.elyazal; +Cc: linux-media, Greg Kroah-Hartman

Hello Aya Mahfouz,

The patch be4aa8157c98: "staging: media: lirc: lirc_zilog.c: replace
custom print macros with dev_* and pr_*" from Oct 26, 2014, leads to
the following static checker warning:

	drivers/staging/media/lirc/lirc_zilog.c:1340 close()
	error: we previously assumed 'ir' could be null (see line 1339)

drivers/staging/media/lirc/lirc_zilog.c
  1333  /* Close the IR device */
  1334  static int close(struct inode *node, struct file *filep)
  1335  {
  1336          /* find our IR struct */
  1337          struct IR *ir = filep->private_data;
  1338  
  1339          if (ir == NULL) {
                    ^^^^^^^^^^
  1340                  dev_err(ir->l.dev, "close: no private_data attached to the file!\n");
                                ^^^^^^^^^

I suggest you just delete the error message.  Can "ir" actually be NULL
here anyway?

  1341                  return -ENODEV;
  1342          }
  1343  

	drivers/staging/media/lirc/lirc_zilog.c:1636 ir_probe()
	error: potential null dereference 'ir'.  (kzalloc returns null)

drivers/staging/media/lirc/lirc_zilog.c
  1614          dev_info(ir->l.dev, "IR unit on %s (i2c-%d) registered as lirc%d and ready\n",
  1615                     adap->name, adap->nr, ir->l.minor);
  1616  
  1617  out_ok:
  1618          if (rx != NULL)
  1619                  put_ir_rx(rx, true);
  1620          if (tx != NULL)
  1621                  put_ir_tx(tx, true);
  1622          put_ir_device(ir, true);
  1623          dev_info(ir->l.dev, "probe of IR %s on %s (i2c-%d) done\n",
  1624                     tx_probe ? "Tx" : "Rx", adap->name, adap->nr);
  1625          mutex_unlock(&ir_devices_lock);
  1626          return 0;
  1627  
  1628  out_put_xx:
  1629          if (rx != NULL)
  1630                  put_ir_rx(rx, true);
                        ^^^^^^^^^^^^^^^^^^^
Btw, I think this is a double free?  On some paths we call put_ir_rx()
before the goto out_put_xx;.

  1631          if (tx != NULL)
  1632                  put_ir_tx(tx, true);
  1633  out_put_ir:
  1634          put_ir_device(ir, true);
  1635  out_no_ir:
  1636          dev_err(ir->l.dev, "%s: probing IR %s on %s (i2c-%d) failed with %d\n",
                        ^^^^^^^^^
Null dereference.

  1637                      __func__, tx_probe ? "Tx" : "Rx", adap->name, adap->nr,
  1638                     ret);
  1639          mutex_unlock(&ir_devices_lock);
  1640          return ret;
  1641  }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
  2014-10-31 13:06 staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_* Dan Carpenter
@ 2014-10-31 14:26 ` Aya Mahfouz
  2014-10-31 14:35   ` Dan Carpenter
  0 siblings, 1 reply; 11+ messages in thread
From: Aya Mahfouz @ 2014-10-31 14:26 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-media, Greg Kroah-Hartman

On Fri, Oct 31, 2014 at 04:06:00PM +0300, Dan Carpenter wrote:
> Hello Aya Mahfouz,
> 

Hello Dan, 

> The patch be4aa8157c98: "staging: media: lirc: lirc_zilog.c: replace
> custom print macros with dev_* and pr_*" from Oct 26, 2014, leads to
> the following static checker warning:
> 
> 	drivers/staging/media/lirc/lirc_zilog.c:1340 close()
> 	error: we previously assumed 'ir' could be null (see line 1339)
> 
> drivers/staging/media/lirc/lirc_zilog.c
>   1333  /* Close the IR device */
>   1334  static int close(struct inode *node, struct file *filep)
>   1335  {
>   1336          /* find our IR struct */
>   1337          struct IR *ir = filep->private_data;
>   1338  
>   1339          if (ir == NULL) {
>                     ^^^^^^^^^^
>   1340                  dev_err(ir->l.dev, "close: no private_data attached to the file!\n");
>                                 ^^^^^^^^^
> 
> I suggest you just delete the error message.  Can "ir" actually be NULL
> here anyway?
>

Since I'm a newbie and this is not my code, I prefer to use pr_err().
 
>   1341                  return -ENODEV;
>   1342          }
>   1343  
> 
> 	drivers/staging/media/lirc/lirc_zilog.c:1636 ir_probe()
> 	error: potential null dereference 'ir'.  (kzalloc returns null)
> 
> drivers/staging/media/lirc/lirc_zilog.c
>   1614          dev_info(ir->l.dev, "IR unit on %s (i2c-%d) registered as lirc%d and ready\n",
>   1615                     adap->name, adap->nr, ir->l.minor);
>   1616  
>   1617  out_ok:
>   1618          if (rx != NULL)
>   1619                  put_ir_rx(rx, true);
>   1620          if (tx != NULL)
>   1621                  put_ir_tx(tx, true);
>   1622          put_ir_device(ir, true);
>   1623          dev_info(ir->l.dev, "probe of IR %s on %s (i2c-%d) done\n",
>   1624                     tx_probe ? "Tx" : "Rx", adap->name, adap->nr);
>   1625          mutex_unlock(&ir_devices_lock);
>   1626          return 0;
>   1627  
>   1628  out_put_xx:
>   1629          if (rx != NULL)
>   1630                  put_ir_rx(rx, true);
>                         ^^^^^^^^^^^^^^^^^^^
> Btw, I think this is a double free?  On some paths we call put_ir_rx()
> before the goto out_put_xx;.
> 

I'll read about this and see how to fix it.

>   1631          if (tx != NULL)
>   1632                  put_ir_tx(tx, true);
>   1633  out_put_ir:
>   1634          put_ir_device(ir, true);
>   1635  out_no_ir:
>   1636          dev_err(ir->l.dev, "%s: probing IR %s on %s (i2c-%d) failed with %d\n",
>                         ^^^^^^^^^
> Null dereference.
> 
>   1637                      __func__, tx_probe ? "Tx" : "Rx", adap->name, adap->nr,
>   1638                     ret);
>   1639          mutex_unlock(&ir_devices_lock);
>   1640          return ret;
>   1641  }
> 

In general, I can send a new patch to fix the aforementioned warnings.
Kindly let me know if you prefer that I send a second version of this
patch.

> regards,
> dan carpenter

Kind Regards,
Aya Saif El-yazal Mahfouz

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
  2014-10-31 14:26 ` Aya Mahfouz
@ 2014-10-31 14:35   ` Dan Carpenter
  2014-11-01 20:05     ` Aya Mahfouz
  2014-11-06 12:46     ` Sean Young
  0 siblings, 2 replies; 11+ messages in thread
From: Dan Carpenter @ 2014-10-31 14:35 UTC (permalink / raw)
  To: Aya Mahfouz; +Cc: linux-media, Greg Kroah-Hartman

On Fri, Oct 31, 2014 at 04:26:45PM +0200, Aya Mahfouz wrote:
> On Fri, Oct 31, 2014 at 04:06:00PM +0300, Dan Carpenter wrote:
> > drivers/staging/media/lirc/lirc_zilog.c
> >   1333  /* Close the IR device */
> >   1334  static int close(struct inode *node, struct file *filep)
> >   1335  {
> >   1336          /* find our IR struct */
> >   1337          struct IR *ir = filep->private_data;
> >   1338  
> >   1339          if (ir == NULL) {
> >                     ^^^^^^^^^^
> >   1340                  dev_err(ir->l.dev, "close: no private_data attached to the file!\n");
> >                                 ^^^^^^^^^
> > 
> > I suggest you just delete the error message.  Can "ir" actually be NULL
> > here anyway?
> >
> 
> Since I'm a newbie and this is not my code, I prefer to use pr_err().

This driver doesn't belong to anyone.  Go ahead and take ownership.  The
message is fairly worthless and no one will miss it.

> 
> In general, I can send a new patch to fix the aforementioned warnings.
> Kindly let me know if you prefer that I send a second version of this
> patch.

No.  The first patch was already applied so send a new patch.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
  2014-10-31 14:35   ` Dan Carpenter
@ 2014-11-01 20:05     ` Aya Mahfouz
  2014-11-06 12:46     ` Sean Young
  1 sibling, 0 replies; 11+ messages in thread
From: Aya Mahfouz @ 2014-11-01 20:05 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-media, Greg Kroah-Hartman

On Fri, Oct 31, 2014 at 05:35:41PM +0300, Dan Carpenter wrote:
> On Fri, Oct 31, 2014 at 04:26:45PM +0200, Aya Mahfouz wrote:
> > On Fri, Oct 31, 2014 at 04:06:00PM +0300, Dan Carpenter wrote:
> > > drivers/staging/media/lirc/lirc_zilog.c
> > >   1333  /* Close the IR device */
> > >   1334  static int close(struct inode *node, struct file *filep)
> > >   1335  {
> > >   1336          /* find our IR struct */
> > >   1337          struct IR *ir = filep->private_data;
> > >   1338  
> > >   1339          if (ir == NULL) {
> > >                     ^^^^^^^^^^
> > >   1340                  dev_err(ir->l.dev, "close: no private_data attached to the file!\n");
> > >                                 ^^^^^^^^^
> > > 
> > > I suggest you just delete the error message.  Can "ir" actually be NULL
> > > here anyway?
> > >
> > 
> > Since I'm a newbie and this is not my code, I prefer to use pr_err().
> 
> This driver doesn't belong to anyone.  Go ahead and take ownership.  The
> message is fairly worthless and no one will miss it.
> 
ok.
> > 
> > In general, I can send a new patch to fix the aforementioned warnings.
> > Kindly let me know if you prefer that I send a second version of this
> > patch.
> 
> No.  The first patch was already applied so send a new patch.
> 
I will fix the static errors that my patch caused. The warning concerning
the double free will require rewriting some parts of the function and was
not caused by my patch. I have a couple of ideas in mind but I need
sometime to apply them. Greg too is not happy about the coding style of
this driver in general.

> regards,
> dan carpenter
> 

Kind Regards,
Aya Saif El-yazal Mahfouz

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
  2014-10-31 14:35   ` Dan Carpenter
  2014-11-01 20:05     ` Aya Mahfouz
@ 2014-11-06 12:46     ` Sean Young
  2014-11-06 13:05       ` Mauro Carvalho Chehab
  1 sibling, 1 reply; 11+ messages in thread
From: Sean Young @ 2014-11-06 12:46 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Aya Mahfouz, linux-media, Greg Kroah-Hartman,
	Mauro Carvalho Chehab

On Fri, Oct 31, 2014 at 05:35:41PM +0300, Dan Carpenter wrote:
> On Fri, Oct 31, 2014 at 04:26:45PM +0200, Aya Mahfouz wrote:
> > On Fri, Oct 31, 2014 at 04:06:00PM +0300, Dan Carpenter wrote:
> > > drivers/staging/media/lirc/lirc_zilog.c
> > >   1333  /* Close the IR device */
> > >   1334  static int close(struct inode *node, struct file *filep)
> > >   1335  {
> > >   1336          /* find our IR struct */
> > >   1337          struct IR *ir = filep->private_data;
> > >   1338  
> > >   1339          if (ir == NULL) {
> > >                     ^^^^^^^^^^
> > >   1340                  dev_err(ir->l.dev, "close: no private_data attached to the file!\n");
> > >                                 ^^^^^^^^^
> > > 
> > > I suggest you just delete the error message.  Can "ir" actually be NULL
> > > here anyway?
> > >
> > 
> > Since I'm a newbie and this is not my code, I prefer to use pr_err().
> 
> This driver doesn't belong to anyone.  Go ahead and take ownership.  The
> message is fairly worthless and no one will miss it.

Speaking of ownership, what this driver really needs is to be ported to 
rc-core. In order to do this it'll need to be able to send raw IR rather
key codes; I've been peering at the firmware but it neither looks like
zilog z8 opcodes nor space/pulse information.

Does anyone have any contacts at Hauppauge who could help with this?

Thanks,

Sean

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
  2014-11-06 12:46     ` Sean Young
@ 2014-11-06 13:05       ` Mauro Carvalho Chehab
  2014-11-06 13:21         ` Sean Young
  0 siblings, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-06 13:05 UTC (permalink / raw)
  To: Sean Young, Jarod Wilson, Andy Walls
  Cc: Dan Carpenter, Aya Mahfouz, linux-media, Greg Kroah-Hartman

Hi Sean,

Em Thu, 06 Nov 2014 12:46:29 +0000
Sean Young <sean@mess.org> escreveu:

> On Fri, Oct 31, 2014 at 05:35:41PM +0300, Dan Carpenter wrote:
> > On Fri, Oct 31, 2014 at 04:26:45PM +0200, Aya Mahfouz wrote:
> > > On Fri, Oct 31, 2014 at 04:06:00PM +0300, Dan Carpenter wrote:
> > > > drivers/staging/media/lirc/lirc_zilog.c
> > > >   1333  /* Close the IR device */
> > > >   1334  static int close(struct inode *node, struct file *filep)
> > > >   1335  {
> > > >   1336          /* find our IR struct */
> > > >   1337          struct IR *ir = filep->private_data;
> > > >   1338  
> > > >   1339          if (ir == NULL) {
> > > >                     ^^^^^^^^^^
> > > >   1340                  dev_err(ir->l.dev, "close: no private_data attached to the file!\n");
> > > >                                 ^^^^^^^^^
> > > > 
> > > > I suggest you just delete the error message.  Can "ir" actually be NULL
> > > > here anyway?
> > > >
> > > 
> > > Since I'm a newbie and this is not my code, I prefer to use pr_err().
> > 
> > This driver doesn't belong to anyone.  Go ahead and take ownership.  The
> > message is fairly worthless and no one will miss it.
> 
> Speaking of ownership, what this driver really needs is to be ported to 
> rc-core. In order to do this it'll need to be able to send raw IR rather
> key codes; I've been peering at the firmware but it neither looks like
> zilog z8 opcodes nor space/pulse information.

Actually, I think that all features provided by this driver were already
migrated into the ir-kbd-i2c (drivers/media/i2c/ir-kbd-i2c.c) driver.

Andy and Jarod worked on this conversion, but we decided, on that time,
to keep lirc_zilog for a while (can't remember why).

Andy/Jarod,

What's the status of the ir-kbd-i2c with regards to Zilog z8 support?

> Does anyone have any contacts at Hauppauge who could help with this?

Probably, it won't be easy to get someone there that worked on it,
as this device is too old.

Anyway, if are there anything still pending, I may be able to get
some contacts at the vendor.

Regards,
Mauro

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
  2014-11-06 13:05       ` Mauro Carvalho Chehab
@ 2014-11-06 13:21         ` Sean Young
       [not found]           ` <697D038C-4BD9-4113-8E7E-B89BACF09AC2@gmail.com>
  0 siblings, 1 reply; 11+ messages in thread
From: Sean Young @ 2014-11-06 13:21 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Jarod Wilson, Andy Walls, Dan Carpenter, Aya Mahfouz, linux-media,
	Greg Kroah-Hartman

On Thu, Nov 06, 2014 at 11:05:49AM -0200, Mauro Carvalho Chehab wrote:
> Hi Sean,
> 
> Em Thu, 06 Nov 2014 12:46:29 +0000
> Sean Young <sean@mess.org> escreveu:
> 
> > On Fri, Oct 31, 2014 at 05:35:41PM +0300, Dan Carpenter wrote:
> > > On Fri, Oct 31, 2014 at 04:26:45PM +0200, Aya Mahfouz wrote:
> > > > On Fri, Oct 31, 2014 at 04:06:00PM +0300, Dan Carpenter wrote:
> > > > > drivers/staging/media/lirc/lirc_zilog.c
> > > > >   1333  /* Close the IR device */
> > > > >   1334  static int close(struct inode *node, struct file *filep)
> > > > >   1335  {
> > > > >   1336          /* find our IR struct */
> > > > >   1337          struct IR *ir = filep->private_data;
> > > > >   1338  
> > > > >   1339          if (ir == NULL) {
> > > > >                     ^^^^^^^^^^
> > > > >   1340                  dev_err(ir->l.dev, "close: no private_data attached to the file!\n");
> > > > >                                 ^^^^^^^^^
> > > > > 
> > > > > I suggest you just delete the error message.  Can "ir" actually be NULL
> > > > > here anyway?
> > > > >
> > > > 
> > > > Since I'm a newbie and this is not my code, I prefer to use pr_err().
> > > 
> > > This driver doesn't belong to anyone.  Go ahead and take ownership.  The
> > > message is fairly worthless and no one will miss it.
> > 
> > Speaking of ownership, what this driver really needs is to be ported to 
> > rc-core. In order to do this it'll need to be able to send raw IR rather
> > key codes; I've been peering at the firmware but it neither looks like
> > zilog z8 opcodes nor space/pulse information.
> 
> Actually, I think that all features provided by this driver were already
> migrated into the ir-kbd-i2c (drivers/media/i2c/ir-kbd-i2c.c) driver.

All the features for the IR receiver are implemented (very nicely) in
ir-kbd-i2c (the ir_rx_z8f0811_haup and ir_rx_z8f0811_hdpvr i2c drivers).

However the IR emitter (i2c driver ir_tx_z8f0811_haup and 
ir_tx_z8f0811_hdpvr) are not implemented there. I wanted to port the 
IR emitter but the driver can only send specific rc5 (IIRC) keycodes, not
raw IR. So I cannot port it.

> Andy and Jarod worked on this conversion, but we decided, on that time,
> to keep lirc_zilog for a while (can't remember why).
> 
> Andy/Jarod,
> 
> What's the status of the ir-kbd-i2c with regards to Zilog z8 support?

Transmitter side.

> > Does anyone have any contacts at Hauppauge who could help with this?
> 
> Probably, it won't be easy to get someone there that worked on it,
> as this device is too old.
> 
> Anyway, if are there anything still pending, I may be able to get
> some contacts at the vendor.

That would be great, I have hardware and I'm happy to re-write the
driver.

Thanks

Sean

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
       [not found]           ` <697D038C-4BD9-4113-8E7E-B89BACF09AC2@gmail.com>
@ 2014-11-06 13:56             ` Andy Walls
  2014-11-09 21:35               ` Sean Young
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Walls @ 2014-11-06 13:56 UTC (permalink / raw)
  To: Sean Young, Mauro Carvalho Chehab
  Cc: Jarod Wilson, Dan Carpenter, Aya Mahfouz, linux-media,
	Greg Kroah-Hartman

On November 6, 2014 8:54:28 AM EST, Andy Walls <awalls.cx18@gmail.com> wrote:
>On November 6, 2014 8:21:13 AM EST, Sean Young <sean@mess.org> wrote:
>>On Thu, Nov 06, 2014 at 11:05:49AM -0200, Mauro Carvalho Chehab wrote:
>>> Hi Sean,
>>> 
>>> Em Thu, 06 Nov 2014 12:46:29 +0000
>>> Sean Young <sean@mess.org> escreveu:
>>> 
>>> > On Fri, Oct 31, 2014 at 05:35:41PM +0300, Dan Carpenter wrote:
>>> > > On Fri, Oct 31, 2014 at 04:26:45PM +0200, Aya Mahfouz wrote:
>>> > > > On Fri, Oct 31, 2014 at 04:06:00PM +0300, Dan Carpenter wrote:
>>> > > > > drivers/staging/media/lirc/lirc_zilog.c
>>> > > > >   1333  /* Close the IR device */
>>> > > > >   1334  static int close(struct inode *node, struct file
>>*filep)
>>> > > > >   1335  {
>>> > > > >   1336          /* find our IR struct */
>>> > > > >   1337          struct IR *ir = filep->private_data;
>>> > > > >   1338  
>>> > > > >   1339          if (ir == NULL) {
>>> > > > >                     ^^^^^^^^^^
>>> > > > >   1340                  dev_err(ir->l.dev, "close: no
>>private_data attached to the file!\n");
>>> > > > >                                 ^^^^^^^^^
>>> > > > > 
>>> > > > > I suggest you just delete the error message.  Can "ir"
>>actually be NULL
>>> > > > > here anyway?
>>> > > > >
>>> > > > 
>>> > > > Since I'm a newbie and this is not my code, I prefer to use
>>pr_err().
>>> > > 
>>> > > This driver doesn't belong to anyone.  Go ahead and take
>>ownership.  The
>>> > > message is fairly worthless and no one will miss it.
>>> > 
>>> > Speaking of ownership, what this driver really needs is to be
>>ported to 
>>> > rc-core. In order to do this it'll need to be able to send raw IR
>>rather
>>> > key codes; I've been peering at the firmware but it neither looks
>>like
>>> > zilog z8 opcodes nor space/pulse information.
>>> 
>>> Actually, I think that all features provided by this driver were
>>already
>>> migrated into the ir-kbd-i2c (drivers/media/i2c/ir-kbd-i2c.c)
>driver.
>>
>>All the features for the IR receiver are implemented (very nicely) in
>>ir-kbd-i2c (the ir_rx_z8f0811_haup and ir_rx_z8f0811_hdpvr i2c
>>drivers).
>>
>>However the IR emitter (i2c driver ir_tx_z8f0811_haup and 
>>ir_tx_z8f0811_hdpvr) are not implemented there. I wanted to port the 
>>IR emitter but the driver can only send specific rc5 (IIRC) keycodes,
>>not
>>raw IR. So I cannot port it.
>>
>>> Andy and Jarod worked on this conversion, but we decided, on that
>>time,
>>> to keep lirc_zilog for a while (can't remember why).
>>> 
>>> Andy/Jarod,
>>> 
>>> What's the status of the ir-kbd-i2c with regards to Zilog z8
>support?
>>
>>Transmitter side.
>>
>>> > Does anyone have any contacts at Hauppauge who could help with
>>this?
>>> 
>>> Probably, it won't be easy to get someone there that worked on it,
>>> as this device is too old.
>>> 
>>> Anyway, if are there anything still pending, I may be able to get
>>> some contacts at the vendor.
>>
>>That would be great, I have hardware and I'm happy to re-write the
>>driver.
>>
>>Thanks
>>
>>Sean
>>--
>>To unsubscribe from this list: send the line "unsubscribe linux-media"
>>in
>>the body of a message to majordomo@vger.kernel.org
>>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>Sean,
>
>Ir-kbd-i2c was never intended for Tx.
>
>You can transmit *short* arbitrary pulse-space streams with the zilog
>chip, by feeding it a parameter block that has the pulse timing
>information and then subsequently has been obfuscated.  The firmware
>file that LIRC uses in userspace is full of predefined versions of
>these things for RC5 and NEC IIRC.  This LIRC firmware file also holds
>the (de)obfuscation key.
>
>I've got a bunch of old notes on this stuff from essentially reverse
>engineering the firmware in the Z8.  IANAL, but to me, its use in
>developing in-kernel stuff could be dubious.
>
>Regards,
>Andy

Resending as plain text for mailing list.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
  2014-11-06 13:56             ` Andy Walls
@ 2014-11-09 21:35               ` Sean Young
  2014-11-17 14:59                 ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 11+ messages in thread
From: Sean Young @ 2014-11-09 21:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Andy Walls, Jarod Wilson, Dan Carpenter, Aya Mahfouz, linux-media,
	Greg Kroah-Hartman

On Thu, Nov 06, 2014 at 08:56:47AM -0500, Andy Walls wrote:
> On November 6, 2014 8:54:28 AM EST, Andy Walls <awalls.cx18@gmail.com> wrote:
> >Sean,
> >
> >Ir-kbd-i2c was never intended for Tx.
> >
> >You can transmit *short* arbitrary pulse-space streams with the zilog
> >chip, by feeding it a parameter block that has the pulse timing
> >information and then subsequently has been obfuscated.  The firmware
> >file that LIRC uses in userspace is full of predefined versions of
> >these things for RC5 and NEC IIRC.  This LIRC firmware file also holds
> >the (de)obfuscation key.
> >
> >I've got a bunch of old notes on this stuff from essentially reverse
> >engineering the firmware in the Z8.  IANAL, but to me, its use in
> >developing in-kernel stuff could be dubious.
> >
> >Regards,
> >Andy

Very interesting.

I had considered reverse engineering the z8 firmware but I never found a
way to access it. I guess we have three options:

1. I could use Andy's notes to implement Tx. I have not seen the original
   firmware code so I'm not contaminated by reverse engineering it. IANAL 
   but I thought this is an acceptable way of writing a driver.

2. Hauppauge could prove us with documentation to write a driver with.

3. Leave it as-is, lirc_zilog will eventually be deleted from staging as it
   can't be ported to rc-core.


Sean

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
  2014-11-09 21:35               ` Sean Young
@ 2014-11-17 14:59                 ` Mauro Carvalho Chehab
  2014-11-20 12:38                   ` Sean Young
  0 siblings, 1 reply; 11+ messages in thread
From: Mauro Carvalho Chehab @ 2014-11-17 14:59 UTC (permalink / raw)
  To: Sean Young
  Cc: Andy Walls, Jarod Wilson, Dan Carpenter, Aya Mahfouz, linux-media,
	Greg Kroah-Hartman

Em Sun, 9 Nov 2014 21:35:17 +0000
Sean Young <sean@mess.org> escreveu:

> On Thu, Nov 06, 2014 at 08:56:47AM -0500, Andy Walls wrote:
> > On November 6, 2014 8:54:28 AM EST, Andy Walls <awalls.cx18@gmail.com> wrote:
> > >Sean,
> > >
> > >Ir-kbd-i2c was never intended for Tx.
> > >
> > >You can transmit *short* arbitrary pulse-space streams with the zilog
> > >chip, by feeding it a parameter block that has the pulse timing
> > >information and then subsequently has been obfuscated.  The firmware
> > >file that LIRC uses in userspace is full of predefined versions of
> > >these things for RC5 and NEC IIRC.  This LIRC firmware file also holds
> > >the (de)obfuscation key.
> > >
> > >I've got a bunch of old notes on this stuff from essentially reverse
> > >engineering the firmware in the Z8.  IANAL, but to me, its use in
> > >developing in-kernel stuff could be dubious.
> > >
> > >Regards,
> > >Andy
> 
> Very interesting.
> 
> I had considered reverse engineering the z8 firmware but I never found a
> way to access it. I guess we have three options:
> 
> 1. I could use Andy's notes to implement Tx. I have not seen the original
>    firmware code so I'm not contaminated by reverse engineering it. IANAL 
>    but I thought this is an acceptable way of writing a driver.
> 
> 2. Hauppauge could prove us with documentation to write a driver with.

I tried to get some info about that, but they are unable to get anything
related to this design so far.

So, I think that, if you have some time to dedicate to it, the best would
be to go for  option #1.

> 3. Leave it as-is, lirc_zilog will eventually be deleted from staging as it
>    can't be ported to rc-core.
> 
> 
> Sean
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_*
  2014-11-17 14:59                 ` Mauro Carvalho Chehab
@ 2014-11-20 12:38                   ` Sean Young
  0 siblings, 0 replies; 11+ messages in thread
From: Sean Young @ 2014-11-20 12:38 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Andy Walls, Jarod Wilson, Dan Carpenter, Aya Mahfouz, linux-media,
	Greg Kroah-Hartman

On Mon, Nov 17, 2014 at 12:59:09PM -0200, Mauro Carvalho Chehab wrote:
> Em Sun, 9 Nov 2014 21:35:17 +0000
> Sean Young <sean@mess.org> escreveu:
> 
> > On Thu, Nov 06, 2014 at 08:56:47AM -0500, Andy Walls wrote:
> > > On November 6, 2014 8:54:28 AM EST, Andy Walls <awalls.cx18@gmail.com> wrote:
> > > >Sean,
> > > >
> > > >Ir-kbd-i2c was never intended for Tx.
> > > >
> > > >You can transmit *short* arbitrary pulse-space streams with the zilog
> > > >chip, by feeding it a parameter block that has the pulse timing
> > > >information and then subsequently has been obfuscated.  The firmware
> > > >file that LIRC uses in userspace is full of predefined versions of
> > > >these things for RC5 and NEC IIRC.  This LIRC firmware file also holds
> > > >the (de)obfuscation key.
> > > >
> > > >I've got a bunch of old notes on this stuff from essentially reverse
> > > >engineering the firmware in the Z8.  IANAL, but to me, its use in
> > > >developing in-kernel stuff could be dubious.
> > > >
> > > >Regards,
> > > >Andy
> > 
> > Very interesting.
> > 
> > I had considered reverse engineering the z8 firmware but I never found a
> > way to access it. I guess we have three options:
> > 
> > 1. I could use Andy's notes to implement Tx. I have not seen the original
> >    firmware code so I'm not contaminated by reverse engineering it. IANAL 
> >    but I thought this is an acceptable way of writing a driver.
> > 
> > 2. Hauppauge could prove us with documentation to write a driver with.
> 
> I tried to get some info about that, but they are unable to get anything
> related to this design so far.
> 
> So, I think that, if you have some time to dedicate to it, the best would
> be to go for  option #1.

Ok, thanks for asking.

Andy -- if you please send your notes please, I can work on implementing
the driver. I have hardware and time for to work on this.

Thanks

Sean

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-11-20 12:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-31 13:06 staging: media: lirc: lirc_zilog.c: replace custom print macros with dev_* and pr_* Dan Carpenter
2014-10-31 14:26 ` Aya Mahfouz
2014-10-31 14:35   ` Dan Carpenter
2014-11-01 20:05     ` Aya Mahfouz
2014-11-06 12:46     ` Sean Young
2014-11-06 13:05       ` Mauro Carvalho Chehab
2014-11-06 13:21         ` Sean Young
     [not found]           ` <697D038C-4BD9-4113-8E7E-B89BACF09AC2@gmail.com>
2014-11-06 13:56             ` Andy Walls
2014-11-09 21:35               ` Sean Young
2014-11-17 14:59                 ` Mauro Carvalho Chehab
2014-11-20 12:38                   ` Sean Young

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).