| Message ID | 20191108130123.6839-8-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | QUICC Engine support on ARM and ARM64 | expand |
On Fri, Nov 8, 2019 at 7:04 AM Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > > +static bool qe_general4_errata(void) > +{ > +#ifdef CONFIG_PPC32 > + return pvr_version_is(PVR_VER_836x) || pvr_version_is(PVR_VER_832x); > +#endif > + return false; > +} > + > /* Program the BRG to the given sampling rate and multiplier > * > * @brg: the BRG, QE_BRG1 - QE_BRG16 > @@ -223,7 +231,7 @@ int qe_setbrg(enum qe_clock brg, unsigned int rate, unsigned int multiplier) > /* Errata QE_General4, which affects some MPC832x and MPC836x SOCs, says > that the BRG divisor must be even if you're not using divide-by-16 > mode. */ Can you also move this comment (and fix the comment formatting so that it's a proper function comment) to qe_general4_errata()?
On 15/11/2019 05.50, Timur Tabi wrote: > On Fri, Nov 8, 2019 at 7:04 AM Rasmus Villemoes > <linux@rasmusvillemoes.dk> wrote: >> >> +static bool qe_general4_errata(void) >> +{ >> +#ifdef CONFIG_PPC32 >> + return pvr_version_is(PVR_VER_836x) || pvr_version_is(PVR_VER_832x); >> +#endif >> + return false; >> +} >> + >> /* Program the BRG to the given sampling rate and multiplier >> * >> * @brg: the BRG, QE_BRG1 - QE_BRG16 >> @@ -223,7 +231,7 @@ int qe_setbrg(enum qe_clock brg, unsigned int rate, unsigned int multiplier) >> /* Errata QE_General4, which affects some MPC832x and MPC836x SOCs, says >> that the BRG divisor must be even if you're not using divide-by-16 >> mode. */ > > Can you also move this comment (and fix the comment formatting so that > it's a proper function comment) to qe_general4_errata()? > I actually thought of doing that, but decided against it because the comment not only mentions the SOCs affected, but also explains the following math/logic. I mean, without that comment nearby, the code is if (qe_general4_errata()) if (some weird condition) divisor++; In contrast, I think the qe_general4_errata() is pretty self-explanatory - is this a SOC affected by that errata (whatever that errata may be about and what the software workaround is). Rasmus
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c index 85737e6f5b62..1d8aa62c7ddf 100644 --- a/drivers/soc/fsl/qe/qe.c +++ b/drivers/soc/fsl/qe/qe.c @@ -197,6 +197,14 @@ EXPORT_SYMBOL(qe_get_brg_clk); #define PVR_VER_836x 0x8083 #define PVR_VER_832x 0x8084 +static bool qe_general4_errata(void) +{ +#ifdef CONFIG_PPC32 + return pvr_version_is(PVR_VER_836x) || pvr_version_is(PVR_VER_832x); +#endif + return false; +} + /* Program the BRG to the given sampling rate and multiplier * * @brg: the BRG, QE_BRG1 - QE_BRG16 @@ -223,7 +231,7 @@ int qe_setbrg(enum qe_clock brg, unsigned int rate, unsigned int multiplier) /* Errata QE_General4, which affects some MPC832x and MPC836x SOCs, says that the BRG divisor must be even if you're not using divide-by-16 mode. */ - if (pvr_version_is(PVR_VER_836x) || pvr_version_is(PVR_VER_832x)) + if (qe_general4_errata()) if (!div16 && (divisor & 1) && (divisor > 3)) divisor++;
Commit e5c5c8d23fef (soc/fsl/qe: only apply QE_General4 workaround on affected SoCs) introduced use of pvr_version_is(), saying The QE_General4 workaround is only valid for the MPC832x and MPC836x SoCs. The other SoCs that embed a QUICC engine are not affected by this hardware bug and thus can use the computed divisors (this was successfully tested on the T1040). I'm reading the above as saying that the errata does not apply to the ARM-based SOCs with QUICC engine. In any case, use of pvr_version_is() must be guarded by CONFIG_PPC32 before we can remove the PPC32 dependency from CONFIG_QUICC_ENGINE, so introduce qe_general4_errata() to keep the necessary #ifdeffery localized to a trivial helper. Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- drivers/soc/fsl/qe/qe.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)