Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1625704 > unrolled thread

Re: [PATCH v7 3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR) SPI protocols

Started byMarek Vasut <marek.vasut@gmail.com>
First post2017-04-19 01:10 +0200
Last post2017-04-19 23:40 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v7 3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR)  SPI protocols Marek Vasut <marek.vasut@gmail.com> - 2017-04-19 01:10 +0200
    Re: [PATCH v7 3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR)  SPI protocols Cyrille Pitchen <cyrille.pitchen@wedev4u.fr> - 2017-04-19 23:00 +0200
      Re: [PATCH v7 3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR)  SPI protocols Marek Vasut <marek.vasut@gmail.com> - 2017-04-19 23:40 +0200

#1625704 — Re: [PATCH v7 3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR) SPI protocols

FromMarek Vasut <marek.vasut@gmail.com>
Date2017-04-19 01:10 +0200
SubjectRe: [PATCH v7 3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR) SPI protocols
Message-ID<txKpY-3KK-43@gated-at.bofh.it>
On 04/19/2017 12:51 AM, Cyrille Pitchen wrote:
> This patch introduces support to Double Transfer Rate (DTR) SPI protocols.
> DTR is used only for Fast Read operations.
> 
> According to manufacturer datasheets, whatever the number of I/O lines
> used during instruction (x) and address/mode/dummy (y) clock cycles, DTR
> is used only during data (z) clock cycles of SPI x-y-z protocols.
> 
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>

[...]

> @@ -282,19 +305,22 @@ struct spi_nor_hwcaps {
>   * As a matter of performances, it is relevant to use Quad SPI protocols first,
>   * then Dual SPI protocols before Fast Read and lastly (Slow) Read.
>   */
> -#define SNOR_HWCAPS_READ_MASK		GENMASK(7, 0)
> +#define SNOR_HWCAPS_READ_MASK		GENMASK(10, 0)
>  #define SNOR_HWCAPS_READ		BIT(0)
>  #define SNOR_HWCAPS_READ_FAST		BIT(1)
> -
> -#define SNOR_HWCAPS_READ_DUAL		GENMASK(4, 2)
> -#define SNOR_HWCAPS_READ_1_1_2		BIT(2)
> -#define SNOR_HWCAPS_READ_1_2_2		BIT(3)
> -#define SNOR_HWCAPS_READ_2_2_2		BIT(4)
> -
> -#define SNOR_HWCAPS_READ_QUAD		GENMASK(7, 5)
> -#define SNOR_HWCAPS_READ_1_1_4		BIT(5)
> -#define SNOR_HWCAPS_READ_1_4_4		BIT(6)
> -#define SNOR_HWCAPS_READ_4_4_4		BIT(7)
> +#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
> +
> +#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
> +#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
> +#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
> +#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
> +#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
> +
> +#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
> +#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
> +#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
> +#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
> +#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)

I can't say I'm a big fan on this re-numeration when you add a new
entry. But unless you have a better idea, we'll have to live with this ...

-- 
Best regards,
Marek Vasut

[toc] | [next] | [standalone]


#1626794

FromCyrille Pitchen <cyrille.pitchen@wedev4u.fr>
Date2017-04-19 23:00 +0200
Message-ID<ty4RI-845-7@gated-at.bofh.it>
In reply to#1625704
Hi Marek,

Le 19/04/2017 à 01:05, Marek Vasut a écrit :
> On 04/19/2017 12:51 AM, Cyrille Pitchen wrote:
>> This patch introduces support to Double Transfer Rate (DTR) SPI protocols.
>> DTR is used only for Fast Read operations.
>>
>> According to manufacturer datasheets, whatever the number of I/O lines
>> used during instruction (x) and address/mode/dummy (y) clock cycles, DTR
>> is used only during data (z) clock cycles of SPI x-y-z protocols.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> 
> [...]
> 
>> @@ -282,19 +305,22 @@ struct spi_nor_hwcaps {
>>   * As a matter of performances, it is relevant to use Quad SPI protocols first,
>>   * then Dual SPI protocols before Fast Read and lastly (Slow) Read.
>>   */
>> -#define SNOR_HWCAPS_READ_MASK		GENMASK(7, 0)
>> +#define SNOR_HWCAPS_READ_MASK		GENMASK(10, 0)
>>  #define SNOR_HWCAPS_READ		BIT(0)
>>  #define SNOR_HWCAPS_READ_FAST		BIT(1)
>> -
>> -#define SNOR_HWCAPS_READ_DUAL		GENMASK(4, 2)
>> -#define SNOR_HWCAPS_READ_1_1_2		BIT(2)
>> -#define SNOR_HWCAPS_READ_1_2_2		BIT(3)
>> -#define SNOR_HWCAPS_READ_2_2_2		BIT(4)
>> -
>> -#define SNOR_HWCAPS_READ_QUAD		GENMASK(7, 5)
>> -#define SNOR_HWCAPS_READ_1_1_4		BIT(5)
>> -#define SNOR_HWCAPS_READ_1_4_4		BIT(6)
>> -#define SNOR_HWCAPS_READ_4_4_4		BIT(7)
>> +#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
>> +
>> +#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
>> +#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
>> +#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
>> +#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
>> +#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
>> +
>> +#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
>> +#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
>> +#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
>> +#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
>> +#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
> 
> I can't say I'm a big fan on this re-numeration when you add a new
> entry. But unless you have a better idea, we'll have to live with this ...
> 

Well, the other solution would be to reserve unused bit position in
patch 1 but would look odd too, wouldn't it?

As explained in the comments just above those definitions, the order of
the bits *does* matter. So maybe in the future, those bits would have to
be reordered again depending on the new features we would add then.

Thanks for your review!

Best regards,

Cyrille

[toc] | [prev] | [next] | [standalone]


#1626813

FromMarek Vasut <marek.vasut@gmail.com>
Date2017-04-19 23:40 +0200
Message-ID<ty5uq-8wq-19@gated-at.bofh.it>
In reply to#1626794
On 04/19/2017 10:20 PM, Cyrille Pitchen wrote:
> Hi Marek,
> 
> Le 19/04/2017 à 01:05, Marek Vasut a écrit :
>> On 04/19/2017 12:51 AM, Cyrille Pitchen wrote:
>>> This patch introduces support to Double Transfer Rate (DTR) SPI protocols.
>>> DTR is used only for Fast Read operations.
>>>
>>> According to manufacturer datasheets, whatever the number of I/O lines
>>> used during instruction (x) and address/mode/dummy (y) clock cycles, DTR
>>> is used only during data (z) clock cycles of SPI x-y-z protocols.
>>>
>>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>>
>> [...]
>>
>>> @@ -282,19 +305,22 @@ struct spi_nor_hwcaps {
>>>   * As a matter of performances, it is relevant to use Quad SPI protocols first,
>>>   * then Dual SPI protocols before Fast Read and lastly (Slow) Read.
>>>   */
>>> -#define SNOR_HWCAPS_READ_MASK		GENMASK(7, 0)
>>> +#define SNOR_HWCAPS_READ_MASK		GENMASK(10, 0)
>>>  #define SNOR_HWCAPS_READ		BIT(0)
>>>  #define SNOR_HWCAPS_READ_FAST		BIT(1)
>>> -
>>> -#define SNOR_HWCAPS_READ_DUAL		GENMASK(4, 2)
>>> -#define SNOR_HWCAPS_READ_1_1_2		BIT(2)
>>> -#define SNOR_HWCAPS_READ_1_2_2		BIT(3)
>>> -#define SNOR_HWCAPS_READ_2_2_2		BIT(4)
>>> -
>>> -#define SNOR_HWCAPS_READ_QUAD		GENMASK(7, 5)
>>> -#define SNOR_HWCAPS_READ_1_1_4		BIT(5)
>>> -#define SNOR_HWCAPS_READ_1_4_4		BIT(6)
>>> -#define SNOR_HWCAPS_READ_4_4_4		BIT(7)
>>> +#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
>>> +
>>> +#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
>>> +#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
>>> +#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
>>> +#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
>>> +#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
>>> +
>>> +#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
>>> +#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
>>> +#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
>>> +#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
>>> +#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
>>
>> I can't say I'm a big fan on this re-numeration when you add a new
>> entry. But unless you have a better idea, we'll have to live with this ...
>>
> 
> Well, the other solution would be to reserve unused bit position in
> patch 1 but would look odd too, wouldn't it?

Yeah, that's not super either ... I was pondering if there might be some
less error-prone way to autogenerate this maybe.

> As explained in the comments just above those definitions, the order of
> the bits *does* matter. So maybe in the future, those bits would have to
> be reordered again depending on the new features we would add then.
> 
> Thanks for your review!
> 
> Best regards,
> 
> Cyrille
> 


-- 
Best regards,
Marek Vasut

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web