On 01/03/2019 15.50，Rasmus Villemoes wrote:
> -----Original Message-----
> From: Rasmus Villemoes <rasmus.villem...@prevas.dk>
> Sent: 2019年3月1日 15:50
> To: Qiang Zhao <qiang.z...@nxp.com>; Leo Li <leoyang...@nxp.com>
> Cc: Scott Wood <o...@buserror.net>; linux-kernel@vger.kernel.org; Timur Tabi
> <ti...@freescale.com>; Rasmus Villemoes <rasmus.villem...@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.villem...@prevas.dk>
> >> Sent: 2019年2月28日 18:31
> >> To: Qiang Zhao <qiang.z...@nxp.com>; Leo Li <leoyang...@nxp.com>
> >> Cc: Scott Wood <o...@buserror.net>; linux-kernel@vger.kernel.org;
> >> Timur Tabi <ti...@freescale.com>; Rasmus Villemoes
> >> <rasmus.villem...@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
