mbox series

[0/3] xive/p9: more cleanups

Message ID 20200624091625.324632-1-clg@kaod.org
Headers show
Series xive/p9: more cleanups | expand

Message

Cédric Le Goater June 24, 2020, 9:16 a.m. UTC
Hello,

To be applied on top of the series "xive/p9: various small cleanups" :

   http://patchwork.ozlabs.org/project/skiboot/list/?series=182976

Thanks,

C.

Cédric Le Goater (3):
  xive/p9: use PAGE_SIZE
  xive/p9: Introduce XIVE_ESB_SIZE
  xive/P9: Use NUM_INT_PRIORITIES in xive_reset()

 hw/xive.c | 55 ++++++++++++++++++++++++++++---------------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

Comments

Dan Horák June 24, 2020, 1:22 p.m. UTC | #1
On Wed, 24 Jun 2020 11:16:22 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> Hello,
> 
> To be applied on top of the series "xive/p9: various small cleanups" :
> 
>    http://patchwork.ozlabs.org/project/skiboot/list/?series=182976
> 
> Thanks,
> 
> C.
> 
> Cédric Le Goater (3):
>   xive/p9: use PAGE_SIZE
>   xive/p9: Introduce XIVE_ESB_SIZE
>   xive/P9: Use NUM_INT_PRIORITIES in xive_reset()

^^^ a nitpick - you have an uppercase P here in the subject

not an expert on the internals, but from a general point of view all
changes look reasonable

Reviewed-by: Dan Horák <dan@danny.cz>


		Dan


> 
>  hw/xive.c | 55 +++++++++++++++++++++++++++
> +--------------------------- 1 file changed, 28 insertions(+), 27
> deletions(-)
> 
> -- 
> 2.25.4
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
Oliver O'Halloran July 3, 2020, 5:33 a.m. UTC | #2
On Wed, Jun 24, 2020 at 7:16 PM Cédric Le Goater <clg@kaod.org> wrote:
>
> Hello,
>
> To be applied on top of the series "xive/p9: various small cleanups" :
>
>    http://patchwork.ozlabs.org/project/skiboot/list/?series=182976

I think a patch went AWOL at some point since there was a conflict
with this hunk:

@@ -1675,8 +1675,8 @@ static bool xive_prealloc_tables(struct xive *x)
        x->eq_ind_count = XIVE_EQ_TABLE_SIZE / XIVE_VSD_SIZE;

        /* Indirect VP table.  Limited to one top page. */
-       al = ALIGN_UP(XIVE_VP_TABLE_SIZE, 0x10000);
-       if (al > 0x10000) {
+       al = ALIGN_UP(XIVE_VP_TABLE_SIZE, PAGE_SIZE);
+       if (al > PAGE_SIZE) {
                xive_err(x, "VP indirect table is too big !\n");
                return false;
        }

After the first series was applied the first context wasn't matching
because it was still

x->eq_ind_count = XIVE_EQ_TABLE_SIZE / 8;

I fixed it up in 86b617c7f74a9122e9572657a9ab9b4c6ba13c4c. Hope that's
ok. Anyway, series merged as of 132f5a8747af.
Cédric Le Goater July 13, 2020, 8:48 a.m. UTC | #3
On 7/3/20 7:33 AM, Oliver O'Halloran wrote:
> On Wed, Jun 24, 2020 at 7:16 PM Cédric Le Goater <clg@kaod.org> wrote:
>>
>> Hello,
>>
>> To be applied on top of the series "xive/p9: various small cleanups" :
>>
>>    http://patchwork.ozlabs.org/project/skiboot/list/?series=182976
> 
> I think a patch went AWOL at some point since there was a conflict
> with this hunk:
> 
> @@ -1675,8 +1675,8 @@ static bool xive_prealloc_tables(struct xive *x)
>         x->eq_ind_count = XIVE_EQ_TABLE_SIZE / XIVE_VSD_SIZE;
> 
>         /* Indirect VP table.  Limited to one top page. */
> -       al = ALIGN_UP(XIVE_VP_TABLE_SIZE, 0x10000);
> -       if (al > 0x10000) {
> +       al = ALIGN_UP(XIVE_VP_TABLE_SIZE, PAGE_SIZE);
> +       if (al > PAGE_SIZE) {
>                 xive_err(x, "VP indirect table is too big !\n");
>                 return false;
>         }
> 
> After the first series was applied the first context wasn't matching
> because it was still
> 
> x->eq_ind_count = XIVE_EQ_TABLE_SIZE / 8;

Yes. I missed 86b617c7f74a it seems.
 
> I fixed it up in 86b617c7f74a9122e9572657a9ab9b4c6ba13c4c. Hope that's
> ok. Anyway, series merged as of 132f5a8747af.

Looks good to me.

Thanks,

C.