On Wed, 2021-07-14 at 18:32 +1000, Alexey Kardashevskiy wrote:

On 13/07/2021 14:47, Leonardo Brás wrote:

quoted

Hello Alexey,

On Fri, 2021-06-18 at 19:26 -0300, Leonardo Brás wrote:

quoted

quoted

quoted

+                                        unsigned long liobn,
unsigned long win_addr,
+                                        unsigned long
window_size,
unsigned long page_shift,
+                                        unsigned long base,
struct
iommu_table_ops *table_ops)

iommu_table_setparms() rather than passing 0 around.

The same comment about "liobn" - set it in
iommu_table_setparms_lpar().
The reviewer will see what field atters in what situation imho.

The idea here was to keep all tbl parameters setting in
_iommu_table_setparms (or iommu_table_setparms_common).

I understand the idea that each one of those is optional in the
other
case, but should we keep whatever value is present in the other
variable (not zeroing the other variable), or do someting like:

tbl->it_index = 0;
tbl->it_base = basep;
(in iommu_table_setparms)

tbl->it_index = liobn;
tbl->it_base = 0;
(in iommu_table_setparms_lpar)

This one is supposed to be a question, but I missed the question
mark.
Sorry about that.

Ah ok :)

quoted

I would like to get your opinion in this :)

Besides making the "base" parameter a pointer, I really do not have 
strong preference, just make it not hurting eyes of a reader, that's
all :)

Ok, done :)

imho in general, rather than answering 5 weeks later, it is more 
productive to address whatever comments were made, add comments (in
the 
code or commit logs) why you are sticking to your initial approach, 
rebase and repost the whole thing. Thanks,

Thanks for the tip, and for the reviewing :)

Best regards,
Leonardo Bras