All of lore.kernel.org
 help / color / mirror / Atom feed
* Question about checking rate_spi in pwrap_init_reg_clock
@ 2015-04-21  1:58 Axel Lin
  2015-04-21 10:31 ` Sascha Hauer
  0 siblings, 1 reply; 12+ messages in thread
From: Axel Lin @ 2015-04-21  1:58 UTC (permalink / raw)
  To: linux-arm-kernel

hi,
The implementation in pwrap_init_reg_clock seems has off-by-one bug.
If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.

I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
Can someone confirm if this is a bug or not?


diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
index db5be1e..e910e19 100644
--- a/drivers/soc/mediatek/mtk-pmic-wrap.c
+++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
@@ -568,9 +568,9 @@ static int pwrap_init_reg_clock(struct pmic_wrapper *wrp)
 
 	rate_spi = clk_get_rate(wrp->clk_spi);
 
-	if (rate_spi > 26000000)
+	if (rate_spi >= 26000000)
 		ck_mhz = 26;
-	else if (rate_spi > 18000000)
+	else if (rate_spi >= 18000000)
 		ck_mhz = 18;
 	else
 		ck_mhz = 0;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Question about checking rate_spi in pwrap_init_reg_clock
  2015-04-21  1:58 Question about checking rate_spi in pwrap_init_reg_clock Axel Lin
@ 2015-04-21 10:31 ` Sascha Hauer
       [not found]   ` <20150421103155.GX6325-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
  0 siblings, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2015-04-21 10:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
> hi,
> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
> 
> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
> Can someone confirm if this is a bug or not?

Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
or should I do it?

sascha

> 
> 
> diff --git a/drivers/soc/mediatek/mtk-pmic-wrap.c b/drivers/soc/mediatek/mtk-pmic-wrap.c
> index db5be1e..e910e19 100644
> --- a/drivers/soc/mediatek/mtk-pmic-wrap.c
> +++ b/drivers/soc/mediatek/mtk-pmic-wrap.c
> @@ -568,9 +568,9 @@ static int pwrap_init_reg_clock(struct pmic_wrapper *wrp)
>  
>  	rate_spi = clk_get_rate(wrp->clk_spi);
>  
> -	if (rate_spi > 26000000)
> +	if (rate_spi >= 26000000)
>  		ck_mhz = 26;
> -	else if (rate_spi > 18000000)
> +	else if (rate_spi >= 18000000)
>  		ck_mhz = 18;
>  	else
>  		ck_mhz = 0;
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Question about checking rate_spi in pwrap_init_reg_clock
  2015-04-21 10:31 ` Sascha Hauer
@ 2015-04-21 11:43       ` Matthias Brugger
  0 siblings, 0 replies; 12+ messages in thread
From: Matthias Brugger @ 2015-04-21 11:43 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Flora Fu, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Axel Lin, linux-arm-kernel, fan. chen

2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
>> hi,
>> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
>> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
>>
>> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
>> Can someone confirm if this is a bug or not?
>
> Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
> or should I do it?

Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
Reading the datasheet and reference kernel code for mt6589 this SPI
waveform configuration looks like magic.

But reading the clock frequency which can have clk_spi in mt8135, they
are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
conclusion that the code is correct. Otherwise the check for bigger
then 18 MHz is useless and should be deleted, if we have a similar
clock distribution in mt8173.

So maybe someone from Mediatek can shed light on this issue.

Thanks,
Matthias

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Question about checking rate_spi in pwrap_init_reg_clock
@ 2015-04-21 11:43       ` Matthias Brugger
  0 siblings, 0 replies; 12+ messages in thread
From: Matthias Brugger @ 2015-04-21 11:43 UTC (permalink / raw)
  To: linux-arm-kernel

2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
>> hi,
>> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
>> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
>>
>> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
>> Can someone confirm if this is a bug or not?
>
> Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
> or should I do it?

Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
Reading the datasheet and reference kernel code for mt6589 this SPI
waveform configuration looks like magic.

But reading the clock frequency which can have clk_spi in mt8135, they
are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
conclusion that the code is correct. Otherwise the check for bigger
then 18 MHz is useless and should be deleted, if we have a similar
clock distribution in mt8173.

So maybe someone from Mediatek can shed light on this issue.

Thanks,
Matthias

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Question about checking rate_spi in pwrap_init_reg_clock
  2015-04-21 11:43       ` Matthias Brugger
@ 2015-04-21 12:52           ` Sascha Hauer
  -1 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2015-04-21 12:52 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Axel Lin,
	linux-arm-kernel, fan. chen

On Tue, Apr 21, 2015 at 01:43:19PM +0200, Matthias Brugger wrote:
> 2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> > On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
> >> hi,
> >> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
> >> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
> >>
> >> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
> >> Can someone confirm if this is a bug or not?
> >
> > Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
> > or should I do it?
> 
> Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
> Reading the datasheet and reference kernel code for mt6589 this SPI
> waveform configuration looks like magic.
> 
> But reading the clock frequency which can have clk_spi in mt8135, they
> are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
> conclusion that the code is correct. Otherwise the check for bigger
> then 18 MHz is useless and should be deleted, if we have a similar
> clock distribution in mt8173.

The CSHEXT and CSLEXT registers extend the time the chipselect stays
high after a SPI transfer or low before a new transfer by the given
number of spi clk cycles.
Looking at the code maybe the real question is not whether the border is
at 25999999Hz or at 26000000Hz, but why the transfers are throttled with
slower clock frequencies. I would assume the other way round makes
sense.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Question about checking rate_spi in pwrap_init_reg_clock
@ 2015-04-21 12:52           ` Sascha Hauer
  0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2015-04-21 12:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 01:43:19PM +0200, Matthias Brugger wrote:
> 2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> > On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
> >> hi,
> >> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
> >> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
> >>
> >> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
> >> Can someone confirm if this is a bug or not?
> >
> > Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
> > or should I do it?
> 
> Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
> Reading the datasheet and reference kernel code for mt6589 this SPI
> waveform configuration looks like magic.
> 
> But reading the clock frequency which can have clk_spi in mt8135, they
> are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
> conclusion that the code is correct. Otherwise the check for bigger
> then 18 MHz is useless and should be deleted, if we have a similar
> clock distribution in mt8173.

The CSHEXT and CSLEXT registers extend the time the chipselect stays
high after a SPI transfer or low before a new transfer by the given
number of spi clk cycles.
Looking at the code maybe the real question is not whether the border is
at 25999999Hz or at 26000000Hz, but why the transfers are throttled with
slower clock frequencies. I would assume the other way round makes
sense.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Question about checking rate_spi in pwrap_init_reg_clock
  2015-04-21 12:52           ` Sascha Hauer
@ 2015-04-21 15:42               ` Matthias Brugger
  -1 siblings, 0 replies; 12+ messages in thread
From: Matthias Brugger @ 2015-04-21 15:42 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Axel Lin,
	linux-arm-kernel, fan. chen

2015-04-21 14:52 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> On Tue, Apr 21, 2015 at 01:43:19PM +0200, Matthias Brugger wrote:
>> 2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
>> > On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
>> >> hi,
>> >> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
>> >> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
>> >>
>> >> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
>> >> Can someone confirm if this is a bug or not?
>> >
>> > Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
>> > or should I do it?
>>
>> Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
>> Reading the datasheet and reference kernel code for mt6589 this SPI
>> waveform configuration looks like magic.
>>
>> But reading the clock frequency which can have clk_spi in mt8135, they
>> are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
>> conclusion that the code is correct. Otherwise the check for bigger
>> then 18 MHz is useless and should be deleted, if we have a similar
>> clock distribution in mt8173.
>
> The CSHEXT and CSLEXT registers extend the time the chipselect stays
> high after a SPI transfer or low before a new transfer by the given
> number of spi clk cycles.
> Looking at the code maybe the real question is not whether the border is
> at 25999999Hz or at 26000000Hz, but why the transfers are throttled with
> slower clock frequencies. I would assume the other way round makes
> sense.

After re-reading the data sheet and the reference kernel
implementation, it seems as if three clocks exist. clk_wrap and
clk_spi and "pmic reg_clk". And some how depending on the three clocks
you have to calculate the values for CSHEXT, CSLEXT etc.

I think we need some clarification here from Mediatek to really
understand what is going on.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Question about checking rate_spi in pwrap_init_reg_clock
@ 2015-04-21 15:42               ` Matthias Brugger
  0 siblings, 0 replies; 12+ messages in thread
From: Matthias Brugger @ 2015-04-21 15:42 UTC (permalink / raw)
  To: linux-arm-kernel

2015-04-21 14:52 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Tue, Apr 21, 2015 at 01:43:19PM +0200, Matthias Brugger wrote:
>> 2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> > On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
>> >> hi,
>> >> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
>> >> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
>> >>
>> >> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
>> >> Can someone confirm if this is a bug or not?
>> >
>> > Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
>> > or should I do it?
>>
>> Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
>> Reading the datasheet and reference kernel code for mt6589 this SPI
>> waveform configuration looks like magic.
>>
>> But reading the clock frequency which can have clk_spi in mt8135, they
>> are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
>> conclusion that the code is correct. Otherwise the check for bigger
>> then 18 MHz is useless and should be deleted, if we have a similar
>> clock distribution in mt8173.
>
> The CSHEXT and CSLEXT registers extend the time the chipselect stays
> high after a SPI transfer or low before a new transfer by the given
> number of spi clk cycles.
> Looking at the code maybe the real question is not whether the border is
> at 25999999Hz or at 26000000Hz, but why the transfers are throttled with
> slower clock frequencies. I would assume the other way round makes
> sense.

After re-reading the data sheet and the reference kernel
implementation, it seems as if three clocks exist. clk_wrap and
clk_spi and "pmic reg_clk". And some how depending on the three clocks
you have to calculate the values for CSHEXT, CSLEXT etc.

I think we need some clarification here from Mediatek to really
understand what is going on.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Question about checking rate_spi in pwrap_init_reg_clock
  2015-04-21 15:42               ` Matthias Brugger
@ 2015-05-08  9:33                   ` Axel Lin
  -1 siblings, 0 replies; 12+ messages in thread
From: Axel Lin @ 2015-05-08  9:33 UTC (permalink / raw)
  To: Matthias Brugger, Leilk Liu
  Cc: fan. chen, Sascha Hauer,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-arm-kernel

CC Leilk Liu

2015-04-21 23:42 GMT+08:00 Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> 2015-04-21 14:52 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
>> On Tue, Apr 21, 2015 at 01:43:19PM +0200, Matthias Brugger wrote:
>>> 2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
>>> > On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
>>> >> hi,
>>> >> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
>>> >> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
>>> >>
>>> >> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
>>> >> Can someone confirm if this is a bug or not?
>>> >
>>> > Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
>>> > or should I do it?
>>>
>>> Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
>>> Reading the datasheet and reference kernel code for mt6589 this SPI
>>> waveform configuration looks like magic.
>>>
>>> But reading the clock frequency which can have clk_spi in mt8135, they
>>> are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
>>> conclusion that the code is correct. Otherwise the check for bigger
>>> then 18 MHz is useless and should be deleted, if we have a similar
>>> clock distribution in mt8173.
>>
>> The CSHEXT and CSLEXT registers extend the time the chipselect stays
>> high after a SPI transfer or low before a new transfer by the given
>> number of spi clk cycles.
>> Looking at the code maybe the real question is not whether the border is
>> at 25999999Hz or at 26000000Hz, but why the transfers are throttled with
>> slower clock frequencies. I would assume the other way round makes
>> sense.
>
> After re-reading the data sheet and the reference kernel
> implementation, it seems as if three clocks exist. clk_wrap and
> clk_spi and "pmic reg_clk". And some how depending on the three clocks
> you have to calculate the values for CSHEXT, CSLEXT etc.
>
> I think we need some clarification here from Mediatek to really
> understand what is going on.

Hi Leilk,
I found you are working on mtk's spi driver.
Any comment about this discussion?

Regards,
Axel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Question about checking rate_spi in pwrap_init_reg_clock
@ 2015-05-08  9:33                   ` Axel Lin
  0 siblings, 0 replies; 12+ messages in thread
From: Axel Lin @ 2015-05-08  9:33 UTC (permalink / raw)
  To: linux-arm-kernel

CC Leilk Liu

2015-04-21 23:42 GMT+08:00 Matthias Brugger <matthias.bgg@gmail.com>:
> 2015-04-21 14:52 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>> On Tue, Apr 21, 2015 at 01:43:19PM +0200, Matthias Brugger wrote:
>>> 2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
>>> > On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
>>> >> hi,
>>> >> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
>>> >> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
>>> >>
>>> >> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
>>> >> Can someone confirm if this is a bug or not?
>>> >
>>> > Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
>>> > or should I do it?
>>>
>>> Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
>>> Reading the datasheet and reference kernel code for mt6589 this SPI
>>> waveform configuration looks like magic.
>>>
>>> But reading the clock frequency which can have clk_spi in mt8135, they
>>> are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
>>> conclusion that the code is correct. Otherwise the check for bigger
>>> then 18 MHz is useless and should be deleted, if we have a similar
>>> clock distribution in mt8173.
>>
>> The CSHEXT and CSLEXT registers extend the time the chipselect stays
>> high after a SPI transfer or low before a new transfer by the given
>> number of spi clk cycles.
>> Looking at the code maybe the real question is not whether the border is
>> at 25999999Hz or at 26000000Hz, but why the transfers are throttled with
>> slower clock frequencies. I would assume the other way round makes
>> sense.
>
> After re-reading the data sheet and the reference kernel
> implementation, it seems as if three clocks exist. clk_wrap and
> clk_spi and "pmic reg_clk". And some how depending on the three clocks
> you have to calculate the values for CSHEXT, CSLEXT etc.
>
> I think we need some clarification here from Mediatek to really
> understand what is going on.

Hi Leilk,
I found you are working on mtk's spi driver.
Any comment about this discussion?

Regards,
Axel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Question about checking rate_spi in pwrap_init_reg_clock
  2015-05-08  9:33                   ` Axel Lin
@ 2015-05-08  9:51                       ` Eddie Huang
  -1 siblings, 0 replies; 12+ messages in thread
From: Eddie Huang @ 2015-05-08  9:51 UTC (permalink / raw)
  To: Axel Lin
  Cc: Leilk Liu, Sascha Hauer, fan. chen,
	linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Matthias Brugger, linux-arm-kernel

Hi Axel,

On Fri, 2015-05-08 at 17:33 +0800, Axel Lin wrote:
> CC Leilk Liu
> 
> 2015-04-21 23:42 GMT+08:00 Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> > 2015-04-21 14:52 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> >> On Tue, Apr 21, 2015 at 01:43:19PM +0200, Matthias Brugger wrote:
> >>> 2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>:
> >>> > On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
> >>> >> hi,
> >>> >> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
> >>> >> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
> >>> >>
> >>> >> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
> >>> >> Can someone confirm if this is a bug or not?
> >>> >
> >>> > Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
> >>> > or should I do it?
> >>>
> >>> Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
> >>> Reading the datasheet and reference kernel code for mt6589 this SPI
> >>> waveform configuration looks like magic.
> >>>
> >>> But reading the clock frequency which can have clk_spi in mt8135, they
> >>> are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
> >>> conclusion that the code is correct. Otherwise the check for bigger
> >>> then 18 MHz is useless and should be deleted, if we have a similar
> >>> clock distribution in mt8173.
> >>
> >> The CSHEXT and CSLEXT registers extend the time the chipselect stays
> >> high after a SPI transfer or low before a new transfer by the given
> >> number of spi clk cycles.
> >> Looking at the code maybe the real question is not whether the border is
> >> at 25999999Hz or at 26000000Hz, but why the transfers are throttled with
> >> slower clock frequencies. I would assume the other way round makes
> >> sense.
> >
> > After re-reading the data sheet and the reference kernel
> > implementation, it seems as if three clocks exist. clk_wrap and
> > clk_spi and "pmic reg_clk". And some how depending on the three clocks
> > you have to calculate the values for CSHEXT, CSLEXT etc.
> >
> > I think we need some clarification here from Mediatek to really
> > understand what is going on.
> 
> Hi Leilk,
> I found you are working on mtk's spi driver.
> Any comment about this discussion?
> 
> Regards,
> Axel
> 
Leilk's SPI driver is general SPI bus driver, different than pwrapper
SPI, which is a specific bus only used by pwrapper. For pwrapper SPI,
"clk_wrap" is clock used in 8173 pwrapper side, "clk_spi" is SPI bus
clock. "pmic reg_clk" is clock in PMIC side. We already discuss this
issue with Sascha, and I believe he will send new patch to solve this
problem.

Eddie



> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Question about checking rate_spi in pwrap_init_reg_clock
@ 2015-05-08  9:51                       ` Eddie Huang
  0 siblings, 0 replies; 12+ messages in thread
From: Eddie Huang @ 2015-05-08  9:51 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Axel,

On Fri, 2015-05-08 at 17:33 +0800, Axel Lin wrote:
> CC Leilk Liu
> 
> 2015-04-21 23:42 GMT+08:00 Matthias Brugger <matthias.bgg@gmail.com>:
> > 2015-04-21 14:52 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> >> On Tue, Apr 21, 2015 at 01:43:19PM +0200, Matthias Brugger wrote:
> >>> 2015-04-21 12:31 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> >>> > On Tue, Apr 21, 2015 at 09:58:07AM +0800, Axel Lin wrote:
> >>> >> hi,
> >>> >> The implementation in pwrap_init_reg_clock seems has off-by-one bug.
> >>> >> If rate_spi is 26000000, current code set ck_mhz to 18 rather than 26.
> >>> >>
> >>> >> I guess it needs below fix, but I'm not 100% sure as I don't have the datasheet.
> >>> >> Can someone confirm if this is a bug or not?
> >>> >
> >>> > Yes, seems to be a bug. Thanks for noting. Will you send a formal patch
> >>> > or should I do it?
> >>>
> >>> Did you have a look on both datasheets/reference kernels, mt8135 and mt8173?
> >>> Reading the datasheet and reference kernel code for mt6589 this SPI
> >>> waveform configuration looks like magic.
> >>>
> >>> But reading the clock frequency which can have clk_spi in mt8135, they
> >>> are 0 MHz, 26 MHz and bigger then 26 MHz, which leads me to the
> >>> conclusion that the code is correct. Otherwise the check for bigger
> >>> then 18 MHz is useless and should be deleted, if we have a similar
> >>> clock distribution in mt8173.
> >>
> >> The CSHEXT and CSLEXT registers extend the time the chipselect stays
> >> high after a SPI transfer or low before a new transfer by the given
> >> number of spi clk cycles.
> >> Looking at the code maybe the real question is not whether the border is
> >> at 25999999Hz or at 26000000Hz, but why the transfers are throttled with
> >> slower clock frequencies. I would assume the other way round makes
> >> sense.
> >
> > After re-reading the data sheet and the reference kernel
> > implementation, it seems as if three clocks exist. clk_wrap and
> > clk_spi and "pmic reg_clk". And some how depending on the three clocks
> > you have to calculate the values for CSHEXT, CSLEXT etc.
> >
> > I think we need some clarification here from Mediatek to really
> > understand what is going on.
> 
> Hi Leilk,
> I found you are working on mtk's spi driver.
> Any comment about this discussion?
> 
> Regards,
> Axel
> 
Leilk's SPI driver is general SPI bus driver, different than pwrapper
SPI, which is a specific bus only used by pwrapper. For pwrapper SPI,
"clk_wrap" is clock used in 8173 pwrapper side, "clk_spi" is SPI bus
clock. "pmic reg_clk" is clock in PMIC side. We already discuss this
issue with Sascha, and I believe he will send new patch to solve this
problem.

Eddie



> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-05-08  9:51 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-21  1:58 Question about checking rate_spi in pwrap_init_reg_clock Axel Lin
2015-04-21 10:31 ` Sascha Hauer
     [not found]   ` <20150421103155.GX6325-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2015-04-21 11:43     ` Matthias Brugger
2015-04-21 11:43       ` Matthias Brugger
     [not found]       ` <CABuKBeJqgWHWNYaFXS-BwEayHEtiYGt+56LLXs3QaO3MHUk0bQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-04-21 12:52         ` Sascha Hauer
2015-04-21 12:52           ` Sascha Hauer
     [not found]           ` <20150421125223.GB6325-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2015-04-21 15:42             ` Matthias Brugger
2015-04-21 15:42               ` Matthias Brugger
     [not found]               ` <CABuKBeJMKpmkVZQvjZOuEtZmVaby6wxhr-i7w6eoHLjqTNNRig-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-05-08  9:33                 ` Axel Lin
2015-05-08  9:33                   ` Axel Lin
     [not found]                   ` <CAFRkauCbpDfT0V6pEcUf=_amcwa4E_t4taA3YzU5dYMmRxGRXA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-05-08  9:51                     ` Eddie Huang
2015-05-08  9:51                       ` Eddie Huang

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.