On 12/16/25 8:36 AM, Aditya Gupta wrote:
> On 25/12/16 07:35AM, Caleb Schlossin wrote:
>>
>>
>> On 12/16/25 6:58 AM, Aditya Gupta wrote:
>>> Hello Caleb,
>>>
>>> On 25/12/15 11:18AM, Caleb Schlossin wrote:
>>>> <...snip...>
>>>>
>>>> --- a/hw/ppc/pnv_psi.c
>>>> +++ b/hw/ppc/pnv_psi.c
>>>> @@ -25,6 +25,7 @@
>>>>  #include "qemu/module.h"
>>>>  #include "system/reset.h"
>>>>  #include "qapi/error.h"
>>>> +#include "migration/vmstate.h"
>>>>  
>>>>  
>>>>  #include "hw/ppc/fdt.h"
>>>> @@ -35,6 +36,8 @@
>>>>  
>>>>  #include <libfdt.h>
>>>>  
>>>> +#undef PSI_DEBUG
>>>> +
>>>
>>> Is this intended or got left over from debugging ?
>>
>> This was indented to aid future debug, if needed.
>>
> 
> If so, i believe this should be removed.
> 
> With this we just unconditionally noped out the #ifdef PSI_DEBUG
> portions in this file, even if the user explicitly compiles with PSI_DEBUG
> 
> What do you say ?
> 
> - Aditya G

Sounds good, I'll remove it and send up a new set of patches.

> 
>>>
>>>>  #define PSIHB_XSCOM_FIR_RW      0x00
>>>>  #define PSIHB_XSCOM_FIR_AND     0x01
>>>>  #define PSIHB_XSCOM_FIR_OR      0x02
>>>> @@ -130,12 +133,11 @@ static void pnv_psi_set_bar(PnvPsi *psi, uint64_t 
>>>> bar)
>>>>  {
>>>>      PnvPsiClass *ppc = PNV_PSI_GET_CLASS(psi);
>>>>      MemoryRegion *sysmem = get_system_memory();
>>>> -    uint64_t old = psi->regs[PSIHB_XSCOM_BAR];
>>>>  
>>>>      psi->regs[PSIHB_XSCOM_BAR] = bar & (ppc->bar_mask | PSIHB_BAR_EN);
>>>>  
>>>>      /* Update MR, always remove it first */
>>>> -    if (old & PSIHB_BAR_EN) {
>>>> +    if (memory_region_is_mapped(&psi->regs_mr)) {
>>>>          memory_region_del_subregion(sysmem, &psi->regs_mr);
>>>>      }
>>>>  
>>>> @@ -975,6 +977,40 @@ static void pnv_psi_register_types(void)
>>>>  
>>>>  type_init(pnv_psi_register_types);
>>>>  
>>>> +#ifdef PSI_DEBUG
>>>> +static void psi_regs_pic_print_info(uint64_t *regs, uint32_t nr_regs,
>>>> +                                    GString *buf) {
>>>> +    uint i, prev_idx = -1;
>>>> +    uint64_t  reg1, prev_reg1 = -1;
>>>> +    uint64_t  reg2, prev_reg2 = -1;
>>>> +    uint64_t  reg3, prev_reg3 = -1;
>>>> +    uint64_t  reg4, prev_reg4 = -1;
>>>
>>> Very minor nitpick, 2 spaces in the declaration between type and name.
>>> checkpatch doesn't point it out, so it's okay with me.
>>>
>>> Looks good to me overall. Please just see if the #undef was intentional,
>>> if so:
>>>
>>> Reviewed-by: Aditya Gupta <[email protected]>
>>>
>>> Thanks,
>>> - Aditya G
>>>
>>


Reply via email to