Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem

From: Sebastian Reichel <sre@kernel.org>
To: Stefan Wahren <stefan.wahren@i2se.com>
Cc: Mark Rutland <mark.rutland@arm.com>, Marek Vasut <marex@denx.de>,
	Pawel Moll <pawel.moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Mark Brown <broonie@kernel.org>,
	linux-pm@vger.kernel.org, Liam Girdwood <lgirdwood@gmail.com>,
	devicetree@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Sascha Hauer <kernel@pengutronix.de>,
	Kumar Gala <galak@codeaurora.org>,
	Shawn Guo <shawn.guo@linaro.org>,
	Fabio Estevam <festevam@gmail.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH RFC 2/3] power: mxs_power: add driver for mxs power subsystem
Date: Wed, 28 Jan 2015 23:59:38 +0100	[thread overview]
Message-ID: <20150128225937.GA12749@earth.universe> (raw)
In-Reply-To: <443299280.47516.1422483725178.JavaMail.open-xchange@oxbaltgw03.schlund.de>

[-- Attachment #1.1: Type: text/plain, Size: 2340 bytes --]

Hi,

On Wed, Jan 28, 2015 at 11:22:05PM +0100, Stefan Wahren wrote:
> > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 20:43 geschrieben:
> > On Tue, Jan 27, 2015 at 07:35:09PM +0100, Stefan Wahren wrote:
> > > > Mark Brown <broonie@kernel.org> hat am 27. Januar 2015 um 01:16
> > > > geschrieben:
> >
> > > > > okay i understand. But doesn't it need a extra driver to set the
> > > > > switching
> > > > > frequency because of the new compatible string?
> >
> > > > I don't understand this bit at all, sorry.
> >
> > > Sebastian suggested a new sub-node in the devicetree:
> >
> > > powerpll {
> > > compatible = "fsl,imx28-power-pll"
> > > #clock-cells = <0>;
> > > clock-frequency = <12345>;
> > > }
> >
> > > and i think that the new compatible string needs a separate driver to take
> > > care
> > > of the switching frequency.
> >
> > I can see that this has been suggested, what I can't understand is why
> > we would wish to do this but I am missing some context here which may
> > make it all perfectly clear.
> 
> The relevant register DC-DC Miscellaneous Register (HW_POWER_MISC) is described
> in chapter 11.12.10 of the reference manual [1].
>
> [1] - http://cache.freescale.com/files/dsp/doc/ref_manual/MCIMX28RM.pdf

Having a look at the datasheet my suggestion for the sub-node
doesn't make sense. I missunderstood how the hardware architecture
looks like, sorry for the noise :(

> changing the switching frequency is not a common feature for this IC. In most
> cases the clock for the DC-DC switching frequency runs at 24 MHz (ref_xtal).
> Unfortunatelly this could cause interferences for example in UHF band. So
> switching to ref_pll and changing clock divider is a option to avoid such
> interferences without changing hardware.
>
> I decided to implement it as a module parameter, because i see it's a
> configuration option.

I think its best to use a DT property then. That way it can be set
appropiatly for boards known to use a band, which could have
interference problems.

So just add a simple property (without a subnode). If I understand
Mark correctly, there's a high chance more drivers will need to
describe the switching frequency, so I suggest to use a vendor
independent property name:

switching-frequency = <12345>;

-- Sebastian

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel