Re: [PATCH v4] xen/char: implement suspend/resume calls for SCIF driver

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

On Thu, Jul 24, 2025 at 3:13 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 24.07.2025 13:41, Mykola Kvach wrote:
> > Hi all,
> >
> > On Tue, Jun 24, 2025 at 12:32 PM Mykola Kvach <xakep.amatop@xxxxxxxxx> 
> > wrote:
> >>
> >> On Tue, Jun 24, 2025 at 11:32 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>>
> >>> On 24.06.2025 10:29, Mykola Kvach wrote:
> >>>> On Tue, Jun 24, 2025 at 10:53 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>>>> On 24.06.2025 09:18, Mykola Kvach wrote:
> >>>>>> @@ -281,6 +313,10 @@ static struct uart_driver __read_mostly 
> >>>>>> scif_uart_driver = {
> >>>>>>      .start_tx     = scif_uart_start_tx,
> >>>>>>      .stop_tx      = scif_uart_stop_tx,
> >>>>>>      .vuart_info   = scif_vuart_info,
> >>>>>> +#ifdef CONFIG_SYSTEM_SUSPEND
> >>>>>> +    .suspend      = scif_uart_suspend,
> >>>>>> +    .resume       = scif_uart_resume,
> >>>>>> +#endif
> >>>>>>  };
> >>>>>
> >>>>> As this being put inside #ifdef was to be expected, imo a prereq change 
> >>>>> is to
> >>>>> also make the struct fields conditional in xen/console.h. I think I did 
> >>>>> even
> >>>>> comment to this effect back at the time.
> >>>>
> >>>> Would you prefer that I include this change in the current patch
> >>>> series, or is it acceptable to address it in a separate patch?
> >>>
> >>> Either way is fine with me. I expect the header fine change to be able to 
> >>> go
> >>> in right away (once submitted), whereas the patch here may take some time 
> >>> for
> >>> people to review.
> >>
> >> Got it, I'll submit a separate patch to make the struct fields and
> >> related code wrapped within SYSTEM_SUSPEND.
> >
> > Jan’s proposal to conditionally include the 'suspend' and 'resume' fields
> > in 'struct uart_driver' under CONFIG_SYSTEM_SUSPEND has already been
> > merged -- thanks!
> >
> > Could you please take another look at this patch when time permits?
>
> That's an Arm driver, so I don't expect the request was meant to go to me
> (as To: having just me was suggesting)?

You're right -- this is an Arm driver, and I didn’t mean to direct
the request solely to you. Others in CC are also involved.

I thought the review of this patch had stalled following your
previous comment, so I just wanted to notify everyone involved
that the related changes have already been merged. With that out
of the way, I hope this remaining patch can now be reviewed and,
if acceptable, merged as well.

I’m not entirely familiar with the proper process for these cases,
so apologies if this ping was inappropriate or caused any
disruption.

>
> Jan

Best regards,
Mykola