On Wed, 2021-07-14 at 18:32 +1000, Alexey Kardashevskiy wrote: > > > On 13/07/2021 14:47, Leonardo Brás wrote: > > Hello Alexey, > > > > On Fri, 2021-06-18 at 19:26 -0300, Leonardo Brás wrote: > > > > > > > > > + 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 :) > > > 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
