subject:"\[PATCH v3 1\/1\] mtd\: nand\: pxa3xx\: Set FORCE

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-30 Thread Chris Packham

Hi Miquel,

On 09/10/17 19:19, Miquel RAYNAL wrote:
> Hello Kalyan,
> 
> On Mon, 9 Oct 2017 02:31:30 +
> Kalyan Kinthada  wrote:
> 
>> On 05/10/17 20:41, Miquel RAYNAL wrote:
>>> Hello Kalyan,
>>>
>>> On Thu, 28 Sep 2017 13:57:56 +1300
>>> Kalyan Kinthada  wrote:
>>>   
 When the arbitration between NOR and NAND flash is enabled
 the  field bit[21] in the Data Flash Control Register
 needs to be set to 1 according to guidleine GL-5830741 mentioned
>>> Typo: guideline ^
>> I will correct this typo in the next patch version.
 in Marvell Errata document MV-S501377-00, Rev. D.
>>> Thanks for that, I checked it.
>>>   
 This commit sets the FORCE_CSX bit to 1 for all
 ARMADA370 variants as the arbitration is always enabled by default.
 This change does not apply for pxa3xx variants because the
 FORCE_CSX bit does not exist/reserved on the NFCv1.
>>> Sorry to bother you again but I am reworking the pxa3xx_nand driver
>>> and so I would like to deeply understand why this is needed because
>>> I will have to integrate it in my work too.
>>>
>>> So please tell me what is your setup, do you really use NAND/NOR
>>> arbitration? Do you have not-Don't Care CS NAND chips? I have some
>>> doubts because even if the spec precises in the NAND controller
>>> chapter that arbitration is always enabled by default, the bit 27
>>> (NfArbiterEn) in the SoC Device Multiplex Register at offset
>>> 0x00018208 is actually at 0 by default (disabled). Did you enable
>>> this bit manually ? I checked with devmem on my setup and this bit
>>> was unset.
>> Yes, my setup use NAND/NOR arbitration and use a Don't
>> care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
>> by default in my case. I did not manually set the bit 27 to 1.
> 
> Actually if you use Don't Care CS NAND chips you should not need the
> force CS bit, as it is mentioned in the guidelines you pointed, this
> bit is only useful for *not* don't care CS NAND chips (and the name
> "force CS" is pretty straight forward too!).

It's less straight forward than you think. It's FORCE_CSX (the X is 
important) which is defined as "force chip select false on busy". What 
it means is that the NF_CSn is de-asserted if the interface is 
arbitrated away from the NAND flash controller. I think that means you 
won't end up with the chip-select for the dev-bus/NOR firing immediately 
when that device is arbitrated.

> Otherwise what bootloader and kernel do you use to have this
> bit set by default? You may also want to check if this bit is set by
> your bootloader by stopping it before it loads Linux and check manually
> the value of this bit with 'md' (on U-Boot).
> 
> Thank you,
> Miquèl
> 
>>
>> Please let me now if you have any other question or do you want me
>> to send the updated patch version with the typo fixed.
>>
>> Thank You,
>> Kalyan
>>
>>>
>>> Thank you for your time,
>>> Miquèl
>>>   
 The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is
 unused so remove it along with NDCR_NAND_MODE.

 Signed-off-by: Kalyan Kinthada
  ---
>>>
>>>   
drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/drivers/mtd/nand/pxa3xx_nand.c
 b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
 +++ b/drivers/mtd/nand/pxa3xx_nand.c
 @@ -67,8 +67,7 @@
#define NDCR_DWIDTH_M   (0x1 << 26)
#define NDCR_PAGE_SZ(0x1 << 24)
#define NDCR_NCSX   (0x1 << 23)
 -#define NDCR_ND_MODE  (0x3 << 21)
 -#define NDCR_NAND_MODE(0x0)
 +#define NDCR_FORCE_CSX(0x1 << 21)
#define NDCR_CLR_PG_CNT (0x1 << 20)
#define NFCV1_NDCR_ARB_CNTL (0x1 << 19)
#define NFCV2_NDCR_STOP_ON_UNCOR(0x1 << 19)
 @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
 pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
info->reg_ndcr = 0x0; /* enable all interrupts */
info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;
info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
info->reg_ndcr |= NDCR_SPARE_EN;

 @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
 pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
 NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-30 Thread Chris Packham

Hi Miquel,

On 09/10/17 19:19, Miquel RAYNAL wrote:
> Hello Kalyan,
> 
> On Mon, 9 Oct 2017 02:31:30 +
> Kalyan Kinthada  wrote:
> 
>> On 05/10/17 20:41, Miquel RAYNAL wrote:
>>> Hello Kalyan,
>>>
>>> On Thu, 28 Sep 2017 13:57:56 +1300
>>> Kalyan Kinthada  wrote:
>>>   
 When the arbitration between NOR and NAND flash is enabled
 the  field bit[21] in the Data Flash Control Register
 needs to be set to 1 according to guidleine GL-5830741 mentioned
>>> Typo: guideline ^
>> I will correct this typo in the next patch version.
 in Marvell Errata document MV-S501377-00, Rev. D.
>>> Thanks for that, I checked it.
>>>   
 This commit sets the FORCE_CSX bit to 1 for all
 ARMADA370 variants as the arbitration is always enabled by default.
 This change does not apply for pxa3xx variants because the
 FORCE_CSX bit does not exist/reserved on the NFCv1.
>>> Sorry to bother you again but I am reworking the pxa3xx_nand driver
>>> and so I would like to deeply understand why this is needed because
>>> I will have to integrate it in my work too.
>>>
>>> So please tell me what is your setup, do you really use NAND/NOR
>>> arbitration? Do you have not-Don't Care CS NAND chips? I have some
>>> doubts because even if the spec precises in the NAND controller
>>> chapter that arbitration is always enabled by default, the bit 27
>>> (NfArbiterEn) in the SoC Device Multiplex Register at offset
>>> 0x00018208 is actually at 0 by default (disabled). Did you enable
>>> this bit manually ? I checked with devmem on my setup and this bit
>>> was unset.
>> Yes, my setup use NAND/NOR arbitration and use a Don't
>> care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
>> by default in my case. I did not manually set the bit 27 to 1.
> 
> Actually if you use Don't Care CS NAND chips you should not need the
> force CS bit, as it is mentioned in the guidelines you pointed, this
> bit is only useful for *not* don't care CS NAND chips (and the name
> "force CS" is pretty straight forward too!).

It's less straight forward than you think. It's FORCE_CSX (the X is 
important) which is defined as "force chip select false on busy". What 
it means is that the NF_CSn is de-asserted if the interface is 
arbitrated away from the NAND flash controller. I think that means you 
won't end up with the chip-select for the dev-bus/NOR firing immediately 
when that device is arbitrated.

> Otherwise what bootloader and kernel do you use to have this
> bit set by default? You may also want to check if this bit is set by
> your bootloader by stopping it before it loads Linux and check manually
> the value of this bit with 'md' (on U-Boot).
> 
> Thank you,
> Miquèl
> 
>>
>> Please let me now if you have any other question or do you want me
>> to send the updated patch version with the typo fixed.
>>
>> Thank You,
>> Kalyan
>>
>>>
>>> Thank you for your time,
>>> Miquèl
>>>   
 The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is
 unused so remove it along with NDCR_NAND_MODE.

 Signed-off-by: Kalyan Kinthada
  ---
>>>
>>>   
drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/drivers/mtd/nand/pxa3xx_nand.c
 b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
 +++ b/drivers/mtd/nand/pxa3xx_nand.c
 @@ -67,8 +67,7 @@
#define NDCR_DWIDTH_M   (0x1 << 26)
#define NDCR_PAGE_SZ(0x1 << 24)
#define NDCR_NCSX   (0x1 << 23)
 -#define NDCR_ND_MODE  (0x3 << 21)
 -#define NDCR_NAND_MODE(0x0)
 +#define NDCR_FORCE_CSX(0x1 << 21)
#define NDCR_CLR_PG_CNT (0x1 << 20)
#define NFCV1_NDCR_ARB_CNTL (0x1 << 19)
#define NFCV2_NDCR_STOP_ON_UNCOR(0x1 << 19)
 @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
 pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
info->reg_ndcr = 0x0; /* enable all interrupts */
info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;
info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
info->reg_ndcr |= NDCR_SPARE_EN;

 @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
 pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
 NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-15 Thread Chris Packham

Hi Boris,

On 15/10/17 02:13, Boris Brezillon wrote:
> Hi Kalyan,
> 
> On Thu, 28 Sep 2017 13:57:56 +1300
> Kalyan Kinthada  wrote:
> 
>> When the arbitration between NOR and NAND flash is enabled
>> the  field bit[21] in the Data Flash Control Register
>> needs to be set to 1 according to guidleine GL-5830741 mentioned
>> in Marvell Errata document MV-S501377-00, Rev. D.
> 
> The real question is, is this patch fixing a real bug or are you just
> setting this bit because a guideline says it should be set?

Yes that's a fair summary. It is in a document called "Functional 
Errata" so there is some feeling here that we (allied telesis) should 
follow up things in these kinds of documents.

> As far as I
> understand, noone fully understands what this bit does and why it needs
> to be set, so if you're not fixing a real bug, I'd prefer keeping the
> code unchanged code until someone complains for a good reason.

Agreed. We've certainly not noticed a change in behaviour on our boards. 
I've requested more information from Marvell through their support 
channels. I'll share what I can if/when they come back with something 
useful.

> 
> Regards,
> 
> Boris
> 
>>
>> This commit sets the FORCE_CSX bit to 1 for all
>> ARMADA370 variants as the arbitration is always enabled by default.
>> This change does not apply for pxa3xx variants because the FORCE_CSX
>> bit does not exist/reserved on the NFCv1.
>>
>> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused so
>> remove it along with NDCR_NAND_MODE.
>>
>> Signed-off-by: Kalyan Kinthada 
>> ---
>>   drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
>> index 85cff68643e0..a6a7a5af7bed 100644
>> --- a/drivers/mtd/nand/pxa3xx_nand.c
>> +++ b/drivers/mtd/nand/pxa3xx_nand.c
>> @@ -67,8 +67,7 @@
>>   #define NDCR_DWIDTH_M  (0x1 << 26)
>>   #define NDCR_PAGE_SZ   (0x1 << 24)
>>   #define NDCR_NCSX  (0x1 << 23)
>> -#define NDCR_ND_MODE(0x3 << 21)
>> -#define NDCR_NAND_MODE  (0x0)
>> +#define NDCR_FORCE_CSX  (0x1 << 21)
>>   #define NDCR_CLR_PG_CNT(0x1 << 20)
>>   #define NFCV1_NDCR_ARB_CNTL(0x1 << 19)
>>   #define NFCV2_NDCR_STOP_ON_UNCOR   (0x1 << 19)
>> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct 
>> pxa3xx_nand_info *info)
>>  info->chunk_size = PAGE_CHUNK_SIZE;
>>  info->reg_ndcr = 0x0; /* enable all interrupts */
>>  info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
>> +/* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
>> +if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
>> +info->reg_ndcr |= NDCR_FORCE_CSX;
>>  info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
>>  info->reg_ndcr |= NDCR_SPARE_EN;
>>   
>> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct 
>> pxa3xx_nand_info *info)
>>  info->reg_ndcr = ndcr &
>>  ~(NDCR_INT_MASK | NDCR_ND_ARB_EN | NFCV1_NDCR_ARB_CNTL);
>>  info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
>> +/* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
>> +if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
>> +info->reg_ndcr |= NDCR_FORCE_CSX;
>>  info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
>>  info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
>>   }
> 
>

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-15 Thread Chris Packham

Hi Boris,

On 15/10/17 02:13, Boris Brezillon wrote:
> Hi Kalyan,
> 
> On Thu, 28 Sep 2017 13:57:56 +1300
> Kalyan Kinthada  wrote:
> 
>> When the arbitration between NOR and NAND flash is enabled
>> the  field bit[21] in the Data Flash Control Register
>> needs to be set to 1 according to guidleine GL-5830741 mentioned
>> in Marvell Errata document MV-S501377-00, Rev. D.
> 
> The real question is, is this patch fixing a real bug or are you just
> setting this bit because a guideline says it should be set?

Yes that's a fair summary. It is in a document called "Functional 
Errata" so there is some feeling here that we (allied telesis) should 
follow up things in these kinds of documents.

> As far as I
> understand, noone fully understands what this bit does and why it needs
> to be set, so if you're not fixing a real bug, I'd prefer keeping the
> code unchanged code until someone complains for a good reason.

Agreed. We've certainly not noticed a change in behaviour on our boards. 
I've requested more information from Marvell through their support 
channels. I'll share what I can if/when they come back with something 
useful.

> 
> Regards,
> 
> Boris
> 
>>
>> This commit sets the FORCE_CSX bit to 1 for all
>> ARMADA370 variants as the arbitration is always enabled by default.
>> This change does not apply for pxa3xx variants because the FORCE_CSX
>> bit does not exist/reserved on the NFCv1.
>>
>> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused so
>> remove it along with NDCR_NAND_MODE.
>>
>> Signed-off-by: Kalyan Kinthada 
>> ---
>>   drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
>> index 85cff68643e0..a6a7a5af7bed 100644
>> --- a/drivers/mtd/nand/pxa3xx_nand.c
>> +++ b/drivers/mtd/nand/pxa3xx_nand.c
>> @@ -67,8 +67,7 @@
>>   #define NDCR_DWIDTH_M  (0x1 << 26)
>>   #define NDCR_PAGE_SZ   (0x1 << 24)
>>   #define NDCR_NCSX  (0x1 << 23)
>> -#define NDCR_ND_MODE(0x3 << 21)
>> -#define NDCR_NAND_MODE  (0x0)
>> +#define NDCR_FORCE_CSX  (0x1 << 21)
>>   #define NDCR_CLR_PG_CNT(0x1 << 20)
>>   #define NFCV1_NDCR_ARB_CNTL(0x1 << 19)
>>   #define NFCV2_NDCR_STOP_ON_UNCOR   (0x1 << 19)
>> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct 
>> pxa3xx_nand_info *info)
>>  info->chunk_size = PAGE_CHUNK_SIZE;
>>  info->reg_ndcr = 0x0; /* enable all interrupts */
>>  info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
>> +/* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
>> +if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
>> +info->reg_ndcr |= NDCR_FORCE_CSX;
>>  info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
>>  info->reg_ndcr |= NDCR_SPARE_EN;
>>   
>> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct 
>> pxa3xx_nand_info *info)
>>  info->reg_ndcr = ndcr &
>>  ~(NDCR_INT_MASK | NDCR_ND_ARB_EN | NFCV1_NDCR_ARB_CNTL);
>>  info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
>> +/* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
>> +if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
>> +info->reg_ndcr |= NDCR_FORCE_CSX;
>>  info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
>>  info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
>>   }
> 
>

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-14 Thread Boris Brezillon

Hi Kalyan,

On Thu, 28 Sep 2017 13:57:56 +1300
Kalyan Kinthada  wrote:

> When the arbitration between NOR and NAND flash is enabled
> the  field bit[21] in the Data Flash Control Register
> needs to be set to 1 according to guidleine GL-5830741 mentioned
> in Marvell Errata document MV-S501377-00, Rev. D.

The real question is, is this patch fixing a real bug or are you just
setting this bit because a guideline says it should be set? As far as I
understand, noone fully understands what this bit does and why it needs
to be set, so if you're not fixing a real bug, I'd prefer keeping the
code unchanged code until someone complains for a good reason.

Regards,

Boris

> 
> This commit sets the FORCE_CSX bit to 1 for all
> ARMADA370 variants as the arbitration is always enabled by default.
> This change does not apply for pxa3xx variants because the FORCE_CSX
> bit does not exist/reserved on the NFCv1.
> 
> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused so
> remove it along with NDCR_NAND_MODE.
> 
> Signed-off-by: Kalyan Kinthada 
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 85cff68643e0..a6a7a5af7bed 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -67,8 +67,7 @@
>  #define NDCR_DWIDTH_M(0x1 << 26)
>  #define NDCR_PAGE_SZ (0x1 << 24)
>  #define NDCR_NCSX(0x1 << 23)
> -#define NDCR_ND_MODE (0x3 << 21)
> -#define NDCR_NAND_MODE   (0x0)
> +#define NDCR_FORCE_CSX   (0x1 << 21)
>  #define NDCR_CLR_PG_CNT  (0x1 << 20)
>  #define NFCV1_NDCR_ARB_CNTL  (0x1 << 19)
>  #define NFCV2_NDCR_STOP_ON_UNCOR (0x1 << 19)
> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct 
> pxa3xx_nand_info *info)
>   info->chunk_size = PAGE_CHUNK_SIZE;
>   info->reg_ndcr = 0x0; /* enable all interrupts */
>   info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
> + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
> + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> + info->reg_ndcr |= NDCR_FORCE_CSX;
>   info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
>   info->reg_ndcr |= NDCR_SPARE_EN;
>  
> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct 
> pxa3xx_nand_info *info)
>   info->reg_ndcr = ndcr &
>   ~(NDCR_INT_MASK | NDCR_ND_ARB_EN | NFCV1_NDCR_ARB_CNTL);
>   info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
> + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
> + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> + info->reg_ndcr |= NDCR_FORCE_CSX;
>   info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
>   info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
>  }

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-14 Thread Boris Brezillon

Hi Kalyan,

On Thu, 28 Sep 2017 13:57:56 +1300
Kalyan Kinthada  wrote:

> When the arbitration between NOR and NAND flash is enabled
> the  field bit[21] in the Data Flash Control Register
> needs to be set to 1 according to guidleine GL-5830741 mentioned
> in Marvell Errata document MV-S501377-00, Rev. D.

The real question is, is this patch fixing a real bug or are you just
setting this bit because a guideline says it should be set? As far as I
understand, noone fully understands what this bit does and why it needs
to be set, so if you're not fixing a real bug, I'd prefer keeping the
code unchanged code until someone complains for a good reason.

Regards,

Boris

> 
> This commit sets the FORCE_CSX bit to 1 for all
> ARMADA370 variants as the arbitration is always enabled by default.
> This change does not apply for pxa3xx variants because the FORCE_CSX
> bit does not exist/reserved on the NFCv1.
> 
> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused so
> remove it along with NDCR_NAND_MODE.
> 
> Signed-off-by: Kalyan Kinthada 
> ---
>  drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
> index 85cff68643e0..a6a7a5af7bed 100644
> --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -67,8 +67,7 @@
>  #define NDCR_DWIDTH_M(0x1 << 26)
>  #define NDCR_PAGE_SZ (0x1 << 24)
>  #define NDCR_NCSX(0x1 << 23)
> -#define NDCR_ND_MODE (0x3 << 21)
> -#define NDCR_NAND_MODE   (0x0)
> +#define NDCR_FORCE_CSX   (0x1 << 21)
>  #define NDCR_CLR_PG_CNT  (0x1 << 20)
>  #define NFCV1_NDCR_ARB_CNTL  (0x1 << 19)
>  #define NFCV2_NDCR_STOP_ON_UNCOR (0x1 << 19)
> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct 
> pxa3xx_nand_info *info)
>   info->chunk_size = PAGE_CHUNK_SIZE;
>   info->reg_ndcr = 0x0; /* enable all interrupts */
>   info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
> + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
> + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> + info->reg_ndcr |= NDCR_FORCE_CSX;
>   info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
>   info->reg_ndcr |= NDCR_SPARE_EN;
>  
> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct 
> pxa3xx_nand_info *info)
>   info->reg_ndcr = ndcr &
>   ~(NDCR_INT_MASK | NDCR_ND_ARB_EN | NFCV1_NDCR_ARB_CNTL);
>   info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
> + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
> + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> + info->reg_ndcr |= NDCR_FORCE_CSX;
>   info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
>   info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
>  }

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-10 Thread Kalyan Kinthada

Hello Miquel,
On 09/10/17 19:18, Miquel RAYNAL wrote:
> Hello Kalyan,
>
> On Mon, 9 Oct 2017 02:31:30 +
> Kalyan Kinthada  wrote:
>
>> On 05/10/17 20:41, Miquel RAYNAL wrote:
>>> Hello Kalyan,
>>>
>>> On Thu, 28 Sep 2017 13:57:56 +1300
>>> Kalyan Kinthada  wrote:
>>>   
 When the arbitration between NOR and NAND flash is enabled
 the  field bit[21] in the Data Flash Control Register
 needs to be set to 1 according to guidleine GL-5830741 mentioned
>>> Typo: guideline ^
>> I will correct this typo in the next patch version.
 in Marvell Errata document MV-S501377-00, Rev. D.
>>> Thanks for that, I checked it.
>>>   
 This commit sets the FORCE_CSX bit to 1 for all
 ARMADA370 variants as the arbitration is always enabled by default.
 This change does not apply for pxa3xx variants because the
 FORCE_CSX bit does not exist/reserved on the NFCv1.
>>> Sorry to bother you again but I am reworking the pxa3xx_nand driver
>>> and so I would like to deeply understand why this is needed because
>>> I will have to integrate it in my work too.
>>>
>>> So please tell me what is your setup, do you really use NAND/NOR
>>> arbitration? Do you have not-Don't Care CS NAND chips? I have some
>>> doubts because even if the spec precises in the NAND controller
>>> chapter that arbitration is always enabled by default, the bit 27
>>> (NfArbiterEn) in the SoC Device Multiplex Register at offset
>>> 0x00018208 is actually at 0 by default (disabled). Did you enable
>>> this bit manually ? I checked with devmem on my setup and this bit
>>> was unset.
>> Yes, my setup use NAND/NOR arbitration and use a Don't
>> care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
>> by default in my case. I did not manually set the bit 27 to 1.
> Actually if you use Don't Care CS NAND chips you should not need the
> force CS bit, as it is mentioned in the guidelines you pointed, this
> bit is only useful for *not* don't care CS NAND chips (and the name
> "force CS" is pretty straight forward too!).
Ok. I understand now. Thank you.
>
> Otherwise what bootloader and kernel do you use to have this
> bit set by default? You may also want to check if this bit is set by
> your bootloader by stopping it before it loads Linux and check manually
> the value of this bit with 'md' (on U-Boot).
The bit27 at offset 0x00018208 is set in bootloader.
Uboot version 2016.11
Linux Kernel 4.4.6

Thanks,
Kalyan
>
> Thank you,
> Miquèl
>
>> Please let me now if you have any other question or do you want me
>> to send the updated patch version with the typo fixed.
>>
>> Thank You,
>> Kalyan
>>
>>> Thank you for your time,
>>> Miquèl
>>>   
 The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is
 unused so remove it along with NDCR_NAND_MODE.

 Signed-off-by: Kalyan Kinthada
  ---
>>>   
drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/drivers/mtd/nand/pxa3xx_nand.c
 b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
 +++ b/drivers/mtd/nand/pxa3xx_nand.c
 @@ -67,8 +67,7 @@
#define NDCR_DWIDTH_M   (0x1 << 26)
#define NDCR_PAGE_SZ(0x1 << 24)
#define NDCR_NCSX   (0x1 << 23)
 -#define NDCR_ND_MODE  (0x3 << 21)
 -#define NDCR_NAND_MODE(0x0)
 +#define NDCR_FORCE_CSX(0x1 << 21)
#define NDCR_CLR_PG_CNT (0x1 << 20)
#define NFCV1_NDCR_ARB_CNTL (0x1 << 19)
#define NFCV2_NDCR_STOP_ON_UNCOR(0x1 << 19)
 @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
 pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
info->reg_ndcr = 0x0; /* enable all interrupts */
info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;
info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
info->reg_ndcr |= NDCR_SPARE_EN;

 @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
 pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
 NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;
info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
}
>>>   
>
>

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-10 Thread Kalyan Kinthada

Hello Miquel,
On 09/10/17 19:18, Miquel RAYNAL wrote:
> Hello Kalyan,
>
> On Mon, 9 Oct 2017 02:31:30 +
> Kalyan Kinthada  wrote:
>
>> On 05/10/17 20:41, Miquel RAYNAL wrote:
>>> Hello Kalyan,
>>>
>>> On Thu, 28 Sep 2017 13:57:56 +1300
>>> Kalyan Kinthada  wrote:
>>>   
 When the arbitration between NOR and NAND flash is enabled
 the  field bit[21] in the Data Flash Control Register
 needs to be set to 1 according to guidleine GL-5830741 mentioned
>>> Typo: guideline ^
>> I will correct this typo in the next patch version.
 in Marvell Errata document MV-S501377-00, Rev. D.
>>> Thanks for that, I checked it.
>>>   
 This commit sets the FORCE_CSX bit to 1 for all
 ARMADA370 variants as the arbitration is always enabled by default.
 This change does not apply for pxa3xx variants because the
 FORCE_CSX bit does not exist/reserved on the NFCv1.
>>> Sorry to bother you again but I am reworking the pxa3xx_nand driver
>>> and so I would like to deeply understand why this is needed because
>>> I will have to integrate it in my work too.
>>>
>>> So please tell me what is your setup, do you really use NAND/NOR
>>> arbitration? Do you have not-Don't Care CS NAND chips? I have some
>>> doubts because even if the spec precises in the NAND controller
>>> chapter that arbitration is always enabled by default, the bit 27
>>> (NfArbiterEn) in the SoC Device Multiplex Register at offset
>>> 0x00018208 is actually at 0 by default (disabled). Did you enable
>>> this bit manually ? I checked with devmem on my setup and this bit
>>> was unset.
>> Yes, my setup use NAND/NOR arbitration and use a Don't
>> care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
>> by default in my case. I did not manually set the bit 27 to 1.
> Actually if you use Don't Care CS NAND chips you should not need the
> force CS bit, as it is mentioned in the guidelines you pointed, this
> bit is only useful for *not* don't care CS NAND chips (and the name
> "force CS" is pretty straight forward too!).
Ok. I understand now. Thank you.
>
> Otherwise what bootloader and kernel do you use to have this
> bit set by default? You may also want to check if this bit is set by
> your bootloader by stopping it before it loads Linux and check manually
> the value of this bit with 'md' (on U-Boot).
The bit27 at offset 0x00018208 is set in bootloader.
Uboot version 2016.11
Linux Kernel 4.4.6

Thanks,
Kalyan
>
> Thank you,
> Miquèl
>
>> Please let me now if you have any other question or do you want me
>> to send the updated patch version with the typo fixed.
>>
>> Thank You,
>> Kalyan
>>
>>> Thank you for your time,
>>> Miquèl
>>>   
 The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is
 unused so remove it along with NDCR_NAND_MODE.

 Signed-off-by: Kalyan Kinthada
  ---
>>>   
drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/drivers/mtd/nand/pxa3xx_nand.c
 b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
 +++ b/drivers/mtd/nand/pxa3xx_nand.c
 @@ -67,8 +67,7 @@
#define NDCR_DWIDTH_M   (0x1 << 26)
#define NDCR_PAGE_SZ(0x1 << 24)
#define NDCR_NCSX   (0x1 << 23)
 -#define NDCR_ND_MODE  (0x3 << 21)
 -#define NDCR_NAND_MODE(0x0)
 +#define NDCR_FORCE_CSX(0x1 << 21)
#define NDCR_CLR_PG_CNT (0x1 << 20)
#define NFCV1_NDCR_ARB_CNTL (0x1 << 19)
#define NFCV2_NDCR_STOP_ON_UNCOR(0x1 << 19)
 @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
 pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
info->reg_ndcr = 0x0; /* enable all interrupts */
info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;
info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
info->reg_ndcr |= NDCR_SPARE_EN;

 @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
 pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
 NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;
info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
}
>>>   
>
>

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-09 Thread Miquel RAYNAL

Hello Kalyan,

On Mon, 9 Oct 2017 02:31:30 +
Kalyan Kinthada  wrote:

> On 05/10/17 20:41, Miquel RAYNAL wrote:
> > Hello Kalyan,
> >
> > On Thu, 28 Sep 2017 13:57:56 +1300
> > Kalyan Kinthada  wrote:
> >  
> >> When the arbitration between NOR and NAND flash is enabled
> >> the  field bit[21] in the Data Flash Control Register
> >> needs to be set to 1 according to guidleine GL-5830741 mentioned  
> > Typo: guideline ^  
> I will correct this typo in the next patch version.
> >> in Marvell Errata document MV-S501377-00, Rev. D.  
> > Thanks for that, I checked it.
> >  
> >> This commit sets the FORCE_CSX bit to 1 for all
> >> ARMADA370 variants as the arbitration is always enabled by default.
> >> This change does not apply for pxa3xx variants because the
> >> FORCE_CSX bit does not exist/reserved on the NFCv1.  
> > Sorry to bother you again but I am reworking the pxa3xx_nand driver
> > and so I would like to deeply understand why this is needed because
> > I will have to integrate it in my work too.
> >
> > So please tell me what is your setup, do you really use NAND/NOR
> > arbitration? Do you have not-Don't Care CS NAND chips? I have some
> > doubts because even if the spec precises in the NAND controller
> > chapter that arbitration is always enabled by default, the bit 27
> > (NfArbiterEn) in the SoC Device Multiplex Register at offset
> > 0x00018208 is actually at 0 by default (disabled). Did you enable
> > this bit manually ? I checked with devmem on my setup and this bit
> > was unset.  
> Yes, my setup use NAND/NOR arbitration and use a Don't
> care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
> by default in my case. I did not manually set the bit 27 to 1.

Actually if you use Don't Care CS NAND chips you should not need the
force CS bit, as it is mentioned in the guidelines you pointed, this
bit is only useful for *not* don't care CS NAND chips (and the name
"force CS" is pretty straight forward too!).

Otherwise what bootloader and kernel do you use to have this
bit set by default? You may also want to check if this bit is set by
your bootloader by stopping it before it loads Linux and check manually
the value of this bit with 'md' (on U-Boot).

Thank you,
Miquèl

> 
> Please let me now if you have any other question or do you want me
> to send the updated patch version with the typo fixed.
> 
> Thank You,
> Kalyan
> 
> >
> > Thank you for your time,
> > Miquèl
> >  
> >> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is
> >> unused so remove it along with NDCR_NAND_MODE.
> >>
> >> Signed-off-by: Kalyan Kinthada
> >>  ---  
> >
> >  
> >>   drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
> >>   1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
> >> b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
> >> 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
> >> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> >> @@ -67,8 +67,7 @@
> >>   #define NDCR_DWIDTH_M(0x1 << 26)
> >>   #define NDCR_PAGE_SZ (0x1 << 24)
> >>   #define NDCR_NCSX(0x1 << 23)
> >> -#define NDCR_ND_MODE  (0x3 << 21)
> >> -#define NDCR_NAND_MODE(0x0)
> >> +#define NDCR_FORCE_CSX(0x1 << 21)
> >>   #define NDCR_CLR_PG_CNT  (0x1 << 20)
> >>   #define NFCV1_NDCR_ARB_CNTL  (0x1 << 19)
> >>   #define NFCV2_NDCR_STOP_ON_UNCOR (0x1 << 19)
> >> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
> >> pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
> >>info->reg_ndcr = 0x0; /* enable all interrupts */
> >>info->reg_ndcr |= (pdata->enable_arbiter) ?
> >> NDCR_ND_ARB_EN : 0;
> >> +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
> >> GL-5830741 */
> >> +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> >> +  info->reg_ndcr |= NDCR_FORCE_CSX;
> >>info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
> >>info->reg_ndcr |= NDCR_SPARE_EN;
> >>   
> >> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
> >> pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
> >>~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
> >> NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
> >> NDCR_ND_ARB_EN : 0;
> >> +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
> >> GL-5830741 */
> >> +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> >> +  info->reg_ndcr |= NDCR_FORCE_CSX;
> >>info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
> >>info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
> >>   }  
> >
> >  



-- 
Miquel Raynal, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-09 Thread Miquel RAYNAL

Hello Kalyan,

On Mon, 9 Oct 2017 02:31:30 +
Kalyan Kinthada  wrote:

> On 05/10/17 20:41, Miquel RAYNAL wrote:
> > Hello Kalyan,
> >
> > On Thu, 28 Sep 2017 13:57:56 +1300
> > Kalyan Kinthada  wrote:
> >  
> >> When the arbitration between NOR and NAND flash is enabled
> >> the  field bit[21] in the Data Flash Control Register
> >> needs to be set to 1 according to guidleine GL-5830741 mentioned  
> > Typo: guideline ^  
> I will correct this typo in the next patch version.
> >> in Marvell Errata document MV-S501377-00, Rev. D.  
> > Thanks for that, I checked it.
> >  
> >> This commit sets the FORCE_CSX bit to 1 for all
> >> ARMADA370 variants as the arbitration is always enabled by default.
> >> This change does not apply for pxa3xx variants because the
> >> FORCE_CSX bit does not exist/reserved on the NFCv1.  
> > Sorry to bother you again but I am reworking the pxa3xx_nand driver
> > and so I would like to deeply understand why this is needed because
> > I will have to integrate it in my work too.
> >
> > So please tell me what is your setup, do you really use NAND/NOR
> > arbitration? Do you have not-Don't Care CS NAND chips? I have some
> > doubts because even if the spec precises in the NAND controller
> > chapter that arbitration is always enabled by default, the bit 27
> > (NfArbiterEn) in the SoC Device Multiplex Register at offset
> > 0x00018208 is actually at 0 by default (disabled). Did you enable
> > this bit manually ? I checked with devmem on my setup and this bit
> > was unset.  
> Yes, my setup use NAND/NOR arbitration and use a Don't
> care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
> by default in my case. I did not manually set the bit 27 to 1.

Actually if you use Don't Care CS NAND chips you should not need the
force CS bit, as it is mentioned in the guidelines you pointed, this
bit is only useful for *not* don't care CS NAND chips (and the name
"force CS" is pretty straight forward too!).

Otherwise what bootloader and kernel do you use to have this
bit set by default? You may also want to check if this bit is set by
your bootloader by stopping it before it loads Linux and check manually
the value of this bit with 'md' (on U-Boot).

Thank you,
Miquèl

> 
> Please let me now if you have any other question or do you want me
> to send the updated patch version with the typo fixed.
> 
> Thank You,
> Kalyan
> 
> >
> > Thank you for your time,
> > Miquèl
> >  
> >> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is
> >> unused so remove it along with NDCR_NAND_MODE.
> >>
> >> Signed-off-by: Kalyan Kinthada
> >>  ---  
> >
> >  
> >>   drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
> >>   1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
> >> b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
> >> 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
> >> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> >> @@ -67,8 +67,7 @@
> >>   #define NDCR_DWIDTH_M(0x1 << 26)
> >>   #define NDCR_PAGE_SZ (0x1 << 24)
> >>   #define NDCR_NCSX(0x1 << 23)
> >> -#define NDCR_ND_MODE  (0x3 << 21)
> >> -#define NDCR_NAND_MODE(0x0)
> >> +#define NDCR_FORCE_CSX(0x1 << 21)
> >>   #define NDCR_CLR_PG_CNT  (0x1 << 20)
> >>   #define NFCV1_NDCR_ARB_CNTL  (0x1 << 19)
> >>   #define NFCV2_NDCR_STOP_ON_UNCOR (0x1 << 19)
> >> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
> >> pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
> >>info->reg_ndcr = 0x0; /* enable all interrupts */
> >>info->reg_ndcr |= (pdata->enable_arbiter) ?
> >> NDCR_ND_ARB_EN : 0;
> >> +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
> >> GL-5830741 */
> >> +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> >> +  info->reg_ndcr |= NDCR_FORCE_CSX;
> >>info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
> >>info->reg_ndcr |= NDCR_SPARE_EN;
> >>   
> >> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
> >> pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
> >>~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
> >> NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
> >> NDCR_ND_ARB_EN : 0;
> >> +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
> >> GL-5830741 */
> >> +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> >> +  info->reg_ndcr |= NDCR_FORCE_CSX;
> >>info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
> >>info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
> >>   }  
> >
> >  



-- 
Miquel Raynal, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-08 Thread Kalyan Kinthada

On 05/10/17 20:41, Miquel RAYNAL wrote:
> Hello Kalyan,
>
> On Thu, 28 Sep 2017 13:57:56 +1300
> Kalyan Kinthada  wrote:
>
>> When the arbitration between NOR and NAND flash is enabled
>> the  field bit[21] in the Data Flash Control Register
>> needs to be set to 1 according to guidleine GL-5830741 mentioned
> Typo: guideline ^
I will correct this typo in the next patch version.
>> in Marvell Errata document MV-S501377-00, Rev. D.
> Thanks for that, I checked it.
>
>> This commit sets the FORCE_CSX bit to 1 for all
>> ARMADA370 variants as the arbitration is always enabled by default.
>> This change does not apply for pxa3xx variants because the FORCE_CSX
>> bit does not exist/reserved on the NFCv1.
> Sorry to bother you again but I am reworking the pxa3xx_nand driver and
> so I would like to deeply understand why this is needed because I will
> have to integrate it in my work too.
>
> So please tell me what is your setup, do you really use NAND/NOR
> arbitration? Do you have not-Don't Care CS NAND chips? I have some
> doubts because even if the spec precises in the NAND controller chapter
> that arbitration is always enabled by default, the bit 27 (NfArbiterEn)
> in the SoC Device Multiplex Register at offset 0x00018208 is actually
> at 0 by default (disabled). Did you enable this bit manually ? I
> checked with devmem on my setup and this bit was unset.
Yes, my setup use NAND/NOR arbitration and use a Don't
care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
by default in my case. I did not manually set the bit 27 to 1.

Please let me now if you have any other question or do you want me
to send the updated patch version with the typo fixed.

Thank You,
Kalyan

>
> Thank you for your time,
> Miquèl
>
>> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused
>> so remove it along with NDCR_NAND_MODE.
>>
>> Signed-off-by: Kalyan Kinthada 
>> ---
>
>
>>   drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
>> b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
>> 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
>> +++ b/drivers/mtd/nand/pxa3xx_nand.c
>> @@ -67,8 +67,7 @@
>>   #define NDCR_DWIDTH_M  (0x1 << 26)
>>   #define NDCR_PAGE_SZ   (0x1 << 24)
>>   #define NDCR_NCSX  (0x1 << 23)
>> -#define NDCR_ND_MODE(0x3 << 21)
>> -#define NDCR_NAND_MODE  (0x0)
>> +#define NDCR_FORCE_CSX  (0x1 << 21)
>>   #define NDCR_CLR_PG_CNT(0x1 << 20)
>>   #define NFCV1_NDCR_ARB_CNTL(0x1 << 19)
>>   #define NFCV2_NDCR_STOP_ON_UNCOR   (0x1 << 19)
>> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
>> pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
>>  info->reg_ndcr = 0x0; /* enable all interrupts */
>>  info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN :
>> 0;
>> +/* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
>> GL-5830741 */
>> +if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
>> +info->reg_ndcr |= NDCR_FORCE_CSX;
>>  info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
>>  info->reg_ndcr |= NDCR_SPARE_EN;
>>   
>> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
>> pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
>>  ~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
>> NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
>> NDCR_ND_ARB_EN : 0;
>> +/* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
>> GL-5830741 */
>> +if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
>> +info->reg_ndcr |= NDCR_FORCE_CSX;
>>  info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
>>  info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
>>   }
>
>

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-08 Thread Kalyan Kinthada

On 05/10/17 20:41, Miquel RAYNAL wrote:
> Hello Kalyan,
>
> On Thu, 28 Sep 2017 13:57:56 +1300
> Kalyan Kinthada  wrote:
>
>> When the arbitration between NOR and NAND flash is enabled
>> the  field bit[21] in the Data Flash Control Register
>> needs to be set to 1 according to guidleine GL-5830741 mentioned
> Typo: guideline ^
I will correct this typo in the next patch version.
>> in Marvell Errata document MV-S501377-00, Rev. D.
> Thanks for that, I checked it.
>
>> This commit sets the FORCE_CSX bit to 1 for all
>> ARMADA370 variants as the arbitration is always enabled by default.
>> This change does not apply for pxa3xx variants because the FORCE_CSX
>> bit does not exist/reserved on the NFCv1.
> Sorry to bother you again but I am reworking the pxa3xx_nand driver and
> so I would like to deeply understand why this is needed because I will
> have to integrate it in my work too.
>
> So please tell me what is your setup, do you really use NAND/NOR
> arbitration? Do you have not-Don't Care CS NAND chips? I have some
> doubts because even if the spec precises in the NAND controller chapter
> that arbitration is always enabled by default, the bit 27 (NfArbiterEn)
> in the SoC Device Multiplex Register at offset 0x00018208 is actually
> at 0 by default (disabled). Did you enable this bit manually ? I
> checked with devmem on my setup and this bit was unset.
Yes, my setup use NAND/NOR arbitration and use a Don't
care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
by default in my case. I did not manually set the bit 27 to 1.

Please let me now if you have any other question or do you want me
to send the updated patch version with the typo fixed.

Thank You,
Kalyan

>
> Thank you for your time,
> Miquèl
>
>> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused
>> so remove it along with NDCR_NAND_MODE.
>>
>> Signed-off-by: Kalyan Kinthada 
>> ---
>
>
>>   drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
>> b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
>> 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
>> +++ b/drivers/mtd/nand/pxa3xx_nand.c
>> @@ -67,8 +67,7 @@
>>   #define NDCR_DWIDTH_M  (0x1 << 26)
>>   #define NDCR_PAGE_SZ   (0x1 << 24)
>>   #define NDCR_NCSX  (0x1 << 23)
>> -#define NDCR_ND_MODE(0x3 << 21)
>> -#define NDCR_NAND_MODE  (0x0)
>> +#define NDCR_FORCE_CSX  (0x1 << 21)
>>   #define NDCR_CLR_PG_CNT(0x1 << 20)
>>   #define NFCV1_NDCR_ARB_CNTL(0x1 << 19)
>>   #define NFCV2_NDCR_STOP_ON_UNCOR   (0x1 << 19)
>> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
>> pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
>>  info->reg_ndcr = 0x0; /* enable all interrupts */
>>  info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN :
>> 0;
>> +/* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
>> GL-5830741 */
>> +if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
>> +info->reg_ndcr |= NDCR_FORCE_CSX;
>>  info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
>>  info->reg_ndcr |= NDCR_SPARE_EN;
>>   
>> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
>> pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
>>  ~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
>> NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
>> NDCR_ND_ARB_EN : 0;
>> +/* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
>> GL-5830741 */
>> +if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
>> +info->reg_ndcr |= NDCR_FORCE_CSX;
>>  info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
>>  info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
>>   }
>
>

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-05 Thread Miquel RAYNAL

Hello Kalyan,

On Thu, 28 Sep 2017 13:57:56 +1300
Kalyan Kinthada  wrote:

> When the arbitration between NOR and NAND flash is enabled
> the  field bit[21] in the Data Flash Control Register
> needs to be set to 1 according to guidleine GL-5830741 mentioned

Typo: guideline ^

> in Marvell Errata document MV-S501377-00, Rev. D.

Thanks for that, I checked it.

> 
> This commit sets the FORCE_CSX bit to 1 for all
> ARMADA370 variants as the arbitration is always enabled by default.
> This change does not apply for pxa3xx variants because the FORCE_CSX
> bit does not exist/reserved on the NFCv1.

Sorry to bother you again but I am reworking the pxa3xx_nand driver and
so I would like to deeply understand why this is needed because I will
have to integrate it in my work too.

So please tell me what is your setup, do you really use NAND/NOR
arbitration? Do you have not-Don't Care CS NAND chips? I have some
doubts because even if the spec precises in the NAND controller chapter
that arbitration is always enabled by default, the bit 27 (NfArbiterEn)
in the SoC Device Multiplex Register at offset 0x00018208 is actually
at 0 by default (disabled). Did you enable this bit manually ? I
checked with devmem on my setup and this bit was unset.

Thank you for your time,
Miquèl

> 
> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused
> so remove it along with NDCR_NAND_MODE.
> 
> Signed-off-by: Kalyan Kinthada 
> ---



>  drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
> b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
> 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -67,8 +67,7 @@
>  #define NDCR_DWIDTH_M(0x1 << 26)
>  #define NDCR_PAGE_SZ (0x1 << 24)
>  #define NDCR_NCSX(0x1 << 23)
> -#define NDCR_ND_MODE (0x3 << 21)
> -#define NDCR_NAND_MODE   (0x0)
> +#define NDCR_FORCE_CSX   (0x1 << 21)
>  #define NDCR_CLR_PG_CNT  (0x1 << 20)
>  #define NFCV1_NDCR_ARB_CNTL  (0x1 << 19)
>  #define NFCV2_NDCR_STOP_ON_UNCOR (0x1 << 19)
> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
> pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
>   info->reg_ndcr = 0x0; /* enable all interrupts */
>   info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN :
> 0;
> + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
> GL-5830741 */
> + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> + info->reg_ndcr |= NDCR_FORCE_CSX;
>   info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
>   info->reg_ndcr |= NDCR_SPARE_EN;
>  
> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
> pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
>   ~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
> NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
> NDCR_ND_ARB_EN : 0;
> + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
> GL-5830741 */
> + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> + info->reg_ndcr |= NDCR_FORCE_CSX;
>   info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
>   info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
>  }



-- 
Miquel Raynal, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-10-05 Thread Miquel RAYNAL

Hello Kalyan,

On Thu, 28 Sep 2017 13:57:56 +1300
Kalyan Kinthada  wrote:

> When the arbitration between NOR and NAND flash is enabled
> the  field bit[21] in the Data Flash Control Register
> needs to be set to 1 according to guidleine GL-5830741 mentioned

Typo: guideline ^

> in Marvell Errata document MV-S501377-00, Rev. D.

Thanks for that, I checked it.

> 
> This commit sets the FORCE_CSX bit to 1 for all
> ARMADA370 variants as the arbitration is always enabled by default.
> This change does not apply for pxa3xx variants because the FORCE_CSX
> bit does not exist/reserved on the NFCv1.

Sorry to bother you again but I am reworking the pxa3xx_nand driver and
so I would like to deeply understand why this is needed because I will
have to integrate it in my work too.

So please tell me what is your setup, do you really use NAND/NOR
arbitration? Do you have not-Don't Care CS NAND chips? I have some
doubts because even if the spec precises in the NAND controller chapter
that arbitration is always enabled by default, the bit 27 (NfArbiterEn)
in the SoC Device Multiplex Register at offset 0x00018208 is actually
at 0 by default (disabled). Did you enable this bit manually ? I
checked with devmem on my setup and this bit was unset.

Thank you for your time,
Miquèl

> 
> The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused
> so remove it along with NDCR_NAND_MODE.
> 
> Signed-off-by: Kalyan Kinthada 
> ---



>  drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/pxa3xx_nand.c
> b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
> 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
> +++ b/drivers/mtd/nand/pxa3xx_nand.c
> @@ -67,8 +67,7 @@
>  #define NDCR_DWIDTH_M(0x1 << 26)
>  #define NDCR_PAGE_SZ (0x1 << 24)
>  #define NDCR_NCSX(0x1 << 23)
> -#define NDCR_ND_MODE (0x3 << 21)
> -#define NDCR_NAND_MODE   (0x0)
> +#define NDCR_FORCE_CSX   (0x1 << 21)
>  #define NDCR_CLR_PG_CNT  (0x1 << 20)
>  #define NFCV1_NDCR_ARB_CNTL  (0x1 << 19)
>  #define NFCV2_NDCR_STOP_ON_UNCOR (0x1 << 19)
> @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
> pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
>   info->reg_ndcr = 0x0; /* enable all interrupts */
>   info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN :
> 0;
> + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
> GL-5830741 */
> + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> + info->reg_ndcr |= NDCR_FORCE_CSX;
>   info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
>   info->reg_ndcr |= NDCR_SPARE_EN;
>  
> @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
> pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
>   ~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
> NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
> NDCR_ND_ARB_EN : 0;
> + /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
> GL-5830741 */
> + if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
> + info->reg_ndcr |= NDCR_FORCE_CSX;
>   info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
>   info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
>  }



-- 
Miquel Raynal, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-09-27 Thread Kalyan Kinthada

When the arbitration between NOR and NAND flash is enabled
the  field bit[21] in the Data Flash Control Register
needs to be set to 1 according to guidleine GL-5830741 mentioned
in Marvell Errata document MV-S501377-00, Rev. D.

This commit sets the FORCE_CSX bit to 1 for all
ARMADA370 variants as the arbitration is always enabled by default.
This change does not apply for pxa3xx variants because the FORCE_CSX
bit does not exist/reserved on the NFCv1.

The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused so
remove it along with NDCR_NAND_MODE.

Signed-off-by: Kalyan Kinthada 
---
 drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 85cff68643e0..a6a7a5af7bed 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -67,8 +67,7 @@
 #define NDCR_DWIDTH_M  (0x1 << 26)
 #define NDCR_PAGE_SZ   (0x1 << 24)
 #define NDCR_NCSX  (0x1 << 23)
-#define NDCR_ND_MODE   (0x3 << 21)
-#define NDCR_NAND_MODE (0x0)
+#define NDCR_FORCE_CSX (0x1 << 21)
 #define NDCR_CLR_PG_CNT(0x1 << 20)
 #define NFCV1_NDCR_ARB_CNTL(0x1 << 19)
 #define NFCV2_NDCR_STOP_ON_UNCOR   (0x1 << 19)
@@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct 
pxa3xx_nand_info *info)
info->chunk_size = PAGE_CHUNK_SIZE;
info->reg_ndcr = 0x0; /* enable all interrupts */
info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
+   /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
+   if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+   info->reg_ndcr |= NDCR_FORCE_CSX;
info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
info->reg_ndcr |= NDCR_SPARE_EN;
 
@@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct 
pxa3xx_nand_info *info)
info->reg_ndcr = ndcr &
~(NDCR_INT_MASK | NDCR_ND_ARB_EN | NFCV1_NDCR_ARB_CNTL);
info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
+   /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
+   if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+   info->reg_ndcr |= NDCR_FORCE_CSX;
info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
 }
-- 
2.14.1

[PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

2017-09-27 Thread Kalyan Kinthada

When the arbitration between NOR and NAND flash is enabled
the  field bit[21] in the Data Flash Control Register
needs to be set to 1 according to guidleine GL-5830741 mentioned
in Marvell Errata document MV-S501377-00, Rev. D.

This commit sets the FORCE_CSX bit to 1 for all
ARMADA370 variants as the arbitration is always enabled by default.
This change does not apply for pxa3xx variants because the FORCE_CSX
bit does not exist/reserved on the NFCv1.

The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is unused so
remove it along with NDCR_NAND_MODE.

Signed-off-by: Kalyan Kinthada 
---
 drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 85cff68643e0..a6a7a5af7bed 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -67,8 +67,7 @@
 #define NDCR_DWIDTH_M  (0x1 << 26)
 #define NDCR_PAGE_SZ   (0x1 << 24)
 #define NDCR_NCSX  (0x1 << 23)
-#define NDCR_ND_MODE   (0x3 << 21)
-#define NDCR_NAND_MODE (0x0)
+#define NDCR_FORCE_CSX (0x1 << 21)
 #define NDCR_CLR_PG_CNT(0x1 << 20)
 #define NFCV1_NDCR_ARB_CNTL(0x1 << 19)
 #define NFCV2_NDCR_STOP_ON_UNCOR   (0x1 << 19)
@@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct 
pxa3xx_nand_info *info)
info->chunk_size = PAGE_CHUNK_SIZE;
info->reg_ndcr = 0x0; /* enable all interrupts */
info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
+   /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
+   if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+   info->reg_ndcr |= NDCR_FORCE_CSX;
info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
info->reg_ndcr |= NDCR_SPARE_EN;
 
@@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct 
pxa3xx_nand_info *info)
info->reg_ndcr = ndcr &
~(NDCR_INT_MASK | NDCR_ND_ARB_EN | NFCV1_NDCR_ARB_CNTL);
info->reg_ndcr |= (pdata->enable_arbiter) ? NDCR_ND_ARB_EN : 0;
+   /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#: GL-5830741 */
+   if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
+   info->reg_ndcr |= NDCR_FORCE_CSX;
info->ndtr0cs0 = nand_readl(info, NDTR0CS0);
info->ndtr1cs0 = nand_readl(info, NDTR1CS0);
 }
-- 
2.14.1

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

[PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

[PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

16 matches

Site Navigation

Mail list logo

Footer information