linux-kernel - RE: [PATCHv8 RFC] pwm: Add Freescale FTM PWM driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 9 Jan 2014 07:57:44 +0000
From:	"Li.Xiubo@...escale.com" <Li.Xiubo@...escale.com>
To:	Bill Pringlemeir <bpringlemeir@...ps.com>
CC:	"thierry.reding@...il.com" <thierry.reding@...il.com>,
	"linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Huan Wang <Huan.Wang@...escale.com>,
	Jingchang Lu <jingchang.lu@...escale.com>
Subject: RE: [PATCHv8 RFC] pwm: Add Freescale FTM PWM driver support


> > +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.
> 

Yes.

I have removed the big-endian support temporarily, and will send followed
patches about this.

 
Thanks,

--
Best Regards,
Xiubo

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/