Re: [PATCH v7] mtd: rawnand: use bit-wise majority to recover the contents of ONFI parameter

[Chris Moore]

Hi,

Le 16/05/2018 à 09:56, Boris Brezillon a écrit :

On Wed, 16 May 2018 09:32:57 +0200
Chris Moore <mo...@free.fr> wrote:


Hi,

Le 15/05/2018 à 09:34, Boris Brezillon a écrit :

On Tue, 15 May 2018 06:45:51 +0200
Chris Moore <mo...@free.fr> wrote:
  

Hi,

Le 13/05/2018 à 06:30, Wan, Jane (Nokia - US/Sunnyvale) a écrit :

Per ONFI specification (Rev. 4.0), if all parameter pages have invalid CRC 
values, the bit-wise majority may be used to recover the contents of the 
parameter pages from the parameter page copies present.

Signed-off-by: Jane Wan <jane@nokia.com>
---
v7: change debug print messages
v6: support the cases that srcbufs are not contiguous
v5: make the bit-wise majority functon generic
v4: move the bit-wise majority code in a separate function
v3: fix warning message detected by kbuild test robot
v2: rebase the changes on top of v4.17-rc1

drivers/mtd/nand/raw/nand_base.c |   50 ++

1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 72f3a89..b43b784 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5087,6 +5087,35 @@ static int nand_flash_detect_ext_param_page(struct 
nand_chip *chip,
}

/*

+ * Recover data with bit-wise majority
+ */
+static void nand_bit_wise_majority(const void **srcbufs,
+  unsigned int nsrcbufs,
+  void *dstbuf,
+  unsigned int bufsize)
+{
+   int i, j, k;
+
+   for (i = 0; i < bufsize; i++) {
+   u8 cnt, val;
+
+   val = 0;
+   for (j = 0; j < 8; j++) {
+   cnt = 0;
+   for (k = 0; k < nsrcbufs; k++) {
+   const u8 *srcbuf = srcbufs[k];
+
+   if (srcbuf[i] & BIT(j))
+   cnt++;
+   }
+   if (cnt > nsrcbufs / 2)
+   val |= BIT(j);
+   }
+   ((u8 *)dstbuf)[i] = val;
+   }
+}
+
+/*
 * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 
otherwise.
 */
static int nand_flash_detect_onfi(struct nand_chip *chip)
@@ -5102,7 +5131,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
return 0;

	/* ONFI chip: allocate a buffer to hold its parameter page */

-   p = kzalloc(sizeof(*p), GFP_KERNEL);
+   p = kzalloc((sizeof(*p) * 3), GFP_KERNEL);
if (!p)
return -ENOMEM;

@@ -5113,21 +5142,32 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)

}

	for (i = 0; i < 3; i++) {

-   ret = nand_read_data_op(chip, p, sizeof(*p), true);
+   ret = nand_read_data_op(chip, [i], sizeof(*p), true);
if (ret) {
ret = 0;
goto free_onfi_param_page;
}

-		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==

+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)[i], 254) ==
le16_to_cpu(p->crc)) {
+   if (i)
+   memcpy(p, [i], sizeof(*p));
break;
}
}

	if (i == 3) {

-   pr_err("Could not find valid ONFI parameter page; aborting\n");
-   goto free_onfi_param_page;
+   const void *srcbufs[3] = {p, p + 1, p + 2};
+
+   pr_warn("Could not find a valid ONFI parameter page, trying bit-wise 
majority to recover it\n");
+   nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
+  sizeof(*p));
+
+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254) !=
+   le16_to_cpu(p->crc)) {
+   pr_err("ONFI parameter recovery failed, aborting\n");
+   goto free_onfi_param_page;
+   }
}

	/* Check version */

This version is still hard coded for a three sample bitwise majority vote.
So why not use the method which I suggested previously for v2 and which
I repeat below?

Because I want the nand_bit_wise_majority() function to work with
nsrcbufs > 3 (the ONFI spec says there's at least 3 copy of the param
page, but NAND vendor can decide to put more). Also, if the X copies of
the PARAM are corrupted (which is rather unlikely), that means we
already spent quite a lot of time reading the different copies and
calculating the CRC, so I think we don't care about perf optimizations
when doing bit-wise majority.
  

The three sample bitwise majority can be implemented without bit level
manipulation using the identity:
majority3(a, b, c) = (a & 

Re: [PATCH v7] mtd: rawnand: use bit-wise majority to recover the contents of ONFI parameter

[Chris Moore]

Hi,

Le 16/05/2018 à 09:56, Boris Brezillon a écrit :

On Wed, 16 May 2018 09:32:57 +0200
Chris Moore  wrote:


Hi,

Le 15/05/2018 à 09:34, Boris Brezillon a écrit :

On Tue, 15 May 2018 06:45:51 +0200
Chris Moore  wrote:
  

Hi,

Le 13/05/2018 à 06:30, Wan, Jane (Nokia - US/Sunnyvale) a écrit :

Per ONFI specification (Rev. 4.0), if all parameter pages have invalid CRC 
values, the bit-wise majority may be used to recover the contents of the 
parameter pages from the parameter page copies present.

Signed-off-by: Jane Wan 
---
v7: change debug print messages
v6: support the cases that srcbufs are not contiguous
v5: make the bit-wise majority functon generic
v4: move the bit-wise majority code in a separate function
v3: fix warning message detected by kbuild test robot
v2: rebase the changes on top of v4.17-rc1

drivers/mtd/nand/raw/nand_base.c |   50 ++

1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 72f3a89..b43b784 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5087,6 +5087,35 @@ static int nand_flash_detect_ext_param_page(struct 
nand_chip *chip,
}

/*

+ * Recover data with bit-wise majority
+ */
+static void nand_bit_wise_majority(const void **srcbufs,
+  unsigned int nsrcbufs,
+  void *dstbuf,
+  unsigned int bufsize)
+{
+   int i, j, k;
+
+   for (i = 0; i < bufsize; i++) {
+   u8 cnt, val;
+
+   val = 0;
+   for (j = 0; j < 8; j++) {
+   cnt = 0;
+   for (k = 0; k < nsrcbufs; k++) {
+   const u8 *srcbuf = srcbufs[k];
+
+   if (srcbuf[i] & BIT(j))
+   cnt++;
+   }
+   if (cnt > nsrcbufs / 2)
+   val |= BIT(j);
+   }
+   ((u8 *)dstbuf)[i] = val;
+   }
+}
+
+/*
 * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 
otherwise.
 */
static int nand_flash_detect_onfi(struct nand_chip *chip)
@@ -5102,7 +5131,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
return 0;

	/* ONFI chip: allocate a buffer to hold its parameter page */

-   p = kzalloc(sizeof(*p), GFP_KERNEL);
+   p = kzalloc((sizeof(*p) * 3), GFP_KERNEL);
if (!p)
return -ENOMEM;

@@ -5113,21 +5142,32 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)

}

	for (i = 0; i < 3; i++) {

-   ret = nand_read_data_op(chip, p, sizeof(*p), true);
+   ret = nand_read_data_op(chip, [i], sizeof(*p), true);
if (ret) {
ret = 0;
goto free_onfi_param_page;
}

-		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==

+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)[i], 254) ==
le16_to_cpu(p->crc)) {
+   if (i)
+   memcpy(p, [i], sizeof(*p));
break;
}
}

	if (i == 3) {

-   pr_err("Could not find valid ONFI parameter page; aborting\n");
-   goto free_onfi_param_page;
+   const void *srcbufs[3] = {p, p + 1, p + 2};
+
+   pr_warn("Could not find a valid ONFI parameter page, trying bit-wise 
majority to recover it\n");
+   nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
+  sizeof(*p));
+
+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254) !=
+   le16_to_cpu(p->crc)) {
+   pr_err("ONFI parameter recovery failed, aborting\n");
+   goto free_onfi_param_page;
+   }
}

	/* Check version */

This version is still hard coded for a three sample bitwise majority vote.
So why not use the method which I suggested previously for v2 and which
I repeat below?

Because I want the nand_bit_wise_majority() function to work with
nsrcbufs > 3 (the ONFI spec says there's at least 3 copy of the param
page, but NAND vendor can decide to put more). Also, if the X copies of
the PARAM are corrupted (which is rather unlikely), that means we
already spent quite a lot of time reading the different copies and
calculating the CRC, so I think we don't care about perf optimizations
when doing bit-wise majority.
  

The three sample bitwise majority can be implemented without bit level
manipulation using the identity:
majority3(a, b, c) = (a & b) | (a & c) | (b & c)
This can be factorized slightly to (a & (b | 

Re: [PATCH v7] mtd: rawnand: use bit-wise majority to recover the contents of ONFI parameter

[Chris Moore]

Hi,

Le 15/05/2018 à 09:34, Boris Brezillon a écrit :

On Tue, 15 May 2018 06:45:51 +0200
Chris Moore <mo...@free.fr> wrote:


Hi,

Le 13/05/2018 à 06:30, Wan, Jane (Nokia - US/Sunnyvale) a écrit :

Per ONFI specification (Rev. 4.0), if all parameter pages have invalid CRC 
values, the bit-wise majority may be used to recover the contents of the 
parameter pages from the parameter page copies present.

Signed-off-by: Jane Wan <jane@nokia.com>
---
v7: change debug print messages
v6: support the cases that srcbufs are not contiguous
v5: make the bit-wise majority functon generic
v4: move the bit-wise majority code in a separate function
v3: fix warning message detected by kbuild test robot
v2: rebase the changes on top of v4.17-rc1
   
drivers/mtd/nand/raw/nand_base.c |   50 ++

   1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 72f3a89..b43b784 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5087,6 +5087,35 @@ static int nand_flash_detect_ext_param_page(struct 
nand_chip *chip,
   }
   
   /*

+ * Recover data with bit-wise majority
+ */
+static void nand_bit_wise_majority(const void **srcbufs,
+  unsigned int nsrcbufs,
+  void *dstbuf,
+  unsigned int bufsize)
+{
+   int i, j, k;
+
+   for (i = 0; i < bufsize; i++) {
+   u8 cnt, val;
+
+   val = 0;
+   for (j = 0; j < 8; j++) {
+   cnt = 0;
+   for (k = 0; k < nsrcbufs; k++) {
+   const u8 *srcbuf = srcbufs[k];
+
+   if (srcbuf[i] & BIT(j))
+   cnt++;
+   }
+   if (cnt > nsrcbufs / 2)
+   val |= BIT(j);
+   }
+   ((u8 *)dstbuf)[i] = val;
+   }
+}
+
+/*
* Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise.
*/
   static int nand_flash_detect_onfi(struct nand_chip *chip)
@@ -5102,7 +5131,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
return 0;
   
   	/* ONFI chip: allocate a buffer to hold its parameter page */

-   p = kzalloc(sizeof(*p), GFP_KERNEL);
+   p = kzalloc((sizeof(*p) * 3), GFP_KERNEL);
if (!p)
return -ENOMEM;
   
@@ -5113,21 +5142,32 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)

}
   
   	for (i = 0; i < 3; i++) {

-   ret = nand_read_data_op(chip, p, sizeof(*p), true);
+   ret = nand_read_data_op(chip, [i], sizeof(*p), true);
if (ret) {
ret = 0;
goto free_onfi_param_page;
}
   
-		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==

+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)[i], 254) ==
le16_to_cpu(p->crc)) {
+   if (i)
+   memcpy(p, [i], sizeof(*p));
break;
}
}
   
   	if (i == 3) {

-   pr_err("Could not find valid ONFI parameter page; aborting\n");
-   goto free_onfi_param_page;
+   const void *srcbufs[3] = {p, p + 1, p + 2};
+
+   pr_warn("Could not find a valid ONFI parameter page, trying bit-wise 
majority to recover it\n");
+   nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
+  sizeof(*p));
+
+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254) !=
+   le16_to_cpu(p->crc)) {
+   pr_err("ONFI parameter recovery failed, aborting\n");
+   goto free_onfi_param_page;
+   }
}
   
   	/* Check version */

This version is still hard coded for a three sample bitwise majority vote.
So why not use the method which I suggested previously for v2 and which
I repeat below?

Because I want the nand_bit_wise_majority() function to work with
nsrcbufs > 3 (the ONFI spec says there's at least 3 copy of the param
page, but NAND vendor can decide to put more). Also, if the X copies of
the PARAM are corrupted (which is rather unlikely), that means we
already spent quite a lot of time reading the different copies and
calculating the CRC, so I think we don't care about perf optimizations
when doing bit-wise majority.


The three sample bitwise majority can be implemented without bit level
manipulation using the identity:
majority3(a, b, c) = (a & b) | (a & c) | (b & c)
This can be factorized slightly to (a & (b | c)) | (b & c)
This enables the operation to be performed 8, 16, 32 or even 64 b

Re: [PATCH v7] mtd: rawnand: use bit-wise majority to recover the contents of ONFI parameter

[Chris Moore]

Hi,

Le 15/05/2018 à 09:34, Boris Brezillon a écrit :

On Tue, 15 May 2018 06:45:51 +0200
Chris Moore  wrote:


Hi,

Le 13/05/2018 à 06:30, Wan, Jane (Nokia - US/Sunnyvale) a écrit :

Per ONFI specification (Rev. 4.0), if all parameter pages have invalid CRC 
values, the bit-wise majority may be used to recover the contents of the 
parameter pages from the parameter page copies present.

Signed-off-by: Jane Wan 
---
v7: change debug print messages
v6: support the cases that srcbufs are not contiguous
v5: make the bit-wise majority functon generic
v4: move the bit-wise majority code in a separate function
v3: fix warning message detected by kbuild test robot
v2: rebase the changes on top of v4.17-rc1
   
drivers/mtd/nand/raw/nand_base.c |   50 ++

   1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 72f3a89..b43b784 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5087,6 +5087,35 @@ static int nand_flash_detect_ext_param_page(struct 
nand_chip *chip,
   }
   
   /*

+ * Recover data with bit-wise majority
+ */
+static void nand_bit_wise_majority(const void **srcbufs,
+  unsigned int nsrcbufs,
+  void *dstbuf,
+  unsigned int bufsize)
+{
+   int i, j, k;
+
+   for (i = 0; i < bufsize; i++) {
+   u8 cnt, val;
+
+   val = 0;
+   for (j = 0; j < 8; j++) {
+   cnt = 0;
+   for (k = 0; k < nsrcbufs; k++) {
+   const u8 *srcbuf = srcbufs[k];
+
+   if (srcbuf[i] & BIT(j))
+   cnt++;
+   }
+   if (cnt > nsrcbufs / 2)
+   val |= BIT(j);
+   }
+   ((u8 *)dstbuf)[i] = val;
+   }
+}
+
+/*
* Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise.
*/
   static int nand_flash_detect_onfi(struct nand_chip *chip)
@@ -5102,7 +5131,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
return 0;
   
   	/* ONFI chip: allocate a buffer to hold its parameter page */

-   p = kzalloc(sizeof(*p), GFP_KERNEL);
+   p = kzalloc((sizeof(*p) * 3), GFP_KERNEL);
if (!p)
return -ENOMEM;
   
@@ -5113,21 +5142,32 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)

}
   
   	for (i = 0; i < 3; i++) {

-   ret = nand_read_data_op(chip, p, sizeof(*p), true);
+   ret = nand_read_data_op(chip, [i], sizeof(*p), true);
if (ret) {
ret = 0;
goto free_onfi_param_page;
}
   
-		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==

+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)[i], 254) ==
le16_to_cpu(p->crc)) {
+   if (i)
+   memcpy(p, [i], sizeof(*p));
break;
}
}
   
   	if (i == 3) {

-   pr_err("Could not find valid ONFI parameter page; aborting\n");
-   goto free_onfi_param_page;
+   const void *srcbufs[3] = {p, p + 1, p + 2};
+
+   pr_warn("Could not find a valid ONFI parameter page, trying bit-wise 
majority to recover it\n");
+   nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
+  sizeof(*p));
+
+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254) !=
+   le16_to_cpu(p->crc)) {
+   pr_err("ONFI parameter recovery failed, aborting\n");
+   goto free_onfi_param_page;
+   }
}
   
   	/* Check version */

This version is still hard coded for a three sample bitwise majority vote.
So why not use the method which I suggested previously for v2 and which
I repeat below?

Because I want the nand_bit_wise_majority() function to work with
nsrcbufs > 3 (the ONFI spec says there's at least 3 copy of the param
page, but NAND vendor can decide to put more). Also, if the X copies of
the PARAM are corrupted (which is rather unlikely), that means we
already spent quite a lot of time reading the different copies and
calculating the CRC, so I think we don't care about perf optimizations
when doing bit-wise majority.


The three sample bitwise majority can be implemented without bit level
manipulation using the identity:
majority3(a, b, c) = (a & b) | (a & c) | (b & c)
This can be factorized slightly to (a & (b | c)) | (b & c)
This enables the operation to be performed 8, 16, 32 or even 64 bits at
a time depending on the hardware.

Re: [PATCH v7] mtd: rawnand: use bit-wise majority to recover the contents of ONFI parameter

[Chris Moore]

Hi,

Le 13/05/2018 à 06:30, Wan, Jane (Nokia - US/Sunnyvale) a écrit :

Per ONFI specification (Rev. 4.0), if all parameter pages have invalid CRC 
values, the bit-wise majority may be used to recover the contents of the 
parameter pages from the parameter page copies present.

Signed-off-by: Jane Wan 
---
v7: change debug print messages
v6: support the cases that srcbufs are not contiguous
v5: make the bit-wise majority functon generic
v4: move the bit-wise majority code in a separate function
v3: fix warning message detected by kbuild test robot
v2: rebase the changes on top of v4.17-rc1
  
drivers/mtd/nand/raw/nand_base.c |   50 ++

  1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 72f3a89..b43b784 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5087,6 +5087,35 @@ static int nand_flash_detect_ext_param_page(struct 
nand_chip *chip,
  }
  
  /*

+ * Recover data with bit-wise majority
+ */
+static void nand_bit_wise_majority(const void **srcbufs,
+  unsigned int nsrcbufs,
+  void *dstbuf,
+  unsigned int bufsize)
+{
+   int i, j, k;
+
+   for (i = 0; i < bufsize; i++) {
+   u8 cnt, val;
+
+   val = 0;
+   for (j = 0; j < 8; j++) {
+   cnt = 0;
+   for (k = 0; k < nsrcbufs; k++) {
+   const u8 *srcbuf = srcbufs[k];
+
+   if (srcbuf[i] & BIT(j))
+   cnt++;
+   }
+   if (cnt > nsrcbufs / 2)
+   val |= BIT(j);
+   }
+   ((u8 *)dstbuf)[i] = val;
+   }
+}
+
+/*
   * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise.
   */
  static int nand_flash_detect_onfi(struct nand_chip *chip)
@@ -5102,7 +5131,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
return 0;
  
  	/* ONFI chip: allocate a buffer to hold its parameter page */

-   p = kzalloc(sizeof(*p), GFP_KERNEL);
+   p = kzalloc((sizeof(*p) * 3), GFP_KERNEL);
if (!p)
return -ENOMEM;
  
@@ -5113,21 +5142,32 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)

}
  
  	for (i = 0; i < 3; i++) {

-   ret = nand_read_data_op(chip, p, sizeof(*p), true);
+   ret = nand_read_data_op(chip, [i], sizeof(*p), true);
if (ret) {
ret = 0;
goto free_onfi_param_page;
}
  
-		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==

+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)[i], 254) ==
le16_to_cpu(p->crc)) {
+   if (i)
+   memcpy(p, [i], sizeof(*p));
break;
}
}
  
  	if (i == 3) {

-   pr_err("Could not find valid ONFI parameter page; aborting\n");
-   goto free_onfi_param_page;
+   const void *srcbufs[3] = {p, p + 1, p + 2};
+
+   pr_warn("Could not find a valid ONFI parameter page, trying bit-wise 
majority to recover it\n");
+   nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
+  sizeof(*p));
+
+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254) !=
+   le16_to_cpu(p->crc)) {
+   pr_err("ONFI parameter recovery failed, aborting\n");
+   goto free_onfi_param_page;
+   }
}
  
  	/* Check version */


This version is still hard coded for a three sample bitwise majority vote.
So why not use the method which I suggested previously for v2 and which 
I repeat below?


The three sample bitwise majority can be implemented without bit level 
manipulation using the identity:

majority3(a, b, c) = (a & b) | (a & c) | (b & c)
This can be factorized slightly to (a & (b | c)) | (b & c)
This enables the operation to be performed 8, 16, 32 or even 64 bits at 
a time depending on the hardware.


This method is not only faster and but also more compact.

Cheers,
Chris

Re: [PATCH v7] mtd: rawnand: use bit-wise majority to recover the contents of ONFI parameter

[Chris Moore]

Hi,

Le 13/05/2018 à 06:30, Wan, Jane (Nokia - US/Sunnyvale) a écrit :

Per ONFI specification (Rev. 4.0), if all parameter pages have invalid CRC 
values, the bit-wise majority may be used to recover the contents of the 
parameter pages from the parameter page copies present.

Signed-off-by: Jane Wan 
---
v7: change debug print messages
v6: support the cases that srcbufs are not contiguous
v5: make the bit-wise majority functon generic
v4: move the bit-wise majority code in a separate function
v3: fix warning message detected by kbuild test robot
v2: rebase the changes on top of v4.17-rc1
  
drivers/mtd/nand/raw/nand_base.c |   50 ++

  1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 72f3a89..b43b784 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -5087,6 +5087,35 @@ static int nand_flash_detect_ext_param_page(struct 
nand_chip *chip,
  }
  
  /*

+ * Recover data with bit-wise majority
+ */
+static void nand_bit_wise_majority(const void **srcbufs,
+  unsigned int nsrcbufs,
+  void *dstbuf,
+  unsigned int bufsize)
+{
+   int i, j, k;
+
+   for (i = 0; i < bufsize; i++) {
+   u8 cnt, val;
+
+   val = 0;
+   for (j = 0; j < 8; j++) {
+   cnt = 0;
+   for (k = 0; k < nsrcbufs; k++) {
+   const u8 *srcbuf = srcbufs[k];
+
+   if (srcbuf[i] & BIT(j))
+   cnt++;
+   }
+   if (cnt > nsrcbufs / 2)
+   val |= BIT(j);
+   }
+   ((u8 *)dstbuf)[i] = val;
+   }
+}
+
+/*
   * Check if the NAND chip is ONFI compliant, returns 1 if it is, 0 otherwise.
   */
  static int nand_flash_detect_onfi(struct nand_chip *chip)
@@ -5102,7 +5131,7 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)
return 0;
  
  	/* ONFI chip: allocate a buffer to hold its parameter page */

-   p = kzalloc(sizeof(*p), GFP_KERNEL);
+   p = kzalloc((sizeof(*p) * 3), GFP_KERNEL);
if (!p)
return -ENOMEM;
  
@@ -5113,21 +5142,32 @@ static int nand_flash_detect_onfi(struct nand_chip *chip)

}
  
  	for (i = 0; i < 3; i++) {

-   ret = nand_read_data_op(chip, p, sizeof(*p), true);
+   ret = nand_read_data_op(chip, [i], sizeof(*p), true);
if (ret) {
ret = 0;
goto free_onfi_param_page;
}
  
-		if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==

+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)[i], 254) ==
le16_to_cpu(p->crc)) {
+   if (i)
+   memcpy(p, [i], sizeof(*p));
break;
}
}
  
  	if (i == 3) {

-   pr_err("Could not find valid ONFI parameter page; aborting\n");
-   goto free_onfi_param_page;
+   const void *srcbufs[3] = {p, p + 1, p + 2};
+
+   pr_warn("Could not find a valid ONFI parameter page, trying bit-wise 
majority to recover it\n");
+   nand_bit_wise_majority(srcbufs, ARRAY_SIZE(srcbufs), p,
+  sizeof(*p));
+
+   if (onfi_crc16(ONFI_CRC_BASE, (u8 *)p, 254) !=
+   le16_to_cpu(p->crc)) {
+   pr_err("ONFI parameter recovery failed, aborting\n");
+   goto free_onfi_param_page;
+   }
}
  
  	/* Check version */


This version is still hard coded for a three sample bitwise majority vote.
So why not use the method which I suggested previously for v2 and which 
I repeat below?


The three sample bitwise majority can be implemented without bit level 
manipulation using the identity:

majority3(a, b, c) = (a & b) | (a & c) | (b & c)
This can be factorized slightly to (a & (b | c)) | (b & c)
This enables the operation to be performed 8, 16, 32 or even 64 bits at 
a time depending on the hardware.


This method is not only faster and but also more compact.

Cheers,
Chris

Re: [PATCH v2 2/2] mtd: rawnand: fsl_ifc: use bit-wise majority to

[Chris Moore]

Hi,

Le 04/05/2018 à 04:09, Wan, Jane (Nokia - US/Sunnyvale) a écrit

The following is the reposting of patch with v2 version indication based on comment on 
"[PATCH 1/2]" (also in the attachment).

Subject: [PATCH v2 2/2] mtd: rawnand: fsl_ifc: use bit-wise majority to
  recover the contents of ONFI parameter

Per ONFI specification (Rev. 4.0), if all parameter pages have invalid
CRC values, the bit-wise majority may be used to recover the contents of
the parameter pages from the parameter page copies present.

Signed-off-by: Jane Wan 
---
  drivers/mtd/nand/raw/nand_base.c |   36 ++--
  1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 72f3a89..464c4fb 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c


[snip]

  
+		pr_info("Recover ONFI params with bit-wise majority\n");

+   for (j = 0; j < pagesize; j++) {
+   v = 0;
+   for (k = 0; k < 8; k++) {
+   m = 0;
+   for (l = 0; l < 3; l++)
+   m += GET_BIT(k, buf[l*pagesize + j]);
+   if (m > 1)
+   v |= BIT(k);
+   }
+   ((u8 *)p)[j] = v;
+   }


I am not familiar with the context of this but the three way bit-wise 
majority can be implemented much more efficiently  using the identity:

majority3(a, b, c) = (a & b) | (a & c) | (b & c)
This can be factorized slightly to (a & (b | c)) | (b & c)
This enables the operation to be performed 8, 16, 32 or even 64 bits at 
a time depending on the hardware.


Cheers,
Chris

Re: [PATCH v2 2/2] mtd: rawnand: fsl_ifc: use bit-wise majority to

[Chris Moore]

Hi,

Le 04/05/2018 à 04:09, Wan, Jane (Nokia - US/Sunnyvale) a écrit

The following is the reposting of patch with v2 version indication based on comment on 
"[PATCH 1/2]" (also in the attachment).

Subject: [PATCH v2 2/2] mtd: rawnand: fsl_ifc: use bit-wise majority to
  recover the contents of ONFI parameter

Per ONFI specification (Rev. 4.0), if all parameter pages have invalid
CRC values, the bit-wise majority may be used to recover the contents of
the parameter pages from the parameter page copies present.

Signed-off-by: Jane Wan 
---
  drivers/mtd/nand/raw/nand_base.c |   36 ++--
  1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 72f3a89..464c4fb 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c


[snip]

  
+		pr_info("Recover ONFI params with bit-wise majority\n");

+   for (j = 0; j < pagesize; j++) {
+   v = 0;
+   for (k = 0; k < 8; k++) {
+   m = 0;
+   for (l = 0; l < 3; l++)
+   m += GET_BIT(k, buf[l*pagesize + j]);
+   if (m > 1)
+   v |= BIT(k);
+   }
+   ((u8 *)p)[j] = v;
+   }


I am not familiar with the context of this but the three way bit-wise 
majority can be implemented much more efficiently  using the identity:

majority3(a, b, c) = (a & b) | (a & c) | (b & c)
This can be factorized slightly to (a & (b | c)) | (b & c)
This enables the operation to be performed 8, 16, 32 or even 64 bits at 
a time depending on the hardware.


Cheers,
Chris

Re: [PATCH v3 4/4] clk: meson: gxbb-aoclk: Add CEC 32k clock

[Chris Moore]

Hi,

Le 31/07/2017 à 14:42, Neil Armstrong a écrit :

The CEC 32K AO Clock is a dual divider with dual counter to provide a more
precise 32768Hz clock for the CEC subsystem from the external xtal.

Signed-off-by: Neil Armstrong 
---
  drivers/clk/meson/Makefile |   2 +-
  drivers/clk/meson/gxbb-aoclk-32k.c | 194 +
  drivers/clk/meson/gxbb-aoclk.c |  21 +++-
  drivers/clk/meson/gxbb-aoclk.h |  16 +++
  4 files changed, 231 insertions(+), 2 deletions(-)
  create mode 100644 drivers/clk/meson/gxbb-aoclk-32k.c

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index de65427..b139d41 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -4,4 +4,4 @@
  
  obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-cpu.o clk-mpll.o clk-audio-divider.o

  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
-obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o gxbb-aoclk.o gxbb-aoclk-regmap.o
+obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o gxbb-aoclk.o gxbb-aoclk-regmap.o 
gxbb-aoclk-32k.o
diff --git a/drivers/clk/meson/gxbb-aoclk-32k.c 
b/drivers/clk/meson/gxbb-aoclk-32k.c


[snip]


+static int aoclk_cec_32k_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+   const struct cec_32k_freq_table *freq = find_cec_32k_freq(rate,
+ parent_rate);
+   struct aoclk_cec_32k *cec_32k = to_aoclk_cec_32k(hw);
+   u32 reg = 0;
+
+   if (!freq)
+   return -EINVAL;
+
+   /* Disable clock */
+   regmap_update_bits(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0,
+  CLK_CNTL0_IN_GATE_EN | CLK_CNTL0_OUT_GATE_EN, 0);
+
+   reg = FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1);
+   if (freq->dualdiv)
+   reg |= CLK_CNTL0_DUALDIV_EN |
+  FIELD_PREP(CLK_CNTL0_N2_MASK, freq->n2 - 1);
+
+   regmap_write(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0, reg);
+
+   reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1);
+   if (freq->dualdiv)
+   reg = FIELD_PREP(CLK_CNTL1_M2_MASK, freq->m2 - 1);


s/=/|=/

Cheers,
Chris

Re: [PATCH v3 4/4] clk: meson: gxbb-aoclk: Add CEC 32k clock

[Chris Moore]

Hi,

Le 31/07/2017 à 14:42, Neil Armstrong a écrit :

The CEC 32K AO Clock is a dual divider with dual counter to provide a more
precise 32768Hz clock for the CEC subsystem from the external xtal.

Signed-off-by: Neil Armstrong 
---
  drivers/clk/meson/Makefile |   2 +-
  drivers/clk/meson/gxbb-aoclk-32k.c | 194 +
  drivers/clk/meson/gxbb-aoclk.c |  21 +++-
  drivers/clk/meson/gxbb-aoclk.h |  16 +++
  4 files changed, 231 insertions(+), 2 deletions(-)
  create mode 100644 drivers/clk/meson/gxbb-aoclk-32k.c

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index de65427..b139d41 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -4,4 +4,4 @@
  
  obj-$(CONFIG_COMMON_CLK_AMLOGIC) += clk-pll.o clk-cpu.o clk-mpll.o clk-audio-divider.o

  obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o
-obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o gxbb-aoclk.o gxbb-aoclk-regmap.o
+obj-$(CONFIG_COMMON_CLK_GXBB)   += gxbb.o gxbb-aoclk.o gxbb-aoclk-regmap.o 
gxbb-aoclk-32k.o
diff --git a/drivers/clk/meson/gxbb-aoclk-32k.c 
b/drivers/clk/meson/gxbb-aoclk-32k.c


[snip]


+static int aoclk_cec_32k_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+   const struct cec_32k_freq_table *freq = find_cec_32k_freq(rate,
+ parent_rate);
+   struct aoclk_cec_32k *cec_32k = to_aoclk_cec_32k(hw);
+   u32 reg = 0;
+
+   if (!freq)
+   return -EINVAL;
+
+   /* Disable clock */
+   regmap_update_bits(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0,
+  CLK_CNTL0_IN_GATE_EN | CLK_CNTL0_OUT_GATE_EN, 0);
+
+   reg = FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1);
+   if (freq->dualdiv)
+   reg |= CLK_CNTL0_DUALDIV_EN |
+  FIELD_PREP(CLK_CNTL0_N2_MASK, freq->n2 - 1);
+
+   regmap_write(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0, reg);
+
+   reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1);
+   if (freq->dualdiv)
+   reg = FIELD_PREP(CLK_CNTL1_M2_MASK, freq->m2 - 1);


s/=/|=/

Cheers,
Chris

Re: [PATCH v2 4/4] clk: meson: gxbb-aoclk: Add CEC 32k clock

[Chris Moore]

Hi,

Sorry I forgot to reply to all in my previous post :(
I hope this corrects things.

Le 28/07/2017 à 11:53, Neil Armstrong a écrit :

[snip]

+static long aoclk_cec_32k_round_rate(struct clk_hw *hw, unsigned long 
rate,

+ unsigned long *prate)
+{
+const struct cec_32k_freq_table *freq = find_cec_32k_freq(rate,
+  *prate);
+
+/* If invalid return first one */
+if (!freq)
+return freq[0].target_rate;


Wouldn't this dereference a null pointer (or am I being stupid this 
morning)?



+
+return freq->target_rate;
+}
+
+static int aoclk_cec_32k_set_rate(struct clk_hw *hw, unsigned long rate,
+  unsigned long parent_rate)
+{
+const struct cec_32k_freq_table *freq = find_cec_32k_freq(rate,
+  parent_rate);
+struct aoclk_cec_32k *cec_32k = to_aoclk_cec_32k(hw);
+u32 reg = 0;
+
+if (!freq)
+return -EINVAL;
+
+/* Disable clock */
+regmap_update_bits(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0,
+   CLK_CNTL0_IN_GATE_EN | CLK_CNTL0_OUT_GATE_EN, 0);
+
+if (freq->dualdiv)
+reg = CLK_CNTL0_DUALDIV_EN |
+  FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1) |
+  FIELD_PREP(CLK_CNTL0_N2_MASK, freq->n2 - 1);
+else
+reg = FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1);
+


Suggestion:

+reg = FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1);
+if (freq->dualdiv)
+reg |= CLK_CNTL0_DUALDIV_EN |
+   FIELD_PREP(CLK_CNTL0_N2_MASK, freq->n2 - 1);

is shorter but maybe generates the same code.


+ regmap_write(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0, reg);
+
+if (freq->dualdiv)
+reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1) |
+  FIELD_PREP(CLK_CNTL1_M2_MASK, freq->m2 - 1);
+else
+reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1);
+


Idem:

+reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1);
+if (freq->dualdiv)
+reg |= FIELD_PREP(CLK_CNTL1_M2_MASK, freq->m2 - 1);

Cheers,
Chris

Re: [PATCH v2 4/4] clk: meson: gxbb-aoclk: Add CEC 32k clock

[Chris Moore]

Hi,

Sorry I forgot to reply to all in my previous post :(
I hope this corrects things.

Le 28/07/2017 à 11:53, Neil Armstrong a écrit :

[snip]

+static long aoclk_cec_32k_round_rate(struct clk_hw *hw, unsigned long 
rate,

+ unsigned long *prate)
+{
+const struct cec_32k_freq_table *freq = find_cec_32k_freq(rate,
+  *prate);
+
+/* If invalid return first one */
+if (!freq)
+return freq[0].target_rate;


Wouldn't this dereference a null pointer (or am I being stupid this 
morning)?



+
+return freq->target_rate;
+}
+
+static int aoclk_cec_32k_set_rate(struct clk_hw *hw, unsigned long rate,
+  unsigned long parent_rate)
+{
+const struct cec_32k_freq_table *freq = find_cec_32k_freq(rate,
+  parent_rate);
+struct aoclk_cec_32k *cec_32k = to_aoclk_cec_32k(hw);
+u32 reg = 0;
+
+if (!freq)
+return -EINVAL;
+
+/* Disable clock */
+regmap_update_bits(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0,
+   CLK_CNTL0_IN_GATE_EN | CLK_CNTL0_OUT_GATE_EN, 0);
+
+if (freq->dualdiv)
+reg = CLK_CNTL0_DUALDIV_EN |
+  FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1) |
+  FIELD_PREP(CLK_CNTL0_N2_MASK, freq->n2 - 1);
+else
+reg = FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1);
+


Suggestion:

+reg = FIELD_PREP(CLK_CNTL0_N1_MASK, freq->n1 - 1);
+if (freq->dualdiv)
+reg |= CLK_CNTL0_DUALDIV_EN |
+   FIELD_PREP(CLK_CNTL0_N2_MASK, freq->n2 - 1);

is shorter but maybe generates the same code.


+ regmap_write(cec_32k->regmap, AO_RTC_ALT_CLK_CNTL0, reg);
+
+if (freq->dualdiv)
+reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1) |
+  FIELD_PREP(CLK_CNTL1_M2_MASK, freq->m2 - 1);
+else
+reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1);
+


Idem:

+reg = FIELD_PREP(CLK_CNTL1_M1_MASK, freq->m1 - 1);
+if (freq->dualdiv)
+reg |= FIELD_PREP(CLK_CNTL1_M2_MASK, freq->m2 - 1);

Cheers,
Chris

Re: [PATCH v4 2/7] tty/serial: meson_uart: update to stable bindings

[Chris Moore]

Le 09/06/2017 à 11:49, Neil Armstrong a écrit :

From: Helmut Klein 

This patch handle the stable UART bindings but also keeps compatibility
with the legacy non-stable bindings until all boards uses them.

Signed-off-by: Helmut Klein 
Signed-off-by: Neil Armstrong 
---
  drivers/tty/serial/meson_uart.c | 109 +---
  1 file changed, 103 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 60f1679..d2c8136 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c


[snip]


+static int meson_uart_probe_clocks(struct platform_device *pdev,
+  struct uart_port *port)
+{
+   struct clk *clk_xtal = NULL;
+   struct clk *clk_pclk = NULL;
+   struct clk *clk_baud = NULL;
+   int ret;
+
+   clk_pclk = devm_clk_get(>dev, "pclk");
+   if (IS_ERR(clk_pclk))
+   return PTR_ERR(clk_pclk);
+
+   clk_xtal = devm_clk_get(>dev, "xtal");
+   if (IS_ERR(clk_xtal))
+   return PTR_ERR(clk_xtal);
+
+   clk_baud = devm_clk_get(>dev, "baud");
+   if (IS_ERR(clk_xtal))


Copy/paste error: s/clk_xtal/clk_baud/


+   return PTR_ERR(clk_baud);

Re: [PATCH v4 2/7] tty/serial: meson_uart: update to stable bindings

[Chris Moore]

Le 09/06/2017 à 11:49, Neil Armstrong a écrit :

From: Helmut Klein 

This patch handle the stable UART bindings but also keeps compatibility
with the legacy non-stable bindings until all boards uses them.

Signed-off-by: Helmut Klein 
Signed-off-by: Neil Armstrong 
---
  drivers/tty/serial/meson_uart.c | 109 +---
  1 file changed, 103 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 60f1679..d2c8136 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c


[snip]


+static int meson_uart_probe_clocks(struct platform_device *pdev,
+  struct uart_port *port)
+{
+   struct clk *clk_xtal = NULL;
+   struct clk *clk_pclk = NULL;
+   struct clk *clk_baud = NULL;
+   int ret;
+
+   clk_pclk = devm_clk_get(>dev, "pclk");
+   if (IS_ERR(clk_pclk))
+   return PTR_ERR(clk_pclk);
+
+   clk_xtal = devm_clk_get(>dev, "xtal");
+   if (IS_ERR(clk_xtal))
+   return PTR_ERR(clk_xtal);
+
+   clk_baud = devm_clk_get(>dev, "baud");
+   if (IS_ERR(clk_xtal))


Copy/paste error: s/clk_xtal/clk_baud/


+   return PTR_ERR(clk_baud);

Re: [PATCH v2 02/18] arm64: dts: amlogic: Sort Makefile

[Chris Moore]

Le 13/05/2017 à 16:33, Andreas Färber a écrit :

Sort the .dtb files alphabetically to make clear where to add new ones.

Signed-off-by: Andreas Färber 
---
  v1 -> v2:
  * Rebased (new boards added)
  * Extended commit message
  
  arch/arm64/boot/dts/amlogic/Makefile | 6 +++---

  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/Makefile 
b/arch/arm64/boot/dts/amlogic/Makefile
index b9ad2db7398b..14fa27ccd589 100644
--- a/arch/arm64/boot/dts/amlogic/Makefile
+++ b/arch/arm64/boot/dts/amlogic/Makefile
@@ -7,15 +7,15 @@ dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-vega-s95-meta.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-vega-s95-telos.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-wetek-hub.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-wetek-play2.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-hwacom-amazetv.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-khadas-vim.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-nexbox-a95x.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-p212.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905d-p230.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905d-p231.dtb


s905d should be before s905x if you are imposing alphabetical order.


-dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-hwacom-amazetv.dtb
-dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-nexbox-a95x.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-gxm-nexbox-a1.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxm-q200.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxm-q201.dtb
-dtb-$(CONFIG_ARCH_MESON) += meson-gxm-nexbox-a1.dtb
  
  always		:= $(dtb-y)

  subdir-y  := $(dts-dirs)

Re: [PATCH v2 02/18] arm64: dts: amlogic: Sort Makefile

[Chris Moore]

Le 13/05/2017 à 16:33, Andreas Färber a écrit :

Sort the .dtb files alphabetically to make clear where to add new ones.

Signed-off-by: Andreas Färber 
---
  v1 -> v2:
  * Rebased (new boards added)
  * Extended commit message
  
  arch/arm64/boot/dts/amlogic/Makefile | 6 +++---

  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/Makefile 
b/arch/arm64/boot/dts/amlogic/Makefile
index b9ad2db7398b..14fa27ccd589 100644
--- a/arch/arm64/boot/dts/amlogic/Makefile
+++ b/arch/arm64/boot/dts/amlogic/Makefile
@@ -7,15 +7,15 @@ dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-vega-s95-meta.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-vega-s95-telos.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-wetek-hub.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-wetek-play2.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-hwacom-amazetv.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-khadas-vim.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-nexbox-a95x.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-p212.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905d-p230.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905d-p231.dtb


s905d should be before s905x if you are imposing alphabetical order.


-dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-hwacom-amazetv.dtb
-dtb-$(CONFIG_ARCH_MESON) += meson-gxl-s905x-nexbox-a95x.dtb
+dtb-$(CONFIG_ARCH_MESON) += meson-gxm-nexbox-a1.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxm-q200.dtb
  dtb-$(CONFIG_ARCH_MESON) += meson-gxm-q201.dtb
-dtb-$(CONFIG_ARCH_MESON) += meson-gxm-nexbox-a1.dtb
  
  always		:= $(dtb-y)

  subdir-y  := $(dts-dirs)

Re: [PATCH 0/3] staging: media: Replace a bit shift.

[Chris Moore]

Hi,

Le 22/03/2017 à 05:26, Arushi Singhal a écrit :

Replace a bit shift by a use of BIT in media driver.

Arushi Singhal (3):
   staging: media: Replace a bit shift by a use of BIT.
   staging: media: davinci_vpfe: Replace a bit shift by a use of BIT.
   staging: media: omap4iss: Replace a bit shift by a use of BIT.

  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 12 +-
  .../atomisp/pci/atomisp2/atomisp_compat_css20.c|  6 ++---
  .../media/atomisp/pci/atomisp2/atomisp_drvfs.c |  6 ++---
  .../media/atomisp/pci/atomisp2/atomisp_v4l2.c  | 18 +++
  .../media/atomisp/pci/atomisp2/hmm/hmm_bo.c|  2 +-
  drivers/staging/media/davinci_vpfe/dm365_ipipe.c   |  2 +-
  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c |  2 +-
  drivers/staging/media/davinci_vpfe/dm365_isif.c| 26 +++---
  drivers/staging/media/davinci_vpfe/dm365_resizer.c |  6 ++---
  drivers/staging/media/omap4iss/iss_csi2.c  |  2 +-
  drivers/staging/media/omap4iss/iss_ipipe.c |  2 +-
  drivers/staging/media/omap4iss/iss_ipipeif.c   |  2 +-
  drivers/staging/media/omap4iss/iss_resizer.c   |  2 +-
  13 files changed, 44 insertions(+), 44 deletions(-)



Most of these replacements add redundant parentheses around the BIT macro.
IMHO this makes the code less readable.
So I suggest (BIT(c)) -> BIT(c).

Cheers,
Chris

Re: [PATCH 0/3] staging: media: Replace a bit shift.

[Chris Moore]

Hi,

Le 22/03/2017 à 05:26, Arushi Singhal a écrit :

Replace a bit shift by a use of BIT in media driver.

Arushi Singhal (3):
   staging: media: Replace a bit shift by a use of BIT.
   staging: media: davinci_vpfe: Replace a bit shift by a use of BIT.
   staging: media: omap4iss: Replace a bit shift by a use of BIT.

  .../media/atomisp/pci/atomisp2/atomisp_cmd.c   | 12 +-
  .../atomisp/pci/atomisp2/atomisp_compat_css20.c|  6 ++---
  .../media/atomisp/pci/atomisp2/atomisp_drvfs.c |  6 ++---
  .../media/atomisp/pci/atomisp2/atomisp_v4l2.c  | 18 +++
  .../media/atomisp/pci/atomisp2/hmm/hmm_bo.c|  2 +-
  drivers/staging/media/davinci_vpfe/dm365_ipipe.c   |  2 +-
  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c |  2 +-
  drivers/staging/media/davinci_vpfe/dm365_isif.c| 26 +++---
  drivers/staging/media/davinci_vpfe/dm365_resizer.c |  6 ++---
  drivers/staging/media/omap4iss/iss_csi2.c  |  2 +-
  drivers/staging/media/omap4iss/iss_ipipe.c |  2 +-
  drivers/staging/media/omap4iss/iss_ipipeif.c   |  2 +-
  drivers/staging/media/omap4iss/iss_resizer.c   |  2 +-
  13 files changed, 44 insertions(+), 44 deletions(-)



Most of these replacements add redundant parentheses around the BIT macro.
IMHO this makes the code less readable.
So I suggest (BIT(c)) -> BIT(c).

Cheers,
Chris