diff mbox

mtd: fsl-quadspi: change the obfuscate macro definition

Message ID 1445624308-27449-1-git-send-email-b45815@freescale.com
State Accepted
Commit 04850c4d8613127a9b488321c0ad83bff7519311
Headers show

Commit Message

Han Xu Oct. 23, 2015, 6:18 p.m. UTC 
Change the READ/WRITE to FSL_READ/FSL_WRITE to de-obfuscate the
concatenating string in LUT macro.

Signed-off-by: Han Xu <b45815@freescale.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Brian Norris Oct. 23, 2015, 6:42 p.m. UTC | #1 
On Fri, Oct 23, 2015 at 01:18:28PM -0500, Han Xu wrote:
> Change the READ/WRITE to FSL_READ/FSL_WRITE to de-obfuscate the
> concatenating string in LUT macro.

This isn't "de-obfuscating" anything. I was saying that your LUT0() and
LUT1() macros are obfuscating things by prepending LUT_ for you
automatically. That only saves you characters, but it makes it more
difficult to read (i.e., obfuscates) -- and incidentally, it also makes
this all more fragile, causing build errors.

So while this patch is probably OK, it's only fixing the build errors,
without actually improving readability.

I'm fine if that's the choice you'd rather make, but if so, I'll rewrite
the commit message for you.

Regards,
Brian

> Signed-off-by: Han Xu <b45815@freescale.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index dc38389..7b10ed4 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -155,15 +155,15 @@
>  #define LUT_MODE		4
>  #define LUT_MODE2		5
>  #define LUT_MODE4		6
> -#define LUT_READ		7
> -#define LUT_WRITE		8
> +#define LUT_FSL_READ		7
> +#define LUT_FSL_WRITE		8
>  #define LUT_JMP_ON_CS		9
>  #define LUT_ADDR_DDR		10
>  #define LUT_MODE_DDR		11
>  #define LUT_MODE2_DDR		12
>  #define LUT_MODE4_DDR		13
> -#define LUT_READ_DDR		14
> -#define LUT_WRITE_DDR		15
> +#define LUT_FSL_READ_DDR		14
> +#define LUT_FSL_WRITE_DDR		15
>  #define LUT_DATA_LEARN		16
>  
>  /*
> @@ -366,7 +366,7 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  
>  	writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
> -	writel(LUT0(DUMMY, PAD1, dummy) | LUT1(READ, PAD4, rxfifo),
> +	writel(LUT0(DUMMY, PAD1, dummy) | LUT1(FSL_READ, PAD4, rxfifo),
>  			base + QUADSPI_LUT(lut_base + 1));
>  
>  	/* Write enable */
> @@ -387,11 +387,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  
>  	writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
> -	writel(LUT0(WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
> +	writel(LUT0(FSL_WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
>  
>  	/* Read Status */
>  	lut_base = SEQID_RDSR * 4;
> -	writel(LUT0(CMD, PAD1, SPINOR_OP_RDSR) | LUT1(READ, PAD1, 0x1),
> +	writel(LUT0(CMD, PAD1, SPINOR_OP_RDSR) | LUT1(FSL_READ, PAD1, 0x1),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Erase a sector */
> @@ -410,17 +410,17 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  
>  	/* READ ID */
>  	lut_base = SEQID_RDID * 4;
> -	writel(LUT0(CMD, PAD1, SPINOR_OP_RDID) | LUT1(READ, PAD1, 0x8),
> +	writel(LUT0(CMD, PAD1, SPINOR_OP_RDID) | LUT1(FSL_READ, PAD1, 0x8),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Write Register */
>  	lut_base = SEQID_WRSR * 4;
> -	writel(LUT0(CMD, PAD1, SPINOR_OP_WRSR) | LUT1(WRITE, PAD1, 0x2),
> +	writel(LUT0(CMD, PAD1, SPINOR_OP_WRSR) | LUT1(FSL_WRITE, PAD1, 0x2),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Read Configuration Register */
>  	lut_base = SEQID_RDCR * 4;
> -	writel(LUT0(CMD, PAD1, SPINOR_OP_RDCR) | LUT1(READ, PAD1, 0x1),
> +	writel(LUT0(CMD, PAD1, SPINOR_OP_RDCR) | LUT1(FSL_READ, PAD1, 0x1),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Write disable */
> -- 
> 1.9.1
>
Han Xu Oct. 26, 2015, 9:31 p.m. UTC | #2 
On Fri, Oct 23, 2015 at 1:42 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Fri, Oct 23, 2015 at 01:18:28PM -0500, Han Xu wrote:
>> Change the READ/WRITE to FSL_READ/FSL_WRITE to de-obfuscate the
>> concatenating string in LUT macro.
>
> This isn't "de-obfuscating" anything. I was saying that your LUT0() and
> LUT1() macros are obfuscating things by prepending LUT_ for you
> automatically. That only saves you characters, but it makes it more
> difficult to read (i.e., obfuscates) -- and incidentally, it also makes
> this all more fragile, causing build errors.
>
> So while this patch is probably OK, it's only fixing the build errors,
> without actually improving readability.
>
> I'm fine if that's the choice you'd rather make, but if so, I'll rewrite
> the commit message for you.

I got it. This patch only fixed the build issue but didn't improve
readability, but modify the macro involves more code change for all
parameters with LUT_ prepending, so I prefer to keep this change.
Thanks.
>
> Regards,
> Brian
>
>> Signed-off-by: Han Xu <b45815@freescale.com>
>> ---
>>  drivers/mtd/spi-nor/fsl-quadspi.c | 20 ++++++++++----------
>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
>> index dc38389..7b10ed4 100644
>> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
>> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
>> @@ -155,15 +155,15 @@
>>  #define LUT_MODE             4
>>  #define LUT_MODE2            5
>>  #define LUT_MODE4            6
>> -#define LUT_READ             7
>> -#define LUT_WRITE            8
>> +#define LUT_FSL_READ         7
>> +#define LUT_FSL_WRITE                8
>>  #define LUT_JMP_ON_CS                9
>>  #define LUT_ADDR_DDR         10
>>  #define LUT_MODE_DDR         11
>>  #define LUT_MODE2_DDR                12
>>  #define LUT_MODE4_DDR                13
>> -#define LUT_READ_DDR         14
>> -#define LUT_WRITE_DDR                15
>> +#define LUT_FSL_READ_DDR             14
>> +#define LUT_FSL_WRITE_DDR            15
>>  #define LUT_DATA_LEARN               16
>>
>>  /*
>> @@ -366,7 +366,7 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>>
>>       writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
>>                       base + QUADSPI_LUT(lut_base));
>> -     writel(LUT0(DUMMY, PAD1, dummy) | LUT1(READ, PAD4, rxfifo),
>> +     writel(LUT0(DUMMY, PAD1, dummy) | LUT1(FSL_READ, PAD4, rxfifo),
>>                       base + QUADSPI_LUT(lut_base + 1));
>>
>>       /* Write enable */
>> @@ -387,11 +387,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>>
>>       writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
>>                       base + QUADSPI_LUT(lut_base));
>> -     writel(LUT0(WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
>> +     writel(LUT0(FSL_WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
>>
>>       /* Read Status */
>>       lut_base = SEQID_RDSR * 4;
>> -     writel(LUT0(CMD, PAD1, SPINOR_OP_RDSR) | LUT1(READ, PAD1, 0x1),
>> +     writel(LUT0(CMD, PAD1, SPINOR_OP_RDSR) | LUT1(FSL_READ, PAD1, 0x1),
>>                       base + QUADSPI_LUT(lut_base));
>>
>>       /* Erase a sector */
>> @@ -410,17 +410,17 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>>
>>       /* READ ID */
>>       lut_base = SEQID_RDID * 4;
>> -     writel(LUT0(CMD, PAD1, SPINOR_OP_RDID) | LUT1(READ, PAD1, 0x8),
>> +     writel(LUT0(CMD, PAD1, SPINOR_OP_RDID) | LUT1(FSL_READ, PAD1, 0x8),
>>                       base + QUADSPI_LUT(lut_base));
>>
>>       /* Write Register */
>>       lut_base = SEQID_WRSR * 4;
>> -     writel(LUT0(CMD, PAD1, SPINOR_OP_WRSR) | LUT1(WRITE, PAD1, 0x2),
>> +     writel(LUT0(CMD, PAD1, SPINOR_OP_WRSR) | LUT1(FSL_WRITE, PAD1, 0x2),
>>                       base + QUADSPI_LUT(lut_base));
>>
>>       /* Read Configuration Register */
>>       lut_base = SEQID_RDCR * 4;
>> -     writel(LUT0(CMD, PAD1, SPINOR_OP_RDCR) | LUT1(READ, PAD1, 0x1),
>> +     writel(LUT0(CMD, PAD1, SPINOR_OP_RDCR) | LUT1(FSL_READ, PAD1, 0x1),
>>                       base + QUADSPI_LUT(lut_base));
>>
>>       /* Write disable */
>> --
>> 1.9.1
>>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Brian Norris Oct. 26, 2015, 9:45 p.m. UTC | #3 
On Fri, Oct 23, 2015 at 01:18:28PM -0500, Han Xu wrote:
> Change the READ/WRITE to FSL_READ/FSL_WRITE to de-obfuscate the
> concatenating string in LUT macro.
> 
> Signed-off-by: Han Xu <b45815@freescale.com>

Rewrote the subject and commit message, since it was misleading and not
informative enough. Pushed to l2-mtd.git.

Brian

> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index dc38389..7b10ed4 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -155,15 +155,15 @@
>  #define LUT_MODE		4
>  #define LUT_MODE2		5
>  #define LUT_MODE4		6
> -#define LUT_READ		7
> -#define LUT_WRITE		8
> +#define LUT_FSL_READ		7
> +#define LUT_FSL_WRITE		8
>  #define LUT_JMP_ON_CS		9
>  #define LUT_ADDR_DDR		10
>  #define LUT_MODE_DDR		11
>  #define LUT_MODE2_DDR		12
>  #define LUT_MODE4_DDR		13
> -#define LUT_READ_DDR		14
> -#define LUT_WRITE_DDR		15
> +#define LUT_FSL_READ_DDR		14
> +#define LUT_FSL_WRITE_DDR		15
>  #define LUT_DATA_LEARN		16
>  
>  /*
> @@ -366,7 +366,7 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  
>  	writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
> -	writel(LUT0(DUMMY, PAD1, dummy) | LUT1(READ, PAD4, rxfifo),
> +	writel(LUT0(DUMMY, PAD1, dummy) | LUT1(FSL_READ, PAD4, rxfifo),
>  			base + QUADSPI_LUT(lut_base + 1));
>  
>  	/* Write enable */
> @@ -387,11 +387,11 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  
>  	writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
>  			base + QUADSPI_LUT(lut_base));
> -	writel(LUT0(WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
> +	writel(LUT0(FSL_WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
>  
>  	/* Read Status */
>  	lut_base = SEQID_RDSR * 4;
> -	writel(LUT0(CMD, PAD1, SPINOR_OP_RDSR) | LUT1(READ, PAD1, 0x1),
> +	writel(LUT0(CMD, PAD1, SPINOR_OP_RDSR) | LUT1(FSL_READ, PAD1, 0x1),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Erase a sector */
> @@ -410,17 +410,17 @@ static void fsl_qspi_init_lut(struct fsl_qspi *q)
>  
>  	/* READ ID */
>  	lut_base = SEQID_RDID * 4;
> -	writel(LUT0(CMD, PAD1, SPINOR_OP_RDID) | LUT1(READ, PAD1, 0x8),
> +	writel(LUT0(CMD, PAD1, SPINOR_OP_RDID) | LUT1(FSL_READ, PAD1, 0x8),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Write Register */
>  	lut_base = SEQID_WRSR * 4;
> -	writel(LUT0(CMD, PAD1, SPINOR_OP_WRSR) | LUT1(WRITE, PAD1, 0x2),
> +	writel(LUT0(CMD, PAD1, SPINOR_OP_WRSR) | LUT1(FSL_WRITE, PAD1, 0x2),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Read Configuration Register */
>  	lut_base = SEQID_RDCR * 4;
> -	writel(LUT0(CMD, PAD1, SPINOR_OP_RDCR) | LUT1(READ, PAD1, 0x1),
> +	writel(LUT0(CMD, PAD1, SPINOR_OP_RDCR) | LUT1(FSL_READ, PAD1, 0x1),
>  			base + QUADSPI_LUT(lut_base));
>  
>  	/* Write disable */
> -- 
> 1.9.1
>
diff mbox

Patch

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index dc38389..7b10ed4 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -155,15 +155,15 @@ 
 #define LUT_MODE		4
 #define LUT_MODE2		5
 #define LUT_MODE4		6
-#define LUT_READ		7
-#define LUT_WRITE		8
+#define LUT_FSL_READ		7
+#define LUT_FSL_WRITE		8
 #define LUT_JMP_ON_CS		9
 #define LUT_ADDR_DDR		10
 #define LUT_MODE_DDR		11
 #define LUT_MODE2_DDR		12
 #define LUT_MODE4_DDR		13
-#define LUT_READ_DDR		14
-#define LUT_WRITE_DDR		15
+#define LUT_FSL_READ_DDR		14
+#define LUT_FSL_WRITE_DDR		15
 #define LUT_DATA_LEARN		16
 
 /*
@@ -366,7 +366,7 @@  static void fsl_qspi_init_lut(struct fsl_qspi *q)
 
 	writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
 			base + QUADSPI_LUT(lut_base));
-	writel(LUT0(DUMMY, PAD1, dummy) | LUT1(READ, PAD4, rxfifo),
+	writel(LUT0(DUMMY, PAD1, dummy) | LUT1(FSL_READ, PAD4, rxfifo),
 			base + QUADSPI_LUT(lut_base + 1));
 
 	/* Write enable */
@@ -387,11 +387,11 @@  static void fsl_qspi_init_lut(struct fsl_qspi *q)
 
 	writel(LUT0(CMD, PAD1, cmd) | LUT1(ADDR, PAD1, addrlen),
 			base + QUADSPI_LUT(lut_base));
-	writel(LUT0(WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
+	writel(LUT0(FSL_WRITE, PAD1, 0), base + QUADSPI_LUT(lut_base + 1));
 
 	/* Read Status */
 	lut_base = SEQID_RDSR * 4;
-	writel(LUT0(CMD, PAD1, SPINOR_OP_RDSR) | LUT1(READ, PAD1, 0x1),
+	writel(LUT0(CMD, PAD1, SPINOR_OP_RDSR) | LUT1(FSL_READ, PAD1, 0x1),
 			base + QUADSPI_LUT(lut_base));
 
 	/* Erase a sector */
@@ -410,17 +410,17 @@  static void fsl_qspi_init_lut(struct fsl_qspi *q)
 
 	/* READ ID */
 	lut_base = SEQID_RDID * 4;
-	writel(LUT0(CMD, PAD1, SPINOR_OP_RDID) | LUT1(READ, PAD1, 0x8),
+	writel(LUT0(CMD, PAD1, SPINOR_OP_RDID) | LUT1(FSL_READ, PAD1, 0x8),
 			base + QUADSPI_LUT(lut_base));
 
 	/* Write Register */
 	lut_base = SEQID_WRSR * 4;
-	writel(LUT0(CMD, PAD1, SPINOR_OP_WRSR) | LUT1(WRITE, PAD1, 0x2),
+	writel(LUT0(CMD, PAD1, SPINOR_OP_WRSR) | LUT1(FSL_WRITE, PAD1, 0x2),
 			base + QUADSPI_LUT(lut_base));
 
 	/* Read Configuration Register */
 	lut_base = SEQID_RDCR * 4;
-	writel(LUT0(CMD, PAD1, SPINOR_OP_RDCR) | LUT1(READ, PAD1, 0x1),
+	writel(LUT0(CMD, PAD1, SPINOR_OP_RDCR) | LUT1(FSL_READ, PAD1, 0x1),
 			base + QUADSPI_LUT(lut_base));
 
 	/* Write disable */