Re: [PATCH for-7.1 08/11] hw/net: Add NPCM8XX PCS Module

[Peter Maydell]

On Tue, 5 Apr 2022 at 23:38, Hao Wu  wrote:
>
> The PCS exists in NPCM8XX's GMAC1 and is used to control the SGMII
> PHY. This implementation contains all the default registers and
> the soft reset feature that are required to load the Linux kernel
> driver. Further features have not been implemented yet.
>
> Signed-off-by: Hao Wu 
> Reviewed-by: Titus Rwantare 

> +static uint16_t npcm_pcs_read_sr_ctl(NPCMPCSState *s, hwaddr offset)
> +{
> +hwaddr regno = offset / sizeof(uint16_t);
> +
> +if (regno >= NPCM_PCS_NR_SR_CTLS) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "%s: SR_CTL read offset 0x%04" HWADDR_PRIx
> +  " is out of range.",

qemu_log_mask strings need to have a trailing "\n" (here and below)


> +static uint64_t npcm_pcs_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +NPCMPCSState *s = opaque;
> +uint16_t v = 0;
> +
> +if (offset == NPCM_PCS_IND_AC_BA) {
> +v = s->indirect_access_base;
> +} else {
> +switch (s->indirect_access_base) {
> +case NPCM_PCS_IND_SR_CTL:
> +v = npcm_pcs_read_sr_ctl(s, offset);
> +break;
> +
> +case NPCM_PCS_IND_SR_MII:
> +v = npcm_pcs_read_sr_mii(s, offset);
> +break;
> +
> +case NPCM_PCS_IND_SR_TIM:
> +v = npcm_pcs_read_sr_tim(s, offset);
> +break;
> +
> +case NPCM_PCS_IND_VR_MII:
> +v = npcm_pcs_read_vr_mii(s, offset);
> +break;
> +
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR,
> +  "%s: Read with invalid indirect address base: 
> 0x%02"

Why are you specifying a width of 2 here? The valid values are definitely
larger than 2 digits wide, so presumably invalid values probably are as well...

> +  PRIx16 "\n", DEVICE(s)->canonical_path,
> +  s->indirect_access_base);
> +}
> +}
> +
> +trace_npcm_pcs_reg_read(DEVICE(s)->canonical_path, 
> s->indirect_access_base,
> +offset, v);
> +return v;
> +}

> +static void npcm_pcs_class_init(ObjectClass *klass, void *data)
> +{
> +DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);

Not sure this is the right category -- this doesn't seem to
be a network device in the usual sense.


> +# npcm_pcs.c
> +npcm_pcs_reg_read(const char *name, uint16_t indirect_access_baes, uint64_t 
> offset, uint16_t value) "%s: IND: 0x%02" PRIx16 " offset: 0x%04" PRIx64 " 
> value: 0x%04" PRIx16
> +npcm_pcs_reg_write(const char *name, uint16_t indirect_access_baes, uint64_t 
> offset, uint16_t value) "%s: IND: 0x%02" PRIx16 " offset: 0x%04" PRIx64 " 
> value: 0x%04" PRIx16

Typo : should be "_base" I assume.


> +
>  # dp8398x.c
>  dp8393x_raise_irq(int isr) "raise irq, isr is 0x%04x"
>  dp8393x_lower_irq(void) "lower irq"
> diff --git a/include/hw/net/npcm_pcs.h b/include/hw/net/npcm_pcs.h
> new file mode 100644
> index 00..bd4f71bf3c
> --- /dev/null
> +++ b/include/hw/net/npcm_pcs.h
> @@ -0,0 +1,42 @@
> +/*
> + * Nuvoton NPCM8xx PCS Module
> + *
> + * Copyright 2022 Google LLC
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but 
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +
> +#ifndef NPCM_PCS_H
> +#define NPCM_PCS_H
> +
> +#include "hw/sysbus.h"
> +
> +#define NPCM_PCS_NR_SR_CTLS (0x12 / sizeof(uint16_t))
> +#define NPCM_PCS_NR_SR_MIIS (0x20 / sizeof(uint16_t))
> +#define NPCM_PCS_NR_SR_TIMS (0x22 / sizeof(uint16_t))
> +#define NPCM_PCS_NR_VR_MIIS (0x1c6 / sizeof(uint16_t))
> +
> +typedef struct NPCMPCSState {
> +SysBusDevice parent;
> +
> +MemoryRegion iomem;
> +
> +uint16_t indirect_access_base;
> +uint16_t sr_ctl[NPCM_PCS_NR_SR_CTLS];
> +uint16_t sr_mii[NPCM_PCS_NR_SR_MIIS];
> +uint16_t sr_tim[NPCM_PCS_NR_SR_TIMS];
> +uint16_t vr_mii[NPCM_PCS_NR_VR_MIIS];
> +} NPCMPCSState;
> +
> +#define TYPE_NPCM_PCS "npcm-pcs"
> +OBJECT_DECLARE_SIMPLE_TYPE(NPCMPCSState, NPCM_PCS)

OBJECT_DECLARE_SIMPLE_TYPE does the 'typedef' for you, so you
don't need to do that in your struct definition if you're using that
macro.

thanks
-- PMM

[PATCH for-7.1 08/11] hw/net: Add NPCM8XX PCS Module

[Hao Wu]

The PCS exists in NPCM8XX's GMAC1 and is used to control the SGMII
PHY. This implementation contains all the default registers and
the soft reset feature that are required to load the Linux kernel
driver. Further features have not been implemented yet.

Signed-off-by: Hao Wu 
Reviewed-by: Titus Rwantare 
---
 hw/net/meson.build|   1 +
 hw/net/npcm_pcs.c | 409 ++
 hw/net/trace-events   |   4 +
 include/hw/net/npcm_pcs.h |  42 
 4 files changed, 456 insertions(+)
 create mode 100644 hw/net/npcm_pcs.c
 create mode 100644 include/hw/net/npcm_pcs.h

diff --git a/hw/net/meson.build b/hw/net/meson.build
index 685b75badb..4cba3e66db 100644
--- a/hw/net/meson.build
+++ b/hw/net/meson.build
@@ -37,6 +37,7 @@ softmmu_ss.add(when: 'CONFIG_SUNHME', if_true: 
files('sunhme.c'))
 softmmu_ss.add(when: 'CONFIG_FTGMAC100', if_true: files('ftgmac100.c'))
 softmmu_ss.add(when: 'CONFIG_SUNGEM', if_true: files('sungem.c'))
 softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_emc.c'))
+softmmu_ss.add(when: 'CONFIG_NPCM8XX', if_true: files('npcm_pcs.c'))
 
 softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_eth.c'))
 softmmu_ss.add(when: 'CONFIG_COLDFIRE', if_true: files('mcf_fec.c'))
diff --git a/hw/net/npcm_pcs.c b/hw/net/npcm_pcs.c
new file mode 100644
index 00..efe5f68d9c
--- /dev/null
+++ b/hw/net/npcm_pcs.c
@@ -0,0 +1,409 @@
+/*
+ * Nuvoton NPCM8xx PCS Module
+ *
+ * Copyright 2022 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+/*
+ * Disclaimer:
+ * Currently we only implemented the default values of the registers and
+ * the soft reset feature. These are required to boot up the GMAC module
+ * in Linux kernel for NPCM845 boards. Other functionalities are not modeled.
+ */
+
+#include "qemu/osdep.h"
+
+#include "exec/hwaddr.h"
+#include "hw/registerfields.h"
+#include "hw/net/npcm_pcs.h"
+#include "migration/vmstate.h"
+#include "qemu/log.h"
+#include "qemu/units.h"
+#include "trace.h"
+
+#define NPCM_PCS_IND_AC_BA  0x1fe
+#define NPCM_PCS_IND_SR_CTL 0x1e00
+#define NPCM_PCS_IND_SR_MII 0x1f00
+#define NPCM_PCS_IND_SR_TIM 0x1f07
+#define NPCM_PCS_IND_VR_MII 0x1f80
+
+REG16(NPCM_PCS_SR_CTL_ID1, 0x08)
+REG16(NPCM_PCS_SR_CTL_ID2, 0x0a)
+REG16(NPCM_PCS_SR_CTL_STS, 0x10)
+
+REG16(NPCM_PCS_SR_MII_CTRL, 0x00)
+REG16(NPCM_PCS_SR_MII_STS, 0x02)
+REG16(NPCM_PCS_SR_MII_DEV_ID1, 0x04)
+REG16(NPCM_PCS_SR_MII_DEV_ID2, 0x06)
+REG16(NPCM_PCS_SR_MII_AN_ADV, 0x08)
+REG16(NPCM_PCS_SR_MII_LP_BABL, 0x0a)
+REG16(NPCM_PCS_SR_MII_AN_EXPN, 0x0c)
+REG16(NPCM_PCS_SR_MII_EXT_STS, 0x1e)
+
+REG16(NPCM_PCS_SR_TIM_SYNC_ABL, 0x10)
+REG16(NPCM_PCS_SR_TIM_SYNC_TX_MAX_DLY_LWR, 0x12)
+REG16(NPCM_PCS_SR_TIM_SYNC_TX_MAX_DLY_UPR, 0x14)
+REG16(NPCM_PCS_SR_TIM_SYNC_TX_MIN_DLY_LWR, 0x16)
+REG16(NPCM_PCS_SR_TIM_SYNC_TX_MIN_DLY_UPR, 0x18)
+REG16(NPCM_PCS_SR_TIM_SYNC_RX_MAX_DLY_LWR, 0x1a)
+REG16(NPCM_PCS_SR_TIM_SYNC_RX_MAX_DLY_UPR, 0x1c)
+REG16(NPCM_PCS_SR_TIM_SYNC_RX_MIN_DLY_LWR, 0x1e)
+REG16(NPCM_PCS_SR_TIM_SYNC_RX_MIN_DLY_UPR, 0x20)
+
+REG16(NPCM_PCS_VR_MII_MMD_DIG_CTRL1, 0x000)
+REG16(NPCM_PCS_VR_MII_AN_CTRL, 0x002)
+REG16(NPCM_PCS_VR_MII_AN_INTR_STS, 0x004)
+REG16(NPCM_PCS_VR_MII_TC, 0x006)
+REG16(NPCM_PCS_VR_MII_DBG_CTRL, 0x00a)
+REG16(NPCM_PCS_VR_MII_EEE_MCTRL0, 0x00c)
+REG16(NPCM_PCS_VR_MII_EEE_TXTIMER, 0x010)
+REG16(NPCM_PCS_VR_MII_EEE_RXTIMER, 0x012)
+REG16(NPCM_PCS_VR_MII_LINK_TIMER_CTRL, 0x014)
+REG16(NPCM_PCS_VR_MII_EEE_MCTRL1, 0x016)
+REG16(NPCM_PCS_VR_MII_DIG_STS, 0x020)
+REG16(NPCM_PCS_VR_MII_ICG_ERRCNT1, 0x022)
+REG16(NPCM_PCS_VR_MII_MISC_STS, 0x030)
+REG16(NPCM_PCS_VR_MII_RX_LSTS, 0x040)
+REG16(NPCM_PCS_VR_MII_MP_TX_BSTCTRL0, 0x070)
+REG16(NPCM_PCS_VR_MII_MP_TX_LVLCTRL0, 0x074)
+REG16(NPCM_PCS_VR_MII_MP_TX_GENCTRL0, 0x07a)
+REG16(NPCM_PCS_VR_MII_MP_TX_GENCTRL1, 0x07c)
+REG16(NPCM_PCS_VR_MII_MP_TX_STS, 0x090)
+REG16(NPCM_PCS_VR_MII_MP_RX_GENCTRL0, 0x0b0)
+REG16(NPCM_PCS_VR_MII_MP_RX_GENCTRL1, 0x0b2)
+REG16(NPCM_PCS_VR_MII_MP_RX_LOS_CTRL0, 0x0ba)
+REG16(NPCM_PCS_VR_MII_MP_MPLL_CTRL0, 0x0f0)
+REG16(NPCM_PCS_VR_MII_MP_MPLL_CTRL1, 0x0f2)
+REG16(NPCM_PCS_VR_MII_MP_MPLL_STS, 0x110)
+REG16(NPCM_PCS_VR_MII_MP_MISC_CTRL2, 0x126)
+REG16(NPCM_PCS_VR_MII_MP_LVL_CTRL, 0x130)
+REG16(NPCM_PCS_VR_MII_MP_MISC_CTRL0, 0x132)
+REG16(NPCM_PCS_VR_MII_MP_MISC_CTRL1, 0x134)
+REG16(NPCM_PCS_VR_MII_DIG_CTRL2, 0x1c2)
+REG16(NPCM_PCS_VR_MII_DIG_ERRCNT_SEL, 0x1c4)
+
+/* Register Fields */
+#define NPCM_PCS_SR_MII_CTRL_RSTBIT(15)
+
+static const uint16_t