Re: [Xen-devel] [PATCH] arm/timer: fix panic when booting with DT

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.03.16 at 02:17, <zhaoshenglong@xxxxxxxxxx> wrote:
> On 2016/3/3 23:44, Stefano Stabellini wrote:
>> On Thu, 3 Mar 2016, Shannon Zhao wrote:
>>> > From: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>>> > 
>>> > While to support ACPI, patch "arm/acpi: Parse GTDT to initialize timer"
>>> > refactors the functions preinit_xen_time and init_xen_time. But it
>>> > wrongly moves the platform_get_irq from init_xen_time to
>>> > preinit_dt_xen_time and this will cause booting failure.
>>> > 
>>> > So move platform_get_irq back to init_xen_time to fix it.
>>> > 
>>> > Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
>> Shannon,
>> thank you for fixing the issue so quickly, very appreciated!
>> 
>> 
>>> >  xen/arch/arm/time.c | 27 ++++++++++++++++-----------
>>> >  1 file changed, 16 insertions(+), 11 deletions(-)
>>> > 
>>> > diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
>>> > index 5f8f974..66a4520 100644
>>> > --- a/xen/arch/arm/time.c
>>> > +++ b/xen/arch/arm/time.c
>>> > @@ -119,7 +119,6 @@ static void __init preinit_dt_xen_time(void)
>>> >      };
>>> >      int res;
>>> >      u32 rate;
>>> > -    unsigned int i;
>>> >  
>>> >      timer = dt_find_matching_node(NULL, timer_ids);
>>> >      if ( !timer )
>>> > @@ -133,16 +132,6 @@ static void __init preinit_dt_xen_time(void)
>>> >          cpu_khz = rate / 1000;
>>> >          timer_dt_clock_frequency = rate;
>>> >      }
>>> > -
>>> > -    /* Retrieve all IRQs for the timer */
>>> > -    for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
>>> > -    {
>>> > -        res = platform_get_irq(timer, i);
>>> > -
>>> > -        if ( res < 0 )
>>> > -            panic("Timer: Unable to retrieve IRQ %u from the device 
>>> > tree", i);
>>> > -        timer_irq[i] = res;
>>> > -    }
>>> >  }
>>> >  
>>> >  void __init preinit_xen_time(void)
>>> > @@ -168,6 +157,22 @@ void __init preinit_xen_time(void)
>>> >  /* Set up the timer on the boot CPU (late init function) */
>>> >  int __init init_xen_time(void)
>>> >  {
>>> > +    int res;
>>> > +    unsigned int i;
>>> > +
>>> > +    if ( acpi_disabled )
>>> > +    {
>>> > +        /* Retrieve all IRQs for the timer */
>>> > +        for ( i = TIMER_PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++ )
>>> > +        {
>>> > +            res = platform_get_irq(timer, i);
>>> > +
>>> > +            if ( res < 0 )
>>> > +                panic("Timer: Unable to retrieve IRQ %u from the device 
>>> > tree", i);
>>> > +            timer_irq[i] = res;
>>> > +        }
>>> > +    }
>> Could you please introduce a small little init_dt_xen_time function and
>> call that instead from here?  Thanks!
>> 
> Not sure if it's necessary since it's only used here. But if you really
> want that I'll add.

Can both of you please aim at getting to an agreement relatively
quickly, to stop the flood of smoke test failure reports? (In general,
Shannon, I would say that if you were asked by the maintainer to
do a certain transformation, there's not overly much reason to ask
back again.) Or alternatively please indicate how far back the
previous series would need to be reverted.

Thanks, Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel