Re: [RFC v2 01/10] fsl_bman: Add drivers for the Freescale DPAA BMan
Hello Kumar, Thanks for taking the time to review this On 02/18/2015 11:43 AM, Kumar Gala wrote: > On Feb 16, 2015, at 9:46 AM, Emil Medve wrote: > >> From: Geoff Thorpe >> >> Change-Id: I075944acf740dbaae861104c17a9ff7247dec1be >> Signed-off-by: Geoff Thorpe >> --- >> drivers/soc/Kconfig |1 + >> drivers/soc/Makefile |1 + >> drivers/soc/freescale/Kconfig| 51 ++ >> drivers/soc/freescale/Makefile |7 + >> drivers/soc/freescale/bman.c | 611 >> drivers/soc/freescale/bman.h | 524 + >> drivers/soc/freescale/bman_api.c | 1033 >> ++ >> drivers/soc/freescale/bman_portal.c | 330 +++ >> drivers/soc/freescale/bman_priv.h| 149 + >> drivers/soc/freescale/dpaa_alloc.c | 404 + >> drivers/soc/freescale/dpaa_sys.h | 235 >> drivers/soc/freescale/qbman_driver.c | 41 ++ >> include/linux/fsl_bman.h | 511 + > > If you are using drivers/soc than the include should probably be > include/soc/freescale/ Will move/rename > Also, any reason you guys aren’t using drivers/soc/fsl ? Will rename Cheers, -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 01/10] fsl_bman: Add drivers for the Freescale DPAA BMan
On Feb 16, 2015, at 9:46 AM, Emil Medve wrote: > From: Geoff Thorpe > > Change-Id: I075944acf740dbaae861104c17a9ff7247dec1be > Signed-off-by: Geoff Thorpe > --- > drivers/soc/Kconfig |1 + > drivers/soc/Makefile |1 + > drivers/soc/freescale/Kconfig| 51 ++ > drivers/soc/freescale/Makefile |7 + > drivers/soc/freescale/bman.c | 611 > drivers/soc/freescale/bman.h | 524 + > drivers/soc/freescale/bman_api.c | 1033 ++ > drivers/soc/freescale/bman_portal.c | 330 +++ > drivers/soc/freescale/bman_priv.h| 149 + > drivers/soc/freescale/dpaa_alloc.c | 404 + > drivers/soc/freescale/dpaa_sys.h | 235 > drivers/soc/freescale/qbman_driver.c | 41 ++ > include/linux/fsl_bman.h | 511 + If you are using drivers/soc than the include should probably be include/soc/freescale/ Also, any reason you guys aren’t using drivers/soc/fsl ? - k-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 01/10] fsl_bman: Add drivers for the Freescale DPAA BMan
Hello Kumar, Thanks for taking the time to review this On 02/18/2015 11:43 AM, Kumar Gala wrote: On Feb 16, 2015, at 9:46 AM, Emil Medve emilian.me...@freescale.com wrote: From: Geoff Thorpe geoff.tho...@freescale.com Change-Id: I075944acf740dbaae861104c17a9ff7247dec1be Signed-off-by: Geoff Thorpe geoff.tho...@freescale.com --- drivers/soc/Kconfig |1 + drivers/soc/Makefile |1 + drivers/soc/freescale/Kconfig| 51 ++ drivers/soc/freescale/Makefile |7 + drivers/soc/freescale/bman.c | 611 drivers/soc/freescale/bman.h | 524 + drivers/soc/freescale/bman_api.c | 1033 ++ drivers/soc/freescale/bman_portal.c | 330 +++ drivers/soc/freescale/bman_priv.h| 149 + drivers/soc/freescale/dpaa_alloc.c | 404 + drivers/soc/freescale/dpaa_sys.h | 235 drivers/soc/freescale/qbman_driver.c | 41 ++ include/linux/fsl_bman.h | 511 + If you are using drivers/soc than the include should probably be include/soc/freescale/ Will move/rename Also, any reason you guys aren’t using drivers/soc/fsl ? Will rename Cheers, -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 01/10] fsl_bman: Add drivers for the Freescale DPAA BMan
On Feb 16, 2015, at 9:46 AM, Emil Medve emilian.me...@freescale.com wrote: From: Geoff Thorpe geoff.tho...@freescale.com Change-Id: I075944acf740dbaae861104c17a9ff7247dec1be Signed-off-by: Geoff Thorpe geoff.tho...@freescale.com --- drivers/soc/Kconfig |1 + drivers/soc/Makefile |1 + drivers/soc/freescale/Kconfig| 51 ++ drivers/soc/freescale/Makefile |7 + drivers/soc/freescale/bman.c | 611 drivers/soc/freescale/bman.h | 524 + drivers/soc/freescale/bman_api.c | 1033 ++ drivers/soc/freescale/bman_portal.c | 330 +++ drivers/soc/freescale/bman_priv.h| 149 + drivers/soc/freescale/dpaa_alloc.c | 404 + drivers/soc/freescale/dpaa_sys.h | 235 drivers/soc/freescale/qbman_driver.c | 41 ++ include/linux/fsl_bman.h | 511 + If you are using drivers/soc than the include should probably be include/soc/freescale/ Also, any reason you guys aren’t using drivers/soc/fsl ? - k-- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v2 01/10] fsl_bman: Add drivers for the Freescale DPAA BMan
On Mon, 2015-02-16 at 09:46 -0600, Emil Medve wrote:
> From: Geoff Thorpe
>
> Change-Id: I075944acf740dbaae861104c17a9ff7247dec1be
> Signed-off-by: Geoff Thorpe
Remove Change-Id.
Provide a description of what BMan is.
> diff --git a/drivers/soc/freescale/Kconfig b/drivers/soc/freescale/Kconfig
> new file mode 100644
> index 000..63bf002
> --- /dev/null
> +++ b/drivers/soc/freescale/Kconfig
> @@ -0,0 +1,51 @@
> +menuconfig FSL_DPA
> + tristate "Freescale DPAA Buffer management"
> + depends on HAS_FSL_QBMAN
> + default y
"default y" is normally a bad idea for drivers (though it can make sense
for options within a driver).
Where is HAS_FSL_QBMAN defined (if the answer is in a later patch,
rearrange it)?
> +if FSL_DPA
> +
> +config FSL_DPA_CHECKING
> + bool "additional driver checking"
Option texts are usually capitalized.
> + default n
> + ---help---
> + Compiles in additional checks to sanity-check the drivers and any
> + use of it by other code. Not recommended for performance.
> +
> +config FSL_DPA_CAN_WAIT
> + bool
> + default y
> +config FSL_DPA_CAN_WAIT_SYNC
> + bool
> + default y
> +
> +config FSL_DPA_PIRQ_FAST
> + bool
> + default y
Use consistent whitespace.
Add help texts to these options to document them, even if they're not
user-visible.
> +config FSL_BMAN_CONFIG
> + bool "BMan device management"
> + default y
> + ---help---
> + If this linux image is running natively, you need this option. If this
> + linux image is running as a guest OS under the hypervisor, only one
> + guest OS ("the control plane") needs this option.
"the hypervisor"? I suspect this refers to the Freescale Embedded
Hypervisor (a.k.a. Topaz), rather than KVM or something else.
It would also be nice if the help text explained what the option does,
rather than just when it's needed.
> +struct bman;
Where is this struct defined?
> +union bman_ecir {
> + u32 ecir_raw;
> + struct {
> + u32 __reserved1:4;
> + u32 portal_num:4;
> + u32 __reserved2:12;
> + u32 numb:4;
> + u32 __reserved3:2;
> + u32 pid:6;
> + } __packed info;
> +};
Get rid of __packed. It causes GCC to generate terrible code with
bitfields. For that matter, consider getting rid of the bitfields.
> +#define BMAN_HWE_TXT(a, b) { .mask = BM_EIRQ_##a, .txt = b }
> +
> +static const struct bman_hwerr_txt bman_hwerr_txts[] = {
> + BMAN_HWE_TXT(IVCI, "Invalid Command Verb"),
> + BMAN_HWE_TXT(FLWI, "FBPR Low Watermark"),
> + BMAN_HWE_TXT(MBEI, "Multi-bit ECC Error"),
> + BMAN_HWE_TXT(SBEI, "Single-bit ECC Error"),
> + BMAN_HWE_TXT(BSCN, "Pool State Change Notification"),
> +};
> +#define BMAN_HWE_COUNT (sizeof(bman_hwerr_txts)/sizeof(struct
> bman_hwerr_txt))
Use ARRAY_SIZE(), here and elsewhere.
> +/* Add this in Kconfig */
> +#define BMAN_ERRS_TO_UNENABLE (BM_EIRQ_FLWI)
Add what to kconfig?
> +/**
> + * bm_err_isr__ - Manipulate global interrupt registers
> + * @v: for accessors that write values, this is the 32-bit value
> + *
> + * Manipulates BMAN_ERR_ISR, BMAN_ERR_IER, BMAN_ERR_ISDR, BMAN_ERR_IIR. All
> + * manipulations except bm_err_isr_[un]inhibit() use 32-bit masks composed of
> + * the BM_EIRQ_*** definitions. Note that "bm_err_isr_enable_write" means
> + * "write the enable register" rather than "enable the write register"!
> + */
> +#define bm_err_isr_status_read(bm) \
> + __bm_err_isr_read(bm, bm_isr_status)
> +#define bm_err_isr_status_clear(bm, m) \
> + __bm_err_isr_write(bm, bm_isr_status, m)
> +#define bm_err_isr_enable_read(bm) \
> + __bm_err_isr_read(bm, bm_isr_enable)
> +#define bm_err_isr_enable_write(bm, v) \
> + __bm_err_isr_write(bm, bm_isr_enable, v)
> +#define bm_err_isr_disable_read(bm) \
> + __bm_err_isr_read(bm, bm_isr_disable)
> +#define bm_err_isr_disable_write(bm, v) \
> + __bm_err_isr_write(bm, bm_isr_disable, v)
> +#define bm_err_isr_inhibit(bm) \
> + __bm_err_isr_write(bm, bm_isr_inhibit, 1)
> +#define bm_err_isr_uninhibit(bm) \
> + __bm_err_isr_write(bm, bm_isr_inhibit, 0)
Is this layer really helpful?
> +/*
> + * TODO: unimplemented registers
> + *
> + * BMAN_POOLk_SDCNT, BMAN_POOLk_HDCNT, BMAN_FULT,
> + * BMAN_VLDPL, BMAN_EECC, BMAN_SBET, BMAN_EINJ
> + */
What does it mean for registers to be unimplemented, in a piece of
software? If you mean accessors to those registers, why is that needed
if nothing uses them (yet)?
> +/* Encapsulate "struct bman *" as a cast of the register space address. */
> +
> +static struct bman *bm_create(void *regs)
> +{
> + return (struct bman *)regs;
> +}
Unnecessary cast -- and unnecessary encapsulation (especially since you
only use it once). It's also missing __iomem -- have you run sparse on
this?
> +static u32 __generate_thresh(u32 val,
[RFC v2 01/10] fsl_bman: Add drivers for the Freescale DPAA BMan
From: Geoff Thorpe
Change-Id: I075944acf740dbaae861104c17a9ff7247dec1be
Signed-off-by: Geoff Thorpe
---
drivers/soc/Kconfig |1 +
drivers/soc/Makefile |1 +
drivers/soc/freescale/Kconfig| 51 ++
drivers/soc/freescale/Makefile |7 +
drivers/soc/freescale/bman.c | 611
drivers/soc/freescale/bman.h | 524 +
drivers/soc/freescale/bman_api.c | 1033 ++
drivers/soc/freescale/bman_portal.c | 330 +++
drivers/soc/freescale/bman_priv.h| 149 +
drivers/soc/freescale/dpaa_alloc.c | 404 +
drivers/soc/freescale/dpaa_sys.h | 235
drivers/soc/freescale/qbman_driver.c | 41 ++
include/linux/fsl_bman.h | 511 +
13 files changed, 3898 insertions(+)
create mode 100644 drivers/soc/freescale/Kconfig
create mode 100644 drivers/soc/freescale/Makefile
create mode 100644 drivers/soc/freescale/bman.c
create mode 100644 drivers/soc/freescale/bman.h
create mode 100644 drivers/soc/freescale/bman_api.c
create mode 100644 drivers/soc/freescale/bman_portal.c
create mode 100644 drivers/soc/freescale/bman_priv.h
create mode 100644 drivers/soc/freescale/dpaa_alloc.c
create mode 100644 drivers/soc/freescale/dpaa_sys.h
create mode 100644 drivers/soc/freescale/qbman_driver.c
create mode 100644 include/linux/fsl_bman.h
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 76d6bd4..6534d8b 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -1,5 +1,6 @@
menu "SOC (System On Chip) specific Drivers"
+source "drivers/soc/freescale/Kconfig"
source "drivers/soc/qcom/Kconfig"
source "drivers/soc/ti/Kconfig"
source "drivers/soc/versatile/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 063113d..a63cc17 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -2,6 +2,7 @@
# Makefile for the Linux Kernel SOC specific device drivers.
#
+obj-$(CONFIG_FSL_DPA) += freescale/
obj-$(CONFIG_ARCH_QCOM)+= qcom/
obj-$(CONFIG_ARCH_TEGRA) += tegra/
obj-$(CONFIG_SOC_TI) += ti/
diff --git a/drivers/soc/freescale/Kconfig b/drivers/soc/freescale/Kconfig
new file mode 100644
index 000..63bf002
--- /dev/null
+++ b/drivers/soc/freescale/Kconfig
@@ -0,0 +1,51 @@
+menuconfig FSL_DPA
+ tristate "Freescale DPAA Buffer management"
+ depends on HAS_FSL_QBMAN
+ default y
+
+if FSL_DPA
+
+config FSL_DPA_CHECKING
+ bool "additional driver checking"
+ default n
+ ---help---
+ Compiles in additional checks to sanity-check the drivers and any
+ use of it by other code. Not recommended for performance.
+
+config FSL_DPA_CAN_WAIT
+ bool
+ default y
+
+config FSL_DPA_CAN_WAIT_SYNC
+ bool
+ default y
+
+config FSL_DPA_PIRQ_FAST
+ bool
+ default y
+
+config FSL_DPA_PIRQ_SLOW
+ bool
+ default y
+
+config FSL_DPA_PORTAL_SHARE
+ bool
+ default y
+
+config FSL_BMAN
+ bool "Freescale Buffer Manager (BMan) support"
+ default y
+
+if FSL_BMAN
+
+config FSL_BMAN_CONFIG
+ bool "BMan device management"
+ default y
+ ---help---
+ If this linux image is running natively, you need this option. If this
+ linux image is running as a guest OS under the hypervisor, only one
+ guest OS ("the control plane") needs this option.
+
+endif # FSL_BMAN
+
+endif # FSL_DPA
diff --git a/drivers/soc/freescale/Makefile b/drivers/soc/freescale/Makefile
new file mode 100644
index 000..3f452ee
--- /dev/null
+++ b/drivers/soc/freescale/Makefile
@@ -0,0 +1,7 @@
+# Common
+obj-$(CONFIG_FSL_DPA) += dpaa_alloc.o
+obj-$(CONFIG_HAS_FSL_QBMAN)+= qbman_driver.o
+
+# Bman
+obj-$(CONFIG_FSL_BMAN) += bman_api.o
+obj-$(CONFIG_FSL_BMAN_CONFIG) += bman.o bman_portal.o
diff --git a/drivers/soc/freescale/bman.c b/drivers/soc/freescale/bman.c
new file mode 100644
index 000..fba6ae0
--- /dev/null
+++ b/drivers/soc/freescale/bman.c
@@ -0,0 +1,611 @@
+/* Copyright (c) 2009-2012 Freescale Semiconductor, Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ * names of its contributors may be used to endorse or promote products
+ * derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed
[RFC v2 01/10] fsl_bman: Add drivers for the Freescale DPAA BMan
From: Geoff Thorpe geoff.tho...@freescale.com Change-Id: I075944acf740dbaae861104c17a9ff7247dec1be Signed-off-by: Geoff Thorpe geoff.tho...@freescale.com --- drivers/soc/Kconfig |1 + drivers/soc/Makefile |1 + drivers/soc/freescale/Kconfig| 51 ++ drivers/soc/freescale/Makefile |7 + drivers/soc/freescale/bman.c | 611 drivers/soc/freescale/bman.h | 524 + drivers/soc/freescale/bman_api.c | 1033 ++ drivers/soc/freescale/bman_portal.c | 330 +++ drivers/soc/freescale/bman_priv.h| 149 + drivers/soc/freescale/dpaa_alloc.c | 404 + drivers/soc/freescale/dpaa_sys.h | 235 drivers/soc/freescale/qbman_driver.c | 41 ++ include/linux/fsl_bman.h | 511 + 13 files changed, 3898 insertions(+) create mode 100644 drivers/soc/freescale/Kconfig create mode 100644 drivers/soc/freescale/Makefile create mode 100644 drivers/soc/freescale/bman.c create mode 100644 drivers/soc/freescale/bman.h create mode 100644 drivers/soc/freescale/bman_api.c create mode 100644 drivers/soc/freescale/bman_portal.c create mode 100644 drivers/soc/freescale/bman_priv.h create mode 100644 drivers/soc/freescale/dpaa_alloc.c create mode 100644 drivers/soc/freescale/dpaa_sys.h create mode 100644 drivers/soc/freescale/qbman_driver.c create mode 100644 include/linux/fsl_bman.h diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig index 76d6bd4..6534d8b 100644 --- a/drivers/soc/Kconfig +++ b/drivers/soc/Kconfig @@ -1,5 +1,6 @@ menu SOC (System On Chip) specific Drivers +source drivers/soc/freescale/Kconfig source drivers/soc/qcom/Kconfig source drivers/soc/ti/Kconfig source drivers/soc/versatile/Kconfig diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile index 063113d..a63cc17 100644 --- a/drivers/soc/Makefile +++ b/drivers/soc/Makefile @@ -2,6 +2,7 @@ # Makefile for the Linux Kernel SOC specific device drivers. # +obj-$(CONFIG_FSL_DPA) += freescale/ obj-$(CONFIG_ARCH_QCOM)+= qcom/ obj-$(CONFIG_ARCH_TEGRA) += tegra/ obj-$(CONFIG_SOC_TI) += ti/ diff --git a/drivers/soc/freescale/Kconfig b/drivers/soc/freescale/Kconfig new file mode 100644 index 000..63bf002 --- /dev/null +++ b/drivers/soc/freescale/Kconfig @@ -0,0 +1,51 @@ +menuconfig FSL_DPA + tristate Freescale DPAA Buffer management + depends on HAS_FSL_QBMAN + default y + +if FSL_DPA + +config FSL_DPA_CHECKING + bool additional driver checking + default n + ---help--- + Compiles in additional checks to sanity-check the drivers and any + use of it by other code. Not recommended for performance. + +config FSL_DPA_CAN_WAIT + bool + default y + +config FSL_DPA_CAN_WAIT_SYNC + bool + default y + +config FSL_DPA_PIRQ_FAST + bool + default y + +config FSL_DPA_PIRQ_SLOW + bool + default y + +config FSL_DPA_PORTAL_SHARE + bool + default y + +config FSL_BMAN + bool Freescale Buffer Manager (BMan) support + default y + +if FSL_BMAN + +config FSL_BMAN_CONFIG + bool BMan device management + default y + ---help--- + If this linux image is running natively, you need this option. If this + linux image is running as a guest OS under the hypervisor, only one + guest OS (the control plane) needs this option. + +endif # FSL_BMAN + +endif # FSL_DPA diff --git a/drivers/soc/freescale/Makefile b/drivers/soc/freescale/Makefile new file mode 100644 index 000..3f452ee --- /dev/null +++ b/drivers/soc/freescale/Makefile @@ -0,0 +1,7 @@ +# Common +obj-$(CONFIG_FSL_DPA) += dpaa_alloc.o +obj-$(CONFIG_HAS_FSL_QBMAN)+= qbman_driver.o + +# Bman +obj-$(CONFIG_FSL_BMAN) += bman_api.o +obj-$(CONFIG_FSL_BMAN_CONFIG) += bman.o bman_portal.o diff --git a/drivers/soc/freescale/bman.c b/drivers/soc/freescale/bman.c new file mode 100644 index 000..fba6ae0 --- /dev/null +++ b/drivers/soc/freescale/bman.c @@ -0,0 +1,611 @@ +/* Copyright (c) 2009-2012 Freescale Semiconductor, Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Freescale Semiconductor nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * + * ALTERNATIVELY,
Re: [RFC v2 01/10] fsl_bman: Add drivers for the Freescale DPAA BMan
On Mon, 2015-02-16 at 09:46 -0600, Emil Medve wrote:
From: Geoff Thorpe geoff.tho...@freescale.com
Change-Id: I075944acf740dbaae861104c17a9ff7247dec1be
Signed-off-by: Geoff Thorpe geoff.tho...@freescale.com
Remove Change-Id.
Provide a description of what BMan is.
diff --git a/drivers/soc/freescale/Kconfig b/drivers/soc/freescale/Kconfig
new file mode 100644
index 000..63bf002
--- /dev/null
+++ b/drivers/soc/freescale/Kconfig
@@ -0,0 +1,51 @@
+menuconfig FSL_DPA
+ tristate Freescale DPAA Buffer management
+ depends on HAS_FSL_QBMAN
+ default y
default y is normally a bad idea for drivers (though it can make sense
for options within a driver).
Where is HAS_FSL_QBMAN defined (if the answer is in a later patch,
rearrange it)?
+if FSL_DPA
+
+config FSL_DPA_CHECKING
+ bool additional driver checking
Option texts are usually capitalized.
+ default n
+ ---help---
+ Compiles in additional checks to sanity-check the drivers and any
+ use of it by other code. Not recommended for performance.
+
+config FSL_DPA_CAN_WAIT
+ bool
+ default y
+config FSL_DPA_CAN_WAIT_SYNC
+ bool
+ default y
+
+config FSL_DPA_PIRQ_FAST
+ bool
+ default y
Use consistent whitespace.
Add help texts to these options to document them, even if they're not
user-visible.
+config FSL_BMAN_CONFIG
+ bool BMan device management
+ default y
+ ---help---
+ If this linux image is running natively, you need this option. If this
+ linux image is running as a guest OS under the hypervisor, only one
+ guest OS (the control plane) needs this option.
the hypervisor? I suspect this refers to the Freescale Embedded
Hypervisor (a.k.a. Topaz), rather than KVM or something else.
It would also be nice if the help text explained what the option does,
rather than just when it's needed.
+struct bman;
Where is this struct defined?
+union bman_ecir {
+ u32 ecir_raw;
+ struct {
+ u32 __reserved1:4;
+ u32 portal_num:4;
+ u32 __reserved2:12;
+ u32 numb:4;
+ u32 __reserved3:2;
+ u32 pid:6;
+ } __packed info;
+};
Get rid of __packed. It causes GCC to generate terrible code with
bitfields. For that matter, consider getting rid of the bitfields.
+#define BMAN_HWE_TXT(a, b) { .mask = BM_EIRQ_##a, .txt = b }
+
+static const struct bman_hwerr_txt bman_hwerr_txts[] = {
+ BMAN_HWE_TXT(IVCI, Invalid Command Verb),
+ BMAN_HWE_TXT(FLWI, FBPR Low Watermark),
+ BMAN_HWE_TXT(MBEI, Multi-bit ECC Error),
+ BMAN_HWE_TXT(SBEI, Single-bit ECC Error),
+ BMAN_HWE_TXT(BSCN, Pool State Change Notification),
+};
+#define BMAN_HWE_COUNT (sizeof(bman_hwerr_txts)/sizeof(struct
bman_hwerr_txt))
Use ARRAY_SIZE(), here and elsewhere.
+/* Add this in Kconfig */
+#define BMAN_ERRS_TO_UNENABLE (BM_EIRQ_FLWI)
Add what to kconfig?
+/**
+ * bm_err_isr_reg_verb - Manipulate global interrupt registers
+ * @v: for accessors that write values, this is the 32-bit value
+ *
+ * Manipulates BMAN_ERR_ISR, BMAN_ERR_IER, BMAN_ERR_ISDR, BMAN_ERR_IIR. All
+ * manipulations except bm_err_isr_[un]inhibit() use 32-bit masks composed of
+ * the BM_EIRQ_*** definitions. Note that bm_err_isr_enable_write means
+ * write the enable register rather than enable the write register!
+ */
+#define bm_err_isr_status_read(bm) \
+ __bm_err_isr_read(bm, bm_isr_status)
+#define bm_err_isr_status_clear(bm, m) \
+ __bm_err_isr_write(bm, bm_isr_status, m)
+#define bm_err_isr_enable_read(bm) \
+ __bm_err_isr_read(bm, bm_isr_enable)
+#define bm_err_isr_enable_write(bm, v) \
+ __bm_err_isr_write(bm, bm_isr_enable, v)
+#define bm_err_isr_disable_read(bm) \
+ __bm_err_isr_read(bm, bm_isr_disable)
+#define bm_err_isr_disable_write(bm, v) \
+ __bm_err_isr_write(bm, bm_isr_disable, v)
+#define bm_err_isr_inhibit(bm) \
+ __bm_err_isr_write(bm, bm_isr_inhibit, 1)
+#define bm_err_isr_uninhibit(bm) \
+ __bm_err_isr_write(bm, bm_isr_inhibit, 0)
Is this layer really helpful?
+/*
+ * TODO: unimplemented registers
+ *
+ * BMAN_POOLk_SDCNT, BMAN_POOLk_HDCNT, BMAN_FULT,
+ * BMAN_VLDPL, BMAN_EECC, BMAN_SBET, BMAN_EINJ
+ */
What does it mean for registers to be unimplemented, in a piece of
software? If you mean accessors to those registers, why is that needed
if nothing uses them (yet)?
+/* Encapsulate struct bman * as a cast of the register space address. */
+
+static struct bman *bm_create(void *regs)
+{
+ return (struct bman *)regs;
+}
Unnecessary cast -- and unnecessary encapsulation (especially since you
only use it once). It's also missing __iomem -- have you run sparse on
this?
+static u32 __generate_thresh(u32 val, int roundup)
+{
+ u32 e = 0; /* co-efficient, exponent */
+
