On 6/25/19 1:59 AM, Vladimir Oltean wrote:
> On Tue, 25 Jun 2019 at 01:17, Marek Vasut <ma...@denx.de> wrote:
>>
>> On 6/24/19 12:35 AM, Marek Vasut wrote:
>>> Add basic SPI regmap support into the driver.
>>>
>>> Previous patches unconver that ksz_spi_write() is always ever called
>>> with len = 1, 2 or 4. We can thus drop the if (len > SPI_TX_BUF_LEN)
>>> check and we can also drop the allocation of the txbuf which is part
>>> of the driver data and wastes 256 bytes for no reason. Regmap covers
>>> the whole thing now.
>>>
>>> Signed-off-by: Marek Vasut <ma...@denx.de>
>>> Cc: Andrew Lunn <and...@lunn.ch>
>>> Cc: Florian Fainelli <f.faine...@gmail.com>
>>> Cc: Tristram Ha <tristram...@microchip.com>
>>> Cc: Woojung Huh <woojung....@microchip.com>
>>
>> [...]
>>
>>> +#define KS_SPIOP_FLAG_MASK(opcode) \
>>> + cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
>>
>> So the robot is complaining about this. I believe this is correct, as
>> the mask should be in native endianness on the register and NOT the
>> native endianness of the CPU.
>>
>> I think a cast would help here, e.g.:
>> - cpu_to_be32((opcode) << (SPI_ADDR_SHIFT + SPI_TURNAROUND_SHIFT))
>> - (__force unsigned long)cpu_to_be32((opcode) << (SPI_ADDR_SHIFT +
>> SPI_TURNAROUND_SHIFT))
>>
>> Does this make sense ?
>>
>>> +#define KSZ_REGMAP_COMMON(width) \
>>> + { \
>>> + .val_bits = (width), \
>>> + .reg_stride = (width) / 8, \
>>> + .reg_bits = SPI_ADDR_SHIFT + SPI_ADDR_ALIGN, \
>>> + .pad_bits = SPI_TURNAROUND_SHIFT, \
>>> + .max_register = BIT(SPI_ADDR_SHIFT) - 1, \
>>> + .cache_type = REGCACHE_NONE, \
>>> + .read_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_RD), \
>>> + .write_flag_mask = KS_SPIOP_FLAG_MASK(KS_SPIOP_WR), \
>>
>> [...]
>>
>> --
>> Best regards,
>> Marek Vasut
>
> Hi Marek,
>
> I saw SPI buffers and endianness and got triggered :)
> Would it make sense to take a look at CONFIG_PACKING for the KSZ9477 driver?
> I don't know how bad the field alignment issue is on that hardware,
> but on SJA1105 it was such a disaster that I couldn't have managed it
> any other way.
How does that help me here ? All I really need is a static constant mask
for the register/flags , 32bit for KSZ9xxx and 16bit for KSZ87xx. I
don't need any dynamic stuff, luckily.
But I'm glad to see TJA1105 driver mainline :)
--
Best regards,
Marek Vasut
