Re: [XEN v5 02/10] xen/arm: Typecast the DT values into paddr_t

[Michal Orzel <michal.orzel@xxxxxxx>]

On 19/04/2023 16:20, Julien Grall wrote:
> 
> 
> On 19/04/2023 14:39, Michal Orzel wrote:
>> Hi Julien,
>>
>> On 19/04/2023 15:26, Julien Grall wrote:
>>>
>>>
>>> Hi,
>>>
>>>>> diff --git a/xen/include/xen/libfdt/libfdt-xen.h 
>>>>> b/xen/include/xen/libfdt/libfdt-xen.h
>>>>> new file mode 100644
>>>>> index 0000000000..3296a368a6
>>>>> --- /dev/null
>>>>> +++ b/xen/include/xen/libfdt/libfdt-xen.h
>>>>> @@ -0,0 +1,55 @@
>>>>> +/*
>>>>> + * SPDX-License-Identifier: GPL-2.0-only
>>>> Our CODING_STYLE says:
>>>> New files should start with a single-line SPDX comment, ..., e.g.
>>>> /* SPDX-License-Identifier: GPL-2.0 */
>>>>
>>>> For me it would be perfectly fine to do as you did but it is not what our 
>>>> docs state
>>>> (i.e. single-line comment). It might be that we need to modify 
>>>> CODING_STYLE instead.
>>>
>>>   From my reading of https://spdx.dev/ids/#how, what you suggest would
>>> not be a valid SPDX-License-Idenfier as everything in needs to be in
>>> *one* (including the begin/end comment).
>> I said what is written in our CODING_STYLE and what we did already for lots 
>> of files, i.e. having 2 comments:
>> /* SPDX-License-Identifier: GPL-2.0 */
>> /*
>>   * ...
>>   */
> 
> My comment was about your suggestion to update the CODING_STYLE not what
> it is currently written. Sorry if this wasn't clear enough.
> 
>>  > In the link you provided, the "*one line*" requirement refers only to
> SPDX short form identifier
>> which does not contain any other information like author, description, etc..
> Right because the SPDX block is intended to be in own block and there is
> a another block for the author, description, etc.
> 
> I am not in favor of changing of the CODING_STYLE and I thought I would
> express it right now to spare the round-trip of writing a patch.
Sure thing :)
So, all in all, we agree that SPDX comment must be a single line comment on its 
own
(which is also stated in our CODING_STYLE) and not like it was placed in this 
patch, right?

If so, somehow the wrong placement sneaked into recently added RISCV files:
arch/riscv/include/asm/csr.h
arch/riscv/include/asm/riscv_encoding.h

~Michal