On 12/10/2016 06:34 PM, Cédric Le Goater wrote:
> Hello,
Hi!
> On 12/10/2016 05:01 AM, Marek Vasut wrote:
>> On 12/09/2016 05:49 PM, Cédric Le Goater wrote:
>>
>> [...]
>>
>>
>>> +static int aspeed_smc_read_from_ahb(void *buf, const void __iomem *src,
>>> + size_t len)
>>> +{
>>> + if (IS_ALIGNED((u32)src, sizeof(u32)) &&
>>> + IS_ALIGNED((u32)buf, sizeof(u32)) &&
>>> + IS_ALIGNED(len, sizeof(u32))) {
>>
>> Did you try compiling this on any 64bit system ?
>
> I cross compile the kernel on a 64bit host and then upload on the
> target. What kind of problem are you forseeing ?
Something about the pointer being clipped to 4 bytes, I'd rather use
uintptr_t cast instead of u32 . I don't think it matters for this check,
but just to be extra precise ...
>>
>>> + while (len > 3) {
>>> + *(u32 *)buf = readl(src);
[...]
>>> +/*
>>> + * Segment Address Registers. Start and end addresses are encoded
>>> + * using 8MB units
>>> + */
>>> +#define SEGMENT_ADDR_REG0 0x30
>>> +#define SEGMENT_ADDR_START(_r) ((((_r) >> 16) & 0xFF) << 23)
>>
>> is that ((r) & 0xff0000) << 7 ?
>>
>>> +#define SEGMENT_ADDR_END(_r) ((((_r) >> 24) & 0xFF) << 23)
>>
>> ((r) & 0xff000000) >> 1 ?
>
> yes.
>
> I rather keep the initial macros though, which I found easier to
> understand.
>
> The Segment Register uses a 8MB unit to encode the start address
> and the end address of the mapping window of a flash SPI slave :
>
> | byte 1 | byte 2 | byte 3 | byte 4 |
> +--------+--------+--------+--------+
> | end | start | 0 | 0 |
Then the above should be in the comment , it's a good explanation :)
Thanks!
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
