* [U-Boot] [PATCH] spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value
@ 2016-08-07 13:53 Chin Liang See
2016-08-07 14:05 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Chin Liang See @ 2016-08-07 13:53 UTC (permalink / raw)
To: u-boot
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 <clsee@altera.com>
Cc: Marek Vasut <marex@denx.de>
Cc: Jagan Teki <jteki@openedev.com>
Cc: Dinh Nguyen <dinguyen@altera.com>
---
drivers/spi/cadence_qspi_apb.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index 1a35d55..e6a8ecb 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -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);
--
2.2.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value
2016-08-07 13:53 [U-Boot] [PATCH] spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value Chin Liang See
@ 2016-08-07 14:05 ` Marek Vasut
2016-08-07 14:11 ` Chin Liang See
0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2016-08-07 14:05 UTC (permalink / raw)
To: u-boot
On 08/07/2016 03:53 PM, Chin Liang See wrote:
> 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 <clsee@altera.com>
> Cc: Marek Vasut <marex@denx.de>
> Cc: Jagan Teki <jteki@openedev.com>
> Cc: Dinh Nguyen <dinguyen@altera.com>
> ---
> drivers/spi/cadence_qspi_apb.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> index 1a35d55..e6a8ecb 100644
> --- a/drivers/spi/cadence_qspi_apb.c
> +++ b/drivers/spi/cadence_qspi_apb.c
> @@ -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 ?
> reg |= div;
> writel(reg, reg_base + CQSPI_REG_CONFIG);
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value
2016-08-07 14:05 ` Marek Vasut
@ 2016-08-07 14:11 ` Chin Liang See
2016-08-07 14:13 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Chin Liang See @ 2016-08-07 14:11 UTC (permalink / raw)
To: u-boot
On Sun, 2016-08-07 at 16:05 +0200, Marek Vasut wrote:
> On 08/07/2016 03:53 PM, Chin Liang See wrote:
> > 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 <clsee@altera.com>
> > Cc: Marek Vasut <marex@denx.de>
> > Cc: Jagan Teki <jteki@openedev.com>
> > Cc: Dinh Nguyen <dinguyen@altera.com>
> > ---
> > drivers/spi/cadence_qspi_apb.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/spi/cadence_qspi_apb.c
> > b/drivers/spi/cadence_qspi_apb.c
> > index 1a35d55..e6a8ecb 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -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 ?
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
>
> > reg |= div;
> > writel(reg, reg_base + CQSPI_REG_CONFIG);
> >
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value
2016-08-07 14:11 ` Chin Liang See
@ 2016-08-07 14:13 ` Marek Vasut
2016-08-07 14:34 ` Chin Liang See
0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2016-08-07 14:13 UTC (permalink / raw)
To: u-boot
On 08/07/2016 04:11 PM, Chin Liang See wrote:
> On Sun, 2016-08-07 at 16:05 +0200, Marek Vasut wrote:
>> On 08/07/2016 03:53 PM, Chin Liang See wrote:
>>> 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 <clsee@altera.com>
>>> Cc: Marek Vasut <marex@denx.de>
>>> Cc: Jagan Teki <jteki@openedev.com>
>>> Cc: Dinh Nguyen <dinguyen@altera.com>
>>> ---
>>> drivers/spi/cadence_qspi_apb.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/spi/cadence_qspi_apb.c
>>> b/drivers/spi/cadence_qspi_apb.c
>>> index 1a35d55..e6a8ecb 100644
>>> --- a/drivers/spi/cadence_qspi_apb.c
>>> +++ b/drivers/spi/cadence_qspi_apb.c
>>> @@ -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 ?
>
> 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.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value
2016-08-07 14:13 ` Marek Vasut
@ 2016-08-07 14:34 ` Chin Liang See
0 siblings, 0 replies; 5+ messages in thread
From: Chin Liang See @ 2016-08-07 14:34 UTC (permalink / raw)
To: u-boot
On Sun, 2016-08-07 at 16:13 +0200, Marek Vasut wrote:
> On 08/07/2016 04:11 PM, Chin Liang See wrote:
> > On Sun, 2016-08-07 at 16:05 +0200, Marek Vasut wrote:
> > > On 08/07/2016 03:53 PM, Chin Liang See wrote:
> > > > 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 <clsee@altera.com>
> > > > Cc: Marek Vasut <marex@denx.de>
> > > > Cc: Jagan Teki <jteki@openedev.com>
> > > > Cc: Dinh Nguyen <dinguyen@altera.com>
> > > > ---
> > > > drivers/spi/cadence_qspi_apb.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/spi/cadence_qspi_apb.c
> > > > b/drivers/spi/cadence_qspi_apb.c
> > > > index 1a35d55..e6a8ecb 100644
> > > > --- a/drivers/spi/cadence_qspi_apb.c
> > > > +++ b/drivers/spi/cadence_qspi_apb.c
> > > > @@ -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 ?
> >
> > 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.
Yup, let me drop v2 then
Thanks
Chin Liang
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-08-07 14:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-07 13:53 [U-Boot] [PATCH] spi: cadence_qspi_apb: Ensure baudrate doesn't exceed max value Chin Liang See
2016-08-07 14:05 ` Marek Vasut
2016-08-07 14:11 ` Chin Liang See
2016-08-07 14:13 ` Marek Vasut
2016-08-07 14:34 ` Chin Liang See
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.