RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Tuesday, September 03, 2013 7:32 PM
> To: Lu Jingchang-B35083
> Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; devicet...@vger.kernel.org
> Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
> 
> On Tue, Sep 03, 2013 at 05:43:21AM +, Lu Jingchang-B35083 wrote:
> >   Do you mean the DMA_SLAVE_CONFIG device_control? Yeah, the slave
> driver could pass
> > the slave_id. But the DMA_SLAVE_CONFIG may be called more than once,
> and the eDMA
> > driver just needs to set the slave id once for any given channel, after
> that the
> > transfer is transparent to the device.
> It depends, for a channel requested, if you are only tranferring to a
> particular
> slave device then it can be confugured once.
> so
> 1. allocate channel
> 2. dmaengine_slave_config()
> 
> then you cnan do preare etc multiple times based on need.
 But if the preferred slave id configuration should be in 
dmaengine_slave_config,
I will send out this change in the next version patch.
Thanks!





Best Regards,
Jingchang


 
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

 -Original Message-
 From: Vinod Koul [mailto:vinod.k...@intel.com]
 Sent: Tuesday, September 03, 2013 7:32 PM
 To: Lu Jingchang-B35083
 Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
 ker...@lists.infradead.org; devicet...@vger.kernel.org
 Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
 
 On Tue, Sep 03, 2013 at 05:43:21AM +, Lu Jingchang-B35083 wrote:
Do you mean the DMA_SLAVE_CONFIG device_control? Yeah, the slave
 driver could pass
  the slave_id. But the DMA_SLAVE_CONFIG may be called more than once,
 and the eDMA
  driver just needs to set the slave id once for any given channel, after
 that the
  transfer is transparent to the device.
 It depends, for a channel requested, if you are only tranferring to a
 particular
 slave device then it can be confugured once.
 so
 1. allocate channel
 2. dmaengine_slave_config()
 
 then you cnan do preare etc multiple times based on need.
 But if the preferred slave id configuration should be in 
dmaengine_slave_config,
I will send out this change in the next version patch.
Thanks!





Best Regards,
Jingchang


 
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Tuesday, September 03, 2013 7:32 PM
> To: Lu Jingchang-B35083
> Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; devicet...@vger.kernel.org
> Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
> 
> On Tue, Sep 03, 2013 at 05:43:21AM +, Lu Jingchang-B35083 wrote:
> >   Do you mean the DMA_SLAVE_CONFIG device_control? Yeah, the slave
> driver could pass
> > the slave_id. But the DMA_SLAVE_CONFIG may be called more than once,
> and the eDMA
> > driver just needs to set the slave id once for any given channel, after
> that the
> > transfer is transparent to the device.
> It depends, for a channel requested, if you are only tranferring to a
> particular
> slave device then it can be confugured once.
> so
> 1. allocate channel
> 2. dmaengine_slave_config()
> 
> then you cnan do preare etc multiple times based on need.
Yeah, I think we should just configure the slave id once and this would make the
configuration simply. But by debugging, I find the dmaengine_slave_config() may 
be
called more one times after requested by on user. So I put the slave id 
configuration
in the channel request phase. And the effect is the same for exclusive channel.
Thanks!


Best Regards,
Jingchang

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Tue, Sep 03, 2013 at 05:43:21AM +, Lu Jingchang-B35083 wrote:
>   Do you mean the DMA_SLAVE_CONFIG device_control? Yeah, the slave driver 
> could pass
> the slave_id. But the DMA_SLAVE_CONFIG may be called more than once, and the 
> eDMA
> driver just needs to set the slave id once for any given channel, after that 
> the 
> transfer is transparent to the device. 
It depends, for a channel requested, if you are only tranferring to a particular
slave device then it can be confugured once.
so
1. allocate channel
2. dmaengine_slave_config()

then you cnan do preare etc multiple times based on need.

~Vinod
>   On the other hand, the DMAMUX's setting procedure requires first disable 
> the dmamux
> before setting, then if it is set in DMA_SLAVE_CONFIG, the repeated setting 
> may be
> complex and unnecessary. The channel is occupied exclusively by the 
> peripheral.
>   So, according the HW feature, I think the eDMA needs only set the slave id 
> once,
> and since the of_dma helper has pass the slave id in on xlate, we can get and 
> set
> the slave id here. How do you think about this?

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

 -Original Message-
 From: Vinod Koul [mailto:vinod.k...@intel.com]
 Sent: Tuesday, September 03, 2013 7:32 PM
 To: Lu Jingchang-B35083
 Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
 ker...@lists.infradead.org; devicet...@vger.kernel.org
 Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
 
 On Tue, Sep 03, 2013 at 05:43:21AM +, Lu Jingchang-B35083 wrote:
Do you mean the DMA_SLAVE_CONFIG device_control? Yeah, the slave
 driver could pass
  the slave_id. But the DMA_SLAVE_CONFIG may be called more than once,
 and the eDMA
  driver just needs to set the slave id once for any given channel, after
 that the
  transfer is transparent to the device.
 It depends, for a channel requested, if you are only tranferring to a
 particular
 slave device then it can be confugured once.
 so
 1. allocate channel
 2. dmaengine_slave_config()
 
 then you cnan do preare etc multiple times based on need.
Yeah, I think we should just configure the slave id once and this would make the
configuration simply. But by debugging, I find the dmaengine_slave_config() may 
be
called more one times after requested by on user. So I put the slave id 
configuration
in the channel request phase. And the effect is the same for exclusive channel.
Thanks!


Best Regards,
Jingchang

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Tue, Sep 03, 2013 at 05:43:21AM +, Lu Jingchang-B35083 wrote:
   Do you mean the DMA_SLAVE_CONFIG device_control? Yeah, the slave driver 
 could pass
 the slave_id. But the DMA_SLAVE_CONFIG may be called more than once, and the 
 eDMA
 driver just needs to set the slave id once for any given channel, after that 
 the 
 transfer is transparent to the device. 
It depends, for a channel requested, if you are only tranferring to a particular
slave device then it can be confugured once.
so
1. allocate channel
2. dmaengine_slave_config()

then you cnan do preare etc multiple times based on need.

~Vinod
   On the other hand, the DMAMUX's setting procedure requires first disable 
 the dmamux
 before setting, then if it is set in DMA_SLAVE_CONFIG, the repeated setting 
 may be
 complex and unnecessary. The channel is occupied exclusively by the 
 peripheral.
   So, according the HW feature, I think the eDMA needs only set the slave id 
 once,
 and since the of_dma helper has pass the slave id in on xlate, we can get and 
 set
 the slave id here. How do you think about this?

-- 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

> > How about change the filter_fn to follow:
> > static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
> > {
> > struct fsl_edma_filter_param *fparam = fn_param;
> > struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > unsigned char val;
> >
> > if (fsl_chan->edmamux->mux_id != fparam->mux_id)
> > return false;
> >
> > val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(fparam-
> >slot_id);
> > fsl_edmamux_config_chan(fsl_chan, val);
> > return true;
> > }
> > In fact the slot_id isn't need elsewhere, and if the filter return true,
> > This channel should be to this request. So no need to save the slave id,
> Right?
> something like
> 
> static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
> {
>   struct fsl_edma_filter_param *fparam = fn_param;
>   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> 
>   if (fsl_chan->edmamux->mux_id != fparam->mux_id)
>   return false;
>   return true;
> }
> 
> in thedriver which calls this:
> 
> before prep:
> 
>   config->slave_id = val;
> 
>   dma_set_slave_config(chan, slave);
> 
  Do you mean the DMA_SLAVE_CONFIG device_control? Yeah, the slave driver could 
pass
the slave_id. But the DMA_SLAVE_CONFIG may be called more than once, and the 
eDMA
driver just needs to set the slave id once for any given channel, after that 
the 
transfer is transparent to the device. 
  On the other hand, the DMAMUX's setting procedure requires first disable the 
dmamux
before setting, then if it is set in DMA_SLAVE_CONFIG, the repeated setting may 
be
complex and unnecessary. The channel is occupied exclusively by the peripheral.
  So, according the HW feature, I think the eDMA needs only set the slave id 
once,
and since the of_dma helper has pass the slave id in on xlate, we can get and 
set
the slave id here. How do you think about this?
  Thanks!









Best Regards,
Jingchang

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Mon, Sep 02, 2013 at 07:32:53AM +, Lu Jingchang-B35083 wrote:
> > > > > > > + chan->private = fn_param;
> > > > > > why do you need to use chan->private?
> > > > > [Lu Jingchang]
> > > > > The private used here is to store the slot_id information, which
> > must
> > > > be used
> > > > > by the DMAMUX in alloc_chan_resources function. Thanks.
> > > > Why dont you pass this in struct dma_slave_config memeber slave_id
> > for
> > > > this.
> > > [Lu Jingchang-b35083]
> > > I will drop this private and setup the slave_id directly in the filter
> > function.
> > why in filter? before calling prepare function you can set the slave
> > config
> How about change the filter_fn to follow:
> static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
> {
> struct fsl_edma_filter_param *fparam = fn_param;
> struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> unsigned char val;
> 
> if (fsl_chan->edmamux->mux_id != fparam->mux_id)
> return false;
> 
> val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(fparam->slot_id);
> fsl_edmamux_config_chan(fsl_chan, val);
> return true;
> }
> In fact the slot_id isn't need elsewhere, and if the filter return true, 
> This channel should be to this request. So no need to save the slave id, 
> Right?
something like

static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
{
struct fsl_edma_filter_param *fparam = fn_param;
struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);

if (fsl_chan->edmamux->mux_id != fparam->mux_id)
return false;
return true;
}

in thedriver which calls this:

before prep:

config->slave_id = val;

dma_set_slave_config(chan, slave);

~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Monday, September 02, 2013 2:37 PM
> To: Lu Jingchang-B35083
> Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; devicet...@vger.kernel.org
> Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
> 
> On Mon, Sep 02, 2013 at 07:10:53AM +, Lu Jingchang-B35083 wrote:
> > > -Original Message-
> > > From: Vinod Koul [mailto:vinod.k...@intel.com]
> > > Sent: Monday, September 02, 2013 12:51 PM
> > > To: Lu Jingchang-B35083
> > > Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
> > > ker...@lists.infradead.org; devicet...@vger.kernel.org
> > > Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver
> support
> > >
> > > On Thu, Aug 29, 2013 at 03:32:04AM +, Lu Jingchang-B35083 wrote:
> > >
> > > Please use a right MUA and wrap your lines at 80chars...
> 
> > > > [Lu Jingchang]
> > > No need to put your name :)
> > [Lu Jingchang-b35083]
> > Aha, the Microsoft Outlook adds this automatically with default option.
> You can change that!!
Thanks.
> > >
> > > > > > +static bool fsl_edma_filter_fn(struct dma_chan *chan, void
> > > *fn_param)
> > > > > > +{
> > > > > > +   struct fsl_edma_filter_param *fparam = fn_param;
> > > > > > +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > > > > > +
> > > > > > +   if (fsl_chan->edmamux->mux_id != fparam->mux_id)
> > > > > > +   return false;
> > > > > > +
> > > > > > +   fsl_chan->slot_id = fparam->slot_id;
> > > > > > +   chan->private = fn_param;
> > > > > why do you need to use chan->private?
> > > > [Lu Jingchang]
> > > > The private used here is to store the slot_id information, which
> must
> > > be used
> > > > by the DMAMUX in alloc_chan_resources function. Thanks.
> > > Why dont you pass this in struct dma_slave_config memeber slave_id
> for
> > > this.
> > [Lu Jingchang-b35083]
> > I will drop this private and setup the slave_id directly in the filter
> function.
> why in filter? before calling prepare function you can set the slave
> config
How about change the filter_fn to follow:
static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
{
struct fsl_edma_filter_param *fparam = fn_param;
struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
unsigned char val;

if (fsl_chan->edmamux->mux_id != fparam->mux_id)
return false;

val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(fparam->slot_id);
fsl_edmamux_config_chan(fsl_chan, val);
return true;
}
In fact the slot_id isn't need elsewhere, and if the filter return true, 
This channel should be to this request. So no need to save the slave id, Right?



Best Regareds,
Jingchang

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Mon, Sep 02, 2013 at 07:10:53AM +, Lu Jingchang-B35083 wrote:
> > -Original Message-
> > From: Vinod Koul [mailto:vinod.k...@intel.com]
> > Sent: Monday, September 02, 2013 12:51 PM
> > To: Lu Jingchang-B35083
> > Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
> > ker...@lists.infradead.org; devicet...@vger.kernel.org
> > Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
> > 
> > On Thu, Aug 29, 2013 at 03:32:04AM +, Lu Jingchang-B35083 wrote:
> > 
> > Please use a right MUA and wrap your lines at 80chars...

> > > [Lu Jingchang]
> > No need to put your name :)
> [Lu Jingchang-b35083] 
> Aha, the Microsoft Outlook adds this automatically with default option.
You can change that!!

> > 
> > > Ok, if it is needed, I will add residue calculation in the next version.
> > Yes this is needed
> > 
> > > > > +static bool fsl_edma_filter_fn(struct dma_chan *chan, void
> > *fn_param)
> > > > > +{
> > > > > + struct fsl_edma_filter_param *fparam = fn_param;
> > > > > + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > > > > +
> > > > > + if (fsl_chan->edmamux->mux_id != fparam->mux_id)
> > > > > + return false;
> > > > > +
> > > > > + fsl_chan->slot_id = fparam->slot_id;
> > > > > + chan->private = fn_param;
> > > > why do you need to use chan->private?
> > > [Lu Jingchang]
> > > The private used here is to store the slot_id information, which must
> > be used
> > > by the DMAMUX in alloc_chan_resources function. Thanks.
> > Why dont you pass this in struct dma_slave_config memeber slave_id for
> > this.
> [Lu Jingchang-b35083] 
> I will drop this private and setup the slave_id directly in the filter 
> function.
why in filter? before calling prepare function you can set the slave config

~Vinod
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

> -Original Message-
> From: Vinod Koul [mailto:vinod.k...@intel.com]
> Sent: Monday, September 02, 2013 12:51 PM
> To: Lu Jingchang-B35083
> Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; devicet...@vger.kernel.org
> Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
> 
> On Thu, Aug 29, 2013 at 03:32:04AM +, Lu Jingchang-B35083 wrote:
> 
> Please use a right MUA and wrap your lines at 80chars...
> 
> >
> > >
> > > > +   return -EINVAL;
> > > > +   }
> > > > +   return 0;
> > > > +
> > > > +   default:
> > > > +   return -ENXIO;
> > > > +   }
> > > > +}
> > > > +
> > > > +static enum dma_status fsl_edma_tx_status(struct dma_chan *chan,
> > > > +   dma_cookie_t cookie, struct dma_tx_state *txstate)
> > > > +{
> > > > +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > > > +
> > > > +   if (fsl_chan->status == DMA_ERROR)
> > > > +   return DMA_ERROR;
> > > > +
> > > > +   return dma_cookie_status(chan, cookie, txstate);
> > > this will tell if the DMA is completed or not only.
> > > You also need to calculate residue for the pending dma
> > >
> > > Since you support cyclic this should be done properly...
> > >
> > > also you cna take more help from vchan support to make your life
> > > simpler...
> > [Lu Jingchang]
> No need to put your name :)
[Lu Jingchang-b35083] 
Aha, the Microsoft Outlook adds this automatically with default option.
> 
> > Ok, if it is needed, I will add residue calculation in the next version.
> Yes this is needed
> 
> > > > +static bool fsl_edma_filter_fn(struct dma_chan *chan, void
> *fn_param)
> > > > +{
> > > > +   struct fsl_edma_filter_param *fparam = fn_param;
> > > > +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > > > +
> > > > +   if (fsl_chan->edmamux->mux_id != fparam->mux_id)
> > > > +   return false;
> > > > +
> > > > +   fsl_chan->slot_id = fparam->slot_id;
> > > > +   chan->private = fn_param;
> > > why do you need to use chan->private?
> > [Lu Jingchang]
> > The private used here is to store the slot_id information, which must
> be used
> > by the DMAMUX in alloc_chan_resources function. Thanks.
> Why dont you pass this in struct dma_slave_config memeber slave_id for
> this.
[Lu Jingchang-b35083] 
I will drop this private and setup the slave_id directly in the filter function.
The slave id is only used in first request phase, if filter is true, the channel
Should be to this slave.
Thanks.







Best Regards,
Jingchang


N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

 -Original Message-
 From: Vinod Koul [mailto:vinod.k...@intel.com]
 Sent: Monday, September 02, 2013 12:51 PM
 To: Lu Jingchang-B35083
 Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
 ker...@lists.infradead.org; devicet...@vger.kernel.org
 Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
 
 On Thu, Aug 29, 2013 at 03:32:04AM +, Lu Jingchang-B35083 wrote:
 
 Please use a right MUA and wrap your lines at 80chars...
 
 
  
+   return -EINVAL;
+   }
+   return 0;
+
+   default:
+   return -ENXIO;
+   }
+}
+
+static enum dma_status fsl_edma_tx_status(struct dma_chan *chan,
+   dma_cookie_t cookie, struct dma_tx_state *txstate)
+{
+   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
+
+   if (fsl_chan-status == DMA_ERROR)
+   return DMA_ERROR;
+
+   return dma_cookie_status(chan, cookie, txstate);
   this will tell if the DMA is completed or not only.
   You also need to calculate residue for the pending dma
  
   Since you support cyclic this should be done properly...
  
   also you cna take more help from vchan support to make your life
   simpler...
  [Lu Jingchang]
 No need to put your name :)
[Lu Jingchang-b35083] 
Aha, the Microsoft Outlook adds this automatically with default option.
 
  Ok, if it is needed, I will add residue calculation in the next version.
 Yes this is needed
 
+static bool fsl_edma_filter_fn(struct dma_chan *chan, void
 *fn_param)
+{
+   struct fsl_edma_filter_param *fparam = fn_param;
+   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
+
+   if (fsl_chan-edmamux-mux_id != fparam-mux_id)
+   return false;
+
+   fsl_chan-slot_id = fparam-slot_id;
+   chan-private = fn_param;
   why do you need to use chan-private?
  [Lu Jingchang]
  The private used here is to store the slot_id information, which must
 be used
  by the DMAMUX in alloc_chan_resources function. Thanks.
 Why dont you pass this in struct dma_slave_config memeber slave_id for
 this.
[Lu Jingchang-b35083] 
I will drop this private and setup the slave_id directly in the filter function.
The slave id is only used in first request phase, if filter is true, the channel
Should be to this slave.
Thanks.







Best Regards,
Jingchang


N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Mon, Sep 02, 2013 at 07:10:53AM +, Lu Jingchang-B35083 wrote:
  -Original Message-
  From: Vinod Koul [mailto:vinod.k...@intel.com]
  Sent: Monday, September 02, 2013 12:51 PM
  To: Lu Jingchang-B35083
  Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
  ker...@lists.infradead.org; devicet...@vger.kernel.org
  Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
  
  On Thu, Aug 29, 2013 at 03:32:04AM +, Lu Jingchang-B35083 wrote:
  
  Please use a right MUA and wrap your lines at 80chars...

   [Lu Jingchang]
  No need to put your name :)
 [Lu Jingchang-b35083] 
 Aha, the Microsoft Outlook adds this automatically with default option.
You can change that!!

  
   Ok, if it is needed, I will add residue calculation in the next version.
  Yes this is needed
  
 +static bool fsl_edma_filter_fn(struct dma_chan *chan, void
  *fn_param)
 +{
 + struct fsl_edma_filter_param *fparam = fn_param;
 + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
 +
 + if (fsl_chan-edmamux-mux_id != fparam-mux_id)
 + return false;
 +
 + fsl_chan-slot_id = fparam-slot_id;
 + chan-private = fn_param;
why do you need to use chan-private?
   [Lu Jingchang]
   The private used here is to store the slot_id information, which must
  be used
   by the DMAMUX in alloc_chan_resources function. Thanks.
  Why dont you pass this in struct dma_slave_config memeber slave_id for
  this.
 [Lu Jingchang-b35083] 
 I will drop this private and setup the slave_id directly in the filter 
 function.
why in filter? before calling prepare function you can set the slave config

~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

 -Original Message-
 From: Vinod Koul [mailto:vinod.k...@intel.com]
 Sent: Monday, September 02, 2013 2:37 PM
 To: Lu Jingchang-B35083
 Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
 ker...@lists.infradead.org; devicet...@vger.kernel.org
 Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
 
 On Mon, Sep 02, 2013 at 07:10:53AM +, Lu Jingchang-B35083 wrote:
   -Original Message-
   From: Vinod Koul [mailto:vinod.k...@intel.com]
   Sent: Monday, September 02, 2013 12:51 PM
   To: Lu Jingchang-B35083
   Cc: shawn@linaro.org; linux-kernel@vger.kernel.org; linux-arm-
   ker...@lists.infradead.org; devicet...@vger.kernel.org
   Subject: Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver
 support
  
   On Thu, Aug 29, 2013 at 03:32:04AM +, Lu Jingchang-B35083 wrote:
  
   Please use a right MUA and wrap your lines at 80chars...
 
[Lu Jingchang]
   No need to put your name :)
  [Lu Jingchang-b35083]
  Aha, the Microsoft Outlook adds this automatically with default option.
 You can change that!!
Thanks.
  
  +static bool fsl_edma_filter_fn(struct dma_chan *chan, void
   *fn_param)
  +{
  +   struct fsl_edma_filter_param *fparam = fn_param;
  +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
  +
  +   if (fsl_chan-edmamux-mux_id != fparam-mux_id)
  +   return false;
  +
  +   fsl_chan-slot_id = fparam-slot_id;
  +   chan-private = fn_param;
 why do you need to use chan-private?
[Lu Jingchang]
The private used here is to store the slot_id information, which
 must
   be used
by the DMAMUX in alloc_chan_resources function. Thanks.
   Why dont you pass this in struct dma_slave_config memeber slave_id
 for
   this.
  [Lu Jingchang-b35083]
  I will drop this private and setup the slave_id directly in the filter
 function.
 why in filter? before calling prepare function you can set the slave
 config
How about change the filter_fn to follow:
static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
{
struct fsl_edma_filter_param *fparam = fn_param;
struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
unsigned char val;

if (fsl_chan-edmamux-mux_id != fparam-mux_id)
return false;

val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(fparam-slot_id);
fsl_edmamux_config_chan(fsl_chan, val);
return true;
}
In fact the slot_id isn't need elsewhere, and if the filter return true, 
This channel should be to this request. So no need to save the slave id, Right?



Best Regareds,
Jingchang

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Mon, Sep 02, 2013 at 07:32:53AM +, Lu Jingchang-B35083 wrote:
   + chan-private = fn_param;
  why do you need to use chan-private?
 [Lu Jingchang]
 The private used here is to store the slot_id information, which
  must
be used
 by the DMAMUX in alloc_chan_resources function. Thanks.
Why dont you pass this in struct dma_slave_config memeber slave_id
  for
this.
   [Lu Jingchang-b35083]
   I will drop this private and setup the slave_id directly in the filter
  function.
  why in filter? before calling prepare function you can set the slave
  config
 How about change the filter_fn to follow:
 static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
 {
 struct fsl_edma_filter_param *fparam = fn_param;
 struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
 unsigned char val;
 
 if (fsl_chan-edmamux-mux_id != fparam-mux_id)
 return false;
 
 val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(fparam-slot_id);
 fsl_edmamux_config_chan(fsl_chan, val);
 return true;
 }
 In fact the slot_id isn't need elsewhere, and if the filter return true, 
 This channel should be to this request. So no need to save the slave id, 
 Right?
something like

static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
{
struct fsl_edma_filter_param *fparam = fn_param;
struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);

if (fsl_chan-edmamux-mux_id != fparam-mux_id)
return false;
return true;
}

in thedriver which calls this:

before prep:

config-slave_id = val;

dma_set_slave_config(chan, slave);

~Vinod
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

  How about change the filter_fn to follow:
  static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
  {
  struct fsl_edma_filter_param *fparam = fn_param;
  struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
  unsigned char val;
 
  if (fsl_chan-edmamux-mux_id != fparam-mux_id)
  return false;
 
  val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(fparam-
 slot_id);
  fsl_edmamux_config_chan(fsl_chan, val);
  return true;
  }
  In fact the slot_id isn't need elsewhere, and if the filter return true,
  This channel should be to this request. So no need to save the slave id,
 Right?
 something like
 
 static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
 {
   struct fsl_edma_filter_param *fparam = fn_param;
   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
 
   if (fsl_chan-edmamux-mux_id != fparam-mux_id)
   return false;
   return true;
 }
 
 in thedriver which calls this:
 
 before prep:
 
   config-slave_id = val;
 
   dma_set_slave_config(chan, slave);
 
  Do you mean the DMA_SLAVE_CONFIG device_control? Yeah, the slave driver could 
pass
the slave_id. But the DMA_SLAVE_CONFIG may be called more than once, and the 
eDMA
driver just needs to set the slave id once for any given channel, after that 
the 
transfer is transparent to the device. 
  On the other hand, the DMAMUX's setting procedure requires first disable the 
dmamux
before setting, then if it is set in DMA_SLAVE_CONFIG, the repeated setting may 
be
complex and unnecessary. The channel is occupied exclusively by the peripheral.
  So, according the HW feature, I think the eDMA needs only set the slave id 
once,
and since the of_dma helper has pass the slave id in on xlate, we can get and 
set
the slave id here. How do you think about this?
  Thanks!









Best Regards,
Jingchang

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Thu, Aug 29, 2013 at 03:32:04AM +, Lu Jingchang-B35083 wrote:

Please use a right MUA and wrap your lines at 80chars...

> 
> > 
> > > + return -EINVAL;
> > > + }
> > > + return 0;
> > > +
> > > + default:
> > > + return -ENXIO;
> > > + }
> > > +}
> > > +
> > > +static enum dma_status fsl_edma_tx_status(struct dma_chan *chan,
> > > + dma_cookie_t cookie, struct dma_tx_state *txstate)
> > > +{
> > > + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > > +
> > > + if (fsl_chan->status == DMA_ERROR)
> > > + return DMA_ERROR;
> > > +
> > > + return dma_cookie_status(chan, cookie, txstate);
> > this will tell if the DMA is completed or not only.
> > You also need to calculate residue for the pending dma
> > 
> > Since you support cyclic this should be done properly...
> > 
> > also you cna take more help from vchan support to make your life
> > simpler...
> [Lu Jingchang]
No need to put your name :)
 
> Ok, if it is needed, I will add residue calculation in the next version. 
Yes this is needed

> > > +static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
> > > +{
> > > + struct fsl_edma_filter_param *fparam = fn_param;
> > > + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > > +
> > > + if (fsl_chan->edmamux->mux_id != fparam->mux_id)
> > > + return false;
> > > +
> > > + fsl_chan->slot_id = fparam->slot_id;
> > > + chan->private = fn_param;
> > why do you need to use chan->private?
> [Lu Jingchang] 
> The private used here is to store the slot_id information, which must be used
> by the DMAMUX in alloc_chan_resources function. Thanks.
Why dont you pass this in struct dma_slave_config memeber slave_id for this.

> 
> > > + return true;
> > > +}
> > > +
> > > +static struct dma_chan *fsl_edma_xlate(struct of_phandle_args
> > *dma_spec,
> > > + struct of_dma *ofdma)
> > > +{
> > > + dma_cap_mask_t mask;
> > > + struct fsl_edma_filter_param fparam;
> > > +
> > > + if (dma_spec->args_count != 2)
> > > + return NULL;
> > > +
> > > + fparam.mux_id = dma_spec->args[0];
> > > + fparam.slot_id = dma_spec->args[1];
> > > +
> > > + dma_cap_zero(mask);
> > > + dma_cap_set(DMA_SLAVE, mask);
> > > + dma_cap_set(DMA_CYCLIC, mask);
> > > + return dma_request_channel(mask, fsl_edma_filter_fn, (void
> > *));
> > > +}
> > > +
> > > +static int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
> > > +{
> > > + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > > + struct fsl_edma_filter_param *data = chan->private;
> > > + unsigned char val;
> > > +
> > > + val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(data->slot_id);
> > okay, so what does the slot_id mean?
> >
> [Lu Jingchang] 
> slot_id is the request source id, a peripheral device ID. Each peripheral
> using DMA has a ID that can be identified by the DMA engine. the peripheral ID
> must be written to the DMAMUX configuration register to route the peripheral
> to DMA engine channel. Thanks.
And thanks makes me belive you dont need this way, slave_id is the thing for
you...

~Vinod

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Thu, Aug 29, 2013 at 03:32:04AM +, Lu Jingchang-B35083 wrote:

Please use a right MUA and wrap your lines at 80chars...

 
  
   + return -EINVAL;
   + }
   + return 0;
   +
   + default:
   + return -ENXIO;
   + }
   +}
   +
   +static enum dma_status fsl_edma_tx_status(struct dma_chan *chan,
   + dma_cookie_t cookie, struct dma_tx_state *txstate)
   +{
   + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
   +
   + if (fsl_chan-status == DMA_ERROR)
   + return DMA_ERROR;
   +
   + return dma_cookie_status(chan, cookie, txstate);
  this will tell if the DMA is completed or not only.
  You also need to calculate residue for the pending dma
  
  Since you support cyclic this should be done properly...
  
  also you cna take more help from vchan support to make your life
  simpler...
 [Lu Jingchang]
No need to put your name :)
 
 Ok, if it is needed, I will add residue calculation in the next version. 
Yes this is needed

   +static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
   +{
   + struct fsl_edma_filter_param *fparam = fn_param;
   + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
   +
   + if (fsl_chan-edmamux-mux_id != fparam-mux_id)
   + return false;
   +
   + fsl_chan-slot_id = fparam-slot_id;
   + chan-private = fn_param;
  why do you need to use chan-private?
 [Lu Jingchang] 
 The private used here is to store the slot_id information, which must be used
 by the DMAMUX in alloc_chan_resources function. Thanks.
Why dont you pass this in struct dma_slave_config memeber slave_id for this.

 
   + return true;
   +}
   +
   +static struct dma_chan *fsl_edma_xlate(struct of_phandle_args
  *dma_spec,
   + struct of_dma *ofdma)
   +{
   + dma_cap_mask_t mask;
   + struct fsl_edma_filter_param fparam;
   +
   + if (dma_spec-args_count != 2)
   + return NULL;
   +
   + fparam.mux_id = dma_spec-args[0];
   + fparam.slot_id = dma_spec-args[1];
   +
   + dma_cap_zero(mask);
   + dma_cap_set(DMA_SLAVE, mask);
   + dma_cap_set(DMA_CYCLIC, mask);
   + return dma_request_channel(mask, fsl_edma_filter_fn, (void
  *)fparam);
   +}
   +
   +static int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
   +{
   + struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
   + struct fsl_edma_filter_param *data = chan-private;
   + unsigned char val;
   +
   + val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(data-slot_id);
  okay, so what does the slot_id mean?
 
 [Lu Jingchang] 
 slot_id is the request source id, a peripheral device ID. Each peripheral
 using DMA has a ID that can be identified by the DMA engine. the peripheral ID
 must be written to the DMAMUX configuration register to route the peripheral
 to DMA engine channel. Thanks.
And thanks makes me belive you dont need this way, slave_id is the thing for
you...

~Vinod

-- 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

> > +   } else {
> since you support cyclic, is there a reasonw why you dont support
> pause/resume?
> is it hw issue or planned in future?
[Lu Jingchang] 
The HW supports start/stop request from the peripheral dma request, I had 
planned to add this in future as requested.
I will add this in the next version patch.
Thanks.

> 
> > +   return -EINVAL;
> > +   }
> > +   return 0;
> > +
> > +   default:
> > +   return -ENXIO;
> > +   }
> > +}
> > +
> > +static enum dma_status fsl_edma_tx_status(struct dma_chan *chan,
> > +   dma_cookie_t cookie, struct dma_tx_state *txstate)
> > +{
> > +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > +
> > +   if (fsl_chan->status == DMA_ERROR)
> > +   return DMA_ERROR;
> > +
> > +   return dma_cookie_status(chan, cookie, txstate);
> this will tell if the DMA is completed or not only.
> You also need to calculate residue for the pending dma
> 
> Since you support cyclic this should be done properly...
> 
> also you cna take more help from vchan support to make your life
> simpler...
[Lu Jingchang] 
Ok, if it is needed, I will add residue calculation in the next version. 
Thanks.
> 
[...]
> > +static struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
> > +   struct dma_chan *chan, dma_addr_t dma_addr, size_t buf_len,
> > +   size_t period_len, enum dma_transfer_direction direction,
> > +   unsigned long flags, void *context)
> > +{
> > +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > +   struct fsl_edma_desc *fsl_desc;
> > +   dma_addr_t dma_buf_next;
> > +   int sg_len, i;
> > +   u32 src_addr, dst_addr, last_sg, nbytes;
> > +   u16 soff, doff, iter;
> > +
> > +   if (!fsl_chan->fsc.dir == DMA_MEM_TO_DEV &&
> > +   !fsl_chan->fsc.dir == DMA_DEV_TO_MEM)
> > +   return NULL;
> you are ignoring the direction arg. Also use is_slave_direction() helper

[...]

> > +   if (!fsl_chan->fsc.dir == DMA_MEM_TO_DEV &&
> > +   !fsl_chan->fsc.dir == DMA_DEV_TO_MEM)
> > +   return NULL;
> ditto
[Lu Jingchang] 
Thanks. I will call the helper function instead.


[...]

> > +static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
> > +{
> > +   struct fsl_edma_filter_param *fparam = fn_param;
> > +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > +
> > +   if (fsl_chan->edmamux->mux_id != fparam->mux_id)
> > +   return false;
> > +
> > +   fsl_chan->slot_id = fparam->slot_id;
> > +   chan->private = fn_param;
> why do you need to use chan->private?
[Lu Jingchang] 
The private used here is to store the slot_id information, which must be used 
by the DMAMUX in alloc_chan_resources function. Thanks.

> > +   return true;
> > +}
> > +
> > +static struct dma_chan *fsl_edma_xlate(struct of_phandle_args
> *dma_spec,
> > +   struct of_dma *ofdma)
> > +{
> > +   dma_cap_mask_t mask;
> > +   struct fsl_edma_filter_param fparam;
> > +
> > +   if (dma_spec->args_count != 2)
> > +   return NULL;
> > +
> > +   fparam.mux_id = dma_spec->args[0];
> > +   fparam.slot_id = dma_spec->args[1];
> > +
> > +   dma_cap_zero(mask);
> > +   dma_cap_set(DMA_SLAVE, mask);
> > +   dma_cap_set(DMA_CYCLIC, mask);
> > +   return dma_request_channel(mask, fsl_edma_filter_fn, (void
> *));
> > +}
> > +
> > +static int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
> > +{
> > +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
> > +   struct fsl_edma_filter_param *data = chan->private;
> > +   unsigned char val;
> > +
> > +   val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(data->slot_id);
> okay, so what does the slot_id mean?
>
[Lu Jingchang] 
slot_id is the request source id, a peripheral device ID. Each peripheral using 
DMA has a ID that can be identified by the DMA engine. the peripheral ID must 
be written to the DMAMUX configuration register to route the peripheral to DMA 
engine channel. Thanks.


Best Regards,
Jingchang
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf��^jǫy�m��@A�a���
0��h���i

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Fri, Aug 16, 2013 at 02:07:54PM +0800, Jingchang Lu wrote:
> Add Freescale enhanced direct memory(eDMA) controller support.
> The eDMA controller deploys DMAMUXs routing DMA request sources(slot)
> to eDMA channels.
> This module can be found on Vybrid and LS-1 SoCs.
> 
+ devicet...@vger.kernel.org, need the binding acked by DT folks...

> Signed-off-by: Alison Wang 
> Signed-off-by: Jingchang Lu 
> ---
> changes in v4:
>   using exact compatible string in binding document.
> 
> changes in v3:
>   handle all pending interrupt one time.
>   add protect lock on dma transfer complete handling.
>   change desc and tcd alloc flag to GFP_NOWAIT.
>   add sanity check and error messages.
> 
> changes in v2:
>   using generic dma-channels property instead of fsl,dma-channel.
>   rename the binding document to fsl-edma.txt.
> 
>  Documentation/devicetree/bindings/dma/fsl-edma.txt |  84 ++
>  drivers/dma/Kconfig|  10 +
>  drivers/dma/Makefile   |   1 +
>  drivers/dma/fsl-edma.c | 855 
> +
>  4 files changed, 950 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/fsl-edma.txt
>  create mode 100644 drivers/dma/fsl-edma.c
> 
> diff --git a/Documentation/devicetree/bindings/dma/fsl-edma.txt 
> b/Documentation/devicetree/bindings/dma/fsl-edma.txt
> new file mode 100644
> index 000..95159da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/fsl-edma.txt
> @@ -0,0 +1,84 @@
> +* Freescale enhanced direct memory access(eDMA) Controller
> +
> +The eDMA engine deploys DMAMUXs routing request sources(slot) to
> +eDMA controller channels.
> +
> +* eDMA Controller
> +Required properties:
> +- compatible :
> +  - "fsl,vf610-edma" for eDMA used similar to that on Vybrid vf610 SoC
> +- reg : Should contain eDMA registers location and length
> +- interrupts : Should contain eDMA interrupt
> +- interrupt-names : Should be "edma-tx" for tx interrupt and
> +  "edma-err" for err interrupt
> +- #dma-cells : Must be <2>.
> +  The first cell specifies the DMAMUX ID. Specific request source
> +  can only be routed by specific DMAMUXs.
> +  the second cell specifies the request source(slot) ID.
> +  See include/dt-bindings/dma/-edma.h for all the supported
> +  request source IDs.
> +- dma-channels : Number of channels supported by the controller
> +- fsl,dma-mux : Phandle of the DMAMUXs deployed by the controller
> +
> +
> +* DMAMUX
> +Required properties:
> +- reg : Should contain DMAMUX registers location and length
> +- fsl,dmamux-id : DMAMUX ID. DMAMUX IDs are unique in each eDMA controller.
> +  inside one eDMA controller, specific request source can only be routed by
> +  one of its DMAMUXs.
> +  However Specific request source may be routed to different eDMA controller,
> +  thus all the DMAMUXs sharing a the same request sources have the same ID.
> +- clocks : Phandle of the clock used by the DMAMUX
> +- clock-names : The clock names
> +
> +Below is the eDMA controller and DMAMUX association, and DMAMUX IDs 
> assignment
> +On Vybrid vf610 SoC, DMAMUX0 and DMAMU3 share the same request source group,
> +and DMAMUX1 and DMAMU2 share the same request source group.
> +
> +eDMA controller  DMAMUXs DMAMUX ID
> +-
> +eDMA0DMAMUX0 0
> + DMAMUX1 1
> +
> +eDMA1DMAMUX2 1
> + DMAMUX3 0
> +
> +Examples:
> +
> +edma0: edma@40018000 {
> + #dma-cells = <2>;
> + compatible = "fsl,vf610-edma";
> + reg = <0x40018000 0x2000>;
> + interrupts = <0 8 0x04>, <0 9 0x04>;
> + interrupt-names = "edma-tx", "edma-err";
> + dma-channels = <32>;
> + fsl,edma-mux = <>, <>;
> +};
> +
> +dmamux0: dmamux@40024000 {
> + reg = <0x40024000 0x1000>;
> + fsl,dmamux-id = <0>;
> + clocks = < VF610_CLK_DMAMUX0>;
> + clock-names = "dmamux";
> +};
> +
> +
> +* DMA clients
> +DMA client drivers that uses the DMA function must use the format described
> +in the dma.txt file, using a three-cell specifier for each channel: a phandle
> +plus two integer cells as described above.
> +
> +Examples:
> +
> +sai2: sai@40031000 {
> + compatible = "fsl,vf610-sai";
> + reg = <0x40031000 0x1000>;
> + interrupts = <0 86 0x04>;
> + clocks = < VF610_CLK_SAI2>;
> + clock-names = "sai";
> + dma-names = "tx", "rx";
> + dmas = < 0 VF610_EDMA_MUXID0_SAI2_TX>,
> + < 0 VF610_EDMA_MUXID0_SAI2_RX>;
> + status = "disabled";
> +};
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 6825957..f9a6e06 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -300,6 +300,16 @@ config DMA_JZ4740
>   select DMA_ENGINE
>   select DMA_VIRTUAL_CHANNELS
>  
> +config FSL_EDMA
> + tristate "Freescale eDMA engine support"
> + depends on OF
> + select 

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

Hi, Vinod,

  Could you please help review this patch? Could it be merged into your next 
tree?
  Thanks!


Best Regards,
Jingchang


> -Original Message-
> From: Lu Jingchang-B35083
> Sent: Friday, August 16, 2013 2:08 PM
> To: vinod.k...@intel.com
> Cc: d...@fb.com; shawn@linaro.org; linux-kernel@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org; Lu Jingchang-B35083; Wang Huan-
> B18965
> Subject: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
> 
> Add Freescale enhanced direct memory(eDMA) controller support.
> The eDMA controller deploys DMAMUXs routing DMA request sources(slot) to
> eDMA channels.
> This module can be found on Vybrid and LS-1 SoCs.
> 
> Signed-off-by: Alison Wang 
> Signed-off-by: Jingchang Lu 
> ---
> changes in v4:
>   using exact compatible string in binding document.
> 
> changes in v3:
>   handle all pending interrupt one time.
>   add protect lock on dma transfer complete handling.
>   change desc and tcd alloc flag to GFP_NOWAIT.
>   add sanity check and error messages.
> 
> changes in v2:
>   using generic dma-channels property instead of fsl,dma-channel.
>   rename the binding document to fsl-edma.txt.
> 
>  Documentation/devicetree/bindings/dma/fsl-edma.txt |  84 ++
>  drivers/dma/Kconfig|  10 +
>  drivers/dma/Makefile   |   1 +
>  drivers/dma/fsl-edma.c | 855
> +
>  4 files changed, 950 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/dma/fsl-edma.txt
>  create mode 100644 drivers/dma/fsl-edma.c
> 
> diff --git a/Documentation/devicetree/bindings/dma/fsl-edma.txt
> b/Documentation/devicetree/bindings/dma/fsl-edma.txt
> new file mode 100644
> index 000..95159da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/fsl-edma.txt
> @@ -0,0 +1,84 @@
> +* Freescale enhanced direct memory access(eDMA) Controller
> +
> +The eDMA engine deploys DMAMUXs routing request sources(slot) to eDMA
> +controller channels.
> +
> +* eDMA Controller
> +Required properties:
> +- compatible :
> +  - "fsl,vf610-edma" for eDMA used similar to that on Vybrid vf610 SoC
> +- reg : Should contain eDMA registers location and length
> +- interrupts : Should contain eDMA interrupt
> +- interrupt-names : Should be "edma-tx" for tx interrupt and
> +  "edma-err" for err interrupt
> +- #dma-cells : Must be <2>.
> +  The first cell specifies the DMAMUX ID. Specific request source
> +  can only be routed by specific DMAMUXs.
> +  the second cell specifies the request source(slot) ID.
> +  See include/dt-bindings/dma/-edma.h for all the supported
> +  request source IDs.
> +- dma-channels : Number of channels supported by the controller
> +- fsl,dma-mux : Phandle of the DMAMUXs deployed by the controller
> +
> +
> +* DMAMUX
> +Required properties:
> +- reg : Should contain DMAMUX registers location and length
> +- fsl,dmamux-id : DMAMUX ID. DMAMUX IDs are unique in each eDMA
> controller.
> +  inside one eDMA controller, specific request source can only be
> +routed by
> +  one of its DMAMUXs.
> +  However Specific request source may be routed to different eDMA
> +controller,
> +  thus all the DMAMUXs sharing a the same request sources have the same
> ID.
> +- clocks : Phandle of the clock used by the DMAMUX
> +- clock-names : The clock names
> +
> +Below is the eDMA controller and DMAMUX association, and DMAMUX IDs
> +assignment On Vybrid vf610 SoC, DMAMUX0 and DMAMU3 share the same
> +request source group, and DMAMUX1 and DMAMU2 share the same request
> source group.
> +
> +eDMA controller  DMAMUXs DMAMUX ID
> +-
> +eDMA0DMAMUX0 0
> + DMAMUX1 1
> +
> +eDMA1DMAMUX2 1
> + DMAMUX3 0
> +
> +Examples:
> +
> +edma0: edma@40018000 {
> + #dma-cells = <2>;
> + compatible = "fsl,vf610-edma";
> + reg = <0x40018000 0x2000>;
> + interrupts = <0 8 0x04>, <0 9 0x04>;
> + interrupt-names = "edma-tx", "edma-err";
> + dma-channels = <32>;
> + fsl,edma-mux = <>, <>; };
> +
> +dmamux0: dmamux@40024000 {
> + reg = <0x40024000 0x1000>;
> + fsl,dmamux-id = <0>;
> + clocks = < VF610_CLK_DMAMUX0>;
> + clock-names = "dmamux";
> +};
> +
> +
> +* DMA clients
> +DMA client drivers that uses the DMA function must use the format
> +described in the dma.txt file, using a three-cell specifier for each
> +channel: a phandle plus two integer cells as described above.
> +
> +Examples:
> +
> +sai2: sai@40031000 {
> + compatible = "fsl,vf610-sai";
> + reg = <0x40031000 0x1000>;
> + interrupts = <0 86 0x04>;
> + clocks = < VF610_CLK_SAI2>;
> + clock-names = "sai";
> + dma-names = "tx", "rx";
> + dmas = < 0 VF610_EDMA_MUXID0_SAI2_TX>,
> + < 0 VF610_EDMA_MUXID0_SAI2_RX>;
> + status = "disabled";
> +};
> diff 

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

Hi, Vinod,

  Could you please help review this patch? Could it be merged into your next 
tree?
  Thanks!


Best Regards,
Jingchang


 -Original Message-
 From: Lu Jingchang-B35083
 Sent: Friday, August 16, 2013 2:08 PM
 To: vinod.k...@intel.com
 Cc: d...@fb.com; shawn@linaro.org; linux-kernel@vger.kernel.org;
 linux-arm-ker...@lists.infradead.org; Lu Jingchang-B35083; Wang Huan-
 B18965
 Subject: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support
 
 Add Freescale enhanced direct memory(eDMA) controller support.
 The eDMA controller deploys DMAMUXs routing DMA request sources(slot) to
 eDMA channels.
 This module can be found on Vybrid and LS-1 SoCs.
 
 Signed-off-by: Alison Wang b18...@freescale.com
 Signed-off-by: Jingchang Lu b35...@freescale.com
 ---
 changes in v4:
   using exact compatible string in binding document.
 
 changes in v3:
   handle all pending interrupt one time.
   add protect lock on dma transfer complete handling.
   change desc and tcd alloc flag to GFP_NOWAIT.
   add sanity check and error messages.
 
 changes in v2:
   using generic dma-channels property instead of fsl,dma-channel.
   rename the binding document to fsl-edma.txt.
 
  Documentation/devicetree/bindings/dma/fsl-edma.txt |  84 ++
  drivers/dma/Kconfig|  10 +
  drivers/dma/Makefile   |   1 +
  drivers/dma/fsl-edma.c | 855
 +
  4 files changed, 950 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/dma/fsl-edma.txt
  create mode 100644 drivers/dma/fsl-edma.c
 
 diff --git a/Documentation/devicetree/bindings/dma/fsl-edma.txt
 b/Documentation/devicetree/bindings/dma/fsl-edma.txt
 new file mode 100644
 index 000..95159da
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/dma/fsl-edma.txt
 @@ -0,0 +1,84 @@
 +* Freescale enhanced direct memory access(eDMA) Controller
 +
 +The eDMA engine deploys DMAMUXs routing request sources(slot) to eDMA
 +controller channels.
 +
 +* eDMA Controller
 +Required properties:
 +- compatible :
 +  - fsl,vf610-edma for eDMA used similar to that on Vybrid vf610 SoC
 +- reg : Should contain eDMA registers location and length
 +- interrupts : Should contain eDMA interrupt
 +- interrupt-names : Should be edma-tx for tx interrupt and
 +  edma-err for err interrupt
 +- #dma-cells : Must be 2.
 +  The first cell specifies the DMAMUX ID. Specific request source
 +  can only be routed by specific DMAMUXs.
 +  the second cell specifies the request source(slot) ID.
 +  See include/dt-bindings/dma/soc-edma.h for all the supported
 +  request source IDs.
 +- dma-channels : Number of channels supported by the controller
 +- fsl,dma-mux : Phandle of the DMAMUXs deployed by the controller
 +
 +
 +* DMAMUX
 +Required properties:
 +- reg : Should contain DMAMUX registers location and length
 +- fsl,dmamux-id : DMAMUX ID. DMAMUX IDs are unique in each eDMA
 controller.
 +  inside one eDMA controller, specific request source can only be
 +routed by
 +  one of its DMAMUXs.
 +  However Specific request source may be routed to different eDMA
 +controller,
 +  thus all the DMAMUXs sharing a the same request sources have the same
 ID.
 +- clocks : Phandle of the clock used by the DMAMUX
 +- clock-names : The clock names
 +
 +Below is the eDMA controller and DMAMUX association, and DMAMUX IDs
 +assignment On Vybrid vf610 SoC, DMAMUX0 and DMAMU3 share the same
 +request source group, and DMAMUX1 and DMAMU2 share the same request
 source group.
 +
 +eDMA controller  DMAMUXs DMAMUX ID
 +-
 +eDMA0DMAMUX0 0
 + DMAMUX1 1
 +
 +eDMA1DMAMUX2 1
 + DMAMUX3 0
 +
 +Examples:
 +
 +edma0: edma@40018000 {
 + #dma-cells = 2;
 + compatible = fsl,vf610-edma;
 + reg = 0x40018000 0x2000;
 + interrupts = 0 8 0x04, 0 9 0x04;
 + interrupt-names = edma-tx, edma-err;
 + dma-channels = 32;
 + fsl,edma-mux = dmamux0, dmamux1; };
 +
 +dmamux0: dmamux@40024000 {
 + reg = 0x40024000 0x1000;
 + fsl,dmamux-id = 0;
 + clocks = clks VF610_CLK_DMAMUX0;
 + clock-names = dmamux;
 +};
 +
 +
 +* DMA clients
 +DMA client drivers that uses the DMA function must use the format
 +described in the dma.txt file, using a three-cell specifier for each
 +channel: a phandle plus two integer cells as described above.
 +
 +Examples:
 +
 +sai2: sai@40031000 {
 + compatible = fsl,vf610-sai;
 + reg = 0x40031000 0x1000;
 + interrupts = 0 86 0x04;
 + clocks = clks VF610_CLK_SAI2;
 + clock-names = sai;
 + dma-names = tx, rx;
 + dmas = edma0 0 VF610_EDMA_MUXID0_SAI2_TX,
 + edma0 0 VF610_EDMA_MUXID0_SAI2_RX;
 + status = disabled;
 +};
 diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index
 6825957..f9a6e06 100644
 --- a/drivers/dma/Kconfig
 +++ 

Re: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Vinod Koul]

On Fri, Aug 16, 2013 at 02:07:54PM +0800, Jingchang Lu wrote:
 Add Freescale enhanced direct memory(eDMA) controller support.
 The eDMA controller deploys DMAMUXs routing DMA request sources(slot)
 to eDMA channels.
 This module can be found on Vybrid and LS-1 SoCs.
 
+ devicet...@vger.kernel.org, need the binding acked by DT folks...

 Signed-off-by: Alison Wang b18...@freescale.com
 Signed-off-by: Jingchang Lu b35...@freescale.com
 ---
 changes in v4:
   using exact compatible string in binding document.
 
 changes in v3:
   handle all pending interrupt one time.
   add protect lock on dma transfer complete handling.
   change desc and tcd alloc flag to GFP_NOWAIT.
   add sanity check and error messages.
 
 changes in v2:
   using generic dma-channels property instead of fsl,dma-channel.
   rename the binding document to fsl-edma.txt.
 
  Documentation/devicetree/bindings/dma/fsl-edma.txt |  84 ++
  drivers/dma/Kconfig|  10 +
  drivers/dma/Makefile   |   1 +
  drivers/dma/fsl-edma.c | 855 
 +
  4 files changed, 950 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/dma/fsl-edma.txt
  create mode 100644 drivers/dma/fsl-edma.c
 
 diff --git a/Documentation/devicetree/bindings/dma/fsl-edma.txt 
 b/Documentation/devicetree/bindings/dma/fsl-edma.txt
 new file mode 100644
 index 000..95159da
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/dma/fsl-edma.txt
 @@ -0,0 +1,84 @@
 +* Freescale enhanced direct memory access(eDMA) Controller
 +
 +The eDMA engine deploys DMAMUXs routing request sources(slot) to
 +eDMA controller channels.
 +
 +* eDMA Controller
 +Required properties:
 +- compatible :
 +  - fsl,vf610-edma for eDMA used similar to that on Vybrid vf610 SoC
 +- reg : Should contain eDMA registers location and length
 +- interrupts : Should contain eDMA interrupt
 +- interrupt-names : Should be edma-tx for tx interrupt and
 +  edma-err for err interrupt
 +- #dma-cells : Must be 2.
 +  The first cell specifies the DMAMUX ID. Specific request source
 +  can only be routed by specific DMAMUXs.
 +  the second cell specifies the request source(slot) ID.
 +  See include/dt-bindings/dma/soc-edma.h for all the supported
 +  request source IDs.
 +- dma-channels : Number of channels supported by the controller
 +- fsl,dma-mux : Phandle of the DMAMUXs deployed by the controller
 +
 +
 +* DMAMUX
 +Required properties:
 +- reg : Should contain DMAMUX registers location and length
 +- fsl,dmamux-id : DMAMUX ID. DMAMUX IDs are unique in each eDMA controller.
 +  inside one eDMA controller, specific request source can only be routed by
 +  one of its DMAMUXs.
 +  However Specific request source may be routed to different eDMA controller,
 +  thus all the DMAMUXs sharing a the same request sources have the same ID.
 +- clocks : Phandle of the clock used by the DMAMUX
 +- clock-names : The clock names
 +
 +Below is the eDMA controller and DMAMUX association, and DMAMUX IDs 
 assignment
 +On Vybrid vf610 SoC, DMAMUX0 and DMAMU3 share the same request source group,
 +and DMAMUX1 and DMAMU2 share the same request source group.
 +
 +eDMA controller  DMAMUXs DMAMUX ID
 +-
 +eDMA0DMAMUX0 0
 + DMAMUX1 1
 +
 +eDMA1DMAMUX2 1
 + DMAMUX3 0
 +
 +Examples:
 +
 +edma0: edma@40018000 {
 + #dma-cells = 2;
 + compatible = fsl,vf610-edma;
 + reg = 0x40018000 0x2000;
 + interrupts = 0 8 0x04, 0 9 0x04;
 + interrupt-names = edma-tx, edma-err;
 + dma-channels = 32;
 + fsl,edma-mux = dmamux0, dmamux1;
 +};
 +
 +dmamux0: dmamux@40024000 {
 + reg = 0x40024000 0x1000;
 + fsl,dmamux-id = 0;
 + clocks = clks VF610_CLK_DMAMUX0;
 + clock-names = dmamux;
 +};
 +
 +
 +* DMA clients
 +DMA client drivers that uses the DMA function must use the format described
 +in the dma.txt file, using a three-cell specifier for each channel: a phandle
 +plus two integer cells as described above.
 +
 +Examples:
 +
 +sai2: sai@40031000 {
 + compatible = fsl,vf610-sai;
 + reg = 0x40031000 0x1000;
 + interrupts = 0 86 0x04;
 + clocks = clks VF610_CLK_SAI2;
 + clock-names = sai;
 + dma-names = tx, rx;
 + dmas = edma0 0 VF610_EDMA_MUXID0_SAI2_TX,
 + edma0 0 VF610_EDMA_MUXID0_SAI2_RX;
 + status = disabled;
 +};
 diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
 index 6825957..f9a6e06 100644
 --- a/drivers/dma/Kconfig
 +++ b/drivers/dma/Kconfig
 @@ -300,6 +300,16 @@ config DMA_JZ4740
   select DMA_ENGINE
   select DMA_VIRTUAL_CHANNELS
  
 +config FSL_EDMA
 + tristate Freescale eDMA engine support
 + depends on OF
 + select DMA_ENGINE
 + select DMA_VIRTUAL_CHANNELS
 + help
 +   Support the Freescale eDMA engine with DMAMUXs managing 

RE: [PATCH v4 3/3] dma: Add Freescale eDMA engine driver support

[Lu Jingchang-B35083]

  +   } else {
 since you support cyclic, is there a reasonw why you dont support
 pause/resume?
 is it hw issue or planned in future?
[Lu Jingchang] 
The HW supports start/stop request from the peripheral dma request, I had 
planned to add this in future as requested.
I will add this in the next version patch.
Thanks.

 
  +   return -EINVAL;
  +   }
  +   return 0;
  +
  +   default:
  +   return -ENXIO;
  +   }
  +}
  +
  +static enum dma_status fsl_edma_tx_status(struct dma_chan *chan,
  +   dma_cookie_t cookie, struct dma_tx_state *txstate)
  +{
  +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
  +
  +   if (fsl_chan-status == DMA_ERROR)
  +   return DMA_ERROR;
  +
  +   return dma_cookie_status(chan, cookie, txstate);
 this will tell if the DMA is completed or not only.
 You also need to calculate residue for the pending dma
 
 Since you support cyclic this should be done properly...
 
 also you cna take more help from vchan support to make your life
 simpler...
[Lu Jingchang] 
Ok, if it is needed, I will add residue calculation in the next version. 
Thanks.
 
[...]
  +static struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
  +   struct dma_chan *chan, dma_addr_t dma_addr, size_t buf_len,
  +   size_t period_len, enum dma_transfer_direction direction,
  +   unsigned long flags, void *context)
  +{
  +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
  +   struct fsl_edma_desc *fsl_desc;
  +   dma_addr_t dma_buf_next;
  +   int sg_len, i;
  +   u32 src_addr, dst_addr, last_sg, nbytes;
  +   u16 soff, doff, iter;
  +
  +   if (!fsl_chan-fsc.dir == DMA_MEM_TO_DEV 
  +   !fsl_chan-fsc.dir == DMA_DEV_TO_MEM)
  +   return NULL;
 you are ignoring the direction arg. Also use is_slave_direction() helper

[...]

  +   if (!fsl_chan-fsc.dir == DMA_MEM_TO_DEV 
  +   !fsl_chan-fsc.dir == DMA_DEV_TO_MEM)
  +   return NULL;
 ditto
[Lu Jingchang] 
Thanks. I will call the helper function instead.


[...]

  +static bool fsl_edma_filter_fn(struct dma_chan *chan, void *fn_param)
  +{
  +   struct fsl_edma_filter_param *fparam = fn_param;
  +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
  +
  +   if (fsl_chan-edmamux-mux_id != fparam-mux_id)
  +   return false;
  +
  +   fsl_chan-slot_id = fparam-slot_id;
  +   chan-private = fn_param;
 why do you need to use chan-private?
[Lu Jingchang] 
The private used here is to store the slot_id information, which must be used 
by the DMAMUX in alloc_chan_resources function. Thanks.

  +   return true;
  +}
  +
  +static struct dma_chan *fsl_edma_xlate(struct of_phandle_args
 *dma_spec,
  +   struct of_dma *ofdma)
  +{
  +   dma_cap_mask_t mask;
  +   struct fsl_edma_filter_param fparam;
  +
  +   if (dma_spec-args_count != 2)
  +   return NULL;
  +
  +   fparam.mux_id = dma_spec-args[0];
  +   fparam.slot_id = dma_spec-args[1];
  +
  +   dma_cap_zero(mask);
  +   dma_cap_set(DMA_SLAVE, mask);
  +   dma_cap_set(DMA_CYCLIC, mask);
  +   return dma_request_channel(mask, fsl_edma_filter_fn, (void
 *)fparam);
  +}
  +
  +static int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
  +{
  +   struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
  +   struct fsl_edma_filter_param *data = chan-private;
  +   unsigned char val;
  +
  +   val = EDMAMUX_CHCFG_ENBL | EDMAMUX_CHCFG_SOURCE(data-slot_id);
 okay, so what does the slot_id mean?

[Lu Jingchang] 
slot_id is the request source id, a peripheral device ID. Each peripheral using 
DMA has a ID that can be identified by the DMA engine. the peripheral ID must 
be written to the DMAMUX configuration register to route the peripheral to DMA 
engine channel. Thanks.


Best Regards,
Jingchang
N�r��yb�X��ǧv�^�)޺{.n�+{zX����ܨ}���Ơz�j:+v���zZ+��+zf���h���~i���z��w���?��)ߢf��^jǫy�m��@A�a���
0��h���i