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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.07.2025 18:48, Mykola Kvach wrote:
> 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.

It's not properly written down anywhere, afaik. My (personal) request
is that you make clear who you expect input from by properly arranging
To: vs Cc:.

Jan