On 24/01/15 22:28, Semen Protsenko wrote: > Some GPMC_CONFIG7 register bits marked as "RESERVED", means they > shouldn't be overwritten. A typical approach to handle such bits called > "Read-Modify-Write". Writing procedure used in gpmc_cs_set_memconf() > utilizes RMW technique, but implemented incorrectly. Due to obvious typo > in code read register value is being rewritten by another value, which > leads to loss of read RESERVED bits. This patch fixes this. > > While at it, replace magic numbers with named constants to improve code > readability. > > Signed-off-by: Semen Protsenko <semen.protsenko@xxxxxxxxxxxxxxx> This is much nicer. Acked-by: Roger Quadros <rogerq@xxxxxx> cheers, -roger > --- > drivers/memory/omap-gpmc.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/memory/omap-gpmc.c b/drivers/memory/omap-gpmc.c > index 24696f5..10eb4ac 100644 > --- a/drivers/memory/omap-gpmc.c > +++ b/drivers/memory/omap-gpmc.c > @@ -153,6 +153,15 @@ > #define GPMC_CONFIG1_FCLK_DIV4 (GPMC_CONFIG1_FCLK_DIV(3)) > #define GPMC_CONFIG7_CSVALID (1 << 6) > > +#define GPMC_CONFIG7_BASEADDRESS_MASK 0x3f > +#define GPMC_CONFIG7_CSVALID_MASK BIT(6) > +#define GPMC_CONFIG7_MASKADDRESS_OFFSET 8 > +#define GPMC_CONFIG7_MASKADDRESS_MASK (0xf << GPMC_CONFIG7_MASKADDRESS_OFFSET) > +/* All CONFIG7 bits except reserved bits */ > +#define GPMC_CONFIG7_MASK (GPMC_CONFIG7_BASEADDRESS_MASK | \ > + GPMC_CONFIG7_CSVALID_MASK | \ > + GPMC_CONFIG7_MASKADDRESS_MASK) > + > #define GPMC_DEVICETYPE_NOR 0 > #define GPMC_DEVICETYPE_NAND 2 > #define GPMC_CONFIG_WRITEPROTECT 0x00000010 > @@ -586,12 +595,15 @@ static int gpmc_cs_set_memconf(int cs, u32 base, u32 size) > if (base & (size - 1)) > return -EINVAL; > > + base >>= GPMC_CHUNK_SHIFT; > mask = (1 << GPMC_SECTION_SHIFT) - size; > + mask >>= GPMC_CHUNK_SHIFT; > + mask <<= GPMC_CONFIG7_MASKADDRESS_OFFSET; > + > l = gpmc_cs_read_reg(cs, GPMC_CS_CONFIG7); > - l &= ~0x3f; > - l = (base >> GPMC_CHUNK_SHIFT) & 0x3f; > - l &= ~(0x0f << 8); > - l |= ((mask >> GPMC_CHUNK_SHIFT) & 0x0f) << 8; > + l &= ~GPMC_CONFIG7_MASK; > + l |= base & GPMC_CONFIG7_BASEADDRESS_MASK; > + l |= mask & GPMC_CONFIG7_MASKADDRESS_MASK; > l |= GPMC_CONFIG7_CSVALID; > gpmc_cs_write_reg(cs, GPMC_CS_CONFIG7, l); > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
