Re: [PATCH v10 3/9] hw/fsi: Introduce IBM's cfam

2024-01-25 Thread Ninad Palsule 

Hello Cedric,



[ clg: - moved object FSIScratchPad under FSICFAMState
    - moved FSIScratchPad code under cfam.c
    - introduced fsi_cfam_instance_init()
    - reworked fsi_cfam_realize() ]


Move the list down before my S-o-b.

Done.




+
+/* Valid, slots, version, type, crc */
+#define CFAM_CONFIG_REG_FSI_SLAVE (ENGINE_CONFIG_NEXT   | \
+ 0x0001   | \
+ 0x5000   | \
+ ENGINE_CONFIG_TYPE_FSI   | \
+    0x000a)
+
+/* Valid, slots, version, type, crc */
+#define CFAM_CONFIG_REG_SCRATCHPAD (ENGINE_CONFIG_NEXT   | \
+ 0x0001   | \
+ 0x1000   | \
+ ENGINE_CONFIG_TYPE_SCRATCHPAD | \
+    0x0007)


I was expecting a macro taking argument to build the config reg value
of each sub engine but that's fine also.


Added single macro.






+
+    memory_region_add_subregion(>mr, 0, >config_iomem);
+    memory_region_add_subregion(>mr, 0x800, >iomem);
+    memory_region_add_subregion(>mr, 0xc00, >lbus.mr);
+
+    /* Add scratchpad engine */
+    if (!qdev_realize(DEVICE(>scratchpad), BUS(>lbus),
+    errp)) {


could be a single line.


Yep, Made it a single line.

Thanks for the review.

Regards,

Ninad

Re: [PATCH v10 3/9] hw/fsi: Introduce IBM's cfam

2024-01-12 Thread Cédric Le Goater 

On 1/11/24 00:15, Ninad Palsule wrote:

This is a part of patchset where IBM's Flexible Service Interface is
introduced.

The Common FRU Access Macro (CFAM), an address space containing
various "engines" that drive accesses on busses internal and external
to the POWER chip. Examples include the SBEFIFO and I2C masters. The
engines hang off of an internal Local Bus (LBUS) which is described
by the CFAM configuration block.

[ clg: - moved object FSIScratchPad under FSICFAMState
- moved FSIScratchPad code under cfam.c
- introduced fsi_cfam_instance_init()
- reworked fsi_cfam_realize() ]



Move the list down before my S-o-b.


Signed-off-by: Andrew Jeffery 
Signed-off-by: Cédric Le Goater 
Signed-off-by: Ninad Palsule 
---
v9:
   - Added more registers to scratchpad
   - Removed unnecessary address space
   - Removed unnecessary header file
   - Defined macros for config values.
   - Cleaned up cfam config read.
---
  include/hw/fsi/cfam.h |  34 
  hw/fsi/cfam.c | 182 ++
  hw/fsi/meson.build|   2 +-
  hw/fsi/trace-events   |   5 ++
  4 files changed, 222 insertions(+), 1 deletion(-)
  create mode 100644 include/hw/fsi/cfam.h
  create mode 100644 hw/fsi/cfam.c

diff --git a/include/hw/fsi/cfam.h b/include/hw/fsi/cfam.h
new file mode 100644
index 00..bba5e3323a
--- /dev/null
+++ b/include/hw/fsi/cfam.h
@@ -0,0 +1,34 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Common FRU Access Macro
+ */
+#ifndef FSI_CFAM_H
+#define FSI_CFAM_H
+
+#include "exec/memory.h"
+
+#include "hw/fsi/fsi.h"
+#include "hw/fsi/lbus.h"
+
+#define TYPE_FSI_CFAM "cfam"
+#define FSI_CFAM(obj) OBJECT_CHECK(FSICFAMState, (obj), TYPE_FSI_CFAM)
+
+/* P9-ism */
+#define CFAM_CONFIG_NR_REGS 0x28
+
+typedef struct FSICFAMState {
+/* < private > */
+FSISlaveState parent;
+
+/* CFAM config address space */
+MemoryRegion config_iomem;
+
+MemoryRegion mr;
+
+FSILBus lbus;
+FSIScratchPad scratchpad;
+} FSICFAMState;
+
+#endif /* FSI_CFAM_H */
diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
new file mode 100644
index 00..d9ed1b532a
--- /dev/null
+++ b/hw/fsi/cfam.c
@@ -0,0 +1,182 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Common FRU Access Macro
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+
+#include "qapi/error.h"
+#include "trace.h"
+
+#include "hw/fsi/cfam.h"
+#include "hw/fsi/fsi.h"
+
+#include "hw/qdev-properties.h"
+
+#define ENGINE_CONFIG_NEXTBE_BIT(0)
+#define ENGINE_CONFIG_TYPE_PEEK   (0x02 << 4)
+#define ENGINE_CONFIG_TYPE_FSI(0x03 << 4)
+#define ENGINE_CONFIG_TYPE_SCRATCHPAD (0x06 << 4)
+
+/* Valid, slots, version, type, crc */
+#define CFAM_CONFIG_REG_PEEK   (ENGINE_CONFIG_NEXT   | \
+0x0001   | \
+0x1000   | \
+ENGINE_CONFIG_TYPE_PEEK  | \
+0x000c)
+
+/* Valid, slots, version, type, crc */
+#define CFAM_CONFIG_REG_FSI_SLAVE  (ENGINE_CONFIG_NEXT   | \
+0x0001   | \
+0x5000   | \
+ENGINE_CONFIG_TYPE_FSI   | \
+0x000a)
+
+/* Valid, slots, version, type, crc */
+#define CFAM_CONFIG_REG_SCRATCHPAD (ENGINE_CONFIG_NEXT   | \
+0x0001   | \
+0x1000   | \
+ENGINE_CONFIG_TYPE_SCRATCHPAD | \
+0x0007)


I was expecting a macro taking argument to build the config reg value
of each sub engine but that's fine also.


+#define TO_REG(x)  ((x) >> 2)
+
+#define CFAM_CONFIG_CHIP_IDTO_REG(0x00)
+#define CFAM_CONFIG_PEEK_STATUSTO_REG(0x04)
+#define CFAM_CONFIG_CHIP_ID_P9 0xc0022d15
+#define CFAM_CONFIG_CHIP_ID_BREAK  0xc0de
+
+static uint64_t fsi_cfam_config_read(void *opaque, hwaddr addr, unsigned size)
+{
+trace_fsi_cfam_config_read(addr, size);
+
+switch (addr) {
+case 0x00:
+return CFAM_CONFIG_CHIP_ID_P9;
+case 0x04:
+return CFAM_CONFIG_REG_PEEK;
+case 0x08:
+return CFAM_CONFIG_REG_FSI_SLAVE;
+case 0xc:
+return CFAM_CONFIG_REG_SCRATCHPAD;
+default:
+/*
+ * The config table contains different engines from 0xc onwards.
+ * The scratch pad is already added at address 0xc. We need to add
+ * future engines from address 0x10 onwards. Returning 0 as engine
+ * is

[PATCH v10 3/9] hw/fsi: Introduce IBM's cfam

2024-01-10 Thread Ninad Palsule 
This is a part of patchset where IBM's Flexible Service Interface is
introduced.

The Common FRU Access Macro (CFAM), an address space containing
various "engines" that drive accesses on busses internal and external
to the POWER chip. Examples include the SBEFIFO and I2C masters. The
engines hang off of an internal Local Bus (LBUS) which is described
by the CFAM configuration block.

[ clg: - moved object FSIScratchPad under FSICFAMState
   - moved FSIScratchPad code under cfam.c
   - introduced fsi_cfam_instance_init()
   - reworked fsi_cfam_realize() ]

Signed-off-by: Andrew Jeffery 
Signed-off-by: Cédric Le Goater 
Signed-off-by: Ninad Palsule 
---
v9:
  - Added more registers to scratchpad
  - Removed unnecessary address space
  - Removed unnecessary header file
  - Defined macros for config values.
  - Cleaned up cfam config read.
---
 include/hw/fsi/cfam.h |  34 
 hw/fsi/cfam.c | 182 ++
 hw/fsi/meson.build|   2 +-
 hw/fsi/trace-events   |   5 ++
 4 files changed, 222 insertions(+), 1 deletion(-)
 create mode 100644 include/hw/fsi/cfam.h
 create mode 100644 hw/fsi/cfam.c

diff --git a/include/hw/fsi/cfam.h b/include/hw/fsi/cfam.h
new file mode 100644
index 00..bba5e3323a
--- /dev/null
+++ b/include/hw/fsi/cfam.h
@@ -0,0 +1,34 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Common FRU Access Macro
+ */
+#ifndef FSI_CFAM_H
+#define FSI_CFAM_H
+
+#include "exec/memory.h"
+
+#include "hw/fsi/fsi.h"
+#include "hw/fsi/lbus.h"
+
+#define TYPE_FSI_CFAM "cfam"
+#define FSI_CFAM(obj) OBJECT_CHECK(FSICFAMState, (obj), TYPE_FSI_CFAM)
+
+/* P9-ism */
+#define CFAM_CONFIG_NR_REGS 0x28
+
+typedef struct FSICFAMState {
+/* < private > */
+FSISlaveState parent;
+
+/* CFAM config address space */
+MemoryRegion config_iomem;
+
+MemoryRegion mr;
+
+FSILBus lbus;
+FSIScratchPad scratchpad;
+} FSICFAMState;
+
+#endif /* FSI_CFAM_H */
diff --git a/hw/fsi/cfam.c b/hw/fsi/cfam.c
new file mode 100644
index 00..d9ed1b532a
--- /dev/null
+++ b/hw/fsi/cfam.c
@@ -0,0 +1,182 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Copyright (C) 2023 IBM Corp.
+ *
+ * IBM Common FRU Access Macro
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+
+#include "qapi/error.h"
+#include "trace.h"
+
+#include "hw/fsi/cfam.h"
+#include "hw/fsi/fsi.h"
+
+#include "hw/qdev-properties.h"
+
+#define ENGINE_CONFIG_NEXTBE_BIT(0)
+#define ENGINE_CONFIG_TYPE_PEEK   (0x02 << 4)
+#define ENGINE_CONFIG_TYPE_FSI(0x03 << 4)
+#define ENGINE_CONFIG_TYPE_SCRATCHPAD (0x06 << 4)
+
+/* Valid, slots, version, type, crc */
+#define CFAM_CONFIG_REG_PEEK   (ENGINE_CONFIG_NEXT   | \
+0x0001   | \
+0x1000   | \
+ENGINE_CONFIG_TYPE_PEEK  | \
+0x000c)
+
+/* Valid, slots, version, type, crc */
+#define CFAM_CONFIG_REG_FSI_SLAVE  (ENGINE_CONFIG_NEXT   | \
+0x0001   | \
+0x5000   | \
+ENGINE_CONFIG_TYPE_FSI   | \
+0x000a)
+
+/* Valid, slots, version, type, crc */
+#define CFAM_CONFIG_REG_SCRATCHPAD (ENGINE_CONFIG_NEXT   | \
+0x0001   | \
+0x1000   | \
+ENGINE_CONFIG_TYPE_SCRATCHPAD | \
+0x0007)
+
+#define TO_REG(x)  ((x) >> 2)
+
+#define CFAM_CONFIG_CHIP_IDTO_REG(0x00)
+#define CFAM_CONFIG_PEEK_STATUSTO_REG(0x04)
+#define CFAM_CONFIG_CHIP_ID_P9 0xc0022d15
+#define CFAM_CONFIG_CHIP_ID_BREAK  0xc0de
+
+static uint64_t fsi_cfam_config_read(void *opaque, hwaddr addr, unsigned size)
+{
+trace_fsi_cfam_config_read(addr, size);
+
+switch (addr) {
+case 0x00:
+return CFAM_CONFIG_CHIP_ID_P9;
+case 0x04:
+return CFAM_CONFIG_REG_PEEK;
+case 0x08:
+return CFAM_CONFIG_REG_FSI_SLAVE;
+case 0xc:
+return CFAM_CONFIG_REG_SCRATCHPAD;
+default:
+/*
+ * The config table contains different engines from 0xc onwards.
+ * The scratch pad is already added at address 0xc. We need to add
+ * future engines from address 0x10 onwards. Returning 0 as engine
+ * is not implemented.
+ */
+return 0;
+}
+}
+
+static void fsi_cfam_config_write(void *opaque, hwaddr addr, uint64_t data,
+  unsigned size)
+{
+FSICFAMState