Messages in this thread | | | From | Qiang Zhao <> | Subject | RE: [PATCH 4/4] soc/fsl/qe: qe.c: support fsl,qe-snums property | Date | Fri, 1 Mar 2019 09:43:07 +0000 |
| |
On 01/03/2019 15.50,Rasmus Villemoes wrote: > -----Original Message----- > From: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > Sent: 2019年3月1日 15:50 > To: Qiang Zhao <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com> > Cc: Scott Wood <oss@buserror.net>; linux-kernel@vger.kernel.org; Timur Tabi > <timur@freescale.com>; Rasmus Villemoes <Rasmus.Villemoes@prevas.se> > Subject: Re: [PATCH 4/4] soc/fsl/qe: qe.c: support fsl,qe-snums property > > On 01/03/2019 04.36, Qiang Zhao wrote: > > On 2019年2月28日 18:31,Rasmus Villemoes wrote: > > > >> -----Original Message----- > >> From: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > >> Sent: 2019年2月28日 18:31 > >> To: Qiang Zhao <qiang.zhao@nxp.com>; Leo Li <leoyang.li@nxp.com> > >> Cc: Scott Wood <oss@buserror.net>; linux-kernel@vger.kernel.org; > >> Timur Tabi <timur@freescale.com>; Rasmus Villemoes > >> <Rasmus.Villemoes@prevas.se> > >> Subject: [PATCH 4/4] soc/fsl/qe: qe.c: support fsl,qe-snums property > >> > > > > So you define 14 snums for MPC8309, but there still be the comment "/* > > No QE ever has fewer than 28 SNUMs */" and it will check if The > num_of_snums "is >28", it will cause confusion, so I suggest to modify 28 to > 14. > > Sure, that needs updating. My thinking was that only legacy DTs would use the > fsl,qe-num-snums, and there would be no need to support lower values than > we used to, since the logic back in qe_snums_init wouldn't handle such values > appropriately anyway. > > > I read the old version QUICC Engine Block Reference Manual, it said > > snums table is not available on MPC8306/MPC8306S/MPC8309, So I think > the code it written long before with this version RM, and at that time, the > snums is at least 28, and nobody modify the code later. > > And now with the new version RM, it support > MPC8306/MPC8306S/MPC8309 > > with snums and have snums fewer then 28, so I think the minimum value > should Be modified to 14. > > Yes. I'll do an extra cleanup patch modifying the code comments appropriately. > But what do you think about the core idea behind this change (and the > preceding cleanup patches)?
Maybe we could modify the comments in this patch, Anyway, the MPC8306/MPC8306S/MPC8309 Is supported with snums and the number is 14, In addition, you assign qe_num_of_snum to i as below. The variable stands for num of snums. Or we could add comments to explain it clearly why qe_num_of_snum is assign to a value fewer than 28 While it says " No QE ever has fewer than 28 SNUMs ".
+ qe = qe_get_device_node(); + if (qe) { + i = of_property_read_variable_u8_array(qe, "fsl,qe-snums", + snums, 14, QE_NUM_OF_SNUM); + of_node_put(qe); + if (i > 0) { + qe_num_of_snum = i; + return; + } + }
Best Regards Qiang Zhao
| |