* [PATCH V2 0/2] Fix issues related to register access in IPQ NAND
@ 2020-06-09 11:10 ` Sivaprakash Murugesan
0 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-09 11:10 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr, peter.ujfalusi, sivaprak,
linux-mtd, linux-kernel
Patch 1: avoids register write to unavailable SFLASHC_BURST_CFG register
Patch 2: set BAM mode only if not set by bootloader
[V2]
* As per review comments from Miquèl split the original patch into two
addressing independent issues.
Sivaprakash Murugesan (2):
mtd: rawnand: qcom: remove write to unavailable register
mtd: rawnand: qcom: set BAM mode only if not set already
drivers/mtd/nand/raw/qcom_nandc.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH V2 0/2] Fix issues related to register access in IPQ NAND
@ 2020-06-09 11:10 ` Sivaprakash Murugesan
0 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-09 11:10 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr, peter.ujfalusi, sivaprak,
linux-mtd, linux-kernel
Patch 1: avoids register write to unavailable SFLASHC_BURST_CFG register
Patch 2: set BAM mode only if not set by bootloader
[V2]
* As per review comments from Miquèl split the original patch into two
addressing independent issues.
Sivaprakash Murugesan (2):
mtd: rawnand: qcom: remove write to unavailable register
mtd: rawnand: qcom: set BAM mode only if not set already
drivers/mtd/nand/raw/qcom_nandc.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
--
2.7.4
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH V2 1/2] mtd: rawnand: qcom: remove write to unavailable register
2020-06-09 11:10 ` Sivaprakash Murugesan
@ 2020-06-09 11:10 ` Sivaprakash Murugesan
-1 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-09 11:10 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr, peter.ujfalusi, sivaprak,
linux-mtd, linux-kernel
SFLASHC_BURST_CFG register is not available on all ipq nand platforms,
it is available only on ipq8064 devices and the nand controller works
without configuring these registers in this platform, so register write
to this can be removed.
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
drivers/mtd/nand/raw/qcom_nandc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 5b11c70..e0afa2c 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -36,7 +36,6 @@
#define NAND_DEV_CMD1 0xa4
#define NAND_DEV_CMD2 0xa8
#define NAND_DEV_CMD_VLD 0xac
-#define SFLASHC_BURST_CFG 0xe0
#define NAND_ERASED_CW_DETECT_CFG 0xe8
#define NAND_ERASED_CW_DETECT_STATUS 0xec
#define NAND_EBI2_ECC_BUF_CFG 0xf0
@@ -2774,7 +2773,6 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
u32 nand_ctrl;
/* kill onenand */
- nandc_write(nandc, SFLASHC_BURST_CFG, 0);
nandc_write(nandc, dev_cmd_reg_addr(nandc, NAND_DEV_CMD_VLD),
NAND_DEV_CMD_VLD_VAL);
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V2 1/2] mtd: rawnand: qcom: remove write to unavailable register
@ 2020-06-09 11:10 ` Sivaprakash Murugesan
0 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-09 11:10 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr, peter.ujfalusi, sivaprak,
linux-mtd, linux-kernel
SFLASHC_BURST_CFG register is not available on all ipq nand platforms,
it is available only on ipq8064 devices and the nand controller works
without configuring these registers in this platform, so register write
to this can be removed.
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
drivers/mtd/nand/raw/qcom_nandc.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 5b11c70..e0afa2c 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -36,7 +36,6 @@
#define NAND_DEV_CMD1 0xa4
#define NAND_DEV_CMD2 0xa8
#define NAND_DEV_CMD_VLD 0xac
-#define SFLASHC_BURST_CFG 0xe0
#define NAND_ERASED_CW_DETECT_CFG 0xe8
#define NAND_ERASED_CW_DETECT_STATUS 0xec
#define NAND_EBI2_ECC_BUF_CFG 0xf0
@@ -2774,7 +2773,6 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
u32 nand_ctrl;
/* kill onenand */
- nandc_write(nandc, SFLASHC_BURST_CFG, 0);
nandc_write(nandc, dev_cmd_reg_addr(nandc, NAND_DEV_CMD_VLD),
NAND_DEV_CMD_VLD_VAL);
--
2.7.4
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already
2020-06-09 11:10 ` Sivaprakash Murugesan
@ 2020-06-09 11:10 ` Sivaprakash Murugesan
-1 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-09 11:10 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr, peter.ujfalusi, sivaprak,
linux-mtd, linux-kernel
BAM mode is set by writing BAM_MODE_EN bit on NAND_CTRL register.
NAND_CTRL is an operational register and in BAM mode operational
registers are read only.
So, before writing into NAND_CTRL register check if BAM mode is already
enabled by bootloader, and set BAM mode only if it is not set already.
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
drivers/mtd/nand/raw/qcom_nandc.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index e0afa2c..7740059 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2779,7 +2779,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
/* enable ADM or BAM DMA */
if (nandc->props->is_bam) {
nand_ctrl = nandc_read(nandc, NAND_CTRL);
- nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
+ /* NAND_CTRL is an operational registers, and CPU
+ * access to operational registers are read only
+ * in BAM mode. So update the NAND_CTRL register
+ * only if it is not in BAM mode. In most cases BAM
+ * mode will be enabled in bootloader
+ */
+ if (!(nand_ctrl | BAM_MODE_EN))
+ nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
} else {
nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already
@ 2020-06-09 11:10 ` Sivaprakash Murugesan
0 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-09 11:10 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr, peter.ujfalusi, sivaprak,
linux-mtd, linux-kernel
BAM mode is set by writing BAM_MODE_EN bit on NAND_CTRL register.
NAND_CTRL is an operational register and in BAM mode operational
registers are read only.
So, before writing into NAND_CTRL register check if BAM mode is already
enabled by bootloader, and set BAM mode only if it is not set already.
Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
---
drivers/mtd/nand/raw/qcom_nandc.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index e0afa2c..7740059 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -2779,7 +2779,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
/* enable ADM or BAM DMA */
if (nandc->props->is_bam) {
nand_ctrl = nandc_read(nandc, NAND_CTRL);
- nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
+ /* NAND_CTRL is an operational registers, and CPU
+ * access to operational registers are read only
+ * in BAM mode. So update the NAND_CTRL register
+ * only if it is not in BAM mode. In most cases BAM
+ * mode will be enabled in bootloader
+ */
+ if (!(nand_ctrl | BAM_MODE_EN))
+ nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
} else {
nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
}
--
2.7.4
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/2] mtd: rawnand: qcom: remove write to unavailable register
2020-06-09 11:10 ` Sivaprakash Murugesan
@ 2020-06-09 14:02 ` Miquel Raynal
-1 siblings, 0 replies; 16+ messages in thread
From: Miquel Raynal @ 2020-06-09 14:02 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: richard, vigneshr, peter.ujfalusi, linux-mtd, linux-kernel
Hi Sivaprakash,
Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
2020 16:40:55 +0530:
> SFLASHC_BURST_CFG register is not available on all ipq nand platforms,
> it is available only on ipq8064 devices and the nand controller works
> without configuring these registers in this platform, so register write
> to this can be removed.
Maybe it works because the bootloader is setting the register itself.
What if you use a different bootloader, or the same bootloader without
NAND support?
I don't think this is a proper fix, you should instead have a different
compatible if the IP is not the same and depending on this compatible
do the write, or not.
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> drivers/mtd/nand/raw/qcom_nandc.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 5b11c70..e0afa2c 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -36,7 +36,6 @@
> #define NAND_DEV_CMD1 0xa4
> #define NAND_DEV_CMD2 0xa8
> #define NAND_DEV_CMD_VLD 0xac
> -#define SFLASHC_BURST_CFG 0xe0
> #define NAND_ERASED_CW_DETECT_CFG 0xe8
> #define NAND_ERASED_CW_DETECT_STATUS 0xec
> #define NAND_EBI2_ECC_BUF_CFG 0xf0
> @@ -2774,7 +2773,6 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
> u32 nand_ctrl;
>
> /* kill onenand */
> - nandc_write(nandc, SFLASHC_BURST_CFG, 0);
> nandc_write(nandc, dev_cmd_reg_addr(nandc, NAND_DEV_CMD_VLD),
> NAND_DEV_CMD_VLD_VAL);
>
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/2] mtd: rawnand: qcom: remove write to unavailable register
@ 2020-06-09 14:02 ` Miquel Raynal
0 siblings, 0 replies; 16+ messages in thread
From: Miquel Raynal @ 2020-06-09 14:02 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: peter.ujfalusi, richard, linux-mtd, vigneshr, linux-kernel
Hi Sivaprakash,
Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
2020 16:40:55 +0530:
> SFLASHC_BURST_CFG register is not available on all ipq nand platforms,
> it is available only on ipq8064 devices and the nand controller works
> without configuring these registers in this platform, so register write
> to this can be removed.
Maybe it works because the bootloader is setting the register itself.
What if you use a different bootloader, or the same bootloader without
NAND support?
I don't think this is a proper fix, you should instead have a different
compatible if the IP is not the same and depending on this compatible
do the write, or not.
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> drivers/mtd/nand/raw/qcom_nandc.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 5b11c70..e0afa2c 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -36,7 +36,6 @@
> #define NAND_DEV_CMD1 0xa4
> #define NAND_DEV_CMD2 0xa8
> #define NAND_DEV_CMD_VLD 0xac
> -#define SFLASHC_BURST_CFG 0xe0
> #define NAND_ERASED_CW_DETECT_CFG 0xe8
> #define NAND_ERASED_CW_DETECT_STATUS 0xec
> #define NAND_EBI2_ECC_BUF_CFG 0xf0
> @@ -2774,7 +2773,6 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
> u32 nand_ctrl;
>
> /* kill onenand */
> - nandc_write(nandc, SFLASHC_BURST_CFG, 0);
> nandc_write(nandc, dev_cmd_reg_addr(nandc, NAND_DEV_CMD_VLD),
> NAND_DEV_CMD_VLD_VAL);
>
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already
2020-06-09 11:10 ` Sivaprakash Murugesan
@ 2020-06-09 14:03 ` Miquel Raynal
-1 siblings, 0 replies; 16+ messages in thread
From: Miquel Raynal @ 2020-06-09 14:03 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: richard, vigneshr, peter.ujfalusi, linux-mtd, linux-kernel
Hi Sivaprakash,
Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
2020 16:40:56 +0530:
> BAM mode is set by writing BAM_MODE_EN bit on NAND_CTRL register.
> NAND_CTRL is an operational register and in BAM mode operational
> registers are read only.
>
> So, before writing into NAND_CTRL register check if BAM mode is already
> enabled by bootloader, and set BAM mode only if it is not set already.
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> drivers/mtd/nand/raw/qcom_nandc.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index e0afa2c..7740059 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -2779,7 +2779,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
> /* enable ADM or BAM DMA */
> if (nandc->props->is_bam) {
> nand_ctrl = nandc_read(nandc, NAND_CTRL);
> - nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
> + /* NAND_CTRL is an operational registers, and CPU
> + * access to operational registers are read only
> + * in BAM mode. So update the NAND_CTRL register
> + * only if it is not in BAM mode. In most cases BAM
> + * mode will be enabled in bootloader
> + */
> + if (!(nand_ctrl | BAM_MODE_EN))
> + nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
> } else {
> nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
> }
Does this currently produces an issue at runtime?
If yes, you should have a Fixes/CC: stable pair of tags.
Also, what is BAM mode? Please tell us in the commit log.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already
@ 2020-06-09 14:03 ` Miquel Raynal
0 siblings, 0 replies; 16+ messages in thread
From: Miquel Raynal @ 2020-06-09 14:03 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: peter.ujfalusi, richard, linux-mtd, vigneshr, linux-kernel
Hi Sivaprakash,
Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
2020 16:40:56 +0530:
> BAM mode is set by writing BAM_MODE_EN bit on NAND_CTRL register.
> NAND_CTRL is an operational register and in BAM mode operational
> registers are read only.
>
> So, before writing into NAND_CTRL register check if BAM mode is already
> enabled by bootloader, and set BAM mode only if it is not set already.
>
> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> ---
> drivers/mtd/nand/raw/qcom_nandc.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index e0afa2c..7740059 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -2779,7 +2779,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
> /* enable ADM or BAM DMA */
> if (nandc->props->is_bam) {
> nand_ctrl = nandc_read(nandc, NAND_CTRL);
> - nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
> + /* NAND_CTRL is an operational registers, and CPU
> + * access to operational registers are read only
> + * in BAM mode. So update the NAND_CTRL register
> + * only if it is not in BAM mode. In most cases BAM
> + * mode will be enabled in bootloader
> + */
> + if (!(nand_ctrl | BAM_MODE_EN))
> + nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
> } else {
> nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
> }
Does this currently produces an issue at runtime?
If yes, you should have a Fixes/CC: stable pair of tags.
Also, what is BAM mode? Please tell us in the commit log.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already
2020-06-09 14:03 ` Miquel Raynal
@ 2020-06-11 4:27 ` Sivaprakash Murugesan
-1 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-11 4:27 UTC (permalink / raw)
To: Miquel Raynal; +Cc: richard, vigneshr, peter.ujfalusi, linux-mtd, linux-kernel
Hi Miquel,
Thanks for the review.
On 6/9/2020 7:33 PM, Miquel Raynal wrote:
> Hi Sivaprakash,
>
> Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
> 2020 16:40:56 +0530:
>
>> BAM mode is set by writing BAM_MODE_EN bit on NAND_CTRL register.
>> NAND_CTRL is an operational register and in BAM mode operational
>> registers are read only.
>>
>> So, before writing into NAND_CTRL register check if BAM mode is already
>> enabled by bootloader, and set BAM mode only if it is not set already.
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>> drivers/mtd/nand/raw/qcom_nandc.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index e0afa2c..7740059 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -2779,7 +2779,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
>> /* enable ADM or BAM DMA */
>> if (nandc->props->is_bam) {
>> nand_ctrl = nandc_read(nandc, NAND_CTRL);
>> - nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
>> + /* NAND_CTRL is an operational registers, and CPU
>> + * access to operational registers are read only
>> + * in BAM mode. So update the NAND_CTRL register
>> + * only if it is not in BAM mode. In most cases BAM
>> + * mode will be enabled in bootloader
>> + */
>> + if (!(nand_ctrl | BAM_MODE_EN))
>> + nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
>> } else {
>> nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
>> }
> Does this currently produces an issue at runtime?
>
> If yes, you should have a Fixes/CC: stable pair of tags.
>
> Also, what is BAM mode? Please tell us in the commit log.
Currently this is not causing any issue on run time.
The writes to this register is silently ignored.
However, this could be an issue in future Hardware designs.
BAM is the DMA engine on QCOM IPQ platforms, sure will explain this
mode in next patchset.
>
> Thanks,
> Miquèl
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already
@ 2020-06-11 4:27 ` Sivaprakash Murugesan
0 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-11 4:27 UTC (permalink / raw)
To: Miquel Raynal; +Cc: peter.ujfalusi, richard, linux-mtd, vigneshr, linux-kernel
Hi Miquel,
Thanks for the review.
On 6/9/2020 7:33 PM, Miquel Raynal wrote:
> Hi Sivaprakash,
>
> Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
> 2020 16:40:56 +0530:
>
>> BAM mode is set by writing BAM_MODE_EN bit on NAND_CTRL register.
>> NAND_CTRL is an operational register and in BAM mode operational
>> registers are read only.
>>
>> So, before writing into NAND_CTRL register check if BAM mode is already
>> enabled by bootloader, and set BAM mode only if it is not set already.
>>
>> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
>> ---
>> drivers/mtd/nand/raw/qcom_nandc.c | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
>> index e0afa2c..7740059 100644
>> --- a/drivers/mtd/nand/raw/qcom_nandc.c
>> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
>> @@ -2779,7 +2779,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
>> /* enable ADM or BAM DMA */
>> if (nandc->props->is_bam) {
>> nand_ctrl = nandc_read(nandc, NAND_CTRL);
>> - nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
>> + /* NAND_CTRL is an operational registers, and CPU
>> + * access to operational registers are read only
>> + * in BAM mode. So update the NAND_CTRL register
>> + * only if it is not in BAM mode. In most cases BAM
>> + * mode will be enabled in bootloader
>> + */
>> + if (!(nand_ctrl | BAM_MODE_EN))
>> + nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
>> } else {
>> nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
>> }
> Does this currently produces an issue at runtime?
>
> If yes, you should have a Fixes/CC: stable pair of tags.
>
> Also, what is BAM mode? Please tell us in the commit log.
Currently this is not causing any issue on run time.
The writes to this register is silently ignored.
However, this could be an issue in future Hardware designs.
BAM is the DMA engine on QCOM IPQ platforms, sure will explain this
mode in next patchset.
>
> Thanks,
> Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/2] mtd: rawnand: qcom: remove write to unavailable register
2020-06-09 14:02 ` Miquel Raynal
@ 2020-06-11 4:30 ` Sivaprakash Murugesan
-1 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-11 4:30 UTC (permalink / raw)
To: Miquel Raynal; +Cc: richard, vigneshr, peter.ujfalusi, linux-mtd, linux-kernel
Hi Miquel,
On 6/9/2020 7:32 PM, Miquel Raynal wrote:
> Hi Sivaprakash,
>
> Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
> 2020 16:40:55 +0530:
>
>> SFLASHC_BURST_CFG register is not available on all ipq nand platforms,
>> it is available only on ipq8064 devices and the nand controller works
>> without configuring these registers in this platform, so register write
>> to this can be removed.
> Maybe it works because the bootloader is setting the register itself.
> What if you use a different bootloader, or the same bootloader without
> NAND support?
>
> I don't think this is a proper fix, you should instead have a different
> compatible if the IP is not the same and depending on this compatible
> do the write, or not.
I understand your point, will fix this in next patch.
Thanks,
Siva
> Thanks,
> Miquèl
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 1/2] mtd: rawnand: qcom: remove write to unavailable register
@ 2020-06-11 4:30 ` Sivaprakash Murugesan
0 siblings, 0 replies; 16+ messages in thread
From: Sivaprakash Murugesan @ 2020-06-11 4:30 UTC (permalink / raw)
To: Miquel Raynal; +Cc: peter.ujfalusi, richard, linux-mtd, vigneshr, linux-kernel
Hi Miquel,
On 6/9/2020 7:32 PM, Miquel Raynal wrote:
> Hi Sivaprakash,
>
> Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
> 2020 16:40:55 +0530:
>
>> SFLASHC_BURST_CFG register is not available on all ipq nand platforms,
>> it is available only on ipq8064 devices and the nand controller works
>> without configuring these registers in this platform, so register write
>> to this can be removed.
> Maybe it works because the bootloader is setting the register itself.
> What if you use a different bootloader, or the same bootloader without
> NAND support?
>
> I don't think this is a proper fix, you should instead have a different
> compatible if the IP is not the same and depending on this compatible
> do the write, or not.
I understand your point, will fix this in next patch.
Thanks,
Siva
> Thanks,
> Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already
2020-06-11 4:27 ` Sivaprakash Murugesan
@ 2020-06-11 7:13 ` Miquel Raynal
-1 siblings, 0 replies; 16+ messages in thread
From: Miquel Raynal @ 2020-06-11 7:13 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: richard, vigneshr, peter.ujfalusi, linux-mtd, linux-kernel
Hi Sivaprakash,
Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Thu, 11 Jun
2020 09:57:59 +0530:
> Hi Miquel,
>
> Thanks for the review.
>
> On 6/9/2020 7:33 PM, Miquel Raynal wrote:
> > Hi Sivaprakash,
> >
> > Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
> > 2020 16:40:56 +0530:
> >
> >> BAM mode is set by writing BAM_MODE_EN bit on NAND_CTRL register.
> >> NAND_CTRL is an operational register and in BAM mode operational
> >> registers are read only.
> >>
> >> So, before writing into NAND_CTRL register check if BAM mode is already
> >> enabled by bootloader, and set BAM mode only if it is not set already.
> >>
> >> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> >> ---
> >> drivers/mtd/nand/raw/qcom_nandc.c | 9 ++++++++-
> >> 1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> >> index e0afa2c..7740059 100644
> >> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> >> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> >> @@ -2779,7 +2779,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
> >> /* enable ADM or BAM DMA */
> >> if (nandc->props->is_bam) {
> >> nand_ctrl = nandc_read(nandc, NAND_CTRL);
> >> - nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
> >> + /* NAND_CTRL is an operational registers, and CPU
> >> + * access to operational registers are read only
> >> + * in BAM mode. So update the NAND_CTRL register
> >> + * only if it is not in BAM mode. In most cases BAM
> >> + * mode will be enabled in bootloader
> >> + */
> >> + if (!(nand_ctrl | BAM_MODE_EN))
> >> + nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
> >> } else {
> >> nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
> >> }
> > Does this currently produces an issue at runtime?
> >
> > If yes, you should have a Fixes/CC: stable pair of tags.
> >
> > Also, what is BAM mode? Please tell us in the commit log.
>
> Currently this is not causing any issue on run time.
>
> The writes to this register is silently ignored.
>
> However, this could be an issue in future Hardware designs.
>
> BAM is the DMA engine on QCOM IPQ platforms, sure will explain this
>
> mode in next patchset.
I don't like so much the idea of DMA being enabled by the Bootloader or
not, this is something that should need to be fixed.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already
@ 2020-06-11 7:13 ` Miquel Raynal
0 siblings, 0 replies; 16+ messages in thread
From: Miquel Raynal @ 2020-06-11 7:13 UTC (permalink / raw)
To: Sivaprakash Murugesan
Cc: peter.ujfalusi, richard, linux-mtd, vigneshr, linux-kernel
Hi Sivaprakash,
Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Thu, 11 Jun
2020 09:57:59 +0530:
> Hi Miquel,
>
> Thanks for the review.
>
> On 6/9/2020 7:33 PM, Miquel Raynal wrote:
> > Hi Sivaprakash,
> >
> > Sivaprakash Murugesan <sivaprak@codeaurora.org> wrote on Tue, 9 Jun
> > 2020 16:40:56 +0530:
> >
> >> BAM mode is set by writing BAM_MODE_EN bit on NAND_CTRL register.
> >> NAND_CTRL is an operational register and in BAM mode operational
> >> registers are read only.
> >>
> >> So, before writing into NAND_CTRL register check if BAM mode is already
> >> enabled by bootloader, and set BAM mode only if it is not set already.
> >>
> >> Signed-off-by: Sivaprakash Murugesan <sivaprak@codeaurora.org>
> >> ---
> >> drivers/mtd/nand/raw/qcom_nandc.c | 9 ++++++++-
> >> 1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> >> index e0afa2c..7740059 100644
> >> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> >> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> >> @@ -2779,7 +2779,14 @@ static int qcom_nandc_setup(struct qcom_nand_controller *nandc)
> >> /* enable ADM or BAM DMA */
> >> if (nandc->props->is_bam) {
> >> nand_ctrl = nandc_read(nandc, NAND_CTRL);
> >> - nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
> >> + /* NAND_CTRL is an operational registers, and CPU
> >> + * access to operational registers are read only
> >> + * in BAM mode. So update the NAND_CTRL register
> >> + * only if it is not in BAM mode. In most cases BAM
> >> + * mode will be enabled in bootloader
> >> + */
> >> + if (!(nand_ctrl | BAM_MODE_EN))
> >> + nandc_write(nandc, NAND_CTRL, nand_ctrl | BAM_MODE_EN);
> >> } else {
> >> nandc_write(nandc, NAND_FLASH_CHIP_SELECT, DM_EN);
> >> }
> > Does this currently produces an issue at runtime?
> >
> > If yes, you should have a Fixes/CC: stable pair of tags.
> >
> > Also, what is BAM mode? Please tell us in the commit log.
>
> Currently this is not causing any issue on run time.
>
> The writes to this register is silently ignored.
>
> However, this could be an issue in future Hardware designs.
>
> BAM is the DMA engine on QCOM IPQ platforms, sure will explain this
>
> mode in next patchset.
I don't like so much the idea of DMA being enabled by the Bootloader or
not, this is something that should need to be fixed.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-06-11 7:14 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09 11:10 [PATCH V2 0/2] Fix issues related to register access in IPQ NAND Sivaprakash Murugesan
2020-06-09 11:10 ` Sivaprakash Murugesan
2020-06-09 11:10 ` [PATCH V2 1/2] mtd: rawnand: qcom: remove write to unavailable register Sivaprakash Murugesan
2020-06-09 11:10 ` Sivaprakash Murugesan
2020-06-09 14:02 ` Miquel Raynal
2020-06-09 14:02 ` Miquel Raynal
2020-06-11 4:30 ` Sivaprakash Murugesan
2020-06-11 4:30 ` Sivaprakash Murugesan
2020-06-09 11:10 ` [PATCH V2 2/2] mtd: rawnand: qcom: set BAM mode only if not set already Sivaprakash Murugesan
2020-06-09 11:10 ` Sivaprakash Murugesan
2020-06-09 14:03 ` Miquel Raynal
2020-06-09 14:03 ` Miquel Raynal
2020-06-11 4:27 ` Sivaprakash Murugesan
2020-06-11 4:27 ` Sivaprakash Murugesan
2020-06-11 7:13 ` Miquel Raynal
2020-06-11 7:13 ` Miquel Raynal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.