[PATCH] macintosh: wrong test in fan_{read,write}_reg()

[roel kluin]

Fix error test in fan_{read,write}_reg()

Signed-off-by: Roel Kluin 
---
 drivers/macintosh/therm_pm72.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

ENODEV and EIO are positive (22 and 8), so the tests did not work.

diff --git a/drivers/macintosh/therm_pm72.c b/drivers/macintosh/therm_pm72.c
index 4454927..6256a08 100644
--- a/drivers/macintosh/therm_pm72.c
+++ b/drivers/macintosh/therm_pm72.c
@@ -443,7 +443,7 @@ static int fan_read_reg(int reg, unsigned char *buf, int nb)
tries = 0;
for (;;) {
nr = i2c_master_recv(fcu, buf, nb);
-   if (nr > 0 || (nr < 0 && nr != ENODEV) || tries >= 100)
+   if (nr > 0 || (nr < 0 && nr != -ENODEV) || tries >= 100)
break;
msleep(10);
++tries;
@@ -464,7 +464,7 @@ static int fan_write_reg(int reg, const unsigned char *ptr, 
int nb)
tries = 0;
for (;;) {
nw = i2c_master_send(fcu, buf, nb);
-   if (nw > 0 || (nw < 0 && nw != EIO) || tries >= 100)
+   if (nw > 0 || (nw < 0 && nw != -EIO) || tries >= 100)
break;
msleep(10);
++tries;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

powerpc: should mem_end be assigned to dt_struct_end in flatten_device_tree()?

[Roel Kluin]

vi arch/powerpc/kernel/prom_init.c +1961 and note that in
flatten_device_tree() we do a

RELOC(dt_struct_end) = PAGE_ALIGN(mem_start);

should that maybe be

RELOC(dt_struct_end) = PAGE_ALIGN(mem_end);

thanks,

Roel
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [Cbe-oss-dev] [PATCH] spufs: Fix test in spufs_switch_log_read()

[Roel Kluin]

size_t len cannot be less than 0.

Signed-off-by: Roel Kluin 
---
>>> Or can this test be removed?
>>
>> I'd prefer just to remove the test.
> 
> Yes, sounds good.

If you meant only the left-hand part, here you go:

diff --git a/arch/powerpc/platforms/cell/spufs/file.c 
b/arch/powerpc/platforms/cell/spufs/file.c
index 884e8bc..64a4c2d 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -2494,7 +2494,7 @@ static ssize_t spufs_switch_log_read(struct file *file, 
char __user *buf,
struct spu_context *ctx = SPUFS_I(inode)->i_ctx;
int error = 0, cnt = 0;
 
-   if (!buf || len < 0)
+   if (!buf)
return -EINVAL;
 
error = spu_acquire(ctx);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: Prevent unsigned wrap in htab_dt_scan_page_sizes()

[Roel Kluin]

Check to prevent unsigned wrap of size before subtraction.

Signed-off-by: Roel Kluin 
---
Is this maybe better or are we certain that size can't wrap?

diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index 1ade7eb..dd2d263 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -287,7 +287,7 @@ static int __init htab_dt_scan_page_sizes(unsigned long 
node,
DBG("Page sizes from device-tree:\n");
size /= 4;
cur_cpu_spec->cpu_features &= ~(CPU_FTR_16M_PAGE);
-   while(size > 0) {
+   while(size >= 3) {
unsigned int shift = prop[0];
unsigned int slbenc = prop[1];
unsigned int lpnum = prop[2];
@@ -296,7 +296,7 @@ static int __init htab_dt_scan_page_sizes(unsigned long 
node,
int idx = -1;
 
size -= 3; prop += 3;
-   while(size > 0 && lpnum) {
+   while(size >= 2 && lpnum) {
if (prop[0] == shift)
lpenc = prop[1];
prop += 2; size -= 2;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] spufs: Fix test in spufs_switch_log_read()

[Roel Kluin]

size_t len cannot be less than 0.

Signed-off-by: Roel Kluin 
---
Or can this test be removed?

diff --git a/arch/powerpc/platforms/cell/spufs/file.c 
b/arch/powerpc/platforms/cell/spufs/file.c
index 884e8bc..d4f304f 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -2494,7 +2494,7 @@ static ssize_t spufs_switch_log_read(struct file *file, 
char __user *buf,
struct spu_context *ctx = SPUFS_I(inode)->i_ctx;
int error = 0, cnt = 0;
 
-   if (!buf || len < 0)
+   if (!buf || (ssize_t)len < 0)
return -EINVAL;
 
error = spu_acquire(ctx);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: linux-next: tree build failure

[roel kluin]

On Tue, Sep 29, 2009 at 11:28 AM, Jan Beulich  wrote:
 Hollis Blanchard  09/29/09 2:00 AM >>>
>>First, I think there is a real bug here, and the code should read like
>>this (to match the comment):
>>    /* type has to be known at build time for optimization */
>>-    BUILD_BUG_ON(__builtin_constant_p(type));
>>+    BUILD_BUG_ON(!__builtin_constant_p(type));
>>
>>However, I get the same build error *both* ways, i.e.
>>__builtin_constant_p(type) evaluates to both 0 and 1? Either that, or
>>the new BUILD_BUG_ON() macro isn't working...
>
> No, at this point of the compilation process it's neither zero nor one,
> it's simply considered non-constant by the compiler at that stage
> (this builtin is used for optimization, not during parsing, and the
> error gets generated when the body of the function gets parsed,
> not when code gets generated from it).
>
> Jan

then maybe

if(__builtin_constant_p(type))
BUILD_BUG_ON(1);

would work?

Roel
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: kmalloc failure ignored in vio_build_iommu_table()

[Roel Kluin]

Prevent NULL dereference if kmalloc() fails.

Signed-off-by: Roel Kluin 
---
Found with sed: http://kernelnewbies.org/roelkluin

diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index 819e59f..1f5266f 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1054,6 +1054,8 @@ static struct iommu_table *vio_build_iommu_table(struct 
vio_dev *dev)
return NULL;
 
tbl = kmalloc(sizeof(*tbl), GFP_KERNEL);
+   if (tbl == NULL)
+   return NULL;
 
of_parse_dma_window(dev->dev.archdata.of_node, dma_window,
&tbl->it_index, &offset, &size);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] 82xx: kmalloc failure ignored in ep8248e_mdio_probe()

[Roel Kluin]

Prevent NULL dereference if kmalloc() fails. Also clean up if
of_mdiobus_register() returns an error.

Signed-off-by: Roel Kluin 
---
Found with sed: http://kernelnewbies.org/roelkluin

Please review.

diff --git a/arch/powerpc/platforms/82xx/ep8248e.c 
b/arch/powerpc/platforms/82xx/ep8248e.c
index 51fcae4..f9aee18 100644
--- a/arch/powerpc/platforms/82xx/ep8248e.c
+++ b/arch/powerpc/platforms/82xx/ep8248e.c
@@ -132,12 +132,25 @@ static int __devinit ep8248e_mdio_probe(struct of_device 
*ofdev,
return -ENOMEM;
 
bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
+   if (bus->irq == NULL) {
+   ret = -ENOMEM;
+   goto err_free_bus;
+   }
 
bus->name = "ep8248e-mdio-bitbang";
bus->parent = &ofdev->dev;
snprintf(bus->id, MII_BUS_ID_SIZE, "%x", res.start);
 
-   return of_mdiobus_register(bus, ofdev->node);
+   ret = of_mdiobus_register(bus, ofdev->node);
+   if (ret)
+   goto err_free_irq;
+
+   return 0;
+err_free_irq:
+   kfree(bus->irq);
+err_free_bus:
+   free_mdio_bitbang(bus);
+   return ret;
 }
 
 static int ep8248e_mdio_remove(struct of_device *ofdev)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: Read buffer overflow

[Roel Kluin]

Check whether index is within bounds before grabbing the element.

Signed-off-by: Roel Kluin 
---
diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
index a0f6838..588a5b0 100644
--- a/drivers/macintosh/macio_asic.c
+++ b/drivers/macintosh/macio_asic.c
@@ -294,10 +294,11 @@ static void macio_setup_interrupts(struct macio_dev *dev)
int i = 0, j = 0;
 
for (;;) {
-   struct resource *res = &dev->interrupt[j];
+   struct resource *res;
 
if (j >= MACIO_DEV_COUNT_IRQS)
break;
+   res = &dev->interrupt[j];
irq = irq_of_parse_and_map(np, i++);
if (irq == NO_IRQ)
break;
@@ -321,9 +322,10 @@ static void macio_setup_resources(struct macio_dev *dev,
int index;
 
for (index = 0; of_address_to_resource(np, index, &r) == 0; index++) {
-   struct resource *res = &dev->resource[index];
+   struct resource *res;
if (index >= MACIO_DEV_COUNT_RESOURCES)
break;
+   res = &dev->resource[index];
*res = r;
res->name = dev_name(&dev->ofdev.dev);
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc/fsl-booke: Read buffer overflow

[Roel Kluin]

cam[tlbcam_index] is checked before tlbcam_index < ARRAY_SIZE(cam)

Signed-off-by: Roel Kluin 
---
diff --git a/arch/powerpc/mm/fsl_booke_mmu.c b/arch/powerpc/mm/fsl_booke_mmu.c
index bb3d659..dc93e95 100644
--- a/arch/powerpc/mm/fsl_booke_mmu.c
+++ b/arch/powerpc/mm/fsl_booke_mmu.c
@@ -161,7 +161,7 @@ unsigned long __init mmu_mapin_ram(void)
unsigned long virt = PAGE_OFFSET;
phys_addr_t phys = memstart_addr;
 
-   while (cam[tlbcam_index] && tlbcam_index < ARRAY_SIZE(cam)) {
+   while (tlbcam_index < ARRAY_SIZE(cam) && cam[tlbcam_index]) {
settlbcam(tlbcam_index, virt, phys, cam[tlbcam_index], 
PAGE_KERNEL_X, 0);
virt += cam[tlbcam_index];
phys += cam[tlbcam_index];
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc: missing tests after ioremap()?

[Roel Kluin]

Missing tests after ioremap()

Signed-off-by: Roel Kluin 
---
Shouldn't we test whether ioremaps fail?

diff --git a/arch/powerpc/platforms/powermac/feature.c 
b/arch/powerpc/platforms/powermac/feature.c
index e6c0040..3a4ebd3 100644
--- a/arch/powerpc/platforms/powermac/feature.c
+++ b/arch/powerpc/platforms/powermac/feature.c
@@ -2589,9 +2589,16 @@ static void __init probe_uninorth(void)
if (address == 0)
return;
uninorth_base = ioremap(address, 0x4);
+   if (uninorth_base == NULL)
+   return;
uninorth_rev = in_be32(UN_REG(UNI_N_VERSION));
-   if (uninorth_maj == 3 || uninorth_maj == 4)
+   if (uninorth_maj == 3 || uninorth_maj == 4) {
u3_ht_base = ioremap(address + U3_HT_CONFIG_BASE, 0x1000);
+   if (u3_ht_base == NULL) {
+   iounmap(uninorth_base);
+   return;
+   }
+   }
 
printk(KERN_INFO "Found %s memory controller & host bridge"
   " @ 0x%08x revision: 0x%02x\n", uninorth_maj == 3 ? "U3" :
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc/cell: replace strncpy by strlcpy

[Roel Kluin]

Replace strncpy() and explicit null-termination by strlcpy()

Signed-off-by: Roel Kluin 
---
Arnd-san, Ken-san,

Thanks for reviewing,

> We prefer to take the patch which is replacing the two lines with one.

Doozo.

diff --git a/arch/powerpc/platforms/cell/celleb_setup.c 
b/arch/powerpc/platforms/cell/celleb_setup.c
index 07c234f..e538455 100644
--- a/arch/powerpc/platforms/cell/celleb_setup.c
+++ b/arch/powerpc/platforms/cell/celleb_setup.c
@@ -80,8 +80,7 @@ static void celleb_show_cpuinfo(struct seq_file *m)
 
 static int __init celleb_machine_type_hack(char *ptr)
 {
-   strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
-   celleb_machine_type[sizeof(celleb_machine_type)-1] = 0;
+   strlcpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
return 0;
 }
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/cell: strncpy does not null terminate string

[roel kluin]

>> With `sizeof(string) - 1` strncpy() will null terminate the string.
>
> No, it won't.

> See the line after the strncpy. This is still required for proper 
> zero-termination.

You're right, sorry for the noise.

Roel
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/cell: strncpy does not null terminate string

[Roel Kluin]

strlcpy() will always null terminate the string.

Signed-off-by: Roel Kluin 
---
Please use this one instead

diff --git a/arch/powerpc/platforms/cell/celleb_setup.c 
b/arch/powerpc/platforms/cell/celleb_setup.c
index 07c234f..1896cd8 100644
--- a/arch/powerpc/platforms/cell/celleb_setup.c
+++ b/arch/powerpc/platforms/cell/celleb_setup.c
@@ -80,7 +80,7 @@ static void celleb_show_cpuinfo(struct seq_file *m)
 
 static int __init celleb_machine_type_hack(char *ptr)
 {
-   strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
+   strlcpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
celleb_machine_type[sizeof(celleb_machine_type)-1] = 0;
return 0;
 }
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc/cell: strncpy does not null terminate string

[Roel Kluin]

With `sizeof(string) - 1` strncpy() will null terminate the string.

Signed-off-by: Roel Kluin 
---
To test this:

#include 
#include 

char a[10];
char b[10];

int main()
{
const char* str = "0123456789012";
strncpy(a, str, sizeof(a));
strncpy(b, str, sizeof(b) - 1);
printf("String a was %s, b was %s\n", a, b);

return 0;
}

Output:
String a was 0123456789012345678, b was 012345678

diff --git a/arch/powerpc/platforms/cell/celleb_setup.c 
b/arch/powerpc/platforms/cell/celleb_setup.c
index 07c234f..cfdbadb 100644
--- a/arch/powerpc/platforms/cell/celleb_setup.c
+++ b/arch/powerpc/platforms/cell/celleb_setup.c
@@ -80,7 +80,7 @@ static void celleb_show_cpuinfo(struct seq_file *m)
 
 static int __init celleb_machine_type_hack(char *ptr)
 {
-   strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
+   strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type) - 1);
celleb_machine_type[sizeof(celleb_machine_type)-1] = 0;
return 0;
 }
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] spufs: remove redundant test on unsigned

[Roel Kluin]

Unsigned `len' cannot be less than 0.

Signed-off-by: Roel Kluin 
---
Or should it be `if (!buf || len > MAX)' and what should MAX be then?

diff --git a/arch/powerpc/platforms/cell/spufs/sputrace.c 
b/arch/powerpc/platforms/cell/spufs/sputrace.c
index d0b1f3f..8f799ee 100644
--- a/arch/powerpc/platforms/cell/spufs/sputrace.c
+++ b/arch/powerpc/platforms/cell/spufs/sputrace.c
@@ -73,7 +73,7 @@ static ssize_t sputrace_read(struct file *file, char __user 
*buf,
 {
int error = 0, cnt = 0;
 
-   if (!buf || len < 0)
+   if (!buf)
return -EINVAL;
 
while (cnt < len) {
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] Sky CPU: redundant or incorrect tests on unsigned

[Roel Kluin]

count is unsigned and cannot be less than 0.

Signed-off-by: Roel Kluin 
---
Can these be removed or do we need an `if (count > MAX)' test, and
what should MAX be then?

diff --git a/drivers/misc/hdpuftrs/hdpu_cpustate.c 
b/drivers/misc/hdpuftrs/hdpu_cpustate.c
index 176fe4e..35000cf 100644
--- a/drivers/misc/hdpuftrs/hdpu_cpustate.c
+++ b/drivers/misc/hdpuftrs/hdpu_cpustate.c
@@ -121,8 +121,6 @@ static ssize_t cpustate_read(struct file *file, char *buf,
 {
unsigned char data;
 
-   if (count < 0)
-   return -EFAULT;
if (count == 0)
return 0;
 
@@ -137,9 +135,6 @@ static ssize_t cpustate_write(struct file *file, const char 
*buf,
 {
unsigned char data;
 
-   if (count < 0)
-   return -EFAULT;
-
if (count == 0)
return 0;
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] KVM: powerpc: beyond ARRAY_SIZE of vcpu->arch.guest_tlb

[Roel Kluin]

Do not go beyond ARRAY_SIZE of vcpu->arch.guest_tlb

Signed-off-by: Roel Kluin 
---
diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c
index 0fce4fb..c2cfd46 100644
--- a/arch/powerpc/kvm/emulate.c
+++ b/arch/powerpc/kvm/emulate.c
@@ -125,7 +125,7 @@ static int kvmppc_emul_tlbwe(struct kvm_vcpu *vcpu, u32 
inst)
ws = get_ws(inst);
 
index = vcpu->arch.gpr[ra];
-   if (index > PPC44x_TLB_SIZE) {
+   if (index >= PPC44x_TLB_SIZE) {
printk("%s: index %d\n", __func__, index);
kvmppc_dump_vcpu(vcpu);
return EMULATE_FAIL;

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] powerpc:beyond ARRAY_SIZE of args.args

[Roel Kluin]

Do not go beyond ARRAY_SIZE of args.args

Signed-off-by: Roel Kluin 
---
I'm quite sure the first is correct, but should maybe `args.nret'
and `nargs + args.nret' also be `>= ARRAY_SIZE(args.args)'?

diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 1f8505c..c94ab76 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -779,7 +779,7 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
return -EFAULT;
 
nargs = args.nargs;
-   if (nargs > ARRAY_SIZE(args.args)
+   if (nargs >= ARRAY_SIZE(args.args)
|| args.nret > ARRAY_SIZE(args.args)
|| nargs + args.nret > ARRAY_SIZE(args.args))
return -EINVAL;

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] tape: beyond ARRAY_SIZE of viocd_diskinfo

[Roel Kluin]

Do not go beyond ARRAY_SIZE of tape_device and viotape_unitinfo

Signed-off-by: Roel Kluin 
Acked-by: Stephen Rothwell 
---
diff --git a/drivers/char/viotape.c b/drivers/char/viotape.c
index ffc9254..042c814 100644
--- a/drivers/char/viotape.c
+++ b/drivers/char/viotape.c
@@ -867,7 +867,7 @@ static int viotape_probe(struct vio_dev *vdev, const struct 
vio_device_id *id)
int j;
struct device_node *node = vdev->dev.archdata.of_node;
 
-   if (i > VIOTAPE_MAX_TAPE)
+   if (i >= VIOTAPE_MAX_TAPE)
return -ENODEV;
if (!node)
return -ENODEV;

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] powerpc/5200: beyond ARRAY_SIZE of mpc52xx_uart_{ports, nodes}

[Roel Kluin]

Do not go beyond ARRAY_SIZE of mpc52xx_uart_{ports,nodes}

Signed-off-by: Roel Kluin 
---
diff --git a/drivers/serial/mpc52xx_uart.c b/drivers/serial/mpc52xx_uart.c
index 7f72f8c..b3feb61 100644
--- a/drivers/serial/mpc52xx_uart.c
+++ b/drivers/serial/mpc52xx_uart.c
@@ -988,7 +988,7 @@ mpc52xx_console_setup(struct console *co, char *options)
pr_debug("mpc52xx_console_setup co=%p, co->index=%i, options=%s\n",
 co, co->index, options);
 
-   if ((co->index < 0) || (co->index > MPC52xx_PSC_MAXNUM)) {
+   if ((co->index < 0) || (co->index >= MPC52xx_PSC_MAXNUM)) {
pr_debug("PSC%x out of range\n", co->index);
return -EINVAL;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] aoa: remove driver_data direct access of struct device

[Roel Kluin]

To avoid direct access to the driver_data pointer in struct device, the
functions dev_get_drvdata() and dev_set_drvdata() should be used.

Signed-off-by: Roel Kluin 
---
diff --git a/sound/aoa/fabrics/layout.c b/sound/aoa/fabrics/layout.c
index fbf5c93..586965f 100644
--- a/sound/aoa/fabrics/layout.c
+++ b/sound/aoa/fabrics/layout.c
@@ -1037,7 +1037,7 @@ static int aoa_fabric_layout_probe(struct soundbus_dev 
*sdev)
}
ldev->selfptr_headphone.ptr = ldev;
ldev->selfptr_lineout.ptr = ldev;
-   sdev->ofdev.dev.driver_data = ldev;
+   dev_set_drvdata(&sdev->ofdev.dev, ldev);
list_add(&ldev->list, &layouts_list);
layouts_list_items++;
 
@@ -1081,7 +1081,7 @@ static int aoa_fabric_layout_probe(struct soundbus_dev 
*sdev)
 
 static int aoa_fabric_layout_remove(struct soundbus_dev *sdev)
 {
-   struct layout_dev *ldev = sdev->ofdev.dev.driver_data;
+   struct layout_dev *ldev = dev_get_drvdata(&sdev->ofdev.dev);
int i;
 
for (i=0; iofdev.dev.driver_data;
+   struct layout_dev *ldev = dev_get_drvdata(&sdev->ofdev.dev);
 
if (ldev->gpio.methods && ldev->gpio.methods->all_amps_off)
ldev->gpio.methods->all_amps_off(&ldev->gpio);
@@ -1124,7 +1124,7 @@ static int aoa_fabric_layout_suspend(struct soundbus_dev 
*sdev, pm_message_t sta
 
 static int aoa_fabric_layout_resume(struct soundbus_dev *sdev)
 {
-   struct layout_dev *ldev = sdev->ofdev.dev.driver_data;
+   struct layout_dev *ldev = dev_get_drvdata(&sdev->ofdev.dev);
 
if (ldev->gpio.methods && ldev->gpio.methods->all_amps_off)
ldev->gpio.methods->all_amps_restore(&ldev->gpio);
diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index 418c84c..4e3b819 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -358,14 +358,14 @@ static int i2sbus_probe(struct macio_dev* dev, const 
struct of_device_id *match)
return -ENODEV;
}
 
-   dev->ofdev.dev.driver_data = control;
+   dev_set_drvdata(&dev->ofdev.dev, control);
 
return 0;
 }
 
 static int i2sbus_remove(struct macio_dev* dev)
 {
-   struct i2sbus_control *control = dev->ofdev.dev.driver_data;
+   struct i2sbus_control *control = dev_get_drvdata(&dev->ofdev.dev);
struct i2sbus_dev *i2sdev, *tmp;
 
list_for_each_entry_safe(i2sdev, tmp, &control->list, item)
@@ -377,7 +377,7 @@ static int i2sbus_remove(struct macio_dev* dev)
 #ifdef CONFIG_PM
 static int i2sbus_suspend(struct macio_dev* dev, pm_message_t state)
 {
-   struct i2sbus_control *control = dev->ofdev.dev.driver_data;
+   struct i2sbus_control *control = dev_get_drvdata(&dev->ofdev.dev);
struct codec_info_item *cii;
struct i2sbus_dev* i2sdev;
int err, ret = 0;
@@ -407,7 +407,7 @@ static int i2sbus_suspend(struct macio_dev* dev, 
pm_message_t state)
 
 static int i2sbus_resume(struct macio_dev* dev)
 {
-   struct i2sbus_control *control = dev->ofdev.dev.driver_data;
+   struct i2sbus_control *control = dev_get_drvdata(&dev->ofdev.dev);
struct codec_info_item *cii;
struct i2sbus_dev* i2sdev;
int err, ret = 0;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] ps3: remove driver_data direct access of struct device

[Roel Kluin]

To avoid direct access to the driver_data pointer in struct device, the
functions dev_get_drvdata() and dev_set_drvdata() should be used.

Signed-off-by: Roel Kluin 
---
Please review. especially note that I removed a

kfree(dev->sbd.core.driver_data);

Is that correct?

 arch/powerpc/include/asm/ps3.h |4 ++--
 drivers/char/ps3flash.c|   11 +--
 drivers/scsi/ps3rom.c  |   10 +-
 drivers/video/ps3fb.c  |6 +++---
 4 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
index cdb6fd8..a55717e 100644
--- a/arch/powerpc/include/asm/ps3.h
+++ b/arch/powerpc/include/asm/ps3.h
@@ -421,12 +421,12 @@ static inline struct ps3_system_bus_driver *
 static inline void ps3_system_bus_set_driver_data(
struct ps3_system_bus_device *dev, void *data)
 {
-   dev->core.driver_data = data;
+   dev_set_drvdata(&dev->core, data);
 }
 static inline void *ps3_system_bus_get_driver_data(
struct ps3_system_bus_device *dev)
 {
-   return dev->core.driver_data;
+   return dev_get_drvdata(&dev->core);
 }
 
 /* These two need global scope for get_dma_ops(). */
diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
index afbe456..6083032 100644
--- a/drivers/char/ps3flash.c
+++ b/drivers/char/ps3flash.c
@@ -108,7 +108,7 @@ static ssize_t ps3flash_read(struct file *file, char __user 
*buf, size_t count,
 loff_t *pos)
 {
struct ps3_storage_device *dev = ps3flash_dev;
-   struct ps3flash_private *priv = dev->sbd.core.driver_data;
+   struct ps3flash_private *priv = dev_get_drvdata(&dev->sbd.core);
u64 size, start_sector, end_sector, offset;
ssize_t sectors_read;
size_t remaining, n;
@@ -173,7 +173,7 @@ static ssize_t ps3flash_write(struct file *file, const char 
__user *buf,
  size_t count, loff_t *pos)
 {
struct ps3_storage_device *dev = ps3flash_dev;
-   struct ps3flash_private *priv = dev->sbd.core.driver_data;
+   struct ps3flash_private *priv = dev_get_drvdata(&dev->sbd.core);
u64 size, chunk_sectors, start_write_sector, end_write_sector,
end_read_sector, start_read_sector, head, tail, offset;
ssize_t res;
@@ -366,7 +366,7 @@ static int __devinit ps3flash_probe(struct 
ps3_system_bus_device *_dev)
goto fail;
}
 
-   dev->sbd.core.driver_data = priv;
+   dev_set_drvdata(&dev->sbd.core, priv);
mutex_init(&priv->mutex);
 
dev->bounce_size = ps3flash_bounce_buffer.size;
@@ -392,7 +392,7 @@ fail_teardown:
ps3stor_teardown(dev);
 fail_free_priv:
kfree(priv);
-   dev->sbd.core.driver_data = NULL;
+   dev_set_drvdata(&dev->sbd.core, NULL);
 fail:
ps3flash_dev = NULL;
return error;
@@ -404,8 +404,7 @@ static int ps3flash_remove(struct ps3_system_bus_device 
*_dev)
 
misc_deregister(&ps3flash_misc);
ps3stor_teardown(dev);
-   kfree(dev->sbd.core.driver_data);
-   dev->sbd.core.driver_data = NULL;
+   dev_set_drvdata(&dev->sbd.core, NULL);
ps3flash_dev = NULL;
return 0;
 }
diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index ca0dd33..f2f840a 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -299,7 +299,7 @@ static irqreturn_t ps3rom_interrupt(int irq, void *data)
return IRQ_HANDLED;
}
 
-   host = dev->sbd.core.driver_data;
+   host = dev_get_drvdata(&dev->sbd.core);
priv = shost_priv(host);
cmd = priv->curr_cmd;
 
@@ -387,7 +387,7 @@ static int __devinit ps3rom_probe(struct 
ps3_system_bus_device *_dev)
}
 
priv = shost_priv(host);
-   dev->sbd.core.driver_data = host;
+   dev_set_drvdata(&dev->sbd.core, host);
priv->dev = dev;
 
/* One device/LUN per SCSI bus */
@@ -407,7 +407,7 @@ static int __devinit ps3rom_probe(struct 
ps3_system_bus_device *_dev)
 
 fail_host_put:
scsi_host_put(host);
-   dev->sbd.core.driver_data = NULL;
+   dev_set_drvdata(&dev->sbd.core, NULL);
 fail_teardown:
ps3stor_teardown(dev);
 fail_free_bounce:
@@ -418,12 +418,12 @@ fail_free_bounce:
 static int ps3rom_remove(struct ps3_system_bus_device *_dev)
 {
struct ps3_storage_device *dev = to_ps3_storage_device(&_dev->core);
-   struct Scsi_Host *host = dev->sbd.core.driver_data;
+   struct Scsi_Host *host = dev_get_drvdata(&dev->sbd.core);
 
scsi_remove_host(host);
ps3stor_teardown(dev);
scsi_host_put(host);
-   dev->sbd.core.driver_data = NULL;
+   dev_set_drvdata(&dev->sbd.core, NULL);
kfree(dev->bounce_buf);
return 0;
 }
diff --git a/drivers/video/ps3fb.c b/drivers/video

[PATCH] powerpc/spufs: negative size in spufs_{regs/fpcr}_write

[Roel Kluin]

When stored in size_t size, the test 'size <= 0' does no longer work.

Signed-off-by: Roel Kluin 
---
diff --git a/arch/powerpc/platforms/cell/spufs/file.c 
b/arch/powerpc/platforms/cell/spufs/file.c
index 0da7f2b..05dba47 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -569,7 +569,7 @@ spufs_regs_write(struct file *file, const char __user 
*buffer,
int ret;
 
size = min_t(ssize_t, sizeof lscsa->gprs - *pos, size);
-   if (size <= 0)
+   if ((ssize_t)size <= 0)
return -EFBIG;
*pos += size;
 
@@ -624,7 +624,7 @@ spufs_fpcr_write(struct file *file, const char __user * 
buffer,
int ret;
 
size = min_t(ssize_t, sizeof(lscsa->fpcr) - *pos, size);
-   if (size <= 0)
+   if ((ssize_t)size <= 0)
return -EFBIG;
 
ret = spu_acquire_saved(ctx);
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] PS3 ps3av_set_video_mode() make id signed

[Roel Kluin]

>> make id signed so a negative id will get noticed
> 
> Thanks for noticing! It looks OK, except...

> 1. You forgot to update the forward declaration of ps3av_set_video_mode() in
>arch/powerpc/include/asm/ps3av.h (did you compile this succesfully?)

fixed that (and no, sorry, I didn't compile test it, I have to focus a bit on
a biotechnology exam, maybe later).

> 2. You forgot to change `u32 id' in ps3av_probe().

ok, changed it. but I am not sure whether the last two hunks are ok. Should
we error out like this or just let the negative value be assigned to
ps3av->ps3av_mode? If not, is there more cleanup required?

> Can you please make these changes, too? Thx again!

No problem.

->8-8<
Make id signed so a negative id will get noticed. Error out if
ps3av_auto_videomode() fails.


Signed-off-by: Roel Kluin 
---
 arch/powerpc/include/asm/ps3av.h |2 +-
 drivers/ps3/ps3av.c  |   16 
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/ps3av.h b/arch/powerpc/include/asm/ps3av.h
index cd24ac1..0427b0b 100644
--- a/arch/powerpc/include/asm/ps3av.h
+++ b/arch/powerpc/include/asm/ps3av.h
@@ -730,7 +730,7 @@ extern int ps3av_cmd_av_get_hw_conf(struct 
ps3av_pkt_av_get_hw_conf *);
 extern int ps3av_cmd_video_get_monitor_info(struct 
ps3av_pkt_av_get_monitor_info *,
u32);
 
-extern int ps3av_set_video_mode(u32);
+extern int ps3av_set_video_mode(int);
 extern int ps3av_set_audio_mode(u32, u32, u32, u32, u32);
 extern int ps3av_get_auto_mode(void);
 extern int ps3av_get_mode(void);
diff --git a/drivers/ps3/ps3av.c b/drivers/ps3/ps3av.c
index 5324978..235e87f 100644
--- a/drivers/ps3/ps3av.c
+++ b/drivers/ps3/ps3av.c
@@ -838,7 +838,7 @@ static int ps3av_get_hw_conf(struct ps3av *ps3av)
 }
 
 /* set mode using id */
-int ps3av_set_video_mode(u32 id)
+int ps3av_set_video_mode(int id)
 {
int size;
u32 option;
@@ -940,7 +940,7 @@ EXPORT_SYMBOL_GPL(ps3av_audio_mute);
 static int ps3av_probe(struct ps3_system_bus_device *dev)
 {
int res;
-   u32 id;
+   int id;
 
dev_dbg(&dev->core, " -> %s:%d\n", __func__, __LINE__);
dev_dbg(&dev->core, "  timeout=%d\n", timeout);
@@ -962,8 +962,10 @@ static int ps3av_probe(struct ps3_system_bus_device *dev)
init_completion(&ps3av->done);
complete(&ps3av->done);
ps3av->wq = create_singlethread_workqueue("ps3avd");
-   if (!ps3av->wq)
+   if (!ps3av->wq) {
+   res = -ENOMEM;
goto fail;
+   }
 
switch (ps3_os_area_get_av_multi_out()) {
case PS3_PARAM_AV_MULTI_OUT_NTSC:
@@ -994,6 +996,12 @@ static int ps3av_probe(struct ps3_system_bus_device *dev)
safe_mode = 1;
 #endif /* CONFIG_FB */
id = ps3av_auto_videomode(&ps3av->av_hw_conf);
+   if (id < 0) {
+   printk(KERN_ERR "%s: invalid id :%d\n", __func__, id);
+   res = -EINVAL;
+   goto fail;
+   }
+
safe_mode = 0;
 
mutex_lock(&ps3av->mutex);
@@ -1007,7 +1015,7 @@ static int ps3av_probe(struct ps3_system_bus_device *dev)
 fail:
kfree(ps3av);
ps3av = NULL;
-   return -ENOMEM;
+   return res;
 }
 
 static int ps3av_remove(struct ps3_system_bus_device *dev)
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] therm_adt746x: signed/unsigned confusion

[Roel Kluin]

As suggested, this is used for signed rather than unsigned

Signed-off-by: Roel Kluin 
---
diff --git a/drivers/macintosh/therm_adt746x.c 
b/drivers/macintosh/therm_adt746x.c
index 82607ad..c0621d5 100644
--- a/drivers/macintosh/therm_adt746x.c
+++ b/drivers/macintosh/therm_adt746x.c
@@ -498,8 +498,8 @@ static ssize_t store_##name(struct device *dev, struct 
device_attribute *attr, c
 #define BUILD_STORE_FUNC_INT(name, data)   \
 static ssize_t store_##name(struct device *dev, struct device_attribute *attr, 
const char *buf, size_t n) \
 {  \
-   u32 val;\
-   val = simple_strtoul(buf, NULL, 10);\
+   int val;\
+   val = simple_strtol(buf, NULL, 10); \
if (val < 0 || val > 255)   \
return -EINVAL; \
printk(KERN_INFO "Setting specified fan speed to %d\n", val);   \

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] PS3 ps3av_set_video_mode() make id signed

[Roel Kluin]

vi drivers/video/ps3fb.c +618
static int ps3fb_set_par(struct fb_info *info)
{
struct ps3fb_par *par = info->par;
... [ and at line 660 ] ...
if (ps3av_set_video_mode(par->new_mode_id))

now new_mode_id is an int

vi drivers/video/ps3fb.c +132
struct ps3fb_par {
...
int new_mode_id;
...
};

vi drivers/ps3/ps3av.c +844
int ps3av_set_video_mode(u32 id)

-^^^

{
...
if (... || id < 0) {

^^^

dev_dbg(&ps3av->dev->core, "%s: error id :%d\n", __func__, id);
return -EINVAL;
}
...
id = ps3av_auto_videomode(&ps3av->av_hw_conf);
if (id < 1) {
-^^^
printk(KERN_ERR "%s: invalid id :%d\n", __func__, id);
return -EINVAL;
}
...
ps3av->ps3av_mode = id;

vi drivers/ps3/ps3av.c +763
static int ps3av_auto_videomode()

---^^^

+42
static struct ps3av {
...
int ps3av_mode;
...
};

->8---8<---
make id signed so a negative id will get noticed

Signed-off-by: Roel Kluin 
---
diff --git a/drivers/ps3/ps3av.c b/drivers/ps3/ps3av.c
index 5324978..7aa6d41 100644
--- a/drivers/ps3/ps3av.c
+++ b/drivers/ps3/ps3av.c
@@ -838,7 +838,7 @@ static int ps3av_get_hw_conf(struct ps3av *ps3av)
 }
 
 /* set mode using id */
-int ps3av_set_video_mode(u32 id)
+int ps3av_set_video_mode(int id)
 {
int size;
u32 option;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] powerpc/mpc5121: fix NULL test

[Roel Kluin]

strcmp on NULL results in a segmentation fault, also, remove the second,
redundant test on dev

Signed-off-by: Roel Kluin 
---
Please verify whether this patch correct.

diff --git a/arch/powerpc/platforms/512x/clock.c 
b/arch/powerpc/platforms/512x/clock.c
index f416014..1bcff94 100644
--- a/arch/powerpc/platforms/512x/clock.c
+++ b/arch/powerpc/platforms/512x/clock.c
@@ -56,12 +56,12 @@ static struct clk *mpc5121_clk_get(struct device *dev, 
const char *id)
int dev_match = 0;
int id_match = 0;
 
-   if (dev == NULL && id == NULL)
+   if (dev == NULL || id == NULL)
return NULL;
 
mutex_lock(&clocks_mutex);
list_for_each_entry(p, &clocks, node) {
-   if (dev && dev == p->dev)
+   if (dev == p->dev)
dev_match++;
if (strcmp(id, p->name) == 0)
id_match++;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] [POWERPC] pasemi: ioremap/iounmap balance and failure handling

[Roel Kluin]

Olof Johansson wrote:
> On Sat, Dec 13, 2008 at 05:45:41PM +0100, Roel Kluin wrote:
>> map_onedev can return NULL, so catch that. Also iounmap if DMA controller 
>> can't be
>> found.

>> +
>>  iob_regs = map_onedev(iob_pdev, 0);
>> +if (iob_regs == NULL) {
>> +BUG();
>> +printk(KERN_WARNING "Can't ioremap I/O Bridge registers\n");
>> +err = -ENODEV;
>> +goto out;
>> +}
> 
> I don't see the point of doing BUG() _and_ error recovery. BUG() is
> definitely heavy handed. Something like "pasemi_dma_init: Can't..." would
> be a good and detailed enough error message.

Thanks for reviewing. This patch should address your comments to patch V1.

As I asked in my V2, I think there may also be a problem with
pasemi_mac_init_module(): if pci_register_driver() fails, then iob_regs won't 
get
iounmapped. right?

Is it ok to add the function pasemi_dma_exit() for these cleanup purposes?
and is it correct that we don't need to EXPORT_SYMBOL(pasemi_dma_exit)?
-8<>8---

map_onedev can return NULL, so catch that. Also iounmap if DMA controller can't 
be
found.

Signed-off-by: Roel Kluin 
---
 arch/powerpc/include/asm/pasemi_dma.h   |3 +++
 arch/powerpc/platforms/pasemi/dma_lib.c |   15 ++-
 drivers/net/pasemi_mac.c|6 +-
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/pasemi_dma.h 
b/arch/powerpc/include/asm/pasemi_dma.h
index 19fd793..7256a2c 100644
--- a/arch/powerpc/include/asm/pasemi_dma.h
+++ b/arch/powerpc/include/asm/pasemi_dma.h
@@ -535,4 +535,7 @@ extern void pasemi_dma_free_fun(int fun);
 /* Initialize the library, must be called before any other functions */
 extern int pasemi_dma_init(void);
 
+/* Clean up the library */
+extern void pasemi_dma_exit(void);
+
 #endif /* ASM_PASEMI_DMA_H */
diff --git a/arch/powerpc/platforms/pasemi/dma_lib.c 
b/arch/powerpc/platforms/pasemi/dma_lib.c
index 217af32..c97fba9 100644
--- a/arch/powerpc/platforms/pasemi/dma_lib.c
+++ b/arch/powerpc/platforms/pasemi/dma_lib.c
@@ -534,14 +534,17 @@ int pasemi_dma_init(void)
err = -ENODEV;
goto out;
}
+
iob_regs = map_onedev(iob_pdev, 0);
+   if (iob_regs == NULL)
+   printk(KERN_WARNING "pasemi_dma_init: Can't ioremap I/O Bridge 
registers\n");
 
dma_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa007, NULL);
if (!dma_pdev) {
BUG();
printk(KERN_WARNING "Can't find DMA controller\n");
err = -ENODEV;
-   goto out;
+   goto out_unmap;
}
dma_regs = map_onedev(dma_pdev, 0);
base_hw_irq = virq_to_hw(dma_pdev->irq);
@@ -624,9 +627,19 @@ int pasemi_dma_init(void)
 
printk(KERN_INFO "PA Semi PWRficient DMA library initialized "
"(%d tx, %d rx channels)\n", num_txch, num_rxch);
+   goto out;
+
+out_unmap:
+   iounmap(iob_regs);
 
 out:
spin_unlock(&init_lock);
return err;
 }
 EXPORT_SYMBOL(pasemi_dma_init);
+
+void pasemi_dma_exit(void)
+{
+   iounmap(iob_regs);
+}
+
diff --git a/drivers/net/pasemi_mac.c b/drivers/net/pasemi_mac.c
index edc0fd5..0897fa0 100644
--- a/drivers/net/pasemi_mac.c
+++ b/drivers/net/pasemi_mac.c
@@ -1919,7 +1919,11 @@ int pasemi_mac_init_module(void)
if (err)
return err;
 
-   return pci_register_driver(&pasemi_mac_driver);
+   err = pci_register_driver(&pasemi_mac_driver);
+   if (err)
+   pasemi_dma_exit();
+
+   return err;
 }
 
 module_init(pasemi_mac_init_module);

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] [POWERPC] pasemi: ioremap/iounmap balance and failure handling

[Roel Kluin]

Roel Kluin wrote:
> map_onedev can return NULL, so catch that. Also iounmap if DMA controller 
> can't be
> found.

I think there may also be a problem with pasemi_mac_init_module(): if
pci_register_driver() fails, then iob_regs won't get iounmapped.

maybe something like the totally untested patch below?

diff --git a/arch/powerpc/include/asm/pasemi_dma.h 
b/arch/powerpc/include/asm/pasemi_dma.h
index 19fd793..7256a2c 100644
--- a/arch/powerpc/include/asm/pasemi_dma.h
+++ b/arch/powerpc/include/asm/pasemi_dma.h
@@ -535,4 +535,7 @@ extern void pasemi_dma_free_fun(int fun);
 /* Initialize the library, must be called before any other functions */
 extern int pasemi_dma_init(void);
 
+/* Clean up the library */
+extern void pasemi_dma_exit(void);
+
 #endif /* ASM_PASEMI_DMA_H */
diff --git a/arch/powerpc/platforms/pasemi/dma_lib.c 
b/arch/powerpc/platforms/pasemi/dma_lib.c
index 217af32..fc0e1f9 100644
--- a/arch/powerpc/platforms/pasemi/dma_lib.c
+++ b/arch/powerpc/platforms/pasemi/dma_lib.c
@@ -534,14 +534,21 @@ int pasemi_dma_init(void)
err = -ENODEV;
goto out;
}
+
iob_regs = map_onedev(iob_pdev, 0);
+   if (iob_regs == NULL) {
+   BUG();
+   printk(KERN_WARNING "Can't ioremap I/O Bridge registers\n");
+   err = -ENODEV;
+   goto out;
+   }
 
dma_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa007, NULL);
if (!dma_pdev) {
BUG();
printk(KERN_WARNING "Can't find DMA controller\n");
err = -ENODEV;
-   goto out;
+   goto out_unmap;
}
dma_regs = map_onedev(dma_pdev, 0);
base_hw_irq = virq_to_hw(dma_pdev->irq);
@@ -624,9 +631,19 @@ int pasemi_dma_init(void)
 
printk(KERN_INFO "PA Semi PWRficient DMA library initialized "
"(%d tx, %d rx channels)\n", num_txch, num_rxch);
+   goto out;
+
+out_unmap:
+   iounmap(iob_regs);
 
 out:
spin_unlock(&init_lock);
return err;
 }
 EXPORT_SYMBOL(pasemi_dma_init);
+
+void pasemi_dma_exit(void)
+{
+   iounmap(iob_regs);
+}
+
diff --git a/drivers/net/pasemi_mac.c b/drivers/net/pasemi_mac.c
index edc0fd5..0897fa0 100644
--- a/drivers/net/pasemi_mac.c
+++ b/drivers/net/pasemi_mac.c
@@ -1919,7 +1919,11 @@ int pasemi_mac_init_module(void)
if (err)
return err;
 
-   return pci_register_driver(&pasemi_mac_driver);
+   err = pci_register_driver(&pasemi_mac_driver);
+   if (err)
+   pasemi_dma_exit();
+
+   return err;
 }
 
 module_init(pasemi_mac_init_module);
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] [POWERPC] pasemi: ioremap/iounmap balance and failure handling

[Roel Kluin]

map_onedev can return NULL, so catch that. Also iounmap if DMA controller can't 
be
found.

Signed-off-by: Roel Kluin 
---
UNTESTED! I am a bit new, so please confirm whether this is correct. 
especially: 

* can we iounmap while init_lock is held?
* is it ok to add another BUG() here?

diff --git a/arch/powerpc/platforms/pasemi/dma_lib.c 
b/arch/powerpc/platforms/pasemi/dma_lib.c
index 217af32..bdf5440 100644
--- a/arch/powerpc/platforms/pasemi/dma_lib.c
+++ b/arch/powerpc/platforms/pasemi/dma_lib.c
@@ -534,14 +534,21 @@ int pasemi_dma_init(void)
err = -ENODEV;
goto out;
}
+
iob_regs = map_onedev(iob_pdev, 0);
+   if (iob_regs == NULL) {
+   BUG();
+   printk(KERN_WARNING "Can't ioremap I/O Bridge registers\n");
+   err = -ENODEV;
+   goto out;
+   }
 
dma_pdev = pci_get_device(PCI_VENDOR_ID_PASEMI, 0xa007, NULL);
if (!dma_pdev) {
BUG();
printk(KERN_WARNING "Can't find DMA controller\n");
err = -ENODEV;
-   goto out;
+   goto out_unmap;
}
dma_regs = map_onedev(dma_pdev, 0);
base_hw_irq = virq_to_hw(dma_pdev->irq);
@@ -624,6 +631,10 @@ int pasemi_dma_init(void)
 
printk(KERN_INFO "PA Semi PWRficient DMA library initialized "
"(%d tx, %d rx channels)\n", num_txch, num_rxch);
+   goto out;
+
+out_unmap:
+   iounmap(iob_regs);
 
 out:
spin_unlock(&init_lock);

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH v2] hv{c,cs,si}_struct make *count signed

[Roel Kluin]

Otherwise count < 0 will fail

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
This was not tested. Is this how it should be fixed?

the locations where the failures occur are in the functions hv*_close, on:

vi drivers/char/hvcs.c +1251

vi drivers/char/hvsi.c +911

vi drivers/char/drivers/char/hvc_console.c +387

diff --git a/drivers/char/hvc_console.h b/drivers/char/hvc_console.h
index 8297dbc..3c85d78 100644
--- a/drivers/char/hvc_console.h
+++ b/drivers/char/hvc_console.h
@@ -48,7 +48,7 @@ struct hvc_struct {
spinlock_t lock;
int index;
struct tty_struct *tty;
-   unsigned int count;
+   int count;
int do_wakeup;
char *outbuf;
int outbuf_size;
diff --git a/drivers/char/hvcs.c b/drivers/char/hvcs.c
index 473d9b1..6e6eb44 100644
--- a/drivers/char/hvcs.c
+++ b/drivers/char/hvcs.c
@@ -269,7 +269,7 @@ struct hvcs_struct {
unsigned int index;
 
struct tty_struct *tty;
-   unsigned int open_count;
+   int open_count;
 
/*
 * Used to tell the driver kernel_thread what operations need to take
diff --git a/drivers/char/hvsi.c b/drivers/char/hvsi.c
index 59c6f9a..af05528 100644
--- a/drivers/char/hvsi.c
+++ b/drivers/char/hvsi.c
@@ -75,7 +75,7 @@ struct hvsi_struct {
spinlock_t lock;
int index;
struct tty_struct *tty;
-   unsigned int count;
+   int count;
uint8_t throttle_buf[128];
uint8_t outbuf[N_OUTBUF]; /* to implement write_room and 
chars_in_buffer */
/* inbuf is for packet reassembly. leave a little room for leftovers. */
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative

[roel kluin]

Andreas Schwab wrote:
> roel kluin <[EMAIL PROTECTED]> writes:
> 
>> -if (--hvcsd->open_count == 0) {
>> +if (hvcsd->open_count == 1) {
>> +hvcsd->open_count--;
> 
> This is not the same.

I think you're missing that I also decrement if (hvcsd->open_count > 1)
If not, please elaborate.

> 
>> -if (--hp->count == 0) {
>> +if (hp->count == 1) {
>> +hp->count--;
> 
> Likewise.

Same here.

Roel
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] powerpc: hvc_close() unsigned hp->count cannot be negative

[roel kluin]

unsigned hp->count cannot be negative

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
Similar to the previous patch but with lock. For hvc_struct, see
vi drivers/char/hvc_console.h +47

diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index 5b819b1..337f6c6 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -366,7 +366,8 @@ static void hvc_close(struct tty_struct *tty, struct file * 
filp)
hp = tty->driver_data;
spin_lock_irqsave(&hp->lock, flags);
 
-   if (--hp->count == 0) {
+   if (hp->count == 1) {
+   hp->count--;
/* We are done with the tty pointer now. */
hp->tty = NULL;
spin_unlock_irqrestore(&hp->lock, flags);
@@ -384,7 +385,9 @@ static void hvc_close(struct tty_struct *tty, struct file * 
filp)
 */
tty_wait_until_sent(tty, HVC_CLOSE_WAIT);
} else {
-   if (hp->count < 0)
+   if (hp->count > 1)
+   hp->count--;
+   else
printk(KERN_ERR "hvc_close %X: oops, count is %d\n",
hp->vtermno, hp->count);
spin_unlock_irqrestore(&hp->lock, flags);

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] [POWERPC] smu_sat_get_sdb_partition() - unsigned len cannot be negative

[roel kluin]

i2c_smbus_read_word_data() returns a s32, which may be negative
but unsigned len cannot be negative.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
for i2c_smbus_read_word_data(), see
vi drivers/i2c/i2c-core.c +1663

diff --git a/drivers/macintosh/windfarm_smu_sat.c 
b/drivers/macintosh/windfarm_smu_sat.c
index 7f2be4b..7847e98 100644
--- a/drivers/macintosh/windfarm_smu_sat.c
+++ b/drivers/macintosh/windfarm_smu_sat.c
@@ -87,11 +87,12 @@ struct smu_sdbp_header *smu_sat_get_sdb_partition(unsigned 
int sat_id, int id,
return NULL;
}
 
-   len = i2c_smbus_read_word_data(&sat->i2c, 9);
-   if (len < 0) {
+   err = i2c_smbus_read_word_data(&sat->i2c, 9);
+   if (err < 0) {
printk(KERN_ERR "smu_sat_get_sdb_part rd len error\n");
return NULL;
}
+   len = err;
if (len == 0) {
printk(KERN_ERR "smu_sat_get_sdb_part no partition %x\n", id);
return NULL;


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

powerpc: hv{cs, si}_close() both unsigned hp->count and hvcsd->open_count cannot be negative

[roel kluin]

unsigned hp->count and hvcsd->open_count cannot be negative

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
Both members of respectively struct hvcs_struct, see
vi drivers/char/hvcs.c +262

and struct hvcs_struct, see
vi drivers/char/hvsi.c +70

diff --git a/drivers/char/hvcs.c b/drivers/char/hvcs.c
index 473d9b1..b228b84 100644
--- a/drivers/char/hvcs.c
+++ b/drivers/char/hvcs.c
@@ -1222,7 +1222,8 @@ static void hvcs_close(struct tty_struct *tty, struct 
file *filp)
hvcsd = tty->driver_data;
 
spin_lock_irqsave(&hvcsd->lock, flags);
-   if (--hvcsd->open_count == 0) {
+   if (hvcsd->open_count == 1) {
+   hvcsd->open_count--;
 
vio_disable_interrupts(hvcsd->vdev);
 
@@ -1248,7 +1249,9 @@ static void hvcs_close(struct tty_struct *tty, struct 
file *filp)
free_irq(irq, hvcsd);
kref_put(&hvcsd->kref, destroy_hvcs_struct);
return;
-   } else if (hvcsd->open_count < 0) {
+   } else if (hvcsd->open_count > 1) {
+   hvcsd->open_count--;
+   } else {
printk(KERN_ERR "HVCS: [EMAIL PROTECTED] open_count: %d"
" is missmanaged.\n",
hvcsd->vdev->unit_address, hvcsd->open_count);
diff --git a/drivers/char/hvsi.c b/drivers/char/hvsi.c
index 59c6f9a..d46bccd 100644
--- a/drivers/char/hvsi.c
+++ b/drivers/char/hvsi.c
@@ -875,7 +875,8 @@ static void hvsi_close(struct tty_struct *tty, struct file 
*filp)
 
spin_lock_irqsave(&hp->lock, flags);
 
-   if (--hp->count == 0) {
+   if (hp->count == 1) {
+   hp->count--;
hp->tty = NULL;
hp->inbuf_end = hp->inbuf; /* discard remaining partial packets 
*/
 
@@ -908,7 +909,9 @@ static void hvsi_close(struct tty_struct *tty, struct file 
*filp)
 
spin_lock_irqsave(&hp->lock, flags);
}
-   } else if (hp->count < 0)
+   } else if (hp->count > 1)
+   hp->count--;
+   else
printk(KERN_ERR "hvsi_close %lu: oops, count is %d\n",
   hp - hvsi_ports, hp->count);
 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] [POWERPC] unsigned speed cannot be negative

[roel kluin]

unsigned speed cannot be negative

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
N.B. It could be possible that a different fix is needed.

diff --git a/arch/powerpc/kernel/udbg_16550.c b/arch/powerpc/kernel/udbg_16550.c
index cb01ebc..7b7da8c 100644
--- a/arch/powerpc/kernel/udbg_16550.c
+++ b/arch/powerpc/kernel/udbg_16550.c
@@ -142,7 +142,7 @@ unsigned int udbg_probe_uart_speed(void __iomem *comport, 
unsigned int clock)
speed = (clock / prescaler) / (divisor * 16);
 
/* sanity check */
-   if (speed < 0 || speed > (clock / 16))
+   if (speed > (clock / 16))
speed = 9600;
 
return speed;


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] ibm_newemac: MAL[12]_IER_EVENTS definition: 2x *_OTE -> *_DE

[roel kluin]

MAL[12]_IER_EVENTS definitions have MAL_IER_OTE twice
but lack MAL_IER_DE

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
 drivers/net/ibm_newemac/mal.h |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ibm_newemac/mal.h b/drivers/net/ibm_newemac/mal.h
index eaa7262..717dc38 100644
--- a/drivers/net/ibm_newemac/mal.h
+++ b/drivers/net/ibm_newemac/mal.h
@@ -102,7 +102,7 @@
 /* MAL V1 IER bits */
 #define   MAL1_IER_NWE 0x0008
 #define   MAL1_IER_SOC_EVENTS  MAL1_IER_NWE
-#define   MAL1_IER_EVENTS  (MAL1_IER_SOC_EVENTS | MAL_IER_OTE | \
+#define   MAL1_IER_EVENTS  (MAL1_IER_SOC_EVENTS | MAL_IER_DE | \
 MAL_IER_OTE | MAL_IER_OE | MAL_IER_PE)
 
 /* MAL V2 IER bits */
@@ -110,7 +110,7 @@
 #define   MAL2_IER_PRE 0x0040
 #define   MAL2_IER_PWE 0x0020
 #define   MAL2_IER_SOC_EVENTS  (MAL2_IER_PT | MAL2_IER_PRE | MAL2_IER_PWE)
-#define   MAL2_IER_EVENTS  (MAL2_IER_SOC_EVENTS | MAL_IER_OTE | \
+#define   MAL2_IER_EVENTS  (MAL2_IER_SOC_EVENTS | MAL_IER_DE | \
 MAL_IER_OTE | MAL_IER_OE | MAL_IER_PE)
 
 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[alsa-devel] [PATCH v2] duplicate SNDRV_PCM_FMTBIT_S{16,24}_BE

[roel kluin]

Takashi Iwai wrote:
> At Tue, 19 Aug 2008 08:15:05 +0200 (CEST),
> Johannes Berg wrote:
>> roel kluin wrote:
>>> untested, is it correct?
>> not a clue, do you know how long ago that was? :)
>> does the driver check endianness anywhere?
> 
> AFAIK snd-aoa supports only bit-endian formats (at least in
> sound/aoa/soundbus/i2sbus-pcm.c), so this addition makes little
> sense.
> 
> Better to drop the duplicated words there.

Thanks Johannes and Takashi,

FWIW this removes the duplicates.
---
Remove duplicate assignment of SNDRV_PCM_FMTBIT_S{16,24}_BE bits

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/sound/aoa/codecs/snd-aoa-codec-tas.c 
b/sound/aoa/codecs/snd-aoa-codec-tas.c
index 7a16a33..6c515b2 100644
--- a/sound/aoa/codecs/snd-aoa-codec-tas.c
+++ b/sound/aoa/codecs/snd-aoa-codec-tas.c
@@ -654,15 +654,13 @@ static struct snd_kcontrol_new bass_control = {
 static struct transfer_info tas_transfers[] = {
{
/* input */
-   .formats = SNDRV_PCM_FMTBIT_S16_BE | SNDRV_PCM_FMTBIT_S16_BE |
-  SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S24_BE,
+   .formats = SNDRV_PCM_FMTBIT_S16_BE | SNDRV_PCM_FMTBIT_S24_BE,
.rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | 
SNDRV_PCM_RATE_48000,
.transfer_in = 1,
},
{
/* output */
-   .formats = SNDRV_PCM_FMTBIT_S16_BE | SNDRV_PCM_FMTBIT_S16_BE |
-  SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S24_BE,
+   .formats = SNDRV_PCM_FMTBIT_S16_BE | SNDRV_PCM_FMTBIT_S24_BE,
.rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | 
SNDRV_PCM_RATE_48000,
.transfer_in = 0,
},

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH v2] powerpc, scc: duplicate SCC_UHC_USBCEN

[roel kluin]

Kou Ishizaki wrote:
> 
> Roel,
> 
>> untested, is it correct?
> 
> Your patch is correct.
> 
> Thanks for your pointing it out and sending the patch. I tested your
> patch and it works good.
> 
> Fortunately, this bug is not fatal because it seems that the SCC-UHC
> sets SCC_UHC_USBEN and SCC_UHC_USBCEN at once.
> 
> 
> Your patch does not contain 'Signed-off-by' line. Could you re-post it
> with your sign?

Sorry for that and thanks for testing.
---
duplicate SCC_UHC_USBCEN

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/platforms/cell/celleb_scc_uhc.c 
b/arch/powerpc/platforms/cell/celleb_scc_uhc.c
index d63b720..b086f33 100644
--- a/arch/powerpc/platforms/cell/celleb_scc_uhc.c
+++ b/arch/powerpc/platforms/cell/celleb_scc_uhc.c
@@ -31,7 +31,7 @@
 
 static inline int uhc_clkctrl_ready(u32 val)
 {
-   const u32 mask = SCC_UHC_USBCEN | SCC_UHC_USBCEN;
+   const u32 mask = SCC_UHC_USBCEN | SCC_UHC_USBEN;
return((val & mask) == mask);
 }
 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] duplicate SNDRV_PCM_FMTBIT_S{16,24}_BE

[roel kluin]

untested, is it correct?
---
duplicate SNDRV_PCM_FMTBIT_S{16,24}_BE

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/sound/aoa/codecs/snd-aoa-codec-tas.c 
b/sound/aoa/codecs/snd-aoa-codec-tas.c
index 7a16a33..c922505 100644
--- a/sound/aoa/codecs/snd-aoa-codec-tas.c
+++ b/sound/aoa/codecs/snd-aoa-codec-tas.c
@@ -654,15 +654,15 @@ static struct snd_kcontrol_new bass_control = {
 static struct transfer_info tas_transfers[] = {
{
/* input */
-   .formats = SNDRV_PCM_FMTBIT_S16_BE | SNDRV_PCM_FMTBIT_S16_BE |
-  SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S24_BE,
+   .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |
+  SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE,
.rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | 
SNDRV_PCM_RATE_48000,
.transfer_in = 1,
},
{
/* output */
-   .formats = SNDRV_PCM_FMTBIT_S16_BE | SNDRV_PCM_FMTBIT_S16_BE |
-  SNDRV_PCM_FMTBIT_S24_BE | SNDRV_PCM_FMTBIT_S24_BE,
+   .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |
+  SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE,
.rates = SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | 
SNDRV_PCM_RATE_48000,
.transfer_in = 0,
},
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] POWERPC: duplicate test of MACIO_FLAG_SCCB_ON

[roel kluin]

untested, is it correct?

arch/powerpc/include/asm/pmac_feature.h:359:
#define MACIO_FLAG_SCCA_ON  0x0001
#define MACIO_FLAG_SCCB_ON  0x0002
---
duplicate test of MACIO_FLAG_SCCB_ON

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/platforms/powermac/feature.c 
b/arch/powerpc/platforms/powermac/feature.c
index 5169ecc..e6c0040 100644
--- a/arch/powerpc/platforms/powermac/feature.c
+++ b/arch/powerpc/platforms/powermac/feature.c
@@ -2677,7 +2677,7 @@ static void __init probe_one_macio(const char *name, 
const char *compat, int typ
macio_chips[i].of_node  = node;
macio_chips[i].type = type;
macio_chips[i].base = base;
-   macio_chips[i].flags= MACIO_FLAG_SCCB_ON | MACIO_FLAG_SCCB_ON;
+   macio_chips[i].flags= MACIO_FLAG_SCCA_ON | MACIO_FLAG_SCCB_ON;
macio_chips[i].name = macio_names[type];
revp = of_get_property(node, "revision-id", NULL);
if (revp)
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] powerpc, scc: duplicate SCC_UHC_USBCEN

[roel kluin]

untested, is it correct?
---
arch/powerpc/platforms/cell/celleb_scc.h:224:
#define SCC_UHC_USBEN   0x0001
#define SCC_UHC_USBCEN  0x0002

---
diff --git a/arch/powerpc/platforms/cell/celleb_scc_uhc.c 
b/arch/powerpc/platforms/cell/celleb_scc_uhc.c
index d63b720..b086f33 100644
--- a/arch/powerpc/platforms/cell/celleb_scc_uhc.c
+++ b/arch/powerpc/platforms/cell/celleb_scc_uhc.c
@@ -31,7 +31,7 @@
 
 static inline int uhc_clkctrl_ready(u32 val)
 {
-   const u32 mask = SCC_UHC_USBCEN | SCC_UHC_USBCEN;
+   const u32 mask = SCC_UHC_USBCEN | SCC_UHC_USBEN;
return((val & mask) == mask);
 }
 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [GIT PULL] one kbuild fix for powerpc

[roel kluin]

2008/6/12 Sam Ravnborg <[EMAIL PROTECTED]>:

> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 508c589..b763aba 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -467,6 +467,26 @@ static void parse_elf_finish(struct elf_info *info)
>release_file(info->hdr, info->size);
>  }
>
> +static int ignore_undef_symbol(struct elf_info *info, const char *symname)
> +{
> +   /* ignore __this_module, it will be resolved shortly */
> +   if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 0)
> +   return 1;
> +   /* ignore global offset table */
> +   if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0)
> +   return 1;
> +   if (info->hdr->e_machine == EM_PPC)
> +   /* Special register function linked on all modules during 
> final link of .ko */
> +   if (strncmp(symname, "_restgpr_", sizeof("_restgpr_") - 1) == 
> 0 ||
> +   strncmp(symname, "_savegpr_", sizeof("_savegpr_") - 1) == 
> 0 ||
> +   strncmp(symname, "_rest32gpr_", sizeof("_rest32gpr_") - 
> 1) == 0 ||
> +   strncmp(symname, "_save32gpr_", sizeof("_save32gpr_") - 
> 1) == 0) {
> +   return 1;
> +   }

Either the indentation is wrong, or you want to move the opening curly
bracket upwards.

> +   /* Do not ignore this symbol */
> +   return 0;
> +}
> +
>  #define CRC_PFX MODULE_SYMBOL_PREFIX "__crc_"
>  #define KSYMTAB_PFX MODULE_SYMBOL_PREFIX "__ksymtab_"
>
> @@ -493,11 +513,7 @@ static void handle_modversions(struct module *mod, 
> struct elf_info *info,
>if (ELF_ST_BIND(sym->st_info) != STB_GLOBAL &&
>ELF_ST_BIND(sym->st_info) != STB_WEAK)
>break;
> -   /* ignore global offset table */
> -   if (strcmp(symname, "_GLOBAL_OFFSET_TABLE_") == 0)
> -   break;
> -   /* ignore __this_module, it will be resolved shortly */
> -   if (strcmp(symname, MODULE_SYMBOL_PREFIX "__this_module") == 
> 0)
> +   if (ignore_undef_symbol(info, symname))
>break;
>  /* cope with newer glibc (2.3.4 or higher) STT_ definition in elf.h */
>  #if defined(STT_REGISTER) || defined(STT_SPARC_REGISTER)
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 2/2 v2] mpic_u3msi: mpic_u3msi: failed allocation unnoticed

[Roel Kluin]

Again thanks to Segher and Joe Perches
---
bitmap_find_free_region(), called by mpic_msi_alloc_hwirqs() may return
signed, but hwirq is unsigned. A failed allocation remains unnoticed.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index 1d5a408..6e2f868 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -115,17 +115,19 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int 
nvec, int type)
struct msi_desc *entry;
struct msi_msg msg;
u64 addr;
+   int ret;
 
addr = find_ht_magic_addr(pdev);
msg.address_lo = addr & 0x;
msg.address_hi = addr >> 32;
 
list_for_each_entry(entry, &pdev->msi_list, list) {
-   hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
-   if (hwirq < 0) {
+   ret = mpic_msi_alloc_hwirqs(msi_mpic, 1);
+   if (ret < 0) {
pr_debug("u3msi: failed allocating hwirq\n");
-   return hwirq;
+   return ret;
}
+   hwirq = ret;
 
virq = irq_create_mapping(msi_mpic->irqhost, hwirq);
if (virq == NO_IRQ) {

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 1/2 v2] powerpc: mpic_pasemi_msi: failed allocation unnoticed

[Roel Kluin]

also please add my signoff to 
[PATCH 2/2 v2] mpic_u3msi: mpic_u3msi: failed allocation unnoticed
---
bitmap_find_free_region(), called by mpic_msi_alloc_hwirqs() may return
signed, but hwirq is unsigned. A failed allocation remains unnoticed.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/sysdev/mpic_pasemi_msi.c 
b/arch/powerpc/sysdev/mpic_pasemi_msi.c
index 33cbfb2..68aff60 100644
--- a/arch/powerpc/sysdev/mpic_pasemi_msi.c
+++ b/arch/powerpc/sysdev/mpic_pasemi_msi.c
@@ -95,6 +95,7 @@ static int pasemi_msi_setup_msi_irqs(struct pci_dev *pdev, 
int nvec, int type)
unsigned int virq;
struct msi_desc *entry;
struct msi_msg msg;
+   int ret;
 
pr_debug("pasemi_msi_setup_msi_irqs, pdev %p nvec %d type %d\n",
 pdev, nvec, type);
@@ -108,8 +109,9 @@ static int pasemi_msi_setup_msi_irqs(struct pci_dev *pdev, 
int nvec, int type)
 * few MSIs for someone, but restrictions will apply to how the
 * sources can be changed independently.
 */
-   hwirq = mpic_msi_alloc_hwirqs(msi_mpic, ALLOC_CHUNK);
-   if (hwirq < 0) {
+   ret = mpic_msi_alloc_hwirqs(msi_mpic, ALLOC_CHUNK);
+   hwirq = ret;
+   if (ret < 0) {
pr_debug("pasemi_msi: failed allocating hwirq\n");
return hwirq;
}

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 2/2 v2] mpic_u3msi: mpic_u3msi: failed allocation unnoticed

[Roel Kluin]

Segher Boessenkool wrote:
>> bitmap_find_free_region(), called by mpic_msi_alloc_hwirqs() may
>> return -ENOMEM, but hwirq of type irq_hw_number_t which is unsigned.
> 
>>  list_for_each_entry(entry, &pdev->msi_list, list) {
>>  hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
>> -if (hwirq < 0) {
>> +if (hwirq == -ENOMEM) {

> 
> Please test for _all_ error values, instead.
>
> Segher

In this case -ENOMEM was _all_ error values, but I get your point.
---
bitmap_find_free_region(), called by mpic_msi_alloc_hwirqs() may return
signed, but hwirq is unsigned. A failed allocation remains unnoticed.

diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index 1d5a408..e790f39 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -115,14 +115,16 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int 
nvec, int type)
struct msi_desc *entry;
struct msi_msg msg;
u64 addr;
+   int ret;
 
addr = find_ht_magic_addr(pdev);
msg.address_lo = addr & 0x;
msg.address_hi = addr >> 32;
 
list_for_each_entry(entry, &pdev->msi_list, list) {
-   hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
-   if (hwirq < 0) {
+   ret = mpic_msi_alloc_hwirqs(msi_mpic, 1);
+   hwirq = ret;
+   if (ret < 0) {
pr_debug("u3msi: failed allocating hwirq\n");
return hwirq;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 1/2] spi_mpc83xx: test below 0 on unsigned irq in mpc83xx_spi_probe()

[Roel Kluin]

mpc83xx_spi->irq is unsigned, so the test fails

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
index be15a62..033fd51 100644
--- a/drivers/spi/spi_mpc83xx.c
+++ b/drivers/spi/spi_mpc83xx.c
@@ -454,12 +454,12 @@ static int __init mpc83xx_spi_probe(struct 
platform_device *dev)
goto put_master;
}
 
-   mpc83xx_spi->irq = platform_get_irq(dev, 0);
-
-   if (mpc83xx_spi->irq < 0) {
-   ret = -ENXIO;
+   ret = platform_get_irq(dev, 0);
+   if (ret < 0)
goto unmap_io;
-   }
+
+   mpc83xx_spi->irq = ret;
+   ret = 0;
 
/* Register for SPI Interrupt */
ret = request_irq(mpc83xx_spi->irq, mpc83xx_spi_irq,

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] spi_mpc83xx: test below 0 on unsigned irq in mpc83xx_spi_probe()

[Roel Kluin]

mpc83xx_spi->irq is unsigned, so the test fails

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/spi/spi_mpc83xx.c b/drivers/spi/spi_mpc83xx.c
index be15a62..033fd51 100644
--- a/drivers/spi/spi_mpc83xx.c
+++ b/drivers/spi/spi_mpc83xx.c
@@ -454,12 +454,12 @@ static int __init mpc83xx_spi_probe(struct 
platform_device *dev)
goto put_master;
}
 
-   mpc83xx_spi->irq = platform_get_irq(dev, 0);
-
-   if (mpc83xx_spi->irq < 0) {
-   ret = -ENXIO;
+   ret = platform_get_irq(dev, 0);
+   if (ret < 0)
goto unmap_io;
-   }
+
+   mpc83xx_spi->irq = ret;
+   ret = 0;
 
/* Register for SPI Interrupt */
ret = request_irq(mpc83xx_spi->irq, mpc83xx_spi_irq,
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 2/2] mpic_u3msi: mpic_u3msi: failed allocation unnoticed

[Roel Kluin]

A similar problem in arch/powerpc/sysdev/mpic_u3msi.c,
sorry for the dup, fixed header.
---
bitmap_find_free_region(), called by mpic_msi_alloc_hwirqs() may
return -ENOMEM, but hwirq of type irq_hw_number_t which is unsigned.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
The functions can be found respectively in:
//---[ vi lib/bitmap.c +807 ]---
//---[ vi arch/powerpc/sysdev/mpic_msi.c +39 ]---

diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index 1d5a408..30a4e27 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -122,7 +122,7 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int 
nvec, int type)
 
list_for_each_entry(entry, &pdev->msi_list, list) {
hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
-   if (hwirq < 0) {
+   if (hwirq == -ENOMEM) {
pr_debug("u3msi: failed allocating hwirq\n");
return hwirq;
}

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 2/2] powerpc: mpic_pasemi_msi: failed allocation unnoticed

[Roel Kluin]

A similar problem in arch/powerpc/sysdev/mpic_u3msi.c:
---
bitmap_find_free_region(), called by mpic_msi_alloc_hwirqs() may
return -ENOMEM, but hwirq of type irq_hw_number_t which is unsigned.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
The functions can be found respectively in:
//---[ vi lib/bitmap.c +807 ]---
//---[ vi arch/powerpc/sysdev/mpic_msi.c +39 ]---

diff --git a/arch/powerpc/sysdev/mpic_u3msi.c b/arch/powerpc/sysdev/mpic_u3msi.c
index 1d5a408..30a4e27 100644
--- a/arch/powerpc/sysdev/mpic_u3msi.c
+++ b/arch/powerpc/sysdev/mpic_u3msi.c
@@ -122,7 +122,7 @@ static int u3msi_setup_msi_irqs(struct pci_dev *pdev, int 
nvec, int type)
 
list_for_each_entry(entry, &pdev->msi_list, list) {
hwirq = mpic_msi_alloc_hwirqs(msi_mpic, 1);
-   if (hwirq < 0) {
+   if (hwirq == -ENOMEM) {
pr_debug("u3msi: failed allocating hwirq\n");
return hwirq;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] powerpc: mpic_pasemi_msi: failed allocation unnoticed

[Roel Kluin]

bitmap_find_free_region(), called by mpic_msi_alloc_hwirqs() may
return -ENOMEM, but hwirq of type irq_hw_number_t which is unsigned.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
The functions can be found respectively in:
//---[ vi lib/bitmap.c +807 ]---
//---[ vi arch/powerpc/sysdev/mpic_msi.c +39 ]---

diff --git a/arch/powerpc/sysdev/mpic_pasemi_msi.c 
b/arch/powerpc/sysdev/mpic_pasemi_msi.c
index 33cbfb2..a15ac5c 100644
--- a/arch/powerpc/sysdev/mpic_pasemi_msi.c
+++ b/arch/powerpc/sysdev/mpic_pasemi_msi.c
@@ -109,7 +109,7 @@ static int pasemi_msi_setup_msi_irqs(struct pci_dev *pdev, 
int nvec, int type)
 * sources can be changed independently.
 */
hwirq = mpic_msi_alloc_hwirqs(msi_mpic, ALLOC_CHUNK);
-   if (hwirq < 0) {
+   if (hwirq == -ENOMEM) {
pr_debug("pasemi_msi: failed allocating hwirq\n");
return hwirq;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 1/1 resend][PPC] replace logical by bitand in pci_process_ISA_OF_ranges(), arch/powerpc/kernel/isa-bridge.c

[Roel Kluin]

in arch/powerpc/kernel/isa-bridge.c:
41:#define ISA_SPACE_MASK 0x1
42:#define ISA_SPACE_IO 0x1
...

83:if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO) {
...
89:if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO)

I think these should be single &'s, I can't test it (no hardware)
please consider the patch below.
--
replace logical by bit and for ISA_SPACE_MASK

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/kernel/isa-bridge.c b/arch/powerpc/kernel/isa-bridge.c
index f0f49d1..406a9e6 100644
--- a/arch/powerpc/kernel/isa-bridge.c
+++ b/arch/powerpc/kernel/isa-bridge.c
@@ -80,13 +80,13 @@ static void __devinit pci_process_ISA_OF_ranges(struct 
device_node *isa_node,
 *  (size depending on dev->n_addr_cells)
 *   cell 5:the size of the range
 */
-   if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO) {
+   if ((range->isa_addr.a_hi & ISA_SPACE_MASK) != ISA_SPACE_IO) {
range++;
rlen -= sizeof(struct isa_range);
if (rlen < sizeof(struct isa_range))
goto inval_range;
}
-   if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO)
+   if ((range->isa_addr.a_hi & ISA_SPACE_MASK) != ISA_SPACE_IO)
goto inval_range;
 
isa_addr = range->isa_addr.a_lo;

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 2/8] powerpc: replace `__attribute' by `__attribute__'

[Roel Kluin]

replace `__attribute' by `__attribute__'

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/platforms/iseries/main_store.h 
b/arch/powerpc/platforms/iseries/main_store.h
index 1a7a3f5..976b23e 100644
--- a/arch/powerpc/platforms/iseries/main_store.h
+++ b/arch/powerpc/platforms/iseries/main_store.h
@@ -61,7 +61,7 @@ struct IoHriMainStoreSegment4 {
 };
 
 /* Main Store VPD for Power4 */
-struct __attribute((packed)) IoHriMainStoreChipInfo1 {
+struct __attribute__((packed)) IoHriMainStoreChipInfo1 {
u32 chipMfgID;
charchipECLevel[4];
 };
@@ -73,14 +73,14 @@ struct IoHriMainStoreVpdIdData {
charserialNumber[12];
 };
 
-struct __attribute((packed)) IoHriMainStoreVpdFruData {
+struct __attribute__((packed)) IoHriMainStoreVpdFruData {
charfruLabel[8];
u8  numberOfSlots;
u8  pluggingType;
u16 slotMapIndex;
 };
 
-struct  __attribute((packed)) IoHriMainStoreAdrRangeBlock {
+struct  __attribute__((packed)) IoHriMainStoreAdrRangeBlock {
void*blockStart;
void*blockEnd;
u32 blockProcChipId;
@@ -88,7 +88,7 @@ struct  __attribute((packed)) IoHriMainStoreAdrRangeBlock {
 
 #define MaxAreaAdrRangeBlocks 4
 
-struct __attribute((packed)) IoHriMainStoreArea4 {
+struct __attribute__((packed)) IoHriMainStoreArea4 {
u32 msVpdFormat;
u8  containedVpdType;
u8  reserved1;

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: ADT746X: logical-bitwise & confusion in set_max_duty_at_crit()

[Roel Kluin]

Segher Boessenkool wrote:
>> It includes suggested changes by Segher Boessenkool and I think this
>> version was tested by Darrick J. Wong
> 
>> -u8 reg;
>> +u8 reg, temp;
>>  struct i2c_client *client = to_i2c_client(dev);
>>  struct adt7473_data *data = i2c_get_clientdata(client);
>> -int temp = simple_strtol(buf, NULL, 10);
>> -temp = temp && 0xFF;
>> +
>> +temp = simple_strtol(buf, NULL, 10) & 0xFF;
> 
> It still does this superfluous "& 0xff", which hides the lack of
> range checking.

Sorry didn't quite grep that
---
logical-bitwise & confusion

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/hwmon/adt7473.c b/drivers/hwmon/adt7473.c
index 9587869..98937d3 100644
--- a/drivers/hwmon/adt7473.c
+++ b/drivers/hwmon/adt7473.c
@@ -566,11 +566,11 @@ static ssize_t set_max_duty_at_crit(struct device *dev,
const char *buf,
size_t count)
 {
-   u8 reg;
+   u8 reg, temp;
struct i2c_client *client = to_i2c_client(dev);
struct adt7473_data *data = i2c_get_clientdata(client);
-   int temp = simple_strtol(buf, NULL, 10);
-   temp = temp && 0xFF;
+
+   temp = simple_strtol(buf, NULL, 10);
 
mutex_lock(&data->lock);
data->max_duty_at_overheat = temp;

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: ADT746X: logical-bitwise & confusion in set_max_duty_at_crit()

[Roel Kluin]

Andrew, I think you may want this patch instead of the other
adt746x-logical-bitwise-confusion-in-set_max_duty_at_crit.patch

It includes suggested changes by Segher Boessenkool and I think this
version was tested by Darrick J. Wong
---
logical-bitwise & confusion

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/hwmon/adt7473.c b/drivers/hwmon/adt7473.c
index 9587869..2a2de73 100644
--- a/drivers/hwmon/adt7473.c
+++ b/drivers/hwmon/adt7473.c
@@ -566,11 +566,11 @@ static ssize_t set_max_duty_at_crit(struct device *dev,
const char *buf,
size_t count)
 {
-   u8 reg;
+   u8 reg, temp;
struct i2c_client *client = to_i2c_client(dev);
struct adt7473_data *data = i2c_get_clientdata(client);
-   int temp = simple_strtol(buf, NULL, 10);
-   temp = temp && 0xFF;
+
+   temp = simple_strtol(buf, NULL, 10) & 0xFF;
 
mutex_lock(&data->lock);
data->max_duty_at_overheat = temp;


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: ADT746X: logical-bitwise & confusion in set_max_duty_at_crit()

[Roel Kluin]

Segher Boessenkool wrote:
>> diff --git a/drivers/hwmon/adt7473.c b/drivers/hwmon/adt7473.c
>> index 9587869..8ea7da2 100644
>> --- a/drivers/hwmon/adt7473.c
>> +++ b/drivers/hwmon/adt7473.c
>> @@ -570,7 +570,7 @@ static ssize_t set_max_duty_at_crit(struct device
>> *dev,
>>  struct i2c_client *client = to_i2c_client(dev);
>>  struct adt7473_data *data = i2c_get_clientdata(client);
>>  int temp = simple_strtol(buf, NULL, 10);
>> -temp = temp && 0xFF;
>> +temp &= 0xFF;
>>
>>  mutex_lock(&data->lock);
>>  data->max_duty_at_overheat = temp;
> 
> The & 0xff here is bogus anyway; temp is only ever used as an u8,
> so just declare it as that, or do proper overflow/underflow checking
> on it.  The patch will need testing on hardware too, since it changes
> behaviour (it should be a bugfix, but who knows).

Maybe someone can test this?
---
logical-bitwise & confusion

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/hwmon/adt7473.c b/drivers/hwmon/adt7473.c
index 9587869..2a2de73 100644
--- a/drivers/hwmon/adt7473.c
+++ b/drivers/hwmon/adt7473.c
@@ -566,11 +566,11 @@ static ssize_t set_max_duty_at_crit(struct device *dev,
const char *buf,
size_t count)
 {
-   u8 reg;
+   u8 reg, temp;
struct i2c_client *client = to_i2c_client(dev);
struct adt7473_data *data = i2c_get_clientdata(client);
-   int temp = simple_strtol(buf, NULL, 10);
-   temp = temp && 0xFF;
+
+   temp = simple_strtol(buf, NULL, 10) & 0xFF;
 
mutex_lock(&data->lock);
data->max_duty_at_overheat = temp;

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: ADT746X: logical-bitwise & confusion in set_max_duty_at_crit()

[Roel Kluin]

Benjamin Herrenschmidt wrote:
> On Mon, 2008-03-10 at 08:46 +0100, Colin Leroy wrote:
>> On Mon, 10 Mar 2008 01:04:33 +0100, Roel Kluin wrote:
>>
>>> logical-bitwise & confusion
>> Looks good to me, but I'm not really maintaining that anymore :-)
>> I'm not sure who does, Cc:ing Benjamin as he'll probably know.
> 
> Nobody is U suspect...
> 
> Send it to the list linuxppc-dev@ozlabs.org, it should be picked up
> anyway.
(linuxppc-dev CC'd)
---
logical-bitwise & confusion

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/hwmon/adt7473.c b/drivers/hwmon/adt7473.c
index 9587869..8ea7da2 100644
--- a/drivers/hwmon/adt7473.c
+++ b/drivers/hwmon/adt7473.c
@@ -570,7 +570,7 @@ static ssize_t set_max_duty_at_crit(struct device *dev,
struct i2c_client *client = to_i2c_client(dev);
struct adt7473_data *data = i2c_get_clientdata(client);
int temp = simple_strtol(buf, NULL, 10);
-   temp = temp && 0xFF;
+   temp &= 0xFF;
 
mutex_lock(&data->lock);
data->max_duty_at_overheat = temp;

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] PPC: in celleb_show_cpuinfo() convert strncpy(x, y, sizeof(x)) to strlcpy

[Roel Kluin]

Roel Kluin wrote:
> This patch was not yet tested. Please confirm it's right.

was too quick with the send button. the batch below is probably better
---
strncpy does not append '\0' if the length of the source string equals
the size parameter, strlcpy does.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/platforms/celleb/setup.c 
b/arch/powerpc/platforms/celleb/setup.c
index f27ae1e..cbe09d9 100644
--- a/arch/powerpc/platforms/celleb/setup.c
+++ b/arch/powerpc/platforms/celleb/setup.c
@@ -81,8 +81,7 @@ static void celleb_show_cpuinfo(struct seq_file *m)
 
 static int __init celleb_machine_type_hack(char *ptr)
 {
-   strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
-   celleb_machine_type[sizeof(celleb_machine_type)-1] = 0;
+   strlcpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
return 0;
 }
 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] PPC: in celleb_show_cpuinfo() convert strncpy(x, y, sizeof(x)) to strlcpy

[Roel Kluin]

This patch was not yet tested. Please confirm it's right.
---
strncpy does not append '\0' if the length of the source string equals
the size parameter, strlcpy does.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/platforms/celleb/setup.c 
b/arch/powerpc/platforms/celleb/setup.c
index f27ae1e..d70fc53 100644
--- a/arch/powerpc/platforms/celleb/setup.c
+++ b/arch/powerpc/platforms/celleb/setup.c
@@ -81,7 +81,7 @@ static void celleb_show_cpuinfo(struct seq_file *m)
 
 static int __init celleb_machine_type_hack(char *ptr)
 {
-   strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
+   strlcpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
celleb_machine_type[sizeof(celleb_machine_type)-1] = 0;
return 0;
 }
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

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;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[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));
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH][drivers/net/fec_mpc52xx.c] duplicate NETIF_MSG_IFDOWN in MPC52xx_MESSAGES_DEFAULT

[Roel Kluin]

Untested patch below, please confirm it's the right fix.
--
duplicate NETIF_MSG_IFDOWN, 2nd should be NETIF_MSG_IFUP

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index f91ee70..ca3c3b3 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -55,7 +55,7 @@ module_param_array_named(mac, mpc52xx_fec_mac_addr, byte, 
NULL, 0);
 MODULE_PARM_DESC(mac, "six hex digits, ie. 0x1,0x2,0xc0,0x01,0xba,0xbe");
 
 #define MPC52xx_MESSAGES_DEFAULT ( NETIF_MSG_DRV | NETIF_MSG_PROBE | \
-   NETIF_MSG_LINK | NETIF_MSG_IFDOWN | NETIF_MSG_IFDOWN )
+   NETIF_MSG_LINK | NETIF_MSG_IFDOWN | NETIF_MSG_IFUP)
 static int debug = -1; /* the above default */
 module_param(debug, int, 0);
 MODULE_PARM_DESC(debug, "debugging messages level");
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c

[Roel Kluin]

>>> On Wed, 2008-01-23 at 23:37 +0100, Roel Kluin wrote:
>>>> -  if (val && 0x1)
>>>> +  if (val & 0x1)

>> Joe Perches wrote:
>>> I think this pattern should be added to checkpatch
>>> +   if ($line =~ /\&\&\s*0[xX]/) {

> On Thu, Jan 24, 2008 at 01:18:48AM +0100, Roel Kluin wrote:
>> I agree but there will be false positives, i'd propose to change that to
>> +if ($line =~ /(?:(?:\(|\&\&|\|\|)\s*0[xX]\s*(?:&&|\|\|)|
>> +(?:\&\&|\|\|)\s*0[xX]\s*(?:\)|&&|\|\|))/) {

Andy Whitcroft wrote:
> That one doesn't even match the original example.  Seems to be missing
> some number matching.  The concept seems sound though.  Basically
> looking for numbers which are definatly adjacent to a boolean or a brace
> on both sides.

You are right. here's a version that does work. It works the same as
git-grep -E 
"((\(|&&|\|\|)[[:space:]]*(0[xX][0-9a-fA-F]+|[0-9]+)[[:space:]]*(&&|\|\|)|(&&|\|\|)[[:space:]]*(0[xX][0-9a-fA-F]+|[0-9]+)[[:space:]]*(\)|&&|\|\|))"

---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 579f50f..62276f7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1337,6 +1337,11 @@ sub process {
}
}
 
+# Check for bitwise tests written as boolean
+   if ($line =~ 
/((\(|&&|\|\|)\s*(0[xX][0-9a-fA-F]+|[0-9]+)\s*(&&|\|\|)|(&&|\|\|)\s*(0[xX][0-9a-fA-F]+|[0-9]+)\s*(\)|&&|\|\|))/)
 {
+   WARN("boolean test with hexadecimal, perhaps just \'&\' 
or \'|\'?\n" . $herecurr);
+   }
+
 # if and else should not have general statements after it
if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ &&
$1 !~ /^\s*(?:\sif|{|\\|$)/) {

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 1/1][PPC] Test value, not 1 in print_insn_spu(), arch/powerpc/xmon/spu-dis.c

[Roel Kluin]

untested, please confirm:
The '|| 1' does nothing, should this be corrected like my patch does?
--
Test value, not 1.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/xmon/spu-dis.c b/arch/powerpc/xmon/spu-dis.c
index e5f8983..74d45fb 100644
--- a/arch/powerpc/xmon/spu-dis.c
+++ b/arch/powerpc/xmon/spu-dis.c
@@ -222,7 +222,7 @@ print_insn_spu (unsigned long insn, unsigned long memaddr)
  break;
case A_U18:
  value = DECODE_INSN_U18 (insn);
- if (value == 0 || 1)
+ if (value == 0 || value == 1)
{
  hex_value = value;
  printf("%u", value);
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH 1/1][PPC] replace logical- by bit-and in pci_process_ISA_OF_ranges(), arch/powerpc/kernel/isa-bridge.c

[Roel Kluin]

in arch/powerpc/kernel/isa-bridge.c:
41:#define ISA_SPACE_MASK 0x1
42:#define ISA_SPACE_IO 0x1
...
54:struct isa_address {
55: u32 a_hi;
...
65: const struct isa_range *range;

83:if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO) {
...
89:if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO)

shouldn't these be a single &?
then consider the untested patch below.
--
replace logical "&&" by bit "&" for ISA_SPACE_MASK

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/kernel/isa-bridge.c b/arch/powerpc/kernel/isa-bridge.c
index f0f49d1..406a9e6 100644
--- a/arch/powerpc/kernel/isa-bridge.c
+++ b/arch/powerpc/kernel/isa-bridge.c
@@ -80,13 +80,13 @@ static void __devinit pci_process_ISA_OF_ranges(struct 
device_node *isa_node,
 *  (size depending on dev->n_addr_cells)
 *   cell 5:the size of the range
 */
-   if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO) {
+   if ((range->isa_addr.a_hi & ISA_SPACE_MASK) != ISA_SPACE_IO) {
range++;
rlen -= sizeof(struct isa_range);
if (rlen < sizeof(struct isa_range))
goto inval_range;
}
-   if ((range->isa_addr.a_hi && ISA_SPACE_MASK) != ISA_SPACE_IO)
+   if ((range->isa_addr.a_hi & ISA_SPACE_MASK) != ISA_SPACE_IO)
goto inval_range;
 
isa_addr = range->isa_addr.a_lo;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c

[Roel Kluin]

Joe Perches wrote:
> On Wed, 2008-01-23 at 23:37 +0100, Roel Kluin wrote:
>> Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
>> ---
>> diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c
>> index ebf9e21..dcfb459 100644
>> --- a/arch/powerpc/boot/4xx.c
>> +++ b/arch/powerpc/boot/4xx.c
>> @@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void)
>>  val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT);
>>  cs = 0;
>>  while (val) {
>> -if (val && 0x1)
>> +if (val & 0x1)
>>  cs++;
>>  val = val >> 1;
> 
> I think this pattern should be added to checkpatch

I agree but...

> Signed-off-by: Joe Perches <[EMAIL PROTECTED]>
> 
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 579f50f..147e573 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1337,6 +1337,11 @@ sub process {
>   }
>   }
>  
> +# Check for bitwise tests written as boolean
> + if ($line =~ /\&\&\s*0[xX]/) {
> + WARN("boolean test with hexadecimal, perhaps just 1 
> \&?\n" . $herecurr);
> + }
> +

when you use git-grep -n "\(&&\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)",
(with s="[[:space:]]*") there will be false positives like

if (relocation < -0x2 || 0x1fffc < relocation)
if (ARCH_SUN4C_SUN4 && addr < 0xe000 && 0x2000 - len < addr) {
(and more)

However, if you search with
git-grep -n " \(&&\|||\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)$s\()\|&&\|||\)"
you won't get these false positives, but you do get the one I fixed.

Also there is the same logic flipped (though it did not give matches at
this time):
git-grep -n " \(&&\|||\|(\)${s}0x\([A-Z0-9]*\|[a-z0-9]*\)$s\(&&\|||\)"

so i'd propose to change that to
--
Catch boolean tests with hexadecimals, based on suggestion by Joe Perches

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 579f50f..8ac7c7b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1337,6 +1337,12 @@ sub process {
}
}
 
+# Check for bitwise tests written as boolean
+   if ($line =~ /(?:(?:\(|\&\&|\|\|)\s*0[xX]\s*(?:&&|\|\|)|
+   (?:\&\&|\|\|)\s*0[xX]\s*(?:\)|&&|\|\|))/) {
+   WARN("boolean test with hexadecimal, perhaps just 1 \& 
or \|?\n" . $herecurr);
+   }
+
 # if and else should not have general statements after it
if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ &&
$1 !~ /^\s*(?:\sif|{|\\|$)/) {
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH][ppc] logical/bitand typo in powerpc/boot/4xx.c

[Roel Kluin]

logical/bitand typo

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/boot/4xx.c b/arch/powerpc/boot/4xx.c
index ebf9e21..dcfb459 100644
--- a/arch/powerpc/boot/4xx.c
+++ b/arch/powerpc/boot/4xx.c
@@ -104,7 +104,7 @@ void ibm4xx_denali_fixup_memsize(void)
val = DDR_GET_VAL(val, DDR_CS_MAP, DDR_CS_MAP_SHIFT);
cs = 0;
while (val) {
-   if (val && 0x1)
+   if (val & 0x1)
cs++;
val = val >> 1;
}
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] balance ioremap/iounmap in {sycamore, walnut}_setup_arch()

[Roel Kluin]

Valentine Barshak wrote:
> Roel Kluin wrote:
>> I guess it should be done after the last usage of kb_data or fpga_status?
> 
> I think no iounmap(kb_data) needed. Looks like the pointer kn_cs (kb_cs
> = kb_data + 1) is used by the serio driver
> (drivers/input/serio/i8042-ppcio.h).
> Please note that we just ioremap and assign pointers here, not actually
> reading the registers.
> Also, looks like you unmap the fpga registers too early.
> Thanks,
> Valentine.

Thanks for your pointers. I'm new, so I'm not sure how this should be done. I
have removed the iounmap(kb_data), and moved the iounmap(fpga_status) lower.
possibly the latter could be done before the sycamore_rtc_base assignment?
--
Balance ioremap/iounmap

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/ppc/platforms/4xx/sycamore.c 
b/arch/ppc/platforms/4xx/sycamore.c
index 8689f3e..6ce4815 100644
--- a/arch/ppc/platforms/4xx/sycamore.c
+++ b/arch/ppc/platforms/4xx/sycamore.c
@@ -132,6 +132,7 @@ sycamore_setup_arch(void)
sycamore_rtc_base = (void *) SYCAMORE_RTC_VADDR;
TODC_INIT(TODC_TYPE_DS1743, sycamore_rtc_base, sycamore_rtc_base,
  sycamore_rtc_base, 8);
+   iounmap(fpga_status);
 
/* Identify the system */
printk(KERN_INFO "IBM Sycamore (IBM405GPr) Platform\n");

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] balance ioremap/iounmap in {sycamore, walnut}_setup_arch()

[Roel Kluin]

I guess it should be done after the last usage of kb_data or fpga_status?
--
Balance ioremap/iounmap

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/ppc/platforms/4xx/sycamore.c 
b/arch/ppc/platforms/4xx/sycamore.c
index 8689f3e..4f3bac1 100644
--- a/arch/ppc/platforms/4xx/sycamore.c
+++ b/arch/ppc/platforms/4xx/sycamore.c
@@ -99,28 +99,30 @@ sycamore_setup_arch(void)
kb_data = ioremap(SYCAMORE_PS2_BASE, 8);
if (!kb_data) {
printk(KERN_CRIT
   "sycamore_setup_arch() kb_data ioremap failed\n");
return;
}
 
kb_cs = kb_data + 1;
+   iounmap(kb_data);
 
fpga_status = ioremap(PPC40x_FPGA_BASE, 8);
if (!fpga_status) {
printk(KERN_CRIT
   "sycamore_setup_arch() fpga_status ioremap failed\n");
return;
}
 
fpga_enable = fpga_status + 1;
fpga_polarity = fpga_status + 2;
fpga_trigger = fpga_status + 3;
fpga_brdc = fpga_status + 4;
+   iounmap(fpga_status);
 
/* split the keyboard and mouse interrupts */
fpga_brdc_data = readb(fpga_brdc);
fpga_brdc_data |= 0x80;
writeb(fpga_brdc_data, fpga_brdc);
 
writeb(0x3, fpga_enable);
 
diff --git a/arch/ppc/platforms/4xx/walnut.c b/arch/ppc/platforms/4xx/walnut.c
index 2f97723..8cfeb94 100644
--- a/arch/ppc/platforms/4xx/walnut.c
+++ b/arch/ppc/platforms/4xx/walnut.c
@@ -81,28 +81,30 @@ walnut_setup_arch(void)
kb_data = ioremap(WALNUT_PS2_BASE, 8);
if (!kb_data) {
printk(KERN_CRIT
   "walnut_setup_arch() kb_data ioremap failed\n");
return;
}
 
kb_cs = kb_data + 1;
+   iounmap(kb_data);
 
fpga_status = ioremap(PPC40x_FPGA_BASE, 8);
if (!fpga_status) {
printk(KERN_CRIT
   "walnut_setup_arch() fpga_status ioremap failed\n");
return;
}
 
fpga_enable = fpga_status + 1;
fpga_polarity = fpga_status + 2;
fpga_trigger = fpga_status + 3;
fpga_brdc = fpga_status + 4;
+   iounmap(fpga_status);
 
/* split the keyboard and mouse interrupts */
fpga_brdc_data = readb(fpga_brdc);
fpga_brdc_data |= 0x80;
writeb(fpga_brdc_data, fpga_brdc);
 
writeb(0x3, fpga_enable);
 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] ioremap/iounmap issue in yucca_setup_pcie_fpga_rootpoint(); arch/ppc/platforms/4xx/yucca.c

[Roel Kluin]

iounmap pcie_reg_fpga_base in default case

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/ppc/platforms/4xx/yucca.c b/arch/ppc/platforms/4xx/yucca.c
index a83b0ba..66a44ff 100644
--- a/arch/ppc/platforms/4xx/yucca.c
+++ b/arch/ppc/platforms/4xx/yucca.c
@@ -211,6 +211,7 @@ static void __init yucca_setup_pcie_fpga_rootpoint(int port)
break;
 
default:
+   iounmap(pcie_reg_fpga_base);
return;
}
 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] very similar, now in walnut_setup_arch(); arch/ppc/platforms/4xx/walnut.c

[Roel Kluin]

iounmap kb_data on error

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/ppc/platforms/4xx/walnut.c b/arch/ppc/platforms/4xx/walnut.c
index 2f97723..04d3f3f 100644
--- a/arch/ppc/platforms/4xx/walnut.c
+++ b/arch/ppc/platforms/4xx/walnut.c
@@ -81,22 +81,23 @@ walnut_setup_arch(void)
kb_data = ioremap(WALNUT_PS2_BASE, 8);
if (!kb_data) {
printk(KERN_CRIT
   "walnut_setup_arch() kb_data ioremap failed\n");
return;
}
 
kb_cs = kb_data + 1;
 
fpga_status = ioremap(PPC40x_FPGA_BASE, 8);
if (!fpga_status) {
+   iounmap(kb_data);
printk(KERN_CRIT
   "walnut_setup_arch() fpga_status ioremap failed\n");
return;
}
 
fpga_enable = fpga_status + 1;
fpga_polarity = fpga_status + 2;
fpga_trigger = fpga_status + 3;
fpga_brdc = fpga_status + 4;
 
/* split the keyboard and mouse interrupts */
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] another ioremap/iounmap issue in sycamore_setup_arch(); arch/ppc/platforms/4xx/sycamore.c

[Roel Kluin]

not yet tested
--
iounmap kb_data on error

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/ppc/platforms/4xx/sycamore.c 
b/arch/ppc/platforms/4xx/sycamore.c
index 8689f3e..c4ac63d 100644
--- a/arch/ppc/platforms/4xx/sycamore.c
+++ b/arch/ppc/platforms/4xx/sycamore.c
@@ -99,22 +99,23 @@ sycamore_setup_arch(void)
kb_data = ioremap(SYCAMORE_PS2_BASE, 8);
if (!kb_data) {
printk(KERN_CRIT
   "sycamore_setup_arch() kb_data ioremap failed\n");
return;
}
 
kb_cs = kb_data + 1;
 
fpga_status = ioremap(PPC40x_FPGA_BASE, 8);
if (!fpga_status) {
+   iounmap(kb_data);
printk(KERN_CRIT
   "sycamore_setup_arch() fpga_status ioremap failed\n");
return;
}
 
fpga_enable = fpga_status + 1;
fpga_polarity = fpga_status + 2;
fpga_trigger = fpga_status + 3;
fpga_brdc = fpga_status + 4;
 
/* split the keyboard and mouse interrupts */
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] mpc8xx_pic_init(); arch/powerpc/sysdev/mpc8xx_pic.c

[Roel Kluin]

Again: untested 
--
iounmap siu_reg on error

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c b/arch/powerpc/sysdev/mpc8xx_pic.c
index 7aa4ff5..1a51251 100644
--- a/arch/powerpc/sysdev/mpc8xx_pic.c
+++ b/arch/powerpc/sysdev/mpc8xx_pic.c
@@ -177,14 +177,15 @@ int mpc8xx_pic_init(void)
siu_reg = ioremap(res.start, res.end - res.start + 1);
if (siu_reg == NULL)
return -EINVAL;
 
mpc8xx_pic_host = irq_alloc_host(of_node_get(np), IRQ_HOST_MAP_LINEAR,
 64, &mpc8xx_pic_host_ops, 64);
if (mpc8xx_pic_host == NULL) {
+   iounmap(siu_reg);
printk(KERN_ERR "MPC8xx PIC: failed to allocate irq host!\n");
ret = -ENOMEM;
}
 
 out:
of_node_put(np);
return ret;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

inbalanced ioremap/iounmap? in cpm_pic_init(); arch/powerpc/sysdev/commproc.c

[Roel Kluin]

It appears to me that ioremap/iounmap in cpm_pic_init() is imbalanced. I am not
certain about this, nor was the patch tested. please review.
--
fix ioremap/iounmap imbalance

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/sysdev/commproc.c b/arch/powerpc/sysdev/commproc.c
index f6a6378..7b1dd9c 100644
--- a/arch/powerpc/sysdev/commproc.c
+++ b/arch/powerpc/sysdev/commproc.c
@@ -152,13 +152,13 @@ unsigned int cpm_pic_init(void)
cpic_reg = ioremap(res.start, res.end - res.start + 1);
if (cpic_reg == NULL)
goto end;
 
sirq = irq_of_parse_and_map(np, 0);
if (sirq == NO_IRQ)
-   goto end;
+   goto io_out;
 
/* Initialize the CPM interrupt controller. */
hwirq = (unsigned int)irq_map[sirq].hwirq;
out_be32(&cpic_reg->cpic_cicr,
(CICR_SCD_SCC4 | CICR_SCC_SCC3 | CICR_SCB_SCC2 | CICR_SCA_SCC1) |
((hwirq/2) << 13) | CICR_HP_MASK);
@@ -167,13 +167,13 @@ unsigned int cpm_pic_init(void)
 
cpm_pic_host = irq_alloc_host(of_node_get(np), IRQ_HOST_MAP_LINEAR,
  64, &cpm_pic_host_ops, 64);
if (cpm_pic_host == NULL) {
printk(KERN_ERR "CPM2 PIC: failed to allocate irq host!\n");
sirq = NO_IRQ;
-   goto end;
+   goto io_out;
}
 
/* Install our own error handler. */
np = of_find_compatible_node(NULL, NULL, "fsl,cpm1");
if (np == NULL)
np = of_find_node_by_type(NULL, "cpm");
@@ -187,13 +187,15 @@ unsigned int cpm_pic_init(void)
goto end;
 
if (setup_irq(eirq, &cpm_error_irqaction))
printk(KERN_ERR "Could not allocate CPM error IRQ!");
 
setbits32(&cpic_reg->cpic_cicr, CICR_IEN);
-
+   goto end;
+io_out:
+   iounmap(cpic_reg);
 end:
of_node_put(np);
return sirq;
 }
 
 void __init cpm_reset(void)
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] Balance alloc/free and ioremap/iounmap in gpio_mdio_probe (powerpc/platforms/pasemi/gpio_mdio.c)

[Roel Kluin]

Olof Johansson wrote:
> On Sun, Nov 04, 2007 at 05:53:40PM +0100, Roel Kluin wrote:
>> I think this is how it should be done, but
>> please review: it was not tested.
> 
> Hi,
> 
> Thanks for the bug report. The mdio driver needs a set of other cleanups
> as well. I have a more comprehensive patch that I'll post tomorrow after
> I have a chance to test them.

Ok, that's fine with me,

Roel

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] Balance alloc/free and ioremap/iounmap in gpio_mdio_probe (powerpc/platforms/pasemi/gpio_mdio.c)

[Roel Kluin]

Nathan Lynch wrote:
> Hi,

I moved res to the wrong function, that's fixed as well as the unlikely's and
the extra new lines.

Thanks for your quick response. Here is an updated version:
--
Upon errors gpio_regs was not iounmapped, and later priv nor new_bus->irq was
freed. Testing succes of the allocation of new_bus->irq was missing as well.

This patch creates a new function __gpio_mdio_register_bus to allow the 
iounmapping after an ioremap.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---

diff --git a/arch/powerpc/platforms/pasemi/gpio_mdio.c 
b/arch/powerpc/platforms/pasemi/gpio_mdio.c
index dae9f65..af5b241 100644
--- a/arch/powerpc/platforms/pasemi/gpio_mdio.c
+++ b/arch/powerpc/platforms/pasemi/gpio_mdio.c
@@ -208,68 +208,50 @@ static int gpio_mdio_write(struct mii_bus *bus, int 
phy_id, int location, u16 va
 }
 
 static int gpio_mdio_reset(struct mii_bus *bus)
 {
/*nothing here - dunno how to reset it*/
return 0;
 }
 
-
-static int __devinit gpio_mdio_probe(struct of_device *ofdev,
-const struct of_device_id *match)
+static int __devinit __gpio_mdio_register_bus(struct of_device *ofdev,
+   const struct of_device_id *match)
 {
struct device *dev = &ofdev->dev;
struct device_node *np = ofdev->node;
-   struct device_node *gpio_np;
struct mii_bus *new_bus;
-   struct resource res;
struct gpio_priv *priv;
const unsigned int *prop;
-   int err = 0;
int i;
 
-   gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
-
-   if (!gpio_np)
-   return -ENODEV;
-
-   err = of_address_to_resource(gpio_np, 0, &res);
-   of_node_put(gpio_np);
-
-   if (err)
-   return -EINVAL;
-
-   if (!gpio_regs)
-   gpio_regs = ioremap(res.start, 0x100);
-
-   if (!gpio_regs)
-   return -EPERM;
-
priv = kzalloc(sizeof(struct gpio_priv), GFP_KERNEL);
if (priv == NULL)
return -ENOMEM;
 
new_bus = kzalloc(sizeof(struct mii_bus), GFP_KERNEL);
 
if (new_bus == NULL)
-   return -ENOMEM;
+   goto free_priv;
 
new_bus->name = "pasemi gpio mdio bus",
new_bus->read = &gpio_mdio_read,
new_bus->write = &gpio_mdio_write,
new_bus->reset = &gpio_mdio_reset,
 
prop = of_get_property(np, "reg", NULL);
new_bus->id = *prop;
new_bus->priv = priv;
 
new_bus->phy_mask = 0;
 
new_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+   if (new_bus->irq == NULL)
+   goto free_new_bus;
+
for(i = 0; i < PHY_MAX_ADDR; ++i)
new_bus->irq[i] = irq_create_mapping(NULL, 10);
 
 
prop = of_get_property(np, "mdc-pin", NULL);
priv->mdc_pin = *prop;
 
prop = of_get_property(np, "mdio-pin", NULL);
@@ -284,17 +266,54 @@ static int __devinit gpio_mdio_probe(struct of_device 
*ofdev,
printk(KERN_ERR "%s: Cannot register as MDIO bus, err %d\n",
new_bus->name, err);
goto bus_register_fail;
}
 
return 0;
 
 bus_register_fail:
+   kfree(new_bus->irq);
+free_new_bus:
kfree(new_bus);
+free_priv:
+   kfree(priv);
+
+   return -ENOMEM;
+}
+
+
+static int __devinit gpio_mdio_probe(struct of_device *ofdev,
+const struct of_device_id *match)
+{
+   struct device_node *gpio_np;
+   struct resource res;
+   int err;
+
+   gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
+
+   if (!gpio_np)
+   return -ENODEV;
+
+   err = of_address_to_resource(gpio_np, 0, &res);
+   of_node_put(gpio_np);
+
+   if (err)
+   return -EINVAL;
+
+   if (!gpio_regs) {
+   gpio_regs = ioremap(res.start, 0x100);
+   if (unlikely(!gpio_regs))
+   return -EPERM;
+
+   err = __gpio_mdio_register_bus(ofdev, match);
+   if (err < 0)
+   iounmap(gpio_regs);
+   } else
+   err = __gpio_mdio_register_bus(ofdev, match);
 
return err;
 }
 
 
 static int gpio_mdio_remove(struct of_device *dev)
 {
struct mii_bus *bus = dev_get_drvdata(&dev->dev);
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] Balance alloc/free and ioremap/iounmap in gpio_mdio_probe (powerpc/platforms/pasemi/gpio_mdio.c)

[Roel Kluin]

I think this is how it should be done, but
please review: it was not tested.
--
Balance alloc/free and ioremap/iounmap

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/platforms/pasemi/gpio_mdio.c 
b/arch/powerpc/platforms/pasemi/gpio_mdio.c
index dae9f65..f250ba4 100644
--- a/arch/powerpc/platforms/pasemi/gpio_mdio.c
+++ b/arch/powerpc/platforms/pasemi/gpio_mdio.c
@@ -208,95 +208,116 @@ static int gpio_mdio_write(struct mii_bus *bus, int 
phy_id, int location, u16 va
 }
 
 static int gpio_mdio_reset(struct mii_bus *bus)
 {
/*nothing here - dunno how to reset it*/
return 0;
 }
 
-
-static int __devinit gpio_mdio_probe(struct of_device *ofdev,
-const struct of_device_id *match)
+static int __devinit __gpio_mdio_register_bus(struct of_device *ofdev,
+   const struct of_device_id *match)
 {
struct device *dev = &ofdev->dev;
struct device_node *np = ofdev->node;
-   struct device_node *gpio_np;
struct mii_bus *new_bus;
struct resource res;
struct gpio_priv *priv;
const unsigned int *prop;
-   int err = 0;
int i;
 
-   gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
-
-   if (!gpio_np)
-   return -ENODEV;
-
-   err = of_address_to_resource(gpio_np, 0, &res);
-   of_node_put(gpio_np);
-
-   if (err)
-   return -EINVAL;
-
-   if (!gpio_regs)
-   gpio_regs = ioremap(res.start, 0x100);
-
-   if (!gpio_regs)
-   return -EPERM;
-
priv = kzalloc(sizeof(struct gpio_priv), GFP_KERNEL);
-   if (priv == NULL)
+   if (unlikely(priv == NULL))
return -ENOMEM;
 
new_bus = kzalloc(sizeof(struct mii_bus), GFP_KERNEL);
 
-   if (new_bus == NULL)
-   return -ENOMEM;
+   if (unlikely(new_bus == NULL))
+   goto free_priv;
 
new_bus->name = "pasemi gpio mdio bus",
new_bus->read = &gpio_mdio_read,
new_bus->write = &gpio_mdio_write,
new_bus->reset = &gpio_mdio_reset,
 
prop = of_get_property(np, "reg", NULL);
new_bus->id = *prop;
new_bus->priv = priv;
 
new_bus->phy_mask = 0;
 
new_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+   if (unlikely(new_bus->irq == NULL))
+   goto free_new_bus;
+
for(i = 0; i < PHY_MAX_ADDR; ++i)
new_bus->irq[i] = irq_create_mapping(NULL, 10);
 
 
prop = of_get_property(np, "mdc-pin", NULL);
priv->mdc_pin = *prop;
 
prop = of_get_property(np, "mdio-pin", NULL);
priv->mdio_pin = *prop;
 
new_bus->dev = dev;
dev_set_drvdata(dev, new_bus);
 
err = mdiobus_register(new_bus);
 
-   if (0 != err) {
+   if (unlikely(0 != err)) {
printk(KERN_ERR "%s: Cannot register as MDIO bus, err %d\n",
new_bus->name, err);
goto bus_register_fail;
}
 
return 0;
 
 bus_register_fail:
+   kfree(new_bus->irq);
+free_new_bus:
kfree(new_bus);
+free_priv:
+   kfree(priv);
+
+   return -ENOMEM;
+}
+
+
+static int __devinit gpio_mdio_probe(struct of_device *ofdev,
+const struct of_device_id *match)
+{
+   struct device_node *gpio_np;
+   int err;
+
+   gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
+
+   if (!gpio_np)
+   return -ENODEV;
+
+   err = of_address_to_resource(gpio_np, 0, &res);
+   of_node_put(gpio_np);
+
+   if (err)
+   return -EINVAL;
+
+   if (!gpio_regs) {
+
+   gpio_regs = ioremap(res.start, 0x100);
+   if (unlikely(!gpio_regs))
+   return -EPERM;
+
+   err = __gpio_mdio_register_bus(ofdev, match);
+   if (err < 0)
+   iounmap(gpio_regs);
+   } else
+   err = __gpio_mdio_register_bus(ofdev, match);
 
return err;
+
 }
 
 
 static int gpio_mdio_remove(struct of_device *dev)
 {
struct mii_bus *bus = dev_get_drvdata(&dev->dev);
 
mdiobus_unregister(bus);

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] Free when ioremap fails in powerpc/platforms/52xx/mpc52xx_pci.c

[Roel Kluin]

Free hose when ioremap fails

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_pci.c 
b/arch/powerpc/platforms/52xx/mpc52xx_pci.c
index 4c6c82a..50f9655 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_pci.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_pci.c
@@ -395,8 +395,10 @@ mpc52xx_add_bridge(struct device_node *node)
hose->ops = &mpc52xx_pci_ops;
 
pci_regs = ioremap(rsrc.start, rsrc.end - rsrc.start + 1);
-   if (!pci_regs)
+   if (!pci_regs) {
+   pcibios_free_controller(hose);
return -ENOMEM;
+   }
 
pci_process_bridge_OF_ranges(hose, node, 1);
 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] allocation fix in ppc/platforms/4xx/luan.c

[Roel Kluin]

Don't allocate hose2 when when hose1 can't be allocated and free hose1 when
hose2 can't be allocated.

Signed-off-by: Roel Kluin <[EMAIL PROTECTED]>
---
diff --git a/arch/ppc/platforms/4xx/luan.c b/arch/ppc/platforms/4xx/luan.c
index 4b16961..b79ebb8 100644
--- a/arch/ppc/platforms/4xx/luan.c
+++ b/arch/ppc/platforms/4xx/luan.c
@@ -230,9 +230,14 @@ luan_setup_hoses(void)
 
/* Allocate hoses for PCIX1 and PCIX2 */
hose1 = pcibios_alloc_controller();
+   if (!hose1)
+   return;
+
hose2 = pcibios_alloc_controller();
-   if (!hose1 || !hose2)
+   if (!hose2) {
+   pcibios_free_controller(hose1);
return;
+   }
 
/* Setup PCIX1 */
hose1->first_busno = 0;
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev