* Re: [PATCH] net: add init-regs for of_phy support
From: Ben Dooks @ 2014-02-17 13:53 UTC (permalink / raw)
To: Mark Rutland
Cc: netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-sh@vger.kernel.org
In-Reply-To: <20140217134448.GA19308@e106331-lin.cambridge.arm.com>
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 think allocating a compatible string and handling it in the kernel is
> better than having arbitrary register poke values in the dt.
>> + prop = of_find_property(of_node, "init-regs", &len);
>> + if (prop) {
>> + if (len % (sizeof(__be32) * 3)) {
>> + dev_err(dev, "init-regs not multiple of 3 entries\n");
>> + return -EINVAL;
>> + }
>> +
>> + ptr = of_prop_next_u32(prop, ptr, ®);
>> + while (ptr != NULL) {
>> + ptr = of_prop_next_u32(prop, ptr, ®);
oops, bad rebase fixup, this should have been deleted.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply
* Re: [PATCH v6 4/8] ARM: sunxi: Add driver for SD/MMC hosts found on Allwinner sunxi SoCs
From: Stefan Monnier @ 2014-02-17 14:01 UTC (permalink / raw)
To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-mmc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1744603.zMUgfmoUUP__34248.3586668552$1392621049$gmane$org@pagira.o2s.ch>
> I'm not really into this licensing and copyright stuff.
> Could you maybe point out what the correct copyright would be?
The dates are when the code is disclosed to the public. So 2014 would
be fine, 2013-2014 might be as well, but 2014-2015 can't be right until
next year.
Stefan
^ permalink raw reply
* [PATCH 1/2] devicetree: Add Xilinx XADC binding documentation
From: Lars-Peter Clausen @ 2014-02-17 14:10 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA, Lars-Peter Clausen, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
devicetree-u79uwXL29TY76Z2rM5mHXA
The Xilinx XADC is a ADC that can be found in the series 7 FPGAs from Xilinx.
The XADC has a DRP interface for communication. Currently two different
frontends for the DRP interface exist. One that is only available on the ZYNQ
family as a hardmacro in the SoC portion of the ZYNQ. The other one is available
on all series 7 platforms and is a softmacro with a AXI interface. This binding
document describes the bindings for both of them since the bindings are very
similar.
Each of them needs:
* A address range where the registers are mapped
* An interrupt number for the device interrupt
* A clock. For the the ZYNQ hardmacro interface this is the modules PCAP
clock, for the AXI softmacro it is the AXI bus interface clock.
Additionally the bindings specify whether an external multiplexer is used and in
which mode it is used. The devicetree bindings also describe which external
channels are connected and in which configuration.
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
---
Changes since v1:
* Remove duplicated paragraph in the description
---
.../devicetree/bindings/iio/adc/xilinx-xadc.txt | 113 +++++++++++++++++++++
1 file changed, 113 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
diff --git a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
new file mode 100644
index 0000000..d9ee909
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
@@ -0,0 +1,113 @@
+Xilinx XADC device driver
+
+This binding document describes the bindings for both of them since the
+bindings are very similar. The Xilinx XADC is a ADC that can be found in the
+series 7 FPGAs from Xilinx. The XADC has a DRP interface for communication.
+Currently two different frontends for the DRP interface exist. One that is only
+available on the ZYNQ family as a hardmacro in the SoC portion of the ZYNQ. The
+other one is available on all series 7 platforms and is a softmacro with a AXI
+interface. This binding document describes the bindings for both of them since
+the bindings are very similar.
+
+Required properties:
+ - compatible: Should be one of
+ * "xlnx,zynq-xadc-1.00.a": When using the ZYNQ device
+ configuration interface to interface to the XADC hardmacro.
+ * "xlnx,axi-xadc-1.00.a": When using the axi-xadc pcore to
+ interface to the XADC hardmacro.
+ - reg: Address and length of the register set for the device
+ - interrupts: Interrupt for the XADC control interface.
+ - clocks: When using the ZYNQ this must be the ZYNQ PCAP clock,
+ when using the AXI-XADC pcore this must be the clock that provides the
+ clock to the AXI bus interface of the core.
+
+Optional properties:
+ - interrupt-parent: phandle to the parent interrupt controller
+ - xlnx,external-mux:
+ * "none": No external multiplexer is used, this is the default
+ if the property is omitted.
+ * "single": External multiplexer mode is used with one
+ multiplexer.
+ * "dual": External multiplexer mode is used with two
+ multiplexers for simultaneous sampling.
+ - xlnx,external-mux-channel: Configures which pair of pins is used to
+ sample data in external mux mode.
+ Valid values for single external multiplexer mode are:
+ 0: VP/VN
+ 1: VAUXP[0]/VAUXN[0]
+ 2: VAUXP[1]/VAUXN[1]
+ ...
+ 16: VAUXP[15]/VAUXN[15]
+ Valid values for dual external multiplexer mode are:
+ 1: VAUXP[0]/VAUXN[0] - VAUXP[8]/VAUXN[8]
+ 2: VAUXP[1]/VAUXN[1] - VAUXP[9]/VAUXN[9]
+ ...
+ 8: VAUXP[7]/VAUXN[7] - VAUXP[15]/VAUXN[15]
+
+ This property needs to be present if the device is configured for
+ external multiplexer mode (either single or dual). If the device is
+ not using external multiplexer mode the property is ignored.
+ - xnlx,channels: List of external channels that are connected to the ADC
+ Required properties:
+ * #address-cells: Should be 1.
+ * #size-cells: Should be 0.
+
+ The child nodes of this node represent the external channels which are
+ connected to the ADC. If the property is no present no external
+ channels will be assumed to be connected.
+
+ Each child node represents one channel and has the following
+ properties:
+ Required properties:
+ * reg: Pair of pins the the channel is connected to.
+ 0: VP/VN
+ 1: VAUXP[0]/VAUXN[0]
+ 2: VAUXP[1]/VAUXN[1]
+ ...
+ 16: VAUXP[15]/VAUXN[15]
+ Note each channel number should only be used at most
+ once.
+ Optional properties:
+ * xlnx,bipolar: If set the channel is used in bipolar
+ mode.
+
+
+Examples:
+ xadc@f8007100 {
+ compatible = "xlnx,zynq-xadc-1.00.a";
+ reg = <0xf8007100 0x20>;
+ interrupts = <0 7 4>;
+ interrupt-parent = <&gic>;
+ clocks = <&pcap_clk>;
+
+ xlnx,channels {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ channel@0 {
+ reg = <0>;
+ };
+ channel@1 {
+ reg = <1>;
+ };
+ channel@8 {
+ reg = <8>;
+ };
+ };
+ };
+
+ xadc@43200000 {
+ compatible = "xlnx,axi-xadc-1.00.a";
+ reg = <0x43200000 0x1000>;
+ interrupts = <0 53 4>;
+ interrupt-parent = <&gic>;
+ clocks = <&fpga1_clk>;
+
+ xlnx,channels {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ channel@0 {
+ reg = <0>;
+ xlnx,bipolar;
+ };
+ };
+ };
--
1.8.0
^ permalink raw reply related
* Re: [PATCH] net: add init-regs for of_phy support
From: Mark Rutland @ 2014-02-17 14:12 UTC (permalink / raw)
To: Ben Dooks
Cc: netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-sh@vger.kernel.org
In-Reply-To: <530213A4.4030400@codethink.co.uk>
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.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v2 4/4] video: mmp: add device tree support
From: Mark Rutland @ 2014-02-17 14:37 UTC (permalink / raw)
To: Zhou Zhu
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Tomi Valkeinen, Jean-Christophe Plagniol-Villard, Haojian Zhuang,
Sascha Hauer, Jingoo Han,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Chao Xie,
Guoqing Li
In-Reply-To: <1389698184-28761-5-git-send-email-zzhu3-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
On Tue, Jan 14, 2014 at 11:16:24AM +0000, Zhou Zhu wrote:
> add device tree support for mmp fb/controller
> the description of DT config is at
> Documentation/devicetree/bindings/fb/mmp-disp.txt
>
> Signed-off-by: Zhou Zhu <zzhu3-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> ---
> Documentation/devicetree/bindings/fb/mmp-disp.txt | 60 ++++++++
> drivers/video/mmp/fb/mmpfb.c | 73 ++++++----
> drivers/video/mmp/hw/mmp_ctrl.c | 160 ++++++++++++++++-----
> 3 files changed, 235 insertions(+), 58 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/fb/mmp-disp.txt
>
> diff --git a/Documentation/devicetree/bindings/fb/mmp-disp.txt b/Documentation/devicetree/bindings/fb/mmp-disp.txt
> new file mode 100644
> index 0000000..80702f5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fb/mmp-disp.txt
> @@ -0,0 +1,60 @@
> +* Marvell MMP Display (MMP_DISP)
> +
> +To config mmp display, 3 parts are required to be set in dts:
> +1. mmp fb
> +Required properties:
> +- compatible: Should be "marvell,<soc>-fb".
Please list the precise values and when they should be used. It makes
searching for them _far_ easier.
> +- marvell,path: Should be the path this fb connecting to.
What type is this? The example implies a phandle.
What type of node does this point to?
It's not explained at this point and it's really unclear what this is.
> +- marvell,overlay-id: Should be the id of overlay this fb is on.
> +- marvell,dmafetch-id: Should be the dma fetch id this fb using.
Are these hardware properties?
Is there any documentation which could make this clearer?
> +- marvell,default-pixfmt: Should be the default pixel format when this fb is
> +turned on.
What format is this? The example is useless. If this is a standard
binding, please refer to the binding document.
> +
> +2. mmp controller
> +Required properties:
> +- compatible: Should be "marvell,<soc>-disp".
Please list the precise set of values.
> +- reg: Should be address and length of the register set for this controller.
> +- interrupts: Should be interrupt of this controller.
Just to check: the device only has on interrupt?
> +
> +Required sub-node:
> +- path:
> +Required properties in this sub-node:
> +-- marvell,overlay_num: Should be number of overlay this path has.
s/_/-/ in property names please.
num-overlays would be a clearer name.
> +-- marvell,output-type: Should be output-type settings
> +-- marvell,path-config: Should be path-config settings
> +-- marvell,link-config: Should be link-config settings
> +-- marvell,rbswap: Should be rbswap settings
These are completely opaque to me. Please describe what these are either
in place or in reference to standard bindings.
Is rbswap a boolean value?
[...]
> + if (!path_np || of_property_read_u32(path_np, "marvell,overlay-num",
> + &path_info->output_type) ||
> + of_property_read_u32(path_np, "marvell,output-type",
> + &path_info->overlay_num)) {
These are reading into the wrong variables.
Cheers,
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: [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Grant Likely @ 2014-02-17 15:54 UTC (permalink / raw)
To: Jason Gunthorpe, Jason Cooper
Cc: Arnd Bergmann, keescook, devicetree, Laura Abbott, linux-kernel,
Rob Herring, Kumar Gala, linux-arm-kernel
In-Reply-To: <20140212182000.GJ5554@obsidianresearch.com>
On Wed, 12 Feb 2014 11:20:00 -0700, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote:
> On Wed, Feb 12, 2014 at 12:45:54PM -0500, Jason Cooper wrote:
>
> > The bootloader would then load this file into ram, and pass the
> > address/size to the kernel either via dt, or commandline. kaslr (run in
> > the decompressor) would consume some of this randomness, and then
> > random.c would consume the rest in a non-crediting initialization.
>
> Sure is a neat idea, but I think in general it would probably be smart
> to include the entire FDT blob in the early random pool, that way you
> get MACs and other machine unique data too.
I applied a patch that did exactly that (109b623629), and then reverted
it (b920ecc82) shortly thereafter because add_device_randomness() is
a rather slow function and FDTs can get large. I'd like to see someone
do a reasonable analysis on the cost of using an FDT for randomness
before I reapply a patch doing something similar. An awful lot of the
FDT data is not very random, but there are certainly portions of it that
are appropriate for the random pool.
g.
^ permalink raw reply
* Re: [RFC/PATCH 0/3] Add devicetree scanning for randomness
From: Arnd Bergmann @ 2014-02-17 16:13 UTC (permalink / raw)
To: linux-arm-kernel
Cc: devicetree, Laura Abbott, Jason Cooper, linux-kernel,
Jason Gunthorpe, Rob Herring, Kumar Gala, Grant Likely, keescook
In-Reply-To: <20140217155419.682F7C401D4@trevor.secretlab.ca>
On Monday 17 February 2014 15:54:19 Grant Likely wrote:
> On Wed, 12 Feb 2014 11:20:00 -0700, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote:
> > On Wed, Feb 12, 2014 at 12:45:54PM -0500, Jason Cooper wrote:
> >
> > > The bootloader would then load this file into ram, and pass the
> > > address/size to the kernel either via dt, or commandline. kaslr (run in
> > > the decompressor) would consume some of this randomness, and then
> > > random.c would consume the rest in a non-crediting initialization.
> >
> > Sure is a neat idea, but I think in general it would probably be smart
> > to include the entire FDT blob in the early random pool, that way you
> > get MACs and other machine unique data too.
>
> I applied a patch that did exactly that (109b623629), and then reverted
> it (b920ecc82) shortly thereafter because add_device_randomness() is
> a rather slow function and FDTs can get large. I'd like to see someone
> do a reasonable analysis on the cost of using an FDT for randomness
> before I reapply a patch doing something similar. An awful lot of the
> FDT data is not very random, but there are certainly portions of it that
> are appropriate for the random pool.
Could we use a faster hash function that scans the entire device tree and
then just feed the output of that into add_device_randomness? We probably
can't expect that there is a lot of entropy in the DT blob, so the
result wouldn't be all that different in terms of quality of the random
seed.
Arnd
^ permalink raw reply
* [PATCH] of_mdio: fix phy interrupt passing
From: Ben Dooks @ 2014-02-17 16:29 UTC (permalink / raw)
To: grant.likely-QSEj5FYQhm4dnm+yROfE0A
Cc: linux-kernel-81qHHgoATdFT9dQujB1mzip2UmYkHbXO,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA,
sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8, Ben Dooks
The of_mdiobus_register_phy() is not setting phy->irq this causing
some drivers to incorrectly assume that the PHY does not have an
IRQ associated with it or install an interrupt handler for the
PHY.
Simplify the code setting irq and set the phy->irq at the same
time so that the case if mdio->irq is not NULL is easier to read.
This fixes the issue:
net eth0: attached PHY 1 (IRQ -1) to driver Micrel KSZ8041RNLI
to the correct:
net eth0: attached PHY 1 (IRQ 416) to driver Micrel KSZ8041RNLI
Signed-off-by: Ben Dooks <ben.dooks-4yDnlxn2s6sWdaTGBSpHTA@public.gmane.org>
---
drivers/of/of_mdio.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 875b7b6..7b3e7b0 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -44,7 +44,7 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
{
struct phy_device *phy;
bool is_c45;
- int rc, prev_irq;
+ int rc;
u32 max_speed = 0;
is_c45 = of_device_is_compatible(child,
@@ -55,11 +55,11 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
return 1;
if (mdio->irq) {
- prev_irq = mdio->irq[addr];
- mdio->irq[addr] =
- irq_of_parse_and_map(child, 0);
- if (!mdio->irq[addr])
- mdio->irq[addr] = prev_irq;
+ rc = irq_of_parse_and_map(child, 0);
+ if (rc > 0) {
+ mdio->irq[addr] = rc;
+ phy->irq = rc;
+ }
}
/* Associate the OF node with the device structure so it
--
1.8.5.3
--
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
* Re: [PATCH RFC v3 3/3] Documentation: arm: define DT idle states bindings
From: Lorenzo Pieralisi @ 2014-02-17 16:34 UTC (permalink / raw)
To: Mark Brown
Cc: Mark Rutland, Mike Turquette, Tomasz Figa, Mark Hambleton,
Russell King, Nicolas Pitre, Daniel Lezcano,
linux-arm-kernel@lists.infradead.org, grant.likely@linaro.org,
Dave P Martin, Charles Garcia-Tobin, devicetree@vger.kernel.org,
Kevin Hilman, linux-pm@vger.kernel.org, Kumar Gala, Rob Herring,
Vincent Guittot, Antti Miettinen, Peter De Schrijver,
Stephen Boyd, Amit
In-Reply-To: <20140216013956.GV4451@sirena.org.uk>
On Sun, Feb 16, 2014 at 01:39:56AM +0000, Mark Brown wrote:
[...]
> > + - cache-state-retained
> > + Usage: See definition
> > + Value type: <none>
> > + Definition: if present cache memory is retained on power down,
> > + otherwise it is lost.
>
> Might be better to define which caches?
I do not expect caches in the same power domain to have different retention
capabilities, so a flag per-state should be enough. If anyone is unhappy
about this please flag it up. List of caches affected can be retrieved by
walking the power-domain specifiers and check those against the caches power
domains.
Thanks,
Lorenzo
^ permalink raw reply
* Re: [PATCH v2 1/5] drivers: of: add initialization code for reserved memory
From: Grant Likely @ 2014-02-17 16:47 UTC (permalink / raw)
To: Tomasz Figa, Benjamin Herrenschmidt
Cc: Tomasz Figa, Marek Szyprowski, linux-kernel, linux-arm-kernel,
linaro-mm-sig, devicetree, linux-doc, Kyungmin Park,
Arnd Bergmann, Michal Nazarewicz, Sascha Hauer, Laura Abbott,
Rob Herring, Olof Johansson, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, Kumar Gala, Nishanth Peethambaran,
Marc, Josh Cartwright
In-Reply-To: <52FA8245.5050707@gmail.com>
On Tue, 11 Feb 2014 21:04:21 +0100, Tomasz Figa <tomasz.figa@gmail.com> wrote:
>
>
> On 11.02.2014 21:02, Benjamin Herrenschmidt wrote:
> > On Tue, 2014-02-11 at 19:01 +0000, Grant Likely wrote:
> >
> >>> except that the former IMHO better suits the definition of memory
> >>> region, which I see as a single contiguous range of memory and can be
> >>> simplified to have a single reg entry per region.
> >>
> >> My point is rather if multiple reg tuples are found in a reserved memory
> >> node, the kernel must respect them and reserve the memory. I'm not
> >> arguing about whether or not that makes for a good binding.
> >
> > agreed.
>
> My point is why, if the binding defines that just a single tuple should
> be provided.
It's irrelevant because it gets processed at a different level. It's
important that the core early setup code can quickly parse all the
reserved regions without having any idea what the end-user binding is.
Multiple reg tuples is just fine in this regard.
Whether or not multiple tuples makes sense is defined for each
particular use case by the driver actually using it without impacting
core initialization code one iota.
g.
^ permalink raw reply
* Re: [PATCH v2 1/5] drivers: of: add initialization code for reserved memory
From: Grant Likely @ 2014-02-17 16:53 UTC (permalink / raw)
To: Josh Cartwright, Tomasz Figa
Cc: Benjamin Herrenschmidt, Tomasz Figa, Marek Szyprowski,
linux-kernel, linux-arm-kernel, linaro-mm-sig, devicetree,
linux-doc, Kyungmin Park, Arnd Bergmann, Michal Nazarewicz,
Sascha Hauer, Laura Abbott, Rob Herring, Olof Johansson,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
Kumar Gala, Nishanth Peethambaran, Marc
In-Reply-To: <20140213194840.GO841@joshc.qualcomm.com>
On Thu, 13 Feb 2014 13:48:40 -0600, Josh Cartwright <joshc@codeaurora.org> wrote:
> On Tue, Feb 11, 2014 at 09:27:36PM +0100, Tomasz Figa wrote:
> > On 11.02.2014 21:19, Josh Cartwright wrote:
> > >On Tue, Feb 11, 2014 at 09:04:21PM +0100, Tomasz Figa wrote:
> > > >On 11.02.2014 21:02, Benjamin Herrenschmidt wrote:
> > > > >On Tue, 2014-02-11 at 19:01 +0000, Grant Likely wrote:
> > > > > > > except that the former IMHO better suits the definition of memory
> > > > > > > region, which I see as a single contiguous range of memory and can be
> > > > > > > simplified to have a single reg entry per region.
> > > > > >
> > > > > > My point is rather if multiple reg tuples are found in a reserved memory
> > > > > > node, the kernel must respect them and reserve the memory. I'm not
> > > > > > arguing about whether or not that makes for a good binding.
> > > > >
> > > > > agreed.
> > > >
> > > > My point is why, if the binding defines that just a single tuple should be
> > > > provided.
> > >
> > > FWIW, the usecase I had mentioned in reply to Grant in the patch 5/5
> > > thread [1] could make use of this. The shared memory region is split
> > > into a main chunk and several "auxiliary" chunk, but collectively these
> > > regions all share the same heap state.
> > >
> > > Josh
> > >
> > > 1: http://lkml.kernel.org/r/20140205192502.GO20228@joshc.qualcomm.com
> >
> > The use case seems fine, but I believe it could be properly represented in
> > device tree using multiple single-reg regions as well, unless the consumer
> > can request a block of memory that crosses boundary of two sub-regions
> > specified by reg entries of single region.
>
> I could probably make a only-one-reg-entry policy work for me, but it
> makes things a bit more awkward. I'd lose the ability to describe
> "this set of regions need to be logically handled together" directly in
> the reserved memory node, and would need to push it up a layer.
>
> reserved-memory {
> smem: smem {
> reg = <...>;
> };
> aux1: auxiliary1 {
> reg = <...>;
> };
> aux2: auxiliary2 {
> reg = <...>;
> };
> ...
> };
If the regions are used for different purposes, it makes sense I think
to have a separate node for each. Multiple tuples would make more sense
for something like valid DMA regions for a broken device that can only
DMA into a few windows; you could have one tuple per window within a
single node.
It would be possible to collect multiple associated nodes under one
parent node which in turn has reserved-memory for its parent:
reserved-memory {
ranges;
reserved-group {
ranges;
smem: smem {
reg = <...>;
};
aux1: auxiliary1 {
reg = <...>;
};
aux2: auxiliary2 {
reg = <...>;
};
};
...
};
g.
^ permalink raw reply
* Re: [PATCH] of_mdio: fix phy interrupt passing
From: Florian Fainelli @ 2014-02-17 17:26 UTC (permalink / raw)
To: Ben Dooks
Cc: Grant Likely, linux-kernel, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev, Linux-sh list,
Sergei Shtylyov
In-Reply-To: <1392654580-3706-1-git-send-email-ben.dooks@codethink.co.uk>
Hi Ben,
2014-02-17 8:29 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
> The of_mdiobus_register_phy() is not setting phy->irq this causing
> some drivers to incorrectly assume that the PHY does not have an
> IRQ associated with it or install an interrupt handler for the
> PHY.
>
> Simplify the code setting irq and set the phy->irq at the same
> time so that the case if mdio->irq is not NULL is easier to read.
The real bug fix, which is not properly explained here, is that
irq_of_parse_and_map() should return values > 0 when the interrupt is
valid, so this makes me wonder why we are not propagating the return
value from irq_of_parse_and_map() in case the call to
of_irq_parse_one() does return something non-zero?
Other than that, I agree with the resolution, but it deserves a better
commit message, and eventually doing this in two steps:
- first fix the bug by changing the if (!mdio->irq[addr]) into a if
(mdio->irq[addr] <= 0)
- second submit a patch which cleanups the assignment
>
> This fixes the issue:
> net eth0: attached PHY 1 (IRQ -1) to driver Micrel KSZ8041RNLI
>
> to the correct:
> net eth0: attached PHY 1 (IRQ 416) to driver Micrel KSZ8041RNLI
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> drivers/of/of_mdio.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 875b7b6..7b3e7b0 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -44,7 +44,7 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
> {
> struct phy_device *phy;
> bool is_c45;
> - int rc, prev_irq;
> + int rc;
> u32 max_speed = 0;
>
> is_c45 = of_device_is_compatible(child,
> @@ -55,11 +55,11 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
> return 1;
>
> if (mdio->irq) {
> - prev_irq = mdio->irq[addr];
> - mdio->irq[addr] =
> - irq_of_parse_and_map(child, 0);
> - if (!mdio->irq[addr])
> - mdio->irq[addr] = prev_irq;
> + rc = irq_of_parse_and_map(child, 0);
> + if (rc > 0) {
> + mdio->irq[addr] = rc;
> + phy->irq = rc;
> + }
> }
>
> /* Associate the OF node with the device structure so it
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Florian
^ permalink raw reply
* Re: [PATCH] net: add init-regs for of_phy support
From: Florian Fainelli @ 2014-02-17 17:33 UTC (permalink / raw)
To: Ben Dooks; +Cc: netdev, devicetree@vger.kernel.org, Linux-sh list
In-Reply-To: <1392642484-19938-2-git-send-email-ben.dooks@codethink.co.uk>
Hi Ben,
2014-02-17 5:08 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
> 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.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> Documentation/devicetree/bindings/net/phy.txt | 12 ++++++
> drivers/net/phy/phy_device.c | 59 ++++++++++++++++++++++++++-
> 2 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index 58307d0..48d8ded 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -20,6 +20,8 @@ Optional Properties:
> assume clause 22. The compatible list may also contain other
> elements.
> - max-speed: Maximum PHY supported speed (10, 100, 1000...)
> +- init-regs: Set of registers to modify at initialisation as a
> + a set of <register set clear>
Should be:
"micrel,led-control-init-val" or something like that.
first cell is the register address, according to the IEEE 802.3 clause 22
second cell is the set bitmask to apply to the register address
specified in the first cell
third cell is the clear bitmask to apply to the register address
specified in the second cell
I would rather see this as a specific PHY node DT property for setting
the LED control register, because this is totally non-standard and you
are touching a proprietary register here.
>
> Example:
>
> @@ -29,3 +31,13 @@ ethernet-phy@0 {
> interrupts = <35 1>;
> reg = <0>;
> };
> +
> +ethernet-phy@0 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + interrupt-parent = <40000>;
> + interrupts = <35 1>;
> + reg = <0>;
> +
> + /* set KSZ8041 LED mode bits correctly */
> + init-reg = <0x1e 0x4000 0xc000>;
> +};
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 82514e7..6741cdb 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -33,6 +33,7 @@
> #include <linux/mdio.h>
> #include <linux/io.h>
> #include <linux/uaccess.h>
> +#include <linux/of.h>
>
> #include <asm/irq.h>
>
> @@ -532,6 +533,57 @@ static int phy_poll_reset(struct phy_device *phydev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static int of_phy_configure(struct phy_device *phydev)
> +{
> + struct device *dev = &phydev->dev;
> + struct device_node *of_node = dev->of_node;
> + struct property *prop;
> + const __be32 *ptr;
> + u32 reg, set, clear;
> + int len;
> + int val;
This does not belong in the generic PHY code unless we are very clear
on what we want to do, and how to do it, which I do not think we are
yet. What exactly is needed here:
- fixing up some design mistake?
- accounting for a specific board design?
In any case a PHY fixup would do the job for you.
> +
> + if (!of_node)
> + of_node = dev->parent->of_node;
> + if (!of_node)
> + return 0;
> +
> + prop = of_find_property(of_node, "init-regs", &len);
> + if (prop) {
> + if (len % (sizeof(__be32) * 3)) {
> + dev_err(dev, "init-regs not multiple of 3 entries\n");
> + return -EINVAL;
> + }
> +
> + ptr = of_prop_next_u32(prop, ptr, ®);
> + while (ptr != NULL) {
> + ptr = of_prop_next_u32(prop, ptr, ®);
> + ptr = of_prop_next_u32(prop, ptr, &set);
> + ptr = of_prop_next_u32(prop, ptr, &clear);
> +
> + val = phy_read(phydev, reg);
> + if (val < 0) {
> + dev_err(dev, "failed to read %d\n", reg);
> + return val;
> + }
> +
> + val &= ~clear;
> + val |= set;
> + phy_write(phydev, reg, val);
> +
> + dev_info(dev, "set d to %04x\n", reg, val);
> +
> + ptr = of_prop_next_u32(prop, ptr, ®);
> + }
> + }
> +
> + return 0;
> +}
> +#else
> +static inline int of_phy_configure(struct phy_device *phydev) { return 0; }
> +#endif
> +
> int phy_init_hw(struct phy_device *phydev)
> {
> int ret;
> @@ -551,7 +603,12 @@ int phy_init_hw(struct phy_device *phydev)
> if (ret < 0)
> return ret;
>
> - return phydev->drv->config_init(phydev);
> + ret = phydev->drv->config_init(phydev);
> +
> + if (ret == 0)
> + ret = of_phy_configure(phydev);
> +
> + return ret;
> }
> EXPORT_SYMBOL(phy_init_hw);
>
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Florian
^ permalink raw reply
* Re: [PATCH] of_mdio: fix phy interrupt passing
From: Ben Dooks @ 2014-02-17 17:42 UTC (permalink / raw)
To: Florian Fainelli
Cc: Grant Likely, linux-kernel, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev, Linux-sh list,
Sergei Shtylyov
In-Reply-To: <CAGVrzcYeuTA0Yd5YyOTuWQfnwLw9zP87MaYJOLwprZRCtvuaXw@mail.gmail.com>
On 17/02/14 17:26, Florian Fainelli wrote:
> Hi Ben,
>
> 2014-02-17 8:29 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
>> The of_mdiobus_register_phy() is not setting phy->irq this causing
>> some drivers to incorrectly assume that the PHY does not have an
>> IRQ associated with it or install an interrupt handler for the
>> PHY.
>>
>> Simplify the code setting irq and set the phy->irq at the same
>> time so that the case if mdio->irq is not NULL is easier to read.
>
> The real bug fix, which is not properly explained here, is that
> irq_of_parse_and_map() should return values > 0 when the interrupt is
> valid, so this makes me wonder why we are not propagating the return
> value from irq_of_parse_and_map() in case the call to
> of_irq_parse_one() does return something non-zero?
No, the first issue is phy->dev never gets set, which causes the
issue. The cleanup was added as it seemed easier to put it in with
this.
I think phy->irq is already initialised to PHY_POLL and thus there
is no need to set phy->irq if the irq_of_parse_and_map() fails.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply
* Re: [PATCH] net: add init-regs for of_phy support
From: Ben Dooks @ 2014-02-17 17:44 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, devicetree@vger.kernel.org, Linux-sh list
In-Reply-To: <CAGVrzcYbw5xLj-gwcup5i0gBdKDDGdyy5wAaEBp7m2E7f2bytg@mail.gmail.com>
On 17/02/14 17:33, Florian Fainelli wrote:
> Hi Ben,
>
> 2014-02-17 5:08 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
>> 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.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>> Documentation/devicetree/bindings/net/phy.txt | 12 ++++++
>> drivers/net/phy/phy_device.c | 59 ++++++++++++++++++++++++++-
>> 2 files changed, 70 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
>> index 58307d0..48d8ded 100644
>> --- a/Documentation/devicetree/bindings/net/phy.txt
>> +++ b/Documentation/devicetree/bindings/net/phy.txt
>> @@ -20,6 +20,8 @@ Optional Properties:
>> assume clause 22. The compatible list may also contain other
>> elements.
>> - max-speed: Maximum PHY supported speed (10, 100, 1000...)
>> +- init-regs: Set of registers to modify at initialisation as a
>> + a set of <register set clear>
>
> Should be:
>
> "micrel,led-control-init-val" or something like that.
>
> first cell is the register address, according to the IEEE 802.3 clause 22
> second cell is the set bitmask to apply to the register address
> specified in the first cell
> third cell is the clear bitmask to apply to the register address
> specified in the second cell
>
> I would rather see this as a specific PHY node DT property for setting
> the LED control register, because this is totally non-standard and you
> are touching a proprietary register here.
I'd rather stay with this than splattering lots and lots of
phy specific additions to each phy driver.
This has the plus it lets board developers set registers in
case of board specific initialisation values that are not
already in the drivers.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply
* Re: [PATCHv11 2/2] dma: Add Freescale eDMA engine driver support
From: Vinod Koul @ 2014-02-17 17:47 UTC (permalink / raw)
To: Jingchang Lu
Cc: dan.j.williams-ral2JQCrhuEAvxtiuMwx3w, arnd-r2nGTMty4D4,
shawn.guo-QSEj5FYQhm4dnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, swarren-3lzwWm7+Weoh9ZMKESR00Q,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Alison Wang
In-Reply-To: <1390209831-15679-1-git-send-email-b35083-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
On Mon, Jan 20, 2014 at 05:23:51PM +0800, Jingchang Lu wrote:
> Add Freescale enhanced direct memory(eDMA) controller support.
> This module can be found on Vybrid and LS-1 SoCs.
>
> Signed-off-by: Alison Wang <b18965-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Jingchang Lu <b35083-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Acked-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Where is the 1/2 in this patch series, I cant find it in archives as well
Also please post to dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
--
~Vinod
> ---
> changes in v11:
> Add dma device_slave_caps definition.
>
> changes in v10:
> define fsl_edma_mutex in fsl_edma_engine instead of global.
> minor changes of binding description.
>
> changes in v9:
> define endian's operating functions instead of macro definition.
> remove the filter function, using dma_get_slave_channel instead.
>
> changes in v8:
> change the edma driver according eDMA dts change.
> add big-endian and little-endian handling.
>
> no changes in v4 ~ v7.
>
> changes in v3:
> add vf610 edma dt-bindings namespace with prefix VF610_*.
>
> changes in v2:
> using generic dma-channels property instead of fsl,dma-channels.
>
> Documentation/devicetree/bindings/dma/fsl-edma.txt | 76 ++
> drivers/dma/Kconfig | 10 +
> drivers/dma/Makefile | 1 +
> drivers/dma/fsl-edma.c | 975 +++++++++++++++++++++
> 4 files changed, 1062 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/fsl-edma.txt
> create mode 100644 drivers/dma/fsl-edma.c
>
> diff --git a/Documentation/devicetree/bindings/dma/fsl-edma.txt b/Documentation/devicetree/bindings/dma/fsl-edma.txt
> new file mode 100644
> index 0000000..191d7bd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/fsl-edma.txt
> @@ -0,0 +1,76 @@
> +* Freescale enhanced Direct Memory Access(eDMA) Controller
> +
> + The eDMA channels have multiplex capability by programmble memory-mapped
> +registers. channels are split into two groups, called DMAMUX0 and DMAMUX1,
> +specific DMA request source can only be multiplexed by any channel of certain
> +group, DMAMUX0 or DMAMUX1, but not both.
> +
> +* eDMA Controller
> +Required properties:
> +- compatible :
> + - "fsl,vf610-edma" for eDMA used similar to that on Vybrid vf610 SoC
> +- reg : Specifies base physical address(s) and size of the eDMA registers.
> + The 1st region is eDMA control register's address and size.
> + The 2nd and the 3rd regions are programmable channel multiplexing
> + control register's address and size.
> +- interrupts : A list of interrupt-specifiers, one for each entry in
> + interrupt-names.
> +- interrupt-names : Should contain:
> + "edma-tx" - the transmission interrupt
> + "edma-err" - the error interrupt
> +- #dma-cells : Must be <2>.
> + The 1st cell specifies the DMAMUX(0 for DMAMUX0 and 1 for DMAMUX1).
> + Specific request source can only be multiplexed by specific channels
> + group called DMAMUX.
> + The 2nd cell specifies the request source(slot) ID.
> + See the SoC's reference manual for all the supported request sources.
> +- dma-channels : Number of channels supported by the controller
> +- clock-names : A list of channel group clock names. Should contain:
> + "dmamux0" - clock name of mux0 group
> + "dmamux1" - clock name of mux1 group
> +- clocks : A list of phandle and clock-specifier pairs, one for each entry in
> + clock-names.
> +
> +Optional properties:
> +- big-endian: If present registers and hardware scatter/gather descriptors
> + of the eDMA are implemented in big endian mode, otherwise in little
> + mode.
> +
> +
> +Examples:
> +
> +edma0: dma-controller@40018000 {
> + #dma-cells = <2>;
> + compatible = "fsl,vf610-edma";
> + reg = <0x40018000 0x2000>,
> + <0x40024000 0x1000>,
> + <0x40025000 0x1000>;
> + interrupts = <0 8 IRQ_TYPE_LEVEL_HIGH>,
> + <0 9 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "edma-tx", "edma-err";
> + dma-channels = <32>;
> + clock-names = "dmamux0", "dmamux1";
> + clocks = <&clks VF610_CLK_DMAMUX0>,
> + <&clks VF610_CLK_DMAMUX1>;
> +};
> +
> +
> +* DMA clients
> +DMA client drivers that uses the DMA function must use the format described
> +in the dma.txt file, using a two-cell specifier for each channel: the 1st
> +specifies the channel group(DMAMUX) in which this request can be multiplexed,
> +and the 2nd specifies the request source.
> +
> +Examples:
> +
> +sai2: sai@40031000 {
> + compatible = "fsl,vf610-sai";
> + reg = <0x40031000 0x1000>;
> + interrupts = <0 86 IRQ_TYPE_LEVEL_HIGH>;
> + clock-names = "sai";
> + clocks = <&clks VF610_CLK_SAI2>;
> + dma-names = "tx", "rx";
> + dmas = <&edma0 0 21>,
> + <&edma0 0 20>;
> + status = "disabled";
> +};
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 9ae6f54..3d8a522 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -342,6 +342,16 @@ config K3_DMA
> Support the DMA engine for Hisilicon K3 platform
> devices.
>
> +config FSL_EDMA
> + tristate "Freescale eDMA engine support"
> + depends on OF
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + help
> + Support the Freescale eDMA engine with programmable channel
> + multiplexing capability for DMA request sources(slot).
> + This module can be found on Freescale Vybrid and LS-1 SoCs.
> +
> config DMA_ENGINE
> bool
>
> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
> index 0a6f08e..e39c56b 100644
> --- a/drivers/dma/Makefile
> +++ b/drivers/dma/Makefile
> @@ -43,3 +43,4 @@ obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
> obj-$(CONFIG_DMA_JZ4740) += dma-jz4740.o
> obj-$(CONFIG_TI_CPPI41) += cppi41.o
> obj-$(CONFIG_K3_DMA) += k3dma.o
> +obj-$(CONFIG_FSL_EDMA) += fsl-edma.o
> diff --git a/drivers/dma/fsl-edma.c b/drivers/dma/fsl-edma.c
> new file mode 100644
> index 0000000..9025300
> --- /dev/null
> +++ b/drivers/dma/fsl-edma.c
> @@ -0,0 +1,975 @@
> +/*
> + * drivers/dma/fsl-edma.c
> + *
> + * Copyright 2013-2014 Freescale Semiconductor, Inc.
> + *
> + * Driver for the Freescale eDMA engine with flexible channel multiplexing
> + * capability for DMA request sources. The eDMA block can be found on some
> + * Vybrid and Layerscape SoCs.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/clk.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmapool.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_dma.h>
> +
> +#include "virt-dma.h"
> +
> +#define EDMA_CR 0x00
> +#define EDMA_ES 0x04
> +#define EDMA_ERQ 0x0C
> +#define EDMA_EEI 0x14
> +#define EDMA_SERQ 0x1B
> +#define EDMA_CERQ 0x1A
> +#define EDMA_SEEI 0x19
> +#define EDMA_CEEI 0x18
> +#define EDMA_CINT 0x1F
> +#define EDMA_CERR 0x1E
> +#define EDMA_SSRT 0x1D
> +#define EDMA_CDNE 0x1C
> +#define EDMA_INTR 0x24
> +#define EDMA_ERR 0x2C
> +
> +#define EDMA_TCD_SADDR(x) (0x1000 + 32 * (x))
> +#define EDMA_TCD_SOFF(x) (0x1004 + 32 * (x))
> +#define EDMA_TCD_ATTR(x) (0x1006 + 32 * (x))
> +#define EDMA_TCD_NBYTES(x) (0x1008 + 32 * (x))
> +#define EDMA_TCD_SLAST(x) (0x100C + 32 * (x))
> +#define EDMA_TCD_DADDR(x) (0x1010 + 32 * (x))
> +#define EDMA_TCD_DOFF(x) (0x1014 + 32 * (x))
> +#define EDMA_TCD_CITER_ELINK(x) (0x1016 + 32 * (x))
> +#define EDMA_TCD_CITER(x) (0x1016 + 32 * (x))
> +#define EDMA_TCD_DLAST_SGA(x) (0x1018 + 32 * (x))
> +#define EDMA_TCD_CSR(x) (0x101C + 32 * (x))
> +#define EDMA_TCD_BITER_ELINK(x) (0x101E + 32 * (x))
> +#define EDMA_TCD_BITER(x) (0x101E + 32 * (x))
> +
> +#define EDMA_CR_EDBG BIT(1)
> +#define EDMA_CR_ERCA BIT(2)
> +#define EDMA_CR_ERGA BIT(3)
> +#define EDMA_CR_HOE BIT(4)
> +#define EDMA_CR_HALT BIT(5)
> +#define EDMA_CR_CLM BIT(6)
> +#define EDMA_CR_EMLM BIT(7)
> +#define EDMA_CR_ECX BIT(16)
> +#define EDMA_CR_CX BIT(17)
> +
> +#define EDMA_SEEI_SEEI(x) ((x) & 0x1F)
> +#define EDMA_CEEI_CEEI(x) ((x) & 0x1F)
> +#define EDMA_CINT_CINT(x) ((x) & 0x1F)
> +#define EDMA_CERR_CERR(x) ((x) & 0x1F)
> +
> +#define EDMA_TCD_ATTR_DSIZE(x) (((x) & 0x0007))
> +#define EDMA_TCD_ATTR_DMOD(x) (((x) & 0x001F) << 3)
> +#define EDMA_TCD_ATTR_SSIZE(x) (((x) & 0x0007) << 8)
> +#define EDMA_TCD_ATTR_SMOD(x) (((x) & 0x001F) << 11)
> +#define EDMA_TCD_ATTR_SSIZE_8BIT (0x0000)
> +#define EDMA_TCD_ATTR_SSIZE_16BIT (0x0100)
> +#define EDMA_TCD_ATTR_SSIZE_32BIT (0x0200)
> +#define EDMA_TCD_ATTR_SSIZE_64BIT (0x0300)
> +#define EDMA_TCD_ATTR_SSIZE_32BYTE (0x0500)
> +#define EDMA_TCD_ATTR_DSIZE_8BIT (0x0000)
> +#define EDMA_TCD_ATTR_DSIZE_16BIT (0x0001)
> +#define EDMA_TCD_ATTR_DSIZE_32BIT (0x0002)
> +#define EDMA_TCD_ATTR_DSIZE_64BIT (0x0003)
> +#define EDMA_TCD_ATTR_DSIZE_32BYTE (0x0005)
> +
> +#define EDMA_TCD_SOFF_SOFF(x) (x)
> +#define EDMA_TCD_NBYTES_NBYTES(x) (x)
> +#define EDMA_TCD_SLAST_SLAST(x) (x)
> +#define EDMA_TCD_DADDR_DADDR(x) (x)
> +#define EDMA_TCD_CITER_CITER(x) ((x) & 0x7FFF)
> +#define EDMA_TCD_DOFF_DOFF(x) (x)
> +#define EDMA_TCD_DLAST_SGA_DLAST_SGA(x) (x)
> +#define EDMA_TCD_BITER_BITER(x) ((x) & 0x7FFF)
> +
> +#define EDMA_TCD_CSR_START BIT(0)
> +#define EDMA_TCD_CSR_INT_MAJOR BIT(1)
> +#define EDMA_TCD_CSR_INT_HALF BIT(2)
> +#define EDMA_TCD_CSR_D_REQ BIT(3)
> +#define EDMA_TCD_CSR_E_SG BIT(4)
> +#define EDMA_TCD_CSR_E_LINK BIT(5)
> +#define EDMA_TCD_CSR_ACTIVE BIT(6)
> +#define EDMA_TCD_CSR_DONE BIT(7)
> +
> +#define EDMAMUX_CHCFG_DIS 0x0
> +#define EDMAMUX_CHCFG_ENBL 0x80
> +#define EDMAMUX_CHCFG_SOURCE(n) ((n) & 0x3F)
> +
> +#define DMAMUX_NR 2
> +
> +#define FSL_EDMA_BUSWIDTHS BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) | \
> + BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) | \
> + BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) | \
> + BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)
> +
> +struct fsl_edma_hw_tcd {
> + u32 saddr;
> + u16 soff;
> + u16 attr;
> + u32 nbytes;
> + u32 slast;
> + u32 daddr;
> + u16 doff;
> + u16 citer;
> + u32 dlast_sga;
> + u16 csr;
> + u16 biter;
> +};
> +
> +struct fsl_edma_sw_tcd {
> + dma_addr_t ptcd;
> + struct fsl_edma_hw_tcd *vtcd;
> +};
> +
> +struct fsl_edma_slave_config {
> + enum dma_transfer_direction dir;
> + enum dma_slave_buswidth addr_width;
> + u32 dev_addr;
> + u32 burst;
> + u32 attr;
> +};
> +
> +struct fsl_edma_chan {
> + struct virt_dma_chan vchan;
> + enum dma_status status;
> + struct fsl_edma_engine *edma;
> + struct fsl_edma_desc *edesc;
> + struct fsl_edma_slave_config fsc;
> + struct dma_pool *tcd_pool;
> +};
> +
> +struct fsl_edma_desc {
> + struct virt_dma_desc vdesc;
> + struct fsl_edma_chan *echan;
> + bool iscyclic;
> + unsigned int n_tcds;
> + struct fsl_edma_sw_tcd tcd[];
> +};
> +
> +struct fsl_edma_engine {
> + struct dma_device dma_dev;
> + void __iomem *membase;
> + void __iomem *muxbase[DMAMUX_NR];
> + struct clk *muxclk[DMAMUX_NR];
> + struct mutex fsl_edma_mutex;
> + u32 n_chans;
> + int txirq;
> + int errirq;
> + bool big_endian;
> + struct fsl_edma_chan chans[];
> +};
> +
> +/*
> + * R/W functions for big- or little-endian registers
> + * the eDMA controller's endian is independent of the CPU core's endian.
> + */
> +
> +static u16 edma_readw(struct fsl_edma_engine *edma, void __iomem *addr)
> +{
> + if (edma->big_endian)
> + return ioread16be(addr);
> + else
> + return ioread16(addr);
> +}
> +
> +static u32 edma_readl(struct fsl_edma_engine *edma, void __iomem *addr)
> +{
> + if (edma->big_endian)
> + return ioread32be(addr);
> + else
> + return ioread32(addr);
> +}
> +
> +static void edma_writeb(struct fsl_edma_engine *edma, u8 val, void __iomem *addr)
> +{
> + iowrite8(val, addr);
> +}
> +
> +static void edma_writew(struct fsl_edma_engine *edma, u16 val, void __iomem *addr)
> +{
> + if (edma->big_endian)
> + iowrite16be(val, addr);
> + else
> + iowrite16(val, addr);
> +}
> +
> +static void edma_writel(struct fsl_edma_engine *edma, u32 val, void __iomem *addr)
> +{
> + if (edma->big_endian)
> + iowrite32be(val, addr);
> + else
> + iowrite32(val, addr);
> +}
> +
> +static struct fsl_edma_chan *to_fsl_edma_chan(struct dma_chan *chan)
> +{
> + return container_of(chan, struct fsl_edma_chan, vchan.chan);
> +}
> +
> +static struct fsl_edma_desc *to_fsl_edma_desc(struct virt_dma_desc *vd)
> +{
> + return container_of(vd, struct fsl_edma_desc, vdesc);
> +}
> +
> +static void fsl_edma_enable_request(struct fsl_edma_chan *fsl_chan)
> +{
> + void __iomem *addr = fsl_chan->edma->membase;
> + u32 ch = fsl_chan->vchan.chan.chan_id;
> +
> + edma_writeb(fsl_chan->edma, EDMA_SEEI_SEEI(ch), addr + EDMA_SEEI);
> + edma_writeb(fsl_chan->edma, ch, addr + EDMA_SERQ);
> +}
> +
> +static void fsl_edma_disable_request(struct fsl_edma_chan *fsl_chan)
> +{
> + void __iomem *addr = fsl_chan->edma->membase;
> + u32 ch = fsl_chan->vchan.chan.chan_id;
> +
> + edma_writeb(fsl_chan->edma, ch, addr + EDMA_CERQ);
> + edma_writeb(fsl_chan->edma, EDMA_CEEI_CEEI(ch), addr + EDMA_CEEI);
> +}
> +
> +static void fsl_edma_chan_mux(struct fsl_edma_chan *fsl_chan,
> + unsigned int slot, bool enable)
> +{
> + u32 ch = fsl_chan->vchan.chan.chan_id;
> + void __iomem *muxaddr = fsl_chan->edma->muxbase[ch / DMAMUX_NR];
> + unsigned chans_per_mux, ch_off;
> +
> + chans_per_mux = fsl_chan->edma->n_chans / DMAMUX_NR;
> + ch_off = fsl_chan->vchan.chan.chan_id % chans_per_mux;
> +
> + if (enable)
> + edma_writeb(fsl_chan->edma,
> + EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(slot),
> + muxaddr + ch_off);
> + else
> + edma_writeb(fsl_chan->edma, EDMAMUX_CHCFG_DIS, muxaddr + ch_off);
> +}
> +
> +static unsigned int fsl_edma_get_tcd_attr(enum dma_slave_buswidth addr_width)
> +{
> + switch (addr_width) {
> + case 1:
> + return EDMA_TCD_ATTR_SSIZE_8BIT | EDMA_TCD_ATTR_DSIZE_8BIT;
> + case 2:
> + return EDMA_TCD_ATTR_SSIZE_16BIT | EDMA_TCD_ATTR_DSIZE_16BIT;
> + case 4:
> + return EDMA_TCD_ATTR_SSIZE_32BIT | EDMA_TCD_ATTR_DSIZE_32BIT;
> + case 8:
> + return EDMA_TCD_ATTR_SSIZE_64BIT | EDMA_TCD_ATTR_DSIZE_64BIT;
> + default:
> + return EDMA_TCD_ATTR_SSIZE_32BIT | EDMA_TCD_ATTR_DSIZE_32BIT;
> + }
> +}
> +
> +static void fsl_edma_free_desc(struct virt_dma_desc *vdesc)
> +{
> + struct fsl_edma_desc *fsl_desc;
> + int i;
> +
> + fsl_desc = to_fsl_edma_desc(vdesc);
> + for (i = 0; i < fsl_desc->n_tcds; i++)
> + dma_pool_free(fsl_desc->echan->tcd_pool,
> + fsl_desc->tcd[i].vtcd,
> + fsl_desc->tcd[i].ptcd);
> + kfree(fsl_desc);
> +}
> +
> +static int fsl_edma_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> + unsigned long arg)
> +{
> + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> + struct dma_slave_config *cfg = (void *)arg;
> + unsigned long flags;
> + LIST_HEAD(head);
> +
> + switch (cmd) {
> + case DMA_TERMINATE_ALL:
> + spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
> + fsl_edma_disable_request(fsl_chan);
> + fsl_chan->edesc = NULL;
> + vchan_get_all_descriptors(&fsl_chan->vchan, &head);
> + spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
> + vchan_dma_desc_free_list(&fsl_chan->vchan, &head);
> + return 0;
> +
> + case DMA_SLAVE_CONFIG:
> + fsl_chan->fsc.dir = cfg->direction;
> + if (cfg->direction == DMA_DEV_TO_MEM) {
> + fsl_chan->fsc.dev_addr = cfg->src_addr;
> + fsl_chan->fsc.addr_width = cfg->src_addr_width;
> + fsl_chan->fsc.burst = cfg->src_maxburst;
> + fsl_chan->fsc.attr = fsl_edma_get_tcd_attr(cfg->src_addr_width);
> + } else if (cfg->direction == DMA_MEM_TO_DEV) {
> + fsl_chan->fsc.dev_addr = cfg->dst_addr;
> + fsl_chan->fsc.addr_width = cfg->dst_addr_width;
> + fsl_chan->fsc.burst = cfg->dst_maxburst;
> + fsl_chan->fsc.attr = fsl_edma_get_tcd_attr(cfg->dst_addr_width);
> + } else {
> + return -EINVAL;
> + }
> + return 0;
> +
> + case DMA_PAUSE:
> + spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
> + if (fsl_chan->edesc) {
> + fsl_edma_disable_request(fsl_chan);
> + fsl_chan->status = DMA_PAUSED;
> + }
> + spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
> + return 0;
> +
> + case DMA_RESUME:
> + spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
> + if (fsl_chan->edesc) {
> + fsl_edma_enable_request(fsl_chan);
> + fsl_chan->status = DMA_IN_PROGRESS;
> + }
> + spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
> + return 0;
> +
> + default:
> + return -ENXIO;
> + }
> +}
> +
> +static size_t fsl_edma_desc_residue(struct fsl_edma_chan *fsl_chan,
> + struct virt_dma_desc *vdesc, bool in_progress)
> +{
> + struct fsl_edma_desc *edesc = fsl_chan->edesc;
> + void __iomem *addr = fsl_chan->edma->membase;
> + u32 ch = fsl_chan->vchan.chan.chan_id;
> + enum dma_transfer_direction dir = fsl_chan->fsc.dir;
> + dma_addr_t cur_addr, dma_addr;
> + size_t len, size;
> + int i;
> +
> + /* calculate the total size in this desc */
> + for (len = i = 0; i < fsl_chan->edesc->n_tcds; i++)
> + len += edma_readl(fsl_chan->edma, &(edesc->tcd[i].vtcd->nbytes))
> + * edma_readw(fsl_chan->edma, &(edesc->tcd[i].vtcd->biter));
> +
> + if (!in_progress)
> + return len;
> +
> + if (dir == DMA_MEM_TO_DEV)
> + cur_addr = edma_readl(fsl_chan->edma, addr + EDMA_TCD_SADDR(ch));
> + else
> + cur_addr = edma_readl(fsl_chan->edma, addr + EDMA_TCD_DADDR(ch));
> +
> + /* figure out the finished and calculate the residue */
> + for (i = 0; i < fsl_chan->edesc->n_tcds; i++) {
> + size = edma_readl(fsl_chan->edma, &(edesc->tcd[i].vtcd->nbytes))
> + * edma_readw(fsl_chan->edma, &(edesc->tcd[i].vtcd->biter));
> + if (dir == DMA_MEM_TO_DEV)
> + dma_addr = edma_readl(fsl_chan->edma,
> + &(edesc->tcd[i].vtcd->saddr));
> + else
> + dma_addr = edma_readl(fsl_chan->edma,
> + &(edesc->tcd[i].vtcd->daddr));
> +
> + len -= size;
> + if (cur_addr > dma_addr && cur_addr < dma_addr + size) {
> + len += dma_addr + size - cur_addr;
> + break;
> + }
> + }
> +
> + return len;
> +}
> +
> +static enum dma_status fsl_edma_tx_status(struct dma_chan *chan,
> + dma_cookie_t cookie, struct dma_tx_state *txstate)
> +{
> + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> + struct virt_dma_desc *vdesc;
> + enum dma_status status;
> + unsigned long flags;
> +
> + status = dma_cookie_status(chan, cookie, txstate);
> + if (status == DMA_COMPLETE)
> + return status;
> +
> + if (!txstate)
> + return fsl_chan->status;
> +
> + spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
> + vdesc = vchan_find_desc(&fsl_chan->vchan, cookie);
> + if (fsl_chan->edesc && cookie == fsl_chan->edesc->vdesc.tx.cookie)
> + txstate->residue = fsl_edma_desc_residue(fsl_chan, vdesc, true);
> + else if (vdesc)
> + txstate->residue = fsl_edma_desc_residue(fsl_chan, vdesc, false);
> + else
> + txstate->residue = 0;
> +
> + spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
> +
> + return fsl_chan->status;
> +}
> +
> +static void fsl_edma_set_tcd_params(struct fsl_edma_chan *fsl_chan,
> + u32 src, u32 dst, u16 attr, u16 soff, u32 nbytes,
> + u32 slast, u16 citer, u16 biter, u32 doff, u32 dlast_sga,
> + u16 csr)
> +{
> + void __iomem *addr = fsl_chan->edma->membase;
> + u32 ch = fsl_chan->vchan.chan.chan_id;
> +
> + /*
> + * TCD parameters have been swapped in fill_tcd_params(),
> + * so just write them to registers in the cpu endian here
> + */
> + writew(0, addr + EDMA_TCD_CSR(ch));
> + writel(src, addr + EDMA_TCD_SADDR(ch));
> + writel(dst, addr + EDMA_TCD_DADDR(ch));
> + writew(attr, addr + EDMA_TCD_ATTR(ch));
> + writew(soff, addr + EDMA_TCD_SOFF(ch));
> + writel(nbytes, addr + EDMA_TCD_NBYTES(ch));
> + writel(slast, addr + EDMA_TCD_SLAST(ch));
> + writew(citer, addr + EDMA_TCD_CITER(ch));
> + writew(biter, addr + EDMA_TCD_BITER(ch));
> + writew(doff, addr + EDMA_TCD_DOFF(ch));
> + writel(dlast_sga, addr + EDMA_TCD_DLAST_SGA(ch));
> + writew(csr, addr + EDMA_TCD_CSR(ch));
> +}
> +
> +static void fill_tcd_params(struct fsl_edma_engine *edma,
> + struct fsl_edma_hw_tcd *tcd, u32 src, u32 dst,
> + u16 attr, u16 soff, u32 nbytes, u32 slast, u16 citer,
> + u16 biter, u16 doff, u32 dlast_sga, bool major_int,
> + bool disable_req, bool enable_sg)
> +{
> + u16 csr = 0;
> +
> + /*
> + * eDMA hardware SGs require the TCD parameters stored in memory
> + * the same endian as the eDMA module so that they can be loaded
> + * automatically by the engine
> + */
> + edma_writel(edma, src, &(tcd->saddr));
> + edma_writel(edma, dst, &(tcd->daddr));
> + edma_writew(edma, attr, &(tcd->attr));
> + edma_writew(edma, EDMA_TCD_SOFF_SOFF(soff), &(tcd->soff));
> + edma_writel(edma, EDMA_TCD_NBYTES_NBYTES(nbytes), &(tcd->nbytes));
> + edma_writel(edma, EDMA_TCD_SLAST_SLAST(slast), &(tcd->slast));
> + edma_writew(edma, EDMA_TCD_CITER_CITER(citer), &(tcd->citer));
> + edma_writew(edma, EDMA_TCD_DOFF_DOFF(doff), &(tcd->doff));
> + edma_writel(edma, EDMA_TCD_DLAST_SGA_DLAST_SGA(dlast_sga), &(tcd->dlast_sga));
> + edma_writew(edma, EDMA_TCD_BITER_BITER(biter), &(tcd->biter));
> + if (major_int)
> + csr |= EDMA_TCD_CSR_INT_MAJOR;
> +
> + if (disable_req)
> + csr |= EDMA_TCD_CSR_D_REQ;
> +
> + if (enable_sg)
> + csr |= EDMA_TCD_CSR_E_SG;
> +
> + edma_writew(edma, csr, &(tcd->csr));
> +}
> +
> +static struct fsl_edma_desc *fsl_edma_alloc_desc(struct fsl_edma_chan *fsl_chan,
> + int sg_len)
> +{
> + struct fsl_edma_desc *fsl_desc;
> + int i;
> +
> + fsl_desc = kzalloc(sizeof(*fsl_desc) + sizeof(struct fsl_edma_sw_tcd) * sg_len,
> + GFP_NOWAIT);
> + if (!fsl_desc)
> + return NULL;
> +
> + fsl_desc->echan = fsl_chan;
> + fsl_desc->n_tcds = sg_len;
> + for (i = 0; i < sg_len; i++) {
> + fsl_desc->tcd[i].vtcd = dma_pool_alloc(fsl_chan->tcd_pool,
> + GFP_NOWAIT, &fsl_desc->tcd[i].ptcd);
> + if (!fsl_desc->tcd[i].vtcd)
> + goto err;
> + }
> + return fsl_desc;
> +
> +err:
> + while (--i >= 0)
> + dma_pool_free(fsl_chan->tcd_pool, fsl_desc->tcd[i].vtcd,
> + fsl_desc->tcd[i].ptcd);
> + kfree(fsl_desc);
> + return NULL;
> +}
> +
> +static struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
> + struct dma_chan *chan, dma_addr_t dma_addr, size_t buf_len,
> + size_t period_len, enum dma_transfer_direction direction,
> + unsigned long flags, void *context)
> +{
> + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> + struct fsl_edma_desc *fsl_desc;
> + dma_addr_t dma_buf_next;
> + int sg_len, i;
> + u32 src_addr, dst_addr, last_sg, nbytes;
> + u16 soff, doff, iter;
> +
> + if (!is_slave_direction(fsl_chan->fsc.dir))
> + return NULL;
> +
> + sg_len = buf_len / period_len;
> + fsl_desc = fsl_edma_alloc_desc(fsl_chan, sg_len);
> + if (!fsl_desc)
> + return NULL;
> + fsl_desc->iscyclic = true;
> +
> + dma_buf_next = dma_addr;
> + nbytes = fsl_chan->fsc.addr_width * fsl_chan->fsc.burst;
> + iter = period_len / nbytes;
> +
> + for (i = 0; i < sg_len; i++) {
> + if (dma_buf_next >= dma_addr + buf_len)
> + dma_buf_next = dma_addr;
> +
> + /* get next sg's physical address */
> + last_sg = fsl_desc->tcd[(i + 1) % sg_len].ptcd;
> +
> + if (fsl_chan->fsc.dir == DMA_MEM_TO_DEV) {
> + src_addr = dma_buf_next;
> + dst_addr = fsl_chan->fsc.dev_addr;
> + soff = fsl_chan->fsc.addr_width;
> + doff = 0;
> + } else {
> + src_addr = fsl_chan->fsc.dev_addr;
> + dst_addr = dma_buf_next;
> + soff = 0;
> + doff = fsl_chan->fsc.addr_width;
> + }
> +
> + fill_tcd_params(fsl_chan->edma, fsl_desc->tcd[i].vtcd, src_addr,
> + dst_addr, fsl_chan->fsc.attr, soff, nbytes, 0,
> + iter, iter, doff, last_sg, true, false, true);
> + dma_buf_next += period_len;
> + }
> +
> + return vchan_tx_prep(&fsl_chan->vchan, &fsl_desc->vdesc, flags);
> +}
> +
> +static struct dma_async_tx_descriptor *fsl_edma_prep_slave_sg(
> + struct dma_chan *chan, struct scatterlist *sgl,
> + unsigned int sg_len, enum dma_transfer_direction direction,
> + unsigned long flags, void *context)
> +{
> + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> + struct fsl_edma_desc *fsl_desc;
> + struct scatterlist *sg;
> + u32 src_addr, dst_addr, last_sg, nbytes;
> + u16 soff, doff, iter;
> + int i;
> +
> + if (!is_slave_direction(fsl_chan->fsc.dir))
> + return NULL;
> +
> + fsl_desc = fsl_edma_alloc_desc(fsl_chan, sg_len);
> + if (!fsl_desc)
> + return NULL;
> + fsl_desc->iscyclic = false;
> +
> + nbytes = fsl_chan->fsc.addr_width * fsl_chan->fsc.burst;
> + for_each_sg(sgl, sg, sg_len, i) {
> + /* get next sg's physical address */
> + last_sg = fsl_desc->tcd[(i + 1) % sg_len].ptcd;
> +
> + if (fsl_chan->fsc.dir == DMA_MEM_TO_DEV) {
> + src_addr = sg_dma_address(sg);
> + dst_addr = fsl_chan->fsc.dev_addr;
> + soff = fsl_chan->fsc.addr_width;
> + doff = 0;
> + } else {
> + src_addr = fsl_chan->fsc.dev_addr;
> + dst_addr = sg_dma_address(sg);
> + soff = 0;
> + doff = fsl_chan->fsc.addr_width;
> + }
> +
> + iter = sg_dma_len(sg) / nbytes;
> + if (i < sg_len - 1) {
> + last_sg = fsl_desc->tcd[(i + 1)].ptcd;
> + fill_tcd_params(fsl_chan->edma, fsl_desc->tcd[i].vtcd,
> + src_addr, dst_addr, fsl_chan->fsc.attr,
> + soff, nbytes, 0, iter, iter, doff, last_sg,
> + false, false, true);
> + } else {
> + last_sg = 0;
> + fill_tcd_params(fsl_chan->edma, fsl_desc->tcd[i].vtcd,
> + src_addr, dst_addr, fsl_chan->fsc.attr,
> + soff, nbytes, 0, iter, iter, doff, last_sg,
> + true, true, false);
> + }
> + }
> +
> + return vchan_tx_prep(&fsl_chan->vchan, &fsl_desc->vdesc, flags);
> +}
> +
> +static void fsl_edma_xfer_desc(struct fsl_edma_chan *fsl_chan)
> +{
> + struct fsl_edma_hw_tcd *tcd;
> + struct virt_dma_desc *vdesc;
> +
> + vdesc = vchan_next_desc(&fsl_chan->vchan);
> + if (!vdesc)
> + return;
> + fsl_chan->edesc = to_fsl_edma_desc(vdesc);
> + tcd = fsl_chan->edesc->tcd[0].vtcd;
> + fsl_edma_set_tcd_params(fsl_chan, tcd->saddr, tcd->daddr, tcd->attr,
> + tcd->soff, tcd->nbytes, tcd->slast, tcd->citer,
> + tcd->biter, tcd->doff, tcd->dlast_sga, tcd->csr);
> + fsl_edma_enable_request(fsl_chan);
> + fsl_chan->status = DMA_IN_PROGRESS;
> +}
> +
> +static irqreturn_t fsl_edma_tx_handler(int irq, void *dev_id)
> +{
> + struct fsl_edma_engine *fsl_edma = dev_id;
> + unsigned int intr, ch;
> + void __iomem *base_addr;
> + struct fsl_edma_chan *fsl_chan;
> +
> + base_addr = fsl_edma->membase;
> +
> + intr = edma_readl(fsl_edma, base_addr + EDMA_INTR);
> + if (!intr)
> + return IRQ_NONE;
> +
> + for (ch = 0; ch < fsl_edma->n_chans; ch++) {
> + if (intr & (0x1 << ch)) {
> + edma_writeb(fsl_edma, EDMA_CINT_CINT(ch),
> + base_addr + EDMA_CINT);
> +
> + fsl_chan = &fsl_edma->chans[ch];
> +
> + spin_lock(&fsl_chan->vchan.lock);
> + if (!fsl_chan->edesc->iscyclic) {
> + list_del(&fsl_chan->edesc->vdesc.node);
> + vchan_cookie_complete(&fsl_chan->edesc->vdesc);
> + fsl_chan->edesc = NULL;
> + fsl_chan->status = DMA_COMPLETE;
> + } else {
> + vchan_cyclic_callback(&fsl_chan->edesc->vdesc);
> + }
> +
> + if (!fsl_chan->edesc)
> + fsl_edma_xfer_desc(fsl_chan);
> +
> + spin_unlock(&fsl_chan->vchan.lock);
> + }
> + }
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t fsl_edma_err_handler(int irq, void *dev_id)
> +{
> + struct fsl_edma_engine *fsl_edma = dev_id;
> + unsigned int err, ch;
> +
> + err = edma_readl(fsl_edma, fsl_edma->membase + EDMA_ERR);
> + if (!err)
> + return IRQ_NONE;
> +
> + for (ch = 0; ch < fsl_edma->n_chans; ch++) {
> + if (err & (0x1 << ch)) {
> + fsl_edma_disable_request(&fsl_edma->chans[ch]);
> + edma_writeb(fsl_edma, EDMA_CERR_CERR(ch),
> + fsl_edma->membase + EDMA_CERR);
> + fsl_edma->chans[ch].status = DMA_ERROR;
> + }
> + }
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t fsl_edma_irq_handler(int irq, void *dev_id)
> +{
> + if (fsl_edma_tx_handler(irq, dev_id) == IRQ_HANDLED)
> + return IRQ_HANDLED;
> +
> + return fsl_edma_err_handler(irq, dev_id);
> +}
> +
> +static void fsl_edma_issue_pending(struct dma_chan *chan)
> +{
> + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
> +
> + if (vchan_issue_pending(&fsl_chan->vchan) && !fsl_chan->edesc)
> + fsl_edma_xfer_desc(fsl_chan);
> +
> + spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
> +}
> +
> +static struct dma_chan *fsl_edma_xlate(struct of_phandle_args *dma_spec,
> + struct of_dma *ofdma)
> +{
> + struct fsl_edma_engine *fsl_edma = ofdma->of_dma_data;
> + struct dma_chan *chan;
> +
> + if (dma_spec->args_count != 2)
> + return NULL;
> +
> + mutex_lock(&fsl_edma->fsl_edma_mutex);
> + list_for_each_entry(chan, &fsl_edma->dma_dev.channels, device_node) {
> + if (chan->client_count)
> + continue;
> + if ((chan->chan_id / DMAMUX_NR) == dma_spec->args[0]) {
> + chan = dma_get_slave_channel(chan);
> + if (chan) {
> + chan->device->privatecnt++;
> + fsl_edma_chan_mux(to_fsl_edma_chan(chan),
> + dma_spec->args[1], true);
> + mutex_unlock(&fsl_edma->fsl_edma_mutex);
> + return chan;
> + }
> + }
> + }
> + mutex_unlock(&fsl_edma->fsl_edma_mutex);
> + return NULL;
> +}
> +
> +static int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
> +{
> + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> +
> + fsl_chan->tcd_pool = dma_pool_create("tcd_pool", chan->device->dev,
> + sizeof(struct fsl_edma_hw_tcd),
> + 32, 0);
> + return 0;
> +}
> +
> +static void fsl_edma_free_chan_resources(struct dma_chan *chan)
> +{
> + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> + unsigned long flags;
> + LIST_HEAD(head);
> +
> + spin_lock_irqsave(&fsl_chan->vchan.lock, flags);
> + fsl_edma_disable_request(fsl_chan);
> + fsl_edma_chan_mux(fsl_chan, 0, false);
> + fsl_chan->edesc = NULL;
> + vchan_get_all_descriptors(&fsl_chan->vchan, &head);
> + spin_unlock_irqrestore(&fsl_chan->vchan.lock, flags);
> +
> + vchan_dma_desc_free_list(&fsl_chan->vchan, &head);
> + dma_pool_destroy(fsl_chan->tcd_pool);
> + fsl_chan->tcd_pool = NULL;
> +}
> +
> +static int fsl_dma_device_slave_caps(struct dma_chan *dchan,
> + struct dma_slave_caps *caps)
> +{
> + caps->src_addr_widths = FSL_EDMA_BUSWIDTHS;
> + caps->dstn_addr_widths = FSL_EDMA_BUSWIDTHS;
> + caps->directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> + caps->cmd_pause = true;
> + caps->cmd_terminate = true;
> +
> + return 0;
> +}
> +
> +static int
> +fsl_edma_irq_init(struct platform_device *pdev, struct fsl_edma_engine *fsl_edma)
> +{
> + int ret;
> +
> + fsl_edma->txirq = platform_get_irq_byname(pdev, "edma-tx");
> + if (fsl_edma->txirq < 0) {
> + dev_err(&pdev->dev, "Can't get edma-tx irq.\n");
> + return fsl_edma->txirq;
> + }
> +
> + fsl_edma->errirq = platform_get_irq_byname(pdev, "edma-err");
> + if (fsl_edma->errirq < 0) {
> + dev_err(&pdev->dev, "Can't get edma-err irq.\n");
> + return fsl_edma->errirq;
> + }
> +
> + if (fsl_edma->txirq == fsl_edma->errirq) {
> + ret = devm_request_irq(&pdev->dev, fsl_edma->txirq,
> + fsl_edma_irq_handler, 0, "eDMA", fsl_edma);
> + if (ret) {
> + dev_err(&pdev->dev, "Can't register eDMA IRQ.\n");
> + return ret;
> + }
> + } else {
> + ret = devm_request_irq(&pdev->dev, fsl_edma->txirq,
> + fsl_edma_tx_handler, 0, "eDMA tx", fsl_edma);
> + if (ret) {
> + dev_err(&pdev->dev, "Can't register eDMA tx IRQ.\n");
> + return ret;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, fsl_edma->errirq,
> + fsl_edma_err_handler, 0, "eDMA err", fsl_edma);
> + if (ret) {
> + dev_err(&pdev->dev, "Can't register eDMA err IRQ.\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int fsl_edma_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct fsl_edma_engine *fsl_edma;
> + struct fsl_edma_chan *fsl_chan;
> + struct resource *res;
> + int len, chans;
> + int ret, i;
> +
> + ret = of_property_read_u32(np, "dma-channels", &chans);
> + if (ret) {
> + dev_err(&pdev->dev, "Can't get dma-channels.\n");
> + return ret;
> + }
> +
> + len = sizeof(*fsl_edma) + sizeof(*fsl_chan) * chans;
> + fsl_edma = devm_kzalloc(&pdev->dev, len, GFP_KERNEL);
> + if (!fsl_edma)
> + return -ENOMEM;
> +
> + fsl_edma->n_chans = chans;
> + mutex_init(&fsl_edma->fsl_edma_mutex);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + fsl_edma->membase = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(fsl_edma->membase))
> + return PTR_ERR(fsl_edma->membase);
> +
> + for (i = 0; i < DMAMUX_NR; i++) {
> + char clkname[32];
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1 + i);
> + fsl_edma->muxbase[i] = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(fsl_edma->muxbase[i]))
> + return PTR_ERR(fsl_edma->muxbase[i]);
> +
> + sprintf(clkname, "dmamux%d", i);
> + fsl_edma->muxclk[i] = devm_clk_get(&pdev->dev, clkname);
> + if (IS_ERR(fsl_edma->muxclk[i])) {
> + dev_err(&pdev->dev, "Missing DMAMUX block clock.\n");
> + return PTR_ERR(fsl_edma->muxclk[i]);
> + }
> +
> + ret = clk_prepare_enable(fsl_edma->muxclk[i]);
> + if (ret) {
> + dev_err(&pdev->dev, "DMAMUX clk block failed.\n");
> + return ret;
> + }
> +
> + }
> +
> + ret = fsl_edma_irq_init(pdev, fsl_edma);
> + if (ret)
> + return ret;
> +
> + fsl_edma->big_endian = of_property_read_bool(np, "big-endian");
> +
> + INIT_LIST_HEAD(&fsl_edma->dma_dev.channels);
> + for (i = 0; i < fsl_edma->n_chans; i++) {
> + struct fsl_edma_chan *fsl_chan = &fsl_edma->chans[i];
> +
> + fsl_chan->edma = fsl_edma;
> +
> + fsl_chan->vchan.desc_free = fsl_edma_free_desc;
> + vchan_init(&fsl_chan->vchan, &fsl_edma->dma_dev);
> +
> + edma_writew(fsl_edma, 0x0, fsl_edma->membase + EDMA_TCD_CSR(i));
> + fsl_edma_chan_mux(fsl_chan, 0, false);
> + }
> +
> + dma_cap_set(DMA_PRIVATE, fsl_edma->dma_dev.cap_mask);
> + dma_cap_set(DMA_SLAVE, fsl_edma->dma_dev.cap_mask);
> + dma_cap_set(DMA_CYCLIC, fsl_edma->dma_dev.cap_mask);
> +
> + fsl_edma->dma_dev.dev = &pdev->dev;
> + fsl_edma->dma_dev.device_alloc_chan_resources
> + = fsl_edma_alloc_chan_resources;
> + fsl_edma->dma_dev.device_free_chan_resources
> + = fsl_edma_free_chan_resources;
> + fsl_edma->dma_dev.device_tx_status = fsl_edma_tx_status;
> + fsl_edma->dma_dev.device_prep_slave_sg = fsl_edma_prep_slave_sg;
> + fsl_edma->dma_dev.device_prep_dma_cyclic = fsl_edma_prep_dma_cyclic;
> + fsl_edma->dma_dev.device_control = fsl_edma_control;
> + fsl_edma->dma_dev.device_issue_pending = fsl_edma_issue_pending;
> + fsl_edma->dma_dev.device_slave_caps = fsl_dma_device_slave_caps;
> +
> + platform_set_drvdata(pdev, fsl_edma);
> +
> + ret = dma_async_device_register(&fsl_edma->dma_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Can't register Freescale eDMA engine.\n");
> + return ret;
> + }
> +
> + ret = of_dma_controller_register(np, fsl_edma_xlate, fsl_edma);
> + if (ret) {
> + dev_err(&pdev->dev, "Can't register Freescale eDMA of_dma.\n");
> + dma_async_device_unregister(&fsl_edma->dma_dev);
> + return ret;
> + }
> +
> + /* enable round robin arbitration */
> + edma_writel(fsl_edma, EDMA_CR_ERGA | EDMA_CR_ERCA, fsl_edma->membase + EDMA_CR);
> +
> + return 0;
> +}
> +
> +static int fsl_edma_remove(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct fsl_edma_engine *fsl_edma = platform_get_drvdata(pdev);
> + int i;
> +
> + of_dma_controller_free(np);
> + dma_async_device_unregister(&fsl_edma->dma_dev);
> +
> + for (i = 0; i < DMAMUX_NR; i++)
> + clk_disable_unprepare(fsl_edma->muxclk[i]);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id fsl_edma_dt_ids[] = {
> + { .compatible = "fsl,vf610-edma", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, fsl_edma_dt_ids);
> +
> +static struct platform_driver fsl_edma_driver = {
> + .driver = {
> + .name = "fsl-edma",
> + .owner = THIS_MODULE,
> + .of_match_table = fsl_edma_dt_ids,
> + },
> + .probe = fsl_edma_probe,
> + .remove = fsl_edma_remove,
> +};
> +
> +module_platform_driver(fsl_edma_driver);
> +
> +MODULE_ALIAS("platform:fsl-edma");
> +MODULE_DESCRIPTION("Freescale eDMA engine driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.8.0
>
>
--
--
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] of_mdio: fix phy interrupt passing
From: Florian Fainelli @ 2014-02-17 17:48 UTC (permalink / raw)
To: Ben Dooks
Cc: Grant Likely, linux-kernel, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev, Linux-sh list,
Sergei Shtylyov
In-Reply-To: <530249EF.4060803@codethink.co.uk>
2014-02-17 9:42 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
> On 17/02/14 17:26, Florian Fainelli wrote:
>>
>> Hi Ben,
>>
>> 2014-02-17 8:29 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
>>>
>>> The of_mdiobus_register_phy() is not setting phy->irq this causing
>>> some drivers to incorrectly assume that the PHY does not have an
>>> IRQ associated with it or install an interrupt handler for the
>>> PHY.
>>>
>>> Simplify the code setting irq and set the phy->irq at the same
>>> time so that the case if mdio->irq is not NULL is easier to read.
>>
>>
>> The real bug fix, which is not properly explained here, is that
>> irq_of_parse_and_map() should return values > 0 when the interrupt is
>> valid, so this makes me wonder why we are not propagating the return
>> value from irq_of_parse_and_map() in case the call to
>> of_irq_parse_one() does return something non-zero?
>
>
> No, the first issue is phy->dev never gets set, which causes the
> issue. The cleanup was added as it seemed easier to put it in with
> this.
Ok, that really needs to be mentioned in the commit message, even
being quite familiar (and possibly dumb too) with the code, I could
not figure this out by reading your patch.
>
> I think phy->irq is already initialised to PHY_POLL and thus there
> is no need to set phy->irq if the irq_of_parse_and_map() fails.
That is correct, the reason why I introduced 7d97637 ("net: of_mdio:
do not overwrite PHY interrupt configuration") was that you are also
allowed to change the irq type before calling into
of_mdiobus_register(), so we want to preserve other irq values being
set here, such as PHY_IGNORE_INTERRUPT. Your patch does take care of
that since it only overrides the irq in case we could parse it.
--
Florian
^ permalink raw reply
* Re: [PATCH] net: add init-regs for of_phy support
From: Florian Fainelli @ 2014-02-17 17:53 UTC (permalink / raw)
To: Ben Dooks; +Cc: netdev, devicetree@vger.kernel.org, Linux-sh list
In-Reply-To: <53024A62.4050208@codethink.co.uk>
2014-02-17 9:44 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
> On 17/02/14 17:33, Florian Fainelli wrote:
>>
>> Hi Ben,
>>
>> 2014-02-17 5:08 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
>>>
>>> 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.
>>>
>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>> ---
>>> Documentation/devicetree/bindings/net/phy.txt | 12 ++++++
>>> drivers/net/phy/phy_device.c | 59
>>> ++++++++++++++++++++++++++-
>>> 2 files changed, 70 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/phy.txt
>>> b/Documentation/devicetree/bindings/net/phy.txt
>>> index 58307d0..48d8ded 100644
>>> --- a/Documentation/devicetree/bindings/net/phy.txt
>>> +++ b/Documentation/devicetree/bindings/net/phy.txt
>>> @@ -20,6 +20,8 @@ Optional Properties:
>>> assume clause 22. The compatible list may also contain other
>>> elements.
>>> - max-speed: Maximum PHY supported speed (10, 100, 1000...)
>>> +- init-regs: Set of registers to modify at initialisation as a
>>> + a set of <register set clear>
>>
>>
>> Should be:
>>
>> "micrel,led-control-init-val" or something like that.
>>
>> first cell is the register address, according to the IEEE 802.3 clause 22
>> second cell is the set bitmask to apply to the register address
>> specified in the first cell
>> third cell is the clear bitmask to apply to the register address
>> specified in the second cell
>>
>> I would rather see this as a specific PHY node DT property for setting
>> the LED control register, because this is totally non-standard and you
>> are touching a proprietary register here.
>
>
> I'd rather stay with this than splattering lots and lots of
> phy specific additions to each phy driver.
And I would rather not do it because:
- Device Tree related problems are already hard enough to debug that
we do not want DT to act as a firmware and provide values for Linux to
digest without even a bit of parsing and validation
- this is not capture by a PHY fixup, and will not be reported as such
in /sys/class/mdio_bus/*/*/phy_has_fixups
- this covers the one time initialization that might be needed, but
does not cover the suspend/resume/reset times where this is also
needed
- this is vendor-specific and sort of calls for being moved into the PHY driver
>
> This has the plus it lets board developers set registers in
> case of board specific initialisation values that are not
> already in the drivers.
And this bloats the code for the other 99% people that do not use
things like this. Do not get me wrong, Ethernet PHYs never work as we
want them to, but we libphy comes with PHY fixups callbacks which
allows you to capture quirks and will ensure that they are run again
at the right time in the PHY state machine cycle.
--
Florian
^ permalink raw reply
* Re: [PATCH] of_mdio: fix phy interrupt passing
From: Ben Dooks @ 2014-02-17 17:56 UTC (permalink / raw)
To: Florian Fainelli
Cc: Grant Likely, linux-kernel, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev, Linux-sh list,
Sergei Shtylyov
In-Reply-To: <CAGVrzcYeuTA0Yd5YyOTuWQfnwLw9zP87MaYJOLwprZRCtvuaXw@mail.gmail.com>
On 17/02/14 17:26, Florian Fainelli wrote:
> Hi Ben,
>
> 2014-02-17 8:29 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
>> The of_mdiobus_register_phy() is not setting phy->irq this causing
>> some drivers to incorrectly assume that the PHY does not have an
>> IRQ associated with it or install an interrupt handler for the
>> PHY.
>>
>> Simplify the code setting irq and set the phy->irq at the same
>> time so that the case if mdio->irq is not NULL is easier to read.
>
> The real bug fix, which is not properly explained here, is that
> irq_of_parse_and_map() should return values > 0 when the interrupt is
> valid, so this makes me wonder why we are not propagating the return
> value from irq_of_parse_and_map() in case the call to
> of_irq_parse_one() does return something non-zero?
rc = irq_of_parse_and_map(child, 0);
if (rc > 0) {
mdio->irq[addr] = rc;
phy->irq = rc;
}
Unfortunately we do not get any idea of what error went wrong
when doing this, all we get it 0 on failure. So we cannot tell
if the problem was due to no interrupt being there or there was
an interrupt and it could not be parsed properly.
The best we can do is assume that the PHY is happy with being
polled and the specific driver will error out if it expects to
be able to work with an interrupt.
If we error out, we may end up stopping any PHYs where there is
no interrupt available from working. Also, if we fail to parse
then we can generally fall back to polling.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply
* Re: [PATCH 2/2] of: search the best compatible match first in __of_match_node()
From: Grant Likely @ 2014-02-17 17:58 UTC (permalink / raw)
To: devicetree-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ
Cc: Kevin Hao, Sebastian Hesselbarth, Stephen N Chivers, Rob Herring
In-Reply-To: <1392355366-1445-3-git-send-email-haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Fri, 14 Feb 2014 13:22:46 +0800, Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Currently, of_match_node compares each given match against all 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, this patch introduces a function to match each of the node's
> compatible strings against all given compatible matches without type and
> name first, before checking the next compatible string. This implies
> that node's compatibles are ordered from specific to generic while
> given matches can be in any order. If we fail to find such a match
> entry, then fall-back to the old method in order to keep compatibility.
>
> Cc: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/of/base.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index ba195fbce4c6..10b51106c854 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -730,13 +730,49 @@ out:
> }
> EXPORT_SYMBOL(of_find_node_with_property);
>
> +static const struct of_device_id *
> +of_match_compatible(const struct of_device_id *matches,
> + const struct device_node *node)
> +{
> + const char *cp;
> + int cplen, l;
> + const struct of_device_id *m;
> +
> + cp = __of_get_property(node, "compatible", &cplen);
> + while (cp && (cplen > 0)) {
> + m = matches;
> + while (m->name[0] || m->type[0] || m->compatible[0]) {
> + /* Only match for the entries without type and name */
> + if (m->name[0] || m->type[0] ||
> + of_compat_cmp(m->compatible, cp,
> + strlen(m->compatible)))
> + m++;
This seems wrong also. The compatible order should be checked for even
when m->name or m->type are set. You actually need to score the entries
to do this properly. The pseudo-code should look like this:
uint best_score = ~0;
of_device_id *best_match = NULL;
for_each(matches) {
uint score = ~0;
for_each_compatible(index) {
if (match->compatible == compatible[index])
score = index * 10;
}
/* Matching name is a bit better than not */
if (match->name == node->name)
score--;
/* Matching type is better than matching name */
/* (but matching both is even better than that */
if (match->type == node->type)
score -= 2;
if (score < best_score)
best_match = match;
}
return best_match;
This is actually very similar to the original code. It is an easy
modification. This is very similar to how the of_fdt_is_compatible()
function works.
g.
--
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] of_mdio: fix phy interrupt passing
From: Ben Dooks @ 2014-02-17 17:58 UTC (permalink / raw)
To: Florian Fainelli
Cc: Grant Likely, linux-kernel, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, netdev, Linux-sh list,
Sergei Shtylyov
In-Reply-To: <CAGVrzcZGW05uNj75VG7w4iSGre4OzEMhHedQfaynekrbhP4NLQ@mail.gmail.com>
On 17/02/14 17:48, Florian Fainelli wrote:
> 2014-02-17 9:42 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
>> On 17/02/14 17:26, Florian Fainelli wrote:
>>>
>>> Hi Ben,
>>>
>>> 2014-02-17 8:29 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
>>>>
>>>> The of_mdiobus_register_phy() is not setting phy->irq this causing
>>>> some drivers to incorrectly assume that the PHY does not have an
>>>> IRQ associated with it or install an interrupt handler for the
>>>> PHY.
>>>>
>>>> Simplify the code setting irq and set the phy->irq at the same
>>>> time so that the case if mdio->irq is not NULL is easier to read.
>>>
>>>
>>> The real bug fix, which is not properly explained here, is that
>>> irq_of_parse_and_map() should return values > 0 when the interrupt is
>>> valid, so this makes me wonder why we are not propagating the return
>>> value from irq_of_parse_and_map() in case the call to
>>> of_irq_parse_one() does return something non-zero?
>>
>>
>> No, the first issue is phy->dev never gets set, which causes the
>> issue. The cleanup was added as it seemed easier to put it in with
>> this.
>
> Ok, that really needs to be mentioned in the commit message, even
> being quite familiar (and possibly dumb too) with the code, I could
> not figure this out by reading your patch.
>
>>
>> I think phy->irq is already initialised to PHY_POLL and thus there
>> is no need to set phy->irq if the irq_of_parse_and_map() fails.
>
> That is correct, the reason why I introduced 7d97637 ("net: of_mdio:
> do not overwrite PHY interrupt configuration") was that you are also
> allowed to change the irq type before calling into
> of_mdiobus_register(), so we want to preserve other irq values being
> set here, such as PHY_IGNORE_INTERRUPT. Your patch does take care of
> that since it only overrides the irq in case we could parse it.
Ok, the first sentence has a spell of thus, but does refer to phy->irq
being the problem we are looking to fix in this patch.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
^ permalink raw reply
* Re: [PATCH 2/2] of: search the best compatible match first in __of_match_node()
From: Grant Likely @ 2014-02-17 17:59 UTC (permalink / raw)
To: Rob Herring, Kevin Hao
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linuxppc-dev,
Sebastian Hesselbarth, Stephen N Chivers, Rob Herring, Kumar Gala
In-Reply-To: <CAL_JsqKq4_1K8EF+PZoP=0=H6tiRxbgdzs9UHHVdbHS014n74Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 14 Feb 2014 09:53:40 -0600, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Feb 13, 2014 at 11:22 PM, Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > Currently, of_match_node compares each given match against all 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, this patch introduces a function to match each of the node's
> > compatible strings against all given compatible matches without type and
> > name first, before checking the next compatible string. This implies
> > that node's compatibles are ordered from specific to generic while
> > given matches can be in any order. If we fail to find such a match
> > entry, then fall-back to the old method in order to keep compatibility.
> >
> > Cc: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Signed-off-by: Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> Looks good to me. I'll put this in next for a few days. I'd really
> like to see some acks and tested-by's before sending to Linus.
As I commented on the patch, I don't think the new solution is correct
either. I've made a suggestion on how to fix it, but in the mean time
the revert should be applied and sent to Linus.
g.
--
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] net: add init-regs for of_phy support
From: Mark Rutland @ 2014-02-17 18:04 UTC (permalink / raw)
To: Ben Dooks
Cc: Florian Fainelli, netdev, devicetree@vger.kernel.org,
Linux-sh list
In-Reply-To: <53024A62.4050208@codethink.co.uk>
On Mon, Feb 17, 2014 at 05:44:02PM +0000, Ben Dooks wrote:
> On 17/02/14 17:33, Florian Fainelli wrote:
> > Hi Ben,
> >
> > 2014-02-17 5:08 GMT-08:00 Ben Dooks <ben.dooks@codethink.co.uk>:
> >> 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.
> >>
> >> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> >> ---
> >> Documentation/devicetree/bindings/net/phy.txt | 12 ++++++
> >> drivers/net/phy/phy_device.c | 59 ++++++++++++++++++++++++++-
> >> 2 files changed, 70 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> >> index 58307d0..48d8ded 100644
> >> --- a/Documentation/devicetree/bindings/net/phy.txt
> >> +++ b/Documentation/devicetree/bindings/net/phy.txt
> >> @@ -20,6 +20,8 @@ Optional Properties:
> >> assume clause 22. The compatible list may also contain other
> >> elements.
> >> - max-speed: Maximum PHY supported speed (10, 100, 1000...)
> >> +- init-regs: Set of registers to modify at initialisation as a
> >> + a set of <register set clear>
> >
> > Should be:
> >
> > "micrel,led-control-init-val" or something like that.
> >
> > first cell is the register address, according to the IEEE 802.3 clause 22
> > second cell is the set bitmask to apply to the register address
> > specified in the first cell
> > third cell is the clear bitmask to apply to the register address
> > specified in the second cell
> >
> > I would rather see this as a specific PHY node DT property for setting
> > the LED control register, because this is totally non-standard and you
> > are touching a proprietary register here.
>
> I'd rather stay with this than splattering lots and lots of
> phy specific additions to each phy driver.
For something that's PHY-specific anyway? Should we remove the rest of
the "phy specific additions" that constitute a driver?
> This has the plus it lets board developers set registers in
> case of board specific initialisation values that are not
> already in the drivers.
This also has the minus that it lets board developers set registers in
arbitrary ways, creating new bugs that will require more effort to work
around in drivers.
If you update the DTs to describe the hardware to the kernel, then the
kernel can later be updated to handle specific PHYs better. That cannot
happen if you try to hide information from the kernel by giving it a
list of arbitrary magic numbers.
Drivers are the place such things should go. The DT is not a place for
shoddy bytecode.
Thanks,
Mark.
^ permalink raw reply
* devicetree repository separation/migration
From: Jason Cooper @ 2014-02-17 18:05 UTC (permalink / raw)
To: Grant Likely, Rob Herring, Ian Campbell, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, galak-sgV2jX0FEOL9JmXXK+q4OQ,
rob-VoJi6FS/r0vR7s880joybQ
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
devicetree-spec-u79uwXL29TY76Z2rM5mHXA,
devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
All,
At last weeks devicetree irc meeting, I took on the task of writing this
email. I'm a bit behind.
One of the outcomes of the ARM/devicetree discussion at the 2013 Kernel
Summit was that we were going to hold off on separating the devicetree
from the Linux kernel tree. The primary reason for this was to get
through the backlog of patches.
It's been several months, and we're seeing evidence of other projects
having difficulty keeping in sync with the kernel tree. Specifically,
barebox is having trouble syncing:
http://list-archives.org/2014/02/07/barebox-lists-infradead-org/devicetree-maintenance-in-barebox/f/5820726136
U-boot has some questionable (admittedly from early on) nodes. I'll
refrain from singling out a board file or commit. Interested
individuals can look through the irc archives for #devicetree. At any
rate, reportedly, Wolfgang confirmed at the U-Boot minisummit that the
dts files in U-Boot would match the ones from the kernel. So U-Boot is
in the same situation as barebox, wrt syncing.
BSD is also adopting devicetree, but I'm not personally familiar with
the status of that. Nor whether it's diverged from what we have.
On the Linux side, it appears to me that we have successfully removed
the bottleneck, and patches seem to be flowing fairly well now.
For the past few months, Ian has been maintaining a devicetree repo at
http://xenbits.xen.org/gitweb/?p=people/ianc/device-tree-rebasing.git;a=summary
Now for the questions:
- Is the Linux development workflow ready for devicetree to move out
of the Linux Kernel?
- Would having the devicetree in it's own repo, with it's own workflow
and release schedule help the other users of it?
- Where should it be hosted?
- How do we envision projects will use it? git submodule? reference
a version tag? (this is primarily targeted at bootloaders that need
to compile in a dtb or subset of a dtb into the bootloader)
Other thoughts I may have missed?
thx,
Jason.
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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] of: give priority to the compatible match in __of_match_node()
From: Grant Likely @ 2014-02-17 18:06 UTC (permalink / raw)
To: Rob Herring, Kevin Hao
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linuxppc-dev,
Sebastian Hesselbarth, Stephen N Chivers, Chris Proctor,
Arnd Bergmann, Scott Wood, Rob Herring
In-Reply-To: <CAL_JsqKMi2H=vwoxrOt8QRA2xJeiLqBKKfLtt4QRCRoFk6JUHg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, 13 Feb 2014 13:01:42 -0600, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Feb 12, 2014 at 5:38 AM, Kevin Hao <haokexin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > When the device node do have a compatible property, we definitely
> > prefer the compatible match besides the type and name. Only if
> > there is no such a match, we then consider the candidate which
> > doesn't have compatible entry but do match the type or name with
> > the device node.
> >
> > This is based on a patch from Sebastian Hesselbarth.
> > http://patchwork.ozlabs.org/patch/319434/
> >
> > I did some code refactoring and also fixed a bug in the original patch.
>
> I'm inclined to just revert this once again and avoid possibly
> breaking yet another platform.
>
> However, I think I would like to see this structured differently. We
> basically have 2 ways of matching: the existing pre-3.14 way and the
> desired match on best compatible only. All new bindings should match
> with the new way and the old way needs to be kept for compatibility.
> So lets structure the code that way. Search the match table first for
> best compatible with name and type NULL, then search the table the old
> way. I realize it appears you are doing this, but it is not clear this
> is the intent of the code. I would like to see this written as a patch
> with commit 105353145eafb3ea919 reverted first and you add a new match
> function to call first and then fallback to the existing function.
I disagree here, partially because it complicates the code and partially
because it leaves an incorrect corner case. Compatible is always the
preferred matching, even on old drivers, but there are bindings that
take both compatible and type or name into account. Even those cases
should rank the compatible order.
g.
--
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
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