* Re: [PATCH] V4L: s5k6a3: Add DT binding documentation
From: Mark Rutland @ 2014-02-19 17:47 UTC (permalink / raw)
To: Sylwester Nawrocki
Cc: Sylwester Nawrocki, devicetree@vger.kernel.org, Rob Herring,
Mauro Carvalho Chehab, linux-media@vger.kernel.org,
linux-samsung-soc@vger.kernel.org, Kyungmin Park, Pawel Moll,
Kumar Gala
In-Reply-To: <53037F8F.3050302@samsung.com>
On Tue, Feb 18, 2014 at 03:43:11PM +0000, Sylwester Nawrocki wrote:
> On 22/12/13 22:27, Sylwester Nawrocki wrote:
> > This patch adds DT binding documentation for the Samsung S5K6A3(YX)
> > raw image sensor.
> >
> > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> > This patch adds missing documentation [1] for the "samsung,s5k6a3"
> > compatible. Rob, can you please merge it through your tree if it
> > looks OK ?
>
> Anyone cares to Ack this patch so it can be merged through the media
> tree ?
It looks fine to me:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
>
> > Thanks,
> > Sylwester
> >
> > [1] http://www.spinics.net/lists/devicetree/msg10693.html
> >
> > Changes since v3 (https://linuxtv.org/patch/20429):
> > - rephrased 'clocks' and 'clock-names' properties' description.
> > ---
> > .../devicetree/bindings/media/samsung-s5k6a3.txt | 33 ++++++++++++++++++++
> > 1 files changed, 33 insertions(+), 0 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
> > new file mode 100644
> > index 0000000..cce01e8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
> > @@ -0,0 +1,33 @@
> > +Samsung S5K6A3(YX) raw image sensor
> > +---------------------------------
> > +
> > +S5K6A3(YX) is a raw image sensor with MIPI CSI-2 and CCP2 image data interfaces
> > +and CCI (I2C compatible) control bus.
> > +
> > +Required properties:
> > +
> > +- compatible : "samsung,s5k6a3";
> > +- reg : I2C slave address of the sensor;
> > +- svdda-supply : core voltage supply;
> > +- svddio-supply : I/O voltage supply;
> > +- afvdd-supply : AF (actuator) voltage supply;
> > +- gpios : specifier of a GPIO connected to the RESET pin;
> > +- clocks : should contain list of phandle and clock specifier pairs
> > + according to common clock bindings for the clocks described
> > + in the clock-names property;
> > +- clock-names : should contain "extclk" entry for the sensor's EXTCLK clock;
> > +
> > +Optional properties:
> > +
> > +- clock-frequency : the frequency at which the "extclk" clock should be
> > + configured to operate, in Hz; if this property is not
> > + specified default 24 MHz value will be used.
> > +
> > +The common video interfaces bindings (see video-interfaces.txt) should be
> > +used to specify link to the image data receiver. The S5K6A3(YX) device
> > +node should contain one 'port' child node with an 'endpoint' subnode.
> > +
> > +Following properties are valid for the endpoint node:
> > +
> > +- data-lanes : (optional) specifies MIPI CSI-2 data lanes as covered in
> > + video-interfaces.txt. The sensor supports only one data lane.
>
>
^ permalink raw reply
* Re: [PATCH v2 1/3] usb: chipidea: msm: Add device tree binding information
From: Courtney Cavin @ 2014-02-19 17:46 UTC (permalink / raw)
To: Ivan T. Ivanov
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Rob Landley, Greg Kroah-Hartman, David Brown,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
In-Reply-To: <1392824602.17130.68.camel@iivanov-dev>
On Wed, Feb 19, 2014 at 04:43:22PM +0100, Ivan T. Ivanov wrote:
>
> Hi,
>
> On Tue, 2014-02-18 at 13:26 -0800, Courtney Cavin wrote:
> > On Tue, Feb 18, 2014 at 02:21:19PM +0100, Ivan T. Ivanov wrote:
> > > From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> > >
> > > Document device tree binding information as required by
> > > the Qualcomm USB controller.
> > >
> > > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> > > ---
> > > .../devicetree/bindings/usb/msm-hsusb.txt | 17 +++++++++++++++++
> >
> > Although you mentioned to Josh that this is intended for "non-standard"
> > Chipidea properties, I don't see any other than requiring that 'dr_mode'
> > must be "peripheral". It would seem that this should all be integrated
> > into a ci3xxx.txt.
>
> Hm, there is no ci3xxx.txt. The closest match is ci-hdrc-imx.txt.
> So it could be ci-hdrc-qcom.txt or my preferred name qcom,ci-hdrc.txt?
Sorry, I was referring to ci13xxx-imx.txt, which was apparently moved to
ci-hdrc-imx.txt. I was recommending to merge the two into one
'ci13xxx.txt', as this binding seems to be a new compatible for the same
basic chip. Now perhaps 'ci-hdrc.txt'.
Although I agree with Josh that this name should be changed, and I think
either of your two suggestions would be acceptable, I would like to at
least discuss the possibility of actually merging the two in this
series.
Comments?
-Courtney
^ permalink raw reply
* Re: [PATCH 1/2] ARM: dts: add device tree source for imx6sx SoC
From: Mark Rutland @ 2014-02-19 17:42 UTC (permalink / raw)
To: Anson Huang
Cc: shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1392796036-19094-1-git-send-email-b20788-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> + intc: interrupt-controller@00a01000 {
> + compatible = "arm,cortex-a9-gic";
> + #interrupt-cells = <3>;
> + #address-cells = <1>;
> + #size-cells = <1>;
Any reason for the #address-cells and #size-cells? The node has no
children, and I don't see an interrupt-map.
> + interrupt-controller;
> + reg = <0x00a01000 0x1000>,
> + <0x00a00100 0x100>;
> + };
> +
> + clocks {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ckil {
> + compatible = "fsl,imx-ckil", "fixed-clock";
> + clock-frequency = <32768>;
> + };
> +
> + osc {
> + compatible = "fsl,imx-osc", "fixed-clock";
> + clock-frequency = <24000000>;
> + };
> + };
What is this clocks container node? It has no compatible property. The
children have no reg entries and thus #address-cells and #size cells are
useless.
Why have this at all? Why not put the clocks directly under the root?
[...]
> + anatop: anatop@020c8000 {
> + compatible = "fsl,imx6sx-anatop", "fsl,imx6q-anatop",
> + "syscon", "simple-bus";
NAK. That compatible list makes _no_ sense. Why are both simple-bus and
syscon there?
> + reg = <0x020c8000 0x1000>;
> + interrupts = <0 49 0x04 0 54 0x04 0 127 0x04>;
Please bracket list entries individually. It's not clear where one
interrupt ends and the next begins.
> + };
> +
> + snvs@020cc000 {
> + compatible = "fsl,sec-v4.0-mon", "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0x020cc000 0x4000>;
> +
> + snvs-rtc-lp@34 {
> + compatible = "fsl,sec-v4.0-mon-rtc-lp";
> + reg = <0x34 0x58>;
> + interrupts = <0 19 0x04 0 20 0x04>;
> + };
> + };
The binding document mentions nothing about fsl,sec-v4.0-mon being a
bus, let alone one compatible with simple-bus. Are the child nodes
usable in isolation, even if the parent node weren't probed?
> + sdma: sdma@020ec000 {
> + compatible = "fsl,imx6q-sdma", "fsl,imx35-sdma";
> + reg = <0x020ec000 0x4000>;
> + interrupts = <0 2 0x04>;
> + clocks = <&clks IMX6SX_CLK_SDMA>, <&clks IMX6SX_CLK_SDMA>;
> + clock-names = "ipg", "ahb";
> + #dma-cells = <3>;
> + fsl,sdma-ram-script-name = "imx/sdma/sdma-imx6sx.bin";
This is insane. Why is this path in the DT at all?
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 00/18] ahci: library-ise ahci_platform, add sunxi driver and cleanup imx driver
From: Tejun Heo @ 2014-02-19 17:42 UTC (permalink / raw)
To: Hans de Goede
Cc: Maxime Ripard, Oliver Schinagl, Richard Zhu, Roger Quadros,
Lee Jones, linux-ide, linux-arm-kernel, devicetree, linux-sunxi
In-Reply-To: <5304E751.5040608@redhat.com>
On Wed, Feb 19, 2014 at 06:18:09PM +0100, Hans de Goede wrote:
> Most of the resources are already devres managed (I use devm functions
> to get them), the problem is not in freeing our reference to the resources,
> the problem is that we've sequences like this:
>
> devm_get_foo
> enable_foo
> disable_foo
> (automatic release foo)
>
> Where enable / disable can be done repeatedly (ie each suspend / resume).
>
> From your review comments, I take it that you want the final disable_foo
> on driver release to happen automatically.
>
> My preference for this would be to extend the devres tracking already present
> in the relevant subsystems to keep track of the enable count done through a
> specific reference, to allow automatic disable (if needed) on release.
>
> But thinking more about this, I think that doing this automatically is a bad
> idea, because then we fixate the shutdown sequence to a certain order (the
> order in which we did the _get_foo for the resources) and the correct order may
> be device specific.
>
> So TL;DR: Yes to making things so that ahci_platform_put_resources gets done
> automatically, no to automating the disable calls.
>
> If I don't hear back from you, then I'll respin the patch-set assuming that
> you agree to the above.
Yeah, sounds good enough to me.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v6 00/18] ahci: library-ise ahci_platform, add sunxi driver and cleanup imx driver
From: Robert Nelson @ 2014-02-19 17:20 UTC (permalink / raw)
To: Hans de Goede
Cc: Tejun Heo, Maxime Ripard, devicetree, Linux-IDE, Oliver Schinagl,
Richard Zhu, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Lee Jones,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Roger Quadros
In-Reply-To: <1392811320-3132-1-git-send-email-hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2328 bytes --]
On Wed, Feb 19, 2014 at 6:01 AM, Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Hi all,
>
> Here is v6 of my patchset for adding ahci-sunxi support. This has been
> tested with Allwinner A10, Allwinner A20 and Freeware imx6x SoCs, including
> suspend / resume. Note that since my last revision the ahci_imx driver has
> also grown imx53 sata support, it would be good if some-one could test that
> with this series.
>
basic file system tests on an imx53-qsb look fine with sata changes..
Using v3.14-rc3 + 20140219 (next) + shawn.guo's imx for-next branch which
wasn't in today next tree..
debian@arm:~$ dmesg | grep ata
[ 0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[ 0.000000] Memory policy: Data cache writeback
[ 0.000000] Memory: 1010076K/1048576K available (7008K kernel code, 527K
rwdata, 2280K rodata, 416K init, 373K bss, 38500K reserved, 524288K highmem)
[ 0.000000] .data : 0xc0984000 - 0xc0a07d60 ( 528 kB)
[ 1.796587] libata version 3.00 loaded.
[ 2.777037] ahci-imx 10000000.sata: unable to find phy
[ 2.784385] ahci-imx 10000000.sata: SSS flag set, parallel bus scan
disabled
[ 2.791495] ahci-imx 10000000.sata: AHCI 0001.0100 32 slots 1 ports 3
Gbps 0x1 impl platform mode
[ 2.800394] ahci-imx 10000000.sata: flags: ncq sntf stag pm led clo only
pmp pio slum part ccc
[ 2.814178] ata1: SATA max UDMA/133 mmio [mem 0x10000000-0x10000fff]
port 0x100 irq 44
[ 3.400107] ata1: softreset failed (device not ready)
[ 3.405174] ata1: applying PMP SRST workaround and retrying
[ 3.610105] ata1: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[ 3.663654] ata1.00: ATA-8: ST9120315AS, 0001SDM1, max UDMA/133
[ 3.669583] ata1.00: 234441648 sectors, multi 16: LBA48 NCQ (depth 31/32)
[ 3.678553] ata1.00: configured for UDMA/133
[ 5.680633] EXT4-fs (mmcblk0p2): mounted filesystem with ordered data
mode. Opts: (null)
Regards,
--
Robert Nelson
http://www.rcn-ee.com/
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/groups/opt_out.
[-- Attachment #2: Type: text/html, Size: 3172 bytes --]
^ permalink raw reply
* Re: [PATCH v6 00/18] ahci: library-ise ahci_platform, add sunxi driver and cleanup imx driver
From: Hans de Goede @ 2014-02-19 17:18 UTC (permalink / raw)
To: Tejun Heo
Cc: Maxime Ripard, Oliver Schinagl, Richard Zhu, Roger Quadros,
Lee Jones, linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20140219162511.GJ10134-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
Hi,
On 02/19/2014 05:25 PM, Tejun Heo wrote:
> Hello, Hans.
>
> On Wed, Feb 19, 2014 at 04:26:43PM +0100, Hans de Goede wrote:
>> The devres usage are really 2 different issues:
>>
>> 1) There is the issue that we're getting clocks by index (this is by design as
>> clk-names are not generic), but there is no devm_get_clk_by_index (or some
>> such), this is a shortcoming of the clk-core, which needs a separate patch
>> to fix. I would rather not delay this patch-set based on getting a patch
>> into another subsys.
>
> Yes, that'd be my usual preference too. The only reason I'm hesitant
> is because nobody seems to be too interested in long term aspect of
> the code base and the only leverage I have seems to be rejecting
> drivers until things become right.
>
>> Note that even with devm_get_clk_by_index we can unfortunately not get rid of
>> ahci_platform_put_resources() as in Roger's follow-up patches it gets used for
>> putting some runtime pm related resources too.
>>
>> I guess this argues for simply turning ahci_platform_get_resources into a
>> devm_ahci_platform_get_resources doing its own devm handling, and then
>> making ahci_platform_put_resources a private function called by the devm
>> framework. If you agree I can do that for the next patch-set.
>
> Note that libata core already does something similar. Please take a
> look at ata_host_alloc() for details. You don't really need to create
> a separate devm_ prefixed variant. Just making the function register
> devres automatically should be enough. If this is too much work, I'm
> okay with deferring it until later if you promise to do it soonish.
>
Ok, I'll try to do this for the next revision.
>> 2) In some comments you also seem to want devm variants of enable / disable
>> resources, or at least have ahci_platform_put_resources do the disable
>> automatically. The problem is that most of the functions called here need to
>> be balanced, they increment / decrement usage counters in the clk / regulator
>> subsystems, so we cannot simply unconditional do an ahci_platform_disable_resources
>> in ahci_platform_put_resources
>>
>> Doing the disable automatically requires tracking the enable state, and doing this
>> per resource, since the whole idea of having a separate ahci_platform_enable_resources
>> is that some drivers may want to override its behavior doing things in a different
>> order.
>>
>> To ensure proper tracking we would then need to offer ahci_platform_enable_regulator,
>> etc. functions, and ensure that all drivers using the ahci_platform framework
>> always go through these, rather then calling directly into the relevant framework.
>>
>> This is all doable, and I'm not against doing this but before spending a couple of
>> hours coding this all up, I would like to hear back from you whether you would like
>> to see this, or would rather keep things as is.
>
> Yes, in principle, I want every resource used by ahci_platform to be
> devres managed. I *think* devres should be flexible enough to handle
> what you're describing (each devres resource can have data for state
> tracking and devres resources can be looked up and modified, so the
> different orders should be okay) but if not I'd be happy to help
> extend it so that it can.
Most of the resources are already devres managed (I use devm functions
to get them), the problem is not in freeing our reference to the resources,
the problem is that we've sequences like this:
devm_get_foo
enable_foo
disable_foo
(automatic release foo)
Where enable / disable can be done repeatedly (ie each suspend / resume).
>From your review comments, I take it that you want the final disable_foo
on driver release to happen automatically.
My preference for this would be to extend the devres tracking already present
in the relevant subsystems to keep track of the enable count done through a
specific reference, to allow automatic disable (if needed) on release.
But thinking more about this, I think that doing this automatically is a bad
idea, because then we fixate the shutdown sequence to a certain order (the
order in which we did the _get_foo for the resources) and the correct order may
be device specific.
So TL;DR: Yes to making things so that ahci_platform_put_resources gets done
automatically, no to automating the disable calls.
If I don't hear back from you, then I'll respin the patch-set assuming that
you agree to the above.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH] net: add init-regs for of_phy support
From: Laurent Pinchart @ 2014-02-19 17:15 UTC (permalink / raw)
To: Mark Rutland
Cc: Ben Dooks, netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-sh@vger.kernel.org
In-Reply-To: <20140217141222.GB19308@e106331-lin.cambridge.arm.com>
Hi Ben,
On Monday 17 February 2014 14:12:22 Mark Rutland wrote:
> On Mon, Feb 17, 2014 at 01:50:28PM +0000, Ben Dooks wrote:
> > On 17/02/14 13:44, Mark Rutland wrote:
> > > On Mon, Feb 17, 2014 at 01:08:04PM +0000, Ben Dooks wrote:
> > >> Add new init-regs field for of_phy nodes and make sure these
> > >> get applied when the phy is configured.
> > >>
> > >> This allows any phy node in an fdt to initialise registers
> > >> that may not be set as standard by the driver at initialisation
> > >> time, such as LED controls.
> > >
> > > Why not have a driver for the particular PHY? If it's not standard we
> > > don't need to pretend it is. If it has some extensions then the standard
> > > compatible string can be a fallback entry in the compatible list.
> >
> > I was trying to provide some useful and reasonably generic code
> > to setup PHYs without having to add specific arguments to each of
> > them.
> >
> > I could have added something like ksz8041,led-mode1 = <1> and
> > updated the micrel driver.
>
> Perhaps. It depends on precisely what the property is describing.
>
> I think describing the hardware and letting Linux figure out what to do
> is better than giving it a set of opaque instructions for it to blindly
> follow.
I second that, I think a LED mode property would be better in this case.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* [PATCH] clk: shmobile: Fix typoe in MSTP clock DT bindings
From: Laurent Pinchart @ 2014-02-19 17:13 UTC (permalink / raw)
To: linux-sh; +Cc: devicetree, Mike Turquette, Geert Uytterhoeven
The DT bindings document a renesas,indices property, while the code, the
DT example and the DT sources all use renesas,clock-indices. Fix the
documentation.
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Mike, this is a v3.14 bug fix. You can find all three Renesas clock bugfixes
for v3.14 in
git://linuxtv.org/pinchartl/fbdev.git clocks/next/drivers
I'll wait a day and send a pull request just to make sure no patch gets
forgotten.
diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
index a6a352c..5992dce 100644
--- a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
+++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
@@ -21,9 +21,9 @@ Required Properties:
must appear in the same order as the output clocks.
- #clock-cells: Must be 1
- clock-output-names: The name of the clocks as free-form strings
- - renesas,indices: Indices of the gate clocks into the group (0 to 31)
+ - renesas,clock-indices: Indices of the gate clocks into the group (0 to 31)
-The clocks, clock-output-names and renesas,indices properties contain one
+The clocks, clock-output-names and renesas,clock-indices properties contain one
entry per gate clock. The MSTP groups are sparsely populated. Unimplemented
gate clocks must not be declared.
--
Regards,
Laurent Pinchart
^ permalink raw reply related
* Re: [PATCH v5 4/5] ARM: Kirkwood: Fix qnap vendor prefix
From: Mark Rutland @ 2014-02-19 17:10 UTC (permalink / raw)
To: Ben Peddell
Cc: Andrew Lunn, Jason Cooper, Pawel Moll, Ian Campbell,
Dmitry Eremin-Solenikov, Anton Vorontsov,
devicetree@vger.kernel.org, Rob Herring, Kumar Gala,
David Woodhouse, linux-arm-kernel@lists.infradead.org
In-Reply-To: <5304AE28.6030209@killerwolves.net>
On Wed, Feb 19, 2014 at 01:14:16PM +0000, Ben Peddell wrote:
> On 19/02/14 23:05, Andrew Lunn wrote:
> >> diff --git a/drivers/power/reset/qnap-poweroff.c b/drivers/power/reset/qnap-poweroff.c
> >> index a75db7f..d350ece 100644
> >> --- a/drivers/power/reset/qnap-poweroff.c
> >> +++ b/drivers/power/reset/qnap-poweroff.c
> >> @@ -41,7 +41,7 @@ static const struct power_off_cfg synology_power_off_cfg = {
> >> };
> >>
> >> static const struct of_device_id qnap_power_off_of_match_table[] = {
> >> - { .compatible = "qnap,power-off",
> >> + { .compatible = "qnapsz,power-off",
> >> .data = &qnap_power_off_cfg,
> >> },
> >
> > Hi Ben
> >
> > This causes issues for devices using old DT blobs. You need to keep
> > the old entry as well as supporting the new vendor prefix.
> >
> > We need to consider, is it worth the hassle of changing the prefix,
> > since it has been in use for a long time. I think using the stock
> > ticker is a "rule of thumb" and not a strict requirement.
>
> So, in this case it would be best to change qnapsz back to qnap in
> patch #1, and completely remove this patch?
If we already have users of the qnap prefix, yes please.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v4 1/2] memory: ti-aemif: introduce AEMIF driver
From: Ivan Khoronzhuk @ 2014-02-19 17:04 UTC (permalink / raw)
To: Greg KH
Cc: santosh.shilimkar-l0cyMroinI0, rob-VoJi6FS/r0vR7s880joybQ,
linux-lFZ/pmaqli7XmaaqVzeoHQ,
galak-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, rob.herring-bsGFqQB8/DxBDgjK7y7TUQ,
swarren-3lzwWm7+Weoh9ZMKESR00Q,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
grygorii.strashko-l0cyMroinI0, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ,
nsekhar-l0cyMroinI0, [initial author] Murali Karicheri
In-Reply-To: <20140219143403.GA31063-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
On 02/19/2014 04:34 PM, Greg KH wrote:
> On Wed, Feb 19, 2014 at 12:32:02PM +0200, Ivan Khoronzhuk wrote:
>>>> + aemif->clk = devm_clk_get(dev, NULL);
>>>> + if (IS_ERR(aemif->clk)) {
>>>> + dev_err(dev, "cannot get clock 'aemif'\n");
>>>> + return PTR_ERR(aemif->clk);
>>> No freeing memory?
>> There is no need to free memory explicitly.
>> devm_kzalloc is used instead of kzalloc.
> Yes, but where does the device on the error path get removed?
Why these functions are needed in that case...
As I see, memory allocated with these functions
are automatically freed on driver detach.
Additionally I've checked if resources are released after
AEMIF driver probe error.
And they're released completely in case of probe error.
:-\
--
Regards,
Ivan Khoronzhuk
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH RFC v1 3/3] clk: Add handling of clk parent and rate assigned from DT
From: Sylwester Nawrocki @ 2014-02-19 16:58 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: mturquette-QSEj5FYQhm4dnm+yROfE0A, linux-lFZ/pmaqli7XmaaqVzeoHQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
galak-sgV2jX0FEOL9JmXXK+q4OQ,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
sw0312.kim-Sze3O3UU22JBDgjK7y7TUQ,
m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
t.figa-Sze3O3UU22JBDgjK7y7TUQ, Sylwester Nawrocki
In-Reply-To: <1392829124-25705-1-git-send-email-s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
This function adds a notifier callback run before a driver is bound to
its driver. It will configure parent clock and clock frequencies based
on [clk-name]-clk-parent and [clk-name]-clk-rate' DT properties.
Signed-off-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
.../devicetree/bindings/clock/clock-bindings.txt | 24 +++++
drivers/clk/clk.c | 92 ++++++++++++++++++++
2 files changed, 116 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
index 7c52c29..d618498 100644
--- a/Documentation/devicetree/bindings/clock/clock-bindings.txt
+++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
@@ -115,3 +115,27 @@ clock signal, and a UART.
("pll" and "pll-switched").
* The UART has its baud clock connected the external oscillator and its
register clock connected to the PLL clock (the "pll-switched" signal)
+
+==Static initial configuration of clock parent and clock frequency==
+
+Some platforms require static configuration of (parts of) the clock controller
+often determined by the board design. Such a configuration can be specified in
+a clock consumer node through [clk-name]-clk-parent and [clk-name]-clk-rate DT
+properties. The former should contain phandle and clock specifier of the parent
+clock, the latter the required clock's frequency value (one cell). "clk-name"
+should be listed in the clock-names property and a phandle and a clock specifier
+pair corresponding to it should be present in the clocks property.
+
+ uart@a000 {
+ compatible = "fsl,imx-uart";
+ reg = <0xa000 0x1000>;
+ ...
+ clocks = <&clkcon 0>, <&clkcon 3>;
+ clock-names = "baud", "mux";
+
+ mux-clk-parent = <&pll 1>;
+ baud-clk-rate = <460800>;
+ };
+
+In this example the pll is set as parent of "mux" clock and frequency of "baud"
+clock is specified as 460800 Hz.
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 19f6f3f..9238e08 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -19,6 +19,7 @@
#include <linux/of.h>
#include <linux/device.h>
#include <linux/init.h>
+#include <linux/platform_device.h>
#include <linux/sched.h>
#include "clk.h"
@@ -2527,6 +2528,97 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
}
EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
+static void __of_clk_assigned_config_set(struct clk *clk, struct clk *pclk,
+ u32 rate)
+{
+ int rc;
+
+ if (rate) {
+ rc = clk_set_rate(clk, rate);
+ if (rc < 0)
+ pr_err("clk: couldn't set rate of clock %s (%d)\n",
+ __clk_get_name(clk), rc);
+ else
+ pr_debug("clk: set rate of clock %s to %u\n",
+ __clk_get_name(clk), rate);
+ }
+
+ if (!IS_ERR(pclk)) {
+ rc = clk_set_parent(clk, pclk);
+ if (rc < 0)
+ pr_err("clk: couldn't set %s as parent of %s (%d)\n",
+ __clk_get_name(pclk), __clk_get_name(clk), rc);
+ else
+ pr_debug("clk: set %s as parent of %s\n",
+ __clk_get_name(pclk), __clk_get_name(clk));
+ }
+}
+
+static void of_clk_assigned_config_parse(struct device_node *node)
+{
+ char prop_name[OF_PROP_NAME_MAXLEN];
+ struct property *prop;
+ const char *clk_name;
+ int rc, index = 0;
+
+ of_property_for_each_string(node, "clock-names", prop, clk_name) {
+ struct clk *clk, *pclk;
+ u32 rate = 0;
+
+ snprintf(prop_name, OF_PROP_NAME_MAXLEN,
+ "%s-clk-parent", clk_name);
+ pclk = of_clk_get_list_entry(node, prop_name, 0);
+
+ snprintf(prop_name, OF_PROP_NAME_MAXLEN,
+ "%s-clk-rate", clk_name);
+ rc = of_property_read_u32(node, prop_name, &rate);
+
+ if (!rc || !IS_ERR(pclk)) {
+ /*
+ * Assuming here of_property_for_each_string() returns
+ * consecutive values of a DT property in ascending
+ * index order.
+ */
+ clk = of_clk_get(node, index);
+
+ if (!IS_ERR(clk))
+ __of_clk_assigned_config_set(clk, pclk, rate);
+ else
+ pr_err("clk: couldn't get clk %s\n", clk_name);
+ }
+ index++;
+ }
+}
+
+
+static int of_clk_setup_notifier_call(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct device *dev = data;
+
+ if (!dev->of_node)
+ return NOTIFY_DONE;
+
+ switch (event) {
+ case BUS_NOTIFY_BIND_DRIVER:
+ /* Parse and configure DT assigned clock parents and rates */
+ of_clk_assigned_config_parse(dev->of_node);
+ break;
+ }
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block of_clk_setup_nb = {
+ .notifier_call = of_clk_setup_notifier_call,
+};
+
+int __init of_clk_setup_notifier_init(void)
+{
+ return bus_register_notifier(&platform_bus_type, &of_clk_setup_nb);
+}
+subsys_initcall(of_clk_setup_notifier_init);
+
/**
* of_clk_init() - Scan and init clock providers from the DT
* @matches: array of compatible values and init functions for providers.
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH RFC v1 2/3] of: Add definition of maximum length of a property name
From: Sylwester Nawrocki @ 2014-02-19 16:58 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: mturquette-QSEj5FYQhm4dnm+yROfE0A, linux-lFZ/pmaqli7XmaaqVzeoHQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
galak-sgV2jX0FEOL9JmXXK+q4OQ,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
sw0312.kim-Sze3O3UU22JBDgjK7y7TUQ,
m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
t.figa-Sze3O3UU22JBDgjK7y7TUQ, Sylwester Nawrocki
In-Reply-To: <1392829124-25705-1-git-send-email-s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Maximum length of a property name is defined by ePAPR (2.2.4.1) as
31 characters. Add a corresponding definition, including the trailing
null space.
Signed-off-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
include/linux/of.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/of.h b/include/linux/of.h
index 70c64ba..7f71221 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -30,6 +30,9 @@
typedef u32 phandle;
typedef u32 ihandle;
+/* Maximum length of name of a property, including terminating null */
+#define OF_PROP_NAME_MAXLEN 32
+
struct property {
char *name;
int length;
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH RFC v1 1/3] clk: Add function to parse an arbitrary clocks list property
From: Sylwester Nawrocki @ 2014-02-19 16:58 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: mturquette-QSEj5FYQhm4dnm+yROfE0A, linux-lFZ/pmaqli7XmaaqVzeoHQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
galak-sgV2jX0FEOL9JmXXK+q4OQ,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
sw0312.kim-Sze3O3UU22JBDgjK7y7TUQ,
m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
t.figa-Sze3O3UU22JBDgjK7y7TUQ, Sylwester Nawrocki
In-Reply-To: <1392829124-25705-1-git-send-email-s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
The of_clk_get_list_entry() function is like of_clk_get() except it allows
to pass name of a DT property containing list of phandles and clock specifiers.
For of_clk_get() it has been hard coded to "clocks".
Signed-off-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Acked-by: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
drivers/clk/clk.h | 3 +++
drivers/clk/clkdev.c | 25 +++++++++++++++++++++----
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/clk.h b/drivers/clk/clk.h
index 795cc9f..dd9def1 100644
--- a/drivers/clk/clk.h
+++ b/drivers/clk/clk.h
@@ -10,7 +10,10 @@
*/
#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
+
struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec);
void of_clk_lock(void);
void of_clk_unlock(void);
+struct clk *of_clk_get_list_entry(struct device_node *np,
+ const char *list_name, int index);
#endif
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index a360b2e..465662e 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -27,17 +27,28 @@ static LIST_HEAD(clocks);
static DEFINE_MUTEX(clocks_mutex);
#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
-struct clk *of_clk_get(struct device_node *np, int index)
+/**
+ * of_clk_get_list_entry() - Parse and lookup a clock referenced by a device node
+ * @np: pointer to clock consumer node
+ * @list_name: name of the clock list property
+ * @index: index to the clock list
+ *
+ * This function parses the @list_name property and together with @index
+ * value indicating an entry of the list uses it to look up the struct clk
+ * from the registered list of clock providers.
+ */
+struct clk *of_clk_get_list_entry(struct device_node *np,
+ const char *list_name, int index)
{
struct of_phandle_args clkspec;
struct clk *clk;
int rc;
- if (index < 0)
+ if (index < 0 || !list_name)
return ERR_PTR(-EINVAL);
- rc = of_parse_phandle_with_args(np, "clocks", "#clock-cells", index,
- &clkspec);
+ rc = of_parse_phandle_with_args(np, list_name, "#clock-cells",
+ index, &clkspec);
if (rc)
return ERR_PTR(rc);
@@ -51,6 +62,12 @@ struct clk *of_clk_get(struct device_node *np, int index)
of_node_put(clkspec.np);
return clk;
}
+EXPORT_SYMBOL(of_clk_get_list_entry);
+
+struct clk *of_clk_get(struct device_node *np, int index)
+{
+ return of_clk_get_list_entry(np, "clocks", index);
+}
EXPORT_SYMBOL(of_clk_get);
/**
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH RFC v1 0/3] clk: Support for DT assigned clk parent and rate
From: Sylwester Nawrocki @ 2014-02-19 16:58 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: mturquette-QSEj5FYQhm4dnm+yROfE0A, linux-lFZ/pmaqli7XmaaqVzeoHQ,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
grant.likely-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
galak-sgV2jX0FEOL9JmXXK+q4OQ,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
sw0312.kim-Sze3O3UU22JBDgjK7y7TUQ,
m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
t.figa-Sze3O3UU22JBDgjK7y7TUQ, Sylwester Nawrocki
This patch set implements setting of parent clocks and clock frequencies
before a driver is bound to the device. This is only for booting with the
device tree case and the parent/rate configuration is supposed to be
specified according to the DT binding previously discussed [1].
Patch 1/3 adds a variant of of_clk_get() function which accepts name of a DT
property containing list of phandle to a clock controller node and the clock
specifier pairs, as opposed to hard coded "clocks" property name in
of_clk_get(). Any better names for the function are welcome.
Patch 2/3 just adds a macro definition for maximum length of a DT property
name. I couldn't find any already available and there are places where plain
numbers are used.
Patch 3/3 actually adds the code parsing device nodes for related DT
properties and performing re-parenting and/or clock frequency setting
as required.
I wasn't sure whether it should be done in a notifier like this, or perhaps
there should be direct calls added into driver core (similarly to pinctrl).
So any errors can be passed up to the callers and driver probe can be retried,
should it happen any required clocks are not yet available at probe() call
time.
[1] http://www.spinics.net/lists/arm-kernel/msg302226.html
Sylwester Nawrocki (3):
clk: Add function to parse an arbitrary clocks list property
of: Add definition of maximum length of a property name
clk: Add handling of clk parent and rate assigned from DT
.../devicetree/bindings/clock/clock-bindings.txt | 24 +++++
drivers/clk/clk.c | 92 ++++++++++++++++++++
drivers/clk/clk.h | 3 +
drivers/clk/clkdev.c | 25 +++++-
include/linux/of.h | 3 +
5 files changed, 143 insertions(+), 4 deletions(-)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC 04/10] base: power: Add generic OF-based power domain look-up
From: Philipp Zabel @ 2014-02-19 16:53 UTC (permalink / raw)
To: Tomasz Figa
Cc: linux-pm, Mark Rutland, devicetree, linux-samsung-soc,
Russell King, Pawel Moll, Len Brown, Greg Kroah-Hartman,
Tomasz Figa, Ian Campbell, Rafael J. Wysocki, linux-kernel,
Rob Herring, Bartlomiej Zolnierkiewicz, Kukjin Kim, Pavel Machek,
Kumar Gala, Stephen Warren, linux-arm-kernel
In-Reply-To: <1389469372-17199-5-git-send-email-tomasz.figa@gmail.com>
Hi Tomasz,
Am Samstag, den 11.01.2014, 20:42 +0100 schrieb Tomasz Figa:
> This patch introduces generic code to perform power domain look-up using
> device tree and automatically bind devices to their power domains.
> Generic device tree binding is introduced to specify power domains of
> devices in their device tree nodes.
>
> Backwards compatibility with legacy Samsung-specific power domain
> bindings is provided, but for now the new code is not compiled when
> CONFIG_ARCH_EXYNOS is selected to avoid collision with legacy code. This
> will change as soon as Exynos power domain code gets converted to use
> the generic framework in further patch.
>
> Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
> ---
> .../devicetree/bindings/power/power_domain.txt | 51 ++++
> drivers/base/power/domain.c | 339 +++++++++++++++++++++
> include/linux/pm_domain.h | 34 +++
> kernel/power/Kconfig | 4 +
> 4 files changed, 428 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/power/power_domain.txt
>
> diff --git a/Documentation/devicetree/bindings/power/power_domain.txt b/Documentation/devicetree/bindings/power/power_domain.txt
> new file mode 100644
> index 0000000..93be5d9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/power_domain.txt
> @@ -0,0 +1,51 @@
> +* Generic power domains
> +
> +System on chip designs are often divided into multiple power domains that
> +can be used for power gating of selected IP blocks for power saving by
> +reduced leakage current.
> +
> +This device tree binding can be used to bind power domain consumer devices
> +with their power domains provided by power domain providers. A power domain
> +provider can be represented by any node in the device tree and can provide
> +one or more power domains. A consumer node can refer to the provider by
> +a phandle and a set of phandle arguments (so called power domain specifier)
> +of length specified by #power-domain-cells property in the power domain
> +provider node.
> +
> +==Power domain providers==
> +
> +Required properties:
> + - #power-domain-cells : Number of cells in a power domain specifier;
> + Typically 0 for nodes representing a single power domain and 1 for nodes
> + providing multiple power domains (e.g. power controllers), but can be
> + any value as specified by device tree binding documentation of particular
> + provider.
> +
> +Example:
> +
> + power: power-controller@12340000 {
> + compatible = "foo,power-controller";
> + reg = <0x12340000 0x1000>;
> + #power-domain-cells = <1>;
> + };
> +
> +The node above defines a power controller that is a power domain provider
> +and expects one cell as its phandle argument.
> +
> +==Power domain consumers==
> +
> +Required properties:
> + - power-domain : A phandle and power domain specifier as defined by bindings
> + of power controller specified by phandle.
> +
> +Example:
> +
> + leaky-device@12350000 {
> + compatible = "foo,i-leak-current";
> + reg = <0x12350000 0x1000>;
> + power-domain = <&power 0>;
> + };
> +
> +The node above defines a typical power domain consumer device, which is located
> +inside power domain with index 0 of power controller represented by node with
> +label "power".
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index bfb8955..6d47498 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -3,12 +3,16 @@
> *
> * Copyright (C) 2011 Rafael J. Wysocki <rjw@sisk.pl>, Renesas Electronics Corp.
> *
> + * Support for Device Tree based power domain providers:
> + * Copyright (C) 2014 Tomasz Figa <tomasz.figa@gmail.com>
> + *
> * This file is released under the GPLv2.
> */
>
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/io.h>
> +#include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/pm_domain.h>
> #include <linux/pm_qos.h>
> @@ -2177,3 +2181,338 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
> list_add(&genpd->gpd_list_node, &gpd_list);
> mutex_unlock(&gpd_list_lock);
> }
> +
> +#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
> +/*
> + * DEVICE TREE BASED POWER DOMAIN PROVIDERS
> + *
> + * The code below implements generic device tree based power domain providers
> + * that bind device tree nodes with generic power domains registered in the
> + * system.
> + *
> + * Any driver that registers generic power domains and need to support binding
> + * of devices to these domains is supposed to register a power domain provider,
> + * which maps a power domain specifier retrieved from device tree to a power
> + * domain.
> + *
> + * Two simple mapping functions have been provided for convenience:
> + * - of_genpd_xlate_simple() for 1:1 device tree node to domain mapping,
> + * - of_genpd_xlate_onecell() for mapping of multiple domains per node
> + * by index.
> + */
> +
> +/**
> + * struct of_genpd_provider - Power domain provider registration structure
> + * @link: Entry in global list of domain providers
> + * @node: Pointer to device tree node of domain provider
> + * @xlate: Provider-specific xlate callback mapping a set of specifier cells
> + * into a power domain.
> + * @data: context pointer to be passed into @xlate callback
> + */
> +struct of_genpd_provider {
> + struct list_head link;
> +
> + struct device_node *node;
> + genpd_xlate_t xlate;
> + void *data;
> +};
> +
> +/* List of registered power domain providers. */
> +static LIST_HEAD(of_genpd_providers);
> +/* Mutex to protect the list above. */
> +static DEFINE_MUTEX(of_genpd_mutex);
> +
> +/**
> + * of_genpd_lock() - Lock access to of_genpd_providers list
> + */
> +static void of_genpd_lock(void)
> +{
> + mutex_lock(&of_genpd_mutex);
> +}
> +
> +/**
> + * of_genpd_unlock() - Unlock access to of_genpd_providers list
> + */
> +static void of_genpd_unlock(void)
> +{
> + mutex_unlock(&of_genpd_mutex);
> +}
> +
> +/**
> + * of_genpd_xlate_simple() - Xlate function for direct node-domain mapping
> + * @genpdspec: OF phandle args to map into a power domain
> + * @data: xlate function private data - pointer to struct generic_pm_domain
> + *
> + * This is a generic xlate function that can be used to model power domains
> + * that have their own device tree nodes. The private data of xlate function
> + * needs to be a valid pointer to struct generic_pm_domain.
> + */
> +struct generic_pm_domain *of_genpd_xlate_simple(
> + struct of_phandle_args *genpdspec,
> + void *data)
> +{
> + if (genpdspec->args_count != 0)
> + return ERR_PTR(-EINVAL);
> + return data;
> +}
> +EXPORT_SYMBOL_GPL(of_genpd_xlate_simple);
> +
> +/**
> + * of_genpd_xlate_onecell() - Xlate function for providers using single index.
> + * @genpdspec: OF phandle args to map into a power domain
> + * @data: xlate function private data - pointer to struct genpd_onecell_data
> + *
> + * This is a generic xlate function that can be used to model simple power
> + * domain controllers that have one device tree node and provide multiple
> + * power domains. A single cell is used as an index to an array of power
> + * domains specified in genpd_onecell_data struct when registering the
> + * provider.
> + */
> +struct generic_pm_domain *of_genpd_xlate_onecell(
> + struct of_phandle_args *genpdspec,
> + void *data)
> +{
> + struct genpd_onecell_data *genpd_data = data;
> + unsigned int idx = genpdspec->args[0];
> +
> + if (genpdspec->args_count != 1)
> + return ERR_PTR(-EINVAL);
> +
> + if (idx >= genpd_data->domain_num) {
> + pr_err("%s: invalid domain index %d\n", __func__, idx);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + return genpd_data->domains[idx];
> +}
> +EXPORT_SYMBOL_GPL(of_genpd_xlate_onecell);
> +
> +/**
> + * of_genpd_add_provider() - Register a domain provider for a node
> + * @np: Device node pointer associated with domain provider
> + * @genpd_src_get: callback for decoding domain
> + * @data: context pointer for @genpd_src_get callback.
> + */
> +int of_genpd_add_provider(struct device_node *np, genpd_xlate_t xlate,
> + void *data)
> +{
> + struct of_genpd_provider *cp;
> +
> + cp = kzalloc(sizeof(struct of_genpd_provider), GFP_KERNEL);
> + if (!cp)
> + return -ENOMEM;
> +
> + cp->node = of_node_get(np);
> + cp->data = data;
> + cp->xlate = xlate;
> +
> + of_genpd_lock();
> + list_add(&cp->link, &of_genpd_providers);
> + of_genpd_unlock();
> + pr_debug("Added domain provider from %s\n", np->full_name);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_genpd_add_provider);
> +
> +/**
> + * of_genpd_del_provider() - Remove a previously registered domain provider
> + * @np: Device node pointer associated with domain provider
> + */
> +void of_genpd_del_provider(struct device_node *np)
> +{
> + struct of_genpd_provider *cp;
> +
> + of_genpd_lock();
> + list_for_each_entry(cp, &of_genpd_providers, link) {
> + if (cp->node == np) {
> + list_del(&cp->link);
> + of_node_put(cp->node);
> + kfree(cp);
> + break;
> + }
> + }
> + of_genpd_unlock();
> +}
> +EXPORT_SYMBOL_GPL(of_genpd_del_provider);
> +
> +/* See of_genpd_get_from_provider(). */
> +static struct generic_pm_domain *__of_genpd_get_from_provider(
> + struct of_phandle_args *genpdspec)
> +{
> + struct of_genpd_provider *provider;
> + struct generic_pm_domain *genpd = ERR_PTR(-ENOENT);
> +
> + /* Check if we have such a provider in our array */
> + list_for_each_entry(provider, &of_genpd_providers, link) {
> + if (provider->node == genpdspec->np)
> + genpd = provider->xlate(genpdspec, provider->data);
> + if (!IS_ERR(genpd))
> + break;
> + }
> +
> + return genpd;
> +}
> +
> +/**
> + * of_genpd_get_from_provider() - Look-up power domain
> + * @genpdspec: OF phandle args to use for look-up
> + *
> + * Looks for domain provider under node specified by @genpdspec and if found
> + * uses xlate function of the provider to map phandle args to a power domain.
> + *
> + * Returns a valid pointer to struct generic_pm_domain on success or ERR_PTR()
> + * on failure.
> + */
> +static struct generic_pm_domain *of_genpd_get_from_provider(
> + struct of_phandle_args *genpdspec)
> +{
> + struct generic_pm_domain *genpd;
> +
> + of_genpd_lock();
> + genpd = __of_genpd_get_from_provider(genpdspec);
> + of_genpd_unlock();
> +
> + return genpd;
> +}
> +
> +/*
> + * DEVICE<->DOMAIN BINDING USING DEVICE TREE LOOK-UP
> + *
> + * The code below registers a notifier for platform bus devices'
> + * BUS_NOTIFY_BIND_DRIVER events and tries to attach devices to their power
> + * domains by looking them up using Device Tree.
> + *
> + * Similarly in BUS_NOTIFY_UNBOUND_DRIVER the device is detached from its
> + * domain, since it no longer supports runtime PM without any driver bound
> + * to it.
> + *
> + * Both generic and legacy Samsung-specific DT bindings are supported to
> + * keep backwards compatibility with existing DTBs.
> + */
> +
> +/**
> + * of_genpd_add_to_domain - Bind device to its power domain using Device Tree.
> + * @dev: Device to bind to its power domain.
> + *
> + * Tries to parse power domain specifier from device's OF node and if succeeds
> + * attaches the device to retrieved power domain.
> + *
> + * Returns 0 on success or negative error code otherwise.
> + */
> +static int of_genpd_add_to_domain(struct device *dev)
> +{
> + struct of_phandle_args pd_args;
> + struct generic_pm_domain *pd;
> + int ret;
> +
> + ret = of_parse_phandle_with_args(dev->of_node, "power-domain",
> + "#power-domain-cells", 0, &pd_args);
> + if (ret < 0) {
> + if (ret != ENOENT)
> + return ret;
> +
> + /*
> + * Try legacy Samsung-specific bindings
> + * (for backwards compatibility of DT ABI)
> + */
> + pd_args.args_count = 0;
> + pd_args.np = of_parse_phandle(dev->of_node,
> + "samsung,power-domain", 0);
> + if (!pd_args.np)
> + return 0;
> + }
> +
> + pd = of_genpd_get_from_provider(&pd_args);
> + if (IS_ERR(pd))
> + return PTR_ERR(pd);
> +
> + dev_dbg(dev, "adding to power domain %s\n", pd->name);
> +
> + while (1) {
> + ret = pm_genpd_add_device(pd, dev);
Since pm_genpd_add_device is used here, no gpd_timing_data can be
provided. Do you have a plan to solve this? Should the timing data be
provided from the device tree?
> + if (ret != -EAGAIN)
> + break;
> + cond_resched();
> + }
> +
> + if (!ret)
> + pm_genpd_dev_need_restore(dev, true);
> +
> + return ret;
> +}
[...]
regards
Philipp
^ permalink raw reply
* Re: [PATCH v6 00/18] ahci: library-ise ahci_platform, add sunxi driver and cleanup imx driver
From: Tejun Heo @ 2014-02-19 16:25 UTC (permalink / raw)
To: Hans de Goede
Cc: Maxime Ripard, Oliver Schinagl, Richard Zhu, Roger Quadros,
Lee Jones, linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <5304CD33.1080704-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Hello, Hans.
On Wed, Feb 19, 2014 at 04:26:43PM +0100, Hans de Goede wrote:
> The devres usage are really 2 different issues:
>
> 1) There is the issue that we're getting clocks by index (this is by design as
> clk-names are not generic), but there is no devm_get_clk_by_index (or some
> such), this is a shortcoming of the clk-core, which needs a separate patch
> to fix. I would rather not delay this patch-set based on getting a patch
> into another subsys.
Yes, that'd be my usual preference too. The only reason I'm hesitant
is because nobody seems to be too interested in long term aspect of
the code base and the only leverage I have seems to be rejecting
drivers until things become right.
> Note that even with devm_get_clk_by_index we can unfortunately not get rid of
> ahci_platform_put_resources() as in Roger's follow-up patches it gets used for
> putting some runtime pm related resources too.
>
> I guess this argues for simply turning ahci_platform_get_resources into a
> devm_ahci_platform_get_resources doing its own devm handling, and then
> making ahci_platform_put_resources a private function called by the devm
> framework. If you agree I can do that for the next patch-set.
Note that libata core already does something similar. Please take a
look at ata_host_alloc() for details. You don't really need to create
a separate devm_ prefixed variant. Just making the function register
devres automatically should be enough. If this is too much work, I'm
okay with deferring it until later if you promise to do it soonish.
> 2) In some comments you also seem to want devm variants of enable / disable
> resources, or at least have ahci_platform_put_resources do the disable
> automatically. The problem is that most of the functions called here need to
> be balanced, they increment / decrement usage counters in the clk / regulator
> subsystems, so we cannot simply unconditional do an ahci_platform_disable_resources
> in ahci_platform_put_resources
>
> Doing the disable automatically requires tracking the enable state, and doing this
> per resource, since the whole idea of having a separate ahci_platform_enable_resources
> is that some drivers may want to override its behavior doing things in a different
> order.
>
> To ensure proper tracking we would then need to offer ahci_platform_enable_regulator,
> etc. functions, and ensure that all drivers using the ahci_platform framework
> always go through these, rather then calling directly into the relevant framework.
>
> This is all doable, and I'm not against doing this but before spending a couple of
> hours coding this all up, I would like to hear back from you whether you would like
> to see this, or would rather keep things as is.
Yes, in principle, I want every resource used by ahci_platform to be
devres managed. I *think* devres should be flexible enough to handle
what you're describing (each devres resource can have data for state
tracking and devres resources can be looked up and modified, so the
different orders should be okay) but if not I'd be happy to help
extend it so that it can.
I'm not sure how many more ahci_platform drivers we'll get but the
rate seems pretty high in recent months, so I think it's worthwhile to
provide full devres coverage even if that complicates ahci_platform a
bit if it can simplify leaf drivers. Again, if you commit to doing it
in the foreseeable future, I'm okay with proceeding as-is, but it
needs to happen one way or another.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH RFC v4 3/3] Documentation: arm: define DT idle states bindings
From: Sebastian Capella @ 2014-02-19 16:04 UTC (permalink / raw)
To: devicetree
Cc: Mark Rutland, Mike Turquette, Tomasz Figa, Mark Hambleton,
Lorenzo Pieralisi, Russell King, Nicolas Pitre, Daniel Lezcano,
linux-arm-kernel, Grant Likely, Dave Martin, Charles Garcia Tobin,
Kevin Hilman, linux-pm, Kumar Gala, Rob Herring, Vincent Guittot,
Antti Miettinen, Peter De Schrijver, Stephen Boyd, Amit Kucheria,
Mark Brown, Santosh Shilimkar <santosh.sh>
In-Reply-To: <1392724051-11950-4-git-send-email-lorenzo.pieralisi@arm.com>
Quoting Lorenzo Pieralisi (2014-02-18 03:47:31)
> + - index
> + Usage: Required
> + Value type: <u32>
> + Definition: It represents the idle state index.
> + An increasing index value implies less power
> + consumption. Index must be given a sequential
> + value = {0, 1, ....}, starting from 0.
One minor comment. In the example, it can be tricky to see how this is sequential
since the states interleave. Not sure if it merits rewording here?
These look good to me!
Thanks!
Sebastian
^ permalink raw reply
* Re: [PATCH v4 07/13] ARM: dts: mvebu: Add a new set of registers to the PMSU node
From: Thomas Petazzoni @ 2014-02-19 16:00 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Daniel Lezcano, Rafael J. Wysocki,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Lorenzo Pieralisi, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Lior Amsalem, Tawfik Bayouk,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nadav Haklai, Ezequiel Garcia,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1392312816-17657-8-git-send-email-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
Dear Gregory CLEMENT,
On Thu, 13 Feb 2014 18:33:30 +0100, Gregory CLEMENT wrote:
> - reg: Should contain PMSU registers location and length. First pair
> - for the per-CPU SW Reset Control registers, second pair for the
> - Power Management Service Unit.
> + for the per-CPU SW Reset Control registers, second pair for the CPU
> + Power Management Service Unit registers, third pair for the Fabric Power
> + Management Service Unit registers.
>
> Example:
>
> -armada-370-xp-pmsu@d0022000 {
> +armada-370-xp-pmsu@22000 {
> compatible = "marvell,armada-370-xp-pmsu";
> - reg = <0xd0022100 0x430>,
> - <0xd0020800 0x20>;
> + reg = <0x22100 0x430>,
> + <0x20800 0x20>,
> + <0x22000 0x24>;
I am not too happy with this because:
*) We have to remove the second register pair from the PMSU. I haven't
yet posted the patches on LAKML for SMP on 375 and 38x, but the 375
doesn't have a PMSU. It however has the same CPU reset control
registers as 370 and XP. Therefore, these 0x20800 registers have to
be handled by a separate driver (that uses the reset framework),
and not handled by the PMSU driver. This way, Armada 370/XP can use
PMSU+CPU reset, while 375 will only use CPU reset.
*) I think making the PMSU register start at 0x22100 was a mistake.
The PMSU registers actually start at 0x22000 and they seem to go all
the way to 0x23000. So we should have only one register pair. This
of course raises some backward compatibility questions for the DT.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 1/7] Documentation: Add device tree bindings for Freescale i.MX GPC
From: Philipp Zabel @ 2014-02-19 16:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Tomasz Figa, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Shawn Guo, Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, Rob Herring, Thomas Abraham,
Kyungmin Park, Kukjin Kim
In-Reply-To: <2846176.fTzGkcQEPX@wuerfel>
Am Mittwoch, den 19.02.2014, 15:56 +0100 schrieb Arnd Bergmann:
> On Wednesday 19 February 2014 15:51:21 Tomasz Figa wrote:
> >
> > Just wanted to share a link with you.
> >
> > http://www.spinics.net/lists/devicetree/msg18051.html
>
> Excellent, thanks for the heads-up.
>
> Philipp, please review that patch and see if you can build on top of that
> for your work.
Thanks, I'll do that.
regards
Philipp
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/3] usb: chipidea: msm: Add device tree support
From: Ivan T. Ivanov @ 2014-02-19 15:48 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Peter Chen, Grant Likely, Rob Herring, Greg Kroah-Hartman,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5303A71B.7060208-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
Hi,
On Tue, 2014-02-18 at 21:31 +0300, Sergei Shtylyov wrote:
> On 02/18/2014 08:14 PM, Ivan T. Ivanov wrote:
>
> >>> From: "Ivan T. Ivanov" <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
>
> >>> Allows controller to be specified via device tree.
> >>> Pass PHY phandle specified in DT to core driver.
>
> >>> Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
> >>> ---
> >>> drivers/usb/chipidea/ci_hdrc_msm.c | 23 ++++++++++++++++++++++-
> >>> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> >> You also need to describe the binding you're creating in
> >> Documentation/devicetree/bindings/usb/<file>.txt.
>
> > Did you check "[PATCH v2 1/3]"?
>
> Did you send it to 'linux-usb'?
Opps, sorry. get_maintainer.pl did not list linux-usb and I forgot
to add it. Would like to resend it?
Regards,
Ivan
>
> WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/3] usb: chipidea: msm: Add device tree binding information
From: Ivan T. Ivanov @ 2014-02-19 15:43 UTC (permalink / raw)
To: Courtney Cavin
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Rob Landley, Greg Kroah-Hartman, David Brown,
devicetree@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
In-Reply-To: <20140218212659.GL1706@sonymobile.com>
Hi,
On Tue, 2014-02-18 at 13:26 -0800, Courtney Cavin wrote:
> On Tue, Feb 18, 2014 at 02:21:19PM +0100, Ivan T. Ivanov wrote:
> > From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> >
> > Document device tree binding information as required by
> > the Qualcomm USB controller.
> >
> > Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
> > ---
> > .../devicetree/bindings/usb/msm-hsusb.txt | 17 +++++++++++++++++
>
> Although you mentioned to Josh that this is intended for "non-standard"
> Chipidea properties, I don't see any other than requiring that 'dr_mode'
> must be "peripheral". It would seem that this should all be integrated
> into a ci3xxx.txt.
Hm, there is no ci3xxx.txt. The closest match is ci-hdrc-imx.txt.
So it could be ci-hdrc-qcom.txt or my preferred name qcom,ci-hdrc.txt?
> > +CI13xxx (Chipidea) USB controllers
> > +
> > +Required properties:
> > +- compatible: should contain "qcom,ci-hdrc"
>
> Is there nothing more specific you could put here? Maybe a hardware
> revision, or something?
>
I am not aware of any version numbers here, sorry.
> > +- reg: offset and length of the register set in the memory map
> > +- interrupts: interrupt-specifier for the controller interrupt.
> > +- usb-phy: phandle for the PHY device
> > +- dr_mode: Sould be "peripheral"
>
> s/Sould/should/
Thanks,
Ivan
>
> -Courtney
^ permalink raw reply
* Re: [PATCH v4 2/4] of: reimplement the matching method for __of_match_node()
From: Grant Likely @ 2014-02-19 15:34 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Kevin Hao, Rob Herring, Sebastian Hesselbarth
In-Reply-To: <CAL_JsqLbjK02Z1qzfP9K+Grp2WwAth8rMctK9myf8ZwJA6PUGQ@mail.gmail.com>
On Wed, 19 Feb 2014 08:54:57 -0600, Rob Herring <robherring2@gmail.com> wrote:
> On Wed, Feb 19, 2014 at 8:14 AM, Grant Likely <grant.likely@linaro.org> wrote:
> > From: Kevin Hao <haokexin@gmail.com>
> >
> > In the current implementation of __of_match_node(), it will compare
> > each given match entry against all the node's compatible strings
> > with of_device_is_compatible().
> >
> > To achieve multiple compatible strings per node with ordering from
> > specific to generic, this requires given matches to be ordered from
> > specific to generic. For most of the drivers this is not true and
> > also an alphabetical ordering is more sane there.
> >
> > Therefore, we define a following priority order for the match, and
> > then scan all the entries to find the best match.
> > 1. specific compatible && type && name
> > 2. specific compatible && type
> > 3. specific compatible && name
> > 4. specific compatible
> > 5. general compatible && type && name
> > 6. general compatible && type
> > 7. general compatible && name
> > 8. general compatible
> > 9. type && name
> > 10. type
> > 11. name
>
> While I agree this should be the right order, I worry that this may be
> changing the matching in some case as all previous attempts have done.
True, but they should be few and far between, and they can be
fixed up with bugfixes in drivers. This time we actually have test cases
that show the behaviour is correct. The previous versions failed the
current test cases (I checked).
> > + /* Compatible match has highest priority */
> > + if (compat && compat[0]) {
> > + cp = __of_get_property(device, "compatible", &cplen);
> > + if (!cp)
> > + return 0;
> > + for (index = 0; cplen > 0; index++, cp += l, cplen -= l) {
> > + l = strlen(cp) + 1;
>
> This could use of_property_for_each_string.
Good point. Changed.
g.
^ permalink raw reply
* Re: [PATH v2 2/2] spi: Add Qualcomm QUP SPI controller support
From: Mark Brown @ 2014-02-19 15:28 UTC (permalink / raw)
To: Ivan T. Ivanov
Cc: Grant Likely, Rob Herring, linux-spi, linux-arm-msm, linux-kernel,
devicetree, Alok Chauhan, Gilad Avidov, Kiran Gunda, Sagar Dharia,
dsneddon
In-Reply-To: <1392308498-30209-1-git-send-email-iivanov@mm-sol.com>
[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]
On Thu, Feb 13, 2014 at 06:21:38PM +0200, Ivan T. Ivanov wrote:
> Qualcomm Universal Peripheral (QUP) core is an AHB slave that
> provides a common data path (an output FIFO and an input FIFO)
> for serial peripheral interface (SPI) mini-core. SPI in master
> mode supports up to 50MHz, up to four chip selects, programmable
> data path from 4 bits to 32 bits and numerous protocol variants.
I've applied this since it mostly looks good but there's a few small
outstanding issues, please submit incremental patches fixing them.
> +#define SPI_CONFIG 0x0300
> +#define SPI_IO_CONTROL 0x0304
> +#define SPI_ERROR_FLAGS 0x0308
> +#define SPI_ERROR_FLAGS_EN 0x030c
You've got lots of defines that are just plain SPI_ which is looking for
namespace collisions - please prefix them.
> + if (spi->chip_select >= spi->master->num_chipselect) {
> + dev_err(controller->dev, "invalid chip_select %d\n",
> + spi->chip_select);
> + return -EINVAL;
> + }
The core does this prior to allowing the slave to be registered at all.
> +
> + if (spi->max_speed_hz > controller->max_speed_hz) {
> + dev_err(controller->dev, "invalid max_speed_hz %d\n",
> + spi->max_speed_hz);
> + return -EINVAL;
> + }
The core will check this for you if you set master->max_speed_hz.
> +#ifdef CONFIG_PM_RUNTIME
> +static int spi_qup_pm_suspend_runtime(struct device *device)
> +{
> + struct spi_master *master = dev_get_drvdata(device);
> + struct spi_qup *controller = spi_master_get_devdata(master);
> + u32 config;
> +
> + /* Enable clocks auto gaiting */
> + config = readl(controller->base + QUP_CONFIG);
> + config |= QUP_CLOCK_AUTO_GATE;
> + writel_relaxed(config, controller->base + QUP_CONFIG);
> + return 0;
> +}
Why not just enable this all the time? I'd have expected some clock API
calls here similar to those in the main suspend and resume paths.
> +MODULE_VERSION("0.4");
Don't bother doing this, nobody will ever keep updating the version
number - the kernel version should be enough.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 00/18] ahci: library-ise ahci_platform, add sunxi driver and cleanup imx driver
From: Hans de Goede @ 2014-02-19 15:26 UTC (permalink / raw)
To: Tejun Heo
Cc: Maxime Ripard, Oliver Schinagl, Richard Zhu, Roger Quadros,
Lee Jones, linux-ide-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20140219150249.GG10134-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
Hi,
On 02/19/2014 04:02 PM, Tejun Heo wrote:
> Hello,
>
> On Wed, Feb 19, 2014 at 01:01:42PM +0100, Hans de Goede wrote:
>> Here is v6 of my patchset for adding ahci-sunxi support. This has been
>> tested with Allwinner A10, Allwinner A20 and Freeware imx6x SoCs, including
>> suspend / resume. Note that since my last revision the ahci_imx driver has
>> also grown imx53 sata support, it would be good if some-one could test that
>> with this series.
>
> Stopping review here. I really like where it's headed in general and
> most review points,
Thanks for the review. I'll respin the patchset taking the various remarks into
account. I hope to have a new version ready at the end of the coming weekend
at the latest.
> except for lack of devres usage, are rather
> cosmetic. I'm not completely decided yet whether we can defer devres
> until later or should go for it in the first round.
The devres usage are really 2 different issues:
1) There is the issue that we're getting clocks by index (this is by design as
clk-names are not generic), but there is no devm_get_clk_by_index (or some
such), this is a shortcoming of the clk-core, which needs a separate patch
to fix. I would rather not delay this patch-set based on getting a patch
into another subsys.
Note that even with devm_get_clk_by_index we can unfortunately not get rid of
ahci_platform_put_resources() as in Roger's follow-up patches it gets used for
putting some runtime pm related resources too.
I guess this argues for simply turning ahci_platform_get_resources into a
devm_ahci_platform_get_resources doing its own devm handling, and then
making ahci_platform_put_resources a private function called by the devm
framework. If you agree I can do that for the next patch-set.
2) In some comments you also seem to want devm variants of enable / disable
resources, or at least have ahci_platform_put_resources do the disable
automatically. The problem is that most of the functions called here need to
be balanced, they increment / decrement usage counters in the clk / regulator
subsystems, so we cannot simply unconditional do an ahci_platform_disable_resources
in ahci_platform_put_resources
Doing the disable automatically requires tracking the enable state, and doing this
per resource, since the whole idea of having a separate ahci_platform_enable_resources
is that some drivers may want to override its behavior doing things in a different
order.
To ensure proper tracking we would then need to offer ahci_platform_enable_regulator,
etc. functions, and ensure that all drivers using the ahci_platform framework
always go through these, rather then calling directly into the relevant framework.
This is all doable, and I'm not against doing this but before spending a couple of
hours coding this all up, I would like to hear back from you whether you would like
to see this, or would rather keep things as is.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v3 15/15] mfd: max14577: Add device tree bindings document
From: Tomasz Figa @ 2014-02-19 15:17 UTC (permalink / raw)
To: Krzysztof Kozlowski, MyungJoo Ham, Chanwoo Choi, Samuel Ortiz,
Lee Jones, Mark Brown, linux-kernel, linux-arm-kernel
Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz, Kyungmin Park,
Dmitry Eremin-Solenikov, David Woodhouse, devicetree, Rob Herring,
Pawel Moll, Mark Rutland
In-Reply-To: <1392627950-26927-16-git-send-email-k.kozlowski@samsung.com>
Hi,
On 17.02.2014 10:05, Krzysztof Kozlowski wrote:
> Add document describing device tree bindings for MAX14577 MFD
> drivers: MFD core, extcon, regulator and charger.
>
> Both MAX14577 and MAX77836 chipsets are documented.
>
> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Tomasz Figa <t.figa@samsung.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: devicetree@vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> ---
> Documentation/devicetree/bindings/mfd/max14577.txt | 149 ++++++++++++++++++++
> 1 file changed, 149 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/max14577.txt
OK, so the "maxim,max14577" compatible string somehow showed up in the
kernel without any documentation, interesting...
Anyway:
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox