LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Mark Brown @ 2010-04-28 13:39 UTC (permalink / raw)
  To: Timur Tabi; +Cc: alsa-devel, kumar.gala, linuxppc-dev, Liam Girdwood
In-Reply-To: <r2oed82fe3e1004280619j29ba990fr9d0fc5b3ffa450f8@mail.gmail.com>

On Wed, Apr 28, 2010 at 08:19:00AM -0500, Timur Tabi wrote:
> On Tue, Apr 27, 2010 at 5:29 PM, Mark Brown

> > You also want to be representing unused pins here.

> If they're unused, how do I represent them?  Can you give me an example?

You should arrange for something to call snd_soc_dapm_nc_pin() on any
unconnected pins.

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Timur Tabi @ 2010-04-28 13:35 UTC (permalink / raw)
  To: Grant Likely; +Cc: alsa-devel, kumar.gala, broonie, linuxppc-dev, Liam Girdwood
In-Reply-To: <q2xfa686aa41004272237ld1ffc08fhebb55ff84b51d1c2@mail.gmail.com>

On Wed, Apr 28, 2010 at 12:37 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:

> Why not? =A0Just have the ssi driver probe routine register the fabric
> device based on the existence of the codec-handle property. =A0It is the
> best way to go about things with the data that you've got available,
> and it is no big deal. =A0The relevant fabric driver can then bind
> against that. =A0You should probably also stuff the ssi device node
> pointer into the fabric device of_node pointer.

And then where do I put the board-specific initialization code that's
currently in the fabric driver?  The programming information for that
initialization is not in the device tree.

It sounds to me like you're saying I should take all the code from the
fabric driver and shove it into the SSI driver, just so that I can
avoid instantiating a platform driver.

Keep in mind that asoc likes to have a different struct device for the
fabric driver and the SSI nodes, so I would need to manually create a
struct device for the fabric device anyway.

>> I don't know when I would ever do this. =A0The two SSI devices are compl=
etely independent. =A0Why would I bind them together into one "device"?
>
> If there is no use case for binding them together, then you're right.
> The current binding is probably just fine. =A0I cannot comment on
> whether or not it will be used that way by platform designers.

Although the 8610 has two SSI devices, the reference board we ship
only has one wired up.  I have a prototype board in the lab that has
both wired up, but even then, I don't think I can get the two SSIs
synchronized in any meaningful fashion.  Mark suggested a technique,
but even if I could get that to work, the driver code would need to be
rewritten to handle two SSI devices in concert.  In short, it might be
theoretically possible, but I'm not going to try to make it work.

> Linux struct device registrations are cheap, and every struct device
> has a device_node pointer available. =A0It is totally fine to have both
> the ssi device and the fabric device point to the same device node if
> that helps solve your problem of finding references to the right
> things in each driver. =A0(Just as long as only one of them is an
> of_platform driver).

But I already have it set up like that.  The SSI driver is an OF
driver, and the fabric driver is a platform driver.  I might be able
to move some code from the fabric driver into the SSI driver to make
it the fabric driver less obnoxious about scanning the device tree.

--=20
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Timur Tabi @ 2010-04-28 13:19 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel, kumar.gala, linuxppc-dev, Liam Girdwood
In-Reply-To: <20100427222913.GE15083@opensource.wolfsonmicro.com>

On Tue, Apr 27, 2010 at 5:29 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 codec1 :codec@4f {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "cirrus,cs427=
0";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0x4f>;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* MCLK source is a stand-al=
one oscillator */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock-frequency =3D <1228800=
0>;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
>
> You also want to be representing unused pins here.

If they're unused, how do I represent them?  Can you give me an example?

>> =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 ssi1: ssi@16000 {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-ssi";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 fsl,mode =3D "i2s-slave";
>
> I'd not include the master/slave decision; it's either implied by the
> fact that the CODEC has a standalone clock, a property of the link/card,
> or a policy decision that the running software can change on a whim.

I know it's redundant, but at the time, it seemed a lot simpler than
walking the device tree.

Frankly, I'd rather not consider minor device tree changes at this
point.  I'm hoping I don't need to change the device tree at all.

>> =A0 =A0 =A0 sound {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-hpcd-sound";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* maybe something like (totally off the top=
 of my head) */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 dai-links =3D <&ssi1 0 &codec 0
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&ssi1 1 &codec 1>=
;
>
> I'm having a hard time loving this. =A0I'd be looking for a lot more
> semantics on the links (things like information about separate clocks
> for the two directions, for example) which makes me think that that
> simple list format is far too simple and you want a list of more complex
> objects.

Yeah, I don't like it either.  It seems arbitrary.

> I think the ideal is something that's purely descriptive of the hardware
> and doesn't include any policy decisions.

I like to think that this is what we have today.

--=20
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Mark Brown @ 2010-04-28 13:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: alsa-devel, kumar.gala, linuxppc-dev, Timur Tabi, Liam Girdwood
In-Reply-To: <1272428729.24542.70.camel@pasglop>

On Wed, Apr 28, 2010 at 02:25:29PM +1000, Benjamin Herrenschmidt wrote:

> I'd stay stick to the basics and move incrementally up until it stops
> making sense:

>  - First, the basics: nodes for actual physical devices. i2c codecs on
> their i2c busses, DMA controllers, etc...

This is already supported by the ASoC core and has been for about a year
or so, it just needs the device drivers to make use of it.  IIRC the
drivers relevant to PowerPC are pretty much doing so already, but I
didn't actually check.

>  - From there, see what simple properties are better off being put in
> those respective nodes because they represent common enough variants of
> functionality or simply because they are specific attributes of a given
> device that aren't too concerned by the way things are interconnected.

This one is just a case of writing boiler plate for the existing
platform data devices have to convert that from device tree into the
existing formats.  It's not a monumentally edifying task, but I don't
think we really need to worry about it here.

>  - Provide at least one identifier (either in a "sound" virtual device,
> or just use the board's own "compatible" property) to have a .c file to
> put everything together.

This has been the big sticking point so far.  It has been difficult to
get people to accept this idea at all, the idea that there may be more
to the machine specific setup than can be readily expressed in static
data and might even vary dynamically at runtime is what seems to upset
people.  I've been told on a number of occasions that instantiating 
machine specific code that's not part of a specific chip is very
difficult and would cause the device tree to be horribly buggy.

What you're suggesting here is exactly what is supposed to happen from
an ASoC point of view.

> The whole thing is a matter of common sense and a bit of taste :-)

The impression that has been created in the past is that there are
inflexible device tree rules which can't be varied.

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Liam Girdwood @ 2010-04-28 12:49 UTC (permalink / raw)
  To: Grant Likely
  Cc: alsa-devel, kumar.gala, Mark Brown, linuxppc-dev, Graeme Gregory,
	Timur Tabi
In-Reply-To: <j2pfa686aa41004271327v1658e33bgd170ecc9735f285b@mail.gmail.com>

On Tue, 2010-04-27 at 14:27 -0600, Grant Likely wrote:

> 
> Unfortunately, I'm in the same boat.  :-(  However, I'll be at UDS in
> 2 weeks time, and I know audio is a big concern for the Ubuntu folks.
> A bunch of the ARM vendors will be there too.  I'll schedule a session
> to talk about audio bindings and hopefully that way make some headway
> on defining a binding that makes sense and is actually useful.

It's not clear if I'm going to UDS atm, but I'll join via IRC. Graeme is
going to UDS so he will probably pop in to the discussion if he's free.

Liam 

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Mark Brown @ 2010-04-28 12:07 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: alsa-devel, kumar.gala, linuxppc-dev, Timur Tabi, Liam Girdwood
In-Reply-To: <1272427811.24542.59.camel@pasglop>

On Wed, Apr 28, 2010 at 02:10:11PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2010-04-27 at 23:29 +0100, Mark Brown wrote:

> > On the other hand from a pragmatic point of view it's just much less
> > hassle to just only provide the mechanism for instantiating a machine
> > with custom code and use that for everything.

> Right, that's the balance to find. A too descriptive device-tree becomes
> a mess, and attempting to deal with any kind of representation in SW is
> horrible.

Sadly this often seems to be what the device tree folks are pushing
for.  I really want whatever you guys come up with to explicitly say
that it's OK to just write standard code like we have at the minute and
not faff around defining device tree representations for everything so
that we can just point people at that when the discussion gets too
bogged down.

> For example, one could imagine a /sound node with simply a "compatible"
> property that matches what we call in AOA terminology a "fabric" driver.
> IE. The one thing specific to the board that puts bits and pieces
> together.

This is the current ASoC design (someone probably ought to look at
merging AOA into ASoC, it's approximately the same hardware and at least
the CPU driver ought to be useful for other systems).

> Now, the device-tree is still obviously useful to provide things like
> the actual i2c IDs of codecs, GPIOs used for various actions, link to
> from various components to their clock source devices, etc.. All these
> things simplify the code, avoids horrid board specific code in the
> actual drivers (codecs, busses, etc...) and overall help keeping the
> code more maintainable. 

You're preaching to the choir here.  With ASoC all this system specific
code ends up in the machine driver (which you guys are calling the
fabric driver).  All the design stuff you're talking about here is
already dealt with as-is, it's just that the parameterisation is done
with C code and data structures in the kernel (which can deal with
multiple boards if it chooses to) rather than with device tree stuff.
The chip specific drivers do not have any board specific code so there
is no meaningful change that's being proposed to them.

The problems with the device tree have been that people have been
hostile to the idea of there being any board specific code at all in the
kernel (or a board specific device tree node for the audio), and that
people keep wanting to define some OF stuff that is supposed to cover a
wide range of boards but makes unrealistic simplifying assumptions about
what general hardware looks like.

> The device-tree helps keep the platform .c file simple and devoid of too
> horrible hacks, it allows to easily pass various configuration data to
> leaf drivers such as i2c thingies, PHY devices etc... without gross
> hooks between these and the platform, but the platform code still has
> the upper hand for doing ad-hoc bits and pieces (or overwriting the
> device-tree based behaviour) if necessary.

Once again, if you can get the device tree guys to buy into this and
stick with it that sounds good but my experience has been that this
isn't where any of these discussions end up.

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Mark Brown @ 2010-04-28  9:16 UTC (permalink / raw)
  To: Grant Likely
  Cc: alsa-devel, kumar.gala, linuxppc-dev, Timur Tabi, Liam Girdwood
In-Reply-To: <i2vfa686aa41004271931wc04b9c11l5f3ebda0acb14b9@mail.gmail.com>

On Tue, Apr 27, 2010 at 08:31:18PM -0600, Grant Likely wrote:
> On Tue, Apr 27, 2010 at 4:29 PM, Mark Brown

> > On the other hand from a pragmatic point of view it's just much less
> > hassle to just only provide the mechanism for instantiating a machine
> > with custom code and use that for everything.

> Also true, but this approach carries with it an incremental cost that
> distributions feel the pain of.  Ultimately I think we'll find a sweet
> spot somewhere in between.

Meh, it's not really much hassle for the distributions - it's all
handled by the kernel, they don't need to explicitly do anything.

None of the machine-specific stuff has ever been a hassle getting stuff
merged, problems have always been in the drivers for the devices which
device tree isn't going to make a blind bit of difference to (other than
the usual discussions about what exactly the device tree should look
like).

^ permalink raw reply

* change_page_attr support on ppc, patch discussion
From: Maindoor @ 2010-04-28  8:27 UTC (permalink / raw)
  To: linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]

Hi,
I am trying to set a page of my data-structure as read-only on a USI MPC8347E.
This is primarily to figure a data structure corruption issue.
Added small support as follows:
ppc/mm/init.c:MMU_setup

    __map_without_bats = 1;
    __map_without_ltlbs = 1;


ppc/mm/pgtable.c

static int __change_page_attr(struct page *page, pgprot_t prot)
{
    pte_t *kpte;
    pmd_t *kpmd;
    unsigned long address;

    BUG_ON(PageHighMem(page));
    address = (unsigned long)page_address(page);

    if (v_mapped_by_bats(address) || v_mapped_by_tlbcam(address))
        return 0;
    if (!get_pteptr(&init_mm, address, &kpte, &kpmd))
        return -EINVAL;
    set_pte_at(&init_mm, address, kpte, mk_pte(page, prot));
    wmb();
    flush_HPTE(0, address, pmd_val(*kpmd));
    pte_unmap(kpte);
    mb();
    return 0;
}

/*
 * Change the page attributes of an page in the linear mapping.
*/
int change_page_attr(struct page *page, int numpages, pgprot_t prot)
{
    int i, err = 0;
    unsigned long flags;

    local_irq_save(flags);
    for (i = 0; i < numpages; i++, page++) {
        err = __change_page_attr(page, prot);
        if (err)
            break;
    }
    local_irq_restore(flags);
    return err;
}
EXPORT_SYMBOL(change_page_attr);

After allocation of my data structure, when I mark the page readonly like this:
change_page_attr(virt_to_page(dat_str), 1, PAGE_READONLY);

it works (throws Oops) but only for the first time, but subsequent writes
are going through. I expect even the subsequent writes to fail.

One more quirkiness to this is, after changing the page attribute I need to
read the element at least once for this to work.

Have I written the supportive api's (above ) correctly ? This is on 2.6.18 kernel.
If there is any other way to catch this corruption please let me know.
kgdb and bdi won't help me as it is a dynamically created data-structure and I
have timing constraints.

Regards,
Maindoor.




      

[-- Attachment #2: Type: text/html, Size: 2827 bytes --]

^ permalink raw reply

* [PATCH v2] powerpc/4xx: Add optional "reset_type" property to control reboot via dts
From: Stefan Roese @ 2010-04-28  8:13 UTC (permalink / raw)
  To: linuxppc-dev

By setting "reset_type" to one of the following values, the default
software reset mechanism may be overidden. Here the possible values of
"reset_type":

  1 - PPC4xx core reset
  2 - PPC4xx chip reset
  3 - PPC4xx system reset (default)

This will be used by a new PPC440SPe board port, which needs a "chip
reset" instead of the default "system reset" to be asserted.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Josh Boyer <jwboyer@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
v2:
- Add small property description to Documentation
- Add sanity check for property value

 Documentation/powerpc/dts-bindings/4xx/reboot.txt |   18 +++++++++++++++
 arch/powerpc/sysdev/ppc4xx_soc.c                  |   24 +++++++++++++++++++-
 2 files changed, 40 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/powerpc/dts-bindings/4xx/reboot.txt

diff --git a/Documentation/powerpc/dts-bindings/4xx/reboot.txt b/Documentation/powerpc/dts-bindings/4xx/reboot.txt
new file mode 100644
index 0000000..d721726
--- /dev/null
+++ b/Documentation/powerpc/dts-bindings/4xx/reboot.txt
@@ -0,0 +1,18 @@
+Reboot property to control system reboot on PPC4xx systems:
+
+By setting "reset_type" to one of the following values, the default
+software reset mechanism may be overidden. Here the possible values of
+"reset_type":
+
+      1 - PPC4xx core reset
+      2 - PPC4xx chip reset
+      3 - PPC4xx system reset (default)
+
+Example:
+
+		cpu@0 {
+			device_type = "cpu";
+			model = "PowerPC,440SPe";
+			...
+			reset-type = <2>;	/* Use chip-reset */
+		};
diff --git a/arch/powerpc/sysdev/ppc4xx_soc.c b/arch/powerpc/sysdev/ppc4xx_soc.c
index 5c01435..d3d6ce3 100644
--- a/arch/powerpc/sysdev/ppc4xx_soc.c
+++ b/arch/powerpc/sysdev/ppc4xx_soc.c
@@ -191,11 +191,31 @@ static int __init ppc4xx_l2c_probe(void)
 arch_initcall(ppc4xx_l2c_probe);
 
 /*
- * At present, this routine just applies a system reset.
+ * Apply a system reset. Alternatively a board specific value may be
+ * provided via the "reset-type" property in the cpu node.
  */
 void ppc4xx_reset_system(char *cmd)
 {
-	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) | DBCR0_RST_SYSTEM);
+	struct device_node *np;
+	u32 reset_type = DBCR0_RST_SYSTEM;
+	const u32 *prop;
+
+	np = of_find_node_by_type(NULL, "cpu");
+	if (np) {
+		prop = of_get_property(np, "reset-type", NULL);
+
+		/*
+		 * Check if property exists and if it is in range:
+		 * 1 - PPC4xx core reset
+		 * 2 - PPC4xx chip reset
+		 * 3 - PPC4xx system reset (default)
+		 */
+		if ((prop) && ((prop[0] >= 1) && (prop[0] <= 3)))
+			reset_type = prop[0] << 28;
+	}
+
+	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) | reset_type);
+
 	while (1)
 		;	/* Just in case the reset doesn't work */
 }
-- 
1.7.1

^ permalink raw reply related

* [PATCH] Fix DEBUG_PAGEALLOC on 603/e300
From: Benjamin Herrenschmidt @ 2010-04-28  7:22 UTC (permalink / raw)
  To: linuxppc-dev

So we tried to speed things up a bit using flush_hash_pages() directly
but that falls over on 603 of course meaning we fail to flush the TLB
properly and we may even end up having it corrupt memory randomly by
accessing a hash table that doesn't exist.

This removes the "optimization" by always going through flush_tlb_page()
for now at least.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Somebody with a 603 or e300 core based FSL SoC to try this out for me ?

It's obviously completely untested :-)

Cheers,
Ben.

diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index b9243e7..95774b4 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -385,11 +385,7 @@ static int __change_page_attr(struct page *page, pgprot_t prot)
 		return -EINVAL;
 	__set_pte_at(&init_mm, address, kpte, mk_pte(page, prot), 0);
 	wmb();
-#ifdef CONFIG_PPC_STD_MMU
-	flush_hash_pages(0, address, pmd_val(*kpmd), 1);
-#else
 	flush_tlb_page(NULL, address);
-#endif
 	pte_unmap(kpte);
 
 	return 0;

^ permalink raw reply related

* Re: [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2010-04-28  7:02 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linuxppc-dev list, Andrew Morton, Linux Kernel list
In-Reply-To: <1272347770.24542.1.camel@pasglop>

On Tue, 2010-04-27 at 15:56 +1000, Benjamin Herrenschmidt wrote:
> Hi Linus !
> 
> PowerPC has been a bit quiet this time around :-) That is until Kumar
> woke me up with a few fixes and defconfig updates for the freescale
> embedded platforms.

I added a few patches to that. Some ps3 defconfig & maintainers change
and a couple of nasty bug fixes.

Here's an updated request.

Cheers,
Ben.

The following changes since commit b91ce4d14a21fc04d165be30319541e0f9204f15:
  Linus Torvalds (1):
        Merge git://git.kernel.org/.../davem/net-2.6

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge

Anton Blanchard (1):
      powerpc/numa: Add form 1 NUMA affinity

Anton Vorontsov (1):
      powerpc/85xx/86xx: Fix build w/ CONFIG_PCI=n

Becky Bruce (1):
      powerpc/fsl_booke: Correct test for MMU_FTR_BIG_PHYS

Benjamin Herrenschmidt (1):
      powerpc/pseries: Flush lazy kernel mappings after unplug operations

Geoff Levand (2):
      powerpc/ps3: Update platform maintainer
      powerpc/ps3: Update ps3_defconfig

Kim Phillips (4):
      powerpc/83xx: add RTC drivers in 83xx defconfig
      powerpc/83xx: enable EPOLL syscall in defconfig
      powerpc/83xx: configure SIL SATA driver in 83xx-wide defconfig
      powerpc/mpc8xxx defconfigs - turn off SYSFS_DEPRECATED

Kumar Gala (2):
      powerpc: 2.6.34 update of defconfigs for embedded 6xx/7xxx, 8xx, 8xxx
      powerpc/fsl-booke: Fix CONFIG_RELOCATABLE support on FSL Book-E ppc32

Wolfgang Ocker (1):
      powerpc/fsl-cpm: Configure clock correctly for SCC

 MAINTAINERS                                       |    8 +-
 arch/powerpc/configs/83xx/asp8347_defconfig       |   44 +++---
 arch/powerpc/configs/83xx/kmeter1_defconfig       |   28 ++--
 arch/powerpc/configs/83xx/mpc8313_rdb_defconfig   |   49 +++---
 arch/powerpc/configs/83xx/mpc8315_rdb_defconfig   |   50 +++---
 arch/powerpc/configs/83xx/mpc832x_mds_defconfig   |   45 +++---
 arch/powerpc/configs/83xx/mpc832x_rdb_defconfig   |   49 +++---
 arch/powerpc/configs/83xx/mpc834x_itx_defconfig   |   44 +++---
 arch/powerpc/configs/83xx/mpc834x_itxgp_defconfig |   43 +++---
 arch/powerpc/configs/83xx/mpc834x_mds_defconfig   |   46 +++---
 arch/powerpc/configs/83xx/mpc836x_mds_defconfig   |   45 +++---
 arch/powerpc/configs/83xx/mpc836x_rdk_defconfig   |   44 +++--
 arch/powerpc/configs/83xx/mpc837x_mds_defconfig   |   46 +++---
 arch/powerpc/configs/83xx/mpc837x_rdb_defconfig   |   51 +++---
 arch/powerpc/configs/83xx/sbc834x_defconfig       |   44 +++---
 arch/powerpc/configs/85xx/ksi8560_defconfig       |   23 ++-
 arch/powerpc/configs/85xx/mpc8540_ads_defconfig   |   25 ++-
 arch/powerpc/configs/85xx/mpc8560_ads_defconfig   |   34 +++--
 arch/powerpc/configs/85xx/mpc85xx_cds_defconfig   |   31 +++--
 arch/powerpc/configs/85xx/sbc8548_defconfig       |   31 +++--
 arch/powerpc/configs/85xx/sbc8560_defconfig       |   25 ++-
 arch/powerpc/configs/85xx/socrates_defconfig      |   51 ++++---
 arch/powerpc/configs/85xx/stx_gp3_defconfig       |   51 ++++---
 arch/powerpc/configs/85xx/tqm8540_defconfig       |   48 +++--
 arch/powerpc/configs/85xx/tqm8541_defconfig       |   54 ++++--
 arch/powerpc/configs/85xx/tqm8548_defconfig       |   47 +++--
 arch/powerpc/configs/85xx/tqm8555_defconfig       |   54 ++++--
 arch/powerpc/configs/85xx/tqm8560_defconfig       |   54 ++++--
 arch/powerpc/configs/85xx/xes_mpc85xx_defconfig   |   59 +++++--
 arch/powerpc/configs/86xx/gef_ppc9a_defconfig     |  102 +++++++-----
 arch/powerpc/configs/86xx/gef_sbc310_defconfig    |   53 ++++---
 arch/powerpc/configs/86xx/gef_sbc610_defconfig    |   96 +++++++----
 arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig  |   34 ++--
 arch/powerpc/configs/86xx/mpc8641_hpcn_defconfig  |   48 ++++--
 arch/powerpc/configs/86xx/sbc8641d_defconfig      |   40 +++--
 arch/powerpc/configs/adder875_defconfig           |   18 +-
 arch/powerpc/configs/c2k_defconfig                |   55 +++----
 arch/powerpc/configs/ep8248e_defconfig            |   20 ++-
 arch/powerpc/configs/ep88xc_defconfig             |   17 +-
 arch/powerpc/configs/linkstation_defconfig        |   48 +++--
 arch/powerpc/configs/mgcoge_defconfig             |   31 +++-
 arch/powerpc/configs/mgsuvd_defconfig             |   19 ++-
 arch/powerpc/configs/mpc7448_hpc2_defconfig       |   26 ++-
 arch/powerpc/configs/mpc8272_ads_defconfig        |   23 ++-
 arch/powerpc/configs/mpc83xx_defconfig            |  124 +++++++++++---
 arch/powerpc/configs/mpc85xx_defconfig            |   62 +++++---
 arch/powerpc/configs/mpc85xx_smp_defconfig        |   64 +++++---
 arch/powerpc/configs/mpc866_ads_defconfig         |   20 +-
 arch/powerpc/configs/mpc86xx_defconfig            |   57 ++++---
 arch/powerpc/configs/mpc885_ads_defconfig         |   20 +-
 arch/powerpc/configs/pq2fads_defconfig            |   22 ++-
 arch/powerpc/configs/prpmc2800_defconfig          |   52 ++++--
 arch/powerpc/configs/ps3_defconfig                |  189 ++++++++++++++-------
 arch/powerpc/configs/storcenter_defconfig         |   33 ++--
 arch/powerpc/include/asm/page.h                   |   15 ++-
 arch/powerpc/kernel/prom_init.c                   |    3 +-
 arch/powerpc/mm/fsl_booke_mmu.c                   |    4 +-
 arch/powerpc/mm/numa.c                            |   17 ++-
 arch/powerpc/platforms/85xx/Kconfig               |    4 +-
 arch/powerpc/platforms/86xx/Kconfig               |    4 +-
 arch/powerpc/platforms/pseries/hotplug-memory.c   |    7 +
 arch/powerpc/sysdev/cpm1.c                        |   14 ++-
 arch/powerpc/sysdev/cpm2.c                        |   11 +-
 drivers/pci/hotplug/rpadlpar_core.c               |    3 +
 drivers/pci/hotplug/rpaphp_core.c                 |    3 +
 65 files changed, 1609 insertions(+), 972 deletions(-)

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Grant Likely @ 2010-04-28  5:37 UTC (permalink / raw)
  To: Timur Tabi; +Cc: alsa-devel, kumar.gala, broonie, linuxppc-dev, Liam Girdwood
In-Reply-To: <4BD74D0C.40303@freescale.com>

On Tue, Apr 27, 2010 at 2:46 PM, Timur Tabi <timur@freescale.com> wrote:
> Grant Likely wrote:
>
>> So, the current 86xx device tree binding assumes a simple layout with
>> a node describing an DAI controller, and another node describing the
>> codec with a single phandle (pointer) from the DAI to the codec. =A0In
>> this configuration, it is completely reasonable for the DAI node to
>> trigger both the instantiation of the ASoC DAI controller device and
>> the sound card device. =A0Linux can treat them as separate even though
>> the current device tree has a simplistic representation.
>
> The only problem with this is that there is board-specific programming th=
at needs to be done (look at mpc8610_hpcd_machine_probe), so we still need =
to have a fabric driver that is independent of the SSI, codec, or DMA drive=
rs. =A0The P1022 also has an SSI, and I'm hoping that all I need to do is c=
reate a new fabric driver, not hack up the SSI driver to support board prog=
ramming.
>
> So if the fabric driver still needs to exist, then it still needs a struc=
t device, and it still needs to register with asoc. =A0I don't see how I ca=
n register the sound card itself in the SSI driver, because it won't know a=
nything about the board-specific code in the fabric driver.

Why not?  Just have the ssi driver probe routine register the fabric
device based on the existence of the codec-handle property.  It is the
best way to go about things with the data that you've got available,
and it is no big deal.  The relevant fabric driver can then bind
against that.  You should probably also stuff the ssi device node
pointer into the fabric device of_node pointer.

>> I've tried very hard to maintain a distinction between device tree
>> binding (representation) and Linux kernel internal implementation
>> details. =A0The real question is whether or not the binding provides
>> sufficient detail for the operating system to figure out what to do.
>
> I think it does, because it's working today.
>
>> In the extreme minimalist case, the audio driver could decide how to
>> configure itself solely on the board name property of the root node.
>> There is nothing wrong with that, but it also means that no data is
>> available to dynamically select common modules or modify connections;
>> it all has to be hard coded.
>
> Well, asoc already has several hard-coded requirements:
>
> =A0 =A0 =A0 =A0machine_data->dai.cpu_dai_drv =3D &fsl_ssi_dai;
> =A0 =A0 =A0 =A0machine_data->dai.codec_dai_drv =3D &cs4270_dai;
> =A0 =A0 =A0 =A0machine_data->dai.codec_drv =3D &soc_codec_device_cs4270;
> =A0 =A0 =A0 =A0machine_data->dai.ops =3D &mpc8610_hpcd_ops;
> =A0 =A0 =A0 =A0machine_data->dai.platform_drv =3D &fsl_soc_platform;
>
> So even though I probe for each device separately and register them separ=
ately, the fabric driver still needs to have hard-coded addresses
>
> Maybe the asoc guys can tell me why I need to register the cpu_dai_drv st=
ructure via platform_device_add(), when it's already being registered via s=
nd_soc_register_dai().
>
>> (I've omitted the DMA nodes and some irrelevant details) =A0This is
>> enough information for a simplistic driver registration that probably
>> makes a lot of assumptions. =A0Such as the ssi represents a single
>> logical sound device. =A0It won't handle complex representations, but in
>> a lot of cases that may be just fine.
>
> Why would I ever represent the SSI as anything but a single logical sound=
 device? =A0Let ALSA handle synchronizing multiple streams together if it w=
ants to.
>
>> =A0 =A0 =A0 sound {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-hpcd-sound";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* maybe something like (totally off the top=
 of my head) */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 dai-links =3D <&ssi1 0 &codec 0
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&ssi1 1 &codec 1>=
;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 };
>
> I don't know when I would ever do this. =A0The two SSI devices are comple=
tely independent. =A0Why would I bind them together into one "device"?

If there is no use case for binding them together, then you're right.
The current binding is probably just fine.  I cannot comment on
whether or not it will be used that way by platform designers.

>> Where the 'sound' node is now the starting point for representing a
>> logical sound device instead of the ssi node. =A0This binding probably
>> makes more sense (but I'm not committing to anything like this until I
>> see a real proposal for a real device).
>
> The only issue I have with this is all it does is turn the fabric driver =
from a platform driver that scans the OF tree, into an OF driver that still=
 needs to query the device tree to get all the data it needs. =A0For exampl=
e, the fabric driver still needs to know the clock frequency and direction =
of the codec node, so that it can call snd_soc_dai_set_fmt() and snd_soc_da=
i_set_sysclk() properly. =A0To eliminate that, we could have the fabric dri=
ver never call these functions, and expect the SSI and codec drivers to gat=
her this information itself. =A0But the codec driver is not an OF driver, s=
o it has no expectation of being able to query the codec node.
>
>> I would solve the problem this way: In the ssi driver, if the
>> codec-node property is present, then call a function to instantiate a
>> simple or platform specific sound card instance that makes the
>> assumptions listed above. =A0If not, then just register the ssi and
>> exit, which leaves the ssi available for a separate driver to pick it
>> up. =A0I wouldn't do this for new platforms, but it gracefully makes use
>> of the data provided in the current 8610 device tree.
>
> Eh, I'll have to think about that. =A0The absence of a codec pointer in t=
he SSI node means that the SSI is not connected to a codec, so it should ju=
st be ignored altogether. =A0An SSI is useless if it's not connected to a c=
odec.

I'm suggesting the case where a third node describes the mappings
between codecs and SSIs.

>> BTW Timur, there is nothing wrong with registering multiple devices
>> that all have the of_node pointer set to the same node.
>
> Sorry, I don't understand what you're getting at.

Linux struct device registrations are cheap, and every struct device
has a device_node pointer available.  It is totally fine to have both
the ssi device and the fabric device point to the same device node if
that helps solve your problem of finding references to the right
things in each driver.  (Just as long as only one of them is an
of_platform driver).

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [microblaze-uclinux] [PATCHv2] [RFC] Xilinx MPMC SDMA subsystem
From: Grant Likely @ 2010-04-28  5:13 UTC (permalink / raw)
  To: steve, Sergey Temerkhanov
  Cc: microblaze-uclinux, linuxppc-dev, Linux Kernel Mailing List
In-Reply-To: <1272421789.2877.21.camel@iscandar.digidescorp.com>

Hi Sergey and Steven,

On Tue, Apr 27, 2010 at 8:29 PM, Steven J. Magnani
<steve@digidescorp.com> wrote:
> On Wed, 2010-04-28 at 02:06 +0400, Sergey Temerkhanov wrote:
>> This is the 2nd version of Xilinx MPMC LocalLink SDMA subsystem
>>
>> Changelog v2:
>> * Changed the functions and struct definition prefix from sdma_ to xllsd=
ma_
>> * Platform bus bindings and various changes by Steven J. Magnani.
>> * Moved source files from arch/powerpc/sysdev to global locations and ad=
ded
>> =A0 CONFIG_XLLSDMA option.
>>
>> Regards, Sergey Temerkhanov,
>> Cifronic ZAO
>>
>> diff -r baced9e29ab5 drivers/dma/Kconfig
>> --- a/drivers/dma/Kconfig =A0 =A0 Tue Apr 27 20:48:50 2010 +0400
>> +++ b/drivers/dma/Kconfig =A0 =A0 Wed Apr 28 02:00:51 2010 +0400
>> @@ -97,6 +97,14 @@
>> =A0 =A0 =A0 =A0 Support the TXx9 SoC internal DMA controller. =A0This ca=
n be
>> =A0 =A0 =A0 =A0 integrated in chips such as the Toshiba TX4927/38/39.
>>
>> +config XLLSDMA

I'd prefer XILINX_LLSDMA.  XLLSDMA could be ambiguous in the global
namespace (for a human reader), particularly as it is something that
few people will actually see.

>> + =A0 =A0 bool "Xilinx MPMC DMA support"
>> + =A0 =A0 depends on XILINX_VIRTEX || MICROBLAZE
>> + =A0 =A0 select DMA_ENGINE
>> + =A0 =A0 help
>> + =A0 =A0 =A0 Support fot Xilinx MPMC LocalLink SDMA. Virtex FPGA family
>> + =A0 =A0 =A0 has it integrated or fabric-based.
>> +
>
> fot --> for
>
> Since the xllsdma driver provides services to other drivers - not to user=
land -
> I think this would be better as a "silent" option, selected automatically=
 when
> something like ll_temac or the forthcoming Xilinx DMA engine is selected.
> If we do it that way, note that XLLSDMA is independent of DMA_ENGINE so
> drivers/Makefile will need to be patched so that the dma subdirectory is
> always "y".

Agreed.  However, looking at this code, I don't see anything that
actually uses DMA_ENGINE here.  Am I missing something?

>> diff -r baced9e29ab5 drivers/dma/Makefile
>> --- a/drivers/dma/Makefile =A0 =A0Tue Apr 27 20:48:50 2010 +0400
>> +++ b/drivers/dma/Makefile =A0 =A0Wed Apr 28 02:00:51 2010 +0400
>> @@ -10,3 +10,4 @@
>> =A0obj-$(CONFIG_AT_HDMAC) +=3D at_hdmac.o
>> =A0obj-$(CONFIG_MX3_IPU) +=3D ipu/
>> =A0obj-$(CONFIG_TXX9_DMAC) +=3D txx9dmac.o
>> +obj-$(CONFIG_XLLSDMA) +=3D xllsdma.o
>> diff -r baced9e29ab5 drivers/dma/xllsdma.c
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/drivers/dma/xllsdma.c =A0 Wed Apr 28 02:00:51 2010 +0400
>> @@ -0,0 +1,887 @@
>> +/*
>> + * SDMA subsystem support for Xilinx MPMC.
>> + *
>> + * Author: Sergey Temerkhanov
>> + * Platform Bus by Steven J. Magnani
>> + *
>> + * Copyright (c) 2008-2010 Cifronic ZAO
>> + *
>> + * This program is free software; you can redistribute =A0it and/or mod=
ify it
>> + * under =A0the terms of =A0the GNU General =A0Public License as publis=
hed by the
>> + * Free Software Foundation; either version 2 of the License, or (at yo=
ur
>> + * option) any later version.
>> + *
>> + */
>> +
>> +#include <linux/delay.h>
>> +#include <linux/kernel.h>
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/errno.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/mutex.h>
>> +#include <linux/wait.h>
>> +#include <linux/list.h>
>> +#include <linux/io.h>
>> +#include <linux/xllsdma.h>
>> +
>> +#include <linux/of_device.h>
>> +#include <linux/of_platform.h>
>> +
>> +#define DRV_VERSION "0.1.0"

Irrelevant, can be dropped

>> +#define DRV_NAME "sdma"

Used only once, drop.

>> +
>> +MODULE_AUTHOR("Sergey Temerkhanov <temerkhanov@cifronik.ru>");
>> +MODULE_DESCRIPTION("Xilinx SDMA driver");
>> +MODULE_LICENSE("GPL");
>> +MODULE_VERSION(DRV_VERSION);
>> +
>> +LIST_HEAD(mpmc_devs);
>> +DEFINE_MUTEX(mpmc_devs_lock);
>> +
>> +void xllsdma_tx_irq_enable(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 u32 tx_cr;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 BUG_ON(sdma->tx_irq =3D=3D NO_IRQ);
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> + =A0 =A0 tx_cr =3D xllsdma_tx_in32(sdma, XLLSDMA_CR);
>> + =A0 =A0 xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr | XLLSDMA_CR_IRQ_EN);
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);

This pattern is used a lot.  Might be worth while to implement
xllsdma_tx_* variants of setbits32, clrbits32 and clrsetbits32.

Also, there are a lot of these little functions; really trivial and
small.  Would it be better to have them as static inlines in the
header file instead of exported globals?

>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_tx_irq_enable);
>> +
>> +void xllsdma_rx_irq_enable(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 u32 rx_cr;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 BUG_ON(sdma->rx_irq =3D=3D NO_IRQ);
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> + =A0 =A0 rx_cr =3D xllsdma_rx_in32(sdma, XLLSDMA_CR);
>> + =A0 =A0 xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr | XLLSDMA_CR_IRQ_EN);
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_rx_irq_enable);
>> +
>> +void xllsdma_tx_irq_disable(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 u32 tx_cr;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> + =A0 =A0 tx_cr =3D xllsdma_tx_in32(sdma, XLLSDMA_CR);
>> + =A0 =A0 xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr & ~XLLSDMA_CR_IRQ_EN)=
;
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_tx_irq_disable);
>> +
>> +void xllsdma_rx_irq_disable(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 u32 rx_cr;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> + =A0 =A0 rx_cr =3D xllsdma_rx_in32(sdma, XLLSDMA_CR);
>> + =A0 =A0 xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr & ~XLLSDMA_CR_IRQ_EN)=
;
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_rx_irq_disable);
>> +
>> +void xllsdma_tx_irq_ack(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 u32 irq_stat;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> + =A0 =A0 irq_stat =3D xllsdma_tx_in32(sdma, XLLSDMA_IRQ);
>> + =A0 =A0 xllsdma_tx_out32(sdma, XLLSDMA_IRQ, irq_stat & XLLSDMA_IRQ_ALL=
_DONE);
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_tx_irq_ack);
>> +
>> +void xllsdma_rx_irq_ack(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 u32 irq_stat;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> + =A0 =A0 irq_stat =3D xllsdma_rx_in32(sdma, XLLSDMA_IRQ);
>> + =A0 =A0 xllsdma_rx_out32(sdma, XLLSDMA_IRQ, irq_stat & XLLSDMA_IRQ_ALL=
_DONE);
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_rx_irq_ack);
>> +
>> +void xllsdma_pause(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 u32 dmacr;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> + =A0 =A0 dmacr =3D xllsdma_read_cr(sdma);
>> + =A0 =A0 dmacr |=3D XLLSDMA_DMACR_TX_PAUSE | XLLSDMA_DMACR_RX_PAUSE;
>> + =A0 =A0 xllsdma_write_cr(sdma, dmacr);
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_pause);
>> +
>> +void xllsdma_resume(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 u32 dmacr;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> + =A0 =A0 dmacr =3D xllsdma_read_cr(sdma);
>> + =A0 =A0 dmacr &=3D ~(XLLSDMA_DMACR_TX_PAUSE | XLLSDMA_DMACR_RX_PAUSE);
>> + =A0 =A0 xllsdma_write_cr(sdma, dmacr);
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_resume);
>> +
>> +int xllsdma_set_coalesce(struct xllsdma_device *sdma, struct xllsdma_co=
alesce *coal)
>> +{
>> + =A0 =A0 u32 tx_cr, rx_cr;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 if (coal->tx_timeout > 255 ||
>> + =A0 =A0 =A0 =A0 coal->rx_timeout > 255 ||
>> + =A0 =A0 =A0 =A0 coal->tx_threshold > 255 ||
>> + =A0 =A0 =A0 =A0 coal->rx_threshold > 255)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> +
>> + =A0 =A0 if (sdma->rx_irq !=3D NO_IRQ) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 rx_cr =3D xllsdma_rx_in32(sdma, XLLSDMA_CR);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (coal->rx_timeout =3D=3D 0) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 coal->rx_timeout =3D 1;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rx_cr &=3D ~XLLSDMA_CR_IRQ_TIM=
EOUT;
>> + =A0 =A0 =A0 =A0 =A0 =A0 } else {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rx_cr |=3D XLLSDMA_CR_IRQ_TIME=
OUT;
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 rx_cr &=3D ~(XLLSDMA_CR_IRQ_THRESHOLD_MSK | XL=
LSDMA_CR_IRQ_TIMEOUT_SH);
>> + =A0 =A0 =A0 =A0 =A0 =A0 rx_cr |=3D (coal->rx_threshold << XLLSDMA_CR_I=
RQ_THRESHOLD_SH)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0& XLLSDMA_CR_IRQ_THRESHOLD_=
MSK;
>> + =A0 =A0 =A0 =A0 =A0 =A0 rx_cr |=3D (coal->rx_timeout << XLLSDMA_CR_IRQ=
_TIMEOUT_SH)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0& XLLSDMA_CR_IRQ_TIMEOUT_MS=
K;
>> + =A0 =A0 =A0 =A0 =A0 =A0 rx_cr |=3D XLLSDMA_CR_LD_IRQ_CNT;
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr);
>> + =A0 =A0 }
>> +
>> + =A0 =A0 if (sdma->tx_irq !=3D NO_IRQ) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 tx_cr =3D xllsdma_tx_in32(sdma, XLLSDMA_CR);
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (coal->tx_timeout =3D=3D 0) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 coal->tx_timeout =3D 1;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tx_cr &=3D ~XLLSDMA_CR_IRQ_TIM=
EOUT;
>> + =A0 =A0 =A0 =A0 =A0 =A0 } else {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tx_cr |=3D XLLSDMA_CR_IRQ_TIME=
OUT;
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 tx_cr &=3D ~(XLLSDMA_CR_IRQ_THRESHOLD_MSK | XL=
LSDMA_CR_IRQ_TIMEOUT_SH);
>> + =A0 =A0 =A0 =A0 =A0 =A0 tx_cr |=3D (coal->tx_threshold << XLLSDMA_CR_I=
RQ_THRESHOLD_SH)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0& XLLSDMA_CR_IRQ_THRESHOLD_=
MSK;
>> + =A0 =A0 =A0 =A0 =A0 =A0 tx_cr |=3D (coal->tx_timeout << XLLSDMA_CR_IRQ=
_TIMEOUT_SH)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0& XLLSDMA_CR_IRQ_TIMEOUT_MS=
K;
>> + =A0 =A0 =A0 =A0 =A0 =A0 tx_cr |=3D XLLSDMA_CR_LD_IRQ_CNT;
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr);
>> + =A0 =A0 }
>> +
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +
>> + =A0 =A0 return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_set_coalesce);
>> +
>> +int xllsdma_get_coalesce(struct xllsdma_device *sdma, struct xllsdma_co=
alesce *coal)
>> +{
>> + =A0 =A0 u32 tx_cr, rx_cr;
>> + =A0 =A0 unsigned long flags;
>> +
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> +
>> + =A0 =A0 tx_cr =3D xllsdma_tx_in32(sdma, XLLSDMA_CR);
>> + =A0 =A0 rx_cr =3D xllsdma_rx_in32(sdma, XLLSDMA_CR);
>> +
>> + =A0 =A0 coal->tx_threshold =3D (tx_cr & XLLSDMA_CR_IRQ_THRESHOLD_MSK)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0>> XLLSDMA_CR_IRQ_T=
HRESHOLD_SH;
>> + =A0 =A0 coal->tx_timeout =3D (tx_cr & XLLSDMA_CR_IRQ_TIMEOUT_MSK)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0>> XLLSDMA_CR_IRQ_TIMEO=
UT_SH;
>> +
>> + =A0 =A0 coal->rx_threshold =3D (rx_cr & XLLSDMA_CR_IRQ_THRESHOLD_MSK)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0>> XLLSDMA_CR_IRQ_T=
HRESHOLD_SH;
>> + =A0 =A0 coal->rx_timeout =3D (rx_cr & XLLSDMA_CR_IRQ_TIMEOUT_MSK)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0>> XLLSDMA_CR_IRQ_T=
IMEOUT_SH;
>> +
>> + =A0 =A0 if (!(tx_cr & XLLSDMA_CR_IRQ_TIMEOUT))
>> + =A0 =A0 =A0 =A0 =A0 =A0 coal->tx_timeout =3D 0;
>> +
>> + =A0 =A0 if (!(rx_cr & XLLSDMA_CR_IRQ_TIMEOUT))
>> + =A0 =A0 =A0 =A0 =A0 =A0 coal->rx_timeout =3D 0;
>> +
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +
>> + =A0 =A0 return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_get_coalesce);
>> +
>> +int xllsdma_tx_submit(struct xllsdma_device *sdma, dma_addr_t desc)
>> +{
>> + =A0 =A0 xllsdma_tx_out32(sdma, XLLSDMA_TDESCR, desc);
>> + =A0 =A0 return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_tx_submit);
>
> Invariant return value can be dropped.

Also are more tiny functions that could be considered for static inlines.

>> +int xllsdma_rx_submit(struct xllsdma_device *sdma, dma_addr_t desc)
>> +{
>> + =A0 =A0 xllsdma_rx_out32(sdma, XLLSDMA_TDESCR, desc);
>> + =A0 =A0 return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_rx_submit);
>>
> Invariant return value can be dropped.
>
>> +
>> +void xllsdma_tx_init(struct xllsdma_device *sdma, dma_addr_t desc)
>> +{
>> + =A0 =A0 xllsdma_tx_out32(sdma, XLLSDMA_CDESCR, desc);
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_tx_init);
>> +
>> +void xllsdma_rx_init(struct xllsdma_device *sdma, dma_addr_t desc)
>> +{
>> + =A0 =A0 xllsdma_rx_out32(sdma, XLLSDMA_CDESCR, desc);
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_rx_init);
>> +
>> +struct xllsdma_device *xllsdma_find_device(int phandle)
>> +{
>> + =A0 =A0 struct mpmc_device *mpmc;
>> + =A0 =A0 struct xllsdma_device *sdma =3D NULL;
>> + =A0 =A0 int found =3D 0;
>> + =A0 =A0 mutex_lock(&mpmc_devs_lock);
>> + =A0 =A0 list_for_each_entry(mpmc, &mpmc_devs, item) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 mutex_lock(&mpmc->devs_lock);
>> + =A0 =A0 =A0 =A0 =A0 =A0 list_for_each_entry(sdma, &mpmc->xllsdma_devs,=
 item) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (sdma->phandle =3D=3D phand=
le) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 found =3D 1;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> + =A0 =A0 =A0 =A0 =A0 =A0 mutex_unlock(&mpmc->devs_lock);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (found)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> + =A0 =A0 =A0 =A0 =A0 =A0 else
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 sdma =3D NULL;
>> + =A0 =A0 }
>> + =A0 =A0 mutex_unlock(&mpmc_devs_lock);

Why is the lock getting dropped on each iteration of the loop?  It
doesn't look necessary to me at all.

>> + =A0 =A0 return sdma;
>> +}
>> +EXPORT_SYMBOL_GPL(xllsdma_find_device);
>
> I'm still concerned that there is no concept of "allocating" or "reservin=
g" a
> channel. This seems to invite accidental concurrent use of a channel, if =
not in
> the field, then during development.

In the device tree use-case I doubt it will be a practical problem.
To get multiple users would require multiple nodes to reference the
same dma device node.

However, I've got concerns about the device model used here.... need
to read the rest of this driver first to figure out what is bothering
me....

>> +static void xllsdma_dev_register(struct mpmc_device *mpmc,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct xllsdma_dev=
ice *sdma)
>> +{
>> + =A0 =A0 mutex_lock(&mpmc->devs_lock);
>> + =A0 =A0 list_add(&sdma->item, &mpmc->xllsdma_devs);
>> + =A0 =A0 mutex_unlock(&mpmc->devs_lock);
>> +}

This driver could use some documentation on what the data model is
here.  Is it that the system can have one or more mpmc_devices, and
each mpmc device can contain one or more xllsdma_device?

How will the ll_temac driver plug into this model?

>> +
>> +static void xllsdma_dev_unregister(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 struct mpmc_device *mpmc =3D sdma->parent;
>> +
>> + =A0 =A0 mutex_lock(&mpmc->devs_lock);
>> + =A0 =A0 list_del(&sdma->item);
>> + =A0 =A0 mutex_unlock(&mpmc->devs_lock);
>> +}
>> +
>> +static void xllsdma_cleanup(struct device *dev)
>> +{
>> + =A0 =A0 struct xllsdma_device *sdma =3D dev_get_drvdata(dev);
>> +
>> + =A0 =A0 if (sdma->tx_irq)
>> + =A0 =A0 =A0 =A0 =A0 =A0 free_irq(sdma->tx_irq, sdma);
>> +
>> + =A0 =A0 if (sdma->rx_irq)
>> + =A0 =A0 =A0 =A0 =A0 =A0 free_irq(sdma->rx_irq, sdma);
>> +
>> + =A0 =A0 if (sdma->memregion.start)
>> + =A0 =A0 =A0 =A0 =A0 =A0 release_mem_region(sdma->memregion.start,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 sdma->memregion.end - sdma->me=
mregion.start + 1);
>> +
>> + =A0 =A0 if (sdma->ioaddr)
>> + =A0 =A0 =A0 =A0 =A0 =A0 iounmap(sdma->ioaddr);
>> +
>> + =A0 =A0 xllsdma_dev_unregister(sdma);
>> + =A0 =A0 kfree(sdma);
>> + =A0 =A0 dev_set_drvdata(dev, NULL);
>> +}
>> +
>> +static void mpmc_dev_register(struct mpmc_device *mpmc)
>> +{
>> + =A0 =A0 mutex_lock(&mpmc_devs_lock);
>> + =A0 =A0 list_add_tail(&mpmc->item, &mpmc_devs);
>> + =A0 =A0 mutex_unlock(&mpmc_devs_lock);
>> +}
>> +
>> +static void mpmc_dev_unregister(struct mpmc_device *mpmc)
>> +{
>> + =A0 =A0 mutex_lock(&mpmc_devs_lock);
>> + =A0 =A0 list_del(&mpmc->item);
>> + =A0 =A0 mutex_unlock(&mpmc_devs_lock);
>> +}
>> +
>> +static void mpmc_cleanup(struct device *dev)
>> +{
>> + =A0 =A0 struct mpmc_device *mpmc =3D dev_get_drvdata(dev);
>> +
>> + =A0 =A0 if (mpmc->registered)
>> + =A0 =A0 =A0 =A0 =A0 =A0 mpmc_dev_unregister(mpmc);

Under what condition would the mpmc not be registered when this
function is called?  I don't see any code path where this would be the
case, or any other users of the .registered data member.

>> +
>> + =A0 =A0 kfree(mpmc);
>> + =A0 =A0 dev_set_drvdata(dev, NULL);
>> +}
>> +
>> +static int __devinit xllsdma_init(struct device *dev, struct resource *=
rx_irq,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct resource=
 *tx_irq, struct resource *mem,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0int phandle)
>> +{
>> + =A0 =A0 struct xllsdma_device *sdma;
>> + =A0 =A0 struct mpmc_device *mpmc;
>> +
>> + =A0 =A0 resource_size_t region_size;
>> + =A0 =A0 int res;
>> +
>> + =A0 =A0 mpmc =3D dev_get_drvdata(dev->parent);
>> +
>> + =A0 =A0 sdma =3D kzalloc(sizeof(struct xllsdma_device), GFP_KERNEL);
>> + =A0 =A0 if (!sdma) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev_err(dev, "Cannot allocate SDMA device\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 return -ENOMEM;
>> + =A0 =A0 }
>> + =A0 =A0 dev_set_drvdata(dev, sdma);
>> + =A0 =A0 sdma->dev =3D dev;
>> +
>> + =A0 =A0 spin_lock_init(&sdma->lock);
>> + =A0 =A0 INIT_LIST_HEAD(&sdma->clients);
>> + =A0 =A0 mutex_init(&sdma->clients_lock);
>> + =A0 =A0 sdma->parent =3D mpmc;
>> + =A0 =A0 sdma->phandle =3D phandle;
>> +
>> + =A0 =A0 region_size =3D mem->end - mem->start + 1;
>> + =A0 =A0 if (!request_mem_region(mem->start, region_size, DRV_NAME)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev_err(dev, "I/O memory region at %p is busy\=
n",
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (void *)mem->start);
>> + =A0 =A0 =A0 =A0 =A0 =A0 return -EBUSY;

Error paths need to unwind allocations and setups.  ie. freeing
memory, releasing irqs, etc.

>> + =A0 =A0 }
>> + =A0 =A0 sdma->memregion =3D *mem;
>> +
>> + =A0 =A0 sdma->ioaddr =3D ioremap(mem->start, region_size);
>> + =A0 =A0 if (!sdma->ioaddr) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev_err(dev, "Cannot ioremap() I/O memory %p\n=
",
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (void *)mem->start);
>> + =A0 =A0 =A0 =A0 =A0 =A0 return -ENOMEM;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 xllsdma_reset(sdma);
>> +
>> + =A0 =A0 sdma->rx_irq =3D NO_IRQ;
>> + =A0 =A0 if (rx_irq) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 res =3D request_irq(rx_irq->start, xllsdma_rx_=
intr,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 IRQF_SHARED, "=
SDMA RX", sdma);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (res) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(dev, "Could not alloca=
te RX interrupt %d.\n",
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rx_irq->start)=
;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return res;
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> + =A0 =A0 =A0 =A0 =A0 =A0 sdma->rx_irq =3D rx_irq->start;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 sdma->tx_irq =3D NO_IRQ;
>> + =A0 =A0 if (tx_irq) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 res =3D request_irq(tx_irq->start, xllsdma_tx_=
intr,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 IRQF_SHARED, "=
SDMA TX", sdma);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (res) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(dev, "Could not alloca=
te TX interrupt %d.\n",
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tx_irq->start)=
;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return res;
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> + =A0 =A0 =A0 =A0 =A0 =A0 sdma->tx_irq =3D tx_irq->start;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 sdma->rx_ack =3D 1;
>> + =A0 =A0 sdma->tx_ack =3D 1;
>> + =A0 =A0 xllsdma_dev_register(mpmc, sdma);
>> +
>> + =A0 =A0 return 0;
>> +}
>> +
>> +static int __devinit mpmc_init(struct device *dev)
>> +{
>> + =A0 =A0 struct mpmc_device *mpmc;
>> +
>> + =A0 =A0 mpmc =3D kzalloc(sizeof(struct mpmc_device), GFP_KERNEL);
>> +
>> + =A0 =A0 if (!mpmc) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev_err(dev, "Cannot allocate MPMC device\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 return -ENOMEM;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 dev_set_drvdata(dev, mpmc);
>> +
>> + =A0 =A0 INIT_LIST_HEAD(&mpmc->xllsdma_devs);
>> + =A0 =A0 mutex_init(&mpmc->devs_lock);
>> +
>> + =A0 =A0 mpmc_dev_register(mpmc);
>> + =A0 =A0 mpmc->registered =3D 1;
>> +
>> + =A0 =A0 return 0;
>> +}
>> +
>> +#ifdef CONFIG_OF
>> +static int xllsdma_of_remove(struct of_device *op)

+static int __devexit xllsdma_of_remove(struct of_device *op)

>> +{
>> + =A0 =A0 xllsdma_cleanup(&op->dev);
>> + =A0 =A0 return 0;
>> +}
>> +
>> +/* Match table for of_platform binding */
>> +static struct of_device_id xllsdma_of_match[] =3D {
>> + =A0 =A0 { .compatible =3D "xlnx,ll-dma-1.00.a" },
>> + =A0 =A0 {},
>> +};
>> +
>> +static int __devinit xllsdma_of_probe(struct of_device *op,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const s=
truct of_device_id *match)
>> +{
>> + =A0 =A0 const int *prop;
>> + =A0 =A0 int phandle;
>> + =A0 =A0 struct resource rx_irq, tx_irq, mem;
>> + =A0 =A0 struct resource *tx_irq_res =3D NULL;
>> + =A0 =A0 struct resource *rx_irq_res =3D NULL;
>> + =A0 =A0 int res;
>> +
>> + =A0 =A0 res =3D of_address_to_resource(op->node, 0, &mem);
>> + =A0 =A0 if (res) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&op->dev, "invalid address\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 return res;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 /* IRQ */
>> + =A0 =A0 res =3D of_irq_to_resource(op->node, 0, &rx_irq);
>> + =A0 =A0 if (res !=3D NO_IRQ)
>> + =A0 =A0 =A0 =A0 =A0 =A0 rx_irq_res =3D &rx_irq;
>> +
>> + =A0 =A0 res =3D of_irq_to_resource(op->node, 1, &tx_irq);
>> + =A0 =A0 if (res !=3D NO_IRQ)
>> + =A0 =A0 =A0 =A0 =A0 =A0 tx_irq_res =3D &tx_irq;
>> +
>> + =A0 =A0 prop =3D of_get_property(op->node, "linux,phandle", NULL);
>> + =A0 =A0 phandle =3D (prop) ? *prop : -1;

Don't use phandles here.  Just store the node pointer directly.  When
looking up a phandle, first convert it to a node pointer, and then use
the node pointer to search.

>> +
>> + =A0 =A0 res =3D xllsdma_init(&op->dev, rx_irq_res, tx_irq_res, &mem, p=
handle);
>> + =A0 =A0 if (res)
>> + =A0 =A0 =A0 =A0 =A0 =A0 xllsdma_of_remove(op);

This looks odd.  If init fails, there is nothing in this function that
needs to be deallocated or unwound.  You can simply do:

+return xllsdma_init(&op->dev, rx_irq_res, tx_irq_res, &mem, phandle);

>> +
>> + =A0 =A0 return res;
>> +}
>> +
>> +static struct of_platform_driver xllsdma_of_driver =3D {
>> + =A0 =A0 .name =A0 =A0 =A0 =A0 =A0 =3D "xilinx-sdma",
>> + =A0 =A0 .match_table =A0 =A0=3D xllsdma_of_match,
>> + =A0 =A0 .probe =A0 =A0 =A0 =A0 =A0=3D xllsdma_of_probe,
>> + =A0 =A0 .remove =A0 =A0 =A0 =A0 =3D xllsdma_of_remove,
>> +};

Should be:
+static struct of_platform_driver xllsdma_of_driver =3D {
+ =A0 =A0 .driver =3D {
+             .name =3D "xilinx-sdma",
+             .owner =3D THIS_MODULE,
+     }
+ =A0 =A0 .match_table =A0 =A0=3D xllsdma_of_match,
+ =A0 =A0 .probe =A0 =A0 =A0 =A0 =A0=3D xllsdma_of_probe,
+ =A0 =A0 .remove =A0 =A0 =A0 =A0 =3D __devexit_p(xllsdma_of_remove),
+};

>> +
>> +int __init xllsdma_of_init(void)
>> +{
>> + =A0 =A0 int ret;
>> +
>> + =A0 =A0 ret =3D of_register_platform_driver(&xllsdma_of_driver);
>> + =A0 =A0 if (ret) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 of_unregister_platform_driver(&xllsdma_of_driv=
er);
>> + =A0 =A0 =A0 =A0 =A0 =A0 printk(KERN_ERR "registering driver failed: er=
r=3D%i", ret);
>> + =A0 =A0 =A0 =A0 =A0 =A0 return ret;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 return 0;
>> +}

If registering fails, unregistering is not needed.  This function
should simply be:

+int __init xllsdma_of_init(void)
+{
+ =A0 =A0 return of_register_platform_driver(&xllsdma_of_driver);
+}

>> +
>> +void xllsdma_of_exit(void)
>> +{
>> + =A0 =A0 of_unregister_platform_driver(&xllsdma_of_driver);
>> +}
>> +
>> +static int mpmc_of_remove(struct of_device *op)

+static int __devexit mpmc_of_remove(struct of_device *op)

>> +{
>> + =A0 =A0 struct device_node *node;
>> + =A0 =A0 struct of_device *ofdev;
>> +
>> + =A0 =A0 for_each_child_of_node(op->node, node) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 ofdev =3D of_find_device_by_node(node);
>> + =A0 =A0 =A0 =A0 =A0 =A0 of_device_unregister(ofdev);
>> + =A0 =A0 =A0 =A0 =A0 =A0 of_device_free(ofdev);
>> + =A0 =A0 }
>> +
>> + =A0 =A0 mpmc_cleanup(&op->dev);
>> + =A0 =A0 return 0;
>> +}
>> +
>> +static int __devinit mpmc_of_probe(struct of_device *op,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const struct of_device_id *mat=
ch)
>> +{
>> + =A0 =A0 int err =3D mpmc_init(&op->dev);
>> + =A0 =A0 if (err)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return err;
>> +
>> + =A0 =A0 of_platform_bus_probe(op->node, xllsdma_of_match, &op->dev);
>> + =A0 =A0 return 0;
>> +}

Okay, I think I've figured out what is bothering me....

While this *does* work; it really is the long way to go about things.
Doing it this way requires going back out to the driver model to tell
it things and trigger a probe on things that *this* driver needs, and
*this* driver already knows about.  It doesn't need to be this
complex.

Rather than register a bunch more of_platform devices, do something
like this instead:

+static int __devinit mpmc_of_probe(struct of_device *op,
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 const struct of_device_id *match)
+{
+ =A0 =A0 struct mpmc_device *mpmc;
+     struct device_node *node;
+
+ =A0 =A0 mpmc =3D mpmc_init(&op->dev);
+ =A0 =A0 if (!mpmc)
+ =A0 =A0 =A0 =A0 =A0 =A0 return -ENODEV;
+
+ =A0 =A0 for_each_child_of_node(op->node, node) {
+             xllsdma_of_init(mpmc, node);
+ =A0 =A0 }
+ =A0 =A0 return 0;
+}

You do *not* need to register a separate struct device for each DMA
channel sub node (at least with regard to the driver model; I don't
know about the dma subsystem).  If you *want* a struct device, then
xllsdma_of_init() is free to register one, but it does not need to be
on the of_platform_bus, and this driver should not require a probe
step for each DMA channel.

>> +
>> +static struct of_device_id =A0__devinitdata mpmc_of_match[] =3D {
>> + =A0 =A0 { .compatible =3D "xlnx,mpmc-4.01.a" },
>> + =A0 =A0 { .compatible =3D "xlnx,mpmc-4.03.a" },
>> + =A0 =A0 {},
>> +};
>> +
>> +static struct of_platform_driver mpmc_of_driver =3D {
>> + =A0 =A0 .name =3D "xilinx-mpmc",
>> + =A0 =A0 .match_table =3D mpmc_of_match,
>> + =A0 =A0 .probe =3D mpmc_of_probe,
>> + =A0 =A0 .remove =3D mpmc_of_remove,
>> +};
>> +
>> +int __init mpmc_of_init(void)
>> +{
>> + =A0 =A0 return of_register_platform_driver(&mpmc_of_driver);
>> +}
>> +
>> +void mpmc_of_exit(void)
>> +{
>> + =A0 =A0 of_unregister_platform_driver(&mpmc_of_driver);
>> +}

Missing the module exit hook?

>> +
>> +subsys_initcall(mpmc_of_init);
>> +subsys_initcall(xllsdma_of_init);

Typically initcall statements are placed immediately after the init
function they reference.

>> +#else =A0 =A0 =A0 =A0/* CONFIG_OF */

Why else?  It is perfectly valid to have both of_platform and platform
bus bindings.  That being said, this split will become unnecessary in
the very near future.  I've eliminated of_platform_bus_type, and
automatically moved all users of it over to the platform bus (without
driver changes).

However, current powerpc and microblaze code makes CONFIG_OF
manditory.  What condition will compile in the platform bus
attachment?

>> +/*---------------------------------------------------------------------=
------
>> + * Platform bus attachment
>> + */
>> +
>> +static __devexit int xllsdma_plat_remove(struct platform_device *pdev)
>> +{
>> + =A0 =A0 xllsdma_cleanup(&pdev->dev);
>> + =A0 =A0 return 0;
>> +}
>> +
>> +static int __devinit xllsdma_plat_probe(struct platform_device *pdev)
>> +{
>> + =A0 =A0 struct resource *rx_irq, *tx_irq, *mem;
>> + =A0 =A0 int err =3D 0;
>> +
>> + =A0 =A0 mem =3D platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + =A0 =A0 if (!mem) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&pdev->dev, "invalid address\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 err =3D -EINVAL;
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto fail;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 /* RX interrupt is optional, and first */
>> + =A0 =A0 rx_irq =3D platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> +
>> + =A0 =A0 /* TX interrupt is optional, and second */
>> + =A0 =A0 tx_irq =3D platform_get_resource(pdev, IORESOURCE_IRQ, 1);
>> +
>
> If it is impossible to create duplicate phandles in Device Tree,
> there should be a check here that no device with pdev->id already exists
> (i.e., it's NOT impossible with platform bus). It might be just as well t=
o
> put the check in xllsdma_init() since that's not a 'hot' code path.

Question: microblaze and powerpc both use the device tree.  What is
the use-case for the non-dts version?

>
>> + =A0 =A0 err =3D xllsdma_init(&pdev->dev, rx_irq, tx_irq, mem, pdev->id=
);
>> + =A0 =A0 if (err)
>> + =A0 =A0 =A0 =A0 =A0 =A0 xllsdma_plat_remove(pdev);
>> +fail:
>> + =A0 =A0 return err;
>> +}
>> +
>> +static struct platform_driver xllsdma_plat_driver =3D {
>> + =A0 =A0 .probe =3D xllsdma_plat_probe,
>> + =A0 =A0 .remove =3D __devexit_p(xllsdma_plat_remove),
>> + =A0 =A0 .driver =3D {
>> + =A0 =A0 =A0 =A0 =A0 =A0 .owner =3D THIS_MODULE,
>> + =A0 =A0 =A0 =A0 =A0 =A0 .name =A0=3D "xilinx-sdma",
>> + =A0 =A0 },
>> +};
>> +
>> +int __init xllsdma_plat_init(void)
>> +{
>> + =A0 =A0 int err =3D platform_driver_register(&xllsdma_plat_driver);
>> + =A0 =A0 if (err) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 platform_driver_unregister(&xllsdma_plat_drive=
r);
>> + =A0 =A0 =A0 =A0 =A0 =A0 printk(KERN_ERR "registering driver failed: er=
r=3D%i", err);
>> + =A0 =A0 =A0 =A0 =A0 =A0 return err;
>> + =A0 =A0 }
>> +
>> + =A0 =A0 return 0;
>> +}
>> +subsys_initcall(xllsdma_plat_init);

Again, if driver registration fails, then the driver doesn't need to
be unregistered.  Same as with the of_platform_bus binding.

>> +
>> +void xllsdma_plat_exit(void)
>> +{
>> + =A0 =A0 platform_driver_unregister(&xllsdma_plat_driver);
>> +}
>> +
>> +static int mpmc_plat_probe(struct platform_device *pdev)
>> +{
>> + =A0 =A0 return mpmc_init(&pdev->dev);
>> +}
>> +
>> +static int __devexit mpmc_plat_remove(struct platform_device *pdev)
>> +{
>> + =A0 =A0 mpmc_cleanup(&pdev->dev);
>> + =A0 =A0 return 0;
>> +}
>> +
>> +static struct platform_driver mpmc_plat_driver =3D {
>> + =A0 =A0 .probe =3D mpmc_plat_probe,
>> + =A0 =A0 .remove =3D __devexit_p(mpmc_plat_remove),
>> + =A0 =A0 .driver =3D {
>> + =A0 =A0 =A0 =A0 =A0 =A0 .owner =3D THIS_MODULE,
>> + =A0 =A0 =A0 =A0 =A0 =A0 .name =A0=3D "xilinx-mpmc",
>> + =A0 =A0 },
>> +};

I'll make the same argument here.  The multiple registrations in this
driver is weird.  This driver already knows about all of it's dma
channels, so why depend on the core driver model to probe a driver
instance for each?

>> +
>> +int __init mpmc_plat_init(void)
>> +{
>> + =A0 =A0 return platform_driver_register(&mpmc_plat_driver);
>> +}
>> +subsys_initcall(mpmc_plat_init);
>> +
>> +void mpmc_plat_exit(void)
>> +{
>> + =A0 =A0 platform_driver_unregister(&mpmc_plat_driver);
>> +}
>> +#endif =A0 =A0 =A0 /* CONFIG_OF */
>> diff -r baced9e29ab5 include/linux/xllsdma.h
>> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
>> +++ b/include/linux/xllsdma.h Wed Apr 28 02:00:51 2010 +0400
>> @@ -0,0 +1,187 @@
>> +/*
>> + * SDMA subsystem support for Xilinx MPMC.
>> + *
>> + * Author: Sergey Temerkhanov
>> + *
>> + * Copyright (c) 2008-2010 Cifronic ZAO
>> + *
>> + * This program is free software; you can redistribute =A0it and/or mod=
ify it
>> + * under =A0the terms of =A0the GNU General =A0Public License as publis=
hed by the
>> + * Free Software Foundation; =A0either version 2 of the =A0License, or =
(at your
>> + * option) any later version.
>> + *
>> + */
>> +
>> +#ifndef __XLLSDMA_H__
>> +#define __XLLSDMA_H__
>> +
>> +#include <linux/types.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#define XLLSDMA_ALIGNMENT =A0 =A00x40
>> +
>> +struct xllsdma_desc {
>> + =A0 =A0 __be32 next;
>> + =A0 =A0 __be32 address;
>> + =A0 =A0 __be32 length;
>> + =A0 =A0 __be32 stat_ctl;
>> + =A0 =A0 __be32 app1;
>> + =A0 =A0 __be32 app2;
>> + =A0 =A0 __be32 app3;
>> + =A0 =A0 __be32 app4;
>> + =A0 =A0 void *virt;
>> + =A0 =A0 u32 flags;
>> +} __attribute__((aligned(XLLSDMA_ALIGNMENT)));
>
> 'virt' and 'flags' are not used by this driver. Putting them in this stru=
cture,
> and using this structure in the API, forces them on all clients, which ca=
n lead
> to inefficiencies (this will certainly be true in the DMA engine driver, =
perhaps
> others). I think a better choice would be to have the API use a structure=
 without
> these two fields. For the convenience of other clients you could define a
> 'superstructure' _not_ used in the API, like:
>
> struct xllsdma_fancy_desc {
> =A0 =A0 =A0 =A0struct xllsdma_desc =A0basic_desc;
> =A0 =A0 =A0 =A0void *virt;
> =A0 =A0 =A0 =A0u32 flags;
> }
>
>> +
>> +
>> +enum {
>> + =A0 =A0 XLLSDMA_STSCTL_ERROR =A0 =A0=3D (1 << 31), /* DMA error */
>> + =A0 =A0 XLLSDMA_STSCTL_IOE =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D (1 << 30), /=
* Interrupt on end */
>> + =A0 =A0 XLLSDMA_STSCTL_SOE =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D (1 << 29), /=
* Stop on end */
>> + =A0 =A0 XLLSDMA_STSCTL_DONE =A0 =A0 =3D (1 << 28), /* DMA completed */
>> + =A0 =A0 XLLSDMA_STSCTL_SOP =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D (1 << 27), /=
* Start of packet */
>> + =A0 =A0 XLLSDMA_STSCTL_EOP =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D (1 << 26), /=
* End of packet */
>> + =A0 =A0 XLLSDMA_STSCTL_BUSY =A0 =A0 =3D (1 << 25), /* DMA busy */
>> + =A0 =A0 XLLSDMA_STSCTL_CSUM =A0 =A0 =3D (1 << 0), =A0/* Checksum enabl=
e */
>> +
>> + =A0 =A0 XLLSDMA_STSCTL_MSK =A0 =A0 =A0 =A0 =A0 =A0 =A0=3D (0xFF << 24)=
, /*Status/control field */
>> +};
>
> The search-and-replace messed up your nice alignment.
>
>> +
>> +/* SDMA client operations */
>> +struct xllsdma_client {
>> + =A0 =A0 void *data;
>> + =A0 =A0 void (*tx_complete) (void *data);
>> + =A0 =A0 void (*rx_complete) (void *data);
>> + =A0 =A0 void (*error) (void *data);
>> + =A0 =A0 void (*reset) (void *data);
>> + =A0 =A0 struct list_head item;
>> +};
>> +
>> +struct xllsdma_coalesce {
>> + =A0 =A0 int tx_threshold;
>> + =A0 =A0 int tx_timeout;
>> +
>> + =A0 =A0 int rx_threshold;
>> + =A0 =A0 int rx_timeout;
>> +};
>> +
>> +#define DEFINE_XLLSDMA_COALESCE(x) struct xllsdma_coalesce x =3D { \
>> + =A0 =A0 .tx_timeout =A0 =A0 =3D 0, \
>> + =A0 =A0 .tx_threshold =A0 =3D 1, \
>> + =A0 =A0 .rx_timeout =A0 =A0 =3D 0, \
>> + =A0 =A0 .rx_threshold =A0 =3D 1, };
>> +
>> +struct mpmc_device {
>> + =A0 =A0 void __iomem =A0 =A0 =A0 =A0 =A0 =A0*ioaddr;
>> +
>> + =A0 =A0 struct resource =A0 =A0 =A0 =A0 memregion;
>> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 irq;
>> +
>> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 registered;
>> + =A0 =A0 struct list_head =A0 =A0 =A0 =A0item;
>> +
>> + =A0 =A0 struct mutex =A0 =A0 =A0 =A0 =A0 =A0devs_lock;
>> + =A0 =A0 struct list_head =A0 =A0 =A0 =A0xllsdma_devs;
>> +};
>> +
>> +struct xllsdma_device {
>> + =A0 =A0 struct device =A0 =A0 =A0 =A0 =A0 *dev;
>> + =A0 =A0 void __iomem =A0 =A0 =A0 =A0 =A0 =A0*ioaddr;
>> + =A0 =A0 wait_queue_head_t =A0 =A0 =A0 wait;
>> +
>> + =A0 =A0 spinlock_t =A0 =A0 =A0 =A0 =A0 =A0 =A0lock;
>> +
>> + =A0 =A0 struct resource =A0 =A0 =A0 =A0 memregion;
>> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rx_irq;
>> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tx_irq;
>> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rx_ack;
>> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 tx_ack;
>> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 phandle;
>> +
>> + =A0 =A0 int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 registered;
>> + =A0 =A0 struct mpmc_device =A0 =A0 =A0*parent;
>> +
>> + =A0 =A0 struct xllsdma_coalesce coal;
>> + =A0 =A0 struct list_head =A0 =A0 =A0 =A0item;
>> +
>> + =A0 =A0 struct mutex =A0 =A0 =A0 =A0 =A0 =A0clients_lock;
>> + =A0 =A0 struct list_head =A0 =A0 =A0 =A0clients;
>> +};
>> +
>> +static inline void xllsdma_add_client(struct xllsdma_device *sdma,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct =
xllsdma_client *client)
>> +{
>> + =A0 =A0 mutex_lock(&sdma->clients_lock);
>> + =A0 =A0 list_add(&client->item, &sdma->clients);
>> + =A0 =A0 mutex_unlock(&sdma->clients_lock);
>> +}
>> +
>> +static inline void xllsdma_del_client(struct xllsdma_device *sdma,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0struct =
xllsdma_client *client)
>> +{
>> + =A0 =A0 mutex_lock(&sdma->clients_lock);
>> + =A0 =A0 list_del(&client->item);
>> + =A0 =A0 mutex_unlock(&sdma->clients_lock);
>> +}
>> +
>> +struct xllsdma_device *xllsdma_find_device(int phandle);
>> +void xllsdma_pause(struct xllsdma_device *sdma);
>> +void xllsdma_resume(struct xllsdma_device *sdma);
>> +void xllsdma_reset(struct xllsdma_device *sdma);
>> +void xllsdma_rx_init(struct xllsdma_device *sdma, dma_addr_t desc);
>> +void xllsdma_tx_init(struct xllsdma_device *sdma, dma_addr_t desc);
>> +
>> +int xllsdma_tx_submit(struct xllsdma_device *sdma, dma_addr_t desc);
>> +int xllsdma_rx_submit(struct xllsdma_device *sdma, dma_addr_t desc);
>> +
>> +void xllsdma_tx_irq_enable(struct xllsdma_device *sdma);
>> +void xllsdma_rx_irq_enable(struct xllsdma_device *sdma);
>> +void xllsdma_tx_irq_disable(struct xllsdma_device *sdma);
>> +void xllsdma_rx_irq_disable(struct xllsdma_device *sdma);
>> +void xllsdma_tx_irq_ack(struct xllsdma_device *sdma);
>> +void xllsdma_rx_irq_ack(struct xllsdma_device *sdma);
>> +
>> +int xllsdma_set_coalesce(struct xllsdma_device *sdma, struct xllsdma_co=
alesce *coal);
>> +int xllsdma_get_coalesce(struct xllsdma_device *sdma, struct xllsdma_co=
alesce *coal);
>> +
>> +static inline int xllsdma_has_tx_irq(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 return sdma->tx_irq !=3D NO_IRQ;
>> +}
>> +
>> +static inline int xllsdma_has_rx_irq(struct xllsdma_device *sdma)
>> +{
>> + =A0 =A0 return sdma->rx_irq !=3D NO_IRQ;
>> +}
>> +
>> +static inline int xllsdma_desc_busy(struct xllsdma_desc *desc)
>> +{
>> + =A0 =A0 return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_=
BUSY);
>> +}
>> +
>> +static inline int xllsdma_desc_done(struct xllsdma_desc *desc)
>> +{
>> + =A0 =A0 return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_=
DONE);
>> +}
>> +
>> +static inline int xllsdma_desc_sop(struct xllsdma_desc *desc)
>> +{
>> + =A0 =A0 return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_=
SOP);
>> +}
>> +
>> +static inline int xllsdma_desc_eop(struct xllsdma_desc *desc)
>> +{
>> + =A0 =A0 return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_=
EOP);
>> +}
>> +
>> +static inline void xllsdma_set_ack(struct xllsdma_device *sdma, int rx_=
ack,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int tx_ack)
>> +{
>> + =A0 =A0 unsigned long flags;
>> + =A0 =A0 spin_lock_irqsave(&sdma->lock, flags);
>> + =A0 =A0 sdma->rx_ack =3D rx_ack;
>> + =A0 =A0 sdma->tx_ack =3D tx_ack;
>> + =A0 =A0 spin_unlock_irqrestore(&sdma->lock, flags);
>> +}
>> +
>> +#endif
>
> ------------------------------------------------------------------------
> =A0Steven J. Magnani =A0 =A0 =A0 =A0 =A0 =A0 =A0 "I claim this network fo=
r MARS!
> =A0www.digidescorp.com =A0 =A0 =A0 =A0 =A0 =A0 =A0Earthling, return my sp=
ace modulator!"
>
> =A0#include <standard.disclaimer>
>
>
>
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [patch 1/1] powerpc: add rcu_read_lock() to gup_fast() implementation
From: Benjamin Herrenschmidt @ 2010-04-28  4:26 UTC (permalink / raw)
  To: akpm; +Cc: npiggin, linuxppc-dev, riel, paulmck, a.p.zijlstra
In-Reply-To: <201004272110.o3RLAl7P019943@imap1.linux-foundation.org>

On Tue, 2010-04-27 at 14:10 -0700, akpm@linux-foundation.org wrote:
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> 
> The powerpc page table freeing relies on the fact that IRQs hold off an
> RCU grace period, this is currently true for all existing RCU
> implementations but is not an assumption Paul wants to support.
> 
> Therefore, also take the RCU read lock along with disabling IRQs to ensure
> the RCU grace period does at least cover these lookups.

Nah, that's not right. The right fix is to use call_rcu_sched() from
the powerpc page table freeing code. Please drop.

Cheers,
Ben.

> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Requested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Nick Piggin <npiggin@suse.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Reviewed-by: Rik van Riel <riel@redhat.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  arch/powerpc/mm/gup.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff -puN arch/powerpc/mm/gup.c~powerpc-add-rcu_read_lock-to-gup_fast-implementation arch/powerpc/mm/gup.c
> --- a/arch/powerpc/mm/gup.c~powerpc-add-rcu_read_lock-to-gup_fast-implementation
> +++ a/arch/powerpc/mm/gup.c
> @@ -142,6 +142,7 @@ int get_user_pages_fast(unsigned long st
>  	 * So long as we atomically load page table pointers versus teardown,
>  	 * we can follow the address down to the the page and take a ref on it.
>  	 */
> +	rcu_read_lock();
>  	local_irq_disable();
>  
>  	pgdp = pgd_offset(mm, addr);
> @@ -162,6 +163,7 @@ int get_user_pages_fast(unsigned long st
>  	} while (pgdp++, addr = next, addr != end);
>  
>  	local_irq_enable();
> +	rcu_read_unlock();
>  
>  	VM_BUG_ON(nr != (end - start) >> PAGE_SHIFT);
>  	return nr;
> @@ -171,6 +173,7 @@ int get_user_pages_fast(unsigned long st
>  
>  slow:
>  		local_irq_enable();
> +		rcu_read_unlock();
>  slow_irqon:
>  		pr_devel("  slow path ! nr = %d\n", nr);
>  
> _

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Benjamin Herrenschmidt @ 2010-04-28  4:25 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, kumar.gala, linuxppc-dev, Timur Tabi, Liam Girdwood
In-Reply-To: <20100427205924.GC15083@opensource.wolfsonmicro.com>

On Tue, 2010-04-27 at 21:59 +0100, Mark Brown wrote:
> It's entirely possible that if the board designer intended the verious
> SSIs to be used in concert they've done something like cross wire the
> clocks which creates a board-specific interrelationship that needs to
> be dealt with. 

In which case, have that in some board specific code :-) Really, as I
said earlier, I think there's no point to aim toward a
uber-representation that can describe everything along with code that
can cope with anything the tree can describe :-) That's just insane.

I'd stay stick to the basics and move incrementally up until it stops
making sense:

 - First, the basics: nodes for actual physical devices. i2c codecs on
their i2c busses, DMA controllers, etc...

 - From there, see what simple properties are better off being put in
those respective nodes because they represent common enough variants of
functionality or simply because they are specific attributes of a given
device that aren't too concerned by the way things are interconnected.

 - Provide at least one identifier (either in a "sound" virtual device,
or just use the board's own "compatible" property) to have a .c file to
put everything together.

>From there, you may decide that 90% of the simple cases can be very
easily represented by a couple of properties in the said "sound" node,
and henceforth, create a simple.c "board" file that matches against a
list of boards known to fit within that simple model, and that pretty
much exclusively use the device-tree to put things together.

But you don't have to.

The whole thing is a matter of common sense and a bit of taste :-)

Cheers,
Ben.

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Benjamin Herrenschmidt @ 2010-04-28  4:19 UTC (permalink / raw)
  To: Timur Tabi; +Cc: alsa-devel, kumar.gala, broonie, linuxppc-dev, Liam Girdwood
In-Reply-To: <4BD74351.9030704@freescale.com>

On Tue, 2010-04-27 at 15:04 -0500, Timur Tabi wrote:
> What I need is something like a hashing function that can convert a
> "struct device_node *" into an "int".  I'm going to have two functions
> that independently parse the device tree and locate a specific node.
> Both functions will "register the node" with asoc, but they'll use an
> integer ID to uniquely identify the node.
> 
> At least, that's the way ASoC likes to operate.  AsoC takes a fixed
> string plus a unique integer.  I could technically create a unique
> string for each DMA device, and have the integer always be 0. 

That's just plain gross and horrible. You could use phandles you
know :-)

Or you could use path in your strings, or something like that.

Note that any time you have a struct device, you have a free device_node
pointer as well.

Cheers,
Ben.

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Benjamin Herrenschmidt @ 2010-04-28  4:18 UTC (permalink / raw)
  To: Grant Likely
  Cc: alsa-devel, kumar.gala, broonie, linuxppc-dev, Timur Tabi,
	Liam Girdwood
In-Reply-To: <s2ifa686aa41004271215tb3c992c1v8996ce5520896c6c@mail.gmail.com>

On Tue, 2010-04-27 at 13:15 -0600, Grant Likely wrote:
> 
> Can you not dynamically assign an id?  If alsa soc needs a unique id
> number, then just create a lookup function.  Something like
> of_asoc_phandle_to_codec_id() that will either return a previously
> assigned id, or will assign a new id.  You shouldn't ever need to add
> data to the tree at runtime. 

Numerical magic IDs are evil... why not a name ? Or it's existing alsa
breakage ? :-)

Cheers,
Ben.

^ permalink raw reply

* Re: [patch 1/1] powerpc: add rcu_read_lock() to gup_fast() implementation
From: Benjamin Herrenschmidt @ 2010-04-28  4:10 UTC (permalink / raw)
  To: Nick Piggin; +Cc: riel, linuxppc-dev, akpm, paulmck, a.p.zijlstra
In-Reply-To: <20100428032931.GE5683@laptop>

On Wed, 2010-04-28 at 13:29 +1000, Nick Piggin wrote:
> I think I nacked this because the rest of the powerpc code also
> assumes irq disables provide an rcu critical section. The plan was
> to convert powerpc pagetable code to use call_rcu_sched.

Right, on my todo list.

Cheers,
Ben.

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Benjamin Herrenschmidt @ 2010-04-28  4:10 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, kumar.gala, linuxppc-dev, Timur Tabi, Liam Girdwood
In-Reply-To: <20100427222913.GE15083@opensource.wolfsonmicro.com>

On Tue, 2010-04-27 at 23:29 +0100, Mark Brown wrote:
> 
> On the other hand from a pragmatic point of view it's just much less
> hassle to just only provide the mechanism for instantiating a machine
> with custom code and use that for everything.

Right, that's the balance to find. A too descriptive device-tree becomes
a mess, and attempting to deal with any kind of representation in SW is
horrible.

There is a fine balance to be found between how much goes into the
device-tree and how much is eventually just a plain C file that puts
things together.

For example, one could imagine a /sound node with simply a "compatible"
property that matches what we call in AOA terminology a "fabric" driver.
IE. The one thing specific to the board that puts bits and pieces
together.

Now, the device-tree is still obviously useful to provide things like
the actual i2c IDs of codecs, GPIOs used for various actions, link to
from various components to their clock source devices, etc.. All these
things simplify the code, avoids horrid board specific code in the
actual drivers (codecs, busses, etc...) and overall help keeping the
code more maintainable. 

This is not an issue specific to audio. The same problem to some extent
shows up at the arch level, which is why I was never too much in favor
of doing a "generic" platform on powerpc, but still want to have a per
board (or at least board family) platform .c file which has the upper
hand, even if it ends up mostly using device-tree based "helpers" to put
things together.

The device-tree helps keep the platform .c file simple and devoid of too
horrible hacks, it allows to easily pass various configuration data to
leaf drivers such as i2c thingies, PHY devices etc... without gross
hooks between these and the platform, but the platform code still has
the upper hand for doing ad-hoc bits and pieces (or overwriting the
device-tree based behaviour) if necessary.

Cheers,
Ben.

^ permalink raw reply

* Re: [patch 1/1] powerpc: add rcu_read_lock() to gup_fast() implementation
From: Nick Piggin @ 2010-04-28  3:29 UTC (permalink / raw)
  To: akpm; +Cc: riel, paulmck, a.p.zijlstra, linuxppc-dev
In-Reply-To: <201004272110.o3RLAl7P019943@imap1.linux-foundation.org>

On Tue, Apr 27, 2010 at 02:10:47PM -0700, Andrew Morton wrote:
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> 
> The powerpc page table freeing relies on the fact that IRQs hold off an
> RCU grace period, this is currently true for all existing RCU
> implementations but is not an assumption Paul wants to support.
> 
> Therefore, also take the RCU read lock along with disabling IRQs to ensure
> the RCU grace period does at least cover these lookups.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Requested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Nick Piggin <npiggin@suse.de>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Reviewed-by: Rik van Riel <riel@redhat.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

I think I nacked this because the rest of the powerpc code also
assumes irq disables provide an rcu critical section. The plan was
to convert powerpc pagetable code to use call_rcu_sched.

> ---
> 
>  arch/powerpc/mm/gup.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff -puN arch/powerpc/mm/gup.c~powerpc-add-rcu_read_lock-to-gup_fast-implementation arch/powerpc/mm/gup.c
> --- a/arch/powerpc/mm/gup.c~powerpc-add-rcu_read_lock-to-gup_fast-implementation
> +++ a/arch/powerpc/mm/gup.c
> @@ -142,6 +142,7 @@ int get_user_pages_fast(unsigned long st
>  	 * So long as we atomically load page table pointers versus teardown,
>  	 * we can follow the address down to the the page and take a ref on it.
>  	 */
> +	rcu_read_lock();
>  	local_irq_disable();
>  
>  	pgdp = pgd_offset(mm, addr);
> @@ -162,6 +163,7 @@ int get_user_pages_fast(unsigned long st
>  	} while (pgdp++, addr = next, addr != end);
>  
>  	local_irq_enable();
> +	rcu_read_unlock();
>  
>  	VM_BUG_ON(nr != (end - start) >> PAGE_SHIFT);
>  	return nr;
> @@ -171,6 +173,7 @@ int get_user_pages_fast(unsigned long st
>  
>  slow:
>  		local_irq_enable();
> +		rcu_read_unlock();
>  slow_irqon:
>  		pr_devel("  slow path ! nr = %d\n", nr);
>  
> _

^ permalink raw reply

* Re: [microblaze-uclinux] [PATCHv2] [RFC] Xilinx MPMC SDMA subsystem
From: Steven J. Magnani @ 2010-04-28  2:29 UTC (permalink / raw)
  To: microblaze-uclinux; +Cc: linuxppc-dev, Linux Kernel Mailing List
In-Reply-To: <1272421344.2877.6.camel@iscandar.digidescorp.com>

On Wed, 2010-04-28 at 02:06 +0400, Sergey Temerkhanov wrote: 
> This is the 2nd version of Xilinx MPMC LocalLink SDMA subsystem
> 
> Changelog v2:
> * Changed the functions and struct definition prefix from sdma_ to xllsdma_
> * Platform bus bindings and various changes by Steven J. Magnani.
> * Moved source files from arch/powerpc/sysdev to global locations and added
>   CONFIG_XLLSDMA option.
> 
> Regards, Sergey Temerkhanov,
> Cifronic ZAO
> 
> diff -r baced9e29ab5 drivers/dma/Kconfig
> --- a/drivers/dma/Kconfig	Tue Apr 27 20:48:50 2010 +0400
> +++ b/drivers/dma/Kconfig	Wed Apr 28 02:00:51 2010 +0400
> @@ -97,6 +97,14 @@
>  	  Support the TXx9 SoC internal DMA controller.  This can be
>  	  integrated in chips such as the Toshiba TX4927/38/39.
>  
> +config XLLSDMA
> +	bool "Xilinx MPMC DMA support"
> +	depends on XILINX_VIRTEX || MICROBLAZE
> +	select DMA_ENGINE
> +	help
> +	  Support fot Xilinx MPMC LocalLink SDMA. Virtex FPGA family
> +	  has it integrated or fabric-based.
> +

fot --> for

Since the xllsdma driver provides services to other drivers - not to userland -
I think this would be better as a "silent" option, selected automatically when
something like ll_temac or the forthcoming Xilinx DMA engine is selected. 
If we do it that way, note that XLLSDMA is independent of DMA_ENGINE so 
drivers/Makefile will need to be patched so that the dma subdirectory is 
always "y".

> 
>  config DMA_ENGINE
>  	bool
>  
> @@ -133,3 +141,5 @@
>  	  DMA Device driver.
>  
>  endif
> +
> +

Some extra white space crept in at the end of the file.

> diff -r baced9e29ab5 drivers/dma/Makefile
> --- a/drivers/dma/Makefile	Tue Apr 27 20:48:50 2010 +0400
> +++ b/drivers/dma/Makefile	Wed Apr 28 02:00:51 2010 +0400
> @@ -10,3 +10,4 @@
>  obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
>  obj-$(CONFIG_MX3_IPU) += ipu/
>  obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o
> +obj-$(CONFIG_XLLSDMA) += xllsdma.o
> diff -r baced9e29ab5 drivers/dma/xllsdma.c
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/drivers/dma/xllsdma.c	Wed Apr 28 02:00:51 2010 +0400
> @@ -0,0 +1,887 @@
> +/*
> + * SDMA subsystem support for Xilinx MPMC.
> + *
> + * Author: Sergey Temerkhanov
> + * Platform Bus by Steven J. Magnani
> + *
> + * Copyright (c) 2008-2010 Cifronic ZAO
> + *
> + * 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/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/mutex.h>
> +#include <linux/wait.h>
> +#include <linux/list.h>
> +#include <linux/io.h>
> +#include <linux/xllsdma.h>
> +
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +
> +#define DRV_VERSION "0.1.0"
> +#define DRV_NAME "sdma"
> +
> +MODULE_AUTHOR("Sergey Temerkhanov <temerkhanov@cifronik.ru>");
> +MODULE_DESCRIPTION("Xilinx SDMA driver");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(DRV_VERSION);
> +
> +LIST_HEAD(mpmc_devs);
> +DEFINE_MUTEX(mpmc_devs_lock);
> +
> +enum {
> +	XLLSDMA_TX_REGS	= 0x00,	/* TX channel registers beginning */
> +	XLLSDMA_RX_REGS	= 0x20,	/* RX channel registers beginning */
> +	XLLSDMA_DMACR	= 0x40,	/* DMA control register */
> +
> +	XLLSDMA_NDESCR	= 0x00,	/* Next descriptor address */
> +	XLLSDMA_BUFA	= 0x04,	/* Current buffer address */
> +	XLLSDMA_BUFL	= 0x08,	/* Current buffer length */
> +	XLLSDMA_CDESCR	= 0x0C,	/* Current descriptor address */
> +	XLLSDMA_TDESCR	= 0x10,	/* Tail descriptor address */
> +	XLLSDMA_CR	= 0x14,	/* Channel control */
> +	XLLSDMA_IRQ	= 0x18,	/* Interrupt register */
> +	XLLSDMA_SR	= 0x1C,	/* Status */
> +};
> +
> +enum {
> +	XLLSDMA_CR_IRQ_TIMEOUT_MSK   = (0xFF << 24),	/* Interrupt coalesce timeout */
> +	XLLSDMA_CR_IRQ_THRESHOLD_MSK = (0xFF << 16),	/* Interrupt coalesce count */
> +	XLLSDMA_CR_MSB_ADDR_MSK	     = (0xF << 12),	/* MSB for 36 bit addressing */
> +	XLLSDMA_CR_APP_EN	  = (1 << 11),	/* Application data mask enable */
> +	XLLSDMA_CR_1_BIT_CNT	  = (1 << 10),	/* All interrupt counters are 1-bit */
> +	XLLSDMA_CR_INT_ON_END	  = (1 << 9),	/* Interrupt-on-end */
> +	XLLSDMA_CR_LD_IRQ_CNT	  = (1 << 8),	/* Load IRQ_COUNT */
> +	XLLSDMA_CR_IRQ_EN	  = (1 << 7),	/* Master interrupt enable */
> +	XLLSDMA_CR_IRQ_ERROR	  = (1 << 2),	/* Error interrupt enable */
> +	XLLSDMA_CR_IRQ_TIMEOUT	  = (1 << 1),	/* Coalesce timeout interrupt enable */
> +	XLLSDMA_CR_IRQ_THRESHOLD  = (1 << 0),	/* Coalesce threshold interrupt enable */
> +
> +	XLLSDMA_CR_IRQ_ALL	  = XLLSDMA_CR_IRQ_EN | XLLSDMA_CR_IRQ_ERROR |
> +					XLLSDMA_CR_IRQ_TIMEOUT | XLLSDMA_CR_IRQ_THRESHOLD,
> +
> +	XLLSDMA_CR_IRQ_TIMEOUT_SH   = 24,
> +	XLLSDMA_CR_IRQ_THRESHOLD_SH = 16,
> +	XLLSDMA_CR_MSB_ADDR_SH	    = 12,
> +
> +	XLLSDMA_IRQ_WRQ_EMPTY	 = (1 << 14),	/* Write Command Queue Empty (rx) */
> +	XLLSDMA_IRQ_PLB_RD_ERROR = (1 << 4),	/* PLB Read Error IRQ */
> +	XLLSDMA_IRQ_PLB_WR_ERROR = (1 << 3),	/* PLB Write Error IRQ */
> +	XLLSDMA_IRQ_ERROR	 = (1 << 2),	/* Error IRQ */
> +	XLLSDMA_IRQ_TIMEOUT	 = (1 << 1),	/* Coalesce timeout IRQ */
> +	XLLSDMA_IRQ_THRESHOLD	 = (1 << 0),	/* Coalesce threshold IRQ */
> +
> +	XLLSDMA_IRQ_ALL_ERR	 = 0x1C,	/* All error interrupt */
> +	XLLSDMA_IRQ_ALL		 = 0x1F,	/* All interrupt bits */
> +	XLLSDMA_IRQ_ALL_DONE	 = 0x3,		/* All work complete interrupt bits */
> +
> +
> +#define XLLSDMA_IRQ_COALESCE_COUNT(x)	((x >> 10) & 0xF)
> +#define XLLSDMA_IRQ_DELAY_COUNT(x)		((x >> 8) & 0x3)
> +
> +	XLLSDMA_SR_ERR_TDESCR	 = (1 << 21),	/* Tail descriptor pointer is invalid */
> +	XLLSDMA_SR_ERR_CMPL	 = (1 << 20),	/* Complete bit is set */
> +	XLLSDMA_SR_ERR_BUFA	 = (1 << 19),	/* Buffer address is invalid */
> +	XLLSDMA_SR_ERR_NDESCR	 = (1 << 18),	/* Next descriptor pointer is invalid */
> +	XLLSDMA_SR_ERR_CDESCR	 = (1 << 17),	/* Current descriptor pointer is invalid */
> +	XLLSDMA_SR_ERR_BUSYWR	 = (1 << 16),	/* Current descriptor modified */
> +	XLLSDMA_SR_ERROR	 = (1 << 7),	/* Error IRQ has occurred */
> +	XLLSDMA_SR_IRQ_ON_END	 = (1 << 6),	/* On-end IRQ has occurred */
> +	XLLSDMA_SR_STOP_ON_END	 = (1 << 5), 	/* Stop on end has occurred */
> +	XLLSDMA_SR_COMPLETED	 = (1 << 4),	/* BD completed */
> +	XLLSDMA_SR_SOP		 = (1 << 3),	/* Current BD has SOP set */
> +	XLLSDMA_SR_EOP		 = (1 << 2),	/* Current BD has EOP set */
> +	XLLSDMA_SR_ENGINE_BUSY	 = (1 << 1),	/* Channel is busy */
> +	
> +
> +	XLLSDMA_DMACR_TX_PAUSE	  = (1 << 29),	/* Pause TX channel */
> +	XLLSDMA_DMACR_RX_PAUSE	  = (1 << 28),	/* Pause RX channel */
> +	XLLSDMA_DMACR_PLB_ERR_DIS = (1 << 5),	/* Disable PLB error detection */
> +	XLLSDMA_DMACR_RX_OVF_DIS  = (1 << 4),	/* Disable error on RX coalesce counter overflows */
> +	XLLSDMA_DMACR_TX_OVF_DIS  = (1 << 3),	/* Disable error on TX coalesce counter overflows */
> +	XLLSDMA_DMACR_TAIL_PTR_EN = (1 << 2),	/* Enable use of tail pointer register */
> +	XLLSDMA_DMACR_EN_ARB_HOLD = (1 << 1),	/* Enable arbitration hold */
> +	XLLSDMA_DMACR_RESET	  = (1 << 0),	/* Reset both channels */
> +};
> +
> +static inline void xllsdma_write_cr(struct xllsdma_device *sdma, u32 value)
> +{
> +	out_be32(sdma->ioaddr + XLLSDMA_DMACR, value);
> +}
> +
> +static inline u32 xllsdma_read_cr(struct xllsdma_device *sdma)
> +{
> +	return in_be32(sdma->ioaddr + XLLSDMA_DMACR);
> +}
> +
> +static inline void xllsdma_tx_out32(struct xllsdma_device *sdma, int reg, u32 value)
> +{
> +	out_be32(sdma->ioaddr + reg + XLLSDMA_TX_REGS, value);
> +}
> +
> +static inline u32 xllsdma_tx_in32(struct xllsdma_device *sdma, int reg)
> +{
> +	return in_be32(sdma->ioaddr + reg + XLLSDMA_TX_REGS);
> +}
> +
> +static inline void xllsdma_rx_out32(struct xllsdma_device *sdma, int reg, u32 value)
> +{
> +	out_be32(sdma->ioaddr + reg + XLLSDMA_RX_REGS, value);
> +}
> +
> +static inline u32 xllsdma_rx_in32(struct xllsdma_device *sdma, int reg)
> +{
> +	return in_be32(sdma->ioaddr + reg + XLLSDMA_RX_REGS);
> +}
> +
> +void xllsdma_reset(struct xllsdma_device *sdma)
> +{
> +	u32 rx_cr, tx_cr, rx_irq, tx_irq;
> +
> +	unsigned long flags;
> +	struct xllsdma_client *client, *tmp;
> +
> +	DEFINE_XLLSDMA_COALESCE(coal);
> +	spin_lock_irqsave(&sdma->lock, flags);
> +
> +	xllsdma_write_cr(sdma, XLLSDMA_DMACR_RESET);
> +
> +	while (xllsdma_read_cr(sdma) & XLLSDMA_DMACR_RESET)
> +		udelay(100);
> +
> +	rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
> +	tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
> +
> +	xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr & ~XLLSDMA_CR_IRQ_ALL);
> +	xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr & ~XLLSDMA_CR_IRQ_ALL);
> +
> +	rx_irq = xllsdma_rx_in32(sdma, XLLSDMA_IRQ);
> +	tx_irq = xllsdma_tx_in32(sdma, XLLSDMA_IRQ);
> +
> +	xllsdma_rx_out32(sdma, XLLSDMA_IRQ, rx_irq);
> +	xllsdma_tx_out32(sdma, XLLSDMA_IRQ, tx_irq);
> +
> +	xllsdma_write_cr(sdma, XLLSDMA_DMACR_TAIL_PTR_EN |
> +		XLLSDMA_DMACR_RX_OVF_DIS | XLLSDMA_DMACR_TX_OVF_DIS);
> +
> +	if (sdma->rx_irq != NO_IRQ) {
> +		xllsdma_rx_out32(sdma, XLLSDMA_CR,
> +			      rx_cr | (XLLSDMA_CR_IRQ_ALL & ~XLLSDMA_CR_IRQ_EN));
> +
> +		rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
> +		xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr | XLLSDMA_CR_IRQ_EN);
> +	}
> +
> +	if (sdma->tx_irq != NO_IRQ) {
> +		xllsdma_tx_out32(sdma, XLLSDMA_CR,
> +			      tx_cr | (XLLSDMA_CR_IRQ_ALL & ~XLLSDMA_CR_IRQ_EN));
> +		tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
> +		xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr | XLLSDMA_CR_IRQ_EN);
> +	}
> +
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +
> +	list_for_each_entry_safe(client, tmp, &sdma->clients, item)
> +		if (likely(client->reset))
> +			client->reset(client->data);
> +
> +	xllsdma_set_coalesce(sdma, &coal);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_reset);
> +
> +void xllsdma_tx_irq_enable(struct xllsdma_device *sdma)
> +{
> +	u32 tx_cr;
> +	unsigned long flags;
> +
> +	BUG_ON(sdma->tx_irq == NO_IRQ);
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +	tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
> +	xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr | XLLSDMA_CR_IRQ_EN);
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_tx_irq_enable);
> +
> +void xllsdma_rx_irq_enable(struct xllsdma_device *sdma)
> +{
> +	u32 rx_cr;
> +	unsigned long flags;
> +
> +	BUG_ON(sdma->rx_irq == NO_IRQ);
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +	rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
> +	xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr | XLLSDMA_CR_IRQ_EN);
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_rx_irq_enable);
> +
> +void xllsdma_tx_irq_disable(struct xllsdma_device *sdma)
> +{
> +	u32 tx_cr;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +	tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
> +	xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr & ~XLLSDMA_CR_IRQ_EN);
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_tx_irq_disable);
> +
> +void xllsdma_rx_irq_disable(struct xllsdma_device *sdma)
> +{
> +	u32 rx_cr;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +	rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
> +	xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr & ~XLLSDMA_CR_IRQ_EN);
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_rx_irq_disable);
> +
> +void xllsdma_tx_irq_ack(struct xllsdma_device *sdma)
> +{
> +	u32 irq_stat;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +	irq_stat = xllsdma_tx_in32(sdma, XLLSDMA_IRQ);
> +	xllsdma_tx_out32(sdma, XLLSDMA_IRQ, irq_stat & XLLSDMA_IRQ_ALL_DONE);
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_tx_irq_ack);
> +
> +void xllsdma_rx_irq_ack(struct xllsdma_device *sdma)
> +{
> +	u32 irq_stat;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +	irq_stat = xllsdma_rx_in32(sdma, XLLSDMA_IRQ);
> +	xllsdma_rx_out32(sdma, XLLSDMA_IRQ, irq_stat & XLLSDMA_IRQ_ALL_DONE);
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_rx_irq_ack);
> +
> +void xllsdma_pause(struct xllsdma_device *sdma)
> +{
> +	u32 dmacr;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +	dmacr = xllsdma_read_cr(sdma);
> +	dmacr |= XLLSDMA_DMACR_TX_PAUSE | XLLSDMA_DMACR_RX_PAUSE;
> +	xllsdma_write_cr(sdma, dmacr);
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_pause);
> +
> +void xllsdma_resume(struct xllsdma_device *sdma)
> +{
> +	u32 dmacr;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +	dmacr = xllsdma_read_cr(sdma);
> +	dmacr &= ~(XLLSDMA_DMACR_TX_PAUSE | XLLSDMA_DMACR_RX_PAUSE);
> +	xllsdma_write_cr(sdma, dmacr);
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_resume);
> +
> +int xllsdma_set_coalesce(struct xllsdma_device *sdma, struct xllsdma_coalesce *coal)
> +{
> +	u32 tx_cr, rx_cr;
> +	unsigned long flags;
> +
> +	if (coal->tx_timeout > 255 ||
> +	    coal->rx_timeout > 255 ||
> +	    coal->tx_threshold > 255 ||
> +	    coal->rx_threshold > 255)
> +		return -EINVAL;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +
> +	if (sdma->rx_irq != NO_IRQ) {
> +		rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
> +
> +		if (coal->rx_timeout == 0) {
> +			coal->rx_timeout = 1;
> +			rx_cr &= ~XLLSDMA_CR_IRQ_TIMEOUT;
> +		} else {
> +			rx_cr |= XLLSDMA_CR_IRQ_TIMEOUT;
> +		}
> +
> +		rx_cr &= ~(XLLSDMA_CR_IRQ_THRESHOLD_MSK | XLLSDMA_CR_IRQ_TIMEOUT_SH);
> +		rx_cr |= (coal->rx_threshold << XLLSDMA_CR_IRQ_THRESHOLD_SH)
> +			 & XLLSDMA_CR_IRQ_THRESHOLD_MSK;
> +		rx_cr |= (coal->rx_timeout << XLLSDMA_CR_IRQ_TIMEOUT_SH)
> +			 & XLLSDMA_CR_IRQ_TIMEOUT_MSK;
> +		rx_cr |= XLLSDMA_CR_LD_IRQ_CNT;
> +
> +		xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr);
> +	}
> +
> +	if (sdma->tx_irq != NO_IRQ) {
> +		tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
> +
> +		if (coal->tx_timeout == 0) {
> +			coal->tx_timeout = 1;
> +			tx_cr &= ~XLLSDMA_CR_IRQ_TIMEOUT;
> +		} else {
> +			tx_cr |= XLLSDMA_CR_IRQ_TIMEOUT;
> +		}
> +
> +		tx_cr &= ~(XLLSDMA_CR_IRQ_THRESHOLD_MSK | XLLSDMA_CR_IRQ_TIMEOUT_SH);
> +		tx_cr |= (coal->tx_threshold << XLLSDMA_CR_IRQ_THRESHOLD_SH)
> +			 & XLLSDMA_CR_IRQ_THRESHOLD_MSK;
> +		tx_cr |= (coal->tx_timeout << XLLSDMA_CR_IRQ_TIMEOUT_SH)
> +			 & XLLSDMA_CR_IRQ_TIMEOUT_MSK;
> +		tx_cr |= XLLSDMA_CR_LD_IRQ_CNT;
> +
> +		xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr);
> +	}
> +
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_set_coalesce);
> +
> +int xllsdma_get_coalesce(struct xllsdma_device *sdma, struct xllsdma_coalesce *coal)
> +{
> +	u32 tx_cr, rx_cr;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&sdma->lock, flags);
> +
> +	tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
> +	rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
> +
> +	coal->tx_threshold = (tx_cr & XLLSDMA_CR_IRQ_THRESHOLD_MSK)
> +			     >> XLLSDMA_CR_IRQ_THRESHOLD_SH;
> +	coal->tx_timeout = (tx_cr & XLLSDMA_CR_IRQ_TIMEOUT_MSK)
> +			   >> XLLSDMA_CR_IRQ_TIMEOUT_SH;
> +
> +	coal->rx_threshold = (rx_cr & XLLSDMA_CR_IRQ_THRESHOLD_MSK)
> +			     >> XLLSDMA_CR_IRQ_THRESHOLD_SH;
> +	coal->rx_timeout = (rx_cr & XLLSDMA_CR_IRQ_TIMEOUT_MSK)
> +			     >> XLLSDMA_CR_IRQ_TIMEOUT_SH;
> +
> +	if (!(tx_cr & XLLSDMA_CR_IRQ_TIMEOUT))
> +		coal->tx_timeout = 0;
> +
> +	if (!(rx_cr & XLLSDMA_CR_IRQ_TIMEOUT))
> +		coal->rx_timeout = 0;
> +
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_get_coalesce);
> +
> +int xllsdma_tx_submit(struct xllsdma_device *sdma, dma_addr_t desc)
> +{
> +	xllsdma_tx_out32(sdma, XLLSDMA_TDESCR, desc);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_tx_submit);

Invariant return value can be dropped.

> 
> +
> +int xllsdma_rx_submit(struct xllsdma_device *sdma, dma_addr_t desc)
> +{
> +	xllsdma_rx_out32(sdma, XLLSDMA_TDESCR, desc);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_rx_submit);
> 
Invariant return value can be dropped.

> +
> +void xllsdma_tx_init(struct xllsdma_device *sdma, dma_addr_t desc)
> +{
> +	xllsdma_tx_out32(sdma, XLLSDMA_CDESCR, desc);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_tx_init);
> +
> +void xllsdma_rx_init(struct xllsdma_device *sdma, dma_addr_t desc)
> +{
> +	xllsdma_rx_out32(sdma, XLLSDMA_CDESCR, desc);
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_rx_init);
> +
> +struct xllsdma_device *xllsdma_find_device(int phandle)
> +{
> +	struct mpmc_device *mpmc;
> +	struct xllsdma_device *sdma = NULL;
> +	int found = 0;
> +	mutex_lock(&mpmc_devs_lock);
> +	list_for_each_entry(mpmc, &mpmc_devs, item) {
> +		mutex_lock(&mpmc->devs_lock);
> +		list_for_each_entry(sdma, &mpmc->xllsdma_devs, item) {
> +			if (sdma->phandle == phandle) {
> +				found = 1;
> +				break;
> +			}
> +		}
> +		mutex_unlock(&mpmc->devs_lock);
> +		if (found)
> +			break;
> +		else
> +			sdma = NULL;
> +	}
> +	mutex_unlock(&mpmc_devs_lock);
> +	return sdma;
> +}
> +EXPORT_SYMBOL_GPL(xllsdma_find_device);

I'm still concerned that there is no concept of "allocating" or "reserving" a 
channel. This seems to invite accidental concurrent use of a channel, if not in
the field, then during development.

> +
> +static irqreturn_t xllsdma_rx_intr(int irq, void *dev_id)
> +{
> +	u32 irq_ack, status;
> +	struct xllsdma_device *sdma = dev_id;
> +	struct xllsdma_client *client, *tmp;
> +
> +	/* Read pending interrupts */
> +	status = xllsdma_rx_in32(sdma, XLLSDMA_IRQ);
> +	irq_ack = status;
> +	irq_ack &= sdma->rx_ack ? XLLSDMA_IRQ_ALL : XLLSDMA_IRQ_ALL_ERR;
> +	xllsdma_rx_out32(sdma, XLLSDMA_IRQ, irq_ack);
> +
> +	if (unlikely(status & XLLSDMA_IRQ_ALL_ERR)) {
> +		dev_err(sdma->dev, "%s: error status: %08x\n", __func__, 
> +			status);
> +		xllsdma_reset(sdma);
> +		list_for_each_entry_safe(client, tmp, &sdma->clients, item)
> +			if (likely(client->error))
> +				client->error(client->data);
> +		return IRQ_HANDLED;
> +	}
> +
> +	if (likely(status & XLLSDMA_IRQ_ALL_DONE)) {
> +		list_for_each_entry_safe(client, tmp, &sdma->clients, item)
> +			if (likely(client->rx_complete))
> +				client->rx_complete(client->data);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t xllsdma_tx_intr(int irq, void *dev_id)
> +{
> +	u32 irq_ack, status;
> +	struct xllsdma_device *sdma = dev_id;
> +	struct xllsdma_client *client, *tmp;
> +
> +	/* Read pending interrupts */
> +	status = xllsdma_tx_in32(sdma, XLLSDMA_IRQ);
> +	irq_ack = status;
> +	irq_ack &= sdma->tx_ack ? XLLSDMA_IRQ_ALL : XLLSDMA_IRQ_ALL_ERR;
> +	xllsdma_tx_out32(sdma, XLLSDMA_IRQ, irq_ack);
> +
> +	if (unlikely(status & XLLSDMA_IRQ_ALL_ERR)) {
> +		dev_err(sdma->dev, "%s: error status: %08x\n", __func__, 
> +			status);
> +		xllsdma_reset(sdma);
> +		list_for_each_entry_safe(client, tmp, &sdma->clients, item)
> +			if (likely(client->error))
> +				client->error(client->data);
> +		return IRQ_HANDLED;
> +	}
> +
> +	if (likely(status & XLLSDMA_IRQ_ALL_DONE)) {
> +		list_for_each_entry_safe(client, tmp, &sdma->clients, item)
> +			if (likely(client->tx_complete))
> +				client->tx_complete(client->data);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void xllsdma_dev_register(struct mpmc_device *mpmc,
> +			      struct xllsdma_device *sdma)
> +{
> +	mutex_lock(&mpmc->devs_lock);
> +	list_add(&sdma->item, &mpmc->xllsdma_devs);
> +	mutex_unlock(&mpmc->devs_lock);
> +}
> +
> +static void xllsdma_dev_unregister(struct xllsdma_device *sdma)
> +{
> +	struct mpmc_device *mpmc = sdma->parent;
> +
> +	mutex_lock(&mpmc->devs_lock);
> +	list_del(&sdma->item);
> +	mutex_unlock(&mpmc->devs_lock);
> +}
> +
> +static void xllsdma_cleanup(struct device *dev)
> +{
> +	struct xllsdma_device *sdma = dev_get_drvdata(dev);
> +
> +	if (sdma->tx_irq)
> +		free_irq(sdma->tx_irq, sdma);
> +
> +	if (sdma->rx_irq)
> +		free_irq(sdma->rx_irq, sdma);
> +
> +	if (sdma->memregion.start)
> +		release_mem_region(sdma->memregion.start,
> +			sdma->memregion.end - sdma->memregion.start + 1);
> +
> +	if (sdma->ioaddr)
> +		iounmap(sdma->ioaddr);
> +
> +	xllsdma_dev_unregister(sdma);
> +	kfree(sdma);
> +	dev_set_drvdata(dev, NULL);
> +}
> +
> +static void mpmc_dev_register(struct mpmc_device *mpmc)
> +{
> +	mutex_lock(&mpmc_devs_lock);
> +	list_add_tail(&mpmc->item, &mpmc_devs);
> +	mutex_unlock(&mpmc_devs_lock);
> +}
> +
> +static void mpmc_dev_unregister(struct mpmc_device *mpmc)
> +{
> +	mutex_lock(&mpmc_devs_lock);
> +	list_del(&mpmc->item);
> +	mutex_unlock(&mpmc_devs_lock);
> +}
> +
> +static void mpmc_cleanup(struct device *dev)
> +{
> +	struct mpmc_device *mpmc = dev_get_drvdata(dev);
> +
> +	if (mpmc->registered)
> +		mpmc_dev_unregister(mpmc);
> +
> +	kfree(mpmc);
> +	dev_set_drvdata(dev, NULL);
> +}
> +
> +static int __devinit xllsdma_init(struct device *dev, struct resource *rx_irq,
> +			       struct resource *tx_irq, struct resource *mem,
> +			       int phandle)
> +{
> +	struct xllsdma_device *sdma;
> +	struct mpmc_device *mpmc;
> +
> +	resource_size_t region_size;
> +	int res;
> +
> +	mpmc = dev_get_drvdata(dev->parent);
> +
> +	sdma = kzalloc(sizeof(struct xllsdma_device), GFP_KERNEL);
> +	if (!sdma) {
> +		dev_err(dev, "Cannot allocate SDMA device\n");
> +		return -ENOMEM;
> +	}
> +	dev_set_drvdata(dev, sdma);
> +	sdma->dev = dev;
> +
> +	spin_lock_init(&sdma->lock);
> +	INIT_LIST_HEAD(&sdma->clients);
> +	mutex_init(&sdma->clients_lock);
> +	sdma->parent = mpmc;
> +	sdma->phandle = phandle;
> +
> +	region_size = mem->end - mem->start + 1;
> +	if (!request_mem_region(mem->start, region_size, DRV_NAME)) {
> +		dev_err(dev, "I/O memory region at %p is busy\n",
> +			(void *)mem->start);
> +		return -EBUSY;
> +	}
> +	sdma->memregion = *mem;
> +
> +	sdma->ioaddr = ioremap(mem->start, region_size);
> +	if (!sdma->ioaddr) {
> +		dev_err(dev, "Cannot ioremap() I/O memory %p\n",
> +			(void *)mem->start);
> +		return -ENOMEM;
> +	}
> +
> +	xllsdma_reset(sdma);
> +
> +	sdma->rx_irq = NO_IRQ;
> +	if (rx_irq) {
> +		res = request_irq(rx_irq->start, xllsdma_rx_intr,
> +				IRQF_SHARED, "SDMA RX", sdma);
> +		if (res) {
> +			dev_err(dev, "Could not allocate RX interrupt %d.\n",
> +				rx_irq->start);
> +			return res;
> +		}
> +		sdma->rx_irq = rx_irq->start;
> +	}
> +
> +	sdma->tx_irq = NO_IRQ;
> +	if (tx_irq) {
> +		res = request_irq(tx_irq->start, xllsdma_tx_intr,
> +				IRQF_SHARED, "SDMA TX", sdma);
> +		if (res) {
> +			dev_err(dev, "Could not allocate TX interrupt %d.\n",
> +				tx_irq->start);
> +			return res;
> +		}
> +		sdma->tx_irq = tx_irq->start;
> +	}
> +
> +	sdma->rx_ack = 1;
> +	sdma->tx_ack = 1;
> +	xllsdma_dev_register(mpmc, sdma);
> +
> +	return 0;
> +}
> +
> +static int __devinit mpmc_init(struct device *dev)
> +{
> +	struct mpmc_device *mpmc;
> +
> +	mpmc = kzalloc(sizeof(struct mpmc_device), GFP_KERNEL);
> +
> +	if (!mpmc) {
> +		dev_err(dev, "Cannot allocate MPMC device\n");
> +		return -ENOMEM;
> +	}
> +
> +	dev_set_drvdata(dev, mpmc);
> +
> +	INIT_LIST_HEAD(&mpmc->xllsdma_devs);
> +	mutex_init(&mpmc->devs_lock);
> +
> +	mpmc_dev_register(mpmc);
> +	mpmc->registered = 1;
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static int xllsdma_of_remove(struct of_device *op)
> +{
> +	xllsdma_cleanup(&op->dev);
> +	return 0;
> +}
> +
> +/* Match table for of_platform binding */
> +static struct of_device_id xllsdma_of_match[] = {
> +	{ .compatible = "xlnx,ll-dma-1.00.a" },
> +	{},
> +};
> +
> +static int __devinit xllsdma_of_probe(struct of_device *op,
> +				   const struct of_device_id *match)
> +{
> +	const int *prop;
> +	int phandle;
> +	struct resource rx_irq, tx_irq, mem;
> +	struct resource *tx_irq_res = NULL;
> +	struct resource *rx_irq_res = NULL;
> +	int res;
> +
> +	res = of_address_to_resource(op->node, 0, &mem);
> +	if (res) {
> +		dev_err(&op->dev, "invalid address\n");
> +		return res;
> +	}
> +
> +	/* IRQ */
> +	res = of_irq_to_resource(op->node, 0, &rx_irq);
> +	if (res != NO_IRQ)
> +		rx_irq_res = &rx_irq;
> +
> +	res = of_irq_to_resource(op->node, 1, &tx_irq);
> +	if (res != NO_IRQ)
> +		tx_irq_res = &tx_irq;
> +
> +	prop = of_get_property(op->node, "linux,phandle", NULL);
> +	phandle = (prop) ? *prop : -1;
> +
> +	res = xllsdma_init(&op->dev, rx_irq_res, tx_irq_res, &mem, phandle);
> +	if (res)
> +		xllsdma_of_remove(op);
> +
> +	return res;
> +}
> +
> +static struct of_platform_driver xllsdma_of_driver = {
> +	.name		= "xilinx-sdma",
> +	.match_table	= xllsdma_of_match,
> +	.probe		= xllsdma_of_probe,
> +	.remove		= xllsdma_of_remove,
> +};
> +
> +int __init xllsdma_of_init(void)
> +{
> +	int ret;
> +
> +	ret = of_register_platform_driver(&xllsdma_of_driver);
> +	if (ret) {
> +		of_unregister_platform_driver(&xllsdma_of_driver);
> +		printk(KERN_ERR "registering driver failed: err=%i", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +void xllsdma_of_exit(void)
> +{
> +	of_unregister_platform_driver(&xllsdma_of_driver);
> +}
> +
> +static int mpmc_of_remove(struct of_device *op)
> +{
> +	struct device_node *node;
> +	struct of_device *ofdev;
> +
> +	for_each_child_of_node(op->node, node) {
> +		ofdev = of_find_device_by_node(node);
> +		of_device_unregister(ofdev);
> +		of_device_free(ofdev);
> +	}
> +
> +	mpmc_cleanup(&op->dev);
> +	return 0;
> +}
> +
> +static int __devinit mpmc_of_probe(struct of_device *op,
> +			const struct of_device_id *match)
> +{
> +	int err = mpmc_init(&op->dev);
> +	if (err)
> +		return err;
> +
> +	of_platform_bus_probe(op->node, xllsdma_of_match, &op->dev);
> +	return 0;
> +}
> +
> +static struct of_device_id  __devinitdata mpmc_of_match[] = {
> +	{ .compatible = "xlnx,mpmc-4.01.a" },
> +	{ .compatible = "xlnx,mpmc-4.03.a" },
> +	{},
> +};
> +
> +static struct of_platform_driver mpmc_of_driver = {
> +	.name = "xilinx-mpmc",
> +	.match_table = mpmc_of_match,
> +	.probe = mpmc_of_probe,
> +	.remove	= mpmc_of_remove,
> +};
> +
> +int __init mpmc_of_init(void)
> +{
> +	return of_register_platform_driver(&mpmc_of_driver);
> +}
> +
> +void mpmc_of_exit(void)
> +{
> +	of_unregister_platform_driver(&mpmc_of_driver);
> +}
> +
> +subsys_initcall(mpmc_of_init);
> +subsys_initcall(xllsdma_of_init);
> +#else	/* CONFIG_OF */
> +/*---------------------------------------------------------------------------
> + * Platform bus attachment
> + */
> +
> +static __devexit int xllsdma_plat_remove(struct platform_device *pdev)
> +{
> +	xllsdma_cleanup(&pdev->dev);
> +	return 0;
> +}
> +
> +static int __devinit xllsdma_plat_probe(struct platform_device *pdev)
> +{
> +	struct resource *rx_irq, *tx_irq, *mem;
> +	int err = 0;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem) {
> +		dev_err(&pdev->dev, "invalid address\n");
> +		err = -EINVAL;
> +		goto fail;
> +	}
> +
> +	/* RX interrupt is optional, and first */
> +	rx_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +
> +	/* TX interrupt is optional, and second */
> +	tx_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
> +

If it is impossible to create duplicate phandles in Device Tree, 
there should be a check here that no device with pdev->id already exists
(i.e., it's NOT impossible with platform bus). It might be just as well to
put the check in xllsdma_init() since that's not a 'hot' code path.

> +	err = xllsdma_init(&pdev->dev, rx_irq, tx_irq, mem, pdev->id);
> +	if (err)
> +		xllsdma_plat_remove(pdev);
> +fail:
> +	return err;
> +}
> +
> +static struct platform_driver xllsdma_plat_driver = {
> +	.probe = xllsdma_plat_probe,
> +	.remove	= __devexit_p(xllsdma_plat_remove),
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name  = "xilinx-sdma",
> +	},
> +};
> +
> +int __init xllsdma_plat_init(void)
> +{
> +	int err = platform_driver_register(&xllsdma_plat_driver);
> +	if (err) {
> +		platform_driver_unregister(&xllsdma_plat_driver);
> +		printk(KERN_ERR "registering driver failed: err=%i", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +subsys_initcall(xllsdma_plat_init);
> +
> +void xllsdma_plat_exit(void)
> +{
> +	platform_driver_unregister(&xllsdma_plat_driver);
> +}
> +
> +static int mpmc_plat_probe(struct platform_device *pdev)
> +{
> +	return mpmc_init(&pdev->dev);
> +}
> +
> +static int __devexit mpmc_plat_remove(struct platform_device *pdev)
> +{
> +	mpmc_cleanup(&pdev->dev);
> +	return 0;
> +}
> +
> +static struct platform_driver mpmc_plat_driver = {
> +	.probe = mpmc_plat_probe,
> +	.remove	= __devexit_p(mpmc_plat_remove),
> +	.driver = {
> +		.owner = THIS_MODULE,
> +		.name  = "xilinx-mpmc",
> +	},
> +};
> +
> +int __init mpmc_plat_init(void)
> +{
> +	return platform_driver_register(&mpmc_plat_driver);
> +}
> +subsys_initcall(mpmc_plat_init);
> +
> +void mpmc_plat_exit(void)
> +{
> +	platform_driver_unregister(&mpmc_plat_driver);
> +}
> +#endif	/* CONFIG_OF */
> diff -r baced9e29ab5 include/linux/xllsdma.h
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/include/linux/xllsdma.h	Wed Apr 28 02:00:51 2010 +0400
> @@ -0,0 +1,187 @@
> +/*
> + * SDMA subsystem support for Xilinx MPMC.
> + *
> + * Author: Sergey Temerkhanov
> + *
> + * Copyright (c) 2008-2010 Cifronic ZAO
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef __XLLSDMA_H__
> +#define __XLLSDMA_H__
> +
> +#include <linux/types.h>
> +#include <linux/dma-mapping.h>
> +
> +#define XLLSDMA_ALIGNMENT	0x40
> +
> +struct xllsdma_desc {
> +	__be32 next;
> +	__be32 address;
> +	__be32 length;
> +	__be32 stat_ctl;
> +	__be32 app1;
> +	__be32 app2;
> +	__be32 app3;
> +	__be32 app4;
> +	void *virt;
> +	u32 flags;
> +} __attribute__((aligned(XLLSDMA_ALIGNMENT)));

'virt' and 'flags' are not used by this driver. Putting them in this structure,
and using this structure in the API, forces them on all clients, which can lead 
to inefficiencies (this will certainly be true in the DMA engine driver, perhaps
others). I think a better choice would be to have the API use a structure without
these two fields. For the convenience of other clients you could define a
'superstructure' _not_ used in the API, like:

struct xllsdma_fancy_desc {
	struct xllsdma_desc  basic_desc;
	void *virt;
	u32 flags;
}

> +
> +
> +enum {
> +	XLLSDMA_STSCTL_ERROR	= (1 << 31), /* DMA error */
> +	XLLSDMA_STSCTL_IOE		= (1 << 30), /* Interrupt on end */
> +	XLLSDMA_STSCTL_SOE		= (1 << 29), /* Stop on end */
> +	XLLSDMA_STSCTL_DONE	= (1 << 28), /* DMA completed */
> +	XLLSDMA_STSCTL_SOP		= (1 << 27), /* Start of packet */
> +	XLLSDMA_STSCTL_EOP		= (1 << 26), /* End of packet */
> +	XLLSDMA_STSCTL_BUSY	= (1 << 25), /* DMA busy */
> +	XLLSDMA_STSCTL_CSUM	= (1 << 0),  /* Checksum enable */
> +
> +	XLLSDMA_STSCTL_MSK		= (0xFF << 24), /*Status/control field */
> +};

The search-and-replace messed up your nice alignment.

> +
> +/* SDMA client operations */
> +struct xllsdma_client {
> +	void *data;
> +	void (*tx_complete) (void *data);
> +	void (*rx_complete) (void *data);
> +	void (*error) (void *data);
> +	void (*reset) (void *data);
> +	struct list_head item;
> +};
> +
> +struct xllsdma_coalesce {
> +	int tx_threshold;
> +	int tx_timeout;
> +
> +	int rx_threshold;
> +	int rx_timeout;
> +};
> +
> +#define DEFINE_XLLSDMA_COALESCE(x) struct xllsdma_coalesce x = { \
> +	.tx_timeout	= 0, \
> +	.tx_threshold	= 1, \
> +	.rx_timeout	= 0, \
> +	.rx_threshold	= 1, };
> +
> +struct mpmc_device {
> +	void __iomem		*ioaddr;
> +
> +	struct resource		memregion;
> +	int			irq;
> +
> +	int			registered;
> +	struct list_head	item;
> +
> +	struct mutex		devs_lock;
> +	struct list_head	xllsdma_devs;
> +};
> +
> +struct xllsdma_device {
> +	struct device		*dev;
> +	void __iomem		*ioaddr;
> +	wait_queue_head_t 	wait;
> +
> +	spinlock_t		lock;
> +
> +	struct resource		memregion;
> +	int			rx_irq;
> +	int			tx_irq;
> +	int			rx_ack;
> +	int			tx_ack;
> +	int			phandle;
> +
> +	int			registered;
> +	struct mpmc_device	*parent;
> +
> +	struct xllsdma_coalesce	coal;
> +	struct list_head	item;
> +
> +	struct mutex		clients_lock;
> +	struct list_head	clients;
> +};
> +
> +static inline void xllsdma_add_client(struct xllsdma_device *sdma,
> +				   struct xllsdma_client *client)
> +{
> +	mutex_lock(&sdma->clients_lock);
> +	list_add(&client->item, &sdma->clients);
> +	mutex_unlock(&sdma->clients_lock);
> +}
> +
> +static inline void xllsdma_del_client(struct xllsdma_device *sdma,
> +				   struct xllsdma_client *client)
> +{
> +	mutex_lock(&sdma->clients_lock);
> +	list_del(&client->item);
> +	mutex_unlock(&sdma->clients_lock);
> +}
> +
> +struct xllsdma_device *xllsdma_find_device(int phandle);
> +void xllsdma_pause(struct xllsdma_device *sdma);
> +void xllsdma_resume(struct xllsdma_device *sdma);
> +void xllsdma_reset(struct xllsdma_device *sdma);
> +void xllsdma_rx_init(struct xllsdma_device *sdma, dma_addr_t desc);
> +void xllsdma_tx_init(struct xllsdma_device *sdma, dma_addr_t desc);
> +
> +int xllsdma_tx_submit(struct xllsdma_device *sdma, dma_addr_t desc);
> +int xllsdma_rx_submit(struct xllsdma_device *sdma, dma_addr_t desc);
> +
> +void xllsdma_tx_irq_enable(struct xllsdma_device *sdma);
> +void xllsdma_rx_irq_enable(struct xllsdma_device *sdma);
> +void xllsdma_tx_irq_disable(struct xllsdma_device *sdma);
> +void xllsdma_rx_irq_disable(struct xllsdma_device *sdma);
> +void xllsdma_tx_irq_ack(struct xllsdma_device *sdma);
> +void xllsdma_rx_irq_ack(struct xllsdma_device *sdma);
> +
> +int xllsdma_set_coalesce(struct xllsdma_device *sdma, struct xllsdma_coalesce *coal);
> +int xllsdma_get_coalesce(struct xllsdma_device *sdma, struct xllsdma_coalesce *coal);
> +
> +static inline int xllsdma_has_tx_irq(struct xllsdma_device *sdma)
> +{
> +	return sdma->tx_irq != NO_IRQ;
> +}
> +
> +static inline int xllsdma_has_rx_irq(struct xllsdma_device *sdma)
> +{
> +	return sdma->rx_irq != NO_IRQ;
> +}
> +
> +static inline int xllsdma_desc_busy(struct xllsdma_desc *desc)
> +{
> +	return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_BUSY);
> +}
> +
> +static inline int xllsdma_desc_done(struct xllsdma_desc *desc)
> +{
> +	return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_DONE);
> +}
> +
> +static inline int xllsdma_desc_sop(struct xllsdma_desc *desc)
> +{
> +	return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_SOP);
> +}
> +
> +static inline int xllsdma_desc_eop(struct xllsdma_desc *desc)
> +{
> +	return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_EOP);
> +}
> +
> +static inline void xllsdma_set_ack(struct xllsdma_device *sdma, int rx_ack,
> +				int tx_ack)
> +{
> +	unsigned long flags;
> +	spin_lock_irqsave(&sdma->lock, flags);
> +	sdma->rx_ack = rx_ack;
> +	sdma->tx_ack = tx_ack;
> +	spin_unlock_irqrestore(&sdma->lock, flags);
> +}
> +
> +#endif

------------------------------------------------------------------------
 Steven J. Magnani               "I claim this network for MARS!
 www.digidescorp.com              Earthling, return my space modulator!"

 #include <standard.disclaimer>

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Grant Likely @ 2010-04-28  2:31 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, kumar.gala, linuxppc-dev, Timur Tabi, Liam Girdwood
In-Reply-To: <20100427222913.GE15083@opensource.wolfsonmicro.com>

On Tue, Apr 27, 2010 at 4:29 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Apr 27, 2010 at 02:24:34PM -0600, Grant Likely wrote:
>
>> However, as you and Mark rightly point out, it completely fails to
>> represent complex sound devices with weird clocking and strange
>> routes. =A0Something like this is probably more appropriate:
>
>>
>> =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 codec1 :codec@4f {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "cirrus,cs427=
0";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 reg =3D <0x4f>;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* MCLK source is a stand-al=
one oscillator */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock-frequency =3D <1228800=
0>;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 };
>
> You also want to be representing unused pins here.
>
>> =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 ssi1: ssi@16000 {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-ssi";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 [...]
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 fsl,mode =3D "i2s-slave";
>
> I'd not include the master/slave decision; it's either implied by the
> fact that the CODEC has a standalone clock, a property of the link/card,
> or a policy decision that the running software can change on a whim.
>
>> =A0 =A0 =A0 sound {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 compatible =3D "fsl,mpc8610-hpcd-sound";
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* maybe something like (totally off the top=
 of my head) */
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 dai-links =3D <&ssi1 0 &codec 0
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&ssi1 1 &codec 1>=
;
>
> I'm having a hard time loving this. =A0I'd be looking for a lot more
> semantics on the links (things like information about separate clocks
> for the two directions, for example) which makes me think that that
> simple list format is far too simple and you want a list of more complex
> objects.

Oh, absolutely.  This example is no where near complete.  Mostly I
just wanted to give a concrete example of a 'virtual' device like Ben
was talking about.  I'm not going to even attempt to draft a complete
binding until I've got time to properly go over the issues involved
and discuss them with you and Liam.

> There's also analogue interconnects to deal with, and jacks (including
> detection and accessories). =A0Jacks can be particularly entertaining
> here.
>
>> Or, in other words, the device tree should *not* be used to describe
>> every possible detail and permutation. =A0It is best used to describe
>> the permutations that are common so that they don't need to be hard
>> coded for each and every board.
>
> I think the ideal is something that's purely descriptive of the hardware
> and doesn't include any policy decisions. =A0Policy decisions covers an
> awful lot of the interesting issues, though - but they're the sort of
> things that can easily be changed with a new software load and therefore
> don't belong in the device tree.

Agreed.

> On the other hand from a pragmatic point of view it's just much less
> hassle to just only provide the mechanism for instantiating a machine
> with custom code and use that for everything.

Also true, but this approach carries with it an incremental cost that
distributions feel the pain of.  Ultimately I think we'll find a sweet
spot somewhere in between.

Cheers,
g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [patch 03/15] powerpc: cpumask: Convert smp_cpus_done to new cpumask API
From: Stephen Rothwell @ 2010-04-28  2:09 UTC (permalink / raw)
  To: Anton Blanchard; +Cc: rusty, linuxppc-dev
In-Reply-To: <20100427013431.787617147@samba.org>

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

Hi Anton,

On Tue, 27 Apr 2010 11:32:34 +1000 Anton Blanchard <anton@samba.org> wrote:
>
> -	old_mask = current->cpus_allowed;
> -	set_cpus_allowed(current, cpumask_of_cpu(boot_cpuid));
> -	
> +	alloc_cpumask_var(&old_mask, GFP_NOWAIT);

You should probably put an explicit include of linux/gfp.h in this file
for this.  (Maybe linux/cpumask.h should include it - but it doesn't.)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [alsa-devel] [PATCH 1/2] powerpc: add platform registration for ALSA SoC drivers
From: Mark Brown @ 2010-04-27 22:29 UTC (permalink / raw)
  To: Grant Likely
  Cc: alsa-devel, kumar.gala, linuxppc-dev, Timur Tabi, Liam Girdwood
In-Reply-To: <g2mfa686aa41004271324g8ff0d67fi3df4336cb25e9acc@mail.gmail.com>

On Tue, Apr 27, 2010 at 02:24:34PM -0600, Grant Likely wrote:

> However, as you and Mark rightly point out, it completely fails to
> represent complex sound devices with weird clocking and strange
> routes.  Something like this is probably more appropriate:

> 
> 	[...]
> 		codec1 :codec@4f {
> 			compatible = "cirrus,cs4270";
> 			reg = <0x4f>;
> 			/* MCLK source is a stand-alone oscillator */
> 			clock-frequency = <12288000>;
> 		};

You also want to be representing unused pins here.

> 	[...]
> 	ssi1: ssi@16000 {
> 		compatible = "fsl,mpc8610-ssi";
> 		[...]
> 		fsl,mode = "i2s-slave";

I'd not include the master/slave decision; it's either implied by the
fact that the CODEC has a standalone clock, a property of the link/card,
or a policy decision that the running software can change on a whim.

> 	sound {
> 		compatible = "fsl,mpc8610-hpcd-sound";
> 		/* maybe something like (totally off the top of my head) */
> 		dai-links = <&ssi1 0 &codec 0
> 		             &ssi1 1 &codec 1>;

I'm having a hard time loving this.  I'd be looking for a lot more
semantics on the links (things like information about separate clocks
for the two directions, for example) which makes me think that that
simple list format is far too simple and you want a list of more complex
objects.

There's also analogue interconnects to deal with, and jacks (including
detection and accessories).  Jacks can be particularly entertaining
here.

> Or, in other words, the device tree should *not* be used to describe
> every possible detail and permutation.  It is best used to describe
> the permutations that are common so that they don't need to be hard
> coded for each and every board.

I think the ideal is something that's purely descriptive of the hardware
and doesn't include any policy decisions.  Policy decisions covers an
awful lot of the interesting issues, though - but they're the sort of
things that can easily be changed with a new software load and therefore
don't belong in the device tree.

On the other hand from a pragmatic point of view it's just much less
hassle to just only provide the mechanism for instantiating a machine
with custom code and use that for everything.

^ permalink raw reply

* [PATCHv2] [RFC] Xilinx MPMC SDMA subsystem
From: Sergey Temerkhanov @ 2010-04-27 22:06 UTC (permalink / raw)
  To: linuxppc-dev, Grant Likely, microblaze-uclinux,
	Linux Kernel Mailing List

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]

This is the 2nd version of Xilinx MPMC LocalLink SDMA subsystem

Changelog v2:
* Changed the functions and struct definition prefix from sdma_ to xllsdma_
* Platform bus bindings and various changes by Steven J. Magnani.
* Moved source files from arch/powerpc/sysdev to global locations and added
  CONFIG_XLLSDMA option.

Regards, Sergey Temerkhanov,
Cifronic ZAO

[-- Attachment #2: sdma.patch --]
[-- Type: text/x-patch, Size: 30469 bytes --]

diff -r baced9e29ab5 drivers/dma/Kconfig
--- a/drivers/dma/Kconfig	Tue Apr 27 20:48:50 2010 +0400
+++ b/drivers/dma/Kconfig	Wed Apr 28 02:00:51 2010 +0400
@@ -97,6 +97,14 @@
 	  Support the TXx9 SoC internal DMA controller.  This can be
 	  integrated in chips such as the Toshiba TX4927/38/39.
 
+config XLLSDMA
+	bool "Xilinx MPMC DMA support"
+	depends on XILINX_VIRTEX || MICROBLAZE
+	select DMA_ENGINE
+	help
+	  Support fot Xilinx MPMC LocalLink SDMA. Virtex FPGA family
+	  has it integrated or fabric-based.
+
 config DMA_ENGINE
 	bool
 
@@ -133,3 +141,5 @@
 	  DMA Device driver.
 
 endif
+
+
diff -r baced9e29ab5 drivers/dma/Makefile
--- a/drivers/dma/Makefile	Tue Apr 27 20:48:50 2010 +0400
+++ b/drivers/dma/Makefile	Wed Apr 28 02:00:51 2010 +0400
@@ -10,3 +10,4 @@
 obj-$(CONFIG_AT_HDMAC) += at_hdmac.o
 obj-$(CONFIG_MX3_IPU) += ipu/
 obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o
+obj-$(CONFIG_XLLSDMA) += xllsdma.o
diff -r baced9e29ab5 drivers/dma/xllsdma.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/drivers/dma/xllsdma.c	Wed Apr 28 02:00:51 2010 +0400
@@ -0,0 +1,887 @@
+/*
+ * SDMA subsystem support for Xilinx MPMC.
+ *
+ * Author: Sergey Temerkhanov
+ * Platform Bus by Steven J. Magnani
+ *
+ * Copyright (c) 2008-2010 Cifronic ZAO
+ *
+ * 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/delay.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/errno.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#include <linux/wait.h>
+#include <linux/list.h>
+#include <linux/io.h>
+#include <linux/xllsdma.h>
+
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+
+#define DRV_VERSION "0.1.0"
+#define DRV_NAME "sdma"
+
+MODULE_AUTHOR("Sergey Temerkhanov <temerkhanov@cifronik.ru>");
+MODULE_DESCRIPTION("Xilinx SDMA driver");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(DRV_VERSION);
+
+LIST_HEAD(mpmc_devs);
+DEFINE_MUTEX(mpmc_devs_lock);
+
+enum {
+	XLLSDMA_TX_REGS	= 0x00,	/* TX channel registers beginning */
+	XLLSDMA_RX_REGS	= 0x20,	/* RX channel registers beginning */
+	XLLSDMA_DMACR	= 0x40,	/* DMA control register */
+
+	XLLSDMA_NDESCR	= 0x00,	/* Next descriptor address */
+	XLLSDMA_BUFA	= 0x04,	/* Current buffer address */
+	XLLSDMA_BUFL	= 0x08,	/* Current buffer length */
+	XLLSDMA_CDESCR	= 0x0C,	/* Current descriptor address */
+	XLLSDMA_TDESCR	= 0x10,	/* Tail descriptor address */
+	XLLSDMA_CR	= 0x14,	/* Channel control */
+	XLLSDMA_IRQ	= 0x18,	/* Interrupt register */
+	XLLSDMA_SR	= 0x1C,	/* Status */
+};
+
+enum {
+	XLLSDMA_CR_IRQ_TIMEOUT_MSK   = (0xFF << 24),	/* Interrupt coalesce timeout */
+	XLLSDMA_CR_IRQ_THRESHOLD_MSK = (0xFF << 16),	/* Interrupt coalesce count */
+	XLLSDMA_CR_MSB_ADDR_MSK	     = (0xF << 12),	/* MSB for 36 bit addressing */
+	XLLSDMA_CR_APP_EN	  = (1 << 11),	/* Application data mask enable */
+	XLLSDMA_CR_1_BIT_CNT	  = (1 << 10),	/* All interrupt counters are 1-bit */
+	XLLSDMA_CR_INT_ON_END	  = (1 << 9),	/* Interrupt-on-end */
+	XLLSDMA_CR_LD_IRQ_CNT	  = (1 << 8),	/* Load IRQ_COUNT */
+	XLLSDMA_CR_IRQ_EN	  = (1 << 7),	/* Master interrupt enable */
+	XLLSDMA_CR_IRQ_ERROR	  = (1 << 2),	/* Error interrupt enable */
+	XLLSDMA_CR_IRQ_TIMEOUT	  = (1 << 1),	/* Coalesce timeout interrupt enable */
+	XLLSDMA_CR_IRQ_THRESHOLD  = (1 << 0),	/* Coalesce threshold interrupt enable */
+
+	XLLSDMA_CR_IRQ_ALL	  = XLLSDMA_CR_IRQ_EN | XLLSDMA_CR_IRQ_ERROR |
+					XLLSDMA_CR_IRQ_TIMEOUT | XLLSDMA_CR_IRQ_THRESHOLD,
+
+	XLLSDMA_CR_IRQ_TIMEOUT_SH   = 24,
+	XLLSDMA_CR_IRQ_THRESHOLD_SH = 16,
+	XLLSDMA_CR_MSB_ADDR_SH	    = 12,
+
+	XLLSDMA_IRQ_WRQ_EMPTY	 = (1 << 14),	/* Write Command Queue Empty (rx) */
+	XLLSDMA_IRQ_PLB_RD_ERROR = (1 << 4),	/* PLB Read Error IRQ */
+	XLLSDMA_IRQ_PLB_WR_ERROR = (1 << 3),	/* PLB Write Error IRQ */
+	XLLSDMA_IRQ_ERROR	 = (1 << 2),	/* Error IRQ */
+	XLLSDMA_IRQ_TIMEOUT	 = (1 << 1),	/* Coalesce timeout IRQ */
+	XLLSDMA_IRQ_THRESHOLD	 = (1 << 0),	/* Coalesce threshold IRQ */
+
+	XLLSDMA_IRQ_ALL_ERR	 = 0x1C,	/* All error interrupt */
+	XLLSDMA_IRQ_ALL		 = 0x1F,	/* All interrupt bits */
+	XLLSDMA_IRQ_ALL_DONE	 = 0x3,		/* All work complete interrupt bits */
+
+
+#define XLLSDMA_IRQ_COALESCE_COUNT(x)	((x >> 10) & 0xF)
+#define XLLSDMA_IRQ_DELAY_COUNT(x)		((x >> 8) & 0x3)
+
+	XLLSDMA_SR_ERR_TDESCR	 = (1 << 21),	/* Tail descriptor pointer is invalid */
+	XLLSDMA_SR_ERR_CMPL	 = (1 << 20),	/* Complete bit is set */
+	XLLSDMA_SR_ERR_BUFA	 = (1 << 19),	/* Buffer address is invalid */
+	XLLSDMA_SR_ERR_NDESCR	 = (1 << 18),	/* Next descriptor pointer is invalid */
+	XLLSDMA_SR_ERR_CDESCR	 = (1 << 17),	/* Current descriptor pointer is invalid */
+	XLLSDMA_SR_ERR_BUSYWR	 = (1 << 16),	/* Current descriptor modified */
+	XLLSDMA_SR_ERROR	 = (1 << 7),	/* Error IRQ has occurred */
+	XLLSDMA_SR_IRQ_ON_END	 = (1 << 6),	/* On-end IRQ has occurred */
+	XLLSDMA_SR_STOP_ON_END	 = (1 << 5), 	/* Stop on end has occurred */
+	XLLSDMA_SR_COMPLETED	 = (1 << 4),	/* BD completed */
+	XLLSDMA_SR_SOP		 = (1 << 3),	/* Current BD has SOP set */
+	XLLSDMA_SR_EOP		 = (1 << 2),	/* Current BD has EOP set */
+	XLLSDMA_SR_ENGINE_BUSY	 = (1 << 1),	/* Channel is busy */
+	
+
+	XLLSDMA_DMACR_TX_PAUSE	  = (1 << 29),	/* Pause TX channel */
+	XLLSDMA_DMACR_RX_PAUSE	  = (1 << 28),	/* Pause RX channel */
+	XLLSDMA_DMACR_PLB_ERR_DIS = (1 << 5),	/* Disable PLB error detection */
+	XLLSDMA_DMACR_RX_OVF_DIS  = (1 << 4),	/* Disable error on RX coalesce counter overflows */
+	XLLSDMA_DMACR_TX_OVF_DIS  = (1 << 3),	/* Disable error on TX coalesce counter overflows */
+	XLLSDMA_DMACR_TAIL_PTR_EN = (1 << 2),	/* Enable use of tail pointer register */
+	XLLSDMA_DMACR_EN_ARB_HOLD = (1 << 1),	/* Enable arbitration hold */
+	XLLSDMA_DMACR_RESET	  = (1 << 0),	/* Reset both channels */
+};
+
+static inline void xllsdma_write_cr(struct xllsdma_device *sdma, u32 value)
+{
+	out_be32(sdma->ioaddr + XLLSDMA_DMACR, value);
+}
+
+static inline u32 xllsdma_read_cr(struct xllsdma_device *sdma)
+{
+	return in_be32(sdma->ioaddr + XLLSDMA_DMACR);
+}
+
+static inline void xllsdma_tx_out32(struct xllsdma_device *sdma, int reg, u32 value)
+{
+	out_be32(sdma->ioaddr + reg + XLLSDMA_TX_REGS, value);
+}
+
+static inline u32 xllsdma_tx_in32(struct xllsdma_device *sdma, int reg)
+{
+	return in_be32(sdma->ioaddr + reg + XLLSDMA_TX_REGS);
+}
+
+static inline void xllsdma_rx_out32(struct xllsdma_device *sdma, int reg, u32 value)
+{
+	out_be32(sdma->ioaddr + reg + XLLSDMA_RX_REGS, value);
+}
+
+static inline u32 xllsdma_rx_in32(struct xllsdma_device *sdma, int reg)
+{
+	return in_be32(sdma->ioaddr + reg + XLLSDMA_RX_REGS);
+}
+
+void xllsdma_reset(struct xllsdma_device *sdma)
+{
+	u32 rx_cr, tx_cr, rx_irq, tx_irq;
+
+	unsigned long flags;
+	struct xllsdma_client *client, *tmp;
+
+	DEFINE_XLLSDMA_COALESCE(coal);
+	spin_lock_irqsave(&sdma->lock, flags);
+
+	xllsdma_write_cr(sdma, XLLSDMA_DMACR_RESET);
+
+	while (xllsdma_read_cr(sdma) & XLLSDMA_DMACR_RESET)
+		udelay(100);
+
+	rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
+	tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
+
+	xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr & ~XLLSDMA_CR_IRQ_ALL);
+	xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr & ~XLLSDMA_CR_IRQ_ALL);
+
+	rx_irq = xllsdma_rx_in32(sdma, XLLSDMA_IRQ);
+	tx_irq = xllsdma_tx_in32(sdma, XLLSDMA_IRQ);
+
+	xllsdma_rx_out32(sdma, XLLSDMA_IRQ, rx_irq);
+	xllsdma_tx_out32(sdma, XLLSDMA_IRQ, tx_irq);
+
+	xllsdma_write_cr(sdma, XLLSDMA_DMACR_TAIL_PTR_EN |
+		XLLSDMA_DMACR_RX_OVF_DIS | XLLSDMA_DMACR_TX_OVF_DIS);
+
+	if (sdma->rx_irq != NO_IRQ) {
+		xllsdma_rx_out32(sdma, XLLSDMA_CR,
+			      rx_cr | (XLLSDMA_CR_IRQ_ALL & ~XLLSDMA_CR_IRQ_EN));
+
+		rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
+		xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr | XLLSDMA_CR_IRQ_EN);
+	}
+
+	if (sdma->tx_irq != NO_IRQ) {
+		xllsdma_tx_out32(sdma, XLLSDMA_CR,
+			      tx_cr | (XLLSDMA_CR_IRQ_ALL & ~XLLSDMA_CR_IRQ_EN));
+		tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
+		xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr | XLLSDMA_CR_IRQ_EN);
+	}
+
+	spin_unlock_irqrestore(&sdma->lock, flags);
+
+	list_for_each_entry_safe(client, tmp, &sdma->clients, item)
+		if (likely(client->reset))
+			client->reset(client->data);
+
+	xllsdma_set_coalesce(sdma, &coal);
+}
+EXPORT_SYMBOL_GPL(xllsdma_reset);
+
+void xllsdma_tx_irq_enable(struct xllsdma_device *sdma)
+{
+	u32 tx_cr;
+	unsigned long flags;
+
+	BUG_ON(sdma->tx_irq == NO_IRQ);
+
+	spin_lock_irqsave(&sdma->lock, flags);
+	tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
+	xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr | XLLSDMA_CR_IRQ_EN);
+	spin_unlock_irqrestore(&sdma->lock, flags);
+}
+EXPORT_SYMBOL_GPL(xllsdma_tx_irq_enable);
+
+void xllsdma_rx_irq_enable(struct xllsdma_device *sdma)
+{
+	u32 rx_cr;
+	unsigned long flags;
+
+	BUG_ON(sdma->rx_irq == NO_IRQ);
+
+	spin_lock_irqsave(&sdma->lock, flags);
+	rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
+	xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr | XLLSDMA_CR_IRQ_EN);
+	spin_unlock_irqrestore(&sdma->lock, flags);
+}
+EXPORT_SYMBOL_GPL(xllsdma_rx_irq_enable);
+
+void xllsdma_tx_irq_disable(struct xllsdma_device *sdma)
+{
+	u32 tx_cr;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdma->lock, flags);
+	tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
+	xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr & ~XLLSDMA_CR_IRQ_EN);
+	spin_unlock_irqrestore(&sdma->lock, flags);
+}
+EXPORT_SYMBOL_GPL(xllsdma_tx_irq_disable);
+
+void xllsdma_rx_irq_disable(struct xllsdma_device *sdma)
+{
+	u32 rx_cr;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdma->lock, flags);
+	rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
+	xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr & ~XLLSDMA_CR_IRQ_EN);
+	spin_unlock_irqrestore(&sdma->lock, flags);
+}
+EXPORT_SYMBOL_GPL(xllsdma_rx_irq_disable);
+
+void xllsdma_tx_irq_ack(struct xllsdma_device *sdma)
+{
+	u32 irq_stat;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdma->lock, flags);
+	irq_stat = xllsdma_tx_in32(sdma, XLLSDMA_IRQ);
+	xllsdma_tx_out32(sdma, XLLSDMA_IRQ, irq_stat & XLLSDMA_IRQ_ALL_DONE);
+	spin_unlock_irqrestore(&sdma->lock, flags);
+}
+EXPORT_SYMBOL_GPL(xllsdma_tx_irq_ack);
+
+void xllsdma_rx_irq_ack(struct xllsdma_device *sdma)
+{
+	u32 irq_stat;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdma->lock, flags);
+	irq_stat = xllsdma_rx_in32(sdma, XLLSDMA_IRQ);
+	xllsdma_rx_out32(sdma, XLLSDMA_IRQ, irq_stat & XLLSDMA_IRQ_ALL_DONE);
+	spin_unlock_irqrestore(&sdma->lock, flags);
+}
+EXPORT_SYMBOL_GPL(xllsdma_rx_irq_ack);
+
+void xllsdma_pause(struct xllsdma_device *sdma)
+{
+	u32 dmacr;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdma->lock, flags);
+	dmacr = xllsdma_read_cr(sdma);
+	dmacr |= XLLSDMA_DMACR_TX_PAUSE | XLLSDMA_DMACR_RX_PAUSE;
+	xllsdma_write_cr(sdma, dmacr);
+	spin_unlock_irqrestore(&sdma->lock, flags);
+}
+EXPORT_SYMBOL_GPL(xllsdma_pause);
+
+void xllsdma_resume(struct xllsdma_device *sdma)
+{
+	u32 dmacr;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdma->lock, flags);
+	dmacr = xllsdma_read_cr(sdma);
+	dmacr &= ~(XLLSDMA_DMACR_TX_PAUSE | XLLSDMA_DMACR_RX_PAUSE);
+	xllsdma_write_cr(sdma, dmacr);
+	spin_unlock_irqrestore(&sdma->lock, flags);
+}
+EXPORT_SYMBOL_GPL(xllsdma_resume);
+
+int xllsdma_set_coalesce(struct xllsdma_device *sdma, struct xllsdma_coalesce *coal)
+{
+	u32 tx_cr, rx_cr;
+	unsigned long flags;
+
+	if (coal->tx_timeout > 255 ||
+	    coal->rx_timeout > 255 ||
+	    coal->tx_threshold > 255 ||
+	    coal->rx_threshold > 255)
+		return -EINVAL;
+
+	spin_lock_irqsave(&sdma->lock, flags);
+
+	if (sdma->rx_irq != NO_IRQ) {
+		rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
+
+		if (coal->rx_timeout == 0) {
+			coal->rx_timeout = 1;
+			rx_cr &= ~XLLSDMA_CR_IRQ_TIMEOUT;
+		} else {
+			rx_cr |= XLLSDMA_CR_IRQ_TIMEOUT;
+		}
+
+		rx_cr &= ~(XLLSDMA_CR_IRQ_THRESHOLD_MSK | XLLSDMA_CR_IRQ_TIMEOUT_SH);
+		rx_cr |= (coal->rx_threshold << XLLSDMA_CR_IRQ_THRESHOLD_SH)
+			 & XLLSDMA_CR_IRQ_THRESHOLD_MSK;
+		rx_cr |= (coal->rx_timeout << XLLSDMA_CR_IRQ_TIMEOUT_SH)
+			 & XLLSDMA_CR_IRQ_TIMEOUT_MSK;
+		rx_cr |= XLLSDMA_CR_LD_IRQ_CNT;
+
+		xllsdma_rx_out32(sdma, XLLSDMA_CR, rx_cr);
+	}
+
+	if (sdma->tx_irq != NO_IRQ) {
+		tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
+
+		if (coal->tx_timeout == 0) {
+			coal->tx_timeout = 1;
+			tx_cr &= ~XLLSDMA_CR_IRQ_TIMEOUT;
+		} else {
+			tx_cr |= XLLSDMA_CR_IRQ_TIMEOUT;
+		}
+
+		tx_cr &= ~(XLLSDMA_CR_IRQ_THRESHOLD_MSK | XLLSDMA_CR_IRQ_TIMEOUT_SH);
+		tx_cr |= (coal->tx_threshold << XLLSDMA_CR_IRQ_THRESHOLD_SH)
+			 & XLLSDMA_CR_IRQ_THRESHOLD_MSK;
+		tx_cr |= (coal->tx_timeout << XLLSDMA_CR_IRQ_TIMEOUT_SH)
+			 & XLLSDMA_CR_IRQ_TIMEOUT_MSK;
+		tx_cr |= XLLSDMA_CR_LD_IRQ_CNT;
+
+		xllsdma_tx_out32(sdma, XLLSDMA_CR, tx_cr);
+	}
+
+	spin_unlock_irqrestore(&sdma->lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xllsdma_set_coalesce);
+
+int xllsdma_get_coalesce(struct xllsdma_device *sdma, struct xllsdma_coalesce *coal)
+{
+	u32 tx_cr, rx_cr;
+	unsigned long flags;
+
+	spin_lock_irqsave(&sdma->lock, flags);
+
+	tx_cr = xllsdma_tx_in32(sdma, XLLSDMA_CR);
+	rx_cr = xllsdma_rx_in32(sdma, XLLSDMA_CR);
+
+	coal->tx_threshold = (tx_cr & XLLSDMA_CR_IRQ_THRESHOLD_MSK)
+			     >> XLLSDMA_CR_IRQ_THRESHOLD_SH;
+	coal->tx_timeout = (tx_cr & XLLSDMA_CR_IRQ_TIMEOUT_MSK)
+			   >> XLLSDMA_CR_IRQ_TIMEOUT_SH;
+
+	coal->rx_threshold = (rx_cr & XLLSDMA_CR_IRQ_THRESHOLD_MSK)
+			     >> XLLSDMA_CR_IRQ_THRESHOLD_SH;
+	coal->rx_timeout = (rx_cr & XLLSDMA_CR_IRQ_TIMEOUT_MSK)
+			     >> XLLSDMA_CR_IRQ_TIMEOUT_SH;
+
+	if (!(tx_cr & XLLSDMA_CR_IRQ_TIMEOUT))
+		coal->tx_timeout = 0;
+
+	if (!(rx_cr & XLLSDMA_CR_IRQ_TIMEOUT))
+		coal->rx_timeout = 0;
+
+	spin_unlock_irqrestore(&sdma->lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xllsdma_get_coalesce);
+
+int xllsdma_tx_submit(struct xllsdma_device *sdma, dma_addr_t desc)
+{
+	xllsdma_tx_out32(sdma, XLLSDMA_TDESCR, desc);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xllsdma_tx_submit);
+
+int xllsdma_rx_submit(struct xllsdma_device *sdma, dma_addr_t desc)
+{
+	xllsdma_rx_out32(sdma, XLLSDMA_TDESCR, desc);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(xllsdma_rx_submit);
+
+void xllsdma_tx_init(struct xllsdma_device *sdma, dma_addr_t desc)
+{
+	xllsdma_tx_out32(sdma, XLLSDMA_CDESCR, desc);
+}
+EXPORT_SYMBOL_GPL(xllsdma_tx_init);
+
+void xllsdma_rx_init(struct xllsdma_device *sdma, dma_addr_t desc)
+{
+	xllsdma_rx_out32(sdma, XLLSDMA_CDESCR, desc);
+}
+EXPORT_SYMBOL_GPL(xllsdma_rx_init);
+
+struct xllsdma_device *xllsdma_find_device(int phandle)
+{
+	struct mpmc_device *mpmc;
+	struct xllsdma_device *sdma = NULL;
+	int found = 0;
+	mutex_lock(&mpmc_devs_lock);
+	list_for_each_entry(mpmc, &mpmc_devs, item) {
+		mutex_lock(&mpmc->devs_lock);
+		list_for_each_entry(sdma, &mpmc->xllsdma_devs, item) {
+			if (sdma->phandle == phandle) {
+				found = 1;
+				break;
+			}
+		}
+		mutex_unlock(&mpmc->devs_lock);
+		if (found)
+			break;
+		else
+			sdma = NULL;
+	}
+	mutex_unlock(&mpmc_devs_lock);
+	return sdma;
+}
+EXPORT_SYMBOL_GPL(xllsdma_find_device);
+
+static irqreturn_t xllsdma_rx_intr(int irq, void *dev_id)
+{
+	u32 irq_ack, status;
+	struct xllsdma_device *sdma = dev_id;
+	struct xllsdma_client *client, *tmp;
+
+	/* Read pending interrupts */
+	status = xllsdma_rx_in32(sdma, XLLSDMA_IRQ);
+	irq_ack = status;
+	irq_ack &= sdma->rx_ack ? XLLSDMA_IRQ_ALL : XLLSDMA_IRQ_ALL_ERR;
+	xllsdma_rx_out32(sdma, XLLSDMA_IRQ, irq_ack);
+
+	if (unlikely(status & XLLSDMA_IRQ_ALL_ERR)) {
+		dev_err(sdma->dev, "%s: error status: %08x\n", __func__, 
+			status);
+		xllsdma_reset(sdma);
+		list_for_each_entry_safe(client, tmp, &sdma->clients, item)
+			if (likely(client->error))
+				client->error(client->data);
+		return IRQ_HANDLED;
+	}
+
+	if (likely(status & XLLSDMA_IRQ_ALL_DONE)) {
+		list_for_each_entry_safe(client, tmp, &sdma->clients, item)
+			if (likely(client->rx_complete))
+				client->rx_complete(client->data);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t xllsdma_tx_intr(int irq, void *dev_id)
+{
+	u32 irq_ack, status;
+	struct xllsdma_device *sdma = dev_id;
+	struct xllsdma_client *client, *tmp;
+
+	/* Read pending interrupts */
+	status = xllsdma_tx_in32(sdma, XLLSDMA_IRQ);
+	irq_ack = status;
+	irq_ack &= sdma->tx_ack ? XLLSDMA_IRQ_ALL : XLLSDMA_IRQ_ALL_ERR;
+	xllsdma_tx_out32(sdma, XLLSDMA_IRQ, irq_ack);
+
+	if (unlikely(status & XLLSDMA_IRQ_ALL_ERR)) {
+		dev_err(sdma->dev, "%s: error status: %08x\n", __func__, 
+			status);
+		xllsdma_reset(sdma);
+		list_for_each_entry_safe(client, tmp, &sdma->clients, item)
+			if (likely(client->error))
+				client->error(client->data);
+		return IRQ_HANDLED;
+	}
+
+	if (likely(status & XLLSDMA_IRQ_ALL_DONE)) {
+		list_for_each_entry_safe(client, tmp, &sdma->clients, item)
+			if (likely(client->tx_complete))
+				client->tx_complete(client->data);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void xllsdma_dev_register(struct mpmc_device *mpmc,
+			      struct xllsdma_device *sdma)
+{
+	mutex_lock(&mpmc->devs_lock);
+	list_add(&sdma->item, &mpmc->xllsdma_devs);
+	mutex_unlock(&mpmc->devs_lock);
+}
+
+static void xllsdma_dev_unregister(struct xllsdma_device *sdma)
+{
+	struct mpmc_device *mpmc = sdma->parent;
+
+	mutex_lock(&mpmc->devs_lock);
+	list_del(&sdma->item);
+	mutex_unlock(&mpmc->devs_lock);
+}
+
+static void xllsdma_cleanup(struct device *dev)
+{
+	struct xllsdma_device *sdma = dev_get_drvdata(dev);
+
+	if (sdma->tx_irq)
+		free_irq(sdma->tx_irq, sdma);
+
+	if (sdma->rx_irq)
+		free_irq(sdma->rx_irq, sdma);
+
+	if (sdma->memregion.start)
+		release_mem_region(sdma->memregion.start,
+			sdma->memregion.end - sdma->memregion.start + 1);
+
+	if (sdma->ioaddr)
+		iounmap(sdma->ioaddr);
+
+	xllsdma_dev_unregister(sdma);
+	kfree(sdma);
+	dev_set_drvdata(dev, NULL);
+}
+
+static void mpmc_dev_register(struct mpmc_device *mpmc)
+{
+	mutex_lock(&mpmc_devs_lock);
+	list_add_tail(&mpmc->item, &mpmc_devs);
+	mutex_unlock(&mpmc_devs_lock);
+}
+
+static void mpmc_dev_unregister(struct mpmc_device *mpmc)
+{
+	mutex_lock(&mpmc_devs_lock);
+	list_del(&mpmc->item);
+	mutex_unlock(&mpmc_devs_lock);
+}
+
+static void mpmc_cleanup(struct device *dev)
+{
+	struct mpmc_device *mpmc = dev_get_drvdata(dev);
+
+	if (mpmc->registered)
+		mpmc_dev_unregister(mpmc);
+
+	kfree(mpmc);
+	dev_set_drvdata(dev, NULL);
+}
+
+static int __devinit xllsdma_init(struct device *dev, struct resource *rx_irq,
+			       struct resource *tx_irq, struct resource *mem,
+			       int phandle)
+{
+	struct xllsdma_device *sdma;
+	struct mpmc_device *mpmc;
+
+	resource_size_t region_size;
+	int res;
+
+	mpmc = dev_get_drvdata(dev->parent);
+
+	sdma = kzalloc(sizeof(struct xllsdma_device), GFP_KERNEL);
+	if (!sdma) {
+		dev_err(dev, "Cannot allocate SDMA device\n");
+		return -ENOMEM;
+	}
+	dev_set_drvdata(dev, sdma);
+	sdma->dev = dev;
+
+	spin_lock_init(&sdma->lock);
+	INIT_LIST_HEAD(&sdma->clients);
+	mutex_init(&sdma->clients_lock);
+	sdma->parent = mpmc;
+	sdma->phandle = phandle;
+
+	region_size = mem->end - mem->start + 1;
+	if (!request_mem_region(mem->start, region_size, DRV_NAME)) {
+		dev_err(dev, "I/O memory region at %p is busy\n",
+			(void *)mem->start);
+		return -EBUSY;
+	}
+	sdma->memregion = *mem;
+
+	sdma->ioaddr = ioremap(mem->start, region_size);
+	if (!sdma->ioaddr) {
+		dev_err(dev, "Cannot ioremap() I/O memory %p\n",
+			(void *)mem->start);
+		return -ENOMEM;
+	}
+
+	xllsdma_reset(sdma);
+
+	sdma->rx_irq = NO_IRQ;
+	if (rx_irq) {
+		res = request_irq(rx_irq->start, xllsdma_rx_intr,
+				IRQF_SHARED, "SDMA RX", sdma);
+		if (res) {
+			dev_err(dev, "Could not allocate RX interrupt %d.\n",
+				rx_irq->start);
+			return res;
+		}
+		sdma->rx_irq = rx_irq->start;
+	}
+
+	sdma->tx_irq = NO_IRQ;
+	if (tx_irq) {
+		res = request_irq(tx_irq->start, xllsdma_tx_intr,
+				IRQF_SHARED, "SDMA TX", sdma);
+		if (res) {
+			dev_err(dev, "Could not allocate TX interrupt %d.\n",
+				tx_irq->start);
+			return res;
+		}
+		sdma->tx_irq = tx_irq->start;
+	}
+
+	sdma->rx_ack = 1;
+	sdma->tx_ack = 1;
+	xllsdma_dev_register(mpmc, sdma);
+
+	return 0;
+}
+
+static int __devinit mpmc_init(struct device *dev)
+{
+	struct mpmc_device *mpmc;
+
+	mpmc = kzalloc(sizeof(struct mpmc_device), GFP_KERNEL);
+
+	if (!mpmc) {
+		dev_err(dev, "Cannot allocate MPMC device\n");
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(dev, mpmc);
+
+	INIT_LIST_HEAD(&mpmc->xllsdma_devs);
+	mutex_init(&mpmc->devs_lock);
+
+	mpmc_dev_register(mpmc);
+	mpmc->registered = 1;
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static int xllsdma_of_remove(struct of_device *op)
+{
+	xllsdma_cleanup(&op->dev);
+	return 0;
+}
+
+/* Match table for of_platform binding */
+static struct of_device_id xllsdma_of_match[] = {
+	{ .compatible = "xlnx,ll-dma-1.00.a" },
+	{},
+};
+
+static int __devinit xllsdma_of_probe(struct of_device *op,
+				   const struct of_device_id *match)
+{
+	const int *prop;
+	int phandle;
+	struct resource rx_irq, tx_irq, mem;
+	struct resource *tx_irq_res = NULL;
+	struct resource *rx_irq_res = NULL;
+	int res;
+
+	res = of_address_to_resource(op->node, 0, &mem);
+	if (res) {
+		dev_err(&op->dev, "invalid address\n");
+		return res;
+	}
+
+	/* IRQ */
+	res = of_irq_to_resource(op->node, 0, &rx_irq);
+	if (res != NO_IRQ)
+		rx_irq_res = &rx_irq;
+
+	res = of_irq_to_resource(op->node, 1, &tx_irq);
+	if (res != NO_IRQ)
+		tx_irq_res = &tx_irq;
+
+	prop = of_get_property(op->node, "linux,phandle", NULL);
+	phandle = (prop) ? *prop : -1;
+
+	res = xllsdma_init(&op->dev, rx_irq_res, tx_irq_res, &mem, phandle);
+	if (res)
+		xllsdma_of_remove(op);
+
+	return res;
+}
+
+static struct of_platform_driver xllsdma_of_driver = {
+	.name		= "xilinx-sdma",
+	.match_table	= xllsdma_of_match,
+	.probe		= xllsdma_of_probe,
+	.remove		= xllsdma_of_remove,
+};
+
+int __init xllsdma_of_init(void)
+{
+	int ret;
+
+	ret = of_register_platform_driver(&xllsdma_of_driver);
+	if (ret) {
+		of_unregister_platform_driver(&xllsdma_of_driver);
+		printk(KERN_ERR "registering driver failed: err=%i", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+void xllsdma_of_exit(void)
+{
+	of_unregister_platform_driver(&xllsdma_of_driver);
+}
+
+static int mpmc_of_remove(struct of_device *op)
+{
+	struct device_node *node;
+	struct of_device *ofdev;
+
+	for_each_child_of_node(op->node, node) {
+		ofdev = of_find_device_by_node(node);
+		of_device_unregister(ofdev);
+		of_device_free(ofdev);
+	}
+
+	mpmc_cleanup(&op->dev);
+	return 0;
+}
+
+static int __devinit mpmc_of_probe(struct of_device *op,
+			const struct of_device_id *match)
+{
+	int err = mpmc_init(&op->dev);
+	if (err)
+		return err;
+
+	of_platform_bus_probe(op->node, xllsdma_of_match, &op->dev);
+	return 0;
+}
+
+static struct of_device_id  __devinitdata mpmc_of_match[] = {
+	{ .compatible = "xlnx,mpmc-4.01.a" },
+	{ .compatible = "xlnx,mpmc-4.03.a" },
+	{},
+};
+
+static struct of_platform_driver mpmc_of_driver = {
+	.name = "xilinx-mpmc",
+	.match_table = mpmc_of_match,
+	.probe = mpmc_of_probe,
+	.remove	= mpmc_of_remove,
+};
+
+int __init mpmc_of_init(void)
+{
+	return of_register_platform_driver(&mpmc_of_driver);
+}
+
+void mpmc_of_exit(void)
+{
+	of_unregister_platform_driver(&mpmc_of_driver);
+}
+
+subsys_initcall(mpmc_of_init);
+subsys_initcall(xllsdma_of_init);
+#else	/* CONFIG_OF */
+/*---------------------------------------------------------------------------
+ * Platform bus attachment
+ */
+
+static __devexit int xllsdma_plat_remove(struct platform_device *pdev)
+{
+	xllsdma_cleanup(&pdev->dev);
+	return 0;
+}
+
+static int __devinit xllsdma_plat_probe(struct platform_device *pdev)
+{
+	struct resource *rx_irq, *tx_irq, *mem;
+	int err = 0;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem) {
+		dev_err(&pdev->dev, "invalid address\n");
+		err = -EINVAL;
+		goto fail;
+	}
+
+	/* RX interrupt is optional, and first */
+	rx_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+
+	/* TX interrupt is optional, and second */
+	tx_irq = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
+
+	err = xllsdma_init(&pdev->dev, rx_irq, tx_irq, mem, pdev->id);
+	if (err)
+		xllsdma_plat_remove(pdev);
+fail:
+	return err;
+}
+
+static struct platform_driver xllsdma_plat_driver = {
+	.probe = xllsdma_plat_probe,
+	.remove	= __devexit_p(xllsdma_plat_remove),
+	.driver = {
+		.owner = THIS_MODULE,
+		.name  = "xilinx-sdma",
+	},
+};
+
+int __init xllsdma_plat_init(void)
+{
+	int err = platform_driver_register(&xllsdma_plat_driver);
+	if (err) {
+		platform_driver_unregister(&xllsdma_plat_driver);
+		printk(KERN_ERR "registering driver failed: err=%i", err);
+		return err;
+	}
+
+	return 0;
+}
+subsys_initcall(xllsdma_plat_init);
+
+void xllsdma_plat_exit(void)
+{
+	platform_driver_unregister(&xllsdma_plat_driver);
+}
+
+static int mpmc_plat_probe(struct platform_device *pdev)
+{
+	return mpmc_init(&pdev->dev);
+}
+
+static int __devexit mpmc_plat_remove(struct platform_device *pdev)
+{
+	mpmc_cleanup(&pdev->dev);
+	return 0;
+}
+
+static struct platform_driver mpmc_plat_driver = {
+	.probe = mpmc_plat_probe,
+	.remove	= __devexit_p(mpmc_plat_remove),
+	.driver = {
+		.owner = THIS_MODULE,
+		.name  = "xilinx-mpmc",
+	},
+};
+
+int __init mpmc_plat_init(void)
+{
+	return platform_driver_register(&mpmc_plat_driver);
+}
+subsys_initcall(mpmc_plat_init);
+
+void mpmc_plat_exit(void)
+{
+	platform_driver_unregister(&mpmc_plat_driver);
+}
+#endif	/* CONFIG_OF */
diff -r baced9e29ab5 include/linux/xllsdma.h
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/include/linux/xllsdma.h	Wed Apr 28 02:00:51 2010 +0400
@@ -0,0 +1,187 @@
+/*
+ * SDMA subsystem support for Xilinx MPMC.
+ *
+ * Author: Sergey Temerkhanov
+ *
+ * Copyright (c) 2008-2010 Cifronic ZAO
+ *
+ * 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.
+ *
+ */
+
+#ifndef __XLLSDMA_H__
+#define __XLLSDMA_H__
+
+#include <linux/types.h>
+#include <linux/dma-mapping.h>
+
+#define XLLSDMA_ALIGNMENT	0x40
+
+struct xllsdma_desc {
+	__be32 next;
+	__be32 address;
+	__be32 length;
+	__be32 stat_ctl;
+	__be32 app1;
+	__be32 app2;
+	__be32 app3;
+	__be32 app4;
+	void *virt;
+	u32 flags;
+} __attribute__((aligned(XLLSDMA_ALIGNMENT)));
+
+
+enum {
+	XLLSDMA_STSCTL_ERROR	= (1 << 31), /* DMA error */
+	XLLSDMA_STSCTL_IOE		= (1 << 30), /* Interrupt on end */
+	XLLSDMA_STSCTL_SOE		= (1 << 29), /* Stop on end */
+	XLLSDMA_STSCTL_DONE	= (1 << 28), /* DMA completed */
+	XLLSDMA_STSCTL_SOP		= (1 << 27), /* Start of packet */
+	XLLSDMA_STSCTL_EOP		= (1 << 26), /* End of packet */
+	XLLSDMA_STSCTL_BUSY	= (1 << 25), /* DMA busy */
+	XLLSDMA_STSCTL_CSUM	= (1 << 0),  /* Checksum enable */
+
+	XLLSDMA_STSCTL_MSK		= (0xFF << 24), /*Status/control field */
+};
+
+/* SDMA client operations */
+struct xllsdma_client {
+	void *data;
+	void (*tx_complete) (void *data);
+	void (*rx_complete) (void *data);
+	void (*error) (void *data);
+	void (*reset) (void *data);
+	struct list_head item;
+};
+
+struct xllsdma_coalesce {
+	int tx_threshold;
+	int tx_timeout;
+
+	int rx_threshold;
+	int rx_timeout;
+};
+
+#define DEFINE_XLLSDMA_COALESCE(x) struct xllsdma_coalesce x = { \
+	.tx_timeout	= 0, \
+	.tx_threshold	= 1, \
+	.rx_timeout	= 0, \
+	.rx_threshold	= 1, };
+
+struct mpmc_device {
+	void __iomem		*ioaddr;
+
+	struct resource		memregion;
+	int			irq;
+
+	int			registered;
+	struct list_head	item;
+
+	struct mutex		devs_lock;
+	struct list_head	xllsdma_devs;
+};
+
+struct xllsdma_device {
+	struct device		*dev;
+	void __iomem		*ioaddr;
+	wait_queue_head_t 	wait;
+
+	spinlock_t		lock;
+
+	struct resource		memregion;
+	int			rx_irq;
+	int			tx_irq;
+	int			rx_ack;
+	int			tx_ack;
+	int			phandle;
+
+	int			registered;
+	struct mpmc_device	*parent;
+
+	struct xllsdma_coalesce	coal;
+	struct list_head	item;
+
+	struct mutex		clients_lock;
+	struct list_head	clients;
+};
+
+static inline void xllsdma_add_client(struct xllsdma_device *sdma,
+				   struct xllsdma_client *client)
+{
+	mutex_lock(&sdma->clients_lock);
+	list_add(&client->item, &sdma->clients);
+	mutex_unlock(&sdma->clients_lock);
+}
+
+static inline void xllsdma_del_client(struct xllsdma_device *sdma,
+				   struct xllsdma_client *client)
+{
+	mutex_lock(&sdma->clients_lock);
+	list_del(&client->item);
+	mutex_unlock(&sdma->clients_lock);
+}
+
+struct xllsdma_device *xllsdma_find_device(int phandle);
+void xllsdma_pause(struct xllsdma_device *sdma);
+void xllsdma_resume(struct xllsdma_device *sdma);
+void xllsdma_reset(struct xllsdma_device *sdma);
+void xllsdma_rx_init(struct xllsdma_device *sdma, dma_addr_t desc);
+void xllsdma_tx_init(struct xllsdma_device *sdma, dma_addr_t desc);
+
+int xllsdma_tx_submit(struct xllsdma_device *sdma, dma_addr_t desc);
+int xllsdma_rx_submit(struct xllsdma_device *sdma, dma_addr_t desc);
+
+void xllsdma_tx_irq_enable(struct xllsdma_device *sdma);
+void xllsdma_rx_irq_enable(struct xllsdma_device *sdma);
+void xllsdma_tx_irq_disable(struct xllsdma_device *sdma);
+void xllsdma_rx_irq_disable(struct xllsdma_device *sdma);
+void xllsdma_tx_irq_ack(struct xllsdma_device *sdma);
+void xllsdma_rx_irq_ack(struct xllsdma_device *sdma);
+
+int xllsdma_set_coalesce(struct xllsdma_device *sdma, struct xllsdma_coalesce *coal);
+int xllsdma_get_coalesce(struct xllsdma_device *sdma, struct xllsdma_coalesce *coal);
+
+static inline int xllsdma_has_tx_irq(struct xllsdma_device *sdma)
+{
+	return sdma->tx_irq != NO_IRQ;
+}
+
+static inline int xllsdma_has_rx_irq(struct xllsdma_device *sdma)
+{
+	return sdma->rx_irq != NO_IRQ;
+}
+
+static inline int xllsdma_desc_busy(struct xllsdma_desc *desc)
+{
+	return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_BUSY);
+}
+
+static inline int xllsdma_desc_done(struct xllsdma_desc *desc)
+{
+	return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_DONE);
+}
+
+static inline int xllsdma_desc_sop(struct xllsdma_desc *desc)
+{
+	return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_SOP);
+}
+
+static inline int xllsdma_desc_eop(struct xllsdma_desc *desc)
+{
+	return desc->stat_ctl & __constant_be32_to_cpu(XLLSDMA_STSCTL_EOP);
+}
+
+static inline void xllsdma_set_ack(struct xllsdma_device *sdma, int rx_ack,
+				int tx_ack)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&sdma->lock, flags);
+	sdma->rx_ack = rx_ack;
+	sdma->tx_ack = tx_ack;
+	spin_unlock_irqrestore(&sdma->lock, flags);
+}
+
+#endif

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox