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.pitc...@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
