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
> >
> >> #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
> >
>
