| Message ID | 20120525082812.GA12238@faui49q.informatik.uni-erlangen.de |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2012-05-25 at 10:28 +0200, Christian Dietrich wrote: > nw_gpio_lock is a raw_spinlock_t, therefore raw_spin_lock_irqsave should be > used. it doesn't make a difference, while rlock is the first element of > spinlock_t. > > Also the check for machine_is_netwinder() is a double check of > CONFIG_ARCH_NETWINDER. > > Signed-off-by: Christian Dietrich <christian.dietrich@informatik.uni-erlangen.de> I do not understand this commit message. Please, split your patch on 2 patches and put a better to each one. Thanks! > --- > drivers/mtd/maps/dc21285.c | 13 +++++-------- > 1 files changed, 5 insertions(+), 8 deletions(-) > > diff --git a/drivers/mtd/maps/dc21285.c b/drivers/mtd/maps/dc21285.c > index 080f060..38fbf23 100644 > --- a/drivers/mtd/maps/dc21285.c > +++ b/drivers/mtd/maps/dc21285.c > @@ -38,9 +38,9 @@ static void nw_en_write(void) > * we want to write a bit pattern XXX1 to Xilinx to enable > * the write gate, which will be open for about the next 2ms. > */ > - spin_lock_irqsave(&nw_gpio_lock, flags); > + raw_spin_lock_irqsave(&nw_gpio_lock, flags); Please, make this to be a separate patch. And surely there are many other places in the kernel which need this conversion? > static void dc21285_write8(struct map_info *map, const map_word d, unsigned long adr) > { > - if (machine_is_netwinder()) > - nw_en_write(); > + nw_en_write(); > *CSR_ROMWRITEREG = adr & 3; I do not understand why this "if" statement is not needed? Why it was there in the first place and why you remove it? Please, describe this in the commit message.
diff --git a/drivers/mtd/maps/dc21285.c b/drivers/mtd/maps/dc21285.c index 080f060..38fbf23 100644 --- a/drivers/mtd/maps/dc21285.c +++ b/drivers/mtd/maps/dc21285.c @@ -38,9 +38,9 @@ static void nw_en_write(void) * we want to write a bit pattern XXX1 to Xilinx to enable * the write gate, which will be open for about the next 2ms. */ - spin_lock_irqsave(&nw_gpio_lock, flags); + raw_spin_lock_irqsave(&nw_gpio_lock, flags); nw_cpld_modify(CPLD_FLASH_WR_ENABLE, CPLD_FLASH_WR_ENABLE); - spin_unlock_irqrestore(&nw_gpio_lock, flags); + raw_spin_unlock_irqrestore(&nw_gpio_lock, flags); /* * let the ISA bus to catch on... @@ -79,8 +79,7 @@ static void dc21285_copy_from(struct map_info *map, void *to, unsigned long from static void dc21285_write8(struct map_info *map, const map_word d, unsigned long adr) { - if (machine_is_netwinder()) - nw_en_write(); + nw_en_write(); *CSR_ROMWRITEREG = adr & 3; adr &= ~3; *(uint8_t*)(map->virt + adr) = d.x[0]; @@ -88,8 +87,7 @@ static void dc21285_write8(struct map_info *map, const map_word d, unsigned long static void dc21285_write16(struct map_info *map, const map_word d, unsigned long adr) { - if (machine_is_netwinder()) - nw_en_write(); + nw_en_write(); *CSR_ROMWRITEREG = adr & 3; adr &= ~3; *(uint16_t*)(map->virt + adr) = d.x[0]; @@ -97,8 +95,7 @@ static void dc21285_write16(struct map_info *map, const map_word d, unsigned lon static void dc21285_write32(struct map_info *map, const map_word d, unsigned long adr) { - if (machine_is_netwinder()) - nw_en_write(); + nw_en_write(); *(uint32_t*)(map->virt + adr) = d.x[0]; }
nw_gpio_lock is a raw_spinlock_t, therefore raw_spin_lock_irqsave should be used. it doesn't make a difference, while rlock is the first element of spinlock_t. Also the check for machine_is_netwinder() is a double check of CONFIG_ARCH_NETWINDER. Signed-off-by: Christian Dietrich <christian.dietrich@informatik.uni-erlangen.de> --- drivers/mtd/maps/dc21285.c | 13 +++++-------- 1 files changed, 5 insertions(+), 8 deletions(-)