Re: Bug: getcwd: cannot access parent directories

[Roel Van de Paar]

Hi Casey,

On 28 September 2018 at 05:46, Casey Schaufler  wrote:

> Please try doing this under strace, in particular
>
> strace mv 3 /dev/shm
>
> What you will see is that the mv command does a copy/delete
> when changing filesystems. The original "2" directory, which
> is your current working directory, will have no parent as the
> original "1" will have been removed.
>
>>>
>>> Back to session 1
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # works, but to completely incorrect directory given the
>>> many 'cd ..'```
>>>
>>> The problem is made more clear here by using 3/4/5/6/7 but you can
>>> also just use a single subdir to see it
>> Roel Van de Paar, Technical Director - Quality Assurance, Percona
>
> The system is working properly.

Thank you for the analysis.

I can see your point, but it looks like this could be improved by
matching the behavior when not changing filesystems.

Also, there is a difference between working when it is a different
filesystem or not - so there is inconsistency in operation.

Re: Bug: getcwd: cannot access parent directories

[Roel Van de Paar]

Hi Casey,

On 28 September 2018 at 05:46, Casey Schaufler  wrote:

> Please try doing this under strace, in particular
>
> strace mv 3 /dev/shm
>
> What you will see is that the mv command does a copy/delete
> when changing filesystems. The original "2" directory, which
> is your current working directory, will have no parent as the
> original "1" will have been removed.
>
>>>
>>> Back to session 1
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # fail
>>>   cd ..   # works, but to completely incorrect directory given the
>>> many 'cd ..'```
>>>
>>> The problem is made more clear here by using 3/4/5/6/7 but you can
>>> also just use a single subdir to see it
>> Roel Van de Paar, Technical Director - Quality Assurance, Percona
>
> The system is working properly.

Thank you for the analysis.

I can see your point, but it looks like this could be improved by
matching the behavior when not changing filesystems.

Also, there is a difference between working when it is a different
filesystem or not - so there is inconsistency in operation.

Re: Bug: getcwd: cannot access parent directories

[Roel Van de Paar]

Ping :)

On 21 September 2018 at 18:16, Roel Van de Paar
 wrote:
>
> [1.] One line summary of the problem:
>
> "cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory" on move of directories to
> another volume
>
> [2.] Full description of the problem/report:
>
> Example output (testcase below):
>
> /tmp/1/2$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../../..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../../../..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../../../../..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../../../../../..$ cd ..
> /tmp$
>
> - The number of ".." required is not correct
> - The number of ".." matches the number of subdirs (see below)
>
> [4.] Kernel information
>
> Tested on latest updates of:
> * Ubuntu Xenial
> * Ubuntu Bionic
> * Centos 7
>
> [7.] A small shell script or example program which triggers the
> problem (if possible)
>
> In session 1
>   cd /tmp
>   mkdir -p 1/2
>   cd 1/2
>
> In session 2 (another terminal session)
>   cd /tmp
>   mkdir -p 3/4/5/6/7
>   mv 1 3/4/5/6/7
>   mv 3 /dev/shm   # where /dev/shm is some other disk. /dev/shm
> reproduction works too. If the disk is the same, the issue will not
> show.
>
> Back to session 1
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # works, but to completely incorrect directory given the
> many 'cd ..'```
>
> The problem is made more clear here by using 3/4/5/6/7 but you can
> also just use a single subdir to see it

Roel Van de Paar, Technical Director - Quality Assurance, Percona

Re: Bug: getcwd: cannot access parent directories

[Roel Van de Paar]

Ping :)

On 21 September 2018 at 18:16, Roel Van de Paar
 wrote:
>
> [1.] One line summary of the problem:
>
> "cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory" on move of directories to
> another volume
>
> [2.] Full description of the problem/report:
>
> Example output (testcase below):
>
> /tmp/1/2$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../../..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../../../..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../../../../..$ cd ..
> cd: error retrieving current directory: getcwd: cannot access parent
> directories: No such file or directory
> /tmp/1/2/../../../../../..$ cd ..
> /tmp$
>
> - The number of ".." required is not correct
> - The number of ".." matches the number of subdirs (see below)
>
> [4.] Kernel information
>
> Tested on latest updates of:
> * Ubuntu Xenial
> * Ubuntu Bionic
> * Centos 7
>
> [7.] A small shell script or example program which triggers the
> problem (if possible)
>
> In session 1
>   cd /tmp
>   mkdir -p 1/2
>   cd 1/2
>
> In session 2 (another terminal session)
>   cd /tmp
>   mkdir -p 3/4/5/6/7
>   mv 1 3/4/5/6/7
>   mv 3 /dev/shm   # where /dev/shm is some other disk. /dev/shm
> reproduction works too. If the disk is the same, the issue will not
> show.
>
> Back to session 1
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # fail
>   cd ..   # works, but to completely incorrect directory given the
> many 'cd ..'```
>
> The problem is made more clear here by using 3/4/5/6/7 but you can
> also just use a single subdir to see it

Roel Van de Paar, Technical Director - Quality Assurance, Percona

Bug: getcwd: cannot access parent directories

[Roel Van de Paar]

[1.] One line summary of the problem:

"cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory" on move of directories to
another volume

[2.] Full description of the problem/report:

Example output (testcase below):

/tmp/1/2$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../../..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../../../..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../../../../..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../../../../../..$ cd ..
/tmp$

- The number of ".." required is not correct
- The number of ".." matches the number of subdirs (see below)

[4.] Kernel information

Tested on latest updates of:
* Ubuntu Xenial
* Ubuntu Bionic
* Centos 7

[7.] A small shell script or example program which triggers the
problem (if possible)

In session 1
  cd /tmp
  mkdir -p 1/2
  cd 1/2

In session 2 (another terminal session)
  cd /tmp
  mkdir -p 3/4/5/6/7
  mv 1 3/4/5/6/7
  mv 3 /dev/shm   # where /dev/shm is some other disk. /dev/shm
reproduction works too. If the disk is the same, the issue will not
show.

Back to session 1
  cd ..   # fail
  cd ..   # fail
  cd ..   # fail
  cd ..   # fail
  cd ..   # fail
  cd ..   # fail
  cd ..   # works, but to completely incorrect directory given the
many 'cd ..'```

The problem is made more clear here by using 3/4/5/6/7 but you can
also just use a single subdir to see it

Bug: getcwd: cannot access parent directories

[Roel Van de Paar]

[1.] One line summary of the problem:

"cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory" on move of directories to
another volume

[2.] Full description of the problem/report:

Example output (testcase below):

/tmp/1/2$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../../..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../../../..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../../../../..$ cd ..
cd: error retrieving current directory: getcwd: cannot access parent
directories: No such file or directory
/tmp/1/2/../../../../../..$ cd ..
/tmp$

- The number of ".." required is not correct
- The number of ".." matches the number of subdirs (see below)

[4.] Kernel information

Tested on latest updates of:
* Ubuntu Xenial
* Ubuntu Bionic
* Centos 7

[7.] A small shell script or example program which triggers the
problem (if possible)

In session 1
  cd /tmp
  mkdir -p 1/2
  cd 1/2

In session 2 (another terminal session)
  cd /tmp
  mkdir -p 3/4/5/6/7
  mv 1 3/4/5/6/7
  mv 3 /dev/shm   # where /dev/shm is some other disk. /dev/shm
reproduction works too. If the disk is the same, the issue will not
show.

Back to session 1
  cd ..   # fail
  cd ..   # fail
  cd ..   # fail
  cd ..   # fail
  cd ..   # fail
  cd ..   # fail
  cd ..   # works, but to completely incorrect directory given the
many 'cd ..'```

The problem is made more clear here by using 3/4/5/6/7 but you can
also just use a single subdir to see it

[PATCH] unset twsi option3 for gconfig as well

[Roel Kluin]

Signed-off-by: Roel Kluin 
---
 drivers/pinctrl/mvebu/pinctrl-dove.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-dove.c 
b/drivers/pinctrl/mvebu/pinctrl-dove.c
index 29f7e4f..360b9b2 100644
--- a/drivers/pinctrl/mvebu/pinctrl-dove.c
+++ b/drivers/pinctrl/mvebu/pinctrl-dove.c
@@ -335,7 +335,7 @@ static int dove_twsi_ctrl_set(struct mvebu_mpp_ctrl *ctrl,
unsigned long gcfg2 = readl(DOVE_GLOBAL_CONFIG_2);
 
gcfg1 &= ~DOVE_TWSI_ENABLE_OPTION1;
-   gcfg2 &= ~(DOVE_TWSI_ENABLE_OPTION2 | DOVE_TWSI_ENABLE_OPTION2);
+   gcfg2 &= ~(DOVE_TWSI_ENABLE_OPTION2 | DOVE_TWSI_ENABLE_OPTION3);
 
switch (config) {
case 1:
-- 
1.8.1.2

--
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] set data enable logic level to low

[Roel Kluin]

Signed-off-by: Roel Kluin 
---
 drivers/video/omap2/dss/display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/omap2/dss/display.c 
b/drivers/video/omap2/dss/display.c
index fafe7c9..669a81f 100644
--- a/drivers/video/omap2/dss/display.c
+++ b/drivers/video/omap2/dss/display.c
@@ -266,7 +266,7 @@ void videomode_to_omap_video_timings(const struct videomode 
*vm,
OMAPDSS_SIG_ACTIVE_LOW;
ovt->de_level = vm->flags & DISPLAY_FLAGS_DE_HIGH ?
OMAPDSS_SIG_ACTIVE_HIGH :
-   OMAPDSS_SIG_ACTIVE_HIGH;
+   OMAPDSS_SIG_ACTIVE_LOW;
ovt->data_pclk_edge = vm->flags & DISPLAY_FLAGS_PIXDATA_POSEDGE ?
OMAPDSS_DRIVE_SIG_RISING_EDGE :
OMAPDSS_DRIVE_SIG_FALLING_EDGE;
-- 
1.8.1.2

--
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/

exynos4: index out of bounds if no pixelcode found

[Roel Kluin]

In case no valid pixelcode is found, an i of -1 after the loop is out of 
bounds for the array.

Signed-off-by: Roel Kluin 
---
diff --git a/drivers/media/platform/exynos4-is/fimc-lite-reg.c 
b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
index 72a343e3b..d0dc7ee 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite-reg.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
@@ -133,7 +133,7 @@ void flite_hw_set_source_format(struct fimc_lite *dev, 
struct flite_frame *f)
int i = ARRAY_SIZE(src_pixfmt_map);
u32 cfg;
 
-   while (--i >= 0) {
+   while (--i) {
if (src_pixfmt_map[i][0] == pixelcode)
break;
}
@@ -240,7 +240,7 @@ static void flite_hw_set_out_order(struct fimc_lite 
*dev, struct flite_frame *f)
u32 cfg = readl(dev->regs + FLITE_REG_CIODMAFMT);
int i = ARRAY_SIZE(pixcode);
 
-   while (--i >= 0)
+   while (--i)
if (pixcode[i][0] == f->fmt->mbus_code)
break;
cfg &= ~FLITE_REG_CIODMAFMT_YCBCR_ORDER_MASK;

--
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/

exynos4: index out of bounds if no pixelcode found

[Roel Kluin]

In case no valid pixelcode is found, an i of -1 after the loop is out of 
bounds for the array.

Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
diff --git a/drivers/media/platform/exynos4-is/fimc-lite-reg.c 
b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
index 72a343e3b..d0dc7ee 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite-reg.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite-reg.c
@@ -133,7 +133,7 @@ void flite_hw_set_source_format(struct fimc_lite *dev, 
struct flite_frame *f)
int i = ARRAY_SIZE(src_pixfmt_map);
u32 cfg;
 
-   while (--i = 0) {
+   while (--i) {
if (src_pixfmt_map[i][0] == pixelcode)
break;
}
@@ -240,7 +240,7 @@ static void flite_hw_set_out_order(struct fimc_lite 
*dev, struct flite_frame *f)
u32 cfg = readl(dev-regs + FLITE_REG_CIODMAFMT);
int i = ARRAY_SIZE(pixcode);
 
-   while (--i = 0)
+   while (--i)
if (pixcode[i][0] == f-fmt-mbus_code)
break;
cfg = ~FLITE_REG_CIODMAFMT_YCBCR_ORDER_MASK;

--
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] set data enable logic level to low

[Roel Kluin]

Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
 drivers/video/omap2/dss/display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/omap2/dss/display.c 
b/drivers/video/omap2/dss/display.c
index fafe7c9..669a81f 100644
--- a/drivers/video/omap2/dss/display.c
+++ b/drivers/video/omap2/dss/display.c
@@ -266,7 +266,7 @@ void videomode_to_omap_video_timings(const struct videomode 
*vm,
OMAPDSS_SIG_ACTIVE_LOW;
ovt-de_level = vm-flags  DISPLAY_FLAGS_DE_HIGH ?
OMAPDSS_SIG_ACTIVE_HIGH :
-   OMAPDSS_SIG_ACTIVE_HIGH;
+   OMAPDSS_SIG_ACTIVE_LOW;
ovt-data_pclk_edge = vm-flags  DISPLAY_FLAGS_PIXDATA_POSEDGE ?
OMAPDSS_DRIVE_SIG_RISING_EDGE :
OMAPDSS_DRIVE_SIG_FALLING_EDGE;
-- 
1.8.1.2

--
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] unset twsi option3 for gconfig as well

[Roel Kluin]

Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
 drivers/pinctrl/mvebu/pinctrl-dove.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-dove.c 
b/drivers/pinctrl/mvebu/pinctrl-dove.c
index 29f7e4f..360b9b2 100644
--- a/drivers/pinctrl/mvebu/pinctrl-dove.c
+++ b/drivers/pinctrl/mvebu/pinctrl-dove.c
@@ -335,7 +335,7 @@ static int dove_twsi_ctrl_set(struct mvebu_mpp_ctrl *ctrl,
unsigned long gcfg2 = readl(DOVE_GLOBAL_CONFIG_2);
 
gcfg1 = ~DOVE_TWSI_ENABLE_OPTION1;
-   gcfg2 = ~(DOVE_TWSI_ENABLE_OPTION2 | DOVE_TWSI_ENABLE_OPTION2);
+   gcfg2 = ~(DOVE_TWSI_ENABLE_OPTION2 | DOVE_TWSI_ENABLE_OPTION3);
 
switch (config) {
case 1:
-- 
1.8.1.2

--
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/

lockdep: testing '0' where '\0' intended?

[Roel Kluin]

Not entirely sure about the assembly part, but shouldn't it...

Test for the nul character rather than the '0' (== 0x30), in the
__get_user_unknown() case.

Signed-off-by: Roel Kluin 
---
diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
index b2c71c5..71b3aba 100644
--- a/kernel/lockdep_proc.c
+++ b/kernel/lockdep_proc.c
@@ -632,7 +632,7 @@ static ssize_t lock_stat_write(struct file *file, 
const char __user *buf,
if (get_user(c, buf))
return -EFAULT;
 
-   if (c != '0')
+   if (c != '\0')
return count;
 
list_for_each_entry(class, _lock_classes, lock_entry)

--
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] jump_label: unlikely(x) > 0

[Roel Kluin]

untested, but wasn't this intended instead?
--
if (unlikely(x) > 0) doesn't seem to help branch prediction

Signed-off-by: Roel Kluin 
---
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index a507907..cf08540 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -132,14 +132,14 @@ static __always_inline void jump_label_init(void)
 
 static __always_inline bool static_key_false(struct static_key *key)
 {
-   if (unlikely(atomic_read(>enabled)) > 0)
+   if (unlikely(atomic_read(>enabled) > 0))
return true;
return false;
 }
 
 static __always_inline bool static_key_true(struct static_key *key)
 {
-   if (likely(atomic_read(>enabled)) > 0)
+   if (likely(atomic_read(>enabled) > 0))
return true;
return false;
 }

--
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/

tty: incorrect test of echo_buf() result for ECHO_OP_START

[Roel Kluin]

Untested, but this looks like a bug to me
---
test echo_buf() result for ECHO_OP_START

Signed-off-by: Roel Kluin 
---
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 7a744b6..42b6cca 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -767,7 +767,7 @@ static size_t __process_echoes(struct tty_struct *tty)
 * of echo overrun before the next commit), then discard enough
 * data at the tail to prevent a subsequent overrun */
while (ldata->echo_commit - tail >= ECHO_DISCARD_WATERMARK) {
-   if (echo_buf(ldata, tail == ECHO_OP_START)) {
+   if (echo_buf(ldata, tail) == ECHO_OP_START) {
if (echo_buf(ldata, tail) == ECHO_OP_ERASE_TAB)
tail += 3;
else

--
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/

tty: incorrect test of echo_buf() result for ECHO_OP_START

[Roel Kluin]

Untested, but this looks like a bug to me
---
test echo_buf() result for ECHO_OP_START

Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 7a744b6..42b6cca 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -767,7 +767,7 @@ static size_t __process_echoes(struct tty_struct *tty)
 * of echo overrun before the next commit), then discard enough
 * data at the tail to prevent a subsequent overrun */
while (ldata-echo_commit - tail = ECHO_DISCARD_WATERMARK) {
-   if (echo_buf(ldata, tail == ECHO_OP_START)) {
+   if (echo_buf(ldata, tail) == ECHO_OP_START) {
if (echo_buf(ldata, tail) == ECHO_OP_ERASE_TAB)
tail += 3;
else

--
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] jump_label: unlikely(x) 0

[Roel Kluin]

untested, but wasn't this intended instead?
--
if (unlikely(x)  0) doesn't seem to help branch prediction

Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index a507907..cf08540 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -132,14 +132,14 @@ static __always_inline void jump_label_init(void)
 
 static __always_inline bool static_key_false(struct static_key *key)
 {
-   if (unlikely(atomic_read(key-enabled))  0)
+   if (unlikely(atomic_read(key-enabled)  0))
return true;
return false;
 }
 
 static __always_inline bool static_key_true(struct static_key *key)
 {
-   if (likely(atomic_read(key-enabled))  0)
+   if (likely(atomic_read(key-enabled)  0))
return true;
return false;
 }

--
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/

lockdep: testing '0' where '\0' intended?

[Roel Kluin]

Not entirely sure about the assembly part, but shouldn't it...

Test for the nul character rather than the '0' (== 0x30), in the
__get_user_unknown() case.

Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
diff --git a/kernel/lockdep_proc.c b/kernel/lockdep_proc.c
index b2c71c5..71b3aba 100644
--- a/kernel/lockdep_proc.c
+++ b/kernel/lockdep_proc.c
@@ -632,7 +632,7 @@ static ssize_t lock_stat_write(struct file *file, 
const char __user *buf,
if (get_user(c, buf))
return -EFAULT;
 
-   if (c != '0')
+   if (c != '\0')
return count;
 
list_for_each_entry(class, all_lock_classes, lock_entry)

--
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: Fix bug in end absolute address in drivers/mtd/devices/slram.c

[Roel Kluin]

On Thu, Sep 20, 2012 at 5:55 PM, Jiri Kosina  wrote:
> On Thu, 20 Sep 2012, Jiri Engelthaler wrote:
>
>> Hello.
>>   I found a bug in parsing end absolute address in
>> drivers/mtd/devices/slram.c. There is checking for startaddress <
>> endaddress and endaddress subtracted by startaddress before check. The
>> bug is there from commit 3afd522de8d8ec446efe957b86e4f63e3dd8ce9d Mon,
>> 19 Jan 2009 01:24:21 +
>>
>> Here is the patch
>>
>> Signed-off-by: Jiri Engelthaler 
>>
>> --- linux-3.2.28/drivers/mtd/devices/slram.c  2012-08-19 19:15:38.0 
>> +0200
>> +++ linux-3.2.28.mod/drivers/mtd/devices/slram.c  2012-09-19
>> 18:29:28.012740703 +0200
>> @@ -266,7 +266,7 @@ static int parse_cmdline(char *devname,
>>
>>   if (*(szlength) != '+') {
>>   devlength = simple_strtoul(szlength, , 0);
>> - devlength = handle_unit(devlength, buffer) - devstart;
>> + devlength = handle_unit(devlength, buffer);
>>   if (devlength < devstart)
>>   goto err_out;
>
> ... adding the suspects to CC so that they are aware of this.

Yes, that's what it should have been. FWIW,

Acked-by: Roel Kluin 
--
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: Fix bug in end absolute address in drivers/mtd/devices/slram.c

[Roel Kluin]

On Thu, Sep 20, 2012 at 5:55 PM, Jiri Kosina jkos...@suse.cz wrote:
 On Thu, 20 Sep 2012, Jiri Engelthaler wrote:

 Hello.
   I found a bug in parsing end absolute address in
 drivers/mtd/devices/slram.c. There is checking for startaddress 
 endaddress and endaddress subtracted by startaddress before check. The
 bug is there from commit 3afd522de8d8ec446efe957b86e4f63e3dd8ce9d Mon,
 19 Jan 2009 01:24:21 +

 Here is the patch

 Signed-off-by: Jiri Engelthaler eng...@gmail.com

 --- linux-3.2.28/drivers/mtd/devices/slram.c  2012-08-19 19:15:38.0 
 +0200
 +++ linux-3.2.28.mod/drivers/mtd/devices/slram.c  2012-09-19
 18:29:28.012740703 +0200
 @@ -266,7 +266,7 @@ static int parse_cmdline(char *devname,

   if (*(szlength) != '+') {
   devlength = simple_strtoul(szlength, buffer, 0);
 - devlength = handle_unit(devlength, buffer) - devstart;
 + devlength = handle_unit(devlength, buffer);
   if (devlength  devstart)
   goto err_out;

 ... adding the suspects to CC so that they are aware of this.

Yes, that's what it should have been. FWIW,

Acked-by: Roel Kluin roel.kl...@gmail.com
--
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] fs/ext4/mballoc.c: Convert to list_for_each_entry_rcu()

[Roel Kluin]

Aneesh Kumar K.V wrote:
> On Tue, Feb 19, 2008 at 01:31:18AM +0100, Roel Kluin wrote:
>> Please verify, this patch was not yet tested
>> ---
>> Convert list_for_each_rcu() to list_for_each_entry_rcu()
>>
>> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
> 
> 
> NACK. This patch doesn't build. You have extra cur in the conversion.
> Right changes attached. 
> 
> ext4:  Convert list_for_each_rcu() to list_for_each_entry_rcu()
> 
> From: Aneesh Kumar K.V <[EMAIL PROTECTED]>
> 
> The list_for_each_entry_rcu() primitive should be used instead of
> list_for_each_rcu(), as the former is easier to use and provides
> better type safety.


Thanks for your review and correction,

Roel
--
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] wireless: Convert to list_for_each_entry_rcu()

[Roel Kluin]

Roel Kluin wrote:
> Please verify, this patch was not yet tested.
> ---
> Convert list_for_each_rcu() to list_for_each_entry_rcu()
> 
> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
> ---
Same mistake as in other patch, please ignore the previous patch
and consider the patch below.
---
Convert list_for_each_rcu() to list_for_each_entry_rcu()

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/net/wireless/strip.c b/drivers/net/wireless/strip.c
index 88efe1b..bced3fe 100644
--- a/drivers/net/wireless/strip.c
+++ b/drivers/net/wireless/strip.c
@@ -962,12 +962,12 @@ static char *time_delta(char buffer[], long time)
 /* get Nth element of the linked list */
 static struct strip *strip_get_idx(loff_t pos) 
 {
-   struct list_head *l;
+   struct strip *str;
int i = 0;
 
-   list_for_each_rcu(l, _list) {
+   list_for_each_entry_rcu(str, _list, list) {
if (pos == i)
-   return list_entry(l, struct strip, list);
+   return str;
++i;
}
return NULL;
--
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] wireless: Convert to list_for_each_entry_rcu()

[Roel Kluin]

Roel Kluin wrote:
 Please verify, this patch was not yet tested.
 ---
 Convert list_for_each_rcu() to list_for_each_entry_rcu()
 
 Signed-off-by: Roel Kluin [EMAIL PROTECTED]
 ---
Same mistake as in other patch, please ignore the previous patch
and consider the patch below.
---
Convert list_for_each_rcu() to list_for_each_entry_rcu()

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/net/wireless/strip.c b/drivers/net/wireless/strip.c
index 88efe1b..bced3fe 100644
--- a/drivers/net/wireless/strip.c
+++ b/drivers/net/wireless/strip.c
@@ -962,12 +962,12 @@ static char *time_delta(char buffer[], long time)
 /* get Nth element of the linked list */
 static struct strip *strip_get_idx(loff_t pos) 
 {
-   struct list_head *l;
+   struct strip *str;
int i = 0;
 
-   list_for_each_rcu(l, strip_list) {
+   list_for_each_entry_rcu(str, strip_list, list) {
if (pos == i)
-   return list_entry(l, struct strip, list);
+   return str;
++i;
}
return NULL;
--
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] fs/ext4/mballoc.c: Convert to list_for_each_entry_rcu()

[Roel Kluin]

Aneesh Kumar K.V wrote:
 On Tue, Feb 19, 2008 at 01:31:18AM +0100, Roel Kluin wrote:
 Please verify, this patch was not yet tested
 ---
 Convert list_for_each_rcu() to list_for_each_entry_rcu()

 Signed-off-by: Roel Kluin [EMAIL PROTECTED]
 
 
 NACK. This patch doesn't build. You have extra cur in the conversion.
 Right changes attached. 
 
 ext4:  Convert list_for_each_rcu() to list_for_each_entry_rcu()
 
 From: Aneesh Kumar K.V [EMAIL PROTECTED]
 
 The list_for_each_entry_rcu() primitive should be used instead of
 list_for_each_rcu(), as the former is easier to use and provides
 better type safety.


Thanks for your review and correction,

Roel
--
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/

[PATCH] wireless: Convert to list_for_each_entry_rcu()

[Roel Kluin]

Please verify, this patch was not yet tested.
---
Convert list_for_each_rcu() to list_for_each_entry_rcu()

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/net/wireless/strip.c b/drivers/net/wireless/strip.c
index 88efe1b..c5aaab8 100644
--- a/drivers/net/wireless/strip.c
+++ b/drivers/net/wireless/strip.c
@@ -963,11 +963,12 @@ static char *time_delta(char buffer[], long time)
 static struct strip *strip_get_idx(loff_t pos) 
 {
struct list_head *l;
+   struct strip *str;
int i = 0;
 
-   list_for_each_rcu(l, _list) {
+   list_for_each_entry_rcu(str, l, _list, list) {
if (pos == i)
-   return list_entry(l, struct strip, list);
+   return str;
++i;
}
return NULL;
--
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/

[PATCH] fs/ext4/mballoc.c: Convert to list_for_each_entry_rcu()

[Roel Kluin]

Please verify, this patch was not yet tested
---
Convert list_for_each_rcu() to list_for_each_entry_rcu()

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index dd0fcfc..0c4dd13 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3127,6 +3127,7 @@ static void ext4_mb_normalize_request(struct 
ext4_allocation_context *ac,
loff_t size, orig_size, start_off;
ext4_lblk_t start, orig_start;
struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
+   struct ext4_prealloc_space *pa;
 
/* do normalize only data requests, metadata requests
   do not need preallocation */
@@ -3212,12 +3213,9 @@ static void ext4_mb_normalize_request(struct 
ext4_allocation_context *ac,
 
/* check we don't cross already preallocated blocks */
rcu_read_lock();
-   list_for_each_rcu(cur, >i_prealloc_list) {
-   struct ext4_prealloc_space *pa;
+   list_for_each_entry_rcu(pa, cur, >i_prealloc_list, pa_inode_list) {
unsigned long pa_end;
 
-   pa = list_entry(cur, struct ext4_prealloc_space, pa_inode_list);
-
if (pa->pa_deleted)
continue;
spin_lock(>pa_lock);
@@ -3259,10 +3257,8 @@ static void ext4_mb_normalize_request(struct 
ext4_allocation_context *ac,
 
/* XXX: extra loop to check we really don't overlap preallocations */
rcu_read_lock();
-   list_for_each_rcu(cur, >i_prealloc_list) {
-   struct ext4_prealloc_space *pa;
+   list_for_each_entry_rcu(pa, cur, >i_prealloc_list, pa_inode_list) {
unsigned long pa_end;
-   pa = list_entry(cur, struct ext4_prealloc_space, pa_inode_list);
spin_lock(>pa_lock);
if (pa->pa_deleted == 0) {
pa_end = pa->pa_lstart + pa->pa_len;
@@ -3397,8 +3393,7 @@ static int ext4_mb_use_preallocated(struct 
ext4_allocation_context *ac)
 
/* first, try per-file preallocation */
rcu_read_lock();
-   list_for_each_rcu(cur, >i_prealloc_list) {
-   pa = list_entry(cur, struct ext4_prealloc_space, pa_inode_list);
+   list_for_each_entry_rcu(pa, cur, >i_prealloc_list, pa_inode_list) {
 
/* all fields in this condition don't change,
 * so we can skip locking for them */
@@ -3430,8 +3425,7 @@ static int ext4_mb_use_preallocated(struct 
ext4_allocation_context *ac)
return 0;
 
rcu_read_lock();
-   list_for_each_rcu(cur, >lg_prealloc_list) {
-   pa = list_entry(cur, struct ext4_prealloc_space, pa_inode_list);
+   list_for_each_entry_rcu(pa, cur, >lg_prealloc_list, pa_inode_list) {
spin_lock(>pa_lock);
if (pa->pa_deleted == 0 && pa->pa_free >= ac->ac_o_ex.fe_len) {
atomic_inc(>pa_count);
--
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] ufs: [bl]e*_add_cpu conversion

[Roel Kluin]

Andrew Morton wrote:
> On Wed, 13 Feb 2008 10:41:44 +0100 Roel Kluin <[EMAIL PROTECTED]> wrote:
> 
>> you may also want these:
>> ---
>> [bl]e_add_cpu conversion in return

> upsets powerpc (at least):
> 
> fs/ufs/swab.h: In function `fs64_add':
> fs/ufs/swab.h:47: warning: passing arg 1 of `le64_add_cpu' from incompatible 
> pointer type
> fs/ufs/swab.h:49: warning: passing arg 1 of `be64_add_cpu' from incompatible 
> pointer type
> fs/ufs/swab.h: In function `fs64_sub':
> fs/ufs/swab.h:58: warning: passing arg 1 of `le64_add_cpu' from incompatible 
> pointer type
> fs/ufs/swab.h:60: warning: passing arg 1 of `be64_add_cpu' from incompatible 
> pointer type

sorry for this. Is it correct to cast like the patch below does?
If not (anyone) feel free to correct and send a patch yourself.
The patch below was *not* tested
---
[bl]e_add_cpu conversion in return (with cast)

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/fs/ufs/swab.h b/fs/ufs/swab.h
index 1683d2b..594d6e8 100644
--- a/fs/ufs/swab.h
+++ b/fs/ufs/swab.h
@@ -44,18 +44,22 @@ static __inline u32
 fs64_add(struct super_block *sbp, u32 *n, int d)
 {
if (UFS_SB(sbp)->s_bytesex == BYTESEX_LE)
-   return *n = cpu_to_le64(le64_to_cpu(*n)+d);
+   le64_add_cpu((__le64 *)n, d);
else
-   return *n = cpu_to_be64(be64_to_cpu(*n)+d);
+   be64_add_cpu((__le64 *)n, d);
+
+   return *n;
 }
 
 static __inline u32
 fs64_sub(struct super_block *sbp, u32 *n, int d)
 {
if (UFS_SB(sbp)->s_bytesex == BYTESEX_LE)
-   return *n = cpu_to_le64(le64_to_cpu(*n)-d);
+   le64_add_cpu((__le64 *)n, -d);
else
-   return *n = cpu_to_be64(be64_to_cpu(*n)-d);
+   be64_add_cpu((__le64 *)n, -d);
+
+   return *n;
 }
 
 static __inline u32
--
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 1/3] Fix Unlikely(x) == y

[Roel Kluin]

David Howells wrote:
> Geert Uytterhoeven <[EMAIL PROTECTED]> wrote:
> 
>> Hence shouldn't we ask the gcc people what's the purpose of
>> __builtin_expect(), if it doesn't live up to its promise?
> 
> __builtin_expect() is useful on FRV where you _have_ to give each branch and
> conditional branch instruction a measure of probability whether the branch
> will be taken.
> 
> David

I was wondering whether some of the uses of likely illustrated below are
incorrect or not useful.

x = likely(X) || Y

for ( ... ; likely(...); ... )

while ( likely(X) )

if ( unlikely(X) &&/|| likely(Y) )

if ( X &&/|| unlikely(Y) ) 

return likely(X);

return likely(X) ? a : b;
--
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/

[PATCH] arch/sh/drivers/heartbeat.c ioremap is expected to succeed

[Roel Kluin]

!unlikely(hd->base) is equivalent to likely(!hd->base) (for instance see
comments with commit fd312561adcc90e924f35d3032d5493aeb4c3017), I think
the ioremap is expected to succeed? please confirm that's right.
The patch below was *not* tested.
---
ioremap is expected to succeed

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/sh/drivers/heartbeat.c b/arch/sh/drivers/heartbeat.c
index b76a14f..ab77b0e 100644
--- a/arch/sh/drivers/heartbeat.c
+++ b/arch/sh/drivers/heartbeat.c
@@ -93,7 +93,7 @@ static int heartbeat_drv_probe(struct platform_device *pdev)
}
 
hd->base = ioremap_nocache(res->start, res->end - res->start + 1);
-   if (!unlikely(hd->base)) {
+   if (unlikely(!hd->base)) {
dev_err(>dev, "ioremap failed\n");
 
if (!pdev->dev.platform_data)
--
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/

[PATCH] arch/sh/drivers/heartbeat.c ioremap is expected to succeed

[Roel Kluin]

!unlikely(hd-base) is equivalent to likely(!hd-base) (for instance see
comments with commit fd312561adcc90e924f35d3032d5493aeb4c3017), I think
the ioremap is expected to succeed? please confirm that's right.
The patch below was *not* tested.
---
ioremap is expected to succeed

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/arch/sh/drivers/heartbeat.c b/arch/sh/drivers/heartbeat.c
index b76a14f..ab77b0e 100644
--- a/arch/sh/drivers/heartbeat.c
+++ b/arch/sh/drivers/heartbeat.c
@@ -93,7 +93,7 @@ static int heartbeat_drv_probe(struct platform_device *pdev)
}
 
hd-base = ioremap_nocache(res-start, res-end - res-start + 1);
-   if (!unlikely(hd-base)) {
+   if (unlikely(!hd-base)) {
dev_err(pdev-dev, ioremap failed\n);
 
if (!pdev-dev.platform_data)
--
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 1/3] Fix Unlikely(x) == y

[Roel Kluin]

David Howells wrote:
 Geert Uytterhoeven [EMAIL PROTECTED] wrote:
 
 Hence shouldn't we ask the gcc people what's the purpose of
 __builtin_expect(), if it doesn't live up to its promise?
 
 __builtin_expect() is useful on FRV where you _have_ to give each branch and
 conditional branch instruction a measure of probability whether the branch
 will be taken.
 
 David

I was wondering whether some of the uses of likely illustrated below are
incorrect or not useful.

x = likely(X) || Y

for ( ... ; likely(...); ... )

while ( likely(X) )

if ( unlikely(X) /|| likely(Y) )

if ( X /|| unlikely(Y) ) 

return likely(X);

return likely(X) ? a : b;
--
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/

[PATCH] wireless: Convert to list_for_each_entry_rcu()

[Roel Kluin]

Please verify, this patch was not yet tested.
---
Convert list_for_each_rcu() to list_for_each_entry_rcu()

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/net/wireless/strip.c b/drivers/net/wireless/strip.c
index 88efe1b..c5aaab8 100644
--- a/drivers/net/wireless/strip.c
+++ b/drivers/net/wireless/strip.c
@@ -963,11 +963,12 @@ static char *time_delta(char buffer[], long time)
 static struct strip *strip_get_idx(loff_t pos) 
 {
struct list_head *l;
+   struct strip *str;
int i = 0;
 
-   list_for_each_rcu(l, strip_list) {
+   list_for_each_entry_rcu(str, l, strip_list, list) {
if (pos == i)
-   return list_entry(l, struct strip, list);
+   return str;
++i;
}
return NULL;
--
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/

[PATCH] fs/ext4/mballoc.c: Convert to list_for_each_entry_rcu()

[Roel Kluin]

Please verify, this patch was not yet tested
---
Convert list_for_each_rcu() to list_for_each_entry_rcu()

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index dd0fcfc..0c4dd13 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -3127,6 +3127,7 @@ static void ext4_mb_normalize_request(struct 
ext4_allocation_context *ac,
loff_t size, orig_size, start_off;
ext4_lblk_t start, orig_start;
struct ext4_inode_info *ei = EXT4_I(ac-ac_inode);
+   struct ext4_prealloc_space *pa;
 
/* do normalize only data requests, metadata requests
   do not need preallocation */
@@ -3212,12 +3213,9 @@ static void ext4_mb_normalize_request(struct 
ext4_allocation_context *ac,
 
/* check we don't cross already preallocated blocks */
rcu_read_lock();
-   list_for_each_rcu(cur, ei-i_prealloc_list) {
-   struct ext4_prealloc_space *pa;
+   list_for_each_entry_rcu(pa, cur, ei-i_prealloc_list, pa_inode_list) {
unsigned long pa_end;
 
-   pa = list_entry(cur, struct ext4_prealloc_space, pa_inode_list);
-
if (pa-pa_deleted)
continue;
spin_lock(pa-pa_lock);
@@ -3259,10 +3257,8 @@ static void ext4_mb_normalize_request(struct 
ext4_allocation_context *ac,
 
/* XXX: extra loop to check we really don't overlap preallocations */
rcu_read_lock();
-   list_for_each_rcu(cur, ei-i_prealloc_list) {
-   struct ext4_prealloc_space *pa;
+   list_for_each_entry_rcu(pa, cur, ei-i_prealloc_list, pa_inode_list) {
unsigned long pa_end;
-   pa = list_entry(cur, struct ext4_prealloc_space, pa_inode_list);
spin_lock(pa-pa_lock);
if (pa-pa_deleted == 0) {
pa_end = pa-pa_lstart + pa-pa_len;
@@ -3397,8 +3393,7 @@ static int ext4_mb_use_preallocated(struct 
ext4_allocation_context *ac)
 
/* first, try per-file preallocation */
rcu_read_lock();
-   list_for_each_rcu(cur, ei-i_prealloc_list) {
-   pa = list_entry(cur, struct ext4_prealloc_space, pa_inode_list);
+   list_for_each_entry_rcu(pa, cur, ei-i_prealloc_list, pa_inode_list) {
 
/* all fields in this condition don't change,
 * so we can skip locking for them */
@@ -3430,8 +3425,7 @@ static int ext4_mb_use_preallocated(struct 
ext4_allocation_context *ac)
return 0;
 
rcu_read_lock();
-   list_for_each_rcu(cur, lg-lg_prealloc_list) {
-   pa = list_entry(cur, struct ext4_prealloc_space, pa_inode_list);
+   list_for_each_entry_rcu(pa, cur, lg-lg_prealloc_list, pa_inode_list) {
spin_lock(pa-pa_lock);
if (pa-pa_deleted == 0  pa-pa_free = ac-ac_o_ex.fe_len) {
atomic_inc(pa-pa_count);
--
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 1/2] [x86] arch/x86/kernel/signal_32.c: replace !likely(x) by likely(!x)

[Roel Kluin]

Roel Kluin wrote:
> Roel Kluin wrote:
>> Replace !likely(x) by likely(!x)

> sorry, please ignore this patch

The patch below shouldn't change semantics.
---
replace !likely(x) by unlikely(!x)

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
index caee1f0..c0fb075 100644
--- a/arch/x86/kernel/signal_32.c
+++ b/arch/x86/kernel/signal_32.c
@@ -303,7 +303,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs * regs, 
size_t frame_size)
 * If we are on the alternate signal stack and would overflow it, don't.
 * Return an always-bogus address instead so we will die with SIGSEGV.
 */
-   if (on_sig_stack(sp) && !likely(on_sig_stack(sp - frame_size)))
+   if (on_sig_stack(sp) && unlikely(!on_sig_stack(sp - frame_size)))
return (void __user *) -1L;
 
/* This is the X/Open sanctioned signal stack switching.  */
--
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 2/2] kernel/{exit.c, signal.c, power/process.c}: replace !likely(x) by likely(!x)

[Roel Kluin]

H. Peter Anvin wrote:
> Roel Kluin wrote:
>> H. Peter Anvin wrote:
>>> Roel Kluin wrote:
>>>> Not entirely sure who to send this to
>>>> ---
>>>> Replace !likely(x) by likely(!x)
>>> Whoa...
>>>
>>> Are you sure this is correct?
>>>
>>> !likely(x) is equivalent to unlikely(!x), not the opposite, so this is a
>>> functional change...
>>>
>> You are right, sorry, Need more caffeine. I will send the right patch
>> later.
> 
> Note I didn't say it was wrong, I just wanted a bit more explanation
> about why it's right, if it is.

No, I just wanted to move the '!' within the parentheses, I didn't want
to change semantics. below is how I guess the patch should have been. 
Thanks for your response.
---
Replace !likely(x) by unlikely(!x)

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/kernel/signal.c b/kernel/signal.c
index 84917fe..cf5d45a 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1308,7 +1308,7 @@ int send_sigqueue(int sig, struct sigqueue *q, struct 
task_struct *p)
 */
rcu_read_lock();
 
-   if (!likely(lock_task_sighand(p, ))) {
+   if (unlikely(!lock_task_sighand(p, ))) {
ret = -1;
goto out_err;
}
@@ -1548,7 +1548,7 @@ static void do_notify_parent_cldstop(struct task_struct 
*tsk, int why)
 
 static inline int may_ptrace_stop(void)
 {
-   if (!likely(current->ptrace & PT_PTRACED))
+   if (unlikely(!(current->ptrace & PT_PTRACED)))
return 0;
/*
 * Are we in the middle of do_coredump?
@@ -1574,7 +1574,7 @@ static int sigkill_pending(struct task_struct *tsk)
 {
return ((sigismember(>pending.signal, SIGKILL) ||
 sigismember(>signal->shared_pending.signal, SIGKILL)) &&
-   !unlikely(sigismember(>blocked, SIGKILL)));
+likely(!sigismember(>blocked, SIGKILL)));
 }
 
 /*
@@ -1625,7 +1625,7 @@ static void ptrace_stop(int exit_code, int clear_code, 
siginfo_t *info)
spin_unlock_irq(>sighand->siglock);
try_to_freeze();
read_lock(_lock);
-   if (!unlikely(killed) && may_ptrace_stop()) {
+   if (likely(!killed) && may_ptrace_stop()) {
do_notify_parent_cldstop(current, CLD_TRAPPED);
read_unlock(_lock);
schedule();
@@ -1717,8 +1717,8 @@ static int do_signal_stop(int signr)
} else {
struct task_struct *t;
 
-   if (!likely(sig->flags & SIGNAL_STOP_DEQUEUED) ||
-   unlikely(sig->group_exit_task))
+   if (unlikely(!(sig->flags & SIGNAL_STOP_DEQUEUED) ||
+   sig->group_exit_task))
return 0;
/*
 * There is no group stop already in progress.

--
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/

[PATCH] fs/ufs/util.h 2nd parameter of fs32_to_cpu is not boolean

[Roel Kluin]

from: fs/befs/endian.h +33
static inline u32
fs32_to_cpu(const struct super_block *sb, fs32 n)
{
if (BEFS_SB(sb)->byte_order == BEFS_BYTESEX_LE)
return le32_to_cpu((__force __le32)n);
else
return be32_to_cpu((__force __be32)n);
}

The 2nd parameter is not boolean
---
Test the return value, rather than passing a boolean

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/fs/ufs/util.h b/fs/ufs/util.h
index b26fc4d..23ceed8 100644
--- a/fs/ufs/util.h
+++ b/fs/ufs/util.h
@@ -58,7 +58,7 @@ ufs_set_fs_state(struct super_block *sb, struct 
ufs_super_block_first *usb1,
 {
switch (UFS_SB(sb)->s_flags & UFS_ST_MASK) {
case UFS_ST_SUNOS:
-   if (fs32_to_cpu(sb, usb3->fs_postblformat == UFS_42POSTBLFMT)) {
+   if (fs32_to_cpu(sb, usb3->fs_postblformat) == UFS_42POSTBLFMT) {
usb1->fs_u0.fs_sun.fs_state = cpu_to_fs32(sb, value);
break;
}
--
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 1/2] [x86] arch/x86/kernel/signal_32.c: replace !likely(x) by likely(!x)

[Roel Kluin]

Roel Kluin wrote:
> Replace !likely(x) by likely(!x)
> 
> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
> ---
> diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
> index caee1f0..335872f 100644
> --- a/arch/x86/kernel/signal_32.c
> +++ b/arch/x86/kernel/signal_32.c
> @@ -303,7 +303,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs * 
> regs, size_t frame_size)
>* If we are on the alternate signal stack and would overflow it, don't.
>* Return an always-bogus address instead so we will die with SIGSEGV.
>*/
> - if (on_sig_stack(sp) && !likely(on_sig_stack(sp - frame_size)))
> + if (on_sig_stack(sp) && likely(!on_sig_stack(sp - frame_size)))
>   return (void __user *) -1L;
>  
>   /* This is the X/Open sanctioned signal stack switching.  */
> --
sorry, please ignore this patch
--
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 2/2] kernel/{exit.c, signal.c, power/process.c}: replace !likely(x) by likely(!x)

[Roel Kluin]

H. Peter Anvin wrote:
> Roel Kluin wrote:
>> Not entirely sure who to send this to
>> ---
>> Replace !likely(x) by likely(!x)
> 
> Whoa...
> 
> Are you sure this is correct?
> 
> !likely(x) is equivalent to unlikely(!x), not the opposite, so this is a
> functional change...
> 
> -hpa
> 
You are right, sorry, Need more caffeine. I will send the right patch later.
--
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/

[PATCH] drivers/mtd/onenand/onenand_base.c unlikely(x) || unlikely(y) => unlikely(x || y)

[Roel Kluin]

Not yet tested.
---
Replace unlikely(x) || unlikely(y) by unlikely(x || y)

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/mtd/onenand/onenand_base.c 
b/drivers/mtd/onenand/onenand_base.c
index 8d7d21b..7334b4a 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1336,7 +1336,7 @@ static int onenand_panic_write(struct mtd_info *mtd, 
loff_t to, size_t len,
}
 
/* Reject writes, which are not page aligned */
-if (unlikely(NOTALIGNED(to)) || unlikely(NOTALIGNED(len))) {
+if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
 printk(KERN_ERR "onenand_panic_write: Attempt to write not 
page aligned data\n");
 return -EINVAL;
 }
@@ -1466,7 +1466,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, 
loff_t to,
}
 
/* Reject writes, which are not page aligned */
-if (unlikely(NOTALIGNED(to)) || unlikely(NOTALIGNED(len))) {
+if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
 printk(KERN_ERR "onenand_write_ops_nolock: Attempt to write 
not page aligned data\n");
 return -EINVAL;
 }
--
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/

[PATCH] kernel/sched.c unlikely(x) || unlikely(y) => unlikely(x || y)

[Roel Kluin]

Not yet tested.
---
Replace unlikely(x) || unlikely(y) by unlikely(x || y)

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/kernel/sched.c b/kernel/sched.c
index f28f19e..816c299 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -137,7 +137,7 @@ static inline void sg_inc_cpu_power(struct sched_group *sg, 
u32 val)
 
 static inline int rt_policy(int policy)
 {
-   if (unlikely(policy == SCHED_FIFO) || unlikely(policy == SCHED_RR))
+   if (unlikely(policy == SCHED_FIFO || policy == SCHED_RR))
return 1;
return 0;
 }
@@ -3832,7 +3832,7 @@ static inline void schedule_debug(struct task_struct 
*prev)
 * schedule() atomically, we ignore that path for now.
 * Otherwise, whine if we are scheduling when we should not be.
 */
-   if (unlikely(in_atomic_preempt_off()) && unlikely(!prev->exit_state))
+   if (unlikely(in_atomic_preempt_off() && !prev->exit_state))
__schedule_bug(prev);
 
profile_hit(SCHED_PROFILING, __builtin_return_address(0));
--
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/

[PATCH 3/3] drivers/s390/block/dcssblk.c: Fix Unlikely(x) != y

[Roel Kluin]

The patch below was not yet tested. If it's correct as it is, please comment.
---
Fix Unlikely(x) != y

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index 3faf053..e6c94db 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -666,7 +666,7 @@ dcssblk_make_request(struct request_queue *q, struct bio 
*bio)
page_addr = (unsigned long)
page_address(bvec->bv_page) + bvec->bv_offset;
source_addr = dev_info->start + (index<<12) + bytes_done;
-   if (unlikely(page_addr & 4095) != 0 || (bvec->bv_len & 4095) != 
0)
+   if (unlikely((page_addr & 4095) != 0) || (bvec->bv_len & 4095) 
!= 0)
// More paranoia.
goto fail;
if (bio_data_dir(bio) == READ) {
--
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/

[PATCH 2/3] drivers/media/video/sn9c102/sn9c102_core.c Fix Unlikely(x) == y

[Roel Kluin]

The patch below was not yet tested. If it's incorrect, please comment.
---
Fix Unlikely(x) == y

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/media/video/sn9c102/sn9c102_core.c 
b/drivers/media/video/sn9c102/sn9c102_core.c
index c40ba3a..66313b1 100644
--- a/drivers/media/video/sn9c102/sn9c102_core.c
+++ b/drivers/media/video/sn9c102/sn9c102_core.c
@@ -528,7 +528,7 @@ sn9c102_find_sof_header(struct sn9c102_device* cam, void* 
mem, size_t len)
 
/* Search for the SOF marker (fixed part) in the header */
for (j = 0, b=cam->sof.bytesread; j+b < sizeof(marker); j++) {
-   if (unlikely(i+j) == len)
+   if (unlikely(i+j == len))
return NULL;
if (*(m+i+j) == marker[cam->sof.bytesread]) {
cam->sof.header[cam->sof.bytesread] = *(m+i+j);
--
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/

[PATCH 1/3] Fix Unlikely(x) == y

[Roel Kluin]

The patch below was not yet tested. If it's correct as it is, please comment.
---
Fix Unlikely(x) == y

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/platforms/ps3/interrupt.c 
b/arch/powerpc/platforms/ps3/interrupt.c
index 3a6db04..a14e5cd 100644
--- a/arch/powerpc/platforms/ps3/interrupt.c
+++ b/arch/powerpc/platforms/ps3/interrupt.c
@@ -709,7 +709,7 @@ static unsigned int ps3_get_irq(void)
asm volatile("cntlzd %0,%1" : "=r" (plug) : "r" (x));
plug &= 0x3f;
 
-   if (unlikely(plug) == NO_IRQ) {
+   if (unlikely(plug == NO_IRQ)) {
pr_debug("%s:%d: no plug found: thread_id %lu\n", __func__,
__LINE__, pd->thread_id);
dump_bmp(_cpu(ps3_private, 0));
--
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/

[PATCH 2/2] kernel/{exit.c, signal.c, power/process.c}: replace !likely(x) by likely(!x)

[Roel Kluin]

Not entirely sure who to send this to
---
Replace !likely(x) by likely(!x)

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/kernel/exit.c b/kernel/exit.c
index 506a957..df207fc 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1497,7 +1497,7 @@ repeat:
/*
 * We don't reap group leaders with subthreads.
 */
-   if (!likely(options & WEXITED))
+   if (likely(!(options & WEXITED)))
continue;
retval = wait_task_zombie(p,
(options & WNOWAIT), infop,
@@ -1508,7 +1508,7 @@ repeat:
 * exit, stop, or stop and then continue.
 */
flag = 1;
-   if (!unlikely(options & WCONTINUED))
+   if (unlikely(!(options & WCONTINUED)))
continue;
retval = wait_task_continued(p,
(options & WNOWAIT), infop,
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 7c2118f..b42254b 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -36,7 +36,7 @@ static inline int freezeable(struct task_struct * p)
  */
 static inline void frozen_process(void)
 {
-   if (!unlikely(current->flags & PF_NOFREEZE)) {
+   if (unlikely(!(current->flags & PF_NOFREEZE))) {
current->flags |= PF_FROZEN;
wmb();
}
diff --git a/kernel/signal.c b/kernel/signal.c
index 84917fe..1155e16 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1308,7 +1308,7 @@ int send_sigqueue(int sig, struct sigqueue *q, struct 
task_struct *p)
 */
rcu_read_lock();
 
-   if (!likely(lock_task_sighand(p, ))) {
+   if (likely(!lock_task_sighand(p, ))) {
ret = -1;
goto out_err;
}
@@ -1548,7 +1548,7 @@ static void do_notify_parent_cldstop(struct task_struct 
*tsk, int why)
 
 static inline int may_ptrace_stop(void)
 {
-   if (!likely(current->ptrace & PT_PTRACED))
+   if (likely(!(current->ptrace & PT_PTRACED)))
return 0;
/*
 * Are we in the middle of do_coredump?
@@ -1574,7 +1574,7 @@ static int sigkill_pending(struct task_struct *tsk)
 {
return ((sigismember(>pending.signal, SIGKILL) ||
 sigismember(>signal->shared_pending.signal, SIGKILL)) &&
-   !unlikely(sigismember(>blocked, SIGKILL)));
+unlikely(!sigismember(>blocked, SIGKILL)));
 }
 
 /*
@@ -1625,7 +1625,7 @@ static void ptrace_stop(int exit_code, int clear_code, 
siginfo_t *info)
spin_unlock_irq(>sighand->siglock);
try_to_freeze();
read_lock(_lock);
-   if (!unlikely(killed) && may_ptrace_stop()) {
+   if (unlikely(!killed) && may_ptrace_stop()) {
do_notify_parent_cldstop(current, CLD_TRAPPED);
read_unlock(_lock);
schedule();
@@ -1717,7 +1717,7 @@ static int do_signal_stop(int signr)
} else {
struct task_struct *t;
 
-   if (!likely(sig->flags & SIGNAL_STOP_DEQUEUED) ||
+   if (likely(!(sig->flags & SIGNAL_STOP_DEQUEUED)) ||
unlikely(sig->group_exit_task))
return 0;
/*

--
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/

[PATCH 1/2] [x86] arch/x86/kernel/signal_32.c: replace !likely(x) by likely(!x)

[Roel Kluin]

Replace !likely(x) by likely(!x)

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
index caee1f0..335872f 100644
--- a/arch/x86/kernel/signal_32.c
+++ b/arch/x86/kernel/signal_32.c
@@ -303,7 +303,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs * regs, 
size_t frame_size)
 * If we are on the alternate signal stack and would overflow it, don't.
 * Return an always-bogus address instead so we will die with SIGSEGV.
 */
-   if (on_sig_stack(sp) && !likely(on_sig_stack(sp - frame_size)))
+   if (on_sig_stack(sp) && likely(!on_sig_stack(sp - frame_size)))
return (void __user *) -1L;
 
/* This is the X/Open sanctioned signal stack switching.  */
--
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: + bluetooth-fix-warning-in-net-bluetooth-hci_sysfsc.patch added to -mm tree

[Roel Kluin]

Andrew Morton wrote:
> On Sat, 16 Feb 2008 01:27:41 -0800 (PST) David Miller <[EMAIL PROTECTED]> 
> wrote:
> 
>> From: [EMAIL PROTECTED]
>> Date: Fri, 15 Feb 2008 20:49:33 -0800
>>
>>> Subject: bluetooth: fix warning in net/bluetooth/hci_sysfs.c
>>> From: Andrew Morton <[EMAIL PROTECTED]>
>>>
>>> net/bluetooth/hci_sysfs.c: In function 'del_conn':
>>> net/bluetooth/hci_sysfs.c:339: warning: suggest parentheses around 
>>> assignment used as truth value
>>>
>>> Cc: Dave Young <[EMAIL PROTECTED]>
>>> Cc: David S. Miller <[EMAIL PROTECTED]>
>>> Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
>> Linus barfed when this exact patch got posted a few weeks
>> ago, he said he considers just adding parenthesis to quiet
>> this warning awful and explicit NULL test should be used.
> 
> - while (dev = device_find_child(>dev, NULL, __match_tty)) {
> + while ((dev = device_find_child(>dev, NULL, __match_tty))) {
> 
> hrm.
> 
> box:/usr/src/linux-2.6.25-rc2> grep -r 'while [(][(].*{$' . | wc -l   
> 
> 1240
> box:/usr/src/linux-2.6.25-rc2> grep -r 'while [(][(].*{$' . | grep -v NULL | 
> wc -l
> 867
> 
> so two thirds of them presently don't bother testing for NULL.  It's a
> pretty common idiom.
> 
> (adds the != NULL, sees that it reaches column 82, drops patch in disgust)

Not my results:

# greps for 'while ((x = y(...)))' see below for variables

git-grep -n -A2 "while" | 
sed -n 
"s/^\([^\.]*\.[chsS]-[0-9]*-\|\([^\.]*\.[chsS]:[0-9]*:\)$s\)\(.*\)$/\2\3/p" |
tr "\n" "@" |
grep -o "@[EMAIL PROTECTED]($z($z$W$z=$z$W$z($h)$z)$z)" |
tr "@\t" " " | tr -s " " | wc -l
262

# while ((x = y(...)) != NULL)

git-grep -n -A2 "while" | 
sed -n 
"s/^\([^\.]*\.[chsS]-[0-9]*-\|\([^\.]*\.[chsS]:[0-9]*:\)$s\)\(.*\)$/\2\3/p" |
tr "\n" "@" |
grep -o "@[EMAIL PROTECTED]($z($z$W$z=$z$W$z($h)$z)$z\!=${z}NULL$z)" |
tr "@\t" " " | tr -s " " | wc -l
333

# while ((x = y(...)) != 0)

git-grep -n -A2 "while" | 
sed -n 
"s/^\([^\.]*\.[chsS]-[0-9]*-\|\([^\.]*\.[chsS]:[0-9]*:\)$s\)\(.*\)$/\2\3/p" |
tr "\n" "@" |
grep -o "@[EMAIL PROTECTED]($z($z$W$z=$z$W$z($h)$z)$z\!=${z}0$z)" |
tr "@\t" " " | tr -s " " | wc -l
45

# while (!(x = y(...)))

git-grep -n -A2 "while" | 
sed -n 
"s/^\([^\.]*\.[chsS]-[0-9]*-\|\([^\.]*\.[chsS]:[0-9]*:\)$s\)\(.*\)$/\2\3/p" |
tr "\n" "@" |
grep -o "@[EMAIL PROTECTED]($z\!$z($z$W$z=$z$W$z($h)$z)$z)" |
tr "@\t" " " | tr -s " " | wc -l
19


V="[A-Za-z_]\+[A-Za-z0-9_]*"
W="$V\(\[$V\]\|\[[0-9]\+\]\|\.$V\|->$V\)*"
s="[[:space:]]*"
h="[^()]*\(([^()]*)[^()]*\)*"
z="[EMAIL PROTECTED]:space:]]*"
--
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: + bluetooth-fix-warning-in-net-bluetooth-hci_sysfsc.patch added to -mm tree

[Roel Kluin]

Andrew Morton wrote:
 On Sat, 16 Feb 2008 01:27:41 -0800 (PST) David Miller [EMAIL PROTECTED] 
 wrote:
 
 From: [EMAIL PROTECTED]
 Date: Fri, 15 Feb 2008 20:49:33 -0800

 Subject: bluetooth: fix warning in net/bluetooth/hci_sysfs.c
 From: Andrew Morton [EMAIL PROTECTED]

 net/bluetooth/hci_sysfs.c: In function 'del_conn':
 net/bluetooth/hci_sysfs.c:339: warning: suggest parentheses around 
 assignment used as truth value

 Cc: Dave Young [EMAIL PROTECTED]
 Cc: David S. Miller [EMAIL PROTECTED]
 Signed-off-by: Andrew Morton [EMAIL PROTECTED]
 Linus barfed when this exact patch got posted a few weeks
 ago, he said he considers just adding parenthesis to quiet
 this warning awful and explicit NULL test should be used.
 
 - while (dev = device_find_child(conn-dev, NULL, __match_tty)) {
 + while ((dev = device_find_child(conn-dev, NULL, __match_tty))) {
 
 hrm.
 
 box:/usr/src/linux-2.6.25-rc2 grep -r 'while [(][(].*{$' . | wc -l   
 
 1240
 box:/usr/src/linux-2.6.25-rc2 grep -r 'while [(][(].*{$' . | grep -v NULL | 
 wc -l
 867
 
 so two thirds of them presently don't bother testing for NULL.  It's a
 pretty common idiom.
 
 (adds the != NULL, sees that it reaches column 82, drops patch in disgust)

Not my results:

# greps for 'while ((x = y(...)))' see below for variables

git-grep -n -A2 while | 
sed -n 
s/^\([^\.]*\.[chsS]-[0-9]*-\|\([^\.]*\.[chsS]:[0-9]*:\)$s\)\(.*\)$/\2\3/p |
tr \n @ |
grep -o @[EMAIL PROTECTED]($z($z$W$z=$z$W$z($h)$z)$z) |
tr @\t   | tr -s   | wc -l
262

# while ((x = y(...)) != NULL)

git-grep -n -A2 while | 
sed -n 
s/^\([^\.]*\.[chsS]-[0-9]*-\|\([^\.]*\.[chsS]:[0-9]*:\)$s\)\(.*\)$/\2\3/p |
tr \n @ |
grep -o @[EMAIL PROTECTED]($z($z$W$z=$z$W$z($h)$z)$z\!=${z}NULL$z) |
tr @\t   | tr -s   | wc -l
333

# while ((x = y(...)) != 0)

git-grep -n -A2 while | 
sed -n 
s/^\([^\.]*\.[chsS]-[0-9]*-\|\([^\.]*\.[chsS]:[0-9]*:\)$s\)\(.*\)$/\2\3/p |
tr \n @ |
grep -o @[EMAIL PROTECTED]($z($z$W$z=$z$W$z($h)$z)$z\!=${z}0$z) |
tr @\t   | tr -s   | wc -l
45

# while (!(x = y(...)))

git-grep -n -A2 while | 
sed -n 
s/^\([^\.]*\.[chsS]-[0-9]*-\|\([^\.]*\.[chsS]:[0-9]*:\)$s\)\(.*\)$/\2\3/p |
tr \n @ |
grep -o @[EMAIL PROTECTED]($z\!$z($z$W$z=$z$W$z($h)$z)$z) |
tr @\t   | tr -s   | wc -l
19


V=[A-Za-z_]\+[A-Za-z0-9_]*
W=$V\(\[$V\]\|\[[0-9]\+\]\|\.$V\|-$V\)*
s=[[:space:]]*
h=[^()]*\(([^()]*)[^()]*\)*
z=[EMAIL PROTECTED]:space:]]*
--
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/

[PATCH 1/2] [x86] arch/x86/kernel/signal_32.c: replace !likely(x) by likely(!x)

[Roel Kluin]

Replace !likely(x) by likely(!x)

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
index caee1f0..335872f 100644
--- a/arch/x86/kernel/signal_32.c
+++ b/arch/x86/kernel/signal_32.c
@@ -303,7 +303,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs * regs, 
size_t frame_size)
 * If we are on the alternate signal stack and would overflow it, don't.
 * Return an always-bogus address instead so we will die with SIGSEGV.
 */
-   if (on_sig_stack(sp)  !likely(on_sig_stack(sp - frame_size)))
+   if (on_sig_stack(sp)  likely(!on_sig_stack(sp - frame_size)))
return (void __user *) -1L;
 
/* This is the X/Open sanctioned signal stack switching.  */
--
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/

[PATCH 2/2] kernel/{exit.c, signal.c, power/process.c}: replace !likely(x) by likely(!x)

[Roel Kluin]

Not entirely sure who to send this to
---
Replace !likely(x) by likely(!x)

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/kernel/exit.c b/kernel/exit.c
index 506a957..df207fc 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1497,7 +1497,7 @@ repeat:
/*
 * We don't reap group leaders with subthreads.
 */
-   if (!likely(options  WEXITED))
+   if (likely(!(options  WEXITED)))
continue;
retval = wait_task_zombie(p,
(options  WNOWAIT), infop,
@@ -1508,7 +1508,7 @@ repeat:
 * exit, stop, or stop and then continue.
 */
flag = 1;
-   if (!unlikely(options  WCONTINUED))
+   if (unlikely(!(options  WCONTINUED)))
continue;
retval = wait_task_continued(p,
(options  WNOWAIT), infop,
diff --git a/kernel/power/process.c b/kernel/power/process.c
index 7c2118f..b42254b 100644
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -36,7 +36,7 @@ static inline int freezeable(struct task_struct * p)
  */
 static inline void frozen_process(void)
 {
-   if (!unlikely(current-flags  PF_NOFREEZE)) {
+   if (unlikely(!(current-flags  PF_NOFREEZE))) {
current-flags |= PF_FROZEN;
wmb();
}
diff --git a/kernel/signal.c b/kernel/signal.c
index 84917fe..1155e16 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1308,7 +1308,7 @@ int send_sigqueue(int sig, struct sigqueue *q, struct 
task_struct *p)
 */
rcu_read_lock();
 
-   if (!likely(lock_task_sighand(p, flags))) {
+   if (likely(!lock_task_sighand(p, flags))) {
ret = -1;
goto out_err;
}
@@ -1548,7 +1548,7 @@ static void do_notify_parent_cldstop(struct task_struct 
*tsk, int why)
 
 static inline int may_ptrace_stop(void)
 {
-   if (!likely(current-ptrace  PT_PTRACED))
+   if (likely(!(current-ptrace  PT_PTRACED)))
return 0;
/*
 * Are we in the middle of do_coredump?
@@ -1574,7 +1574,7 @@ static int sigkill_pending(struct task_struct *tsk)
 {
return ((sigismember(tsk-pending.signal, SIGKILL) ||
 sigismember(tsk-signal-shared_pending.signal, SIGKILL)) 
-   !unlikely(sigismember(tsk-blocked, SIGKILL)));
+unlikely(!sigismember(tsk-blocked, SIGKILL)));
 }
 
 /*
@@ -1625,7 +1625,7 @@ static void ptrace_stop(int exit_code, int clear_code, 
siginfo_t *info)
spin_unlock_irq(current-sighand-siglock);
try_to_freeze();
read_lock(tasklist_lock);
-   if (!unlikely(killed)  may_ptrace_stop()) {
+   if (unlikely(!killed)  may_ptrace_stop()) {
do_notify_parent_cldstop(current, CLD_TRAPPED);
read_unlock(tasklist_lock);
schedule();
@@ -1717,7 +1717,7 @@ static int do_signal_stop(int signr)
} else {
struct task_struct *t;
 
-   if (!likely(sig-flags  SIGNAL_STOP_DEQUEUED) ||
+   if (likely(!(sig-flags  SIGNAL_STOP_DEQUEUED)) ||
unlikely(sig-group_exit_task))
return 0;
/*

--
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/

[PATCH 1/3] Fix Unlikely(x) == y

[Roel Kluin]

The patch below was not yet tested. If it's correct as it is, please comment.
---
Fix Unlikely(x) == y

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/arch/powerpc/platforms/ps3/interrupt.c 
b/arch/powerpc/platforms/ps3/interrupt.c
index 3a6db04..a14e5cd 100644
--- a/arch/powerpc/platforms/ps3/interrupt.c
+++ b/arch/powerpc/platforms/ps3/interrupt.c
@@ -709,7 +709,7 @@ static unsigned int ps3_get_irq(void)
asm volatile(cntlzd %0,%1 : =r (plug) : r (x));
plug = 0x3f;
 
-   if (unlikely(plug) == NO_IRQ) {
+   if (unlikely(plug == NO_IRQ)) {
pr_debug(%s:%d: no plug found: thread_id %lu\n, __func__,
__LINE__, pd-thread_id);
dump_bmp(per_cpu(ps3_private, 0));
--
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/

[PATCH 3/3] drivers/s390/block/dcssblk.c: Fix Unlikely(x) != y

[Roel Kluin]

The patch below was not yet tested. If it's correct as it is, please comment.
---
Fix Unlikely(x) != y

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/s390/block/dcssblk.c b/drivers/s390/block/dcssblk.c
index 3faf053..e6c94db 100644
--- a/drivers/s390/block/dcssblk.c
+++ b/drivers/s390/block/dcssblk.c
@@ -666,7 +666,7 @@ dcssblk_make_request(struct request_queue *q, struct bio 
*bio)
page_addr = (unsigned long)
page_address(bvec-bv_page) + bvec-bv_offset;
source_addr = dev_info-start + (index12) + bytes_done;
-   if (unlikely(page_addr  4095) != 0 || (bvec-bv_len  4095) != 
0)
+   if (unlikely((page_addr  4095) != 0) || (bvec-bv_len  4095) 
!= 0)
// More paranoia.
goto fail;
if (bio_data_dir(bio) == READ) {
--
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/

[PATCH 2/3] drivers/media/video/sn9c102/sn9c102_core.c Fix Unlikely(x) == y

[Roel Kluin]

The patch below was not yet tested. If it's incorrect, please comment.
---
Fix Unlikely(x) == y

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/media/video/sn9c102/sn9c102_core.c 
b/drivers/media/video/sn9c102/sn9c102_core.c
index c40ba3a..66313b1 100644
--- a/drivers/media/video/sn9c102/sn9c102_core.c
+++ b/drivers/media/video/sn9c102/sn9c102_core.c
@@ -528,7 +528,7 @@ sn9c102_find_sof_header(struct sn9c102_device* cam, void* 
mem, size_t len)
 
/* Search for the SOF marker (fixed part) in the header */
for (j = 0, b=cam-sof.bytesread; j+b  sizeof(marker); j++) {
-   if (unlikely(i+j) == len)
+   if (unlikely(i+j == len))
return NULL;
if (*(m+i+j) == marker[cam-sof.bytesread]) {
cam-sof.header[cam-sof.bytesread] = *(m+i+j);
--
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/

[PATCH] kernel/sched.c unlikely(x) || unlikely(y) = unlikely(x || y)

[Roel Kluin]

Not yet tested.
---
Replace unlikely(x) || unlikely(y) by unlikely(x || y)

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/kernel/sched.c b/kernel/sched.c
index f28f19e..816c299 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -137,7 +137,7 @@ static inline void sg_inc_cpu_power(struct sched_group *sg, 
u32 val)
 
 static inline int rt_policy(int policy)
 {
-   if (unlikely(policy == SCHED_FIFO) || unlikely(policy == SCHED_RR))
+   if (unlikely(policy == SCHED_FIFO || policy == SCHED_RR))
return 1;
return 0;
 }
@@ -3832,7 +3832,7 @@ static inline void schedule_debug(struct task_struct 
*prev)
 * schedule() atomically, we ignore that path for now.
 * Otherwise, whine if we are scheduling when we should not be.
 */
-   if (unlikely(in_atomic_preempt_off())  unlikely(!prev-exit_state))
+   if (unlikely(in_atomic_preempt_off()  !prev-exit_state))
__schedule_bug(prev);
 
profile_hit(SCHED_PROFILING, __builtin_return_address(0));
--
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/

[PATCH] drivers/mtd/onenand/onenand_base.c unlikely(x) || unlikely(y) = unlikely(x || y)

[Roel Kluin]

Not yet tested.
---
Replace unlikely(x) || unlikely(y) by unlikely(x || y)

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/mtd/onenand/onenand_base.c 
b/drivers/mtd/onenand/onenand_base.c
index 8d7d21b..7334b4a 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -1336,7 +1336,7 @@ static int onenand_panic_write(struct mtd_info *mtd, 
loff_t to, size_t len,
}
 
/* Reject writes, which are not page aligned */
-if (unlikely(NOTALIGNED(to)) || unlikely(NOTALIGNED(len))) {
+if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
 printk(KERN_ERR onenand_panic_write: Attempt to write not 
page aligned data\n);
 return -EINVAL;
 }
@@ -1466,7 +1466,7 @@ static int onenand_write_ops_nolock(struct mtd_info *mtd, 
loff_t to,
}
 
/* Reject writes, which are not page aligned */
-if (unlikely(NOTALIGNED(to)) || unlikely(NOTALIGNED(len))) {
+if (unlikely(NOTALIGNED(to) || NOTALIGNED(len))) {
 printk(KERN_ERR onenand_write_ops_nolock: Attempt to write 
not page aligned data\n);
 return -EINVAL;
 }
--
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 1/2] [x86] arch/x86/kernel/signal_32.c: replace !likely(x) by likely(!x)

[Roel Kluin]

Roel Kluin wrote:
 Replace !likely(x) by likely(!x)
 
 Signed-off-by: Roel Kluin [EMAIL PROTECTED]
 ---
 diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
 index caee1f0..335872f 100644
 --- a/arch/x86/kernel/signal_32.c
 +++ b/arch/x86/kernel/signal_32.c
 @@ -303,7 +303,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs * 
 regs, size_t frame_size)
* If we are on the alternate signal stack and would overflow it, don't.
* Return an always-bogus address instead so we will die with SIGSEGV.
*/
 - if (on_sig_stack(sp)  !likely(on_sig_stack(sp - frame_size)))
 + if (on_sig_stack(sp)  likely(!on_sig_stack(sp - frame_size)))
   return (void __user *) -1L;
  
   /* This is the X/Open sanctioned signal stack switching.  */
 --
sorry, please ignore this patch
--
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 2/2] kernel/{exit.c, signal.c, power/process.c}: replace !likely(x) by likely(!x)

[Roel Kluin]

H. Peter Anvin wrote:
 Roel Kluin wrote:
 Not entirely sure who to send this to
 ---
 Replace !likely(x) by likely(!x)
 
 Whoa...
 
 Are you sure this is correct?
 
 !likely(x) is equivalent to unlikely(!x), not the opposite, so this is a
 functional change...
 
 -hpa
 
You are right, sorry, Need more caffeine. I will send the right patch later.
--
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 2/2] kernel/{exit.c, signal.c, power/process.c}: replace !likely(x) by likely(!x)

[Roel Kluin]

H. Peter Anvin wrote:
 Roel Kluin wrote:
 H. Peter Anvin wrote:
 Roel Kluin wrote:
 Not entirely sure who to send this to
 ---
 Replace !likely(x) by likely(!x)
 Whoa...

 Are you sure this is correct?

 !likely(x) is equivalent to unlikely(!x), not the opposite, so this is a
 functional change...

 You are right, sorry, Need more caffeine. I will send the right patch
 later.
 
 Note I didn't say it was wrong, I just wanted a bit more explanation
 about why it's right, if it is.

No, I just wanted to move the '!' within the parentheses, I didn't want
to change semantics. below is how I guess the patch should have been. 
Thanks for your response.
---
Replace !likely(x) by unlikely(!x)

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/kernel/signal.c b/kernel/signal.c
index 84917fe..cf5d45a 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1308,7 +1308,7 @@ int send_sigqueue(int sig, struct sigqueue *q, struct 
task_struct *p)
 */
rcu_read_lock();
 
-   if (!likely(lock_task_sighand(p, flags))) {
+   if (unlikely(!lock_task_sighand(p, flags))) {
ret = -1;
goto out_err;
}
@@ -1548,7 +1548,7 @@ static void do_notify_parent_cldstop(struct task_struct 
*tsk, int why)
 
 static inline int may_ptrace_stop(void)
 {
-   if (!likely(current-ptrace  PT_PTRACED))
+   if (unlikely(!(current-ptrace  PT_PTRACED)))
return 0;
/*
 * Are we in the middle of do_coredump?
@@ -1574,7 +1574,7 @@ static int sigkill_pending(struct task_struct *tsk)
 {
return ((sigismember(tsk-pending.signal, SIGKILL) ||
 sigismember(tsk-signal-shared_pending.signal, SIGKILL)) 
-   !unlikely(sigismember(tsk-blocked, SIGKILL)));
+likely(!sigismember(tsk-blocked, SIGKILL)));
 }
 
 /*
@@ -1625,7 +1625,7 @@ static void ptrace_stop(int exit_code, int clear_code, 
siginfo_t *info)
spin_unlock_irq(current-sighand-siglock);
try_to_freeze();
read_lock(tasklist_lock);
-   if (!unlikely(killed)  may_ptrace_stop()) {
+   if (likely(!killed)  may_ptrace_stop()) {
do_notify_parent_cldstop(current, CLD_TRAPPED);
read_unlock(tasklist_lock);
schedule();
@@ -1717,8 +1717,8 @@ static int do_signal_stop(int signr)
} else {
struct task_struct *t;
 
-   if (!likely(sig-flags  SIGNAL_STOP_DEQUEUED) ||
-   unlikely(sig-group_exit_task))
+   if (unlikely(!(sig-flags  SIGNAL_STOP_DEQUEUED) ||
+   sig-group_exit_task))
return 0;
/*
 * There is no group stop already in progress.

--
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 1/2] [x86] arch/x86/kernel/signal_32.c: replace !likely(x) by likely(!x)

[Roel Kluin]

Roel Kluin wrote:
 Roel Kluin wrote:
 Replace !likely(x) by likely(!x)

 sorry, please ignore this patch

The patch below shouldn't change semantics.
---
replace !likely(x) by unlikely(!x)

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
index caee1f0..c0fb075 100644
--- a/arch/x86/kernel/signal_32.c
+++ b/arch/x86/kernel/signal_32.c
@@ -303,7 +303,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs * regs, 
size_t frame_size)
 * If we are on the alternate signal stack and would overflow it, don't.
 * Return an always-bogus address instead so we will die with SIGSEGV.
 */
-   if (on_sig_stack(sp)  !likely(on_sig_stack(sp - frame_size)))
+   if (on_sig_stack(sp)  unlikely(!on_sig_stack(sp - frame_size)))
return (void __user *) -1L;
 
/* This is the X/Open sanctioned signal stack switching.  */
--
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/

[PATCH] fs/ufs/util.h 2nd parameter of fs32_to_cpu is not boolean

[Roel Kluin]

from: fs/befs/endian.h +33
static inline u32
fs32_to_cpu(const struct super_block *sb, fs32 n)
{
if (BEFS_SB(sb)-byte_order == BEFS_BYTESEX_LE)
return le32_to_cpu((__force __le32)n);
else
return be32_to_cpu((__force __be32)n);
}

The 2nd parameter is not boolean
---
Test the return value, rather than passing a boolean

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/fs/ufs/util.h b/fs/ufs/util.h
index b26fc4d..23ceed8 100644
--- a/fs/ufs/util.h
+++ b/fs/ufs/util.h
@@ -58,7 +58,7 @@ ufs_set_fs_state(struct super_block *sb, struct 
ufs_super_block_first *usb1,
 {
switch (UFS_SB(sb)-s_flags  UFS_ST_MASK) {
case UFS_ST_SUNOS:
-   if (fs32_to_cpu(sb, usb3-fs_postblformat == UFS_42POSTBLFMT)) {
+   if (fs32_to_cpu(sb, usb3-fs_postblformat) == UFS_42POSTBLFMT) {
usb1-fs_u0.fs_sun.fs_state = cpu_to_fs32(sb, value);
break;
}
--
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: Announce: Linux-next (Or Andrew's dream :-))

[Roel Kluin]

Alan Cox wrote:
>> Evolution in nature and changes in code are different because in code junk
>> and bugs are constantly removed. In biology junk is allowed and may provide
>> a pool for future development. Linux development is intended and not
>> survival.
> 
> I would be interested to see any evidence (rather than intuition) to
> support that, given that both appear to be the same kind of structure and
> that structure is nowadays fairly well understood.

What do you mean with structure, the evolution? that both are a language?

--
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: Announce: Linux-next (Or Andrew's dream :-))

[Roel Kluin]

Linus Torvalds wrote:
> 
> On Wed, 13 Feb 2008, Roel Kluin wrote:
>> In nature there is a lot of duplication: several copies of genes can exist
>> and different copies may have a distinct evolution.
> 
> This is true of very complex animals, but much less so when looking at 
> things like bacteria (and arguably, any current sw project is closer to 
> bacteria in complexity than anything mammalian).
> 
> In bacteria (and viruses), duplication of DNA/RNA is a big cost of living 
> in general, and as a result there is *much* less junk DNA. So in an 
> evolutionary sense, it's much closer to what the kernel should have (with 
> occasional duplication of code and interfaces to allow new functionality, 
> but rather aggressive pruning of the excess baggage).

I like the comparison, and while I wrote my comment I have to admit I was
also thinking of bacteria and virusses as an exception: There the speed of
replication can be an important factor for survival. Less DNA means faster
replication and therefore the pressure is on removal of junk DNA. It can
be disputed however that removal of 'junk sourcecode' is a survival factor
for the linux kernel but the benefit may be disputable as well.

> In other words, all of these choices are a matter of "balance". In some 
> areas, excess code is not a sufficient downside, and we keep even broken 
> source code around with no actual function, "just because" (or rather, 
> because the cost of carrying it around is so small that nobody cares). 
> 
> That's true in the kernel as in biology: check out not just deprecated 
> code, but the drivers and other odds-and-ends that are explicitly marked 
> as non-coding DNA (we just happen to call them BROKEN in our Kconfig).
> 
>   Linus

Maybe we can elaborate a bit on this comparison, just for fun:

I think not the linux kernel alone, but rather the entire Linux OS could
be compared with a cell. The kernel source encodes vital software parts
including the interactions with hardware - the environment for the
computer. Gcc can be compared with the (transcription and) translation
machinery. DNA can be seen as a language that encodes proteins, with
biological functions: Some are vital, including ones that allow
interactions with the environment: The cellular environment is beyond
the membrane. Interactions occur through membrane receptors, channels,
etc.

Interaction between proteins can be compared with functions selectively
calling other functions. Activation of certain proteins can cause a
cascade of protein interactions, comparable with function calls in a
loop: the activated protein activates particular protein(s) several
times. Some proteins influence intracellular messengers: cellular
global variables.

Transmembrane receptors responding to extracellular signals transmit
this through conformational changes across the membrane to the
intracellular region: These structural changes may allow interactions
with new proteins. Maybe comparable with a combination of hardware
interrupts, signals and the userspace?

The response to extracellular signals often depends on several
sequential interactions between proteins. This provides a protective
layer that can be compared with the kernelspace layer. This is where 
the comparison probably becomes too biased to continue.
--
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: Announce: Linux-next (Or Andrew's dream :-))

[Roel Kluin]

Linus Torvalds wrote:
 
 On Wed, 13 Feb 2008, Roel Kluin wrote:
 In nature there is a lot of duplication: several copies of genes can exist
 and different copies may have a distinct evolution.
 
 This is true of very complex animals, but much less so when looking at 
 things like bacteria (and arguably, any current sw project is closer to 
 bacteria in complexity than anything mammalian).
 
 In bacteria (and viruses), duplication of DNA/RNA is a big cost of living 
 in general, and as a result there is *much* less junk DNA. So in an 
 evolutionary sense, it's much closer to what the kernel should have (with 
 occasional duplication of code and interfaces to allow new functionality, 
 but rather aggressive pruning of the excess baggage).

I like the comparison, and while I wrote my comment I have to admit I was
also thinking of bacteria and virusses as an exception: There the speed of
replication can be an important factor for survival. Less DNA means faster
replication and therefore the pressure is on removal of junk DNA. It can
be disputed however that removal of 'junk sourcecode' is a survival factor
for the linux kernel but the benefit may be disputable as well.

 In other words, all of these choices are a matter of balance. In some 
 areas, excess code is not a sufficient downside, and we keep even broken 
 source code around with no actual function, just because (or rather, 
 because the cost of carrying it around is so small that nobody cares). 
 
 That's true in the kernel as in biology: check out not just deprecated 
 code, but the drivers and other odds-and-ends that are explicitly marked 
 as non-coding DNA (we just happen to call them BROKEN in our Kconfig).
 
   Linus

Maybe we can elaborate a bit on this comparison, just for fun:

I think not the linux kernel alone, but rather the entire Linux OS could
be compared with a cell. The kernel source encodes vital software parts
including the interactions with hardware - the environment for the
computer. Gcc can be compared with the (transcription and) translation
machinery. DNA can be seen as a language that encodes proteins, with
biological functions: Some are vital, including ones that allow
interactions with the environment: The cellular environment is beyond
the membrane. Interactions occur through membrane receptors, channels,
etc.

Interaction between proteins can be compared with functions selectively
calling other functions. Activation of certain proteins can cause a
cascade of protein interactions, comparable with function calls in a
loop: the activated protein activates particular protein(s) several
times. Some proteins influence intracellular messengers: cellular
global variables.

Transmembrane receptors responding to extracellular signals transmit
this through conformational changes across the membrane to the
intracellular region: These structural changes may allow interactions
with new proteins. Maybe comparable with a combination of hardware
interrupts, signals and the userspace?

The response to extracellular signals often depends on several
sequential interactions between proteins. This provides a protective
layer that can be compared with the kernelspace layer. This is where 
the comparison probably becomes too biased to continue.
--
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: Announce: Linux-next (Or Andrew's dream :-))

[Roel Kluin]

Alan Cox wrote:
 Evolution in nature and changes in code are different because in code junk
 and bugs are constantly removed. In biology junk is allowed and may provide
 a pool for future development. Linux development is intended and not
 survival.
 
 I would be interested to see any evidence (rather than intuition) to
 support that, given that both appear to be the same kind of structure and
 that structure is nowadays fairly well understood.

What do you mean with structure, the evolution? that both are a language?

--
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: Announce: Linux-next (Or Andrew's dream :-))

[Roel Kluin]

Linus Torvalds wrote:
> 
> On Tue, 12 Feb 2008, Greg KH wrote:
>>> That's the point.
>> Not it isn't.  To quote you a number of years ago:
>>  "Linux is evolution, not intelligent design"
> 
> Umm. Have you read a lot of books on evolution?
> 
> It doesn't sound like you have.
> 
> The fact is, evolution often does odd (and "suboptimal") things exactly 
> because it does incremental changes that DO NOT BREAK at any point.

This is not entirely true if the pressure for changes are removed. For 
instance in mammals the bones in the ear are what used to be gills in fish.
When fish became amphibians the gills weren't needed as much and evolution
took a side path.

> The examples are legion. The mammalian eye has the retina "backwards", 
> with the blind spot appearing because the fundmanetal infrastructure (the 
> optical nerves) actually being in *front* of the light sensor and needing 
> a hole in the retina to get the information (and blood flow) to go to the 
> brain!
> 
> In other words, exactly *because* evolution requires "bisectability" (any 
> non-viable point in between is a dead end by definition) and does things 
> incrementally, it doesn't do big flips. It fixes the problems on an 
> incremental scale both when it comes to the details and when it comes to 
> both "details" (actual protein-coding genes that code directly for some 
> expression) and "infrastructure" (homeobox and non-coding genes).

In nature there is a lot of duplication: several copies of genes can exist
and different copies may have a distinct evolution. There is also a lot of
'junk' DNA that doesn't code for anything (although it may have regulating
functions). In there some copies of genes may remain that are inactivated,
as well as parts of virusses, slowly obtaining random mutations because
there is no pressure on the evolution of them. Some may eventually become
active again and have different functions.

The duplication also often ensures there is fallback when random mutations
are acquired and a protein is knocked out. Besides the two chromosomes
several proteins also can have overlapping functions. The result is more
like a balance. 
 
Evolution in nature and changes in code are different because in code junk
and bugs are constantly removed. In biology junk is allowed and may provide
a pool for future development. Linux development is intended and not
survival.
--
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/

[drivers/video/sis/init301.c] SiS_Pr->ChipType >= SIS_661 not evaluated twice

[Roel Kluin]

Not sure whether this is important, but in drivers/video/sis/init301.c:1557:

  if((SiS_Pr->ChipType >= SIS_661) || (SiS_Pr->SiS_ROMNew)) {
 SiS_Pr->SiS_LCDTypeInfo = (SiS_GetReg(SiS_Pr->SiS_P3d4,0x39) & 0x7c) >> 2;
  } else if((SiS_Pr->ChipType < SIS_315H) || (SiS_Pr->ChipType >= SIS_661)) {
 SiS_Pr->SiS_LCDTypeInfo = temp >> 4;

note the duplicate test for 'SiS_Pr->ChipType >= SIS_661'
if it was true in the first test, the second test won't be evaluated.
--
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] ufs: [bl]e*_add_cpu conversion

[Roel Kluin]

[EMAIL PROTECTED] wrote:
> replace all:
> big/little_endian_variable = 
> cpu_to_[bl]eX([bl]eX_to_cpu(big/little_endian_variable) +
>   expression_in_cpu_byteorder);
> with:
>   [bl]eX_add_cpu(/little_endian_variable, 
> expression_in_cpu_byteorder);

you may also want these:
---
[bl]e_add_cpu conversion in return

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/fs/ufs/swab.h b/fs/ufs/swab.h
index 1683d2b..a1e3000 100644
--- a/fs/ufs/swab.h
+++ b/fs/ufs/swab.h
@@ -44,18 +44,22 @@ static __inline u32
 fs64_add(struct super_block *sbp, u32 *n, int d)
 {
if (UFS_SB(sbp)->s_bytesex == BYTESEX_LE)
-   return *n = cpu_to_le64(le64_to_cpu(*n)+d);
+   le64_add_cpu(n, d);
else
-   return *n = cpu_to_be64(be64_to_cpu(*n)+d);
+   be64_add_cpu(n, d);
+
+   return *n;
 }
 
 static __inline u32
 fs64_sub(struct super_block *sbp, u32 *n, int d)
 {
if (UFS_SB(sbp)->s_bytesex == BYTESEX_LE)
-   return *n = cpu_to_le64(le64_to_cpu(*n)-d);
+   le64_add_cpu(n, -d);
else
-   return *n = cpu_to_be64(be64_to_cpu(*n)-d);
+   be64_add_cpu(n, -d);
+
+   return *n;
 }
 
 static __inline u32

--
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] crypto: be*_add_cpu conversion

[Roel Kluin]

[EMAIL PROTECTED] wrote:
> From: Marcin Slusarz <[EMAIL PROTECTED]>
> 
> replace all:
> big_endian_variable = cpu_to_beX(beX_to_cpu(big_endian_variable) +
>   expression_in_cpu_byteorder);
> with:
>   beX_add_cpu(_endian_variable, expression_in_cpu_byteorder);
> generated with semantic patch
> 
> Signed-off-by: Marcin Slusarz <[EMAIL PROTECTED]>
> Cc: Herbert Xu <[EMAIL PROTECTED]>
> Cc: David S. Miller <[EMAIL PROTECTED]>
> ---
>  crypto/lrw.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/crypto/lrw.c b/crypto/lrw.c
> index 9d52e58..4d93928 100644
> --- a/crypto/lrw.c
> +++ b/crypto/lrw.c
> @@ -92,7 +92,7 @@ struct sinfo {
>  static inline void inc(be128 *iv)
>  {
>   if (!(iv->b = cpu_to_be64(be64_to_cpu(iv->b) + 1)))

maybe you also want instead of the line above:

be64_add_cpu(>b, 1);
if (!iv->b)

> - iv->a = cpu_to_be64(be64_to_cpu(iv->a) + 1);
> + be64_add_cpu(>a, 1);
>  }
>  
>  static inline void lrw_round(struct sinfo *s, void *dst, const void *src)

--
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] crypto: be*_add_cpu conversion

[Roel Kluin]

[EMAIL PROTECTED] wrote:
 From: Marcin Slusarz [EMAIL PROTECTED]
 
 replace all:
 big_endian_variable = cpu_to_beX(beX_to_cpu(big_endian_variable) +
   expression_in_cpu_byteorder);
 with:
   beX_add_cpu(big_endian_variable, expression_in_cpu_byteorder);
 generated with semantic patch
 
 Signed-off-by: Marcin Slusarz [EMAIL PROTECTED]
 Cc: Herbert Xu [EMAIL PROTECTED]
 Cc: David S. Miller [EMAIL PROTECTED]
 ---
  crypto/lrw.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/crypto/lrw.c b/crypto/lrw.c
 index 9d52e58..4d93928 100644
 --- a/crypto/lrw.c
 +++ b/crypto/lrw.c
 @@ -92,7 +92,7 @@ struct sinfo {
  static inline void inc(be128 *iv)
  {
   if (!(iv-b = cpu_to_be64(be64_to_cpu(iv-b) + 1)))

maybe you also want instead of the line above:

be64_add_cpu(iv-b, 1);
if (!iv-b)

 - iv-a = cpu_to_be64(be64_to_cpu(iv-a) + 1);
 + be64_add_cpu(iv-a, 1);
  }
  
  static inline void lrw_round(struct sinfo *s, void *dst, const void *src)

--
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] ufs: [bl]e*_add_cpu conversion

[Roel Kluin]

[EMAIL PROTECTED] wrote:
 replace all:
 big/little_endian_variable = 
 cpu_to_[bl]eX([bl]eX_to_cpu(big/little_endian_variable) +
   expression_in_cpu_byteorder);
 with:
   [bl]eX_add_cpu(big/little_endian_variable, 
 expression_in_cpu_byteorder);

you may also want these:
---
[bl]e_add_cpu conversion in return

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/fs/ufs/swab.h b/fs/ufs/swab.h
index 1683d2b..a1e3000 100644
--- a/fs/ufs/swab.h
+++ b/fs/ufs/swab.h
@@ -44,18 +44,22 @@ static __inline u32
 fs64_add(struct super_block *sbp, u32 *n, int d)
 {
if (UFS_SB(sbp)-s_bytesex == BYTESEX_LE)
-   return *n = cpu_to_le64(le64_to_cpu(*n)+d);
+   le64_add_cpu(n, d);
else
-   return *n = cpu_to_be64(be64_to_cpu(*n)+d);
+   be64_add_cpu(n, d);
+
+   return *n;
 }
 
 static __inline u32
 fs64_sub(struct super_block *sbp, u32 *n, int d)
 {
if (UFS_SB(sbp)-s_bytesex == BYTESEX_LE)
-   return *n = cpu_to_le64(le64_to_cpu(*n)-d);
+   le64_add_cpu(n, -d);
else
-   return *n = cpu_to_be64(be64_to_cpu(*n)-d);
+   be64_add_cpu(n, -d);
+
+   return *n;
 }
 
 static __inline u32

--
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/

[drivers/video/sis/init301.c] SiS_Pr-ChipType = SIS_661 not evaluated twice

[Roel Kluin]

Not sure whether this is important, but in drivers/video/sis/init301.c:1557:

  if((SiS_Pr-ChipType = SIS_661) || (SiS_Pr-SiS_ROMNew)) {
 SiS_Pr-SiS_LCDTypeInfo = (SiS_GetReg(SiS_Pr-SiS_P3d4,0x39)  0x7c)  2;
  } else if((SiS_Pr-ChipType  SIS_315H) || (SiS_Pr-ChipType = SIS_661)) {
 SiS_Pr-SiS_LCDTypeInfo = temp  4;

note the duplicate test for 'SiS_Pr-ChipType = SIS_661'
if it was true in the first test, the second test won't be evaluated.
--
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: Announce: Linux-next (Or Andrew's dream :-))

[Roel Kluin]

Linus Torvalds wrote:
 
 On Tue, 12 Feb 2008, Greg KH wrote:
 That's the point.
 Not it isn't.  To quote you a number of years ago:
  Linux is evolution, not intelligent design
 
 Umm. Have you read a lot of books on evolution?
 
 It doesn't sound like you have.
 
 The fact is, evolution often does odd (and suboptimal) things exactly 
 because it does incremental changes that DO NOT BREAK at any point.

This is not entirely true if the pressure for changes are removed. For 
instance in mammals the bones in the ear are what used to be gills in fish.
When fish became amphibians the gills weren't needed as much and evolution
took a side path.

 The examples are legion. The mammalian eye has the retina backwards, 
 with the blind spot appearing because the fundmanetal infrastructure (the 
 optical nerves) actually being in *front* of the light sensor and needing 
 a hole in the retina to get the information (and blood flow) to go to the 
 brain!
 
 In other words, exactly *because* evolution requires bisectability (any 
 non-viable point in between is a dead end by definition) and does things 
 incrementally, it doesn't do big flips. It fixes the problems on an 
 incremental scale both when it comes to the details and when it comes to 
 both details (actual protein-coding genes that code directly for some 
 expression) and infrastructure (homeobox and non-coding genes).

In nature there is a lot of duplication: several copies of genes can exist
and different copies may have a distinct evolution. There is also a lot of
'junk' DNA that doesn't code for anything (although it may have regulating
functions). In there some copies of genes may remain that are inactivated,
as well as parts of virusses, slowly obtaining random mutations because
there is no pressure on the evolution of them. Some may eventually become
active again and have different functions.

The duplication also often ensures there is fallback when random mutations
are acquired and a protein is knocked out. Besides the two chromosomes
several proteins also can have overlapping functions. The result is more
like a balance. 
 
Evolution in nature and changes in code are different because in code junk
and bugs are constantly removed. In biology junk is allowed and may provide
a pool for future development. Linux development is intended and not
survival.
--
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?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent

[Roel Kluin]

John David Anglin wrote:
>> James Bottomley wrote:
>>> On Mon, 2008-02-11 at 17:23 +0100, Roel Kluin wrote:
>>>> duplicate pa11_dma_alloc_consistent; more appropriate appears
>>>> pa11_dma_alloc_noncoherent here. 
>>>>
>>>> Not tested, please confirm that this fix is correct
>>> No, it looks completely incorrect to me.  What makes you think a pcxl
>>> box has a problem with coherency?
>> Ok, please ignore the patch then. It just appeared suspicious to me
>> that the function did exist, but the names assigned were different.
> 
> How about a comment?

Based on James Bottomley's explanation maybe a comment like this?
---
Explain why dma_alloc_noncoherent is only used for boxes PA7200 and below

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 9448d4e..fc3325a 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -567,6 +567,10 @@ static void *fail_alloc_consistent(struct device *dev, 
size_t size,
return NULL;
 }
 
+/*
+ * dma_alloc_noncoherent is a fallback for boxes PA7200 and below which
+ * cannot allocate coherent memory.
+ */
 static void *pa11_dma_alloc_noncoherent(struct device *dev, size_t size,
  dma_addr_t *dma_handle, gfp_t flag)
 {
@@ -586,6 +590,10 @@ static void pa11_dma_free_noncoherent(struct device *dev, 
size_t size,
return;
 }
 
+/*
+ * PCXL allocates coherent memory even for dma_alloc_noncoherent() due to the
+ * uncached trick for coherent memory.
+ */
 struct hppa_dma_ops pcx_dma_ops = {
.dma_supported =pa11_dma_supported,
.alloc_consistent = fail_alloc_consistent,

--
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/

[PATCH][fs/cifs/cifsfs.c] Make use of cifs_xquota_get

[Roel Kluin]

Functions cifs_xquota_set and cifs_xquota_get at respectively
fs/cifs/cifsfs.c:367 and 392 are entirely similar - except for
whitespace

struct quotactl_ops contains function pointers .set_xquota and
.get_xquota that both get the address of cifs_xquota_set.
cifs_xquota_get isn't called anywhere else in the kernel.

The patch below makes use of the function cifs_xquota_get, As
an alternative the entire function cifs_xquota_get could be 
removed.
---
Make use of cifs_xquota_get

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index fcc4342..339b829 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -461,7 +461,7 @@ int cifs_xstate_get(struct super_block *sb, struct 
fs_quota_stat *qstats)
 
 static struct quotactl_ops cifs_quotactl_ops = {
.set_xquota = cifs_xquota_set,
-   .get_xquota = cifs_xquota_set,
+   .get_xquota = cifs_xquota_get,
.set_xstate = cifs_xstate_set,
.get_xstate = cifs_xstate_get,
 };
--
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?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?

[Roel Kluin]

James Bottomley wrote:
> On Mon, 2008-02-11 at 17:23 +0100, Roel Kluin wrote:
>> duplicate pa11_dma_alloc_consistent; more appropriate appears
>> pa11_dma_alloc_noncoherent here. 
>>
>> Not tested, please confirm that this fix is correct
> 
> No, it looks completely incorrect to me.  What makes you think a pcxl
> box has a problem with coherency?

Ok, please ignore the patch then. It just appeared suspicious to me
that the function did exist, but the names assigned were different.
--
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?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?

[Roel Kluin]

Matthew Wilcox wrote:
> On Mon, Feb 11, 2008 at 05:23:33PM +0100, Roel Kluin wrote:
>> duplicate pa11_dma_alloc_consistent; more appropriate appears
>> pa11_dma_alloc_noncoherent here. 
>>
>> Not tested, please confirm that this fix is correct
> 
> I don't think it is.  The memories are fading, so I don't recall why it
> is we do it this way, but I'm pretty sure it's correct the way it is.
> 
Maybe this helps a bit: later I found that something similar occurs in 
drivers/parisc/{ccio-dma.c, sba_iommu.c}:

1010: .alloc_consistent = ccio_alloc_consistent,
  .alloc_noncoherent =ccio_alloc_consistent,

1036: .alloc_consistent = sba_alloc_consistent,
  .alloc_noncoherent =sba_alloc_consistent,

However in these files the functions {ccio_alloc, sba_alloc}_noncoherent
do not exist.
--
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/

[PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?

[Roel Kluin]

duplicate pa11_dma_alloc_consistent; more appropriate appears
pa11_dma_alloc_noncoherent here. 

Not tested, please confirm that this fix is correct
---
fix noncoherent allocation

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 9448d4e..0e8b71f 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -549,7 +549,7 @@ static void pa11_dma_sync_sg_for_device(struct device *dev, 
struct scatterlist *
 struct hppa_dma_ops pcxl_dma_ops = {
.dma_supported =pa11_dma_supported,
.alloc_consistent = pa11_dma_alloc_consistent,
-   .alloc_noncoherent =pa11_dma_alloc_consistent,
+   .alloc_noncoherent =pa11_dma_alloc_noncoherent,
.free_consistent =  pa11_dma_free_consistent,
.map_single =   pa11_dma_map_single,
.unmap_single = pa11_dma_unmap_single,
--
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/

[PATCH][drivers/pnp/pnpacpi/core.c] __initdata is not an identifier

[Roel Kluin]

sparse complains at drivers/pnp/pnpacpi/core.c:39 with the error:
Trying to use reserved word '__attribute__' as identifier
Expected ) in function declarator, got ".init.data"

and at drivers/pnp/pnpacpi/core.c:49:38 with the error: 
undefined identifier 'excluded_id_list'

With the patch below these sparse complaints do not occur
---
__initdata is not an identifier

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index 662b4c2..c283a9a 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -36,7 +36,7 @@ static int num = 0;
  * have irqs (PIC, Timer) because we call acpi_register_gsi.
  * Finally, only devices that have a CRS method need to be in this list.
  */
-static struct __initdata acpi_device_id excluded_id_list[] = {
+static struct acpi_device_id excluded_id_list[] __initdata = {
{"PNP0C09", 0}, /* EC */
{"PNP0C0F", 0}, /* Link device */
{"PNP", 0}, /* PIC */
--
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?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?

[Roel Kluin]

James Bottomley wrote:
 On Mon, 2008-02-11 at 17:23 +0100, Roel Kluin wrote:
 duplicate pa11_dma_alloc_consistent; more appropriate appears
 pa11_dma_alloc_noncoherent here. 

 Not tested, please confirm that this fix is correct
 
 No, it looks completely incorrect to me.  What makes you think a pcxl
 box has a problem with coherency?

Ok, please ignore the patch then. It just appeared suspicious to me
that the function did exist, but the names assigned were different.
--
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/

[PATCH?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?

[Roel Kluin]

duplicate pa11_dma_alloc_consistent; more appropriate appears
pa11_dma_alloc_noncoherent here. 

Not tested, please confirm that this fix is correct
---
fix noncoherent allocation

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 9448d4e..0e8b71f 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -549,7 +549,7 @@ static void pa11_dma_sync_sg_for_device(struct device *dev, 
struct scatterlist *
 struct hppa_dma_ops pcxl_dma_ops = {
.dma_supported =pa11_dma_supported,
.alloc_consistent = pa11_dma_alloc_consistent,
-   .alloc_noncoherent =pa11_dma_alloc_consistent,
+   .alloc_noncoherent =pa11_dma_alloc_noncoherent,
.free_consistent =  pa11_dma_free_consistent,
.map_single =   pa11_dma_map_single,
.unmap_single = pa11_dma_unmap_single,
--
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?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent

[Roel Kluin]

John David Anglin wrote:
 James Bottomley wrote:
 On Mon, 2008-02-11 at 17:23 +0100, Roel Kluin wrote:
 duplicate pa11_dma_alloc_consistent; more appropriate appears
 pa11_dma_alloc_noncoherent here. 

 Not tested, please confirm that this fix is correct
 No, it looks completely incorrect to me.  What makes you think a pcxl
 box has a problem with coherency?
 Ok, please ignore the patch then. It just appeared suspicious to me
 that the function did exist, but the names assigned were different.
 
 How about a comment?

Based on James Bottomley's explanation maybe a comment like this?
---
Explain why dma_alloc_noncoherent is only used for boxes PA7200 and below

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/arch/parisc/kernel/pci-dma.c b/arch/parisc/kernel/pci-dma.c
index 9448d4e..fc3325a 100644
--- a/arch/parisc/kernel/pci-dma.c
+++ b/arch/parisc/kernel/pci-dma.c
@@ -567,6 +567,10 @@ static void *fail_alloc_consistent(struct device *dev, 
size_t size,
return NULL;
 }
 
+/*
+ * dma_alloc_noncoherent is a fallback for boxes PA7200 and below which
+ * cannot allocate coherent memory.
+ */
 static void *pa11_dma_alloc_noncoherent(struct device *dev, size_t size,
  dma_addr_t *dma_handle, gfp_t flag)
 {
@@ -586,6 +590,10 @@ static void pa11_dma_free_noncoherent(struct device *dev, 
size_t size,
return;
 }
 
+/*
+ * PCXL allocates coherent memory even for dma_alloc_noncoherent() due to the
+ * uncached trick for coherent memory.
+ */
 struct hppa_dma_ops pcx_dma_ops = {
.dma_supported =pa11_dma_supported,
.alloc_consistent = fail_alloc_consistent,

--
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/

[PATCH][fs/cifs/cifsfs.c] Make use of cifs_xquota_get

[Roel Kluin]

Functions cifs_xquota_set and cifs_xquota_get at respectively
fs/cifs/cifsfs.c:367 and 392 are entirely similar - except for
whitespace

struct quotactl_ops contains function pointers .set_xquota and
.get_xquota that both get the address of cifs_xquota_set.
cifs_xquota_get isn't called anywhere else in the kernel.

The patch below makes use of the function cifs_xquota_get, As
an alternative the entire function cifs_xquota_get could be 
removed.
---
Make use of cifs_xquota_get

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index fcc4342..339b829 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -461,7 +461,7 @@ int cifs_xstate_get(struct super_block *sb, struct 
fs_quota_stat *qstats)
 
 static struct quotactl_ops cifs_quotactl_ops = {
.set_xquota = cifs_xquota_set,
-   .get_xquota = cifs_xquota_set,
+   .get_xquota = cifs_xquota_get,
.set_xstate = cifs_xstate_set,
.get_xstate = cifs_xstate_get,
 };
--
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?][arch/parisc/kernel/pci-dma.c] pcxl_dma_ops.alloc_noncoherent = pa11_dma_alloc_consistent?

[Roel Kluin]

Matthew Wilcox wrote:
 On Mon, Feb 11, 2008 at 05:23:33PM +0100, Roel Kluin wrote:
 duplicate pa11_dma_alloc_consistent; more appropriate appears
 pa11_dma_alloc_noncoherent here. 

 Not tested, please confirm that this fix is correct
 
 I don't think it is.  The memories are fading, so I don't recall why it
 is we do it this way, but I'm pretty sure it's correct the way it is.
 
Maybe this helps a bit: later I found that something similar occurs in 
drivers/parisc/{ccio-dma.c, sba_iommu.c}:

1010: .alloc_consistent = ccio_alloc_consistent,
  .alloc_noncoherent =ccio_alloc_consistent,

1036: .alloc_consistent = sba_alloc_consistent,
  .alloc_noncoherent =sba_alloc_consistent,

However in these files the functions {ccio_alloc, sba_alloc}_noncoherent
do not exist.
--
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/

[PATCH][drivers/pnp/pnpacpi/core.c] __initdata is not an identifier

[Roel Kluin]

sparse complains at drivers/pnp/pnpacpi/core.c:39 with the error:
Trying to use reserved word '__attribute__' as identifier
Expected ) in function declarator, got .init.data

and at drivers/pnp/pnpacpi/core.c:49:38 with the error: 
undefined identifier 'excluded_id_list'

With the patch below these sparse complaints do not occur
---
__initdata is not an identifier

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index 662b4c2..c283a9a 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -36,7 +36,7 @@ static int num = 0;
  * have irqs (PIC, Timer) because we call acpi_register_gsi.
  * Finally, only devices that have a CRS method need to be in this list.
  */
-static struct __initdata acpi_device_id excluded_id_list[] = {
+static struct acpi_device_id excluded_id_list[] __initdata = {
{PNP0C09, 0}, /* EC */
{PNP0C0F, 0}, /* Link device */
{PNP, 0}, /* PIC */
--
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/

[PATCH] [arch/arm/mach-pnx4008/gpio.c] duplication in if ... else if branches

[Roel Kluin]

There is a duplication of code in the following section. Possibly it was
copy-pasted and it was forgotten to edit these lines? otherwise consider
removing the duplicate lines with the patch below.
---
Different if ... else if branches do the same, remove duplication

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/arm/mach-pnx4008/gpio.c b/arch/arm/mach-pnx4008/gpio.c
index 1ab84ce..7a3b190 100644
--- a/arch/arm/mach-pnx4008/gpio.c
+++ b/arch/arm/mach-pnx4008/gpio.c
@@ -122,16 +122,11 @@ int pnx4008_gpio_register_pin(unsigned short pin)
unsigned long bit = GPIO_BIT(pin);
int ret = -EBUSY;   /* Already in use */
 
gpio_lock();
 
-   if (GPIO_ISBID(pin)) {
-   if (access_map[GPIO_INDEX] & bit)
-   goto out;
-   access_map[GPIO_INDEX] |= bit;
-
-   } else if (GPIO_ISRAM(pin)) {
+   if (GPIO_ISBID(pin) || GPIO_ISRAM(pin)) {
if (access_map[GPIO_INDEX] & bit)
goto out;
access_map[GPIO_INDEX] |= bit;
 
} else if (GPIO_ISMUX(pin)) {
--
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/

[PATCH] [arch/arm/mach-pnx4008/gpio.c] duplication in if ... else if branches

[Roel Kluin]

There is a duplication of code in the following section. Possibly it was
copy-pasted and it was forgotten to edit these lines? otherwise consider
removing the duplicate lines with the patch below.
---
Different if ... else if branches do the same, remove duplication

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/arch/arm/mach-pnx4008/gpio.c b/arch/arm/mach-pnx4008/gpio.c
index 1ab84ce..7a3b190 100644
--- a/arch/arm/mach-pnx4008/gpio.c
+++ b/arch/arm/mach-pnx4008/gpio.c
@@ -122,16 +122,11 @@ int pnx4008_gpio_register_pin(unsigned short pin)
unsigned long bit = GPIO_BIT(pin);
int ret = -EBUSY;   /* Already in use */
 
gpio_lock();
 
-   if (GPIO_ISBID(pin)) {
-   if (access_map[GPIO_INDEX]  bit)
-   goto out;
-   access_map[GPIO_INDEX] |= bit;
-
-   } else if (GPIO_ISRAM(pin)) {
+   if (GPIO_ISBID(pin) || GPIO_ISRAM(pin)) {
if (access_map[GPIO_INDEX]  bit)
goto out;
access_map[GPIO_INDEX] |= bit;
 
} else if (GPIO_ISMUX(pin)) {
--
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/

[PATCH][drivers/usb/serial/ftdi_sio.c] add missing '|'

[Roel Kluin]

add missing '|'

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 90dcc62..2173561 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -394,7 +394,7 @@ static const char *ftdi_chip_name[] = {
 /* End TIOCMIWAIT */
 
 #define FTDI_IMPL_ASYNC_FLAGS = ( ASYNC_SPD_HI | ASYNC_SPD_VHI \
- ASYNC_SPD_CUST | ASYNC_SPD_SHI | ASYNC_SPD_WARP )
+ | ASYNC_SPD_CUST | ASYNC_SPD_SHI | ASYNC_SPD_WARP )
 
 /* function prototypes for a FTDI serial converter */
 static int  ftdi_sio_probe (struct usb_serial *serial, const struct 
usb_device_id *id);

--
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][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)

[Roel Kluin]

Greg KH wrote:
> On Tue, Feb 05, 2008 at 09:34:54AM +0100, Roel Kluin wrote:
>> Henrique de Moraes Holschuh wrote:
>>> On Tue, 05 Feb 2008, Roel Kluin wrote:
>>>> Roland Dreier wrote:

>>>>>  > Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
>>>>>  > this be replaced by
>>>>>
>>>>> Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
>>>>> (based on the comment).
>>>> Thanks Roland, for your info

>>> ACK.  This needs to be sent to stable as well.  I think both 2.6.22 and
>>> 2.6.23 need this patch.
>>>
>> CC [EMAIL PROTECTED]
> 
> Please send us a real copy of the patch, when it goes into Linus's tree,
> so we know what to apply, and that it is safe to do so.

It's a oneliner and the patch is from linus' tree.
---
in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad, a safety net for TPEC
fan control mode was added. Quoting that patch:

"The Linux ThinkPad community is not positive that all ThinkPads that do
HFSP EC fan control do implement full-speed and auto modes, some of the
earlier ones supporting HFSP might not.

If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
lower three bits that set the fan level. And as thinkpad-acpi was leaving
these set to zero, it would stop(!) the fan, which is Not A Good Thing.
So, as a safety net, we now make sure to also set the fan level part of the
HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
mode."

However, in the section below the test for the FULL-SPEED bits was not added:
the AUTO bits were tested twice. This patch corrects this and ensures that
the fan level part of the  HFSP register is set to a minimum of speed 4 for
auto mode.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index cf56647..3c323fe 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -4138,7 +4138,7 @@ static int fan_set_level(int level)
 * or FULLSPEED mode bits and just ignore them */
if (level & TP_EC_FAN_FULLSPEED)
level |= 7; /* safety min speed 7 */
-   else if (level & TP_EC_FAN_FULLSPEED)
+   else if (level & TP_EC_FAN_AUTO)
level |= 4; /* safety min speed 4 */
 
if (!acpi_ec_write(fan_status_offset, level))

--
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][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'

[Roel Kluin]

James Bottomley wrote:
> On Mon, 2008-02-04 at 23:36 +0100, Roel Kluin wrote:

>> Note the duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'

>> -   if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
>> +   if (SCpnt->sc_data_direction == DMA_TO_DEVICE) {
>>cpp->xdir = DTD_IN;
>>return;
>>}
>> else if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {

> 
> Well spotted, this is definitely a huge bug in the driver.
> 
> However, I think on closer examination that DTD_IN actually means
> transfer from device to host, so DMA_FROM_DEVICE is correct for that
> case.  It should be DMA_TO_DEVICE in the else if() piece.
> 
> Could you correct and resend the patch and I'll apply it.

Thanks for your review.
---
Direction of data transfer 'DMA_FROM_DEVICE' was tested twice. DTD_OUT
means  transfer from host to device. This should occur when the
direction of data transfer (sc_data_direction) is 'DMA_TO_DEVICE'.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
index 662c004..58d7eee 100644
--- a/drivers/scsi/u14-34f.c
+++ b/drivers/scsi/u14-34f.c
@@ -1215,9 +1215,9 @@ static void scsi_to_dev_dir(unsigned int i, unsigned int 
j) {
if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
   cpp->xdir = DTD_IN;
   return;
   }
-   else if (SCpnt->sc_data_direction == DMA_FROM_DEVICE) {
+   else if (SCpnt->sc_data_direction == DMA_TO_DEVICE) {
   cpp->xdir = DTD_OUT;
   return;
   }
else if (SCpnt->sc_data_direction == DMA_NONE) {

--
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/HP7XX] - Add combined LCD / BL driver for platform HP Jornada 7xx handhelds

[Roel Kluin]

Kristoffer Ericson wrote:
> Oki, here is attempt #2 (btw, new mail or just keep thread?)
> 
> Tested to build and checkpatch.
> 
> diff --git a/drivers/video/backlight/jornada720_bllcd.c 
> b/drivers/video/backlight/jornada720_bllcd.c

> +static int jornada_bl_update_status(struct backlight_device *dev)
> +{
> + int ret = 0;
> +
> + jornada_ssp_start();
> +
> + if (dev->props.power != FB_BLANK_UNBLANK ||
> + dev->props.fb_blank != FB_BLANK_UNBLANK) {
> + ret = jornada_ssp_inout(BRIGHTNESSOFF);
> + if (ret == -ETIMEDOUT)

since there is no curly bracket here...

> + printk(KERN_ERR "jornada720_bl: \
> + BrightnessOff timeout\n");

...indentation is wrong in lines below

> + /* backlight off */
> + PPSR &= ~PPC_LDD1;
> + PPDR |= PPC_LDD1;
> +
> + } else  {   /* backlight on */

too much indentation in lines below as well

> + PPSR |= PPC_LDD1;
> + if ((jornada_ssp_inout(SETBRIGHTNESS)) == TXDUMMY) {
> + /* send brightness value (0 is max, 255 lowest) */
> + if (jornada_ssp_byte(JORNADA_BL_MAX_BRIGHTNESS -
> +  dev->props.brightness) != -ETIMEDOUT)
> + ret = (JORNADA_BL_MAX_BRIGHTNESS -
> + dev->props.brightness);
> + } else
> + printk(KERN_ERR "jornada720_bl: \
> + SetBrightness timeout\n");
> + }
> + jornada_ssp_end();
> +
> + return ret;
> +}

> +static int jornada_lcd_set_contrast(struct lcd_device *pdev, int contrast)
> +{
> + int ret = 0;
> +
> + jornada_ssp_start();
> +
> + ret = jornada_ssp_inout(SETCONTRAST);
> +
> + if (ret == -ETIMEDOUT)
> + printk(KERN_ERR "jornada_lcd: failure to set contrast\n");
> +  else

whitespace before else

> + ret = jornada_ssp_byte(contrast);
> +
> + jornada_ssp_end();
> +
> + return ret;
> +}
> +
> +static int jornada_lcd_set_power(struct lcd_device  *pdev, int power)
> +{
> + if (power != FB_BLANK_UNBLANK) {
> + /* turn off LCD */
> + PPSR &= ~PPC_LDD2;
> + PPDR |= PPC_LDD2;
> + } else {
> + /* turn on LCD */
> + PPSR |= PPC_LDD2;
> + }

whitespace before curly bracket

> +
> + return 0;
> +}
--
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/HP7XX] - Add combined LCD / BL driver for platform HP Jornada 7xx handhelds

[Roel Kluin]

Roel Kluin wrote:
> Kristoffer Ericson wrote:
>> Greetings,
>>
>> Richard I've cleaned up the driver by checking with checkpatch.pl as Russell 
>> suggested. I would appreciate it if you could
>> look through the patch again and give comments since the patch looks 
>> somewhat different.
>>
>> I can add patch for Kconfig/Makefile later on but suspect there will be some 
>> changes required before that.
>>
>> Oh and to answer your question regarding MAX/MIN values of the backlight, 0 
>> = max and 255 = min. So we simply turn it around
>> by returning 255 - value. Same thing when we need to set value.
>>
>> Best wishes
>> Kristoffer Ericson
>>
>> diff --git a/drivers/video/backlight/jornada720_bllcd.c 
>> b/drivers/video/backlight/jornada720_bllcd.c

>> +static int jornada_bl_get_brightness(struct backlight_device *dev)
>> +{
>> +int ret;
>> +
>> +/* check if backlight is on */
>> +if (!(PPSR & PPC_LDD1))
>> +return 255;
> return JORNADA_BL_MAX_BRIGHTNESS;
> 
>> +
>> +jornada_ssp_start();
>> +if (jornada_ssp_inout(GETBRIGHTNESS) == -ETIMEDOUT) {
>> +printk(KERN_ERR "jornada720_bl: GetBrightness failed\n");
>> +ret = 256;
> ret = JORNADA_BL_MAX_BRIGHTNESS + 1;
>> +} else
>> +ret = jornada_ssp_inout(TXDUMMY);

forgot to mention missing curly bracket here

>> +
>> +jornada_ssp_end();
>> +
>> +/* 0 is max brightness */
>> +return (255 - ret);
> return (JORNADA_BL_MAX_BRIGHTNESS - ret);
>> +}
--
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/HP7XX] - Add combined LCD / BL driver for platform HP Jornada 7xx handhelds

[Roel Kluin]

Kristoffer Ericson wrote:
> Greetings,
> 
> Richard I've cleaned up the driver by checking with checkpatch.pl as Russell 
> suggested. I would appreciate it if you could
> look through the patch again and give comments since the patch looks somewhat 
> different.
> 
> I can add patch for Kconfig/Makefile later on but suspect there will be some 
> changes required before that.
> 
> Oh and to answer your question regarding MAX/MIN values of the backlight, 0 = 
> max and 255 = min. So we simply turn it around
> by returning 255 - value. Same thing when we need to set value.
> 
> Best wishes
> Kristoffer Ericson
> 
> diff --git a/drivers/video/backlight/jornada720_bllcd.c 
> b/drivers/video/backlight/jornada720_bllcd.c
> new file mode 100644
> index 000..4967b86
> --- /dev/null
> +++ b/drivers/video/backlight/jornada720_bllcd.c
> @@ -0,0 +1,284 @@
> +/*
> + * drivers/video/backlight/jornada720_bllcd.c
> + *
> + * Backlight and LCD Driver for HP Jornada 720
> + * Copyright (C) 2007 Kristoffer Ericson <[EMAIL PROTECTED]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * or any later version as published by the Free Software Foundation.
> + *
> + */
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +MODULE_AUTHOR("Kristoffer Ericson <[EMAIL PROTECTED]>");
> +MODULE_DESCRIPTION("HP Jornada 710/720/728 Backlight/LCD Driver");
> +MODULE_LICENSE("GPL");
> +
> +#define JORNADA_LCD_MAX_CONTRAST 0xff
> +#define JORNADA_LCD_DEFAULT_CONTRAST 0x80
> +#define JORNADA_BL_MAX_BRIGHTNESS0xff
> +#define JORNADA_BL_DEFAULT_BRIGHTNESS0x19
> +
> +struct jornada_bllcd_device {
> + struct backlight_device *jorn_backlight_device;
> + struct lcd_device *jorn_lcd_device;
> +};
> +
> +/*
> + * BACKLIGHT HANDLING ROUTINES
> + */
> +static int jornada_bl_get_brightness(struct backlight_device *dev)
> +{
> + int ret;
> +
> + /* check if backlight is on */
> + if (!(PPSR & PPC_LDD1))
> + return 255;
return JORNADA_BL_MAX_BRIGHTNESS;

> +
> + jornada_ssp_start();
> + if (jornada_ssp_inout(GETBRIGHTNESS) == -ETIMEDOUT) {
> + printk(KERN_ERR "jornada720_bl: GetBrightness failed\n");
> + ret = 256;
ret = JORNADA_BL_MAX_BRIGHTNESS + 1;
> + } else
> + ret = jornada_ssp_inout(TXDUMMY);
> +
> + jornada_ssp_end();
> +
> + /* 0 is max brightness */
> + return (255 - ret);
return (JORNADA_BL_MAX_BRIGHTNESS - ret);
> +}
> +
> +static int jornada_bl_update_status(struct backlight_device *dev)
> +{
> + int ret = 0;
> +
> + jornada_ssp_start();
> +
> + if (dev->props.power != FB_BLANK_UNBLANK ||
> + dev->props.fb_blank != FB_BLANK_UNBLANK) {
> + ret = jornada_ssp_inout(BRIGHTNESSOFF);
> + if (ret == -ETIMEDOUT)
> + printk(KERN_ERR "jornada720_bl:
> + BrightnessOff timeout\n");
> + else {
> + /* backlight off */
> + PPSR &= ~PPC_LDD1;
> + PPDR |= PPC_LDD1;
> + }
> + } else {/* backlight on */
> + PPSR |= PPC_LDD1;

missing curly bracket

> +
> + if ((jornada_ssp_inout(SETBRIGHTNESS)) == TXDUMMY) {
> + /* send brightness value (0 is max, 255 lowest) */
> + if (jornada_ssp_byte(255 - dev->props.brightness) != -ETIMEDOUT)
> + ret = (255 - dev->props.brightness);

similarly JORNADA_BL_MAX_BRIGHTNESS in the three lines above

> + } else
> + printk(KERN_ERR "jornada720_bl: SetBrightness timeout\n");
> + }
> +
> + jornada_ssp_end();
> +
> + return ret;
> +}
> +
> +/* LCD HANDLING FUNCTIONS
> +   == */
> +static int jornada_lcd_set_contrast(struct lcd_device *pdev, int contrast)
> +{
> + int ret = 0;
> +
> + jornada_ssp_start();
> +
> + ret = jornada_ssp_inout(SETCONTRAST);
> +
> + if (ret == -ETIMEDOUT)
> + printk(KERN_INFO "jornada_lcd: failure to set contrast\n");
> +  else
> + ret = jornada_ssp_byte(contrast);
> +
> + jornada_ssp_end();
> +
> + return ret;
> +}
> +
> +static int jornada_lcd_set_power(struct lcd_device  *pdev, int power)
> +{
> + if (power != FB_BLANK_UNBLANK) {
> + /* turn off LCD */
> + PPSR &= ~PPC_LDD2;
> + PPDR |= PPC_LDD2;
> + } else
> + /* turn on LCD */
> + PPSR |= PPC_LDD2;

wrong indent

> +
> + return 0;
> +}
> +
> +static int jornada_lcd_get_power(struct lcd_device *pdev)
> +{
> + if (PPSR & PPC_LDD2)
> + return FB_BLANK_UNBLANK;
> + else
> + return FB_BLANK_POWERDOWN;
> +}
> +
> +static int jornada_lcd_get_contrast(struct lcd_device *pdev)
> +{
> + int ret;
> +
> + /* Don't 

Re: [PATCH/HP7XX] - Add combined LCD / BL driver for platform HP Jornada 7xx handhelds

[Roel Kluin]

Roel Kluin wrote:
 Kristoffer Ericson wrote:
 Greetings,

 Richard I've cleaned up the driver by checking with checkpatch.pl as Russell 
 suggested. I would appreciate it if you could
 look through the patch again and give comments since the patch looks 
 somewhat different.

 I can add patch for Kconfig/Makefile later on but suspect there will be some 
 changes required before that.

 Oh and to answer your question regarding MAX/MIN values of the backlight, 0 
 = max and 255 = min. So we simply turn it around
 by returning 255 - value. Same thing when we need to set value.

 Best wishes
 Kristoffer Ericson

 diff --git a/drivers/video/backlight/jornada720_bllcd.c 
 b/drivers/video/backlight/jornada720_bllcd.c

 +static int jornada_bl_get_brightness(struct backlight_device *dev)
 +{
 +int ret;
 +
 +/* check if backlight is on */
 +if (!(PPSR  PPC_LDD1))
 +return 255;
 return JORNADA_BL_MAX_BRIGHTNESS;
 
 +
 +jornada_ssp_start();
 +if (jornada_ssp_inout(GETBRIGHTNESS) == -ETIMEDOUT) {
 +printk(KERN_ERR jornada720_bl: GetBrightness failed\n);
 +ret = 256;
 ret = JORNADA_BL_MAX_BRIGHTNESS + 1;
 +} else
 +ret = jornada_ssp_inout(TXDUMMY);

forgot to mention missing curly bracket here

 +
 +jornada_ssp_end();
 +
 +/* 0 is max brightness */
 +return (255 - ret);
 return (JORNADA_BL_MAX_BRIGHTNESS - ret);
 +}
--
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/HP7XX] - Add combined LCD / BL driver for platform HP Jornada 7xx handhelds

[Roel Kluin]

Kristoffer Ericson wrote:
 Greetings,
 
 Richard I've cleaned up the driver by checking with checkpatch.pl as Russell 
 suggested. I would appreciate it if you could
 look through the patch again and give comments since the patch looks somewhat 
 different.
 
 I can add patch for Kconfig/Makefile later on but suspect there will be some 
 changes required before that.
 
 Oh and to answer your question regarding MAX/MIN values of the backlight, 0 = 
 max and 255 = min. So we simply turn it around
 by returning 255 - value. Same thing when we need to set value.
 
 Best wishes
 Kristoffer Ericson
 
 diff --git a/drivers/video/backlight/jornada720_bllcd.c 
 b/drivers/video/backlight/jornada720_bllcd.c
 new file mode 100644
 index 000..4967b86
 --- /dev/null
 +++ b/drivers/video/backlight/jornada720_bllcd.c
 @@ -0,0 +1,284 @@
 +/*
 + * drivers/video/backlight/jornada720_bllcd.c
 + *
 + * Backlight and LCD Driver for HP Jornada 720
 + * Copyright (C) 2007 Kristoffer Ericson [EMAIL PROTECTED]
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2
 + * or any later version as published by the Free Software Foundation.
 + *
 + */
 +#include linux/module.h
 +#include linux/kernel.h
 +#include linux/init.h
 +#include linux/backlight.h
 +#include linux/lcd.h
 +#include linux/delay.h
 +#include linux/platform_device.h
 +#include linux/fb.h
 +#include linux/device.h
 +#include asm/hardware.h
 +#include asm/arch/jornada720.h
 +#include video/s1d13xxxfb.h
 +
 +MODULE_AUTHOR(Kristoffer Ericson [EMAIL PROTECTED]);
 +MODULE_DESCRIPTION(HP Jornada 710/720/728 Backlight/LCD Driver);
 +MODULE_LICENSE(GPL);
 +
 +#define JORNADA_LCD_MAX_CONTRAST 0xff
 +#define JORNADA_LCD_DEFAULT_CONTRAST 0x80
 +#define JORNADA_BL_MAX_BRIGHTNESS0xff
 +#define JORNADA_BL_DEFAULT_BRIGHTNESS0x19
 +
 +struct jornada_bllcd_device {
 + struct backlight_device *jorn_backlight_device;
 + struct lcd_device *jorn_lcd_device;
 +};
 +
 +/*
 + * BACKLIGHT HANDLING ROUTINES
 + */
 +static int jornada_bl_get_brightness(struct backlight_device *dev)
 +{
 + int ret;
 +
 + /* check if backlight is on */
 + if (!(PPSR  PPC_LDD1))
 + return 255;
return JORNADA_BL_MAX_BRIGHTNESS;

 +
 + jornada_ssp_start();
 + if (jornada_ssp_inout(GETBRIGHTNESS) == -ETIMEDOUT) {
 + printk(KERN_ERR jornada720_bl: GetBrightness failed\n);
 + ret = 256;
ret = JORNADA_BL_MAX_BRIGHTNESS + 1;
 + } else
 + ret = jornada_ssp_inout(TXDUMMY);
 +
 + jornada_ssp_end();
 +
 + /* 0 is max brightness */
 + return (255 - ret);
return (JORNADA_BL_MAX_BRIGHTNESS - ret);
 +}
 +
 +static int jornada_bl_update_status(struct backlight_device *dev)
 +{
 + int ret = 0;
 +
 + jornada_ssp_start();
 +
 + if (dev-props.power != FB_BLANK_UNBLANK ||
 + dev-props.fb_blank != FB_BLANK_UNBLANK) {
 + ret = jornada_ssp_inout(BRIGHTNESSOFF);
 + if (ret == -ETIMEDOUT)
 + printk(KERN_ERR jornada720_bl:
 + BrightnessOff timeout\n);
 + else {
 + /* backlight off */
 + PPSR = ~PPC_LDD1;
 + PPDR |= PPC_LDD1;
 + }
 + } else {/* backlight on */
 + PPSR |= PPC_LDD1;

missing curly bracket

 +
 + if ((jornada_ssp_inout(SETBRIGHTNESS)) == TXDUMMY) {
 + /* send brightness value (0 is max, 255 lowest) */
 + if (jornada_ssp_byte(255 - dev-props.brightness) != -ETIMEDOUT)
 + ret = (255 - dev-props.brightness);

similarly JORNADA_BL_MAX_BRIGHTNESS in the three lines above

 + } else
 + printk(KERN_ERR jornada720_bl: SetBrightness timeout\n);
 + }
 +
 + jornada_ssp_end();
 +
 + return ret;
 +}
 +
 +/* LCD HANDLING FUNCTIONS
 +   == */
 +static int jornada_lcd_set_contrast(struct lcd_device *pdev, int contrast)
 +{
 + int ret = 0;
 +
 + jornada_ssp_start();
 +
 + ret = jornada_ssp_inout(SETCONTRAST);
 +
 + if (ret == -ETIMEDOUT)
 + printk(KERN_INFO jornada_lcd: failure to set contrast\n);
 +  else
 + ret = jornada_ssp_byte(contrast);
 +
 + jornada_ssp_end();
 +
 + return ret;
 +}
 +
 +static int jornada_lcd_set_power(struct lcd_device  *pdev, int power)
 +{
 + if (power != FB_BLANK_UNBLANK) {
 + /* turn off LCD */
 + PPSR = ~PPC_LDD2;
 + PPDR |= PPC_LDD2;
 + } else
 + /* turn on LCD */
 + PPSR |= PPC_LDD2;

wrong indent

 +
 + return 0;
 +}
 +
 +static int jornada_lcd_get_power(struct lcd_device *pdev)
 +{
 + if (PPSR  PPC_LDD2)
 + return FB_BLANK_UNBLANK;
 + else
 + return FB_BLANK_POWERDOWN;
 +}
 +
 +static int jornada_lcd_get_contrast(struct lcd_device *pdev)
 +{
 + int ret;
 +
 + /* Don't set 

Re: [PATCH/HP7XX] - Add combined LCD / BL driver for platform HP Jornada 7xx handhelds

[Roel Kluin]

Kristoffer Ericson wrote:
 Oki, here is attempt #2 (btw, new mail or just keep thread?)
 
 Tested to build and checkpatch.
 
 diff --git a/drivers/video/backlight/jornada720_bllcd.c 
 b/drivers/video/backlight/jornada720_bllcd.c

 +static int jornada_bl_update_status(struct backlight_device *dev)
 +{
 + int ret = 0;
 +
 + jornada_ssp_start();
 +
 + if (dev-props.power != FB_BLANK_UNBLANK ||
 + dev-props.fb_blank != FB_BLANK_UNBLANK) {
 + ret = jornada_ssp_inout(BRIGHTNESSOFF);
 + if (ret == -ETIMEDOUT)

since there is no curly bracket here...

 + printk(KERN_ERR jornada720_bl: \
 + BrightnessOff timeout\n);

...indentation is wrong in lines below

 + /* backlight off */
 + PPSR = ~PPC_LDD1;
 + PPDR |= PPC_LDD1;
 +
 + } else  {   /* backlight on */

too much indentation in lines below as well

 + PPSR |= PPC_LDD1;
 + if ((jornada_ssp_inout(SETBRIGHTNESS)) == TXDUMMY) {
 + /* send brightness value (0 is max, 255 lowest) */
 + if (jornada_ssp_byte(JORNADA_BL_MAX_BRIGHTNESS -
 +  dev-props.brightness) != -ETIMEDOUT)
 + ret = (JORNADA_BL_MAX_BRIGHTNESS -
 + dev-props.brightness);
 + } else
 + printk(KERN_ERR jornada720_bl: \
 + SetBrightness timeout\n);
 + }
 + jornada_ssp_end();
 +
 + return ret;
 +}

 +static int jornada_lcd_set_contrast(struct lcd_device *pdev, int contrast)
 +{
 + int ret = 0;
 +
 + jornada_ssp_start();
 +
 + ret = jornada_ssp_inout(SETCONTRAST);
 +
 + if (ret == -ETIMEDOUT)
 + printk(KERN_ERR jornada_lcd: failure to set contrast\n);
 +  else

whitespace before else

 + ret = jornada_ssp_byte(contrast);
 +
 + jornada_ssp_end();
 +
 + return ret;
 +}
 +
 +static int jornada_lcd_set_power(struct lcd_device  *pdev, int power)
 +{
 + if (power != FB_BLANK_UNBLANK) {
 + /* turn off LCD */
 + PPSR = ~PPC_LDD2;
 + PPDR |= PPC_LDD2;
 + } else {
 + /* turn on LCD */
 + PPSR |= PPC_LDD2;
 + }

whitespace before curly bracket

 +
 + return 0;
 +}
--
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][drivers/scsi/u14-34f.c] duplicate test 'SCpnt-sc_data_direction == DMA_FROM_DEVICE'

[Roel Kluin]

James Bottomley wrote:
 On Mon, 2008-02-04 at 23:36 +0100, Roel Kluin wrote:

 Note the duplicate test 'SCpnt-sc_data_direction == DMA_FROM_DEVICE'

 -   if (SCpnt-sc_data_direction == DMA_FROM_DEVICE) {
 +   if (SCpnt-sc_data_direction == DMA_TO_DEVICE) {
cpp-xdir = DTD_IN;
return;
}
 else if (SCpnt-sc_data_direction == DMA_FROM_DEVICE) {

 
 Well spotted, this is definitely a huge bug in the driver.
 
 However, I think on closer examination that DTD_IN actually means
 transfer from device to host, so DMA_FROM_DEVICE is correct for that
 case.  It should be DMA_TO_DEVICE in the else if() piece.
 
 Could you correct and resend the patch and I'll apply it.

Thanks for your review.
---
Direction of data transfer 'DMA_FROM_DEVICE' was tested twice. DTD_OUT
means  transfer from host to device. This should occur when the
direction of data transfer (sc_data_direction) is 'DMA_TO_DEVICE'.

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/scsi/u14-34f.c b/drivers/scsi/u14-34f.c
index 662c004..58d7eee 100644
--- a/drivers/scsi/u14-34f.c
+++ b/drivers/scsi/u14-34f.c
@@ -1215,9 +1215,9 @@ static void scsi_to_dev_dir(unsigned int i, unsigned int 
j) {
if (SCpnt-sc_data_direction == DMA_FROM_DEVICE) {
   cpp-xdir = DTD_IN;
   return;
   }
-   else if (SCpnt-sc_data_direction == DMA_FROM_DEVICE) {
+   else if (SCpnt-sc_data_direction == DMA_TO_DEVICE) {
   cpp-xdir = DTD_OUT;
   return;
   }
else if (SCpnt-sc_data_direction == DMA_NONE) {

--
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][drivers/misc/thinkpad_acpi.c] duplicate test if (level TP_EC_FAN_FULLSPEED)

[Roel Kluin]

Greg KH wrote:
 On Tue, Feb 05, 2008 at 09:34:54AM +0100, Roel Kluin wrote:
 Henrique de Moraes Holschuh wrote:
 On Tue, 05 Feb 2008, Roel Kluin wrote:
 Roland Dreier wrote:

   Note the duplicate test 'if (level  TP_EC_FAN_FULLSPEED)'. should
   this be replaced by

 Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
 (based on the comment).
 Thanks Roland, for your info

 ACK.  This needs to be sent to stable as well.  I think both 2.6.22 and
 2.6.23 need this patch.

 CC [EMAIL PROTECTED]
 
 Please send us a real copy of the patch, when it goes into Linus's tree,
 so we know what to apply, and that it is safe to do so.

It's a oneliner and the patch is from linus' tree.
---
in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad, a safety net for TPEC
fan control mode was added. Quoting that patch:

The Linux ThinkPad community is not positive that all ThinkPads that do
HFSP EC fan control do implement full-speed and auto modes, some of the
earlier ones supporting HFSP might not.

If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
lower three bits that set the fan level. And as thinkpad-acpi was leaving
these set to zero, it would stop(!) the fan, which is Not A Good Thing.
So, as a safety net, we now make sure to also set the fan level part of the
HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
mode.

However, in the section below the test for the FULL-SPEED bits was not added:
the AUTO bits were tested twice. This patch corrects this and ensures that
the fan level part of the  HFSP register is set to a minimum of speed 4 for
auto mode.

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index cf56647..3c323fe 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -4138,7 +4138,7 @@ static int fan_set_level(int level)
 * or FULLSPEED mode bits and just ignore them */
if (level  TP_EC_FAN_FULLSPEED)
level |= 7; /* safety min speed 7 */
-   else if (level  TP_EC_FAN_FULLSPEED)
+   else if (level  TP_EC_FAN_AUTO)
level |= 4; /* safety min speed 4 */
 
if (!acpi_ec_write(fan_status_offset, level))

--
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/

[PATCH][drivers/usb/serial/ftdi_sio.c] add missing '|'

[Roel Kluin]

add missing '|'

Signed-off-by: Roel Kluin [EMAIL PROTECTED]
---
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 90dcc62..2173561 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -394,7 +394,7 @@ static const char *ftdi_chip_name[] = {
 /* End TIOCMIWAIT */
 
 #define FTDI_IMPL_ASYNC_FLAGS = ( ASYNC_SPD_HI | ASYNC_SPD_VHI \
- ASYNC_SPD_CUST | ASYNC_SPD_SHI | ASYNC_SPD_WARP )
+ | ASYNC_SPD_CUST | ASYNC_SPD_SHI | ASYNC_SPD_WARP )
 
 /* function prototypes for a FTDI serial converter */
 static int  ftdi_sio_probe (struct usb_serial *serial, const struct 
usb_device_id *id);

--
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][drivers/misc/thinkpad_acpi.c] duplicate test if (level & TP_EC_FAN_FULLSPEED)

[Roel Kluin]

Henrique de Moraes Holschuh wrote:
> On Tue, 05 Feb 2008, Roel Kluin wrote:
>> Roland Dreier wrote:
>>>  > /* safety net should the EC not support AUTO
>>>  >  * or FULLSPEED mode bits and just ignore them */
>>>  > if (level & TP_EC_FAN_FULLSPEED)
>>>  > level |= 7; /* safety min speed 7 */
>>>  > else if (level & TP_EC_FAN_FULLSPEED)
>>>  > level |= 4; /* safety min speed 4 */
>>>  > 
>>>  > Note the duplicate test 'if (level & TP_EC_FAN_FULLSPEED)'. should
>>>  > this be replaced by
>>>
>>> Actually I suspect one of the two tests should be against TP_EC_FAN_AUTO
>>> (based on the comment).
>> Thanks Roland, for your info
>>
>> based on the comments in commit eaa7571b2d1a08873e4bdd8e6db3431df61cd9ad,
>> I think this should be modified like below:
>>
>> ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode
>> The Linux ThinkPad community is not positive that all ThinkPads that do
>> HFSP EC fan control do implement full-speed and auto modes, some of the
>> earlier ones supporting HFSP might not.
>>
>> If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
>> lower three bits that set the fan level. And as thinkpad-acpi was leaving
>> these set to zero, it would stop(!) the fan, which is Not A Good Thing.
>> So, as a safety net, we now make sure to also set the fan level part of the
>> HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
>> mode.
>> --
>> second TP_EC_FAN_FULLSPEED should be P_EC_FAN_AUTO
>>
>>
>> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
>> ---
>> diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
>> index cf56647..3c323fe 100644
>> --- a/drivers/misc/thinkpad_acpi.c
>> +++ b/drivers/misc/thinkpad_acpi.c
>> @@ -4138,7 +4138,7 @@ static int fan_set_level(int level)
>>   * or FULLSPEED mode bits and just ignore them */
>>  if (level & TP_EC_FAN_FULLSPEED)
>>  level |= 7; /* safety min speed 7 */
>> -else if (level & TP_EC_FAN_FULLSPEED)
>> +else if (level & TP_EC_FAN_AUTO)
>>  level |= 4; /* safety min speed 4 */
>>  
>>  if (!acpi_ec_write(fan_status_offset, level))
> 
> ACK.  This needs to be sent to stable as well.  I think both 2.6.22 and
> 2.6.23 need this patch.
> 

CC [EMAIL PROTECTED]
--
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][drivers/scsi/u14-34f.c] duplicate test 'SCpnt->sc_data_direction == DMA_FROM_DEVICE'

[Roel Kluin]

[EMAIL PROTECTED] wrote:
>  Good to know that somebody still uses the Ultrastor 14f board :).
> Yes, this typo was introduced by somebody doing massive editing to all
> scsi drivers long ago.
> Cheers,
> --db
Actually, I do not own a Ultrastor 14f board. I found this by searching for
if (test)
...
else if (exactly same test)
...
Thanks,
Roel
--
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][drivers/scsi/u14-34f.c] duplicate test 'SCpnt-sc_data_direction == DMA_FROM_DEVICE'

[Roel Kluin]

[EMAIL PROTECTED] wrote:
  Good to know that somebody still uses the Ultrastor 14f board :).
 Yes, this typo was introduced by somebody doing massive editing to all
 scsi drivers long ago.
 Cheers,
 --db
Actually, I do not own a Ultrastor 14f board. I found this by searching for
if (test)
...
else if (exactly same test)
...
Thanks,
Roel
--
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/