lkml.org 
[lkml]   [2020]   [Jun]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 1/4] powerpc/pseries/iommu: Update call to ibm,query-pe-dma-windows
From
Date


On 23/06/2020 12:14, Leonardo Bras wrote:
> On Tue, 2020-06-23 at 11:12 +1000, Alexey Kardashevskiy wrote:
> [snip]
>>>>> +static int query_ddw_out_sz(struct device_node *par_dn)
>>>>
>>>> Can easily be folded into query_ddw().
>>>
>>> Sure, but it will get inlined by the compiler, and I think it reads
>>> better this way.
>>> I mean, I understand you have a reason to think it's better to fold it
>>> in query_ddw(), and I would like to better understand that to improve
>>> my code in the future.
>>
>> You have numbers 5 and 6 (the number of parameters) twice in the file,
>> this is why I brought it up. query_ddw_out_sz() can potentially return
>> something else than 5 or 6 and you will have to change the callsite(s)
>> then, since these are not macros, this allows to think there may be more
>> places with 5 and 6. Dunno. A single function will simplify things imho.
>
> That's a good point. Thanks!
>
>>
>>
>>>>> +{
>>>>> + int ret;
>>>>> + u32 ddw_ext[3];
>>>>> +
>>>>> + ret = of_property_read_u32_array(par_dn, "ibm,ddw-extensions",
>>>>> + &ddw_ext[0], 3);
>>>>> + if (ret || ddw_ext[0] < 2 || ddw_ext[2] != 1)
>>>>
>>>> Oh that PAPR thing again :-/
>>>>
>>>> ===
>>>> The “ibm,ddw-extensions” property value is a list of integers the first
>>>> integer indicates the number of extensions implemented and subsequent
>>>> integers, one per extension, provide a value associated with that
>>>> extension.
>>>> ===
>>>>
>>>> So ddw_ext[0] is length.
>>>> Listindex==2 is for "reset" says PAPR and
>>>> Listindex==3 is for this new 64bit "largest_available_block".
>>>>
>>>> So I'd expect ddw_ext[2] to have the "reset" token and ddw_ext[3] to
>>>> have "1" for this new feature but indexes are smaller. I am confused.
>>>> Either way these "2" and "3" needs to be defined in macros, "0" probably
>>>> too.
>>>
>>> Remember these indexes are not C-like 0-starting indexes, where the
>>> size would be Listindex==1.
>>
>> Yeah I can see that is the assumption but out of curiosity - is it
>> written anywhere? Across PAPR, they index bytes from 1 but bits from 0 :-/
>
> From LoPAR:
> The “ibm,ddw-extensions” property value is a list of integers the first
> integer indicates the number of extensions implemented and subsequent
> integers, one per extension, provide a value associated with that
> extension.
>
> And the list/table then shows extensions from 2 on onwards:
> List index 2 : Token of the ibm,reset-pe-dma-windows RTAS Call
> (...)


I means a place saying "we number things starting from 1 and not from
zero", this kind of thing. The code implementing the spec uses the C
language so it would make sense to count from zero, otoh the writer
probably did not write any code for ages :)



--
Alexey

\
 
 \ /
  Last update: 2020-06-23 04:30    [W:0.050 / U:2.360 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site