linux-kernel - RE: [PATCHv6 1/4] 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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 4 Dec 2013 06:01:29 +0000
From:	Li Xiubo <Li.Xiubo@...escale.com>
To:	Mark Rutland <mark.rutland@....com>,
	Thierry Reding <thierry.reding@...il.com>,
	"galak@...eaurora.org" <galak@...eaurora.org>
CC:	Mike Turquette <mturquette@...aro.org>,
	Shawn Guo <Shawn.Guo@...escale.com>,
	"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
	"swarren@...dotorg.org" <swarren@...dotorg.org>,
	"t.figa@...sung.com" <t.figa@...sung.com>,
	"grant.likely@...aro.org" <grant.likely@...aro.org>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	"rob@...dley.net" <rob@...dley.net>,
	"ian.campbell@...rix.com" <ian.campbell@...rix.com>,
	Pawel Moll <Pawel.Moll@....com>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	Huan Wang <Huan.Wang@...escale.com>,
	Jingchang Lu <jingchang.lu@...escale.com>
Subject: RE: [PATCHv6 1/4] pwm: Add Freescale FTM PWM driver support

> > > > +static int fsl_pwm_parse_clk_ps(struct fsl_pwm_chip *fpc)
> > > > +{
> > > > +	int ret;
> > > > +	struct of_phandle_args clkspec;
> > > > +	struct device_node *np = fpc->chip.dev->of_node;
> > > > +
> > > > +	fpc->sys_clk = devm_clk_get(fpc->chip.dev, "ftm0");
> > > > +	if (IS_ERR(fpc->sys_clk)) {
> > > > +		ret = PTR_ERR(fpc->sys_clk);
> > > > +		dev_err(fpc->chip.dev,
> > > > +				"failed to get \"ftm0\" clock %d\n", ret);
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	fpc->counter_clk = devm_clk_get(fpc->chip.dev, "ftm0_counter");
> > > > +	if (IS_ERR(fpc->counter_clk)) {
> > > > +		ret = PTR_ERR(fpc->counter_clk);
> > > > +		dev_err(fpc->chip.dev,
> > > > +				"failed to get \"ftm0_counter\" clock %d\n",
> > > > +				ret);
> > > > +		return ret;
> > > > +	}
> > > > +
> > > > +	ret = of_parse_phandle_with_args(np, "clocks", "#clock-cells",
> 1,
> > > > +					&clkspec);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	fpc->counter_clk_select = clkspec.args[0];
> > >
> > > This isn't at all pretty. But given that once you have access to a
> > > struct clk there's no way to identify it, I don't know of a better
> > > alternative.
> >
> > Hi Mike,
> >
> > I've seen this crop up a number of times now, to varying degrees of
> > gravity. In this particular case, the driver needs to know the type of a
> > clock because it needs to program this hardware differently depending on
> > which clock feeds the counter. Since there is no way to obtain any kind
> > of identifying information from a struct clk, drivers need to rely on
> > hacks like this and manually reach into the device tree to obtain that
> > information.
> 
> Which property of the clock is the consumer concerned with in this case?
> 

The "ftm0_counter" clock.


> From a quick look at the driver it looks like there are actually a
> number of different input lines to the device that share the clock-name
> "ftm0_counter", though they are actually separate and each has a
> different divider. Have I got that right?
> 

Yes mostly, there are three different input lines wired to the counter
clock, but they share only one divider. And between the three lines and
the divider there is one mux inside of the FTM IP block.
The three clock sources are:
"ftm0",
"ftm0-fix",
"ftm0-ext".
		 _____________________________________________________
		|									|
		|   +++++++++++		FTM Module				|
ftm0 -------|-->+         +							|
		|   +         +     +++++++++++++     ++++++++++++++	|
ftm0-fix ---|-->+   MUX   +---->+  divider  +---->+  counter   +	|
		|   +         +     +++++++++++++     ++++++++++++++	|
ftm0-ext ---|-->+         +							|
		|   +++++++++++							|
		|_____________________________________________________|



> If that's the case, having a unique clock-names value for each of those
> lines would be the solution I'd expect. Then you just have to list the
> one(s) that are wired up, and the driver can figure out the appropriate
> line to use either by requesting by name until it finds a match or
> inspecting the clock-names property.
> 

Hi Kumar,
In the list archives for mails of "[RFC][PATCHv5 4/4] Documentation: Add
device tree bindings for Freescale FTM PWM.",we have discussed about this.
Is there any different with your suggestions or ideas? 


> Is there some other property of the parent that we care about here?
> 

As I know, not yet.

--
Best Regards,
 

--
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/