RE: [PATCH 0/5 v3] Porting RapidIO driver from ppc to powerpc architecture and adding memory mapped RapidIO driver.
Yes, I'm working on it. Do not worry about it. Wei. > -Original Message- > From: Kumar Gala [mailto:[EMAIL PROTECTED] > Sent: Tuesday, October 30, 2007 4:31 AM > To: [EMAIL PROTECTED] > Cc: Zhang Wei-r63237; [EMAIL PROTECTED]; > [EMAIL PROTECTED]; [EMAIL PROTECTED]; > linux-kernel@vger.kernel.org > Subject: Re: [PATCH 0/5 v3] Porting RapidIO driver from ppc > to powerpc architecture and adding memory mapped RapidIO driver. > > > On Oct 29, 2007, at 2:38 PM, Phil Terry wrote: > > > Can anyone tell me what the status of these are? What kernel > > release are > > they targetted for? Currently I'm trying to work with 2.6.23 plus > > these > > patches locally. > > hopefully 2.6.25. I'd like to get the documentation updates in > 2.6.24 if we have agreement on them. > > - k > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 0/5 v3] Porting RapidIO driver from ppc to powerpc architecture and adding memory mapped RapidIO driver.
Yes, I'm working on it. Do not worry about it. Wei. -Original Message- From: Kumar Gala [mailto:[EMAIL PROTECTED] Sent: Tuesday, October 30, 2007 4:31 AM To: [EMAIL PROTECTED] Cc: Zhang Wei-r63237; [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; linux-kernel@vger.kernel.org Subject: Re: [PATCH 0/5 v3] Porting RapidIO driver from ppc to powerpc architecture and adding memory mapped RapidIO driver. On Oct 29, 2007, at 2:38 PM, Phil Terry wrote: Can anyone tell me what the status of these are? What kernel release are they targetted for? Currently I'm trying to work with 2.6.23 plus these patches locally. hopefully 2.6.25. I'd like to get the documentation updates in 2.6.24 if we have agreement on them. - k - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xxprocessors.
Hi,
> > +static void fsl_dma_set_src(dma_addr_t addr,
> > + struct dma_async_tx_descriptor
> *tx, int index)
> > +{
>
> What is index supposed to mean? It's not used, or documented
> anywhere than
> I can see.
I've also got more document here. Hi, Dan, could you give me some
explanation about this API? :)
>
> > + else {
> > + /* Run the link descriptor callback function */
> > + if (desc->async_tx.callback) {
> > +
> spin_unlock_irqrestore(_chan->desc_lock,
> > + flags);
> > + dev_dbg(fsl_chan->device->dev,
> > + "link descriptor %p
> callback\n", desc);
> > + desc->async_tx.callback(
> > +
> desc->async_tx.callback_param);
> > +
> spin_lock_irqsave(_chan->desc_lock, flags);
>
> After dropping the lock, you can no longer assume that your
> iterator is
> still valid; you need to work off of the list head.
>
list_for_each_entry_safe() is used here. I think the safe should be ok.
:P
> > + /* Find the first un-transfer desciptor */
> > + for (ld_node = fsl_chan->ld_queue.next;
> > + (ld_node != _chan->ld_queue)
> > + && (DMA_SUCCESS == dma_async_is_complete(
> > +
> to_fsl_desc(ld_node)->async_tx.cookie,
> > + fsl_chan->completed_cookie,
> > + fsl_chan->common.cookie));
> > + ld_node = ld_node->next);
>
> Call fsl_dma_is_complete directly, don't waste time going through the
> virtual call.
>
> And you have a recursive lock usage here; fsl_dma_is_complete calls
> fsl_chan_ld_cleanup, which acquires desc_lock, but you
> already have it.
>
> Couldn't you just call fsl_chan_ld_cleanup, and then check
> what's at the
> head of the list?
>
I'll split interrupt and poll here.
> > +static irqreturn_t fsl_dma_do_interrupt(int irq, void *data)
> > +{
> > + struct fsl_dma_device *fdev = (struct fsl_dma_device *)data;
> > + struct fsl_dma_chan *fsl_chan = NULL;
> > + u32 gsr;
> > + int ch_nr;
> > + struct dma_chan *int_chan;
> > +
> > + gsr = (fdev->feature & FSL_DMA_BIG_ENDIAN) ?
> in_be32(fdev->reg_base)
> > + : in_le32(fdev->reg_base);
> > + ch_nr = (32 - ffs(gsr)) / 8;
> > +
> > + list_for_each_entry(int_chan, >common.channels,
> device_node)
> > + if (to_fsl_chan(int_chan)->id == ch_nr)
> > + fsl_chan = to_fsl_chan(int_chan);
>
> Why not use an array of channels?
The list is used in dma engine core file. And it's possible that there
are not all channel listed in dts and array.
> > +
> > + return fsl_chan ? fsl_dma_chan_do_interrupt(irq,
> fsl_chan) : IRQ_NONE;
> > +
> > +}
> > +
> > +static void dma_do_tasklet(unsigned long unused)
> > +{
> > + struct fsl_desc_sw *desc, *_desc;
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(_ln_lock, flags);
> > + list_for_each_entry_safe(desc, _desc, _ln_chain, node) {
> > + struct fsl_dma_chan *fsl_chan =
> > +
> to_fsl_chan(desc->async_tx.chan);
> > + /* Run the link descriptor callback function */
> > + if (desc->async_tx.callback) {
> > + spin_unlock_irqrestore(_ln_lock, flags);
> > + dev_dbg(fsl_chan->device->dev,
> > + "dma_tasklet: link descriptor
> %p callback\n",
> > + desc);
> > + desc->async_tx.callback(
> > + desc->async_tx.callback_param);
> > + spin_lock_irqsave(_ln_lock, flags);
> > + }
> > + /* Recycle it! */
> > + list_del(>node);
>
> You should remove it from the list before dropping the lock,
> as otherwise
> something else could come along and remove it again.
All right!
>
> > + if (strcmp(match->compatible, "fsl,mpc8540-dma-channel") == 0)
> > + new_fsl_chan->feature = FSL_DMA_IP_86XX |
> FSL_DMA_BIG_ENDIAN;
>
> Shouldn't it be 85XX, to be consistent?
>
> > + else if (strcmp(match->compatible,
> "fsl,mpc8349-dma-channel") == 0)
> > + new_fsl_chan->feature = FSL_DMA_IP_83XX |
> FSL_DMA_LITTLE_ENDIAN;
>
> You could have the features be part of the match struct, so
> you don't have
> to do extra strcmps.
>
Can I use the data field of struct of_device_id?
>
> > +static struct of_device_id of_fsl_dma_ids[] = {
> > + { .compatible = "fsl,dma", },
> > +};
>
> Why do we need to bind to the parent node at all?
Yes, the MPC83xx should get interrupt source from DMA device register.
>
> > +/* There is no asm instructions for 64 bits reverse loads
> and stores */
> > +static u64 in_le64(const u64 __iomem *addr)
> > +{
>
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xxprocessors.
Hi,
+static void fsl_dma_set_src(dma_addr_t addr,
+ struct dma_async_tx_descriptor
*tx, int index)
+{
What is index supposed to mean? It's not used, or documented
anywhere than
I can see.
I've also got more document here. Hi, Dan, could you give me some
explanation about this API? :)
+ else {
+ /* Run the link descriptor callback function */
+ if (desc-async_tx.callback) {
+
spin_unlock_irqrestore(fsl_chan-desc_lock,
+ flags);
+ dev_dbg(fsl_chan-device-dev,
+ link descriptor %p
callback\n, desc);
+ desc-async_tx.callback(
+
desc-async_tx.callback_param);
+
spin_lock_irqsave(fsl_chan-desc_lock, flags);
After dropping the lock, you can no longer assume that your
iterator is
still valid; you need to work off of the list head.
list_for_each_entry_safe() is used here. I think the safe should be ok.
:P
+ /* Find the first un-transfer desciptor */
+ for (ld_node = fsl_chan-ld_queue.next;
+ (ld_node != fsl_chan-ld_queue)
+(DMA_SUCCESS == dma_async_is_complete(
+
to_fsl_desc(ld_node)-async_tx.cookie,
+ fsl_chan-completed_cookie,
+ fsl_chan-common.cookie));
+ ld_node = ld_node-next);
Call fsl_dma_is_complete directly, don't waste time going through the
virtual call.
And you have a recursive lock usage here; fsl_dma_is_complete calls
fsl_chan_ld_cleanup, which acquires desc_lock, but you
already have it.
Couldn't you just call fsl_chan_ld_cleanup, and then check
what's at the
head of the list?
I'll split interrupt and poll here.
+static irqreturn_t fsl_dma_do_interrupt(int irq, void *data)
+{
+ struct fsl_dma_device *fdev = (struct fsl_dma_device *)data;
+ struct fsl_dma_chan *fsl_chan = NULL;
+ u32 gsr;
+ int ch_nr;
+ struct dma_chan *int_chan;
+
+ gsr = (fdev-feature FSL_DMA_BIG_ENDIAN) ?
in_be32(fdev-reg_base)
+ : in_le32(fdev-reg_base);
+ ch_nr = (32 - ffs(gsr)) / 8;
+
+ list_for_each_entry(int_chan, fdev-common.channels,
device_node)
+ if (to_fsl_chan(int_chan)-id == ch_nr)
+ fsl_chan = to_fsl_chan(int_chan);
Why not use an array of channels?
The list is used in dma engine core file. And it's possible that there
are not all channel listed in dts and array.
+
+ return fsl_chan ? fsl_dma_chan_do_interrupt(irq,
fsl_chan) : IRQ_NONE;
+
+}
+
+static void dma_do_tasklet(unsigned long unused)
+{
+ struct fsl_desc_sw *desc, *_desc;
+ unsigned long flags;
+
+ spin_lock_irqsave(recy_ln_lock, flags);
+ list_for_each_entry_safe(desc, _desc, recy_ln_chain, node) {
+ struct fsl_dma_chan *fsl_chan =
+
to_fsl_chan(desc-async_tx.chan);
+ /* Run the link descriptor callback function */
+ if (desc-async_tx.callback) {
+ spin_unlock_irqrestore(recy_ln_lock, flags);
+ dev_dbg(fsl_chan-device-dev,
+ dma_tasklet: link descriptor
%p callback\n,
+ desc);
+ desc-async_tx.callback(
+ desc-async_tx.callback_param);
+ spin_lock_irqsave(recy_ln_lock, flags);
+ }
+ /* Recycle it! */
+ list_del(desc-node);
You should remove it from the list before dropping the lock,
as otherwise
something else could come along and remove it again.
All right!
+ if (strcmp(match-compatible, fsl,mpc8540-dma-channel) == 0)
+ new_fsl_chan-feature = FSL_DMA_IP_86XX |
FSL_DMA_BIG_ENDIAN;
Shouldn't it be 85XX, to be consistent?
+ else if (strcmp(match-compatible,
fsl,mpc8349-dma-channel) == 0)
+ new_fsl_chan-feature = FSL_DMA_IP_83XX |
FSL_DMA_LITTLE_ENDIAN;
You could have the features be part of the match struct, so
you don't have
to do extra strcmps.
Can I use the data field of struct of_device_id?
+static struct of_device_id of_fsl_dma_ids[] = {
+ { .compatible = fsl,dma, },
+};
Why do we need to bind to the parent node at all?
Yes, the MPC83xx should get interrupt source from DMA device register.
+/* There is no asm instructions for 64 bits reverse loads
and stores */
+static u64 in_le64(const u64 __iomem *addr)
+{
+ return le64_to_cpu(in_be64(addr));
+}
+
+static void out_le64(u64 __iomem *addr, u64 val)
+{
+ out_be64(addr, cpu_to_le64(val));
+}
+#endif
You can use asm instructions for this, as
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
Hi, Dan,
Does I have followed your new API? :-)
> > ---
> Greetings,
>
> Please copy me on any updates to this driver, drivers/dma, or
> crypto/async_tx.
Ok.
>
> Below are a few review comments...
>
> Regards,
> Dan
>
> > +/**
> > + * fsl_dma_alloc_descriptor - Allocate descriptor from
> channel's DMA pool.
> > + *
> > + * Return - The descriptor allocated. NULL for failed.
> > + */
> > +static struct fsl_desc_sw *fsl_dma_alloc_descriptor(
> > + struct fsl_dma_chan
> *fsl_chan,
> > + gfp_t flags)
> > +{
> > + dma_addr_t pdesc;
> > + struct fsl_desc_sw *desc_sw;
> > +
> > + desc_sw = dma_pool_alloc(fsl_chan->desc_pool,
> flags, );
> > + if (desc_sw) {
> > + memset(desc_sw, 0, sizeof(struct fsl_desc_sw));
> > + dma_async_tx_descriptor_init(_sw->async_tx,
> > + _chan->common);
> > + desc_sw->async_tx.tx_set_src = fsl_dma_set_src;
> > + desc_sw->async_tx.tx_set_dest = fsl_dma_set_dest;
> > + desc_sw->async_tx.tx_submit = fsl_dma_tx_submit;
> > + INIT_LIST_HEAD(_sw->async_tx.tx_list);
> > + desc_sw->async_tx.phys = pdesc;
> > + }
> > +
> > + return desc_sw;
> > +}
>
> I suggest pre-allocating the descriptors:
> 1/ It alleviates the need to initialize these async_tx fields
> which never change
The dma pool has already stored the descriptors in it's list,
> 2/ The GFP_ATOMIC allocation can be removed.
>
Ok.
> iop-adma gets by with one PAGE_SIZE buffer (128 descriptors).
>
> [..]
> > +static irqreturn_t fsl_dma_chan_do_interrupt(int irq, void *data)
> > +{
> > + struct fsl_dma_chan *fsl_chan = (struct fsl_dma_chan *)data;
> > + dma_addr_t stat;
> > +
> > + stat = get_sr(fsl_chan);
> > + dev_dbg(fsl_chan->device->dev, "event: channel %d,
> stat = 0x%x\n",
> > + fsl_chan->id, stat);
> > + set_sr(fsl_chan, stat); /* Clear the event
> register */
> > +
> > + stat &= ~(FSL_DMA_SR_CB | FSL_DMA_SR_CH);
> > + if (!stat)
> > + return IRQ_NONE;
> > +
> > + /* If the link descriptor segment transfer finishes,
> > +* we will recycle the used descriptor.
> > +*/
> > + if (stat & FSL_DMA_SR_EOSI) {
> > + dev_dbg(fsl_chan->device->dev, "event:
> End-of-segments INT\n");
> > + dev_dbg(fsl_chan->device->dev, "event:
> clndar 0x%016llx, "
> > + "nlndar 0x%016llx\n",
> (u64)get_cdar(fsl_chan),
> > + (u64)get_ndar(fsl_chan));
> > + stat &= ~FSL_DMA_SR_EOSI;
> > + fsl_chan_ld_cleanup(fsl_chan, 1);
> > + }
> > +
> > + /* If it current transfer is the end-of-transfer,
> > +* we should clear the Channel Start bit for
> > +* prepare next transfer.
> > +*/
> > + if (stat & (FSL_DMA_SR_EOLNI | FSL_DMA_SR_EOCDI)) {
> > + dev_dbg(fsl_chan->device->dev, "event:
> End-of-link INT\n");
> > + stat &= ~FSL_DMA_SR_EOLNI;
> > + fsl_chan_xfer_ld_queue(fsl_chan);
> > + }
> > +
> > + if (stat)
> > + dev_dbg(fsl_chan->device->dev, "event:
> unhandled sr 0x%02x\n",
> > + stat);
> > +
> > + dev_dbg(fsl_chan->device->dev, "event: Exit\n");
> > + tasklet_hi_schedule(_tasklet);
> > + return IRQ_HANDLED;
> > +}
>
> This driver implements locking and callbacks inconsistently with how
> it is done in ioatdma and iop-adma. The big changes are that all
> locks have been upgraded from 'spin_lock_bh' to 'spin_lock_irqsave',
> and async_tx callbacks can happen in irq context. I would like to
> keep everything in bottom-half context to lessen the restrictions on
> what async_tx clients can perform in their callback routines. What
> are the implications of moving 'fsl_chan_ld_cleanup' to the tasklet
> and changing the 'tasklet_hi_schedule' to 'tasklet_schedule'?
A good suggestion :), I need some investigation here.
>
> [..]
> > +static struct dma_chan
> *of_find_dma_chan_by_phandle(phandle phandle)
> > +{
> > + struct device_node *np;
> > + struct dma_chan *chan;
> > + struct fsl_dma_device *fdev;
> > +
> > + np = of_find_node_by_phandle(phandle);
> > + if (!np || !of_device_is_compatible(np->parent, "fsl,dma"))
> > + return NULL;
> > +
> > + fdev =
> dev_get_drvdata(_find_device_by_node(np->parent)->dev);
> > +
> > + list_for_each_entry(chan, >common.channels,
> device_node)
> > + if
> (to_of_device(to_fsl_chan(chan)->chan_dev)->node == np)
> > + return chan;
> > + return NULL;
> > +}
> > +EXPORT_SYMBOL(of_find_dma_chan_by_phandle);
>
> This routine implies that there
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
> > If this is experimental, perhaps you should mark the depends line as > such > depends on on DMA_ENGINE && PPC && EXPERIMENTAL I'll add EXPERIMENTAL for MPC83xx only. > > [...] > > >+ > >+fsl_dma_memcpy_issue_pending(chan); > >+while (fsl_dma_is_complete(chan, cookie, NULL, NULL) > >+!= DMA_SUCCESS); > > Again, is it possible to hang your thread here? > > [...] I'll add msleep here. Thanks! - zw - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
Hi,
> --- /dev/null
> > +++ b/drivers/dma/fsldma.c
> > @@ -0,0 +1,995 @@
>
> Thanks for using kernel-doc notation. However, ...
>
> > +/**
> > + * fsl_dma_alloc_descriptor - Allocate descriptor from
> channel's DMA pool.
>
> Function parameters need to be listed & described here.
> See Documentation/kernel-doc-nano-HOWTO.txt or other source files
> for examples.
>
> (Applies to all documented function interfaces here.)
All right, I'll add full descriptions here. :P
>
> > + *
> > + * Return - The descriptor allocated. NULL for failed.
> > + */
> > +static struct fsl_desc_sw *fsl_dma_alloc_descriptor(
> > + struct fsl_dma_chan *fsl_chan,
> > + gfp_t flags)
> > +{
> ...
> > +}
>
> > +/**
> > + * fsl_chan_xfer_ld_queue -- Transfer the link descriptors
> in channel
> > + * ld_queue.
>
> The function's "short description" (unfortunately) must be on only one
> line. E.g.:
>
> * fsl_chan_xfer_ld_queue - Transfer link descriptors in
> channel ld_queue.
>
How about it's length greater than 80?
> > + */
> > +static void fsl_chan_xfer_ld_queue(struct fsl_dma_chan *fsl_chan)
> > +{
> ...
> > +}
>
> > diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
> > new file mode 100644
> > index 000..05be9ed
> > --- /dev/null
> > +++ b/drivers/dma/fsldma.h
> > @@ -0,0 +1,188 @@
> > +struct fsl_dma_chan_regs {
> > + __mix32 mr; /* 0x00 - Mode Register */
> > + __mix32 sr; /* 0x04 - Status Register */
> > + __mix64 cdar; /* 0x08 - Cureent descriptor
> address register */
>
> Current
>
I'll fix it.
Thanks!
- zw
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
Hi,
--- /dev/null
+++ b/drivers/dma/fsldma.c
@@ -0,0 +1,995 @@
Thanks for using kernel-doc notation. However, ...
+/**
+ * fsl_dma_alloc_descriptor - Allocate descriptor from
channel's DMA pool.
Function parameters need to be listed described here.
See Documentation/kernel-doc-nano-HOWTO.txt or other source files
for examples.
(Applies to all documented function interfaces here.)
All right, I'll add full descriptions here. :P
+ *
+ * Return - The descriptor allocated. NULL for failed.
+ */
+static struct fsl_desc_sw *fsl_dma_alloc_descriptor(
+ struct fsl_dma_chan *fsl_chan,
+ gfp_t flags)
+{
...
+}
+/**
+ * fsl_chan_xfer_ld_queue -- Transfer the link descriptors
in channel
+ * ld_queue.
The function's short description (unfortunately) must be on only one
line. E.g.:
* fsl_chan_xfer_ld_queue - Transfer link descriptors in
channel ld_queue.
How about it's length greater than 80?
+ */
+static void fsl_chan_xfer_ld_queue(struct fsl_dma_chan *fsl_chan)
+{
...
+}
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
new file mode 100644
index 000..05be9ed
--- /dev/null
+++ b/drivers/dma/fsldma.h
@@ -0,0 +1,188 @@
+struct fsl_dma_chan_regs {
+ __mix32 mr; /* 0x00 - Mode Register */
+ __mix32 sr; /* 0x04 - Status Register */
+ __mix64 cdar; /* 0x08 - Cureent descriptor
address register */
Current
I'll fix it.
Thanks!
- zw
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
If this is experimental, perhaps you should mark the depends line as such depends on on DMA_ENGINE PPC EXPERIMENTAL I'll add EXPERIMENTAL for MPC83xx only. [...] + +fsl_dma_memcpy_issue_pending(chan); +while (fsl_dma_is_complete(chan, cookie, NULL, NULL) +!= DMA_SUCCESS); Again, is it possible to hang your thread here? [...] I'll add msleep here. Thanks! - zw - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 5/5] Add DMA engine driver for Freescale MPC85xx processors.
Hi, Dan,
Does I have followed your new API? :-)
---
Greetings,
Please copy me on any updates to this driver, drivers/dma, or
crypto/async_tx.
Ok.
Below are a few review comments...
Regards,
Dan
+/**
+ * fsl_dma_alloc_descriptor - Allocate descriptor from
channel's DMA pool.
+ *
+ * Return - The descriptor allocated. NULL for failed.
+ */
+static struct fsl_desc_sw *fsl_dma_alloc_descriptor(
+ struct fsl_dma_chan
*fsl_chan,
+ gfp_t flags)
+{
+ dma_addr_t pdesc;
+ struct fsl_desc_sw *desc_sw;
+
+ desc_sw = dma_pool_alloc(fsl_chan-desc_pool,
flags, pdesc);
+ if (desc_sw) {
+ memset(desc_sw, 0, sizeof(struct fsl_desc_sw));
+ dma_async_tx_descriptor_init(desc_sw-async_tx,
+ fsl_chan-common);
+ desc_sw-async_tx.tx_set_src = fsl_dma_set_src;
+ desc_sw-async_tx.tx_set_dest = fsl_dma_set_dest;
+ desc_sw-async_tx.tx_submit = fsl_dma_tx_submit;
+ INIT_LIST_HEAD(desc_sw-async_tx.tx_list);
+ desc_sw-async_tx.phys = pdesc;
+ }
+
+ return desc_sw;
+}
I suggest pre-allocating the descriptors:
1/ It alleviates the need to initialize these async_tx fields
which never change
The dma pool has already stored the descriptors in it's list,
2/ The GFP_ATOMIC allocation can be removed.
Ok.
iop-adma gets by with one PAGE_SIZE buffer (128 descriptors).
[..]
+static irqreturn_t fsl_dma_chan_do_interrupt(int irq, void *data)
+{
+ struct fsl_dma_chan *fsl_chan = (struct fsl_dma_chan *)data;
+ dma_addr_t stat;
+
+ stat = get_sr(fsl_chan);
+ dev_dbg(fsl_chan-device-dev, event: channel %d,
stat = 0x%x\n,
+ fsl_chan-id, stat);
+ set_sr(fsl_chan, stat); /* Clear the event
register */
+
+ stat = ~(FSL_DMA_SR_CB | FSL_DMA_SR_CH);
+ if (!stat)
+ return IRQ_NONE;
+
+ /* If the link descriptor segment transfer finishes,
+* we will recycle the used descriptor.
+*/
+ if (stat FSL_DMA_SR_EOSI) {
+ dev_dbg(fsl_chan-device-dev, event:
End-of-segments INT\n);
+ dev_dbg(fsl_chan-device-dev, event:
clndar 0x%016llx,
+ nlndar 0x%016llx\n,
(u64)get_cdar(fsl_chan),
+ (u64)get_ndar(fsl_chan));
+ stat = ~FSL_DMA_SR_EOSI;
+ fsl_chan_ld_cleanup(fsl_chan, 1);
+ }
+
+ /* If it current transfer is the end-of-transfer,
+* we should clear the Channel Start bit for
+* prepare next transfer.
+*/
+ if (stat (FSL_DMA_SR_EOLNI | FSL_DMA_SR_EOCDI)) {
+ dev_dbg(fsl_chan-device-dev, event:
End-of-link INT\n);
+ stat = ~FSL_DMA_SR_EOLNI;
+ fsl_chan_xfer_ld_queue(fsl_chan);
+ }
+
+ if (stat)
+ dev_dbg(fsl_chan-device-dev, event:
unhandled sr 0x%02x\n,
+ stat);
+
+ dev_dbg(fsl_chan-device-dev, event: Exit\n);
+ tasklet_hi_schedule(dma_tasklet);
+ return IRQ_HANDLED;
+}
This driver implements locking and callbacks inconsistently with how
it is done in ioatdma and iop-adma. The big changes are that all
locks have been upgraded from 'spin_lock_bh' to 'spin_lock_irqsave',
and async_tx callbacks can happen in irq context. I would like to
keep everything in bottom-half context to lessen the restrictions on
what async_tx clients can perform in their callback routines. What
are the implications of moving 'fsl_chan_ld_cleanup' to the tasklet
and changing the 'tasklet_hi_schedule' to 'tasklet_schedule'?
A good suggestion :), I need some investigation here.
[..]
+static struct dma_chan
*of_find_dma_chan_by_phandle(phandle phandle)
+{
+ struct device_node *np;
+ struct dma_chan *chan;
+ struct fsl_dma_device *fdev;
+
+ np = of_find_node_by_phandle(phandle);
+ if (!np || !of_device_is_compatible(np-parent, fsl,dma))
+ return NULL;
+
+ fdev =
dev_get_drvdata(of_find_device_by_node(np-parent)-dev);
+
+ list_for_each_entry(chan, fdev-common.channels,
device_node)
+ if
(to_of_device(to_fsl_chan(chan)-chan_dev)-node == np)
+ return chan;
+ return NULL;
+}
+EXPORT_SYMBOL(of_find_dma_chan_by_phandle);
This routine implies that there is a piece of code somewhere that
wants to select which channels it can use. A similar effect can be
achieved by registering a dma_client with the dmaengine interface
('dma_async_client_register'). Then when the client code makes a call
to
RE: [PATCH 3/5 v3] Add the platform device support with RapidIO to MPC8641HPCN platform.
Hi, Arnd,
I can change it as you metioned now.
Thanks!
-zw
> -Original Message-
> From: [EMAIL PROTECTED]
> [mailto:[EMAIL PROTECTED] On Behalf Of Arnd Bergmann
> Sent: Sunday, July 29, 2007 9:57 PM
> To: [EMAIL PROTECTED]
> Cc: Zhang Wei-r63237; [EMAIL PROTECTED];
> [EMAIL PROTECTED]; [EMAIL PROTECTED];
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 3/5 v3] Add the platform device support
> with RapidIO to MPC8641HPCN platform.
>
> On Thursday 26 July 2007, Zhang Wei wrote:
> > +
> > +static struct of_device_id mpc86xx_of_ids[] = {
> > + { .type = "soc", },
> > + { .compatible = "fsl,rapidio-delta", },
> > + {},
> > +};
>
> With the device tree source you have posted in 2/5, the
> rapidio node is
> a child of the soc bus, and it doesn't have any children of its own.
> Therefore it is completely equivalent to _only_ add the soc type
> to mpc86xx_of_ids[], as in
>
> static struct of_device_id mpc86xx_of_ids[] = {
>{ .type = "soc", },
>{},
> };
>
> Even if you intend to add children to the rapidio node in the future,
> I'd think it would be more appropriate to have those scanned by
> the rapidio bus driver, not by of_platform.
>
> Arnd <><
>
> -
> To unsubscribe from this list: send the line "unsubscribe
> linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/5 v3] Add the platform device support with RapidIO to MPC8641HPCN platform.
Hi, Arnd,
I can change it as you metioned now.
Thanks!
-zw
-Original Message-
From: [EMAIL PROTECTED]
[mailto:[EMAIL PROTECTED] On Behalf Of Arnd Bergmann
Sent: Sunday, July 29, 2007 9:57 PM
To: [EMAIL PROTECTED]
Cc: Zhang Wei-r63237; [EMAIL PROTECTED];
[EMAIL PROTECTED]; [EMAIL PROTECTED];
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/5 v3] Add the platform device support
with RapidIO to MPC8641HPCN platform.
On Thursday 26 July 2007, Zhang Wei wrote:
+
+static struct of_device_id mpc86xx_of_ids[] = {
+ { .type = soc, },
+ { .compatible = fsl,rapidio-delta, },
+ {},
+};
With the device tree source you have posted in 2/5, the
rapidio node is
a child of the soc bus, and it doesn't have any children of its own.
Therefore it is completely equivalent to _only_ add the soc type
to mpc86xx_of_ids[], as in
static struct of_device_id mpc86xx_of_ids[] = {
{ .type = soc, },
{},
};
Even if you intend to add children to the rapidio node in the future,
I'd think it would be more appropriate to have those scanned by
the rapidio bus driver, not by of_platform.
Arnd
-
To unsubscribe from this list: send the line unsubscribe
linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5 v3] Add the explanation and a sample of RapidIO OF node to the document of booting-without-of.txt file.
Hi, Kumar, > + RapidIO is a definition of a system interconnect. This node add > > + the support for RapidIO processor in kernel. The node name is > > + suggested to be 'rapidio'. > > + > > + Required properties: > > + > > +- compatible : Using "fsl,rapidio-delta" for Freescale PowerPC > > + RapidIO controller. > > +- #address-cells : Address representation for > "rapidio" devices. > > + This field represents the number of cells needed to represent > > + the RapidIO address of the registers. > > Can you explain this a little further. I'm a bit confused by > 'RapidIO address of the registers'. > I want to present "This field represents the number of cells [needed to represent the RapidIO address] of the registers." Maybe I should remove 'of the registers' to be more clear. Thanks! -zw - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 4/5 v3] Add RapidIO support to powerpc architecture.
Hi, Kumar, > -Original Message- > From: Kumar Gala [mailto:[EMAIL PROTECTED] > On Jul 26, 2007, at 3:42 AM, Zhang Wei wrote: > > > This patch adds the RapidIO support to the powerpc architecture. > > Some files are moved from ppc. OF-tree and OF-device supports are > > added. > > New silicons such as MPC8548, MPC8641 with serial RapidIO > > controller are > > all supported. > > Memory driver hardware operations are added. > > Global mport variables are changed to master port private variables. > > Multi master ports are supported. > > > > Signed-off-by: Zhang Wei <[EMAIL PROTECTED]> > > --- > > arch/powerpc/Kconfig |8 + > > arch/powerpc/kernel/Makefile |1 + > > arch/powerpc/kernel/rio.c | 64 ++ > > arch/powerpc/sysdev/Makefile |1 + > > arch/powerpc/sysdev/fsl_rio.c | 1455 > ++ > > +++ > > how much of this moved from ppc85xx_rio.c? >From the code size, 2/3 are moved from pcc85xx_rio.c with clean up, 1/3 are new coding. > > > arch/powerpc/sysdev/fsl_rio.h | 20 + > > 6 files changed, 1549 insertions(+), 0 deletions(-) > > create mode 100644 arch/powerpc/kernel/rio.c > > create mode 100644 arch/powerpc/sysdev/fsl_rio.c > > create mode 100644 arch/powerpc/sysdev/fsl_rio.h > > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > > index 00099ef..45f32f1 100644 > > --- a/arch/powerpc/Kconfig > > +++ b/arch/powerpc/Kconfig > > @@ -492,6 +492,14 @@ source "drivers/pci/Kconfig" > > > > source "drivers/pcmcia/Kconfig" > > > > +config RAPIDIO > > + bool "RapidIO support" if MPC8540 || MPC8560 || MPC8641 > || MPC8548 > > + help > > + If you say Y here, the kernel will include drivers and > > + infrastructure code to support RapidIO interconnect devices. > > why not make this depend on something like HAS_RAPIDIO and let the > boards select HAS_RAPIDIO if they have it It's more clear, We can know how many and which processors support RapidIO. :-) > > > + > > +source "drivers/rapidio/Kconfig" > > + > > source "drivers/pci/hotplug/Kconfig" > > > > endmenu > > diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/ > > Makefile > > index 42c42ec..02d4100 100644 > > --- a/arch/powerpc/kernel/Makefile > > +++ b/arch/powerpc/kernel/Makefile > > @@ -70,6 +70,7 @@ pci64-$(CONFIG_PPC64) += > pci_64.o pci_dn.o isa- > > bridge.o > > pci32-$(CONFIG_PPC32) := pci_32.o > > obj-$(CONFIG_PCI) += $(pci64-y) $(pci32-y) pci-common.o > > obj-$(CONFIG_PCI_MSI) += msi.o > > +obj-$(CONFIG_RAPIDIO) += rio.o > > should probably live in sysdev/rio.c This just keep the same position of ppc arch. Thanks! -zw - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 4/5 v3] Add RapidIO support to powerpc architecture.
Hi, Kumar, -Original Message- From: Kumar Gala [mailto:[EMAIL PROTECTED] On Jul 26, 2007, at 3:42 AM, Zhang Wei wrote: This patch adds the RapidIO support to the powerpc architecture. Some files are moved from ppc. OF-tree and OF-device supports are added. New silicons such as MPC8548, MPC8641 with serial RapidIO controller are all supported. Memory driver hardware operations are added. Global mport variables are changed to master port private variables. Multi master ports are supported. Signed-off-by: Zhang Wei [EMAIL PROTECTED] --- arch/powerpc/Kconfig |8 + arch/powerpc/kernel/Makefile |1 + arch/powerpc/kernel/rio.c | 64 ++ arch/powerpc/sysdev/Makefile |1 + arch/powerpc/sysdev/fsl_rio.c | 1455 ++ +++ how much of this moved from ppc85xx_rio.c? From the code size, 2/3 are moved from pcc85xx_rio.c with clean up, 1/3 are new coding. arch/powerpc/sysdev/fsl_rio.h | 20 + 6 files changed, 1549 insertions(+), 0 deletions(-) create mode 100644 arch/powerpc/kernel/rio.c create mode 100644 arch/powerpc/sysdev/fsl_rio.c create mode 100644 arch/powerpc/sysdev/fsl_rio.h diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 00099ef..45f32f1 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -492,6 +492,14 @@ source drivers/pci/Kconfig source drivers/pcmcia/Kconfig +config RAPIDIO + bool RapidIO support if MPC8540 || MPC8560 || MPC8641 || MPC8548 + help + If you say Y here, the kernel will include drivers and + infrastructure code to support RapidIO interconnect devices. why not make this depend on something like HAS_RAPIDIO and let the boards select HAS_RAPIDIO if they have it It's more clear, We can know how many and which processors support RapidIO. :-) + +source drivers/rapidio/Kconfig + source drivers/pci/hotplug/Kconfig endmenu diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/ Makefile index 42c42ec..02d4100 100644 --- a/arch/powerpc/kernel/Makefile +++ b/arch/powerpc/kernel/Makefile @@ -70,6 +70,7 @@ pci64-$(CONFIG_PPC64) += pci_64.o pci_dn.o isa- bridge.o pci32-$(CONFIG_PPC32) := pci_32.o obj-$(CONFIG_PCI) += $(pci64-y) $(pci32-y) pci-common.o obj-$(CONFIG_PCI_MSI) += msi.o +obj-$(CONFIG_RAPIDIO) += rio.o should probably live in sysdev/rio.c This just keep the same position of ppc arch. Thanks! -zw - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5 v3] Add the explanation and a sample of RapidIO OF node to the document of booting-without-of.txt file.
Hi, Kumar, + RapidIO is a definition of a system interconnect. This node add + the support for RapidIO processor in kernel. The node name is + suggested to be 'rapidio'. + + Required properties: + +- compatible : Using fsl,rapidio-delta for Freescale PowerPC + RapidIO controller. +- #address-cells : Address representation for rapidio devices. + This field represents the number of cells needed to represent + the RapidIO address of the registers. Can you explain this a little further. I'm a bit confused by 'RapidIO address of the registers'. I want to present This field represents the number of cells [needed to represent the RapidIO address] of the registers. Maybe I should remove 'of the registers' to be more clear. Thanks! -zw - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/4] Extend the DMA-engine API.
Hi, Dan, Thanks! I get it. It's so lucky we have the same target. When your patch could be accepted? Cheers, Wei. > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Dan Williams > Sent: Thursday, July 12, 2007 12:57 AM > To: Zhang Wei-r63237 > Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; > [EMAIL PROTECTED]; [EMAIL PROTECTED]; > linux-kernel@vger.kernel.org; [EMAIL PROTECTED] > Subject: Re: [PATCH 3/4] Extend the DMA-engine API. > > On 7/11/07, Zhang Wei-r63237 <[EMAIL PROTECTED]> wrote: > > Hi, Dan, > > > > Do you mention here: > http://marc.info/?l=linux-raid=118290909614463=2 ? > > I see the async_tx is located at crypto/ of the above page, > but my patch is for DMA engine in drivers/dma and for DMA > engine driver. > > > > Thanks! > > Wei. > > Hi Wei, > > I was referring to: > http://marc.info/?l=linux-raid=118290909528910=2 > > async_tx is an api that exploits the raw capabilities of the new > dmaengine interface. For your case when the existing api calls do not > provide the proper interface you can open code something like the > following: > > tx = dev->device_prep_dma_(chan, len, int_flag) > tx->tx_set_src(dma_addr_t, tx, index /* for multi-source ops */) > tx->tx_set_dest(dma_addr_t, tx, index) > tx->tx_submit(tx) > > The expectation is that the most common usages of dmaengines will use > async_tx calls, or the 'dma_async_memcpy_foo_to_bar' helper routines. > > -- > Dan > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/4] Extend the DMA-engine API.
Hi, Dan, Thanks! I get it. It's so lucky we have the same target. When your patch could be accepted? Cheers, Wei. -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Dan Williams Sent: Thursday, July 12, 2007 12:57 AM To: Zhang Wei-r63237 Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]; linux-kernel@vger.kernel.org; [EMAIL PROTECTED] Subject: Re: [PATCH 3/4] Extend the DMA-engine API. On 7/11/07, Zhang Wei-r63237 [EMAIL PROTECTED] wrote: Hi, Dan, Do you mention here: http://marc.info/?l=linux-raidm=118290909614463w=2 ? I see the async_tx is located at crypto/ of the above page, but my patch is for DMA engine in drivers/dma and for DMA engine driver. Thanks! Wei. Hi Wei, I was referring to: http://marc.info/?l=linux-raidm=118290909528910w=2 async_tx is an api that exploits the raw capabilities of the new dmaengine interface. For your case when the existing api calls do not provide the proper interface you can open code something like the following: tx = dev-device_prep_dma_operation(chan, len, int_flag) tx-tx_set_src(dma_addr_t, tx, index /* for multi-source ops */) tx-tx_set_dest(dma_addr_t, tx, index) tx-tx_submit(tx) The expectation is that the most common usages of dmaengines will use async_tx calls, or the 'dma_async_memcpy_foo_to_bar' helper routines. -- Dan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/4] Extend the DMA-engine API.
Hi, Dan, Do you mention here: http://marc.info/?l=linux-raid=118290909614463=2 ? I see the async_tx is located at crypto/ of the above page, but my patch is for DMA engine in drivers/dma and for DMA engine driver. Thanks! Wei. > -Original Message- > Subject: Re: [PATCH 3/4] Extend the DMA-engine API. > > On 7/10/07, Zhang Wei <[EMAIL PROTECTED]> wrote: > > Add channel wait queue and transfer callback dma_xfer_callback(). > > If the DMA controller and driver support interrupt, when the > > transfer is finished, it will wakeup the wait queue > > and call the callback function of the channel. > > > > Add dma_async_raw_xfer() to API and device_raw_xfer() to > struct dma_device > > for RAW physical address DMA transfer, which will be used > at transfer > > between I/O address and memory address. > > > > Please review the async_tx API patch series[1], it should meet your > needs. What you call "raw" mode support is now default for the > dmaengine driver interface. I plan to request that this series be > merged for 2.6.23. > > Regards, > Dan > > [1]http://marc.info/?l=linux-raid=2=1=md-accel=b > - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/4] Extend the DMA-engine API.
Hi, Dan, Do you mention here: http://marc.info/?l=linux-raidm=118290909614463w=2 ? I see the async_tx is located at crypto/ of the above page, but my patch is for DMA engine in drivers/dma and for DMA engine driver. Thanks! Wei. -Original Message- Subject: Re: [PATCH 3/4] Extend the DMA-engine API. On 7/10/07, Zhang Wei [EMAIL PROTECTED] wrote: Add channel wait queue and transfer callback dma_xfer_callback(). If the DMA controller and driver support interrupt, when the transfer is finished, it will wakeup the wait queue and call the callback function of the channel. Add dma_async_raw_xfer() to API and device_raw_xfer() to struct dma_device for RAW physical address DMA transfer, which will be used at transfer between I/O address and memory address. Please review the async_tx API patch series[1], it should meet your needs. What you call raw mode support is now default for the dmaengine driver interface. I plan to request that this series be merged for 2.6.23. Regards, Dan [1]http://marc.info/?l=linux-raidw=2r=1s=md-accelq=b - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5 v2] Add the explanation and a sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Segher, > DTS sector to the document of booting-without-of.txt file. > > >>> +- #address-cells : Address representation for > >> "rapidio" devices. > >>> + This field represents the number of cells needed > to represent > >>> + the RapidIO address of the registers. For > >> supporting more than > >>> + 32-bits RapidIO address, this field should be <2>. > >>> + See 1) above for more details on defining #address-cells. > >> > >> What does the RapidIO standard say about number of address > >> bits? You want to follow that, so all RapidIO devices can > >> use the same #address-cells, not just the FSL ones. Also, > >> are there different kinds of address spaces on the bus, or > >> is it just one big memory-like space? > > > > I've checked the specification of RapidIO. The supporting of RapidIO > > extended address modes are 66, 50 and 34 bit. > > These three are all two bits more than some "regular" size -- > do those two extra bits have some special meaning perhaps, > like an address space identifier or something? > > > The Freescale's silicons is only support 34 bit address now. > > Do you mean I should not use words -- 'should be <2>'? > > The #address-cells should be assigned according the address mode > > supported by silicon. > > No. The #address-cells is determined by the bus binding, > so that all RapidIO busses on the planet can be represented > in a similar way in the OF device tree. Take for example > the PCI binding, which gives you three address cells -- one > to distinguish between different address spaces (configuration > space, legacy I/O space, memory mapped space) and to contain > some flags (prefetchable vs. non-prefetchable, etc.); the > other two 32-bit cells contain a 64-bit address, although > config and legacy I/O never are more than 32 bit, and many > PCI devices can't do 64-bit addressing at all. > > Now, there is no OF binding for RapidIO yet of course, but > it would be good to start thinking about one while doing > the binding for your specific controller -- it will make > life easier down the line for everyone, including yourself. > How about I add more words here for more clear expression? Such as "<2> for 34 and 50 bit address, <3> for 66 bit address". Thanks! Wei. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5 v2] Add the explanation and a sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Segher, DTS sector to the document of booting-without-of.txt file. +- #address-cells : Address representation for rapidio devices. + This field represents the number of cells needed to represent + the RapidIO address of the registers. For supporting more than + 32-bits RapidIO address, this field should be 2. + See 1) above for more details on defining #address-cells. What does the RapidIO standard say about number of address bits? You want to follow that, so all RapidIO devices can use the same #address-cells, not just the FSL ones. Also, are there different kinds of address spaces on the bus, or is it just one big memory-like space? I've checked the specification of RapidIO. The supporting of RapidIO extended address modes are 66, 50 and 34 bit. These three are all two bits more than some regular size -- do those two extra bits have some special meaning perhaps, like an address space identifier or something? The Freescale's silicons is only support 34 bit address now. Do you mean I should not use words -- 'should be 2'? The #address-cells should be assigned according the address mode supported by silicon. No. The #address-cells is determined by the bus binding, so that all RapidIO busses on the planet can be represented in a similar way in the OF device tree. Take for example the PCI binding, which gives you three address cells -- one to distinguish between different address spaces (configuration space, legacy I/O space, memory mapped space) and to contain some flags (prefetchable vs. non-prefetchable, etc.); the other two 32-bit cells contain a 64-bit address, although config and legacy I/O never are more than 32 bit, and many PCI devices can't do 64-bit addressing at all. Now, there is no OF binding for RapidIO yet of course, but it would be good to start thinking about one while doing the binding for your specific controller -- it will make life easier down the line for everyone, including yourself. How about I add more words here for more clear expression? Such as 2 for 34 and 50 bit address, 3 for 66 bit address. Thanks! Wei. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5 v2] Add the explanation and a sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Segher, > > +- #address-cells : Address representation for > "rapidio" devices. > > + This field represents the number of cells needed to represent > > + the RapidIO address of the registers. For > supporting more than > > + 32-bits RapidIO address, this field should be <2>. > > + See 1) above for more details on defining #address-cells. > > What does the RapidIO standard say about number of address > bits? You want to follow that, so all RapidIO devices can > use the same #address-cells, not just the FSL ones. Also, > are there different kinds of address spaces on the bus, or > is it just one big memory-like space? > > I've checked the specification of RapidIO. The supporting of RapidIO extended address modes are 66, 50 and 34 bit. The Freescale's silicons is only support 34 bit address now. Do you mean I should not use words -- 'should be <2>'? The #address-cells should be assigned according the address mode supported by silicon. Thanks! Wei. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/5 v2] Add the platform device support with RapidIO to MPC8641HPCN platform.
Hi, Arnd,
>
> On Wednesday 27 June 2007, Zhang Wei wrote:
> > +static struct of_device_id mpc86xx_of_ids[] = {
> > + { .type = "soc", },
> > + { .compatible = "fsl,rapidio-delta", },
> > + {},
> > +};
> > +
> > +static __init int mpc86xx_of_device_init(void)
> > +{
> > + return of_platform_bus_probe(NULL, mpc86xx_of_ids, NULL);
> > +}
>
> This will add any devices below the "fsl,rapidio-delta" device
> as an of_device. Is that what you actually want? I would guess that
> you want to add the bridge itself, not the devices below it.
>
> Is the rapidio device at the root of the device tree, and if so, why
> not under the soc bus?
>
RapidIO is rather a bus that a device although these is no other nodes defined
in its sector now.
Thanks!
Wei.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 3/5 v2] Add the platform device support with RapidIO to MPC8641HPCN platform.
Hi, Arnd,
On Wednesday 27 June 2007, Zhang Wei wrote:
+static struct of_device_id mpc86xx_of_ids[] = {
+ { .type = soc, },
+ { .compatible = fsl,rapidio-delta, },
+ {},
+};
+
+static __init int mpc86xx_of_device_init(void)
+{
+ return of_platform_bus_probe(NULL, mpc86xx_of_ids, NULL);
+}
This will add any devices below the fsl,rapidio-delta device
as an of_device. Is that what you actually want? I would guess that
you want to add the bridge itself, not the devices below it.
Is the rapidio device at the root of the device tree, and if so, why
not under the soc bus?
RapidIO is rather a bus that a device although these is no other nodes defined
in its sector now.
Thanks!
Wei.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5 v2] Add the explanation and a sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Segher, +- #address-cells : Address representation for rapidio devices. + This field represents the number of cells needed to represent + the RapidIO address of the registers. For supporting more than + 32-bits RapidIO address, this field should be 2. + See 1) above for more details on defining #address-cells. What does the RapidIO standard say about number of address bits? You want to follow that, so all RapidIO devices can use the same #address-cells, not just the FSL ones. Also, are there different kinds of address spaces on the bus, or is it just one big memory-like space? I've checked the specification of RapidIO. The supporting of RapidIO extended address modes are 66, 50 and 34 bit. The Freescale's silicons is only support 34 bit address now. Do you mean I should not use words -- 'should be 2'? The #address-cells should be assigned according the address mode supported by silicon. Thanks! Wei. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Kumar and Segher,
>
> > "..8641.." "..8641d.." "..8548.." "..8548e.." "..8543.."
> "..8543e.."
> > "..8572.." "..8572e.." "..8567.." "..8567e.." "..8568.." "..8568e.."
>
> You don't need to mention _all_ compatible devices in
> the "compatible" property, only the few that matter;
> typically the oldest one, and sometimes some intermediate
> device that has extra features over the original one.
>
The oldest one is difficult to find out sometime. Can we only set the
self name in dts, such as "fsl, rapidio-8641", and add this 'compatible'
property to the driver ids arrays? Such as:
static struct of_device_id of_rio_rpn_ids[] = {
{ .compatible = "fsl, rapidio-8540",},
{ .compatible = "fsl, rapidio-8560",},
{ .compatible = "fsl, rapidio-8641",},
{ .compatible = "fsl, rapidio-8548",},
{},
};
How about that?
> It isn't useful to add "compatible" entries that no OS
> probes for.
>
> >> Concrete names are good.
> >
> > While I agree concrete names are good, we put these 'blocks' in so
> > many devices that using the device to match on is pointless.
>
> You *definitely* should put the device name for _this_
> device in there, in case it needs some special workaround.
>
> > I'm all for making up a name like 'Grande', 'Del',
> 'Janeiro'. This is
> > effective what we did with gianfar. The name gets picked up pretty
> > quickly by people.
>
> That can be used as the "base" name, yes.
>
Do you have the name list? I can change my codes according them.
How about 'Mercurary', 'Venus', 'Earth', 'Mars', 'Saturn', 'Jupiter',
'Uranus', 'Neptune',
Or 'Aries', 'Taurus', 'Gemini', 'Cancer', 'Leo', 'Virgo', 'Libra',
'Scorpius', 'Sagittarius', 'Capricornus', 'Aquarius', 'Pisces' ?
Thanks!
Wei.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Kumar and Segher,
..8641.. ..8641d.. ..8548.. ..8548e.. ..8543..
..8543e..
..8572.. ..8572e.. ..8567.. ..8567e.. ..8568.. ..8568e..
You don't need to mention _all_ compatible devices in
the compatible property, only the few that matter;
typically the oldest one, and sometimes some intermediate
device that has extra features over the original one.
The oldest one is difficult to find out sometime. Can we only set the
self name in dts, such as fsl, rapidio-8641, and add this 'compatible'
property to the driver ids arrays? Such as:
static struct of_device_id of_rio_rpn_ids[] = {
{ .compatible = fsl, rapidio-8540,},
{ .compatible = fsl, rapidio-8560,},
{ .compatible = fsl, rapidio-8641,},
{ .compatible = fsl, rapidio-8548,},
{},
};
How about that?
It isn't useful to add compatible entries that no OS
probes for.
Concrete names are good.
While I agree concrete names are good, we put these 'blocks' in so
many devices that using the device to match on is pointless.
You *definitely* should put the device name for _this_
device in there, in case it needs some special workaround.
I'm all for making up a name like 'Grande', 'Del',
'Janeiro'. This is
effective what we did with gianfar. The name gets picked up pretty
quickly by people.
That can be used as the base name, yes.
Do you have the name list? I can change my codes according them.
How about 'Mercurary', 'Venus', 'Earth', 'Mars', 'Saturn', 'Jupiter',
'Uranus', 'Neptune',
Or 'Aries', 'Taurus', 'Gemini', 'Cancer', 'Leo', 'Virgo', 'Libra',
'Scorpius', 'Sagittarius', 'Capricornus', 'Aquarius', 'Pisces' ?
Thanks!
Wei.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Segher, > > >>> +- device_type : Should be "rapidio" > >> > >> There is no OF binding, so no. > > > > So, we need to define it. > > If you want to. Until that has been done, don't use > a "device_type". Linux won't use it, anyway. Do you have another ideas about that? Only remove it? > > >>> +- compatible : Should be "fsl,rapidio-v0.0" or > >> "fsl,rapidio-v1.0" > >>> + and so on. The version number is got from IP Block Revision > >>> + Register of RapidIO controller. > >> > >> It's better to use real device names, just like everyone > >> else. > > > > Some silicons of Freescale processor are the same RapidIO > controller, > > such as mpc8540/mpc8560 are the same (v0.0), mpc8548/mpc8641 are the > > same (v1.0). For v1.0 RapidIO controller, should we use mpc8548 or > > mpc8641? Those will make people confused. > > Not at all. On an 8641 it could be > > compatible = "fsl,mpc8641-rapidio" "fsl,mpc8548-rapidio"; > > which states "this is the 8641 thing and it is compatible > to the 8548 thing". Perfectly clear. > > > Using IP Block Revision is a > > clear choice. > > I don't think so. For one thing, it describes a version of > a cell design, not a version of an actual device. For another > thing, if I hear "8641" I know what you're talking about (sort > of, anyway), but I draw a blank stare if you say "v1.0". I'm > sure I'm not the only one. Concrete names are good. > >From the different view ways, there are different results. Getting the version from RapidIO IP revision register is clear to me. :) > >>> +- #address-cells : Address representation for > >> "rapidio" devices. > >>> + This field represents the number of cells needed > to represent > >>> + the RapidIO address of the registers. For > >> supporting more than > >>> + 36-bits RapidIO address, this field should be <2>. > >> > >> More than 32 bit? > > > > Yes, RapidIO bus address width is 34 bits. > > You said "more than 36 bit", I tried to ask if that is a typo > perhaps. Ya, caught by you! I'll fix it in next version. :) > > >> No. The format of an "interrupts" entry is defined by > >> the interrupt domain this device sits in, not by the > >> device itself. > >> > > Do you misunderstand the meaning of 'interrupts'? > > Hahaha. No, I don't misunderstand what the "interrupts" property > means. Perhaps you do? > > > These interrupts is > > issued from the RapidIO controller to the pic controller for tx, rx, > > err, doorbell and message. > > But the rapidio node doesn't know or care what the interrupts > are connected to, and neither should it. That's what the > interrupt mapping recommended practice is for. > There are no rapidio device in it. Doorbell, msg are all parts of rapidio controller. For example, 8641 rapidio controller have 2 msg unit: msg0 and msg1. They are not rapidio devices. Each msg unit has the tx_irq and rx_irq. > >>> For this sector, interrupts order should be > >>> + >>> + msg2_tx_irq msg2_rx_irq ... msgN_tx_irq msgN_rx_irq>. > >> > >> That's to be defined in the binding for your specific device, > >> not in a more generic rapidio binding. > > > > These description is just for compatible="fsl,rapidio-v*.*" rapidio > > controller. > > Okay, good. Please make that way more obvious then :-) > > >>> + #address-cells = <2>; > >> > >> You want a #size-cells as well. > > > > The size is not used in this sector, so no defined. > > The size _is_ used; in the "ranges" property in this node, > for example. It is also needed to describe the "reg" for > any child node of this node. > > A non-existant "#size-cells" means 1, and "#address-cells" > means 2, so in principle you could do without these > properties; but Linux doesn't parse the tree correctly in > that case (which reminds me, I have some more patches to > send). > Ok, I'll add it in the next version for more religious. Thanks! Wei. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Segher, > -Original Message- > Subject: Re: [PATCH 1/5] Add the explanation and sample of > RapidIO DTS sector to the document of booting-without-of.txt file. > > > + k) RapidIO > > + > > + Required properties: > > + > > +- device_type : Should be "rapidio" > > There is no OF binding, so no. So, we need to define it. > > > +- compatible : Should be "fsl,rapidio-v0.0" or > "fsl,rapidio-v1.0" > > + and so on. The version number is got from IP Block Revision > > + Register of RapidIO controller. > > It's better to use real device names, just like everyone > else. Some silicons of Freescale processor are the same RapidIO controller, such as mpc8540/mpc8560 are the same (v0.0), mpc8548/mpc8641 are the same (v1.0). For v1.0 RapidIO controller, should we use mpc8548 or mpc8641? Those will make people confused. Using IP Block Revision is a clear choice. > > > +- #address-cells : Address representation for > "rapidio" devices. > > + This field represents the number of cells needed to represent > > + the RapidIO address of the registers. For > supporting more than > > + 36-bits RapidIO address, this field should be <2>. > > More than 32 bit? Yes, RapidIO bus address width is 34 bits. > > > +- interrupt-parent : the phandle for the interrupt > controller that > > + services interrupts for this device. > > Not required, depends on the interrupt tree of the system. > > > +- interrupts : where a is the interrupt number and b is a > > + field that represents an encoding of the sense and level > > + information for the interrupt. > > No. The format of an "interrupts" entry is defined by > the interrupt domain this device sits in, not by the > device itself. > Do you misunderstand the meaning of 'interrupts'? These interrupts is issued from the RapidIO controller to the pic controller for tx, rx, err, doorbell and message. > > For this sector, interrupts order should be > > + > + msg2_tx_irq msg2_rx_irq ... msgN_tx_irq msgN_rx_irq>. > > That's to be defined in the binding for your specific device, > not in a more generic rapidio binding. These description is just for compatible="fsl,rapidio-v*.*" rapidio controller. > > > + #address-cells = <2>; > > You want a #size-cells as well. > The size is not used in this sector, so no defined. Thanks! Wei. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Segher, -Original Message- Subject: Re: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file. + k) RapidIO + + Required properties: + +- device_type : Should be rapidio There is no OF binding, so no. So, we need to define it. +- compatible : Should be fsl,rapidio-v0.0 or fsl,rapidio-v1.0 + and so on. The version number is got from IP Block Revision + Register of RapidIO controller. It's better to use real device names, just like everyone else. Some silicons of Freescale processor are the same RapidIO controller, such as mpc8540/mpc8560 are the same (v0.0), mpc8548/mpc8641 are the same (v1.0). For v1.0 RapidIO controller, should we use mpc8548 or mpc8641? Those will make people confused. Using IP Block Revision is a clear choice. +- #address-cells : Address representation for rapidio devices. + This field represents the number of cells needed to represent + the RapidIO address of the registers. For supporting more than + 36-bits RapidIO address, this field should be 2. More than 32 bit? Yes, RapidIO bus address width is 34 bits. +- interrupt-parent : the phandle for the interrupt controller that + services interrupts for this device. Not required, depends on the interrupt tree of the system. +- interrupts : a b where a is the interrupt number and b is a + field that represents an encoding of the sense and level + information for the interrupt. No. The format of an interrupts entry is defined by the interrupt domain this device sits in, not by the device itself. Do you misunderstand the meaning of 'interrupts'? These interrupts is issued from the RapidIO controller to the pic controller for tx, rx, err, doorbell and message. For this sector, interrupts order should be + err_irq bell_outb_irq bell_inb_irq msg1_tx_irq msg1_rx_irq + msg2_tx_irq msg2_rx_irq ... msgN_tx_irq msgN_rx_irq. That's to be defined in the binding for your specific device, not in a more generic rapidio binding. These description is just for compatible=fsl,rapidio-v*.* rapidio controller. + #address-cells = 2; You want a #size-cells as well. The size is not used in this sector, so no defined. Thanks! Wei. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 1/5] Add the explanation and sample of RapidIO DTS sector to the document of booting-without-of.txt file.
Hi, Segher, +- device_type : Should be rapidio There is no OF binding, so no. So, we need to define it. If you want to. Until that has been done, don't use a device_type. Linux won't use it, anyway. Do you have another ideas about that? Only remove it? +- compatible : Should be fsl,rapidio-v0.0 or fsl,rapidio-v1.0 + and so on. The version number is got from IP Block Revision + Register of RapidIO controller. It's better to use real device names, just like everyone else. Some silicons of Freescale processor are the same RapidIO controller, such as mpc8540/mpc8560 are the same (v0.0), mpc8548/mpc8641 are the same (v1.0). For v1.0 RapidIO controller, should we use mpc8548 or mpc8641? Those will make people confused. Not at all. On an 8641 it could be compatible = fsl,mpc8641-rapidio fsl,mpc8548-rapidio; which states this is the 8641 thing and it is compatible to the 8548 thing. Perfectly clear. Using IP Block Revision is a clear choice. I don't think so. For one thing, it describes a version of a cell design, not a version of an actual device. For another thing, if I hear 8641 I know what you're talking about (sort of, anyway), but I draw a blank stare if you say v1.0. I'm sure I'm not the only one. Concrete names are good. From the different view ways, there are different results. Getting the version from RapidIO IP revision register is clear to me. :) +- #address-cells : Address representation for rapidio devices. + This field represents the number of cells needed to represent + the RapidIO address of the registers. For supporting more than + 36-bits RapidIO address, this field should be 2. More than 32 bit? Yes, RapidIO bus address width is 34 bits. You said more than 36 bit, I tried to ask if that is a typo perhaps. Ya, caught by you! I'll fix it in next version. :) No. The format of an interrupts entry is defined by the interrupt domain this device sits in, not by the device itself. Do you misunderstand the meaning of 'interrupts'? Hahaha. No, I don't misunderstand what the interrupts property means. Perhaps you do? These interrupts is issued from the RapidIO controller to the pic controller for tx, rx, err, doorbell and message. But the rapidio node doesn't know or care what the interrupts are connected to, and neither should it. That's what the interrupt mapping recommended practice is for. There are no rapidio device in it. Doorbell, msg are all parts of rapidio controller. For example, 8641 rapidio controller have 2 msg unit: msg0 and msg1. They are not rapidio devices. Each msg unit has the tx_irq and rx_irq. For this sector, interrupts order should be + err_irq bell_outb_irq bell_inb_irq msg1_tx_irq msg1_rx_irq + msg2_tx_irq msg2_rx_irq ... msgN_tx_irq msgN_rx_irq. That's to be defined in the binding for your specific device, not in a more generic rapidio binding. These description is just for compatible=fsl,rapidio-v*.* rapidio controller. Okay, good. Please make that way more obvious then :-) + #address-cells = 2; You want a #size-cells as well. The size is not used in this sector, so no defined. The size _is_ used; in the ranges property in this node, for example. It is also needed to describe the reg for any child node of this node. A non-existant #size-cells means 1, and #address-cells means 2, so in principle you could do without these properties; but Linux doesn't parse the tree correctly in that case (which reminds me, I have some more patches to send). Ok, I'll add it in the next version for more religious. Thanks! Wei. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/5] Add RapidIO sector to MPC8641HPCN board dts file.
Hi, Phil,
> > +++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
> > @@ -329,6 +329,19 @@
> > >;
> > };
> >
> > + [EMAIL PROTECTED] {
> > + device_type = "rapidio";
> > + compatible = "fsl,rapidio-v1.0";
> > + #address-cells = <2>;
> > + reg = ;
> > + ranges = <0 0 c000 2000>;
>
> Don't understand the range setup... The code uses c000 as
> the start
> and 2000 as the size for the rapidio law. So whats the 0 0 for?
> At first I thought address-cells 2 means that the range numbers are
> pairs expressing 64-bit numbers, eg start 0 0, size c000 2000
> but thats not right...
>
RapidIO address width is 34bits, so we use <2> for #address-cells.
"0 0" is the RapidIO bus address, since the #address-cells is 2, it's
double zero. "c000" is the parent bus address, which is memory
address. "2000" is the size.
So, they means mapping the address of memory 0xc000 to RapidIO bus
address 0x0 and the size is 0x2000.
> Sorry if I'm asking stupid questions
>
> And thanks for the patch, just what I need.
>
> BTW do you have any notes/documentation on the space
> allocation routines
> intended use. I'm trying to work it out but I don't see any
> actual users
> of it yet... I need something like it for my distributed DMA driver,
> hence the interest.
Sorry, there is no documentation now. But I'll post an example for using
them.
>
> Cheers
> Phil
>
> > + interrupt-parent = <>;
> > + /* err_irq bell_outb_irq bell_inb_irq
> > + msg1_tx_irq msg1_rx_irq
> > + msg2_tx_irq
> msg2_rx_irq */
> > + interrupts = <30 2 31 2 32 2 35 2 36 2
> 37 2 38 2>;
> > + };
> > +
> > mpic: [EMAIL PROTECTED] {
> > clock-frequency = <0>;
> > interrupt-controller;
>
>
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 2/5] Add RapidIO sector to MPC8641HPCN board dts file.
Hi, Phil,
+++ b/arch/powerpc/boot/dts/mpc8641_hpcn.dts
@@ -329,6 +329,19 @@
;
};
+ [EMAIL PROTECTED] {
+ device_type = rapidio;
+ compatible = fsl,rapidio-v1.0;
+ #address-cells = 2;
+ reg = c 2;
+ ranges = 0 0 c000 2000;
Don't understand the range setup... The code uses c000 as
the start
and 2000 as the size for the rapidio law. So whats the 0 0 for?
At first I thought address-cells 2 means that the range numbers are
pairs expressing 64-bit numbers, eg start 0 0, size c000 2000
but thats not right...
RapidIO address width is 34bits, so we use 2 for #address-cells.
0 0 is the RapidIO bus address, since the #address-cells is 2, it's
double zero. c000 is the parent bus address, which is memory
address. 2000 is the size.
So, they means mapping the address of memory 0xc000 to RapidIO bus
address 0x0 and the size is 0x2000.
Sorry if I'm asking stupid questions
And thanks for the patch, just what I need.
BTW do you have any notes/documentation on the space
allocation routines
intended use. I'm trying to work it out but I don't see any
actual users
of it yet... I need something like it for my distributed DMA driver,
hence the interest.
Sorry, there is no documentation now. But I'll post an example for using
them.
Cheers
Phil
+ interrupt-parent = mpic;
+ /* err_irq bell_outb_irq bell_inb_irq
+ msg1_tx_irq msg1_rx_irq
+ msg2_tx_irq
msg2_rx_irq */
+ interrupts = 30 2 31 2 32 2 35 2 36 2
37 2 38 2;
+ };
+
mpic: [EMAIL PROTECTED] {
clock-frequency = 0;
interrupt-controller;
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
