spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value

PATCH New Chin Liang See 2 participants 5 messages Aug 7, 2016 SPI

Share & tools

Chin Liang See

author

2016-08-07 13:53 UTC #0 lore raw

PATCH 1/1 spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value

Ensuring the baudrate divisor value doesn't exceed the max value in the calculation.It will be capped at max value to ensure the correct value being written into the register.

Signed-off-by Chin Liang See <[email protected]>

Cc Marek Vasut <[email protected]>

Cc Jagan Teki <[email protected]>

Cc Dinh Nguyen <[email protected]>

---
 drivers/spi/cadence_qspi_apb.c | 4 ++++
 1 file changed, 4 insertions(+)

drivers/spi/cadence_qspi_apb.c +4 -0

index 1a35d55..e6a8ecb 100644
@@ -293,6 +293,10 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
 	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
 	      ref_clk_hz, sclk_hz, div);
 
+	/* ensure the baud rate doesn't exceed the max value */
+	if (div > CQSPI_REG_CONFIG_BAUD_MASK)
+		div = CQSPI_REG_CONFIG_BAUD_MASK;
+
 	div = (div & CQSPI_REG_CONFIG_BAUD_MASK) << CQSPI_REG_CONFIG_BAUD_LSB;
 	reg |= div;
 	writel(reg, reg_base + CQSPI_REG_CONFIG);

Signature · 1 line

2.2.2

Marek Vasut

2016-08-07 14:05 UTC #1 lore raw

PATCH 1/1 spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value

Quoted 17 lines · Chin Liang See

Ensuring the baudrate divisor value doesn't exceed the max value in the calculation.It will be capped at max value to ensure the correct value being written into the register.
Signed-off-by Chin Liang See <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Jagan Teki <[email protected]>
Cc Dinh Nguyen <[email protected]>
---
 drivers/spi/cadence_qspi_apb.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/spi/cadence_qspi_apb.c +0 -0
index 1a35d55..e6a8ecb 100644

drivers/spi/cadence_qspi_apb.c void cadence_qspi_apb_config_baudrate_div(void *reg_base, · line 300

Quoted 8 lines

drivers/spi/cadence_qspi_apb.c +4 -0

@@ -293,6 +293,10 @@ void cadence_qspi_apb_config_baudrate_div(void *reg_base,
 	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
 	      ref_clk_hz, sclk_hz, div);
 
+	/* ensure the baud rate doesn't exceed the max value */
+	if (div > CQSPI_REG_CONFIG_BAUD_MASK)
+		div = CQSPI_REG_CONFIG_BAUD_MASK;
+
 	div = (div & CQSPI_REG_CONFIG_BAUD_MASK) << CQSPI_REG_CONFIG_BAUD_LSB;

The capping happens here ^ already , doesn't it ?

Quoted 3 lines

drivers/spi/cadence_qspi_apb.c +0 -0
 	reg |= div;
 	writel(reg, reg_base + CQSPI_REG_CONFIG);

Signature · 2 lines

Best regards,
Marek Vasut

Chin Liang See

author

2016-08-07 14:11 UTC #2 lore raw

PATCH 1/1 spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value

Quoted 23 lines · Marek Vasut

Quoted 22 lines · Chin Liang See
Ensuring the baudrate divisor value doesn't exceed the max value in the calculation.It will be capped at max value to ensure the correct value being written into the register.
Signed-off-by Chin Liang See <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Jagan Teki <[email protected]>
Cc Dinh Nguyen <[email protected]>
---
 drivers/spi/cadence_qspi_apb.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/spi/cadence_qspi_apb.c +0 -0
index 1a35d55..e6a8ecb 100644
@@ -293,6 +293,10 @@ void cadence_qspi_apb_config_baudrate_div(void
 *reg_base,
 	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
 	      ref_clk_hz, sclk_hz, div);
 

Quoted 8 lines

drivers/spi/cadence_qspi_apb.c void cadence_qspi_apb_config_baudrate_div(void *reg_base,
Quoted 6 lines
drivers/spi/cadence_qspi_apb.c +4 -0
+	/* ensure the baud rate doesn't exceed the max value */
+	if (div > CQSPI_REG_CONFIG_BAUD_MASK)
+		div = CQSPI_REG_CONFIG_BAUD_MASK;
+
 	div = (div & CQSPI_REG_CONFIG_BAUD_MASK) <<
 CQSPI_REG_CONFIG_BAUD_LSB;
The capping happens here ^ already , doesn't it ?

I have initial thought too until someone file me a bug on this. The bug happen when the calculated div = 16. After and with the mask, the value writen to register is actually 0 (register field for baudrate divisor).

Thanks
Chin Liang

Quoted 6 lines

Quoted 4 lines
drivers/spi/cadence_qspi_apb.c +0 -0
 	reg |= div;
 	writel(reg, reg_base + CQSPI_REG_CONFIG);

Marek Vasut

2016-08-07 14:13 UTC #3 lore raw

PATCH 1/1 spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value

Quoted 33 lines · Chin Liang See

Quoted 23 lines · Marek Vasut
Quoted 22 lines · Chin Liang See
Ensuring the baudrate divisor value doesn't exceed the max value in the calculation.It will be capped at max value to ensure the correct value being written into the register.
Signed-off-by Chin Liang See <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Jagan Teki <[email protected]>
Cc Dinh Nguyen <[email protected]>
---
 drivers/spi/cadence_qspi_apb.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/spi/cadence_qspi_apb.c +0 -0
index 1a35d55..e6a8ecb 100644
@@ -293,6 +293,10 @@ void cadence_qspi_apb_config_baudrate_div(void
 *reg_base,
 	debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n", __func__,
 	      ref_clk_hz, sclk_hz, div);
 
Quoted 8 lines
drivers/spi/cadence_qspi_apb.c void cadence_qspi_apb_config_baudrate_div(void *reg_base,
Quoted 6 lines
drivers/spi/cadence_qspi_apb.c +4 -0
+	/* ensure the baud rate doesn't exceed the max value */
+	if (div > CQSPI_REG_CONFIG_BAUD_MASK)
+		div = CQSPI_REG_CONFIG_BAUD_MASK;
+
 	div = (div & CQSPI_REG_CONFIG_BAUD_MASK) <<
 CQSPI_REG_CONFIG_BAUD_LSB;
The capping happens here ^ already , doesn't it ?

Quoted 3 lines

I have initial thought too until someone file me a bug on this. The bug happen when the calculated div = 16. After and with the mask, the value writen to register is actually 0 (register field for baudrate divisor).

So this is what the problem is all about, thanks for clarifying (and that should be in the commit message!). But then, you don't need the masking, it can be turned into reg = div << CQSPI_REG_CONFIG_BAUD_LSB.

Signature · 2 lines

Best regards,
Marek Vasut

Chin Liang See

author

2016-08-07 14:34 UTC #4 lore raw

PATCH 1/1 spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value

Quoted 45 lines · Marek Vasut

Quoted 37 lines · Chin Liang See
Quoted 26 lines · Marek Vasut
Quoted 25 lines · Chin Liang See
Ensuring the baudrate divisor value doesn't exceed the max value
in the calculation.It will be capped at max value to ensure the correct value being written into the register.
Signed-off-by Chin Liang See <[email protected]>
Cc Marek Vasut <[email protected]>
Cc Jagan Teki <[email protected]>
Cc Dinh Nguyen <[email protected]>
---
 drivers/spi/cadence_qspi_apb.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/spi/cadence_qspi_apb.c +0 -0
index 1a35d55..e6a8ecb 100644
@@ -293,6 +293,10 @@ void
cadence_qspi_apb_config_baudrate_div(void *reg_base,
debug("%s: ref_clk %dHz sclk %dHz Div 0x%x\n",
__func__,
ref_clk_hz, sclk_hz, div);
Quoted 9 lines
Quoted 7 lines
drivers/spi/cadence_qspi_apb.c +1 -0
+	/* ensure the baud rate doesn't exceed the max value
*/
drivers/spi/cadence_qspi_apb.c +3 -0
+	if (div > CQSPI_REG_CONFIG_BAUD_MASK)
+		div = CQSPI_REG_CONFIG_BAUD_MASK;
+
 	div = (div & CQSPI_REG_CONFIG_BAUD_MASK) <<
CQSPI_REG_CONFIG_BAUD_LSB;
The capping happens here ^ already , doesn't it ?
Quoted 6 lines
I have initial thought too until someone file me a bug on this. The bug
happen when the calculated div = 16. After and with the mask, the value
writen to register is actually 0 (register field for baudrate divisor).

Quoted 4 lines

So this is what the problem is all about, thanks for clarifying (and that should be in the commit message!). But then, you don't need the masking, it can be turned into reg = div << CQSPI_REG_CONFIG_BAUD_LSB.

Yup, let me drop v2 then

Thanks
Chin Liang

Quoted 1 line