From: bpringlemeir@nbsps•com (Bill Pringlemeir)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCHv8 RFC] pwm: Add Freescale FTM PWM driver support
Date: Wed, 08 Jan 2014 11:36:44 -0500 [thread overview]
Message-ID: <87lhyqa0rn.fsf@nbsps.com> (raw)
In-Reply-To: 1388726661-3391-1-git-send-email-Li.Xiubo@freescale.com
On 3 Jan 2014, Li.Xiubo at freescale.com wrote:
> The FTM PWM device can be found on Vybrid VF610 Tower and
> Layerscape LS-1 SoCs.
>
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale•com>
> Signed-off-by: Alison Wang <b18965@freescale•com>
> Signed-off-by: Jingchang Lu <b35083@freescale•com>
> Reviewed-by: Sascha Hauer <s.hauer@pengutronix•de>
> ---
>
> Hi Thierry, Bill
[snip]
> +static unsigned long fsl_pwm_calculate_period_cycles(struct fsl_pwm_chip *fpc,
> + unsigned long period_ns,
> + enum fsl_pwm_clk index)
> +{
> + bool bg = fpc->big_endian;
> + int ret;
> +
> + fpc->counter_clk_select = FTM_SC_CLK(bg, index);
Yes, this is the spirit of what I was suggesting. The code is much less
efficient/bigger on the Vybrid with this run-time detection; but this is
more efficient/smaller than previous versions. I think that 'bg' can be
a compiler '#define' base on the configured SOC-systems. Ie, if the
kernel config only has 'Vybrid' or only 'LayerScape', then 'bg' can be a
hard coded value. The compiler will produce much better code in these
cases.
Also, maybe 'distro' people may want to make a 'hand-held' (Debian) or a
'router' (OpenWRT) distribution and they would only pick either 'Vybrid'
or 'LayerScape'. However, if someone wants an 'every ARM under the
sun', then the code still works. So, I think that the code is better
setup for a subsequent patch set like this (or at least just a good).
Especially, the stuff on the I/O swapping in the 'readl()' and
'writel()' is no longer needed; I think you can use the same function
for both SOCs.
> +#define __FTM_SWAP32(v) ((u32)(\
> + (((u32)(v) & (u32)0x000000ffUL) << 24) |\
> + (((u32)(v) & (u32)0x0000ff00UL) << 8) |\
> + (((u32)(v) & (u32)0x00ff0000UL) >> 8) |\
> + (((u32)(v) & (u32)0xff000000UL) >> 24)))
> +#define FTM_SWAP32(b, v) (b ? __FTM_SWAP32(v) : v)
I think that there are macros that you could use here. For instance,
'#include <linux/swab.h>' (powerpc and arm) has some assembler macros
that are quite fast for swapping. If the kernel config has ARCH >= 6
for ARM, then the very fast 'rev' instruction is used. If not, then a
generic version is used as you have coded. The PowerPC (another
possible future ARCH for QorIQ/Layerscape SOC?) always has inline
assembler macros.
So,
+ #include <linux/swab.h>
...
+ #define FTM_SWAP32(b, v) (b ? __swab32(v) : v)
might be better.
Suggested-by: Bill Pringlemeir <bpringlemeir@nbsps•com>
Thanks,
Bill Pringlemeir.
next prev parent reply other threads:[~2014-01-08 16:36 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-03 5:24 [PATCHv8 RFC] pwm: Add Freescale FTM PWM driver support Xiubo Li
2014-01-03 7:45 ` Dmitry Torokhov
2014-01-03 9:16 ` Li.Xiubo at freescale.com
2014-01-03 23:08 ` Dmitry Torokhov
2014-01-06 5:25 ` Li.Xiubo at freescale.com
2014-01-08 16:36 ` Bill Pringlemeir [this message]
2014-01-09 7:57 ` Li.Xiubo at freescale.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87lhyqa0rn.fsf@nbsps.com \
--to=bpringlemeir@nbsps$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox