Andy,
Thanks for the comments.
On 10/8/24 11:23 PM, Andy Shevchenko wrote:
> On Tue, Oct 08, 2024 at 02:19:15PM +0800, bingbu.cao@xxxxxxxxx wrote:
>> From: Bingbu Cao <bingbu.cao@xxxxxxxxx>
>>
>> This patch fixes the build errors with `i386-allmodconfig`, the
>> errors are caused by wrong type cast and 64-bit division.
>
> Thanks for the change, my comments below.
>
> ...
>
>> /* Shared structure between driver and FW - do not modify */
>> struct ipu6_fw_sys_queue {
>> - u64 host_address;
>> + uintptr_t host_address;
>
> Okay, in the given semantic this probably should be phys_addr_t.
> BUT, is this address somehow is going to be used by IPU6 hardware?
> If "yes", the type shall not be changed.
>
> Looking at types used I hope the answer is "no", otherwise the types
> in the structures should be properly choose WRT endianess (and what
> __packed is doing here? Is it part of the protocol?).
You are correct, the type here should not be changed.
>
>> u32 vied_address;
>> u32 size;
>> u32 token_size;
>> @@ -40,7 +40,7 @@ struct ipu6_fw_sys_queue {
>> } __packed;
>>
>> struct ipu6_fw_sys_queue_res {
>> - u64 host_address;
>> + uintptr_t host_address;
>
> Ditto.
>
>> u32 vied_address;
>> u32 reg;
>> } __packed;
>
> ...
>
>> - dev_dbg(dev, "write: reg 0x%lx = data 0x%x", base + addr - isys_base,
>> - data);
>> + dev_dbg(dev, "write: reg 0x%lx = data 0x%x",
>> + (ulong)(base + addr - isys_base), data);
>
> No, one should use proper specifiers for this. And what the heck 'ulong' is?
> Where is it being defined?
>
> ...
>
>> - dev_dbg(dev, "read: reg 0x%lx = data 0x%x", base + addr - isys_base,
>> - data);
>> + dev_dbg(dev, "read: reg 0x%lx = data 0x%x",
>> + (ulong)(base + addr - isys_base), data);
>
> Ditto.
>
> ...
>
>> pg_offset = server_fw_addr - dma_addr;
>> - prog = (struct ipu6_cell_program *)((u64)isp->cpd_fw->data + pg_offset);
>> + prog = (struct ipu6_cell_program *)((uintptr_t)isp->cpd_fw->data +
>> + pg_offset);
>
> Side Q: What are the alignment requirements for the prog pointer?
It should be 64 bytes alignment.
>
--
Best regards,
Bingbu Cao
