Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices

[David Härdeman]

October 27, 2016 4:36 PM, "Sean Young"  wrote:
> Since we have to be able to switch between waiting and not waiting,
> we need some sort of ABI for this. I think this warrants a new ioctl;
> I'm not sure how else it can be done. I'll be sending out a patch
> shortly.

Hi Sean,

have you considered using a module param for the LIRC bridge module instead? As 
far as I understand it, this is an issue which is entirely internal to LIRC, 
and it's also not something which really needs to be changed on a per-device 
level (either you have a modern LIRC daemon or you don't, and all drivers 
should behave the same, no?).

Another advantage is that the parameter would then go away if and when the lirc 
bridge ever goes away (yes, I can still dream, can't I?).

Regards,
David

Re: [PATCH v2 5/7] [media] ir-lirc-codec: don't wait any transmitting time for tx only devices

[David Härdeman]

October 27, 2016 4:36 PM, "Sean Young"  wrote:
> Since we have to be able to switch between waiting and not waiting,
> we need some sort of ABI for this. I think this warrants a new ioctl;
> I'm not sure how else it can be done. I'll be sending out a patch
> shortly.

Hi Sean,

have you considered using a module param for the LIRC bridge module instead? As 
far as I understand it, this is an issue which is entirely internal to LIRC, 
and it's also not something which really needs to be changed on a per-device 
level (either you have a modern LIRC daemon or you don't, and all drivers 
should behave the same, no?).

Another advantage is that the parameter would then go away if and when the lirc 
bridge ever goes away (yes, I can still dream, can't I?).

Regards,
David

Re: [media] winbond-cir: Move assignments for three variables in wbcir_shutdown()

[David Härdeman]

October 19, 2016 3:38 PM, "SF Markus Elfring"  
wrote:
>>> Move the setting for the local variables "mask", "match" and "rc6_csl"
>>> behind the source code for a condition check by this function
>>> at the beginning.
>> 
>> Again, I can't see what the point is?
> 
> * How do you think about to set these variables only after the initial
> check succeded?

I prefer setting variables early so that no thinking about whether they're 
initialized or not is necessary later. 

> * Do you care for data access locality?

Not unless you can show measurable performance improvements?

Re: [media] winbond-cir: Move assignments for three variables in wbcir_shutdown()

[David Härdeman]

October 19, 2016 3:38 PM, "SF Markus Elfring"  
wrote:
>>> Move the setting for the local variables "mask", "match" and "rc6_csl"
>>> behind the source code for a condition check by this function
>>> at the beginning.
>> 
>> Again, I can't see what the point is?
> 
> * How do you think about to set these variables only after the initial
> check succeded?

I prefer setting variables early so that no thinking about whether they're 
initialized or not is necessary later. 

> * Do you care for data access locality?

Not unless you can show measurable performance improvements?

Re: [PATCH 2/5] [media] winbond-cir: Move a variable assignment in wbcir_tx()

[David Härdeman]

October 14, 2016 1:42 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 07:34:46 +0200
> 
> Move the assignment for the local variable "data" behind the source code
> for a memory allocation by this function.

Sorry, I can't see what the point is?


> Signed-off-by: Markus Elfring 
> ---
> drivers/media/rc/winbond-cir.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 59050f5..fd997f0 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -655,7 +655,7 @@ wbcir_txmask(struct rc_dev *dev, u32 mask)
> static int
> wbcir_tx(struct rc_dev *dev, unsigned *b, unsigned count)
> {
> - struct wbcir_data *data = dev->priv;
> + struct wbcir_data *data;
> unsigned *buf;
> unsigned i;
> unsigned long flags;
> @@ -668,6 +668,7 @@ wbcir_tx(struct rc_dev *dev, unsigned *b, unsigned count)
> for (i = 0; i < count; i++)
> buf[i] = DIV_ROUND_CLOSEST(b[i], 10);
> 
> + data = dev->priv;
> /* Not sure if this is possible, but better safe than sorry */
> spin_lock_irqsave(>spinlock, flags);
> if (data->txstate != WBCIR_TXSTATE_INACTIVE) {
> -- 
> 2.10.1

Re: [PATCH 2/5] [media] winbond-cir: Move a variable assignment in wbcir_tx()

[David Härdeman]

October 14, 2016 1:42 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 07:34:46 +0200
> 
> Move the assignment for the local variable "data" behind the source code
> for a memory allocation by this function.

Sorry, I can't see what the point is?


> Signed-off-by: Markus Elfring 
> ---
> drivers/media/rc/winbond-cir.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 59050f5..fd997f0 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -655,7 +655,7 @@ wbcir_txmask(struct rc_dev *dev, u32 mask)
> static int
> wbcir_tx(struct rc_dev *dev, unsigned *b, unsigned count)
> {
> - struct wbcir_data *data = dev->priv;
> + struct wbcir_data *data;
> unsigned *buf;
> unsigned i;
> unsigned long flags;
> @@ -668,6 +668,7 @@ wbcir_tx(struct rc_dev *dev, unsigned *b, unsigned count)
> for (i = 0; i < count; i++)
> buf[i] = DIV_ROUND_CLOSEST(b[i], 10);
> 
> + data = dev->priv;
> /* Not sure if this is possible, but better safe than sorry */
> spin_lock_irqsave(>spinlock, flags);
> if (data->txstate != WBCIR_TXSTATE_INACTIVE) {
> -- 
> 2.10.1

Re: [PATCH 1/5] [media] winbond-cir: Use kmalloc_array() in wbcir_tx()

[David Härdeman]

October 14, 2016 1:41 PM, "SF Markus Elfring" <elfr...@users.sourceforge.net> 
wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Fri, 14 Oct 2016 07:19:00 +0200
> 
> A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>

Sure...why not...

Signed-off-by: David Härdeman <da...@hardeman.nu>


> ---
> drivers/media/rc/winbond-cir.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 95ae60e..59050f5 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -660,7 +660,7 @@ wbcir_tx(struct rc_dev *dev, unsigned *b, unsigned count)
> unsigned i;
> unsigned long flags;
> 
> - buf = kmalloc(count * sizeof(*b), GFP_KERNEL);
> + buf = kmalloc_array(count, sizeof(*b), GFP_KERNEL);
> if (!buf)
> return -ENOMEM;
> 
> -- 
> 2.10.1

Re: [PATCH 1/5] [media] winbond-cir: Use kmalloc_array() in wbcir_tx()

[David Härdeman]

October 14, 2016 1:41 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 07:19:00 +0200
> 
> A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "kmalloc_array".
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring 

Sure...why not...

Signed-off-by: David Härdeman 


> ---
> drivers/media/rc/winbond-cir.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 95ae60e..59050f5 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -660,7 +660,7 @@ wbcir_tx(struct rc_dev *dev, unsigned *b, unsigned count)
> unsigned i;
> unsigned long flags;
> 
> - buf = kmalloc(count * sizeof(*b), GFP_KERNEL);
> + buf = kmalloc_array(count, sizeof(*b), GFP_KERNEL);
> if (!buf)
> return -ENOMEM;
> 
> -- 
> 2.10.1

Re: [PATCH 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection

[David Härdeman]

October 15, 2016 3:30 PM, "Sean Young"  wrote:
> On Fri, Oct 14, 2016 at 01:44:02PM +0200, SF Markus Elfring wrote:
> 
>> From: Markus Elfring 
>> Date: Fri, 14 Oct 2016 12:48:41 +0200
>> 
>> The local variable "do_wake" was set to "false" after an invalid system
>> setting was detected so that a bit of error handling was triggered.
>> 
>> * Replace these assignments by direct jumps to the source code with the
>> desired exception handling.
>> 
>> * Delete this status variable and a corresponding check which became
>> unnecessary with this refactoring.
>> 
>> Signed-off-by: Markus Elfring 
>> ---
>> drivers/media/rc/winbond-cir.c | 78 
>> ++
>> 1 file changed, 34 insertions(+), 44 deletions(-)
>> 
>> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
>> index 9d05e17..3d286b9 100644
>> --- a/drivers/media/rc/winbond-cir.c
>> +++ b/drivers/media/rc/winbond-cir.c
>> @@ -699,16 +699,13 @@ wbcir_shutdown(struct pnp_dev *device)
>> {
>> struct device *dev = >dev;
>> struct wbcir_data *data = pnp_get_drvdata(device);
>> - bool do_wake = true;
>> u8 match[11];
>> u8 mask[11];
>> u8 rc6_csl;
>> int i;
>> 
>> - if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) {
>> - do_wake = false;
>> - goto finish;
>> - }
>> + if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev))
>> + goto clear_bits;
>> 
>> rc6_csl = 0;
>> memset(match, 0, sizeof(match));
>> @@ -716,9 +713,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> switch (protocol) {
>> case IR_PROTOCOL_RC5:
>> if (wake_sc > 0xFFF) {
>> - do_wake = false;
>> dev_err(dev, "RC5 - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> /* Mask = 13 bits, ex toggle */
>> @@ -735,9 +731,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> 
>> case IR_PROTOCOL_NEC:
>> if (wake_sc > 0xFF) {
>> - do_wake = false;
>> dev_err(dev, "NEC - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> mask[0] = mask[1] = mask[2] = mask[3] = 0xFF;
>> @@ -757,9 +752,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> 
>> if (wake_rc6mode == 0) {
>> if (wake_sc > 0x) {
>> - do_wake = false;
>> dev_err(dev, "RC6 - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> /* Command */
>> @@ -813,9 +807,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> } else if (wake_sc <= 0x007F) {
>> rc6_csl = 60;
>> } else {
>> - do_wake = false;
>> dev_err(dev, "RC6 - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> /* Header */
>> @@ -825,49 +818,38 @@ wbcir_shutdown(struct pnp_dev *device)
>> mask[i++] = 0x0F;
>> 
>> } else {
>> - do_wake = false;
>> dev_err(dev, "RC6 - Invalid wake mode\n");
>> + goto clear_bits;
>> }
>> 
>> break;
>> 
>> default:
>> - do_wake = false;
>> - break;
>> + goto clear_bits;
>> }
>> 
>> -finish:
>> - if (do_wake) {
>> - /* Set compare and compare mask */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> - WBCIR_REGSEL_COMPARE | WBCIR_REG_ADDR0,
>> - 0x3F);
>> - outsb(data->wbase + WBCIR_REG_WCEIR_DATA, match, 11);
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> - WBCIR_REGSEL_MASK | WBCIR_REG_ADDR0,
>> - 0x3F);
>> - outsb(data->wbase + WBCIR_REG_WCEIR_DATA, mask, 11);
>> -
>> - /* RC6 Compare String Len */
>> - outb(rc6_csl, data->wbase + WBCIR_REG_WCEIR_CSL);
>> -
>> - /* Clear status bits NEC_REP, BUFF, MSG_END, MATCH */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_STS, 0x17, 0x17);
>> + /* Set compare and compare mask */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> + WBCIR_REGSEL_COMPARE | WBCIR_REG_ADDR0,
>> + 0x3F);
>> + outsb(data->wbase + WBCIR_REG_WCEIR_DATA, match, 11);
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> + WBCIR_REGSEL_MASK | WBCIR_REG_ADDR0,
>> + 0x3F);
>> + outsb(data->wbase + WBCIR_REG_WCEIR_DATA, mask, 11);
>> 
>> - /* Clear BUFF_EN, Clear END_EN, Set MATCH_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x01, 0x07);
>> + /* RC6 Compare String Len */
>> + outb(rc6_csl, data->wbase + WBCIR_REG_WCEIR_CSL);
>> 
>> - /* Set CEIR_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>> -
>> - } else {
>> - /* Clear BUFF_EN, Clear END_EN, Clear MATCH_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
>> + /* Clear status bits NEC_REP, BUFF, MSG_END, MATCH */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_STS, 0x17, 0x17);
>> 
>> - /* Clear CEIR_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x00, 0x01);
>> - }
>> + /* Clear BUFF_EN, Clear END_EN, Set MATCH_EN */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x01, 0x07);
>> 
>> + /* Set CEIR_EN */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>> +set_irqmask:
>> /*
>> * ACPI will set the HW disable bit for SP3 which means that the
>> * output signals are left in an undefined state which may cause
>> @@ -876,6 +858,14 @@ 

Re: [PATCH 5/5] [media] winbond-cir: Move a variable assignment in two functions

[David Härdeman]

October 14, 2016 1:45 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 13:13:11 +0200
> 
> Move the assignment for the local variable "data" behind the source code
> for condition checks by these functions.

Why?

> Signed-off-by: Markus Elfring 
> ---
> drivers/media/rc/winbond-cir.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 3d286b9..716b1fe 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -566,7 +566,7 @@ wbcir_set_carrier_report(struct rc_dev *dev, int enable)
> static int
> wbcir_txcarrier(struct rc_dev *dev, u32 carrier)
> {
> - struct wbcir_data *data = dev->priv;
> + struct wbcir_data *data;
> unsigned long flags;
> u8 val;
> u32 freq;
> @@ -592,6 +592,7 @@ wbcir_txcarrier(struct rc_dev *dev, u32 carrier)
> break;
> }
> 
> + data = dev->priv;
> spin_lock_irqsave(>spinlock, flags);
> if (data->txstate != WBCIR_TXSTATE_INACTIVE) {
> spin_unlock_irqrestore(>spinlock, flags);
> @@ -611,7 +612,7 @@ wbcir_txcarrier(struct rc_dev *dev, u32 carrier)
> static int
> wbcir_txmask(struct rc_dev *dev, u32 mask)
> {
> - struct wbcir_data *data = dev->priv;
> + struct wbcir_data *data;
> unsigned long flags;
> u8 val;
> 
> @@ -637,6 +638,7 @@ wbcir_txmask(struct rc_dev *dev, u32 mask)
> return -EINVAL;
> }
> 
> + data = dev->priv;
> spin_lock_irqsave(>spinlock, flags);
> if (data->txstate != WBCIR_TXSTATE_INACTIVE) {
> spin_unlock_irqrestore(>spinlock, flags);
> -- 
> 2.10.1

Re: [PATCH 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection

[David Härdeman]

October 15, 2016 3:30 PM, "Sean Young"  wrote:
> On Fri, Oct 14, 2016 at 01:44:02PM +0200, SF Markus Elfring wrote:
> 
>> From: Markus Elfring 
>> Date: Fri, 14 Oct 2016 12:48:41 +0200
>> 
>> The local variable "do_wake" was set to "false" after an invalid system
>> setting was detected so that a bit of error handling was triggered.
>> 
>> * Replace these assignments by direct jumps to the source code with the
>> desired exception handling.
>> 
>> * Delete this status variable and a corresponding check which became
>> unnecessary with this refactoring.
>> 
>> Signed-off-by: Markus Elfring 
>> ---
>> drivers/media/rc/winbond-cir.c | 78 
>> ++
>> 1 file changed, 34 insertions(+), 44 deletions(-)
>> 
>> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
>> index 9d05e17..3d286b9 100644
>> --- a/drivers/media/rc/winbond-cir.c
>> +++ b/drivers/media/rc/winbond-cir.c
>> @@ -699,16 +699,13 @@ wbcir_shutdown(struct pnp_dev *device)
>> {
>> struct device *dev = >dev;
>> struct wbcir_data *data = pnp_get_drvdata(device);
>> - bool do_wake = true;
>> u8 match[11];
>> u8 mask[11];
>> u8 rc6_csl;
>> int i;
>> 
>> - if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) {
>> - do_wake = false;
>> - goto finish;
>> - }
>> + if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev))
>> + goto clear_bits;
>> 
>> rc6_csl = 0;
>> memset(match, 0, sizeof(match));
>> @@ -716,9 +713,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> switch (protocol) {
>> case IR_PROTOCOL_RC5:
>> if (wake_sc > 0xFFF) {
>> - do_wake = false;
>> dev_err(dev, "RC5 - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> /* Mask = 13 bits, ex toggle */
>> @@ -735,9 +731,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> 
>> case IR_PROTOCOL_NEC:
>> if (wake_sc > 0xFF) {
>> - do_wake = false;
>> dev_err(dev, "NEC - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> mask[0] = mask[1] = mask[2] = mask[3] = 0xFF;
>> @@ -757,9 +752,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> 
>> if (wake_rc6mode == 0) {
>> if (wake_sc > 0x) {
>> - do_wake = false;
>> dev_err(dev, "RC6 - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> /* Command */
>> @@ -813,9 +807,8 @@ wbcir_shutdown(struct pnp_dev *device)
>> } else if (wake_sc <= 0x007F) {
>> rc6_csl = 60;
>> } else {
>> - do_wake = false;
>> dev_err(dev, "RC6 - Invalid wake scancode\n");
>> - break;
>> + goto clear_bits;
>> }
>> 
>> /* Header */
>> @@ -825,49 +818,38 @@ wbcir_shutdown(struct pnp_dev *device)
>> mask[i++] = 0x0F;
>> 
>> } else {
>> - do_wake = false;
>> dev_err(dev, "RC6 - Invalid wake mode\n");
>> + goto clear_bits;
>> }
>> 
>> break;
>> 
>> default:
>> - do_wake = false;
>> - break;
>> + goto clear_bits;
>> }
>> 
>> -finish:
>> - if (do_wake) {
>> - /* Set compare and compare mask */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> - WBCIR_REGSEL_COMPARE | WBCIR_REG_ADDR0,
>> - 0x3F);
>> - outsb(data->wbase + WBCIR_REG_WCEIR_DATA, match, 11);
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> - WBCIR_REGSEL_MASK | WBCIR_REG_ADDR0,
>> - 0x3F);
>> - outsb(data->wbase + WBCIR_REG_WCEIR_DATA, mask, 11);
>> -
>> - /* RC6 Compare String Len */
>> - outb(rc6_csl, data->wbase + WBCIR_REG_WCEIR_CSL);
>> -
>> - /* Clear status bits NEC_REP, BUFF, MSG_END, MATCH */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_STS, 0x17, 0x17);
>> + /* Set compare and compare mask */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> + WBCIR_REGSEL_COMPARE | WBCIR_REG_ADDR0,
>> + 0x3F);
>> + outsb(data->wbase + WBCIR_REG_WCEIR_DATA, match, 11);
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_INDEX,
>> + WBCIR_REGSEL_MASK | WBCIR_REG_ADDR0,
>> + 0x3F);
>> + outsb(data->wbase + WBCIR_REG_WCEIR_DATA, mask, 11);
>> 
>> - /* Clear BUFF_EN, Clear END_EN, Set MATCH_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x01, 0x07);
>> + /* RC6 Compare String Len */
>> + outb(rc6_csl, data->wbase + WBCIR_REG_WCEIR_CSL);
>> 
>> - /* Set CEIR_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>> -
>> - } else {
>> - /* Clear BUFF_EN, Clear END_EN, Clear MATCH_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
>> + /* Clear status bits NEC_REP, BUFF, MSG_END, MATCH */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_STS, 0x17, 0x17);
>> 
>> - /* Clear CEIR_EN */
>> - wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x00, 0x01);
>> - }
>> + /* Clear BUFF_EN, Clear END_EN, Set MATCH_EN */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x01, 0x07);
>> 
>> + /* Set CEIR_EN */
>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>> +set_irqmask:
>> /*
>> * ACPI will set the HW disable bit for SP3 which means that the
>> * output signals are left in an undefined state which may cause
>> @@ -876,6 +858,14 @@ wbcir_shutdown(struct pnp_dev *device)
>> */
>> wbcir_set_irqmask(data, 

Re: [PATCH 5/5] [media] winbond-cir: Move a variable assignment in two functions

[David Härdeman]

October 14, 2016 1:45 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 13:13:11 +0200
> 
> Move the assignment for the local variable "data" behind the source code
> for condition checks by these functions.

Why?

> Signed-off-by: Markus Elfring 
> ---
> drivers/media/rc/winbond-cir.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index 3d286b9..716b1fe 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -566,7 +566,7 @@ wbcir_set_carrier_report(struct rc_dev *dev, int enable)
> static int
> wbcir_txcarrier(struct rc_dev *dev, u32 carrier)
> {
> - struct wbcir_data *data = dev->priv;
> + struct wbcir_data *data;
> unsigned long flags;
> u8 val;
> u32 freq;
> @@ -592,6 +592,7 @@ wbcir_txcarrier(struct rc_dev *dev, u32 carrier)
> break;
> }
> 
> + data = dev->priv;
> spin_lock_irqsave(>spinlock, flags);
> if (data->txstate != WBCIR_TXSTATE_INACTIVE) {
> spin_unlock_irqrestore(>spinlock, flags);
> @@ -611,7 +612,7 @@ wbcir_txcarrier(struct rc_dev *dev, u32 carrier)
> static int
> wbcir_txmask(struct rc_dev *dev, u32 mask)
> {
> - struct wbcir_data *data = dev->priv;
> + struct wbcir_data *data;
> unsigned long flags;
> u8 val;
> 
> @@ -637,6 +638,7 @@ wbcir_txmask(struct rc_dev *dev, u32 mask)
> return -EINVAL;
> }
> 
> + data = dev->priv;
> spin_lock_irqsave(>spinlock, flags);
> if (data->txstate != WBCIR_TXSTATE_INACTIVE) {
> spin_unlock_irqrestore(>spinlock, flags);
> -- 
> 2.10.1

Re: [media] winbond-cir: Move a variable assignment in wbcir_tx()

[David Härdeman]

October 19, 2016 3:32 PM, "SF Markus Elfring"  
wrote:
>>> Move the assignment for the local variable "data" behind the source code
>>> for a memory allocation by this function.
>> 
>> Sorry, I can't see what the point is?
> 
> * How do you think about to avoid a variable assignment in case
> that this memory allocation failed anyhow?

There is no memory allocation that can fail at this point.

> * Do you care for data access locality?

Not unless you can show measurable performance improvements?

Re: [PATCH 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection

[David Härdeman]

October 15, 2016 6:42 PM, "SF Markus Elfring"  
wrote:
>>> + /* Set CEIR_EN */
>>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>>> +set_irqmask:
>>> /*
>>> * ACPI will set the HW disable bit for SP3 which means that the
>>> * output signals are left in an undefined state which may cause
>>> @@ -876,6 +858,14 @@ wbcir_shutdown(struct pnp_dev *device)
>>> */
>>> wbcir_set_irqmask(data, WBCIR_IRQ_NONE);
>>> disable_irq(data->irq);
>>> + return;
>>> +clear_bits:
>>> + /* Clear BUFF_EN, Clear END_EN, Clear MATCH_EN */
>>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
>>> +
>>> + /* Clear CEIR_EN */
>>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x00, 0x01);
>>> + goto set_irqmask;
>> 
>> I'm not convinced that adding a goto which goes backwards is making this
>> code any more readible, just so that a local variable can be dropped.
> 
> Thanks for your feedback.
> 
> Is such a "backward jump" usual and finally required when you would like
> to move a bit of common error handling code to the end without using extra
> local variables and a few statements should still be performed after it?
> 

I'm sorry, I can't parse this.

Re: [media] winbond-cir: Move a variable assignment in wbcir_tx()

[David Härdeman]

October 19, 2016 3:32 PM, "SF Markus Elfring"  
wrote:
>>> Move the assignment for the local variable "data" behind the source code
>>> for a memory allocation by this function.
>> 
>> Sorry, I can't see what the point is?
> 
> * How do you think about to avoid a variable assignment in case
> that this memory allocation failed anyhow?

There is no memory allocation that can fail at this point.

> * Do you care for data access locality?

Not unless you can show measurable performance improvements?

Re: [PATCH 4/5] [media] winbond-cir: One variable and its check less in wbcir_shutdown() after error detection

[David Härdeman]

October 15, 2016 6:42 PM, "SF Markus Elfring"  
wrote:
>>> + /* Set CEIR_EN */
>>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x01, 0x01);
>>> +set_irqmask:
>>> /*
>>> * ACPI will set the HW disable bit for SP3 which means that the
>>> * output signals are left in an undefined state which may cause
>>> @@ -876,6 +858,14 @@ wbcir_shutdown(struct pnp_dev *device)
>>> */
>>> wbcir_set_irqmask(data, WBCIR_IRQ_NONE);
>>> disable_irq(data->irq);
>>> + return;
>>> +clear_bits:
>>> + /* Clear BUFF_EN, Clear END_EN, Clear MATCH_EN */
>>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
>>> +
>>> + /* Clear CEIR_EN */
>>> + wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_CTL, 0x00, 0x01);
>>> + goto set_irqmask;
>> 
>> I'm not convinced that adding a goto which goes backwards is making this
>> code any more readible, just so that a local variable can be dropped.
> 
> Thanks for your feedback.
> 
> Is such a "backward jump" usual and finally required when you would like
> to move a bit of common error handling code to the end without using extra
> local variables and a few statements should still be performed after it?
> 

I'm sorry, I can't parse this.

Re: [PATCH 3/5] [media] winbond-cir: Move assignments for three variables in wbcir_shutdown()

[David Härdeman]

October 14, 2016 1:43 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 10:40:12 +0200
> 
> Move the setting for the local variables "mask", "match" and "rc6_csl"
> behind the source code for a condition check by this function
> at the beginning.
 
Again, I can't see what the point is?

> Signed-off-by: Markus Elfring 
> ---
> drivers/media/rc/winbond-cir.c | 8 
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index fd997f0..9d05e17 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -702,17 +702,17 @@ wbcir_shutdown(struct pnp_dev *device)
> bool do_wake = true;
> u8 match[11];
> u8 mask[11];
> - u8 rc6_csl = 0;
> + u8 rc6_csl;
> int i;
> 
> - memset(match, 0, sizeof(match));
> - memset(mask, 0, sizeof(mask));
> -
> if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) {
> do_wake = false;
> goto finish;
> }
> 
> + rc6_csl = 0;
> + memset(match, 0, sizeof(match));
> + memset(mask, 0, sizeof(mask));
> switch (protocol) {
> case IR_PROTOCOL_RC5:
> if (wake_sc > 0xFFF) {
> -- 
> 2.10.1

Re: [PATCH 3/5] [media] winbond-cir: Move assignments for three variables in wbcir_shutdown()

[David Härdeman]

October 14, 2016 1:43 PM, "SF Markus Elfring"  
wrote:
> From: Markus Elfring 
> Date: Fri, 14 Oct 2016 10:40:12 +0200
> 
> Move the setting for the local variables "mask", "match" and "rc6_csl"
> behind the source code for a condition check by this function
> at the beginning.
 
Again, I can't see what the point is?

> Signed-off-by: Markus Elfring 
> ---
> drivers/media/rc/winbond-cir.c | 8 
> 1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c
> index fd997f0..9d05e17 100644
> --- a/drivers/media/rc/winbond-cir.c
> +++ b/drivers/media/rc/winbond-cir.c
> @@ -702,17 +702,17 @@ wbcir_shutdown(struct pnp_dev *device)
> bool do_wake = true;
> u8 match[11];
> u8 mask[11];
> - u8 rc6_csl = 0;
> + u8 rc6_csl;
> int i;
> 
> - memset(match, 0, sizeof(match));
> - memset(mask, 0, sizeof(mask));
> -
> if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) {
> do_wake = false;
> goto finish;
> }
> 
> + rc6_csl = 0;
> + memset(match, 0, sizeof(match));
> + memset(mask, 0, sizeof(mask));
> switch (protocol) {
> case IR_PROTOCOL_RC5:
> if (wake_sc > 0xFFF) {
> -- 
> 2.10.1

Re: [PATCH] Fix RC5 decoding with Fintek CIR chipset

[David Härdeman]

On Sat, May 14, 2016 at 06:01:26PM +0100, Jonathan McDowell wrote:
>Fix RC5 decoding with Fintek CIR chipset
>
>Commit e87b540be2dd02552fb9244d50ae8b4e4619a34b tightened up the RC5
>decoding by adding a check for trailing silence to ensure a valid RC5
>command had been received. Unfortunately the trailer length checked was
>10 units and the Fintek CIR device does not want to provide details of a
>space longer than 6350us. This meant that RC5 remotes working on a
>Fintek setup on 3.16 failed on 3.17 and later. Fix this by shortening
>the trailer check to 6 units (allowing for a previous space in the
>received remote command).
>
>Signed-off-by: Jonathan McDowell <nood...@earth.li>
Signed-off-by: David Härdeman <da...@hardeman.nu>

>Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=117221
>Cc: sta...@vger.kernel.org
>
>-
>diff --git a/drivers/media/rc/ir-rc5-decoder.c 
>b/drivers/media/rc/ir-rc5-decoder.c
>index 6ffe776..a0fd4e6 100644
>--- a/drivers/media/rc/ir-rc5-decoder.c
>+++ b/drivers/media/rc/ir-rc5-decoder.c
>@@ -29,7 +29,7 @@
> #define RC5_BIT_START (1 * RC5_UNIT)
> #define RC5_BIT_END   (1 * RC5_UNIT)
> #define RC5X_SPACE(4 * RC5_UNIT)
>-#define RC5_TRAILER   (10 * RC5_UNIT) /* In reality, approx 100 */
>+#define RC5_TRAILER   (6 * RC5_UNIT) /* In reality, approx 100 */
> 
> enum rc5_state {
>   STATE_INACTIVE,
>-
>
>J.
>
>-- 
>What did the first punk rock girl wear to your school?
>

-- 
David Härdeman

Re: [PATCH] Fix RC5 decoding with Fintek CIR chipset

[David Härdeman]

On Sat, May 14, 2016 at 06:01:26PM +0100, Jonathan McDowell wrote:
>Fix RC5 decoding with Fintek CIR chipset
>
>Commit e87b540be2dd02552fb9244d50ae8b4e4619a34b tightened up the RC5
>decoding by adding a check for trailing silence to ensure a valid RC5
>command had been received. Unfortunately the trailer length checked was
>10 units and the Fintek CIR device does not want to provide details of a
>space longer than 6350us. This meant that RC5 remotes working on a
>Fintek setup on 3.16 failed on 3.17 and later. Fix this by shortening
>the trailer check to 6 units (allowing for a previous space in the
>received remote command).
>
>Signed-off-by: Jonathan McDowell 
Signed-off-by: David Härdeman 

>Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=117221
>Cc: sta...@vger.kernel.org
>
>-
>diff --git a/drivers/media/rc/ir-rc5-decoder.c 
>b/drivers/media/rc/ir-rc5-decoder.c
>index 6ffe776..a0fd4e6 100644
>--- a/drivers/media/rc/ir-rc5-decoder.c
>+++ b/drivers/media/rc/ir-rc5-decoder.c
>@@ -29,7 +29,7 @@
> #define RC5_BIT_START (1 * RC5_UNIT)
> #define RC5_BIT_END   (1 * RC5_UNIT)
> #define RC5X_SPACE(4 * RC5_UNIT)
>-#define RC5_TRAILER   (10 * RC5_UNIT) /* In reality, approx 100 */
>+#define RC5_TRAILER   (6 * RC5_UNIT) /* In reality, approx 100 */
> 
> enum rc5_state {
>   STATE_INACTIVE,
>-----
>
>J.
>
>-- 
>What did the first punk rock girl wear to your school?
>

-- 
David Härdeman

Re: mceusb: sysfs: cannot create duplicate filename '/class/rc/rc0' (race condition between multiple RC_CORE devices)

[David Härdeman]

On 2015-03-30 22:21, Stefan Lippers-Hollmann wrote:

On 2015-03-30, David Härdeman wrote:

On 2015-03-30 17:30, Stefan Lippers-Hollmann wrote:
> This is a follow-up for:
>http://lkml.kernel.org/r/<201412181916.18051.s@gmx.de>
>http://lkml.kernel.org/r/<201412302211.40801.s@gmx.de>

I can't swear that it's the case but I'm guessing this might be fixed 
by

the patches I posted earlier (in particular the one that converted
rc-core to use the IDA infrastructure for keeping track of registered
minor device numbers).


Do you have a pointer to that patch (-queue) or a tree containing it?
So far I've only found https://patchwork.linuxtv.org/patch/23370/
with those keywords, respectively the thread at
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/76514
which seems to be partially applied, anything I could test (reproducing
the problem takes its time, probably 4-10 weeks to be really sure, but
I'd be happy to try or forward port the required parts).


Hi,

I can try providing you with an updated version of the patch when I have 
time...otherwise I think you've found all that can be found :)


//David

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mceusb: sysfs: cannot create duplicate filename '/class/rc/rc0' (race condition between multiple RC_CORE devices)

[David Härdeman]

On 2015-03-30 17:30, Stefan Lippers-Hollmann wrote:

Hi

This is a follow-up for:
http://lkml.kernel.org/r/<201412181916.18051.s@gmx.de>
http://lkml.kernel.org/r/<201412302211.40801.s@gmx.de>



I can't swear that it's the case but I'm guessing this might be fixed by 
the patches I posted earlier (in particular the one that converted 
rc-core to use the IDA infrastructure for keeping track of registered 
minor device numbers).


//D


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mceusb: sysfs: cannot create duplicate filename '/class/rc/rc0' (race condition between multiple RC_CORE devices)

[David Härdeman]

On 2015-03-30 17:30, Stefan Lippers-Hollmann wrote:

Hi

This is a follow-up for:
http://lkml.kernel.org/r/201412181916.18051.s@gmx.de
http://lkml.kernel.org/r/201412302211.40801.s@gmx.de



I can't swear that it's the case but I'm guessing this might be fixed by 
the patches I posted earlier (in particular the one that converted 
rc-core to use the IDA infrastructure for keeping track of registered 
minor device numbers).


//D


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mceusb: sysfs: cannot create duplicate filename '/class/rc/rc0' (race condition between multiple RC_CORE devices)

[David Härdeman]

On 2015-03-30 22:21, Stefan Lippers-Hollmann wrote:

On 2015-03-30, David Härdeman wrote:

On 2015-03-30 17:30, Stefan Lippers-Hollmann wrote:
 This is a follow-up for:
http://lkml.kernel.org/r/201412181916.18051.s@gmx.de
http://lkml.kernel.org/r/201412302211.40801.s@gmx.de

I can't swear that it's the case but I'm guessing this might be fixed 
by

the patches I posted earlier (in particular the one that converted
rc-core to use the IDA infrastructure for keeping track of registered
minor device numbers).


Do you have a pointer to that patch (-queue) or a tree containing it?
So far I've only found https://patchwork.linuxtv.org/patch/23370/
with those keywords, respectively the thread at
http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/76514
which seems to be partially applied, anything I could test (reproducing
the problem takes its time, probably 4-10 weeks to be really sure, but
I'd be happy to try or forward port the required parts).


Hi,

I can try providing you with an updated version of the patch when I have 
time...otherwise I think you've found all that can be found :)


//David

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On Tue, Oct 28, 2014 at 08:43:14PM +0200, Tomas Melin wrote:
>IR receiver using nuvoton-cir and lirc required additional configuration
>steps after upgrade from kernel 3.16 to 3.17-rcX.
>Bisected regression to commit da6e162d6a4607362f8478c715c797d84d449f8b
>("[media] rc-core: simplify sysfs code").
>
>The regression comes from adding function change_protocol in
>ir-raw.c. It changes behaviour so that only the protocol enabled by driver's
>map_name will be active after registration. This breaks user space behaviour,
>lirc does not get key press signals anymore.
>
>Enable lirc protocol by default for ir raw decoders to restore original 
>behaviour.
>
>Signed-off-by: Tomas Melin 

Acked-by: David Härdeman 

>---
> drivers/media/rc/rc-ir-raw.c |1 -
> drivers/media/rc/rc-main.c   |2 ++
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
>index e8fff2a..b732ac6 100644
>--- a/drivers/media/rc/rc-ir-raw.c
>+++ b/drivers/media/rc/rc-ir-raw.c
>@@ -262,7 +262,6 @@ int ir_raw_event_register(struct rc_dev *dev)
>   return -ENOMEM;
> 
>   dev->raw->dev = dev;
>-  dev->enabled_protocols = ~0;
>   dev->change_protocol = change_protocol;
>   rc = kfifo_alloc(>raw->kfifo,
>sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
>diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
>index a7991c7..8d3b74c 100644
>--- a/drivers/media/rc/rc-main.c
>+++ b/drivers/media/rc/rc-main.c
>@@ -1421,6 +1421,8 @@ int rc_register_device(struct rc_dev *dev)
> 
>   if (dev->change_protocol) {
>   u64 rc_type = (1 << rc_map->rc_type);
>+  if (dev->driver_type == RC_DRIVER_IR_RAW)
>+  rc_type |= RC_BIT_LIRC;
>   rc = dev->change_protocol(dev, _type);
>   if (rc < 0)
>   goto out_raw;
>-- 
>1.7.10.4
>

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On 2014-10-26 20:33, Tomas Melin wrote:
On Sat, Oct 25, 2014 at 12:03 PM, David Härdeman  
wrote:

Wouldn't something like this be a simpler way of achieving the same
result? (untested):


The idea was to remove the empty change_protocol function that had
been added in the breaking commit.


The empty function was added for a reason? The presence of a 
change_protocol function implies that the receiver supports protocol 
changing (whether it's via the raw IR decoding or in hardware).



IMHO, it would be better to not have functions that don't do anything.
Actually, another problem with that empty function is that if the
driver first sets up a "real" change_protocol function and related
data, and then calls rc_register_device, the driver defined
change_protocol function would be overwritten.


change_protocol is only set if it's a driver that does in-kernel 
decoding...meaning that it generates pulse/space timings...meaning that 
hardware protocol changes aren't necessary?



diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index a7991c7..d521f20 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1421,6 +1421,9 @@ int rc_register_device(struct rc_dev *dev)

if (dev->change_protocol) {
u64 rc_type = (1 << rc_map->rc_type);
+   if (dev->driver_type == RC_DRIVER_IR_RAW)
+   rc_type |= RC_BIT_LIRC;
+
rc = dev->change_protocol(dev, _type);
if (rc < 0)
goto out_raw;


But otherwise yes, your suggestion could work, with the addition that
we still need to update enabled_protocols (and not init
enabled_protocols anymore in ir_raw_event_register() ).


First, enabled_protocols is already taken care of in the above patch 
(the line after "goto out_raw" is "dev->enabled_protocols = rc_type;")?


Second, initializing enabled_protocols to some default in 
ir_raw_event_register() might not be strictly necessary but it also 
doesn't hurt since that happens before dev->change_protocol() is called 
in rc_register_device()?



+   dev->enabled_protocols = (rc_type | RC_BIT_LIRC);

Please let me know your preferences on which you prefer, and, if
needed, I'll make a new patch version.


I'd prefer the above, minimal, approach. But it's Mauro who decides in 
the end.


Regards,
David

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On 2014-10-26 20:33, Tomas Melin wrote:
On Sat, Oct 25, 2014 at 12:03 PM, David Härdeman da...@hardeman.nu 
wrote:

Wouldn't something like this be a simpler way of achieving the same
result? (untested):


The idea was to remove the empty change_protocol function that had
been added in the breaking commit.


The empty function was added for a reason? The presence of a 
change_protocol function implies that the receiver supports protocol 
changing (whether it's via the raw IR decoding or in hardware).



IMHO, it would be better to not have functions that don't do anything.
Actually, another problem with that empty function is that if the
driver first sets up a real change_protocol function and related
data, and then calls rc_register_device, the driver defined
change_protocol function would be overwritten.


change_protocol is only set if it's a driver that does in-kernel 
decoding...meaning that it generates pulse/space timings...meaning that 
hardware protocol changes aren't necessary?



diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index a7991c7..d521f20 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1421,6 +1421,9 @@ int rc_register_device(struct rc_dev *dev)

if (dev-change_protocol) {
u64 rc_type = (1  rc_map-rc_type);
+   if (dev-driver_type == RC_DRIVER_IR_RAW)
+   rc_type |= RC_BIT_LIRC;
+
rc = dev-change_protocol(dev, rc_type);
if (rc  0)
goto out_raw;


But otherwise yes, your suggestion could work, with the addition that
we still need to update enabled_protocols (and not init
enabled_protocols anymore in ir_raw_event_register() ).


First, enabled_protocols is already taken care of in the above patch 
(the line after goto out_raw is dev-enabled_protocols = rc_type;)?


Second, initializing enabled_protocols to some default in 
ir_raw_event_register() might not be strictly necessary but it also 
doesn't hurt since that happens before dev-change_protocol() is called 
in rc_register_device()?



+   dev-enabled_protocols = (rc_type | RC_BIT_LIRC);

Please let me know your preferences on which you prefer, and, if
needed, I'll make a new patch version.


I'd prefer the above, minimal, approach. But it's Mauro who decides in 
the end.


Regards,
David

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On Tue, Oct 28, 2014 at 08:43:14PM +0200, Tomas Melin wrote:
IR receiver using nuvoton-cir and lirc required additional configuration
steps after upgrade from kernel 3.16 to 3.17-rcX.
Bisected regression to commit da6e162d6a4607362f8478c715c797d84d449f8b
([media] rc-core: simplify sysfs code).

The regression comes from adding function change_protocol in
ir-raw.c. It changes behaviour so that only the protocol enabled by driver's
map_name will be active after registration. This breaks user space behaviour,
lirc does not get key press signals anymore.

Enable lirc protocol by default for ir raw decoders to restore original 
behaviour.

Signed-off-by: Tomas Melin tomas.me...@iki.fi

Acked-by: David Härdeman da...@hardeman.nu

---
 drivers/media/rc/rc-ir-raw.c |1 -
 drivers/media/rc/rc-main.c   |2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index e8fff2a..b732ac6 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -262,7 +262,6 @@ int ir_raw_event_register(struct rc_dev *dev)
   return -ENOMEM;
 
   dev-raw-dev = dev;
-  dev-enabled_protocols = ~0;
   dev-change_protocol = change_protocol;
   rc = kfifo_alloc(dev-raw-kfifo,
sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index a7991c7..8d3b74c 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1421,6 +1421,8 @@ int rc_register_device(struct rc_dev *dev)
 
   if (dev-change_protocol) {
   u64 rc_type = (1  rc_map-rc_type);
+  if (dev-driver_type == RC_DRIVER_IR_RAW)
+  rc_type |= RC_BIT_LIRC;
   rc = dev-change_protocol(dev, rc_type);
   if (rc  0)
   goto out_raw;
-- 
1.7.10.4


-- 
David Härdeman
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On Tue, Oct 21, 2014 at 09:30:17PM +0300, Tomas Melin wrote:
>IR reciever using nuvoton-cir and lirc required additional configuration
>steps after upgrade from kernel 3.16 to 3.17-rcX.
>Bisected regression to commit da6e162d6a4607362f8478c715c797d84d449f8b
>("[media] rc-core: simplify sysfs code").
>
>The regression comes from adding empty function change_protocol in
>ir-raw.c. It changes behaviour so that only the protocol enabled by driver's
>map_name will be active after registration. This breaks user space behaviour,
>lirc does not get key press signals anymore.
>
>Changed back to original behaviour by removing empty function
>change_protocol in ir-raw.c. Instead only calling change_protocol for

Wouldn't something like this be a simpler way of achieving the same
result? (untested):


diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index a7991c7..d521f20 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1421,6 +1421,9 @@ int rc_register_device(struct rc_dev *dev)
 
if (dev->change_protocol) {
u64 rc_type = (1 << rc_map->rc_type);
+   if (dev->driver_type == RC_DRIVER_IR_RAW)
+   rc_type |= RC_BIT_LIRC;
+
rc = dev->change_protocol(dev, _type);
if (rc < 0)
goto out_raw;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On Tue, Oct 21, 2014 at 09:30:17PM +0300, Tomas Melin wrote:
IR reciever using nuvoton-cir and lirc required additional configuration
steps after upgrade from kernel 3.16 to 3.17-rcX.
Bisected regression to commit da6e162d6a4607362f8478c715c797d84d449f8b
([media] rc-core: simplify sysfs code).

The regression comes from adding empty function change_protocol in
ir-raw.c. It changes behaviour so that only the protocol enabled by driver's
map_name will be active after registration. This breaks user space behaviour,
lirc does not get key press signals anymore.

Changed back to original behaviour by removing empty function
change_protocol in ir-raw.c. Instead only calling change_protocol for

Wouldn't something like this be a simpler way of achieving the same
result? (untested):


diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index a7991c7..d521f20 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1421,6 +1421,9 @@ int rc_register_device(struct rc_dev *dev)
 
if (dev-change_protocol) {
u64 rc_type = (1  rc_map-rc_type);
+   if (dev-driver_type == RC_DRIVER_IR_RAW)
+   rc_type |= RC_BIT_LIRC;
+
rc = dev-change_protocol(dev, rc_type);
if (rc  0)
goto out_raw;

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] [media] rc-core: change enabled_protocol default setting

[David Härdeman]

On 2014-10-19 12:21, Tomas Melin wrote:

Change default setting for enabled protocols.
Instead of enabling all protocols, disable all except lirc during 
registration.

Reduces overhead since all protocols not handled by default.
Protocol to use will be enabled when keycode table is written by 
userspace.


I can see the appeal in this, but now you've disabled automatic decoding 
for the protocol specified by the keymap for raw drivers? So this would 
also be a change, right?


I agree with Mauro that the "proper" long-term fix would be to teach the 
LIRC userspace daemon to enable the lirc protocol as/when necessary, but 
something similar to the patch below (but lirc + keymap protocol...if 
that's possible to implement in a non-intrusive manner, I haven't 
checked TBH) might be a good idea as an interim measure?





Signed-off-by: Tomas Melin 
---
 drivers/media/rc/rc-ir-raw.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/rc-ir-raw.c 
b/drivers/media/rc/rc-ir-raw.c

index a118539..63d23d0 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -256,7 +256,8 @@ int ir_raw_event_register(struct rc_dev *dev)
return -ENOMEM;

dev->raw->dev = dev;
-   dev->enabled_protocols = ~0;
+   /* by default, disable all but lirc*/
+   dev->enabled_protocols = RC_BIT_LIRC;
rc = kfifo_alloc(>raw->kfifo,
 sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
 GFP_KERNEL);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On 2014-10-19 12:21, Tomas Melin wrote:

IR reciever using nuvoton-cir and lirc was not working anymore after
upgrade from kernel 3.16 to 3.17-rcX.
Bisected regression to commit da6e162d6a4607362f8478c715c797d84d449f8b
("[media] rc-core: simplify sysfs code").


FWIW, I think "not working" is slightly misleading. "Required additional 
configuration steps" is a better description, isn't it?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/2] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On 2014-10-19 12:21, Tomas Melin wrote:

IR reciever using nuvoton-cir and lirc was not working anymore after
upgrade from kernel 3.16 to 3.17-rcX.
Bisected regression to commit da6e162d6a4607362f8478c715c797d84d449f8b
([media] rc-core: simplify sysfs code).


FWIW, I think not working is slightly misleading. Required additional 
configuration steps is a better description, isn't it?


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] [media] rc-core: change enabled_protocol default setting

[David Härdeman]

On 2014-10-19 12:21, Tomas Melin wrote:

Change default setting for enabled protocols.
Instead of enabling all protocols, disable all except lirc during 
registration.

Reduces overhead since all protocols not handled by default.
Protocol to use will be enabled when keycode table is written by 
userspace.


I can see the appeal in this, but now you've disabled automatic decoding 
for the protocol specified by the keymap for raw drivers? So this would 
also be a change, right?


I agree with Mauro that the proper long-term fix would be to teach the 
LIRC userspace daemon to enable the lirc protocol as/when necessary, but 
something similar to the patch below (but lirc + keymap protocol...if 
that's possible to implement in a non-intrusive manner, I haven't 
checked TBH) might be a good idea as an interim measure?





Signed-off-by: Tomas Melin tomas.me...@iki.fi
---
 drivers/media/rc/rc-ir-raw.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/rc-ir-raw.c 
b/drivers/media/rc/rc-ir-raw.c

index a118539..63d23d0 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -256,7 +256,8 @@ int ir_raw_event_register(struct rc_dev *dev)
return -ENOMEM;

dev-raw-dev = dev;
-   dev-enabled_protocols = ~0;
+   /* by default, disable all but lirc*/
+   dev-enabled_protocols = RC_BIT_LIRC;
rc = kfifo_alloc(dev-raw-kfifo,
 sizeof(struct ir_raw_event) * MAX_IR_EVENT_SIZE,
 GFP_KERNEL);

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH resend] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On Thu, Oct 09, 2014 at 09:30:36PM +0300, Tomas Melin wrote:
>IR reciever using nuvoton-cir and lirc was not working anymore after
>upgrade from kernel 3.16 to 3.17-rcX.
>Bisected regression to commit da6e162d6a4607362f8478c715c797d84d449f8b
>("[media] rc-core: simplify sysfs code").
>
>The regression comes from adding function change_protocol in
>ir-raw.c. During registration, ir_raw_event_register enables all protocols
>by default. Later, rc_register_device also tests dev->change_protocol and
>changes the enabled protocols based on rc_map type. However, rc_map type
>only defines a single specific protocol, so in the case of a more generic
>driver, this disables all protocols but the one defined by the map.
>
>Changed back to original behaviour by removing empty function
>change_protocol in ir-raw.c. Instead only calling change_protocol for
>drivers that actually have the function set up.

I think this is already addressed in this thread:
http://www.spinics.net/lists/linux-media/msg79865.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH resend] [media] rc-core: fix protocol_change regression in ir_raw_event_register

[David Härdeman]

On Thu, Oct 09, 2014 at 09:30:36PM +0300, Tomas Melin wrote:
IR reciever using nuvoton-cir and lirc was not working anymore after
upgrade from kernel 3.16 to 3.17-rcX.
Bisected regression to commit da6e162d6a4607362f8478c715c797d84d449f8b
([media] rc-core: simplify sysfs code).

The regression comes from adding function change_protocol in
ir-raw.c. During registration, ir_raw_event_register enables all protocols
by default. Later, rc_register_device also tests dev-change_protocol and
changes the enabled protocols based on rc_map type. However, rc_map type
only defines a single specific protocol, so in the case of a more generic
driver, this disables all protocols but the one defined by the map.

Changed back to original behaviour by removing empty function
change_protocol in ir-raw.c. Instead only calling change_protocol for
drivers that actually have the function set up.

I think this is already addressed in this thread:
http://www.spinics.net/lists/linux-media/msg79865.html

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Logitech Unifying Receiver - power button not being reported?

[David Härdeman]

Hi,

the Logitech Unifying Receiver (i.e. the USB dongle that's used for e.g.
the k400r wireless keyboard) does not seem to report any keyboard events
for the power button (I can resume from suspend-to-ram since the dongle
seems to do the right thing, but I can't do the opposite since no event
seems to be generated).

Is this a known limitation? Something that can be fixed? Is there
something I've overlooked?

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Logitech Unifying Receiver - power button not being reported?

[David Härdeman]

Hi,

the Logitech Unifying Receiver (i.e. the USB dongle that's used for e.g.
the k400r wireless keyboard) does not seem to report any keyboard events
for the power button (I can resume from suspend-to-ram since the dongle
seems to do the right thing, but I can't do the opposite since no event
seems to be generated).

Is this a known limitation? Something that can be fixed? Is there
something I've overlooked?

-- 
David Härdeman
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v7 2/3] [media] rc: add sunxi-ir driver

[David Härdeman]

On Thu, May 15, 2014 at 03:56:41AM +0600, Alexander Bersenev wrote:
>This patch adds driver for sunxi IR controller.
>It is based on Alexsey Shestacov's work based on the original driver
>supplied by Allwinner.
>

...

>+static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
>+{
>+  unsigned long status;
>+  unsigned char dt;
>+  unsigned int cnt, rc;
>+  struct sunxi_ir *ir = dev_id;
>+  DEFINE_IR_RAW_EVENT(rawir);
>+
>+  spin_lock(>ir_lock);
>+
>+  status = readl(ir->base + SUNXI_IR_RXSTA_REG);
>+
>+  /* clean all pending statuses */
>+  writel(status | REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
>+
>+  if (status & REG_RXINT_RAI_EN) {
>+  /* How many messages in fifo */
>+  rc  = (status >> REG_RXSTA_RAC__SHIFT) & REG_RXSTA_RAC__MASK;
>+  /* Sanity check */
>+  rc = rc > SUNXI_IR_FIFO_SIZE ? SUNXI_IR_FIFO_SIZE : rc;
>+  /* If we have data */
>+  for (cnt = 0; cnt < rc; cnt++) {
>+  /* for each bit in fifo */
>+  dt = readb(ir->base + SUNXI_IR_RXFIFO_REG);
>+  rawir.pulse = (dt & 0x80) != 0;
>+  rawir.duration = (dt & 0x7f) * SUNXI_IR_SAMPLE;

Can the hardware actually return a zero duration or should that be dt &
0x7f + 1?

(Not familiar with this particular hardware but I know I've seen that
behaviour before).

>+  ir_raw_event_store_with_filter(ir->rc, );
>+  }
>+  }
>+
>+  if (status & REG_RXINT_ROI_EN) {
>+  ir_raw_event_reset(ir->rc);
>+  } else if (status & REG_RXINT_RPEI_EN) {
>+  ir_raw_event_set_idle(ir->rc, true);
>+  ir_raw_event_handle(ir->rc);
>+  }
>+
>+  spin_unlock(>ir_lock);
>+
>+  return IRQ_HANDLED;
>+}


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v7 2/3] [media] rc: add sunxi-ir driver

[David Härdeman]

On Thu, May 15, 2014 at 03:56:41AM +0600, Alexander Bersenev wrote:
This patch adds driver for sunxi IR controller.
It is based on Alexsey Shestacov's work based on the original driver
supplied by Allwinner.


...

+static irqreturn_t sunxi_ir_irq(int irqno, void *dev_id)
+{
+  unsigned long status;
+  unsigned char dt;
+  unsigned int cnt, rc;
+  struct sunxi_ir *ir = dev_id;
+  DEFINE_IR_RAW_EVENT(rawir);
+
+  spin_lock(ir-ir_lock);
+
+  status = readl(ir-base + SUNXI_IR_RXSTA_REG);
+
+  /* clean all pending statuses */
+  writel(status | REG_RXSTA_CLEARALL, ir-base + SUNXI_IR_RXSTA_REG);
+
+  if (status  REG_RXINT_RAI_EN) {
+  /* How many messages in fifo */
+  rc  = (status  REG_RXSTA_RAC__SHIFT)  REG_RXSTA_RAC__MASK;
+  /* Sanity check */
+  rc = rc  SUNXI_IR_FIFO_SIZE ? SUNXI_IR_FIFO_SIZE : rc;
+  /* If we have data */
+  for (cnt = 0; cnt  rc; cnt++) {
+  /* for each bit in fifo */
+  dt = readb(ir-base + SUNXI_IR_RXFIFO_REG);
+  rawir.pulse = (dt  0x80) != 0;
+  rawir.duration = (dt  0x7f) * SUNXI_IR_SAMPLE;

Can the hardware actually return a zero duration or should that be dt 
0x7f + 1?

(Not familiar with this particular hardware but I know I've seen that
behaviour before).

+  ir_raw_event_store_with_filter(ir-rc, rawir);
+  }
+  }
+
+  if (status  REG_RXINT_ROI_EN) {
+  ir_raw_event_reset(ir-rc);
+  } else if (status  REG_RXINT_RPEI_EN) {
+  ir_raw_event_set_idle(ir-rc, true);
+  ir_raw_event_handle(ir-rc);
+  }
+
+  spin_unlock(ir-ir_lock);
+
+  return IRQ_HANDLED;
+}


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [GIT PULL for v3.15-rc1] media updates

[David Härdeman]

On Thu, Apr 03, 2014 at 01:11:43PM -0300, Mauro Carvalho Chehab wrote:
>Hi Linus,
>
>Please pull from:
>  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
> v4l_for_linus
>
...
>James Hogan (27):
...
>  [media] media: rc: add sysfs scancode filtering interface
>  [media] media: rc: change 32bit NEC scancode format
...
>  [media] rc-main: add generic scancode filtering

Umm...we (mostly James and I, but you as well) have been discussing on
the linux-media whether those patches shouldn't be reverted...this pull
request seems to have overlooked that discussion...or have I missed
something?

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [GIT PULL for v3.15-rc1] media updates

[David Härdeman]

On Thu, Apr 03, 2014 at 01:11:43PM -0300, Mauro Carvalho Chehab wrote:
Hi Linus,

Please pull from:
  git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media 
 v4l_for_linus

...
James Hogan (27):
...
  [media] media: rc: add sysfs scancode filtering interface
  [media] media: rc: change 32bit NEC scancode format
...
  [media] rc-main: add generic scancode filtering

Umm...we (mostly James and I, but you as well) have been discussing on
the linux-media whether those patches shouldn't be reverted...this pull
request seems to have overlooked that discussion...or have I missed
something?

-- 
David Härdeman
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [media] winbond-cir: remove deprecated IRQF_DISABLED

[David Härdeman]

On 2013-10-13 08:11, Michael Opdenacker wrote:

This patch proposes to remove the use of the IRQF_DISABLED flag

It's a NOOP since 2.6.35 and it will be removed one day.

Signed-off-by: Michael Opdenacker 


Acked-by: David Härdeman 

---
 drivers/media/rc/winbond-cir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/winbond-cir.c 
b/drivers/media/rc/winbond-cir.c

index 98bd496..904baf4 100644
--- a/drivers/media/rc/winbond-cir.c
+++ b/drivers/media/rc/winbond-cir.c
@@ -1110,7 +1110,7 @@ wbcir_probe(struct pnp_dev *device, const struct
pnp_device_id *dev_id)
}

err = request_irq(data->irq, wbcir_irq_handler,
- IRQF_DISABLED, DRVNAME, device);
+ 0, DRVNAME, device);
if (err) {
dev_err(dev, "Failed to claim IRQ %u\n", data->irq);
err = -EBUSY;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] [media] winbond-cir: remove deprecated IRQF_DISABLED

[David Härdeman]

On 2013-10-13 08:11, Michael Opdenacker wrote:

This patch proposes to remove the use of the IRQF_DISABLED flag

It's a NOOP since 2.6.35 and it will be removed one day.

Signed-off-by: Michael Opdenacker 
michael.opdenac...@free-electrons.com

Acked-by: David Härdeman da...@hardeman.nu

---
 drivers/media/rc/winbond-cir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/winbond-cir.c 
b/drivers/media/rc/winbond-cir.c

index 98bd496..904baf4 100644
--- a/drivers/media/rc/winbond-cir.c
+++ b/drivers/media/rc/winbond-cir.c
@@ -1110,7 +1110,7 @@ wbcir_probe(struct pnp_dev *device, const struct
pnp_device_id *dev_id)
}

err = request_irq(data-irq, wbcir_irq_handler,
- IRQF_DISABLED, DRVNAME, device);
+ 0, DRVNAME, device);
if (err) {
dev_err(dev, Failed to claim IRQ %u\n, data-irq);
err = -EBUSY;

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][v2] xhci: correctly enable interrupts

[David Härdeman]

On Mon, Mar 04, 2013 at 09:22:04AM +0100, Hannes Reinecke wrote:
>xhci has its own interrupt enabling routine, which will try to
>use MSI-X/MSI if present. So the usb core shouldn't try to enable
>legacy interrupts; on some machines the xhci legacy IRQ setting
>is invalid.
>
>Cc: Bjorn Helgaas 
>Cc: Oliver Neukum 
>Cc: Thomas Renninger 
>Cc: Yinghai Lu 
>Cc: Frederik Himpe 
>Cc: David Haerdeman 
>Cc: Alan Stern 
>Signed-off-by: Hannes Reinecke 

No idea if it's the "right" solution but it works for me.

Tested-by: David Härdeman 

>
>diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
>index 622b4a4..2647e75 100644
>--- a/drivers/usb/core/hcd-pci.c
>+++ b/drivers/usb/core/hcd-pci.c
>@@ -173,6 +173,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
>pci_device_id *id)
>   struct hc_driver*driver;
>   struct usb_hcd  *hcd;
>   int retval;
>+  int hcd_irq = 0;
> 
>   if (usb_disabled())
>   return -ENODEV;
>@@ -187,15 +188,21 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
>pci_device_id *id)
>   return -ENODEV;
>   dev->current_state = PCI_D0;
> 
>-  /* The xHCI driver supports MSI and MSI-X,
>-   * so don't fail if the BIOS doesn't provide a legacy IRQ.
>+  /*
>+   * The xHCI driver supports MSI and MSI-X,
>+   * so don't fail if the BIOS doesn't provide a legacy IRQ
>+   * and do not try to enable legacy IRQs.
>*/
>-  if (!dev->irq && (driver->flags & HCD_MASK) != HCD_USB3) {
>-  dev_err(>dev,
>-  "Found HC with no IRQ.  Check BIOS/PCI %s setup!\n",
>-  pci_name(dev));
>-  retval = -ENODEV;
>-  goto disable_pci;
>+  if ((driver->flags & HCD_MASK) != HCD_USB3) {
>+  if (!dev->irq) {
>+  dev_err(>dev,
>+  "Found HC with no IRQ.  "
>+  "Check BIOS/PCI %s setup!\n",
>+  pci_name(dev));
>+  retval = -ENODEV;
>+  goto disable_pci;
>+  }
>+  hcd_irq = dev->irq;
>   }
> 
>   hcd = usb_create_hcd(driver, >dev, pci_name(dev));
>@@ -245,7 +252,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
>pci_device_id *id)
> 
>   pci_set_master(dev);
> 
>-  retval = usb_add_hcd(hcd, dev->irq, IRQF_SHARED);
>+  retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED);
>   if (retval != 0)
>   goto unmap_registers;
>   set_hs_companion(dev, hcd);
>

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][v2] xhci: correctly enable interrupts

[David Härdeman]

On Mon, Mar 04, 2013 at 09:22:04AM +0100, Hannes Reinecke wrote:
xhci has its own interrupt enabling routine, which will try to
use MSI-X/MSI if present. So the usb core shouldn't try to enable
legacy interrupts; on some machines the xhci legacy IRQ setting
is invalid.

Cc: Bjorn Helgaas bhelg...@google.com
Cc: Oliver Neukum oneu...@suse.de
Cc: Thomas Renninger tr...@suse.de
Cc: Yinghai Lu ying...@kernel.org
Cc: Frederik Himpe fhi...@vub.ac.be
Cc: David Haerdeman da...@hardeman.nu
Cc: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Hannes Reinecke h...@suse.de

No idea if it's the right solution but it works for me.

Tested-by: David Härdeman da...@hardeman.nu


diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index 622b4a4..2647e75 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -173,6 +173,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
   struct hc_driver*driver;
   struct usb_hcd  *hcd;
   int retval;
+  int hcd_irq = 0;
 
   if (usb_disabled())
   return -ENODEV;
@@ -187,15 +188,21 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
   return -ENODEV;
   dev-current_state = PCI_D0;
 
-  /* The xHCI driver supports MSI and MSI-X,
-   * so don't fail if the BIOS doesn't provide a legacy IRQ.
+  /*
+   * The xHCI driver supports MSI and MSI-X,
+   * so don't fail if the BIOS doesn't provide a legacy IRQ
+   * and do not try to enable legacy IRQs.
*/
-  if (!dev-irq  (driver-flags  HCD_MASK) != HCD_USB3) {
-  dev_err(dev-dev,
-  Found HC with no IRQ.  Check BIOS/PCI %s setup!\n,
-  pci_name(dev));
-  retval = -ENODEV;
-  goto disable_pci;
+  if ((driver-flags  HCD_MASK) != HCD_USB3) {
+  if (!dev-irq) {
+  dev_err(dev-dev,
+  Found HC with no IRQ.  
+  Check BIOS/PCI %s setup!\n,
+  pci_name(dev));
+  retval = -ENODEV;
+  goto disable_pci;
+  }
+  hcd_irq = dev-irq;
   }
 
   hcd = usb_create_hcd(driver, dev-dev, pci_name(dev));
@@ -245,7 +252,7 @@ int usb_hcd_pci_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
 
   pci_set_master(dev);
 
-  retval = usb_add_hcd(hcd, dev-irq, IRQF_SHARED);
+  retval = usb_add_hcd(hcd, hcd_irq, IRQF_SHARED);
   if (retval != 0)
   goto unmap_registers;
   set_hs_companion(dev, hcd);


-- 
David Härdeman
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] usb: correctly enable interrupts for xhci

[David Härdeman]

On Fri, Mar 01, 2013 at 09:52:54AM +0100, Hannes Reinecke wrote:
>xhci might run with MSI/MSI-X only, with no support for legacy
>interrupts. On these devices the request_irq() call in usb_add_hcd()
>will fail, causing the entire device to fail.
>For xhci this is especially painful as the driver will enable
>interrupts during xhci_run(), so the initial call to request_irq()
>is not required anyway.
>
>This patch adds a flag 'msix_supported' to struct usb_hcd, which
>will cause usb_add_hcd() to skip interrupt setup. This flag is
>set in xhci-pci, so the registration will skip the first request_irq()
>and can proceed.
>
>Cc: Bjorn Helgaas 
>Cc: Oliver Neukum 
>Cc: Thomas Renninger 
>Cc: Yinghai Lu 
>Cc: Frederik Himpe 
>Cc: David Haerdeman 
>Signed-off-by: Hannes Reinecke 

It doesn't seem to work. I just tried applying the patch to the 3.8
kernel in Debian experimental and this was the result during boot:

[1.203390] xhci_hcd :00:14.0: can't derive routing for PCI INT A
[1.203393] xhci_hcd :00:14.0: PCI INT A: no GSI
[1.203419] xhci_hcd :00:14.0: setting latency timer to 64
[1.203423] xhci_hcd :00:14.0: xHCI Host Controller
[1.203429] xhci_hcd :00:14.0: new USB bus registered, assigned bus 
number 1
[1.203533] xhci_hcd :00:14.0: cache line size of 64 is not supported
[1.203535] xhci_hcd :00:14.0: request interrupt 255 failed
[1.203580] xhci_hcd :00:14.0: USB bus 1 deregistered
[1.203598] xhci_hcd :00:14.0: can't derive routing for PCI INT A
[1.203600] xhci_hcd :00:14.0: init :00:14.0 fail, -22
[1.203643] xhci_hcd: probe of :00:14.0 failed with error -22

Regards,
David

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] usb: correctly enable interrupts for xhci

[David Härdeman]

On Fri, Mar 01, 2013 at 09:52:54AM +0100, Hannes Reinecke wrote:
xhci might run with MSI/MSI-X only, with no support for legacy
interrupts. On these devices the request_irq() call in usb_add_hcd()
will fail, causing the entire device to fail.
For xhci this is especially painful as the driver will enable
interrupts during xhci_run(), so the initial call to request_irq()
is not required anyway.

This patch adds a flag 'msix_supported' to struct usb_hcd, which
will cause usb_add_hcd() to skip interrupt setup. This flag is
set in xhci-pci, so the registration will skip the first request_irq()
and can proceed.

Cc: Bjorn Helgaas bhelg...@google.com
Cc: Oliver Neukum oneu...@suse.de
Cc: Thomas Renninger tr...@suse.de
Cc: Yinghai Lu ying...@kernel.org
Cc: Frederik Himpe fhi...@vub.ac.be
Cc: David Haerdeman da...@hardeman.nu
Signed-off-by: Hannes Reinecke h...@suse.de

It doesn't seem to work. I just tried applying the patch to the 3.8
kernel in Debian experimental and this was the result during boot:

[1.203390] xhci_hcd :00:14.0: can't derive routing for PCI INT A
[1.203393] xhci_hcd :00:14.0: PCI INT A: no GSI
[1.203419] xhci_hcd :00:14.0: setting latency timer to 64
[1.203423] xhci_hcd :00:14.0: xHCI Host Controller
[1.203429] xhci_hcd :00:14.0: new USB bus registered, assigned bus 
number 1
[1.203533] xhci_hcd :00:14.0: cache line size of 64 is not supported
[1.203535] xhci_hcd :00:14.0: request interrupt 255 failed
[1.203580] xhci_hcd :00:14.0: USB bus 1 deregistered
[1.203598] xhci_hcd :00:14.0: can't derive routing for PCI INT A
[1.203600] xhci_hcd :00:14.0: init :00:14.0 fail, -22
[1.203643] xhci_hcd: probe of :00:14.0 failed with error -22

Regards,
David

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] pci: do not try to assign irq 255

[David Härdeman]

On Thu, Feb 21, 2013 at 07:53:14AM +0100, Hannes Reinecke wrote:
>On 02/20/2013 05:57 PM, Yinghai Lu wrote:
>>it seems you mess pin with interrupt line.
>>
>>current code:
>> unsigned char irq;
>>
>> pci_read_config_byte(dev, PCI_INTERRUPT_PIN, );
>> dev->pin = irq;
>> if (irq)
>> pci_read_config_byte(dev, PCI_INTERRUPT_LINE, );
>> dev->irq = irq;
>>
>>so if the device does not have interrupt pin implemented, pin should be zero.
>>and  pin and irq in dev should
>>be all 0.
>>
>But the device _has_ an interrupt pin implemented.
>The whole point here is that the interrupt line is _NOT_ zero.
>
...
>
>So at one point we have to decide that ->irq is not valid, despite it
>being not set to zero.
>An alternative fix would be this:
>
>diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
>index 68a921d..4a480cb 100644
>--- a/drivers/acpi/pci_irq.c
>+++ b/drivers/acpi/pci_irq.c
>@@ -469,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
>} else {
>dev_warn(>dev, "PCI INT %c: no GSI\n",
> pin_name(pin));
>+   dev->irq = 0;
>}
>return 0;
>}
>
>Which probably is a better solution, as here ->irq is _definitely_
>not valid, so we should reset it to '0' to avoid confusion on upper
>layers.
>

Is there any agreement on how to proceed?

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] pci: do not try to assign irq 255

[David Härdeman]

On Thu, Feb 21, 2013 at 07:53:14AM +0100, Hannes Reinecke wrote:
On 02/20/2013 05:57 PM, Yinghai Lu wrote:
it seems you mess pin with interrupt line.

current code:
 unsigned char irq;

 pci_read_config_byte(dev, PCI_INTERRUPT_PIN, irq);
 dev-pin = irq;
 if (irq)
 pci_read_config_byte(dev, PCI_INTERRUPT_LINE, irq);
 dev-irq = irq;

so if the device does not have interrupt pin implemented, pin should be zero.
and  pin and irq in dev should
be all 0.

But the device _has_ an interrupt pin implemented.
The whole point here is that the interrupt line is _NOT_ zero.

...

So at one point we have to decide that -irq is not valid, despite it
being not set to zero.
An alternative fix would be this:

diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
index 68a921d..4a480cb 100644
--- a/drivers/acpi/pci_irq.c
+++ b/drivers/acpi/pci_irq.c
@@ -469,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *dev)
} else {
dev_warn(dev-dev, PCI INT %c: no GSI\n,
 pin_name(pin));
+   dev-irq = 0;
}
return 0;
}

Which probably is a better solution, as here -irq is _definitely_
not valid, so we should reset it to '0' to avoid confusion on upper
layers.


Is there any agreement on how to proceed?

-- 
David Härdeman
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] pci: do not try to assign irq 255

[David Härdeman]

On Mon, Feb 18, 2013 at 11:09:53AM +0100, Hannes Reinecke wrote:
>The PCI config space reseves a byte for the interrupt line,
>so irq 255 actually refers to 'not set'.
>However, the 'irq' field for struct pci_dev is an integer,
>so the original meaning is lost, causing the system to
>assign an interrupt '255', which fails.
>
>So we should _not_ assign an interrupt value here, and
>allow upper layers to fixup things.
>
>This patch make PCI devices with MSI interrupts only
>(like the xhci device on certain HP laptops) work properly.

Just tested and it works for me. Thank you.

Tested-by: David Härdeman 

>Cc: Frederik Himpe 
>Cc: Oliver Neukum 
>Cc: David Haerdeman 
>Cc: linux-...@vger.kernel.org
>Cc: linux-...@vger.kernel.org
>Signed-off-by: Hannes Reinecke 
>
>diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>index 6186f03..a2db887f 100644
>--- a/drivers/pci/probe.c
>+++ b/drivers/pci/probe.c
>@@ -923,7 +923,8 @@ static void pci_read_irq(struct pci_dev *dev)
>   dev->pin = irq;
>   if (irq)
>   pci_read_config_byte(dev, PCI_INTERRUPT_LINE, );
>-  dev->irq = irq;
>+  if (irq < 255)
>+  dev->irq = irq;
> }
> 
> void set_pcie_port_type(struct pci_dev *pdev)
>

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] pci: do not try to assign irq 255

[David Härdeman]

On Mon, Feb 18, 2013 at 11:09:53AM +0100, Hannes Reinecke wrote:
The PCI config space reseves a byte for the interrupt line,
so irq 255 actually refers to 'not set'.
However, the 'irq' field for struct pci_dev is an integer,
so the original meaning is lost, causing the system to
assign an interrupt '255', which fails.

So we should _not_ assign an interrupt value here, and
allow upper layers to fixup things.

This patch make PCI devices with MSI interrupts only
(like the xhci device on certain HP laptops) work properly.

Just tested and it works for me. Thank you.

Tested-by: David Härdeman da...@hardeman.nu

Cc: Frederik Himpe fhi...@vub.ac.be
Cc: Oliver Neukum oneu...@suse.de
Cc: David Haerdeman da...@hardeman.nu
Cc: linux-...@vger.kernel.org
Cc: linux-...@vger.kernel.org
Signed-off-by: Hannes Reinecke h...@suse.de

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6186f03..a2db887f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -923,7 +923,8 @@ static void pci_read_irq(struct pci_dev *dev)
   dev-pin = irq;
   if (irq)
   pci_read_config_byte(dev, PCI_INTERRUPT_LINE, irq);
-  dev-irq = irq;
+  if (irq  255)
+  dev-irq = irq;
 }
 
 void set_pcie_port_type(struct pci_dev *pdev)


-- 
David Härdeman
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: xhci module fails when booting in UEFI mode

[David Härdeman]

On Thu, Jan 10, 2013 at 11:15:56AM +, Frederik Himpe wrote:
>I've got a HP EliteBook 8470p on which I installed Debian Wheezy in UEFI 
>mode. With both the 3.2 kernel from Wheezy, as the 3.7.1 kernel from 
>experimental, xhci fails to initialize and my USB mouse connected to one 
>of these ports is not recognized at all. The USB3 ports work fine in 
>Windows.
>
>[1.316248] xhci_hcd :00:14.0: can't derive routing for PCI INT A
>[1.316251] xhci_hcd :00:14.0: PCI INT A: no GSI
>[1.316253] 
>[1.316277] xhci_hcd :00:14.0: setting latency timer to 64
>[1.316281] xhci_hcd :00:14.0: xHCI Host Controller
>[1.316287] xhci_hcd :00:14.0: new USB bus registered, assigned 
>bus number 1
>[1.316393] xhci_hcd :00:14.0: cache line size of 64 is not 
>supported
>[1.316395] xhci_hcd :00:14.0: request interrupt 255 failed
>[1.316447] xhci_hcd :00:14.0: USB bus 1 deregistered
>[1.316466] xhci_hcd :00:14.0: can't derive routing for PCI INT A
>[1.316467] xhci_hcd :00:14.0: init :00:14.0 fail, -22
>[1.316505] xhci_hcd: probe of :00:14.0 failed with error -22
>
>Full dmesg, lspci, lsusb and lsmod can be found here:
>http://artipc10.vub.ac.be/~frederik/uefi-xhci/
>
>I found this report about the same problem on a HP Probook system: 
>https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1072918 

I have the same problem (HP Elitebook 8570p in my case).

This thread seems related:
https://lkml.org/lkml/2012/2/13/453

No idea what happened to the patches in that thread though...


-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: xhci module fails when booting in UEFI mode

[David Härdeman]

On Thu, Jan 10, 2013 at 11:15:56AM +, Frederik Himpe wrote:
I've got a HP EliteBook 8470p on which I installed Debian Wheezy in UEFI 
mode. With both the 3.2 kernel from Wheezy, as the 3.7.1 kernel from 
experimental, xhci fails to initialize and my USB mouse connected to one 
of these ports is not recognized at all. The USB3 ports work fine in 
Windows.

[1.316248] xhci_hcd :00:14.0: can't derive routing for PCI INT A
[1.316251] xhci_hcd :00:14.0: PCI INT A: no GSI
[1.316253] 
[1.316277] xhci_hcd :00:14.0: setting latency timer to 64
[1.316281] xhci_hcd :00:14.0: xHCI Host Controller
[1.316287] xhci_hcd :00:14.0: new USB bus registered, assigned 
bus number 1
[1.316393] xhci_hcd :00:14.0: cache line size of 64 is not 
supported
[1.316395] xhci_hcd :00:14.0: request interrupt 255 failed
[1.316447] xhci_hcd :00:14.0: USB bus 1 deregistered
[1.316466] xhci_hcd :00:14.0: can't derive routing for PCI INT A
[1.316467] xhci_hcd :00:14.0: init :00:14.0 fail, -22
[1.316505] xhci_hcd: probe of :00:14.0 failed with error -22

Full dmesg, lspci, lsusb and lsmod can be found here:
http://artipc10.vub.ac.be/~frederik/uefi-xhci/

I found this report about the same problem on a HP Probook system: 
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1072918 

I have the same problem (HP Elitebook 8570p in my case).

This thread seems related:
https://lkml.org/lkml/2012/2/13/453

No idea what happened to the patches in that thread though...


-- 
David Härdeman
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] rc-core: don't return from store_protocols without releasing device mutex

[David Härdeman]

On Sat, Dec 08, 2012 at 07:50:50AM -0500, Sasha Levin wrote:
>Commit c003ab1b ("[media] rc-core: add separate defines for protocol bitmaps
>and numbers") has introduced a bug which allows store_protocols() to return
>without releasing the device mutex it's holding.
>
>Doing that would cause infinite hangs waiting on device mutex next time
>around.
>
>Signed-off-by: Sasha Levin 

Acked-by: David Härdeman 

>---
> drivers/media/rc/rc-main.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
>index 601d1ac1..0510f4d 100644
>--- a/drivers/media/rc/rc-main.c
>+++ b/drivers/media/rc/rc-main.c
>@@ -890,7 +890,8 @@ static ssize_t store_protocols(struct device *device,
> 
>   if (i == ARRAY_SIZE(proto_names)) {
>   IR_dprintk(1, "Unknown protocol: '%s'\n", tmp);
>-  return -EINVAL;
>+  ret = -EINVAL;
>+  goto out;
>   }
> 
>   count++;
>-- 
>1.8.0
>

-- 
David Härdeman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] rc-core: don't return from store_protocols without releasing device mutex

[David Härdeman]

On Sat, Dec 08, 2012 at 07:50:50AM -0500, Sasha Levin wrote:
Commit c003ab1b ([media] rc-core: add separate defines for protocol bitmaps
and numbers) has introduced a bug which allows store_protocols() to return
without releasing the device mutex it's holding.

Doing that would cause infinite hangs waiting on device mutex next time
around.

Signed-off-by: Sasha Levin sasha.le...@oracle.com

Acked-by: David Härdeman da...@hardeman.nu

---
 drivers/media/rc/rc-main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 601d1ac1..0510f4d 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -890,7 +890,8 @@ static ssize_t store_protocols(struct device *device,
 
   if (i == ARRAY_SIZE(proto_names)) {
   IR_dprintk(1, Unknown protocol: '%s'\n, tmp);
-  return -EINVAL;
+  ret = -EINVAL;
+  goto out;
   }
 
   count++;
-- 
1.8.0


-- 
David Härdeman
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] hid-picolcd_cir: fix compilation

[David Härdeman]

Commit c003ab1bedf0 ("[media] rc-core: add separate defines for
protocol bitmaps and numbers") overlooked hid-picolcd.

This patch (against git.linuxtv.org/media_tree.git, branch
staging/for_v3.8) fixes the compilation breakage.

Signed-off-by: David Härdeman 
---
 drivers/hid/hid-picolcd_cir.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-picolcd_cir.c b/drivers/hid/hid-picolcd_cir.c
index 13ca919..a79e95b 100644
--- a/drivers/hid/hid-picolcd_cir.c
+++ b/drivers/hid/hid-picolcd_cir.c
@@ -116,7 +116,7 @@ int picolcd_init_cir(struct picolcd_data *data, struct 
hid_report *report)
 
rdev->priv = data;
rdev->driver_type  = RC_DRIVER_IR_RAW;
-   rdev->allowed_protos   = RC_TYPE_ALL;
+   rdev->allowed_protos   = RC_BIT_ALL;
rdev->open = picolcd_cir_open;
rdev->close= picolcd_cir_close;
rdev->input_name   = data->hdev->name;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] hid-picolcd_cir: fix compilation

[David Härdeman]

Commit c003ab1bedf0 ([media] rc-core: add separate defines for
protocol bitmaps and numbers) overlooked hid-picolcd.

This patch (against git.linuxtv.org/media_tree.git, branch
staging/for_v3.8) fixes the compilation breakage.

Signed-off-by: David Härdeman da...@hardeman.nu
---
 drivers/hid/hid-picolcd_cir.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-picolcd_cir.c b/drivers/hid/hid-picolcd_cir.c
index 13ca919..a79e95b 100644
--- a/drivers/hid/hid-picolcd_cir.c
+++ b/drivers/hid/hid-picolcd_cir.c
@@ -116,7 +116,7 @@ int picolcd_init_cir(struct picolcd_data *data, struct 
hid_report *report)
 
rdev-priv = data;
rdev-driver_type  = RC_DRIVER_IR_RAW;
-   rdev-allowed_protos   = RC_TYPE_ALL;
+   rdev-allowed_protos   = RC_BIT_ALL;
rdev-open = picolcd_cir_open;
rdev-close= picolcd_cir_close;
rdev-input_name   = data-hdev-name;

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: build failure after merge of the v4l-dvb tree

[David Härdeman]

On Mon, 29 Oct 2012 11:14:03 +1100, Stephen Rothwell

wrote:
> Hi Mauro,
> 
> After merging the v4l-dvb tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
> 
> drivers/hid/hid-picolcd_cir.c: In function 'picolcd_init_cir':
> drivers/hid/hid-picolcd_cir.c:119:27: error: 'RC_TYPE_ALL' undeclared
> (first use in this function)

I can fix it, should be easy (rename RC_TYPE_ALL to RC_BIT_ALL).

> Caused by commit c003ab1bedf0 ("[media] rc-core: add separate defines
for
> protocol bitmaps and numbers").  The new reference was introduced in
> commit ae08e324146c ("HID: picoLCD: Add support for CIR") which was
> merged by Linus on October 2 into v3.7-rc1 via the hid tree.
> 
> I have used the v4l-dvb tree from next-20121026 for today.

The question is in which form you want the patch - as a separate patch or
a new version of commit c003ab1bedf0 (i.e. the entire patch)? Also, against
which tree should I create the patch?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: build failure after merge of the v4l-dvb tree

[David Härdeman]

On Mon, 29 Oct 2012 11:14:03 +1100, Stephen Rothwell
s...@canb.auug.org.au
wrote:
 Hi Mauro,
 
 After merging the v4l-dvb tree, today's linux-next build (x86_64
 allmodconfig) failed like this:
 
 drivers/hid/hid-picolcd_cir.c: In function 'picolcd_init_cir':
 drivers/hid/hid-picolcd_cir.c:119:27: error: 'RC_TYPE_ALL' undeclared
 (first use in this function)

I can fix it, should be easy (rename RC_TYPE_ALL to RC_BIT_ALL).

 Caused by commit c003ab1bedf0 ([media] rc-core: add separate defines
for
 protocol bitmaps and numbers).  The new reference was introduced in
 commit ae08e324146c (HID: picoLCD: Add support for CIR) which was
 merged by Linus on October 2 into v3.7-rc1 via the hid tree.
 
 I have used the v4l-dvb tree from next-20121026 for today.

The question is in which form you want the patch - as a separate patch or
a new version of commit c003ab1bedf0 (i.e. the entire patch)? Also, against
which tree should I create the patch?


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ata: ahci: Enable enclosure management via LED (resend)

[David Härdeman]

On Thu, October 25, 2007 07:58, Jeff Garzik wrote:
> Kristen Carlson Accardi wrote:
>> Enable enclosure management via LED
>>
>> As described in the AHCI spec, some AHCI controllers may support
>> Enclosure management via a variety of protocols.  This patch
>> adds support for the LED message type that is specified in
>> AHCI 1.1 and higher.
>
> Comments:
...
> 8) when is this actually used?  do you have a sample user in userspace?
>   does a userspace daemon watch disk activity and manage LEDs somehow?
> I'm a bit cloudy on the usage need of this.

I think the idea is that if mdadm (for example) kicks out a faulty disk
from an array, it could also activate a LED (usually a different LED than
the "disk activity" LED) on the corresponding enclosure so that the admin
knows when standing in front of the storage server which disk to pull out
and replace.

-- 
David Härdeman

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] ata: ahci: Enable enclosure management via LED (resend)

[David Härdeman]

On Thu, October 25, 2007 07:58, Jeff Garzik wrote:
 Kristen Carlson Accardi wrote:
 Enable enclosure management via LED

 As described in the AHCI spec, some AHCI controllers may support
 Enclosure management via a variety of protocols.  This patch
 adds support for the LED message type that is specified in
 AHCI 1.1 and higher.

 Comments:
...
 8) when is this actually used?  do you have a sample user in userspace?
   does a userspace daemon watch disk activity and manage LEDs somehow?
 I'm a bit cloudy on the usage need of this.

I think the idea is that if mdadm (for example) kicks out a faulty disk
from an array, it could also activate a LED (usually a different LED than
the disk activity LED) on the corresponding enclosure so that the admin
knows when standing in front of the storage server which disk to pull out
and replace.

-- 
David Härdeman

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Changing the number of keys supported by the input subsystem

[David Härdeman]

Currently the IR input handling of drivers/media/dvb/ttpci/budget-ci.c 
requires a rc5_device module option which specifies the rc5 device code 
to listen to (or as an alternative, all devices). This means, among 
other things, that multiple remotes can't be used with different 
keymaps.


My idea was to drop the module parameter and allow all rc5 device and 
command combinations to be mapped using one keyboard map.


rc5 device codes are 5 bits, command codes are 6 bits, giving a total of 
2048 possible keycodes.


However, the input subsystem is currently limited to 512 entries. So my 
question is whether it would be possible to extend that and/or make the 
size of the keymap (the keybit member of struct input_dev) dynamic?


--
David Härdeman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Changing the number of keys supported by the input subsystem

[David Härdeman]

Currently the IR input handling of drivers/media/dvb/ttpci/budget-ci.c 
requires a rc5_device module option which specifies the rc5 device code 
to listen to (or as an alternative, all devices). This means, among 
other things, that multiple remotes can't be used with different 
keymaps.


My idea was to drop the module parameter and allow all rc5 device and 
command combinations to be mapped using one keyboard map.


rc5 device codes are 5 bits, command codes are 6 bits, giving a total of 
2048 possible keycodes.


However, the input subsystem is currently limited to 512 entries. So my 
question is whether it would be possible to extend that and/or make the 
size of the keymap (the keybit member of struct input_dev) dynamic?


--
David Härdeman
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: RFC: A revised timerfd API

[David Härdeman]

On Sat, Sep 22, 2007 at 06:07:14PM +0200, Michael Kerrisk wrote:

On 9/22/07, Bernd Eckenfels <[EMAIL PROTECTED]> wrote:

In article <[EMAIL PROTECTED]> you wrote:
>  1. This design stretches the POSIX timers API in strange
> ways.

Maybe it is possible to reimplement the POSIX API in usermode using the
kernel's FD implementation?


It's a clever idea...  Without thinking on it too long, I'm not sure
whether or not there might be some details which would make this
difficult.


It seems to be a dangerous idea. It has the potential of breaking 
userspace applications that rely on POSIX timers not creating fd's.


Image code like this:

/* Close stdin, stdout, stderr */
close(0);
close(1);
close(2);

/* Oh, a timer would be nice */
timer_create(x, y, z);

/* Create new stdin, stdout, stderr */
fd = open("/dev/null", flags);
dup(fd);
dup(fd);

Unless timer_create does some magic to avoid using the lowest available 
fd, this would suddenly break as the timerfd would be fd 0.


--
David Härdeman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: RFC: A revised timerfd API

[David Härdeman]

On Sat, Sep 22, 2007 at 06:07:14PM +0200, Michael Kerrisk wrote:

On 9/22/07, Bernd Eckenfels [EMAIL PROTECTED] wrote:

In article [EMAIL PROTECTED] you wrote:
  1. This design stretches the POSIX timers API in strange
 ways.

Maybe it is possible to reimplement the POSIX API in usermode using the
kernel's FD implementation?


It's a clever idea...  Without thinking on it too long, I'm not sure
whether or not there might be some details which would make this
difficult.


It seems to be a dangerous idea. It has the potential of breaking 
userspace applications that rely on POSIX timers not creating fd's.


Image code like this:

/* Close stdin, stdout, stderr */
close(0);
close(1);
close(2);

/* Oh, a timer would be nice */
timer_create(x, y, z);

/* Create new stdin, stdout, stderr */
fd = open(/dev/null, flags);
dup(fd);
dup(fd);

Unless timer_create does some magic to avoid using the lowest available 
fd, this would suddenly break as the timerfd would be fd 0.


--
David Härdeman
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: RFC: A revised timerfd API

[David Härdeman]

On Tue, September 18, 2007 13:30, Thomas Gleixner wrote:
>>> timer_gettime(fd | POSIX_TIMER_FD, .);
>
> If we use the most significant bit for POSIX_TIMER_FD, we should be
> fine.

I think alternative b) - three new syscalls, sounds better.

The only negatives so far are that it adds more syscalls and that it might
require code duplication with posix timers. The syscall numbers argument
seemed not to be very important and the code duplication should be fixable
by refactoring the code so that more is shared between the two systems (I
assume).

Overloading file descriptors with flags looks ugly, is there any other
syscall which does that?

-- 
David Härdeman
(sorry Thomas for the dupe, I missed replying to all on the first msg).


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: RFC: A revised timerfd API

[David Härdeman]

On Tue, September 18, 2007 09:30, Michael Kerrisk wrote:
> > b) Create a timerfd interface analogous to POSIX timers
>
> Create an interface analogous to POSIX timers:
> fd = timerfd_create(clockid, flags);
> timerfd_settime(fd, flags, newtimervalue, _to_next_expire);
> timerfd_gettime(fd, _to_next_expire);
...
> > c) Integrate timerfd with POSIX timers
>
> Make a very simple timerfd call that is integrated with the
> POSIX timers API.  The POSIX timers API is detailed here:
> http://linux.die.net/man/3/timer_create
> http://linux.die.net/man/3/timer_settime
>
> Under the POSIX timers API, a new timer is created using:
>
> int timer_create(clockid_t clockid, struct sigevent *evp,
> timer_t *timerid);
>
> We could then have a timerfd() call that returns a file descriptor
> for the newly created 'timerid':
>
> fd = timerfd(timer_t timerid);

Wouldn't this remove some of the usefulness of the timerfd?

For example, if a timerfd is one of the fd's that is returned by a
epoll_wait syscall, you manually need to do the mapping between the
timerfd and the timerid in order to be able to modify the timer.

The advantage of solution b) above is that the fd is everything that is
needed to work with the timer. With solution c) you have to keep two
references to the same timer around and use one of them depending on what
you want to do with the timer.

Also, if the timerfd is close():d, does that remove the underlying timer
(invalidate the timerid) as well?

-- 
David Härdeman

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: RFC: A revised timerfd API

[David Härdeman]

On Tue, September 18, 2007 09:30, Michael Kerrisk wrote:
  b) Create a timerfd interface analogous to POSIX timers

 Create an interface analogous to POSIX timers:
 fd = timerfd_create(clockid, flags);
 timerfd_settime(fd, flags, newtimervalue, time_to_next_expire);
 timerfd_gettime(fd, time_to_next_expire);
...
  c) Integrate timerfd with POSIX timers

 Make a very simple timerfd call that is integrated with the
 POSIX timers API.  The POSIX timers API is detailed here:
 http://linux.die.net/man/3/timer_create
 http://linux.die.net/man/3/timer_settime

 Under the POSIX timers API, a new timer is created using:

 int timer_create(clockid_t clockid, struct sigevent *evp,
 timer_t *timerid);

 We could then have a timerfd() call that returns a file descriptor
 for the newly created 'timerid':

 fd = timerfd(timer_t timerid);

Wouldn't this remove some of the usefulness of the timerfd?

For example, if a timerfd is one of the fd's that is returned by a
epoll_wait syscall, you manually need to do the mapping between the
timerfd and the timerid in order to be able to modify the timer.

The advantage of solution b) above is that the fd is everything that is
needed to work with the timer. With solution c) you have to keep two
references to the same timer around and use one of them depending on what
you want to do with the timer.

Also, if the timerfd is close():d, does that remove the underlying timer
(invalidate the timerid) as well?

-- 
David Härdeman

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: RFC: A revised timerfd API

[David Härdeman]

On Tue, September 18, 2007 13:30, Thomas Gleixner wrote:
 timer_gettime(fd | POSIX_TIMER_FD, .);

 If we use the most significant bit for POSIX_TIMER_FD, we should be
 fine.

I think alternative b) - three new syscalls, sounds better.

The only negatives so far are that it adds more syscalls and that it might
require code duplication with posix timers. The syscall numbers argument
seemed not to be very important and the code duplication should be fixable
by refactoring the code so that more is shared between the two systems (I
assume).

Overloading file descriptors with flags looks ugly, is there any other
syscall which does that?

-- 
David Härdeman
(sorry Thomas for the dupe, I missed replying to all on the first msg).


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: splice question

[David Härdeman]

On Sun, Aug 12, 2007 at 12:41:54PM -0700, Linus Torvalds wrote:

On Sun, 12 Aug 2007, David H?rdeman wrote:

Otherwise I guess I'd have to add a second pipe, then (in a loop)
tee() from the first to the second pipe and then splice from the second pipe
to a socket. Doesn't sound very elegant and would need quite a lot of extra
syscalls.


You really should think of this as a memcpy(), and you'll be in the right 
mindframe. The system calls themselves are cheap.


Ok, I've implemented it using two pipes, and it works. But it does seem 
a bit wasteful...in case one client is not keeping up, the data will 
have to be tee():ed first from pipe1 to pipe2, only to then find out 
that the splice() from pipe2 to socket only does a partial transfer 
after which the data in pipe2 has to be thrown away and then the loop 
starts over with the next client.


A tee() from pipe1 to the socket could (I imagine) realize immediately 
that the socket does not have enough buffer space and return EWOULDBLOCK 
and avoid at least one copy?


--
David Härdeman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

splice question

[David Härdeman]

I'm currently toying with splice. The test program I have sends a MPEG2 
transport stream from a dvb card and sends it to N clients via http.


Since the dvb frontend driver doesn't support splice 
(linux/drivers/media/dvb/dvb-core/dmxdev.c I guess, anyone interested in 
adding splice support to it?) I had to do a normal read() to a 
user-space buffer and then write that to a pipe.


Once the data is in the pipe, my idea was to tee() from the pipe to each 
client socket using nonblocking ops, and then consume the data by 
splicing it to /dev/null.


The problem is that tee() doesn't support sockets. Is this a limitation 
that would be easy to fix?


Otherwise I guess I'd have to add a second pipe, then (in a loop)
tee() from the first to the second pipe and then splice from the 
second pipe to a socket. Doesn't sound very elegant and would need quite 
a lot of extra syscalls.


--
David Härdeman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

splice question

[David Härdeman]

I'm currently toying with splice. The test program I have sends a MPEG2 
transport stream from a dvb card and sends it to N clients via http.


Since the dvb frontend driver doesn't support splice 
(linux/drivers/media/dvb/dvb-core/dmxdev.c I guess, anyone interested in 
adding splice support to it?) I had to do a normal read() to a 
user-space buffer and then write that to a pipe.


Once the data is in the pipe, my idea was to tee() from the pipe to each 
client socket using nonblocking ops, and then consume the data by 
splicing it to /dev/null.


The problem is that tee() doesn't support sockets. Is this a limitation 
that would be easy to fix?


Otherwise I guess I'd have to add a second pipe, then (in a loop)
tee() from the first to the second pipe and then splice from the 
second pipe to a socket. Doesn't sound very elegant and would need quite 
a lot of extra syscalls.


--
David Härdeman
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: splice question

[David Härdeman]

On Sun, Aug 12, 2007 at 12:41:54PM -0700, Linus Torvalds wrote:

On Sun, 12 Aug 2007, David H?rdeman wrote:

Otherwise I guess I'd have to add a second pipe, then (in a loop)
tee() from the first to the second pipe and then splice from the second pipe
to a socket. Doesn't sound very elegant and would need quite a lot of extra
syscalls.


You really should think of this as a memcpy(), and you'll be in the right 
mindframe. The system calls themselves are cheap.


Ok, I've implemented it using two pipes, and it works. But it does seem 
a bit wasteful...in case one client is not keeping up, the data will 
have to be tee():ed first from pipe1 to pipe2, only to then find out 
that the splice() from pipe2 to socket only does a partial transfer 
after which the data in pipe2 has to be thrown away and then the loop 
starts over with the next client.


A tee() from pipe1 to the socket could (I imagine) realize immediately 
that the socket does not have enough buffer space and return EWOULDBLOCK 
and avoid at least one copy?


--
David Härdeman
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] general: convert "kernel" subdirectory to UTF-8

[David Härdeman]

On Wed, April 18, 2007 6:56, Dmitry Torokhov said:
> On Tuesday 17 April 2007 13:19, John Anthony Kazos Jr. wrote:
>> Â /*
>> - * Samma på svenska..
>> + * Samma på svenska..
>> Â  */
>
> Translating this comment into english so more people could understand it
> would be better option.

The comment says "(The) same in swedish"

-- 
David Härdeman

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] general: convert kernel subdirectory to UTF-8

[David Härdeman]

On Wed, April 18, 2007 6:56, Dmitry Torokhov said:
 On Tuesday 17 April 2007 13:19, John Anthony Kazos Jr. wrote:
 Â /*
 - * Samma på svenska..
 + * Samma på svenska..
 Â  */

 Translating this comment into english so more people could understand it
 would be better option.

The comment says (The) same in swedish

-- 
David Härdeman

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Consolidate dprintk and friends?

[David Härdeman]

A case-insensitive grep through the 2.6.13 tree for "define printk" 
yields 378 hits ("define dbg": 736, "define warn": 50, "define info": 
65).


Maybe it would be a good idea to create include/linux/debug.h as 
included inline below (or to add the content in include/linux/kernel.h 
where pr_debug is already located)? 


Then a module could simply do:

#include 
MODULE_DEBUG;

and then sprinkle debug() statements where appropriate.

It would also make the name and description of the debug variable in 
sysfs more consistent. 

I tried searching the lkml archives, but couldn't find any discussion on 
the topic...so is it a good idea?


Re,
David


--- /dev/null   2005-09-01 20:05:20.395616648 +0200
+++ linux-2.6.13/include/linux/debug.h  2005-09-01 20:01:56.0 +0200
@@ -0,0 +1,52 @@
+#ifndef _LINUX_DEBUG_H
+#define _LINUX_DEBUG_H
+
+/*
+ * unconditional messages 
+ */

+
+/* unc(onditional)_debug should not be used, but rather debug or pr_debug */
+#define unc_debug(fmt,arg...) printk(KERN_DEBUG fmt, ##arg)
+
+/* convenience macros */
+#define info(fmt,arg...) printk(KERN_INFO fmt, ##arg)
+#define warn(fmt,arg...) printk(KERN_WARN fmt, ##arg)
+#define err(fmt,arg...) printk(KERN_EMERG fmt, ##arg)
+
+
+
+/*
+ * debug messages (de)activated at runtime 
+ */

+
+/* the variable which (de)activates debugging messages */
+#ifndef DEBUG_VARIABLE
+#define DEBUG_VARIABLE debug
+#endif
+
+/* quick macro to add a debug parameter to a module */
+#define MODULE_DEBUG \
+do { \
+   static unsigned int DEBUG_VARIABLE = 0; \
+   module_param(DEBUG_VARIABLE, uint, 0644); \
+   MODULE_PARM_DESC(DEBUG_VARIABLE, "Enable debug messages"); \
+} while(0)
+
+/* regular debug messages */
+#define debug(fmt,arg...) do { if (DEBUG_VARIABLE) unc_debug(fmt, ##arg); } 
while (0)
+
+/* different levels of debug messages (only output if DEBUG_VARIABLE is large 
enough) */
+#define ldebug(level,fmt,arg...) do { if (DEBUG_VARIABLE >= level) 
unc_debug(fmt, ##arg); } while (0)
+
+
+
+/* 
+ * debug messages (de)activated by the preprocessor 
+ */

+#ifdef DEBUG
+#define pr_debug(fmt,arg...) unc_debug(fmt, ##arg)
+#else
+#define pr_debug(fmt,arg...) do { } while (0)
+#endif
+
+#endif /* _LINUX_DEBUG_H */
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Consolidate dprintk and friends?

[David Härdeman]

A case-insensitive grep through the 2.6.13 tree for define printk 
yields 378 hits (define dbg: 736, define warn: 50, define info: 
65).


Maybe it would be a good idea to create include/linux/debug.h as 
included inline below (or to add the content in include/linux/kernel.h 
where pr_debug is already located)? 


Then a module could simply do:

#include linux/debug.h
MODULE_DEBUG;

and then sprinkle debug() statements where appropriate.

It would also make the name and description of the debug variable in 
sysfs more consistent. 

I tried searching the lkml archives, but couldn't find any discussion on 
the topic...so is it a good idea?


Re,
David


--- /dev/null   2005-09-01 20:05:20.395616648 +0200
+++ linux-2.6.13/include/linux/debug.h  2005-09-01 20:01:56.0 +0200
@@ -0,0 +1,52 @@
+#ifndef _LINUX_DEBUG_H
+#define _LINUX_DEBUG_H
+
+/*
+ * unconditional messages 
+ */

+
+/* unc(onditional)_debug should not be used, but rather debug or pr_debug */
+#define unc_debug(fmt,arg...) printk(KERN_DEBUG fmt, ##arg)
+
+/* convenience macros */
+#define info(fmt,arg...) printk(KERN_INFO fmt, ##arg)
+#define warn(fmt,arg...) printk(KERN_WARN fmt, ##arg)
+#define err(fmt,arg...) printk(KERN_EMERG fmt, ##arg)
+
+
+
+/*
+ * debug messages (de)activated at runtime 
+ */

+
+/* the variable which (de)activates debugging messages */
+#ifndef DEBUG_VARIABLE
+#define DEBUG_VARIABLE debug
+#endif
+
+/* quick macro to add a debug parameter to a module */
+#define MODULE_DEBUG \
+do { \
+   static unsigned int DEBUG_VARIABLE = 0; \
+   module_param(DEBUG_VARIABLE, uint, 0644); \
+   MODULE_PARM_DESC(DEBUG_VARIABLE, Enable debug messages); \
+} while(0)
+
+/* regular debug messages */
+#define debug(fmt,arg...) do { if (DEBUG_VARIABLE) unc_debug(fmt, ##arg); } 
while (0)
+
+/* different levels of debug messages (only output if DEBUG_VARIABLE is large 
enough) */
+#define ldebug(level,fmt,arg...) do { if (DEBUG_VARIABLE = level) 
unc_debug(fmt, ##arg); } while (0)
+
+
+
+/* 
+ * debug messages (de)activated by the preprocessor 
+ */

+#ifdef DEBUG
+#define pr_debug(fmt,arg...) unc_debug(fmt, ##arg)
+#else
+#define pr_debug(fmt,arg...) do { } while (0)
+#endif
+
+#endif /* _LINUX_DEBUG_H */
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: LSM root_plug module questions

[David Härdeman]

Chris Wright ([EMAIL PROTECTED]) wrote:
> * David Härdeman ([EMAIL PROTECTED]) wrote:
> > I'm currently playing around with the security/root_plug.c LSM module
> you'll have better luck on the lsm list

Thanks for the pointer

> > 1) What's the recommended way of telling that someone is logging in to
> > the computer (via ssh, virtual console, serial console, X, whatever)
> > with LSM? Look for open() on /dev/pts?
>
> logging in...this is really a userspace notion, so via PAM.  creating a
> new process or changing credentials of a new process are the types of
> things that lsm watches (and of course, opening of files).

Yes, I realized that by reading the include/linux/security.h comments
describing the security hooks. The question is rather if there is something
which all the different methods of logging in have in common that can be
caught with a LSM hook?

> > 2) root_plug currently scans the usb device tree looking for the
> > appropriate device each time it's needed. In the interest of making the
> > result of the lookup cached, it is possible for a module to register so
> > that it is notified when a usb device is added/removed?
>
> I don't think that can be done in a race free manner.  Perhaps get the
> device and check its state, but you'd have to ask usb folks.  ATM, it's
> only checked during exec of root process.

The reason that I wanted to do caching is that I want to add more checks
to the root_plug module. For instance, to deny all socket accept() and
connect() calls when the USB module is missing (to not break already
established connections but not allow any new ones, e.g. to lock out any
new SSH sessions).

I'm assuming that this could introduce the need for some kind of caching
of the results of the USB-device-present check as the number of checks
increase.

Regards,
David

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: LSM root_plug module questions

[David Härdeman]

Chris Wright ([EMAIL PROTECTED]) wrote:
 * David Härdeman ([EMAIL PROTECTED]) wrote:
  I'm currently playing around with the security/root_plug.c LSM module
 you'll have better luck on the lsm list

Thanks for the pointer

  1) What's the recommended way of telling that someone is logging in to
  the computer (via ssh, virtual console, serial console, X, whatever)
  with LSM? Look for open() on /dev/pts?

 logging in...this is really a userspace notion, so via PAM.  creating a
 new process or changing credentials of a new process are the types of
 things that lsm watches (and of course, opening of files).

Yes, I realized that by reading the include/linux/security.h comments
describing the security hooks. The question is rather if there is something
which all the different methods of logging in have in common that can be
caught with a LSM hook?

  2) root_plug currently scans the usb device tree looking for the
  appropriate device each time it's needed. In the interest of making the
  result of the lookup cached, it is possible for a module to register so
  that it is notified when a usb device is added/removed?

 I don't think that can be done in a race free manner.  Perhaps get the
 device and check its state, but you'd have to ask usb folks.  ATM, it's
 only checked during exec of root process.

The reason that I wanted to do caching is that I want to add more checks
to the root_plug module. For instance, to deny all socket accept() and
connect() calls when the USB module is missing (to not break already
established connections but not allow any new ones, e.g. to lock out any
new SSH sessions).

I'm assuming that this could introduce the need for some kind of caching
of the results of the USB-device-present check as the number of checks
increase.

Regards,
David

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

LSM root_plug module questions

[David Härdeman]

Hi,

I'm currently playing around with the security/root_plug.c LSM module 
and I have two questions:


1) What's the recommended way of telling that someone is logging in to 
the computer (via ssh, virtual console, serial console, X, whatever) 
with LSM? Look for open() on /dev/pts?


2) root_plug currently scans the usb device tree looking for the 
appropriate device each time it's needed. In the interest of making the 
result of the lookup cached, it is possible for a module to register so 
that it is notified when a usb device is added/removed?


Regards,
David

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

LSM root_plug module questions

[David Härdeman]

Hi,

I'm currently playing around with the security/root_plug.c LSM module 
and I have two questions:


1) What's the recommended way of telling that someone is logging in to 
the computer (via ssh, virtual console, serial console, X, whatever) 
with LSM? Look for open() on /dev/pts?


2) root_plug currently scans the usb device tree looking for the 
appropriate device each time it's needed. In the interest of making the 
result of the lookup cached, it is possible for a module to register so 
that it is notified when a usb device is added/removed?


Regards,
David

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Use sg_init_one where appropriate

[David Härdeman]

On Sat, Aug 27, 2005 at 02:06:26PM +0200, Ingo Oeser wrote:
In short: please remove all "const" markers from the function, 
	try to uninline it somewhere and resend.


Indeed, only buflen should be const, thanks for pointing it out.
A new patch is below.


And while you are at it, please check, wether this can be uninlined,
since it does a lot of things and is called from quite some sites
then.


I wouldn't quite agree, four lines isn't exactly "a lot of things". And 
I guess that there would have been objections when the function was 
first created [1]. Anyway, using sg_init_one more and changing it to be a 
regular function seems like orthogonal issues to me.


Regards,
David Härdeman
[1] http://www.ussg.iu.edu/hypermail/linux/kernel/0409.1/0257.html




This patch uses sg_init_one in some places where it was duplicated.

Signed-off-by: David Härdeman <[EMAIL PROTECTED]>



crypto/hmac.c |   23 ++-
crypto/tcrypt.c   |   29 -
drivers/md/dm-crypt.c |   11 ---
drivers/net/wireless/airo.c   |5 ++---
drivers/scsi/arm/scsi.h   |6 ++
drivers/scsi/libata-core.c|   10 ++
drivers/scsi/sg.c |5 ++---
drivers/usb/misc/usbtest.c|6 ++
include/linux/scatterlist.h   |3 ++-
net/ipv6/addrconf.c   |9 +++--
net/sunrpc/auth_gss/gss_krb5_crypto.c |   22 ++
11 files changed, 39 insertions(+), 90 deletions(-)



Index: linux-sginitone/include/linux/scatterlist.h
===
--- linux-sginitone.orig/include/linux/scatterlist.h2005-03-02 
08:38:32.0 +0100
+++ linux-sginitone/include/linux/scatterlist.h 2005-08-27 20:29:46.0 
+0200
@@ -2,7 +2,8 @@
#define _LINUX_SCATTERLIST_H

static inline void sg_init_one(struct scatterlist *sg,
-  u8 *buf, unsigned int buflen)
+  u8 *buf,
+  const unsigned int buflen)
{
memset(sg, 0, sizeof(*sg));

Index: linux-sginitone/crypto/hmac.c
===
--- linux-sginitone.orig/crypto/hmac.c  2005-03-02 08:38:09.0 +0100
+++ linux-sginitone/crypto/hmac.c   2005-08-27 00:32:26.0 +0200
@@ -19,17 +19,15 @@
#include 
#include 
#include 
+#include 
#include "internal.h"

static void hash_key(struct crypto_tfm *tfm, u8 *key, unsigned int keylen)
{
struct scatterlist tmp;
-   
-   tmp.page = virt_to_page(key);
-   tmp.offset = offset_in_page(key);
-   tmp.length = keylen;
+
+   sg_init_one(tmp, key, keylen);
crypto_digest_digest(tfm, , 1, key);
-   
}

int crypto_alloc_hmac_block(struct crypto_tfm *tfm)
@@ -70,10 +68,7 @@
for (i = 0; i < crypto_tfm_alg_blocksize(tfm); i++)
ipad[i] ^= 0x36;

-   tmp.page = virt_to_page(ipad);
-   tmp.offset = offset_in_page(ipad);
-   tmp.length = crypto_tfm_alg_blocksize(tfm);
-   
+   sg_init_one(tmp, ipad, crypto_tfm_alg_blocksize(tfm));
crypto_digest_init(tfm);
crypto_digest_update(tfm, , 1);
}
@@ -104,17 +99,11 @@
for (i = 0; i < crypto_tfm_alg_blocksize(tfm); i++)
opad[i] ^= 0x5c;

-   tmp.page = virt_to_page(opad);
-   tmp.offset = offset_in_page(opad);
-   tmp.length = crypto_tfm_alg_blocksize(tfm);
-
+   sg_init_one(tmp, opad, crypto_tfm_alg_blocksize(tfm));
crypto_digest_init(tfm);
crypto_digest_update(tfm, , 1);

-   tmp.page = virt_to_page(out);
-   tmp.offset = offset_in_page(out);
-   tmp.length = crypto_tfm_alg_digestsize(tfm);
-   
+   sg_init_one(tmp, out, crypto_tfm_alg_blocksize(tfm));
crypto_digest_update(tfm, , 1);
crypto_digest_final(tfm, out);
}
Index: linux-sginitone/crypto/tcrypt.c
===
--- linux-sginitone.orig/crypto/tcrypt.c2005-08-19 22:40:05.0 
+0200
+++ linux-sginitone/crypto/tcrypt.c 2005-08-27 00:45:05.0 +0200
@@ -21,6 +21,7 @@
#include 
#include 
#include 
+#include 
#include 
#include 
#include 
@@ -109,9 +110,7 @@
memset (result, 0, 64);

p = hash_tv[i].plaintext;
-   sg[0].page = virt_to_page (p);
-   sg[0].offset = offset_in_page (p);
-   sg[0].length = hash_tv[i].psize;
+   sg_init_one([0], p, hash_tv[i].psize);

crypto_digest_init (tfm);
if (tfm->crt_u.digest.dit_setkey) {
@@ -146,9 +145,7 @@
hash_tv[i].tap[k]); 
temp += hash_tv[i].tap[k];
p = [IDX[k]];
-  

Re: [PATCH] Use sg_init_one where appropriate

[David Härdeman]

On Sat, Aug 27, 2005 at 02:06:26PM +0200, Ingo Oeser wrote:
In short: please remove all const markers from the function, 
	try to uninline it somewhere and resend.


Indeed, only buflen should be const, thanks for pointing it out.
A new patch is below.


And while you are at it, please check, wether this can be uninlined,
since it does a lot of things and is called from quite some sites
then.


I wouldn't quite agree, four lines isn't exactly a lot of things. And 
I guess that there would have been objections when the function was 
first created [1]. Anyway, using sg_init_one more and changing it to be a 
regular function seems like orthogonal issues to me.


Regards,
David Härdeman
[1] http://www.ussg.iu.edu/hypermail/linux/kernel/0409.1/0257.html




This patch uses sg_init_one in some places where it was duplicated.

Signed-off-by: David Härdeman [EMAIL PROTECTED]



crypto/hmac.c |   23 ++-
crypto/tcrypt.c   |   29 -
drivers/md/dm-crypt.c |   11 ---
drivers/net/wireless/airo.c   |5 ++---
drivers/scsi/arm/scsi.h   |6 ++
drivers/scsi/libata-core.c|   10 ++
drivers/scsi/sg.c |5 ++---
drivers/usb/misc/usbtest.c|6 ++
include/linux/scatterlist.h   |3 ++-
net/ipv6/addrconf.c   |9 +++--
net/sunrpc/auth_gss/gss_krb5_crypto.c |   22 ++
11 files changed, 39 insertions(+), 90 deletions(-)



Index: linux-sginitone/include/linux/scatterlist.h
===
--- linux-sginitone.orig/include/linux/scatterlist.h2005-03-02 
08:38:32.0 +0100
+++ linux-sginitone/include/linux/scatterlist.h 2005-08-27 20:29:46.0 
+0200
@@ -2,7 +2,8 @@
#define _LINUX_SCATTERLIST_H

static inline void sg_init_one(struct scatterlist *sg,
-  u8 *buf, unsigned int buflen)
+  u8 *buf,
+  const unsigned int buflen)
{
memset(sg, 0, sizeof(*sg));

Index: linux-sginitone/crypto/hmac.c
===
--- linux-sginitone.orig/crypto/hmac.c  2005-03-02 08:38:09.0 +0100
+++ linux-sginitone/crypto/hmac.c   2005-08-27 00:32:26.0 +0200
@@ -19,17 +19,15 @@
#include linux/highmem.h
#include linux/slab.h
#include asm/scatterlist.h
+#include linux/scatterlist.h
#include internal.h

static void hash_key(struct crypto_tfm *tfm, u8 *key, unsigned int keylen)
{
struct scatterlist tmp;
-   
-   tmp.page = virt_to_page(key);
-   tmp.offset = offset_in_page(key);
-   tmp.length = keylen;
+
+   sg_init_one(tmp, key, keylen);
crypto_digest_digest(tfm, tmp, 1, key);
-   
}

int crypto_alloc_hmac_block(struct crypto_tfm *tfm)
@@ -70,10 +68,7 @@
for (i = 0; i  crypto_tfm_alg_blocksize(tfm); i++)
ipad[i] ^= 0x36;

-   tmp.page = virt_to_page(ipad);
-   tmp.offset = offset_in_page(ipad);
-   tmp.length = crypto_tfm_alg_blocksize(tfm);
-   
+   sg_init_one(tmp, ipad, crypto_tfm_alg_blocksize(tfm));
crypto_digest_init(tfm);
crypto_digest_update(tfm, tmp, 1);
}
@@ -104,17 +99,11 @@
for (i = 0; i  crypto_tfm_alg_blocksize(tfm); i++)
opad[i] ^= 0x5c;

-   tmp.page = virt_to_page(opad);
-   tmp.offset = offset_in_page(opad);
-   tmp.length = crypto_tfm_alg_blocksize(tfm);
-
+   sg_init_one(tmp, opad, crypto_tfm_alg_blocksize(tfm));
crypto_digest_init(tfm);
crypto_digest_update(tfm, tmp, 1);

-   tmp.page = virt_to_page(out);
-   tmp.offset = offset_in_page(out);
-   tmp.length = crypto_tfm_alg_digestsize(tfm);
-   
+   sg_init_one(tmp, out, crypto_tfm_alg_blocksize(tfm));
crypto_digest_update(tfm, tmp, 1);
crypto_digest_final(tfm, out);
}
Index: linux-sginitone/crypto/tcrypt.c
===
--- linux-sginitone.orig/crypto/tcrypt.c2005-08-19 22:40:05.0 
+0200
+++ linux-sginitone/crypto/tcrypt.c 2005-08-27 00:45:05.0 +0200
@@ -21,6 +21,7 @@
#include linux/mm.h
#include linux/slab.h
#include asm/scatterlist.h
+#include linux/scatterlist.h
#include linux/string.h
#include linux/crypto.h
#include linux/highmem.h
@@ -109,9 +110,7 @@
memset (result, 0, 64);

p = hash_tv[i].plaintext;
-   sg[0].page = virt_to_page (p);
-   sg[0].offset = offset_in_page (p);
-   sg[0].length = hash_tv[i].psize;
+   sg_init_one(sg[0], p, hash_tv[i].psize);

crypto_digest_init (tfm);
if (tfm-crt_u.digest.dit_setkey) {
@@ -146,9 +145,7 @@
hash_tv[i].tap[k

[PATCH] Use sg_init_one where appropriate

[David Härdeman]

The same code as in sg_init_one can be found in a number of places, this 
patch changes them to call the function instead.


Signed-off-by: David Härdeman <[EMAIL PROTECTED]>

---

crypto/hmac.c |   23 ++-
crypto/tcrypt.c   |   29 -
drivers/md/dm-crypt.c |   11 ---
drivers/net/wireless/airo.c   |5 ++---
drivers/scsi/arm/scsi.h   |6 ++
drivers/scsi/libata-core.c|   10 ++
drivers/scsi/sg.c |5 ++---
drivers/usb/misc/usbtest.c|6 ++
include/linux/scatterlist.h   |5 +++--
net/ipv6/addrconf.c   |9 +++--
net/sunrpc/auth_gss/gss_krb5_crypto.c |   22 ++
11 files changed, 40 insertions(+), 91 deletions(-)



Index: linux-sginitone/include/linux/scatterlist.h
===
--- linux-sginitone.orig/include/linux/scatterlist.h2005-03-02 
08:38:32.0 +0100
+++ linux-sginitone/include/linux/scatterlist.h 2005-08-27 00:20:53.0 
+0200
@@ -1,8 +1,9 @@
#ifndef _LINUX_SCATTERLIST_H
#define _LINUX_SCATTERLIST_H

-static inline void sg_init_one(struct scatterlist *sg,
-  u8 *buf, unsigned int buflen)
+static inline void sg_init_one(const struct scatterlist *sg,
+  const u8 *buf,
+  const unsigned int buflen)
{
memset(sg, 0, sizeof(*sg));

Index: linux-sginitone/crypto/hmac.c
===
--- linux-sginitone.orig/crypto/hmac.c  2005-03-02 08:38:09.0 +0100
+++ linux-sginitone/crypto/hmac.c   2005-08-27 00:32:26.0 +0200
@@ -19,17 +19,15 @@
#include 
#include 
#include 
+#include 
#include "internal.h"

static void hash_key(struct crypto_tfm *tfm, u8 *key, unsigned int keylen)
{
struct scatterlist tmp;
-   
-   tmp.page = virt_to_page(key);
-   tmp.offset = offset_in_page(key);
-   tmp.length = keylen;
+
+   sg_init_one(tmp, key, keylen);
crypto_digest_digest(tfm, , 1, key);
-   
}

int crypto_alloc_hmac_block(struct crypto_tfm *tfm)
@@ -70,10 +68,7 @@
for (i = 0; i < crypto_tfm_alg_blocksize(tfm); i++)
ipad[i] ^= 0x36;

-   tmp.page = virt_to_page(ipad);
-   tmp.offset = offset_in_page(ipad);
-   tmp.length = crypto_tfm_alg_blocksize(tfm);
-   
+   sg_init_one(tmp, ipad, crypto_tfm_alg_blocksize(tfm));
crypto_digest_init(tfm);
crypto_digest_update(tfm, , 1);
}
@@ -104,17 +99,11 @@
for (i = 0; i < crypto_tfm_alg_blocksize(tfm); i++)
opad[i] ^= 0x5c;

-   tmp.page = virt_to_page(opad);
-   tmp.offset = offset_in_page(opad);
-   tmp.length = crypto_tfm_alg_blocksize(tfm);
-
+   sg_init_one(tmp, opad, crypto_tfm_alg_blocksize(tfm));
crypto_digest_init(tfm);
crypto_digest_update(tfm, , 1);

-   tmp.page = virt_to_page(out);
-   tmp.offset = offset_in_page(out);
-   tmp.length = crypto_tfm_alg_digestsize(tfm);
-   
+   sg_init_one(tmp, out, crypto_tfm_alg_blocksize(tfm));
crypto_digest_update(tfm, , 1);
crypto_digest_final(tfm, out);
}
Index: linux-sginitone/crypto/tcrypt.c
===
--- linux-sginitone.orig/crypto/tcrypt.c2005-08-19 22:40:05.0 
+0200
+++ linux-sginitone/crypto/tcrypt.c 2005-08-27 00:45:05.0 +0200
@@ -21,6 +21,7 @@
#include 
#include 
#include 
+#include 
#include 
#include 
#include 
@@ -109,9 +110,7 @@
memset (result, 0, 64);

p = hash_tv[i].plaintext;
-   sg[0].page = virt_to_page (p);
-   sg[0].offset = offset_in_page (p);
-   sg[0].length = hash_tv[i].psize;
+   sg_init_one([0], p, hash_tv[i].psize);

crypto_digest_init (tfm);
if (tfm->crt_u.digest.dit_setkey) {
@@ -146,9 +145,7 @@
hash_tv[i].tap[k]); 
temp += hash_tv[i].tap[k];
p = [IDX[k]];
-   sg[k].page = virt_to_page (p);
-   sg[k].offset = offset_in_page (p);
-   sg[k].length = hash_tv[i].tap[k];
+   sg_init_one([k], p, hash_tv[i].tap[k]);
}

crypto_digest_digest (tfm, sg, hash_tv[i].np, result);
@@ -203,9 +200,7 @@

p = hmac_tv[i].plaintext;
klen = hmac_tv[i].ksize;
-   sg[0].page = virt_to_page(p);
-   sg[0].offset = offset_in_page(p);
-   sg[0].length = hmac_tv[i].psize;
+   sg_init_one

[PATCH] Use sg_init_one where appropriate

[David Härdeman]

The same code as in sg_init_one can be found in a number of places, this 
patch changes them to call the function instead.


Signed-off-by: David Härdeman [EMAIL PROTECTED]

---

crypto/hmac.c |   23 ++-
crypto/tcrypt.c   |   29 -
drivers/md/dm-crypt.c |   11 ---
drivers/net/wireless/airo.c   |5 ++---
drivers/scsi/arm/scsi.h   |6 ++
drivers/scsi/libata-core.c|   10 ++
drivers/scsi/sg.c |5 ++---
drivers/usb/misc/usbtest.c|6 ++
include/linux/scatterlist.h   |5 +++--
net/ipv6/addrconf.c   |9 +++--
net/sunrpc/auth_gss/gss_krb5_crypto.c |   22 ++
11 files changed, 40 insertions(+), 91 deletions(-)



Index: linux-sginitone/include/linux/scatterlist.h
===
--- linux-sginitone.orig/include/linux/scatterlist.h2005-03-02 
08:38:32.0 +0100
+++ linux-sginitone/include/linux/scatterlist.h 2005-08-27 00:20:53.0 
+0200
@@ -1,8 +1,9 @@
#ifndef _LINUX_SCATTERLIST_H
#define _LINUX_SCATTERLIST_H

-static inline void sg_init_one(struct scatterlist *sg,
-  u8 *buf, unsigned int buflen)
+static inline void sg_init_one(const struct scatterlist *sg,
+  const u8 *buf,
+  const unsigned int buflen)
{
memset(sg, 0, sizeof(*sg));

Index: linux-sginitone/crypto/hmac.c
===
--- linux-sginitone.orig/crypto/hmac.c  2005-03-02 08:38:09.0 +0100
+++ linux-sginitone/crypto/hmac.c   2005-08-27 00:32:26.0 +0200
@@ -19,17 +19,15 @@
#include linux/highmem.h
#include linux/slab.h
#include asm/scatterlist.h
+#include linux/scatterlist.h
#include internal.h

static void hash_key(struct crypto_tfm *tfm, u8 *key, unsigned int keylen)
{
struct scatterlist tmp;
-   
-   tmp.page = virt_to_page(key);
-   tmp.offset = offset_in_page(key);
-   tmp.length = keylen;
+
+   sg_init_one(tmp, key, keylen);
crypto_digest_digest(tfm, tmp, 1, key);
-   
}

int crypto_alloc_hmac_block(struct crypto_tfm *tfm)
@@ -70,10 +68,7 @@
for (i = 0; i  crypto_tfm_alg_blocksize(tfm); i++)
ipad[i] ^= 0x36;

-   tmp.page = virt_to_page(ipad);
-   tmp.offset = offset_in_page(ipad);
-   tmp.length = crypto_tfm_alg_blocksize(tfm);
-   
+   sg_init_one(tmp, ipad, crypto_tfm_alg_blocksize(tfm));
crypto_digest_init(tfm);
crypto_digest_update(tfm, tmp, 1);
}
@@ -104,17 +99,11 @@
for (i = 0; i  crypto_tfm_alg_blocksize(tfm); i++)
opad[i] ^= 0x5c;

-   tmp.page = virt_to_page(opad);
-   tmp.offset = offset_in_page(opad);
-   tmp.length = crypto_tfm_alg_blocksize(tfm);
-
+   sg_init_one(tmp, opad, crypto_tfm_alg_blocksize(tfm));
crypto_digest_init(tfm);
crypto_digest_update(tfm, tmp, 1);

-   tmp.page = virt_to_page(out);
-   tmp.offset = offset_in_page(out);
-   tmp.length = crypto_tfm_alg_digestsize(tfm);
-   
+   sg_init_one(tmp, out, crypto_tfm_alg_blocksize(tfm));
crypto_digest_update(tfm, tmp, 1);
crypto_digest_final(tfm, out);
}
Index: linux-sginitone/crypto/tcrypt.c
===
--- linux-sginitone.orig/crypto/tcrypt.c2005-08-19 22:40:05.0 
+0200
+++ linux-sginitone/crypto/tcrypt.c 2005-08-27 00:45:05.0 +0200
@@ -21,6 +21,7 @@
#include linux/mm.h
#include linux/slab.h
#include asm/scatterlist.h
+#include linux/scatterlist.h
#include linux/string.h
#include linux/crypto.h
#include linux/highmem.h
@@ -109,9 +110,7 @@
memset (result, 0, 64);

p = hash_tv[i].plaintext;
-   sg[0].page = virt_to_page (p);
-   sg[0].offset = offset_in_page (p);
-   sg[0].length = hash_tv[i].psize;
+   sg_init_one(sg[0], p, hash_tv[i].psize);

crypto_digest_init (tfm);
if (tfm-crt_u.digest.dit_setkey) {
@@ -146,9 +145,7 @@
hash_tv[i].tap[k]); 
temp += hash_tv[i].tap[k];
p = xbuf[IDX[k]];
-   sg[k].page = virt_to_page (p);
-   sg[k].offset = offset_in_page (p);
-   sg[k].length = hash_tv[i].tap[k];
+   sg_init_one(sg[k], p, hash_tv[i].tap[k]);
}

crypto_digest_digest (tfm, sg, hash_tv[i].np, result);
@@ -203,9 +200,7 @@

p = hmac_tv[i].plaintext;
klen = hmac_tv[i].ksize;
-   sg[0

Re: [-mm PATCH] remove use of pci_find_device in watchdog driver for Intel 6300ESB chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:30:15PM -0700, Naveen Gupta wrote:
[...}

-while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
-if (pci_match_id(esb_pci_tbl, dev)) {
-esb_pci = dev;
-break;
-}
-}
+   while (ids->vendor && ids->device) {
+   if ((dev = pci_get_device(ids->vendor, ids->device, dev)) != 
NULL) {
+   esb_pci = dev;
+   break;
+   }
+   ids++;
+   }


I'm certainly not sure about this, but the proposed while loop looks a 
bit unconventional, wouldn't something like:


for_each_pci_dev(dev)
if (pci_match_id(esb_pci_tbl, dev)) {
esb_pci = dev;
break;
}
}

be better?

//David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [-mm PATCH] set correct bit in reload register of Watchdog Timer for Intel 6300 chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:02:19PM -0700, Naveen Gupta wrote:


This patch writes into bit 8 of the reload register to perform the
correct 'Reload Sequence' instead of writing into bit 4 of Watchdog for
Intel 6300ESB chipset.

Signed-off-by: Naveen Gupta <[EMAIL PROTECTED]>


Acked-by: David Härdeman <[EMAIL PROTECTED]>

Thanks alot Naveen.

//David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:21:05PM -0700, Naveen Gupta wrote:


This patch sets the WDT_ENABLE bit of the Lock Register to enable the
watchdog and WDT_LOCK bit only if nowayout is set. The old code always
sets the WDT_LOCK bit of watchdog timer for Intel 6300ESB chipset. So, we
end up locking the watchdog instead of enabling it.

Signed-off-by: Naveen Gupta <[EMAIL PROTECTED]>


Acked-by: David Härdeman <[EMAIL PROTECTED]>

Thanks Naveen.

//David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:21:05PM -0700, Naveen Gupta wrote:


This patch sets the WDT_ENABLE bit of the Lock Register to enable the
watchdog and WDT_LOCK bit only if nowayout is set. The old code always
sets the WDT_LOCK bit of watchdog timer for Intel 6300ESB chipset. So, we
end up locking the watchdog instead of enabling it.


Naveen,

thanks alot for testing the driver further and finding these bugs. I've 
not been able to do so myself as the only computers available to me with 
this watchdog are production-servers meaning that I've only been able to 
test during scheduled downtimes.


Have you tested and verified that the driver works after these patches 
have been applied?


Re,
David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:21:05PM -0700, Naveen Gupta wrote:


This patch sets the WDT_ENABLE bit of the Lock Register to enable the
watchdog and WDT_LOCK bit only if nowayout is set. The old code always
sets the WDT_LOCK bit of watchdog timer for Intel 6300ESB chipset. So, we
end up locking the watchdog instead of enabling it.


Naveen,

thanks alot for testing the driver further and finding these bugs. I've 
not been able to do so myself as the only computers available to me with 
this watchdog are production-servers meaning that I've only been able to 
test during scheduled downtimes.


Have you tested and verified that the driver works after these patches 
have been applied?


Re,
David
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [-mm PATCH] set correct bit in reload register of Watchdog Timer for Intel 6300 chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:02:19PM -0700, Naveen Gupta wrote:


This patch writes into bit 8 of the reload register to perform the
correct 'Reload Sequence' instead of writing into bit 4 of Watchdog for
Intel 6300ESB chipset.

Signed-off-by: Naveen Gupta [EMAIL PROTECTED]


Acked-by: David Härdeman [EMAIL PROTECTED]

Thanks alot Naveen.

//David
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [-mm PATCH] set enable bit instead of lock bit of Watchdog Timer in Intel 6300 chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:21:05PM -0700, Naveen Gupta wrote:


This patch sets the WDT_ENABLE bit of the Lock Register to enable the
watchdog and WDT_LOCK bit only if nowayout is set. The old code always
sets the WDT_LOCK bit of watchdog timer for Intel 6300ESB chipset. So, we
end up locking the watchdog instead of enabling it.

Signed-off-by: Naveen Gupta [EMAIL PROTECTED]


Acked-by: David Härdeman [EMAIL PROTECTED]

Thanks Naveen.

//David
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [-mm PATCH] remove use of pci_find_device in watchdog driver for Intel 6300ESB chipset

[David Härdeman]

On Mon, Aug 15, 2005 at 02:30:15PM -0700, Naveen Gupta wrote:
[...}

-while ((dev = pci_find_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
-if (pci_match_id(esb_pci_tbl, dev)) {
-esb_pci = dev;
-break;
-}
-}
+   while (ids-vendor  ids-device) {
+   if ((dev = pci_get_device(ids-vendor, ids-device, dev)) != 
NULL) {
+   esb_pci = dev;
+   break;
+   }
+   ids++;
+   }


I'm certainly not sure about this, but the proposed while loop looks a 
bit unconventional, wouldn't something like:


for_each_pci_dev(dev)
if (pci_match_id(esb_pci_tbl, dev)) {
esb_pci = dev;
break;
}
}

be better?

//David
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: IBM Thinkpad G41 PCMCIA problems [Was: Yenta TI: ... no PCI

[David Härdeman]

On Sun, 20 Feb 2005, Linus Torvalds wrote:
Russell, what do your eagle-eyes think of a patch like this?
Steven, does this fix it without the need for any kernel command line (or
any other patches, for that matter - ie revert all the transparency-
changing ones)?
		Linus
I tried the patch on my G40 with 1Gb RAM (previous thread about it is at 
http://marc.theaimsgroup.com/?t=11088915341=1=2), and it works 
great, PCI resources are now located at 0x4000 and no further 
tricks/patches are necessary.

Re,
David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

6300ESB watchdog driver

[David Härdeman]

Hi
I wrote earlier to the list[1] asking for a driver for the watchdog 
included in the 6300ESB chipset. I got a 2.4 driver via private email 
from Ross Biro which I've changed into what I hope resembles a 2.6 
driver (which was done by looking a lot at the watchdog drivers already 
in the 2.6 tree).

I've attached the result, and I'm hoping to get some feedback on the 
coding as a first step. I can't actually test it on the hardware right 
now as I won't have physical access until April. So my own tests have 
been limited to "compiles-without-warnings" and 
"can-be-insmodded-in-other-machine-without-oops".

As I said, it's the first thing I've ever written, so I'm guessing it 
has a few rough edges. Feedback welcome :-)

Re,
David
[1] http://marc.theaimsgroup.com/?l=linux-kernel=110711079825794=2
[2] http://marc.theaimsgroup.com/?l=linux-kernel=110711973917746=2
diff -Nur linux-2.6.10.orig/drivers/char/watchdog/i6300esb.c 
linux-2.6.10/drivers/char/watchdog/i6300esb.c
--- linux-2.6.10.orig/drivers/char/watchdog/i6300esb.c  1970-01-01 
01:00:00.0 +0100
+++ linux-2.6.10/drivers/char/watchdog/i6300esb.c   2005-02-07 
23:27:08.0 +0100
@@ -0,0 +1,508 @@
+/*
+ * i6300esb 0.03:  Watchdog timer driver for Intel 6300ESB chipset
+ *
+ * (c) Copyright 2004 Google Inc. 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ * 
+ *  based on i810-tco.c which is
+ *
+ * (c) Copyright 2000  kernel concepts <[EMAIL PROTECTED]>
+ * developed for
+ *  Jentro AG, Haar/Munich (Germany)
+ *
+ * which is in turn based on softdog.c by Alan Cox <[EMAIL PROTECTED]>
+ *
+ * The timer is implemented in the following I/O controller hubs:
+ * (See the intel documentation on http://developer.intel.com.)
+ * 6300ESB chip : document number 300641-003
+ * 
+ *  2004YYZZ Ross Biro
+ * Initial version 0.01
+ *  2004YYZZ Ross Biro
+ * Version 0.02
+ *  20050210 David Härdeman <[EMAIL PROTECTED]>
+ *  Ported driver to kernel 2.6
+ */
+
+/*
+ *  Includes, defines, variables, module parameters, ...
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include "i6300esb.h"
+
+/* Module and version information */
+#define ESB_VERSION "0.03"
+#define ESB_MODULE_NAME "i6300ESB timer"
+#define ESB_DRIVER_NAME ESB_MODULE_NAME ", v" ESB_VERSION
+#define PFX ESB_MODULE_NAME ": "
+
+/* internal variables */
+static void __iomem *BASEADDR;
+static spinlock_t esb_lock; /* Guards the hardware */
+static unsigned long timer_alive;
+static struct pci_dev *esb_pci;
+static unsigned short triggered; /* The status of the watchdog upon boot */
+static char esb_expect_close;
+
+/* module parameters */
+#define WATCHDOG_HEARTBEAT 30   /* 30 sec default heartbeat 
(1 (2 * 0x03ff))
+   return -EINVAL;
+   
+   spin_lock(_lock);
+   
+   /* We shift by 9, so if we are passed a value of 1 sec,
+* val will be 1 << 9 = 512, then write that to two
+* timers => 2 * 512 = 1024 (which is decremented at 1KHz)
+*/
+   val = time << 9;
+   
+   /* Write timer 1 */
+   esb_unlock_registers();
+   writel(val, ESB_TIMER1_REG);
+   
+   /* Write timer 2 */
+   esb_unlock_registers();
+writel(val, ESB_TIMER2_REG);
+   
+/* Reload */
+   esb_unlock_registers();
+   writew(0x10, ESB_RELOAD_REG);
+
+   /* FIXME: Do we need to flush everything out? */
+   
+   /* Done */
+   heartbeat = time;
+   spin_unlock(_lock);
+   return 0;
+}
+
+static int esb_timer_read (void)
+{
+   u32 count;
+
+   /* This isn't documented, and doesn't take into
+ * acount which stage is running, but it looks
+ * like a 20 bit count down, so we might as well report it.
+ */
+pci_read_config_dword(esb_pci, 0x64, );
+return (int)count;
+}
+
+/*
+ * /dev/watchdog handling
+ */
+
+static int esb_open (struct inode *inode, struct file *file)
+{
+/* /dev/watchdog can only be opened once */
+if (test_and_set_bit(0, _alive))
+return -EBUSY;
+
+/* Reload and activate timer */
+esb_timer_keepalive ();
+esb_timer_start ();
+
+   return nonseekable_open(inode, file);
+}
+
+static int esb_release (struct inode *inode, struct file *file)
+{
+/* Shut off the timer. */
+if (esb_expect_close == 42) {
+esb_timer_stop ();
+} else {
+pr

Re: IBM Thinkpad G41 PCMCIA problems

[David Härdeman]

On Sun, Feb 20, 2005 at 10:19:02AM +, Russell King wrote:
On Sun, Feb 20, 2005 at 10:52:12AM +0100, David Härdeman wrote:
Is the hole between 0x36f6fa000 and 0x3f70?
Looks like it.
And what would be the proper way of fixing it (assuming that IBM won't 
issue a fixed BIOS)?
Try passing:
reserve=0x3f6fa000,0x6000
to the kernel.
Thanks a bunch, that worked, now I don't have to patch my kernel 
to get PCMCIA working anymore :-)

//David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: IBM Thinkpad G41 PCMCIA problems

[David Härdeman]

On Sun, Feb 20, 2005 at 09:26:59AM +, Russell King wrote:
On Sun, Feb 20, 2005 at 10:22:09AM +0100, David Härdeman wrote:
I see the same problem with an IBM Thinkpad G40, and only when there is 
1Gb of memory or more in the machine.
Check to see if your e820 map has a hole in it, and whether any of
your Cardbus bridge memory / region 0 resources appear in it.
If your e820 map contains a hole, I'd suspect another buggy bios.
e820 map:
BIOS-provided physical RAM map:
BIOS-e820:  - 0009f000 (usable)
BIOS-e820: 0009f000 - 000a (reserved)
BIOS-e820: 000ce000 - 000d (reserved)
BIOS-e820: 000dc000 - 0010 (reserved)
BIOS-e820: 0010 - 0f6f (usable)
BIOS-e820: 0f6f - 0f70 (reserved)
BIOS-e820: 0f70 - 3f6f (usable)
BIOS-e820: 3f6f - 3f6f8000 (ACPI data)
BIOS-e820: 3f6f8000 - 3f6fa000 (ACPI NVS)
BIOS-e820: 3f70 - 4000 (reserved)
BIOS-e820: ff80 - 0001 (reserved)
118MB HIGHMEM available.
896MB LOWMEM available.
Is the hole between 0x36f6fa000 and 0x3f70?
And what would be the proper way of fixing it (assuming that IBM won't 
issue a fixed BIOS)?

Re,
David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: IBM Thinkpad G41 PCMCIA problems

[David Härdeman]

Steven Rostedt wrote:
I have a IBM Thinkpad G41 with a pentium4M with Hyperthreading.  I can't
get the PCMCIA working at all.  I've tried turning off hyperthreading,
I've tried with and without preempt, I've even added pci=noacpi. I've
added Len's ACPI patches, but nothing works.

I see the same problem with an IBM Thinkpad G40, and only when there is 
1Gb of memory or more in the machine.

The problem was reported the first time (to my knowledge), here:
http://www.ussg.iu.edu/hypermail/linux/kernel/0306.3/0956.html
by a Thinkpad T40 user.
So the problem seems to affect at least three different Thinkpad models.
The workaround I've seen so far have either been to disable 
pci_fixup_transparent_bridge (as mentioned in this thread) or to raise 
the value of pci_mem_start.

Re,
David
lspci -vvv with pci_fixup_transparent bridge disabled:
:00:1e.0 PCI bridge: Intel Corp. 82801 PCI Bridge (rev 81) (prog-if 00 
[Normal decode])
   Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR+ FastB2B-
   Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- SERR- Reset- FastB2B-
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: IBM Thinkpad G41 PCMCIA problems

[David Härdeman]

Steven Rostedt wrote:
I have a IBM Thinkpad G41 with a pentium4M with Hyperthreading.  I can't
get the PCMCIA working at all.  I've tried turning off hyperthreading,
I've tried with and without preempt, I've even added pci=noacpi. I've
added Len's ACPI patches, but nothing works.

I see the same problem with an IBM Thinkpad G40, and only when there is 
1Gb of memory or more in the machine.

The problem was reported the first time (to my knowledge), here:
http://www.ussg.iu.edu/hypermail/linux/kernel/0306.3/0956.html
by a Thinkpad T40 user.
So the problem seems to affect at least three different Thinkpad models.
The workaround I've seen so far have either been to disable 
pci_fixup_transparent_bridge (as mentioned in this thread) or to raise 
the value of pci_mem_start.

Re,
David
lspci -vvv with pci_fixup_transparent bridge disabled:
:00:1e.0 PCI bridge: Intel Corp. 82801 PCI Bridge (rev 81) (prog-if 00 
[Normal decode])
   Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR+ FastB2B-
   Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast TAbort- TAbort- 
MAbort- SERR- PERR+
   Latency: 0
   Bus: primary=00, secondary=02, subordinate=05, sec-latency=64
   I/O behind bridge: 3000-6fff
   Memory behind bridge: d020-dfff
   Prefetchable memory behind bridge: f000-f7ff
   BridgeCtl: Parity- SERR- NoISA+ VGA- MAbort- Reset- FastB2B-
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/