[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20190301145744.623-1-rasmus.villemoes@prevas.dk>
Date: Fri, 1 Mar 2019 14:57:59 +0000
From: Rasmus Villemoes <rasmus.villemoes@...vas.dk>
To: Qiang Zhao <qiang.zhao@....com>, Leo Li <leoyang.li@....com>
CC: Scott Wood <oss@...error.net>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Rasmus Villemoes <Rasmus.Villemoes@...vas.se>
Subject: [PATCH 5/4] soc/fsl/qe: qe.c: fold qe_get_num_of_snums into
qe_snums_init
The comment "No QE ever has fewer than 28 SNUMs" is false; e.g. the
MPC8309 has 14. The code path returning -EINVAL is also a recipe for
instant disaster, since the caller (qe_snums_init) uncritically
assigns the return value to the unsigned qe_num_of_snum, and would
thus proceed to attempt to copy 4GB from snum_init_46[] to the snum[]
array.
So fold the handling of the legacy fsl,qe-num-snums into
qe_snums_init, and make sure we do not end up using the snum_init_46
array in cases other than the two where we know it makes sense.
Signed-off-by: Rasmus Villemoes <rasmus.villemoes@...vas.dk>
---
Qiang: Something like this. I prefer not to fold it into patch 4/4
since it really fixes unrelated issues (the wrong comment and the
bogus handling of unexpected return values from qe_get_num_of_snums),
but I can do so if you wish.
drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
drivers/soc/fsl/qe/qe.c | 54 +++++++----------------
include/soc/fsl/qe/qe.h | 2 +-
3 files changed, 19 insertions(+), 39 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 22a817da861e..3b2e28614e7b 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3842,7 +3842,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
}
if (max_speed == SPEED_1000) {
- unsigned int snums = qe_get_num_of_snums();
+ unsigned int snums = qe_num_of_snum;
/* configure muram FIFOs for gigabit operation */
ug_info->uf_info.urfs = UCC_GETH_URFS_GIGA_INIT;
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 8b915129077a..f8ddbb5b0eb5 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -52,7 +52,8 @@ EXPORT_SYMBOL(qe_immr);
static u8 snums[QE_NUM_OF_SNUM]; /* Dynamically allocated SNUMs */
static DECLARE_BITMAP(snum_state, QE_NUM_OF_SNUM);
-static unsigned int qe_num_of_snum;
+unsigned int qe_num_of_snum;
+EXPORT_SYMBOL(qe_num_of_snum);
static phys_addr_t qebase = -1;
@@ -308,26 +309,34 @@ static void qe_snums_init(void)
int i;
bitmap_zero(snum_state, QE_NUM_OF_SNUM);
+ qe_num_of_snum = 28; /* The default number of snum for threads is 28 */
qe = qe_get_device_node();
if (qe) {
i = of_property_read_variable_u8_array(qe, "fsl,qe-snums",
snums, 1, QE_NUM_OF_SNUM);
- of_node_put(qe);
if (i > 0) {
+ of_node_put(qe);
qe_num_of_snum = i;
return;
}
+ /*
+ * Fall back to legacy binding of using the value of
+ * fsl,qe-num-snums to choose one of the static arrays
+ * above.
+ */
+ of_property_read_u32(qe, "fsl,qe-num-snums", &qe_num_of_snum);
+ of_node_put(qe);
}
- qe_num_of_snum = qe_get_num_of_snums();
-
if (qe_num_of_snum == 76)
snum_init = snum_init_76;
- else
+ else if (qe_num_of_snum == 28 || qe_num_of_snum == 46)
snum_init = snum_init_46;
-
- for (i = 0; i < qe_num_of_snum; i++)
- snums[i] = snum_init[i];
+ else {
+ pr_err("QE: unsupported value of fsl,qe-num-snums: %u\n", qe_num_of_snum);
+ return;
+ }
+ memcpy(snums, snum_init, qe_num_of_snum);
}
int qe_get_snum(void)
@@ -649,35 +658,6 @@ unsigned int qe_get_num_of_risc(void)
}
EXPORT_SYMBOL(qe_get_num_of_risc);
-unsigned int qe_get_num_of_snums(void)
-{
- struct device_node *qe;
- int size;
- unsigned int num_of_snums;
- const u32 *prop;
-
- num_of_snums = 28; /* The default number of snum for threads is 28 */
- qe = qe_get_device_node();
- if (!qe)
- return num_of_snums;
-
- prop = of_get_property(qe, "fsl,qe-num-snums", &size);
- if (prop && size == sizeof(*prop)) {
- num_of_snums = *prop;
- if ((num_of_snums < 28) || (num_of_snums > QE_NUM_OF_SNUM)) {
- /* No QE ever has fewer than 28 SNUMs */
- pr_err("QE: number of snum is invalid\n");
- of_node_put(qe);
- return -EINVAL;
- }
- }
-
- of_node_put(qe);
-
- return num_of_snums;
-}
-EXPORT_SYMBOL(qe_get_num_of_snums);
-
static int __init qe_init(void)
{
struct device_node *np;
diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
index b3d1aff5e8ad..af5739850bf4 100644
--- a/include/soc/fsl/qe/qe.h
+++ b/include/soc/fsl/qe/qe.h
@@ -212,7 +212,7 @@ int qe_setbrg(enum qe_clock brg, unsigned int rate, unsigned int multiplier);
int qe_get_snum(void);
void qe_put_snum(u8 snum);
unsigned int qe_get_num_of_risc(void);
-unsigned int qe_get_num_of_snums(void);
+extern unsigned int qe_num_of_snum;
static inline int qe_alive_during_sleep(void)
{
--
2.20.1
Powered by blists - more mailing lists