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