Message ID | 1458057390-20756-3-git-send-email-jonathanh@nvidia.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
On Tue, Mar 15, 2016 at 03:56:29PM +0000, Jon Hunter wrote: > +static void tegra_adma_request_free(struct tegra_adma_chan *tdc) > +{ > + struct tegra_adma *tdma = tdc->tdma; > + > + if (!tdc->sreq_reserved) > + return; > + > + switch (tdc->sreq_dir) { > + case DMA_MEM_TO_DEV: > + clear_bit(tdc->sreq_index, &tdma->tx_requests_reserved); > + break; empty line here woould be nicer > + ret = dma_cookie_status(dc, cookie, txstate); > + if (ret == DMA_COMPLETE || !txstate) > + return ret; > + > + spin_lock_irqsave(&tdc->vc.lock, flags); > + > + vd = vchan_find_desc(&tdc->vc, cookie); > + if (vd) { > + desc = to_tegra_adma_desc(&vd->tx); > + residual = desc->ch_regs.tc; Here we are filling up residue for desc found in issued list > + } else if (tdc->desc && tdc->desc->vd.tx.cookie == cookie) { > + residual = tegra_adma_get_residue(tdc); Well if it is not issued then why we we need to caluclate, its full size of descriptor > + } else { > + residual = 0; why this? > +static struct dma_async_tx_descriptor *tegra_adma_prep_slave_sg( > + struct dma_chan *dc, struct scatterlist *sgl, unsigned int sg_len, > + enum dma_transfer_direction direction, unsigned long flags, > + void *context) > +{ > + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); > + > + dev_warn(tdc2dev(tdc), "scatter-gather transfers are not supported\n"); > + > + return NULL; > +} Why do we need this placeholder, If you dont support slave_sg dont add this as capability > +static const struct tegra_adma_chip_data tegra210_chip_data = { > + .nr_channels = 22, > +}; why should this be hard coded in kernel and not queried from something like DT? This case seems to be hardware property > + dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); > + dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); > + dma_cap_set(DMA_CYCLIC, tdma->dma_dev.cap_mask); I think you should not set DMA_SLAVE, do you need caps to be exported. I think that should be exported for cyclic too, let me know if that was the issue?
On 05/04/16 22:36, Vinod Koul wrote: > On Tue, Mar 15, 2016 at 03:56:29PM +0000, Jon Hunter wrote: > >> +static void tegra_adma_request_free(struct tegra_adma_chan *tdc) >> +{ >> + struct tegra_adma *tdma = tdc->tdma; >> + >> + if (!tdc->sreq_reserved) >> + return; >> + >> + switch (tdc->sreq_dir) { >> + case DMA_MEM_TO_DEV: >> + clear_bit(tdc->sreq_index, &tdma->tx_requests_reserved); >> + break; > > empty line here woould be nicer OK. >> + ret = dma_cookie_status(dc, cookie, txstate); >> + if (ret == DMA_COMPLETE || !txstate) >> + return ret; >> + >> + spin_lock_irqsave(&tdc->vc.lock, flags); >> + >> + vd = vchan_find_desc(&tdc->vc, cookie); >> + if (vd) { >> + desc = to_tegra_adma_desc(&vd->tx); >> + residual = desc->ch_regs.tc; > > Here we are filling up residue for desc found in issued list > >> + } else if (tdc->desc && tdc->desc->vd.tx.cookie == cookie) { >> + residual = tegra_adma_get_residue(tdc); > > Well if it is not issued then why we we need to caluclate, its full size of > descriptor This is the current/active descriptor which has been removed from the issued list and so it needs to be calculated. >> + } else { >> + residual = 0; > > why this? So either the descriptor is still in the submitted list or it has completed but it has not been marked complete yet. In both cases, we don't have access to the descriptor and so the residue is marked as 0 and we return the current status. Please note this is based upon the omap dma driver (drivers/dma/omap-dma.c) which does the same. Other dma driver do very similar things here as well. >> +static struct dma_async_tx_descriptor *tegra_adma_prep_slave_sg( >> + struct dma_chan *dc, struct scatterlist *sgl, unsigned int sg_len, >> + enum dma_transfer_direction direction, unsigned long flags, >> + void *context) >> +{ >> + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); >> + >> + dev_warn(tdc2dev(tdc), "scatter-gather transfers are not supported\n"); >> + >> + return NULL; >> +} > > Why do we need this placeholder, If you dont support slave_sg dont add this > as capability So, AFAICT, dma_async_device_register() does not check to see if device_prep_slave_sg() is valid AND none of the functions dmaengine_prep_slave_single(), dmaengine_prep_slave_single() and dmaengine_prep_rio_sg() check to see if the function pointer is valid before calling chan->device->device_prep_slave_sg(). So it seems that we always expect this function pointer to be valid. So should the inline functions ensure the function pointer is valid before attempting to call them? If so I can add a patch for this. Otherwise it seems the driver needs a stub. It would be a massive change to add a new capability, say SLAVE_SG, and populate this for all existing drivers. >> +static const struct tegra_adma_chip_data tegra210_chip_data = { >> + .nr_channels = 22, >> +}; > > why should this be hard coded in kernel and not queried from something like > DT? This case seems to be hardware property Originally, I did have this in DT, however, the Tegra maintainers prefer this and this is consistent with the other Tegra DMA driver (see driver/dma/tegra20-apb-dma.c) [0]. >> + dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); >> + dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); >> + dma_cap_set(DMA_CYCLIC, tdma->dma_dev.cap_mask); > > I think you should not set DMA_SLAVE, do you need caps to be exported. I > think that should be exported for cyclic too, let me know if that was the > issue? > Why should I not be setting DMA_SLAVE? Should I not be calling dma_get_any_slave_channel() in the xlate? I think that I do want to set DMA_CYCLIC as well to ensure that we check that the device->device_prep_dma_cyclic() function pointer is populated when registering the DMA controller. Cheers Jon [0] http://marc.info/?l=linux-tegra&m=144423393825053&w=2 -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 11, 2016 at 03:09:05PM +0100, Jon Hunter wrote: > > On 05/04/16 22:36, Vinod Koul wrote: > > On Tue, Mar 15, 2016 at 03:56:29PM +0000, Jon Hunter wrote: > > > >> +static void tegra_adma_request_free(struct tegra_adma_chan *tdc) > >> +{ > >> + struct tegra_adma *tdma = tdc->tdma; > >> + > >> + if (!tdc->sreq_reserved) > >> + return; > >> + > >> + switch (tdc->sreq_dir) { > >> + case DMA_MEM_TO_DEV: > >> + clear_bit(tdc->sreq_index, &tdma->tx_requests_reserved); > >> + break; > > > > empty line here woould be nicer > > OK. > > >> + ret = dma_cookie_status(dc, cookie, txstate); > >> + if (ret == DMA_COMPLETE || !txstate) > >> + return ret; > >> + > >> + spin_lock_irqsave(&tdc->vc.lock, flags); > >> + > >> + vd = vchan_find_desc(&tdc->vc, cookie); > >> + if (vd) { > >> + desc = to_tegra_adma_desc(&vd->tx); > >> + residual = desc->ch_regs.tc; > > > > Here we are filling up residue for desc found in issued list > > > >> + } else if (tdc->desc && tdc->desc->vd.tx.cookie == cookie) { > >> + residual = tegra_adma_get_residue(tdc); > > > > Well if it is not issued then why we we need to caluclate, its full size of > > descriptor > > This is the current/active descriptor which has been removed from the > issued list and so it needs to be calculated. > > >> + } else { > >> + residual = 0; > > > > why this? > > So either the descriptor is still in the submitted list or it has > completed but it has not been marked complete yet. In both cases, we > don't have access to the descriptor and so the residue is marked as 0 > and we return the current status. Please note this is based upon the > omap dma driver (drivers/dma/omap-dma.c) which does the same. Other dma > driver do very similar things here as well. Yes that's right, wanted to esnure people do understand this :) > > >> +static struct dma_async_tx_descriptor *tegra_adma_prep_slave_sg( > >> + struct dma_chan *dc, struct scatterlist *sgl, unsigned int sg_len, > >> + enum dma_transfer_direction direction, unsigned long flags, > >> + void *context) > >> +{ > >> + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); > >> + > >> + dev_warn(tdc2dev(tdc), "scatter-gather transfers are not supported\n"); > >> + > >> + return NULL; > >> +} > > > > Why do we need this placeholder, If you dont support slave_sg dont add this > > as capability > > So, AFAICT, dma_async_device_register() does not check to see if > device_prep_slave_sg() is valid AND none of the functions > dmaengine_prep_slave_single(), dmaengine_prep_slave_single() and > dmaengine_prep_rio_sg() check to see if the function pointer is valid > before calling chan->device->device_prep_slave_sg(). So it seems that we > always expect this function pointer to be valid. > > So should the inline functions ensure the function pointer is valid > before attempting to call them? If so I can add a patch for this. > Otherwise it seems the driver needs a stub. It would be a massive change > to add a new capability, say SLAVE_SG, and populate this for all > existing drivers. No we should check here, it's indeed a miss, not sure why none complained about this. I was assuming this is due to caps not considering cyclic case, so I fixed that up. I will fix these cases too, thanks for reporting > >> +static const struct tegra_adma_chip_data tegra210_chip_data = { > >> + .nr_channels = 22, > >> +}; > > > > why should this be hard coded in kernel and not queried from something like > > DT? This case seems to be hardware property > > Originally, I did have this in DT, however, the Tegra maintainers prefer > this and this is consistent with the other Tegra DMA driver (see > driver/dma/tegra20-apb-dma.c) [0]. But this creates a problem when you have next generation of controller with different channel count! How do we solve then? > > >> + dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); > >> + dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); > >> + dma_cap_set(DMA_CYCLIC, tdma->dma_dev.cap_mask); > > > > I think you should not set DMA_SLAVE, do you need caps to be exported. I > > think that should be exported for cyclic too, let me know if that was the > > issue? > > > > Why should I not be setting DMA_SLAVE? Should I not be calling > dma_get_any_slave_channel() in the xlate? > > I think that I do want to set DMA_CYCLIC as well to ensure that we check > that the device->device_prep_dma_cyclic() function pointer is populated > when registering the DMA controller. Only setting DMA_CYCLIC should do, if you see any issues around that please get back, we cna fix those :)
On 12/04/16 15:10, Vinod Koul wrote: > On Mon, Apr 11, 2016 at 03:09:05PM +0100, Jon Hunter wrote: >> On 05/04/16 22:36, Vinod Koul wrote: >>> On Tue, Mar 15, 2016 at 03:56:29PM +0000, Jon Hunter wrote: [snip] >> So, AFAICT, dma_async_device_register() does not check to see if >> device_prep_slave_sg() is valid AND none of the functions >> dmaengine_prep_slave_single(), dmaengine_prep_slave_single() and >> dmaengine_prep_rio_sg() check to see if the function pointer is valid >> before calling chan->device->device_prep_slave_sg(). So it seems that we >> always expect this function pointer to be valid. >> >> So should the inline functions ensure the function pointer is valid >> before attempting to call them? If so I can add a patch for this. >> Otherwise it seems the driver needs a stub. It would be a massive change >> to add a new capability, say SLAVE_SG, and populate this for all >> existing drivers. > > No we should check here, it's indeed a miss, not sure why none complained > about this. I was assuming this is due to caps not considering cyclic case, > so I fixed that up. > > I will fix these cases too, thanks for reporting Ok, great. >>>> +static const struct tegra_adma_chip_data tegra210_chip_data = { >>>> + .nr_channels = 22, >>>> +}; >>> >>> why should this be hard coded in kernel and not queried from something like >>> DT? This case seems to be hardware property >> >> Originally, I did have this in DT, however, the Tegra maintainers prefer >> this and this is consistent with the other Tegra DMA driver (see >> driver/dma/tegra20-apb-dma.c) [0]. > > But this creates a problem when you have next generation of controller with > different channel count! > How do we solve then? Same way we solve this for the tegra20-apb-dma driver by having different chip data per SoC in the driver ... 1259 /* Tegra20 specific DMA controller information */ 1260 static const struct tegra_dma_chip_data tegra20_dma_chip_data = { 1261 .nr_channels = 16, 1262 .channel_reg_size = 0x20, 1263 .max_dma_count = 1024UL * 64, 1264 .support_channel_pause = false, 1265 .support_separate_wcount_reg = false, 1266 }; 1267 1268 /* Tegra30 specific DMA controller information */ 1269 static const struct tegra_dma_chip_data tegra30_dma_chip_data = { 1270 .nr_channels = 32, 1271 .channel_reg_size = 0x20, 1272 .max_dma_count = 1024UL * 64, 1273 .support_channel_pause = false, 1274 .support_separate_wcount_reg = false, 1275 }; 1276 1277 /* Tegra114 specific DMA controller information */ 1278 static const struct tegra_dma_chip_data tegra114_dma_chip_data = { 1279 .nr_channels = 32, 1280 .channel_reg_size = 0x20, 1281 .max_dma_count = 1024UL * 64, 1282 .support_channel_pause = true, 1283 .support_separate_wcount_reg = false, 1284 }; 1285 1286 /* Tegra148 specific DMA controller information */ 1287 static const struct tegra_dma_chip_data tegra148_dma_chip_data = { 1288 .nr_channels = 32, 1289 .channel_reg_size = 0x40, 1290 .max_dma_count = 1024UL * 64, 1291 .support_channel_pause = true, 1292 .support_separate_wcount_reg = true, 1293 }; You may still say this should be in the DT, but the Tegra maintainers prefer this data in the driver. >>>> + dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); >>>> + dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); >>>> + dma_cap_set(DMA_CYCLIC, tdma->dma_dev.cap_mask); >>> >>> I think you should not set DMA_SLAVE, do you need caps to be exported. I >>> think that should be exported for cyclic too, let me know if that was the >>> issue? >>> >> >> Why should I not be setting DMA_SLAVE? Should I not be calling >> dma_get_any_slave_channel() in the xlate? >> >> I think that I do want to set DMA_CYCLIC as well to ensure that we check >> that the device->device_prep_dma_cyclic() function pointer is populated >> when registering the DMA controller. > > Only setting DMA_CYCLIC should do, if you see any issues around that please > get back, we cna fix those :) It did not work for me because dma_get_any_slave_channel() wants a DMA device with the DMA_SLAVE bit capability set. So if I remove this above, then requesting the channel fails via dma_get_any_slave_channel() fails. Is there something I don't understand here? Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 12, 2016 at 05:23:32PM +0100, Jon Hunter wrote: > >>> why should this be hard coded in kernel and not queried from something like > >>> DT? This case seems to be hardware property > >> > >> Originally, I did have this in DT, however, the Tegra maintainers prefer > >> this and this is consistent with the other Tegra DMA driver (see > >> driver/dma/tegra20-apb-dma.c) [0]. > > > > But this creates a problem when you have next generation of controller with > > different channel count! > > How do we solve then? > > Same way we solve this for the tegra20-apb-dma driver by having > different chip data per SoC in the driver ... > > 1259 /* Tegra20 specific DMA controller information */ > 1260 static const struct tegra_dma_chip_data tegra20_dma_chip_data = { > 1261 .nr_channels = 16, > 1262 .channel_reg_size = 0x20, > 1263 .max_dma_count = 1024UL * 64, > 1264 .support_channel_pause = false, > 1265 .support_separate_wcount_reg = false, > 1266 }; > 1267 > 1268 /* Tegra30 specific DMA controller information */ > 1269 static const struct tegra_dma_chip_data tegra30_dma_chip_data = { > 1270 .nr_channels = 32, > 1271 .channel_reg_size = 0x20, > 1272 .max_dma_count = 1024UL * 64, > 1273 .support_channel_pause = false, > 1274 .support_separate_wcount_reg = false, > 1275 }; > 1276 > 1277 /* Tegra114 specific DMA controller information */ > 1278 static const struct tegra_dma_chip_data tegra114_dma_chip_data = { > 1279 .nr_channels = 32, > 1280 .channel_reg_size = 0x20, > 1281 .max_dma_count = 1024UL * 64, > 1282 .support_channel_pause = true, > 1283 .support_separate_wcount_reg = false, > 1284 }; > 1285 > 1286 /* Tegra148 specific DMA controller information */ > 1287 static const struct tegra_dma_chip_data tegra148_dma_chip_data = { > 1288 .nr_channels = 32, > 1289 .channel_reg_size = 0x40, > 1290 .max_dma_count = 1024UL * 64, > 1291 .support_channel_pause = true, > 1292 .support_separate_wcount_reg = true, > 1293 }; > > You may still say this should be in the DT, but the Tegra maintainers > prefer this data in the driver. Okay I don't see a a rationale behind this not being in DT, Stephan? > > >>>> + dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); > >>>> + dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); > >>>> + dma_cap_set(DMA_CYCLIC, tdma->dma_dev.cap_mask); > >>> > >>> I think you should not set DMA_SLAVE, do you need caps to be exported. I > >>> think that should be exported for cyclic too, let me know if that was the > >>> issue? > >>> > >> > >> Why should I not be setting DMA_SLAVE? Should I not be calling > >> dma_get_any_slave_channel() in the xlate? > >> > >> I think that I do want to set DMA_CYCLIC as well to ensure that we check > >> that the device->device_prep_dma_cyclic() function pointer is populated > >> when registering the DMA controller. > > > > Only setting DMA_CYCLIC should do, if you see any issues around that please > > get back, we cna fix those :) > > It did not work for me because dma_get_any_slave_channel() wants a DMA > device with the DMA_SLAVE bit capability set. So if I remove this above, > then requesting the channel fails via dma_get_any_slave_channel() fails. > Is there something I don't understand here? You should use dma_request_channel() we cleaned up the APIs and recommend to use dma_request_channel() for slave usages. See Documentation update in a8135d0d79e9: (dmaengine: core: Introduce new, universal API to request a channel) Thanks
On 04/13/2016 07:49 AM, Vinod Koul wrote: > On Tue, Apr 12, 2016 at 05:23:32PM +0100, Jon Hunter wrote: >>>>> why should this be hard coded in kernel and not queried from something like >>>>> DT? This case seems to be hardware property >>>> >>>> Originally, I did have this in DT, however, the Tegra maintainers prefer >>>> this and this is consistent with the other Tegra DMA driver (see >>>> driver/dma/tegra20-apb-dma.c) [0]. >>> >>> But this creates a problem when you have next generation of controller with >>> different channel count! >>> How do we solve then? >> >> Same way we solve this for the tegra20-apb-dma driver by having >> different chip data per SoC in the driver ... >> >> 1259 /* Tegra20 specific DMA controller information */ >> 1260 static const struct tegra_dma_chip_data tegra20_dma_chip_data = { >> 1261 .nr_channels = 16, >> 1262 .channel_reg_size = 0x20, >> 1263 .max_dma_count = 1024UL * 64, >> 1264 .support_channel_pause = false, >> 1265 .support_separate_wcount_reg = false, >> 1266 }; >> 1267 >> 1268 /* Tegra30 specific DMA controller information */ >> 1269 static const struct tegra_dma_chip_data tegra30_dma_chip_data = { >> 1270 .nr_channels = 32, >> 1271 .channel_reg_size = 0x20, >> 1272 .max_dma_count = 1024UL * 64, >> 1273 .support_channel_pause = false, >> 1274 .support_separate_wcount_reg = false, >> 1275 }; >> 1276 >> 1277 /* Tegra114 specific DMA controller information */ >> 1278 static const struct tegra_dma_chip_data tegra114_dma_chip_data = { >> 1279 .nr_channels = 32, >> 1280 .channel_reg_size = 0x20, >> 1281 .max_dma_count = 1024UL * 64, >> 1282 .support_channel_pause = true, >> 1283 .support_separate_wcount_reg = false, >> 1284 }; >> 1285 >> 1286 /* Tegra148 specific DMA controller information */ >> 1287 static const struct tegra_dma_chip_data tegra148_dma_chip_data = { >> 1288 .nr_channels = 32, >> 1289 .channel_reg_size = 0x40, >> 1290 .max_dma_count = 1024UL * 64, >> 1291 .support_channel_pause = true, >> 1292 .support_separate_wcount_reg = true, >> 1293 }; >> >> You may still say this should be in the DT, but the Tegra maintainers >> prefer this data in the driver. > > Okay I don't see a a rationale behind this not being in DT, Stephan? The rationale for putting such data into DT is that when a new SoC comes out, someone can simply modify the DT to change the parameter and the existing driver will magically work on the new HW. In practice, at least for HW blocks on Tegra, this doesn't turn out to be true very often at all. We almost /always/ need some change to the driver, because it needs to manipulate some additional clock, program some new register/field, adapt to register/field layout changes, work around bugs differently, etc. As such, expecting driver code changes is the norm not the exception. Given that, we have two choices about how to represent certain data in DT: a) Put (hard-code) the data into the driver, and just use it. Perhaps this data is in a table in order to support different values per SoC, as in the code quoted above. This is very simple. b) Put the data into DT, and write extra driver code to parse it, all just to extract the exact same value we would have hard-coded into the driver. This is useless overhead /unless/ it allows re-use of the driver unchanged on new SoCs, which rarely happens. As such, it's much simpler to choose option (a). If an initial version of a driver hard-codes some value and by magic a new version of the HW comes out for which the only difference is some value like this, then: a) It's likely that the value has increased rather than decreased on new HW, so the existing hard-coded driver can work just as well as on old HW, but simply not exposing all HW capabilities. This won't always be true, but seems just as reasonable to expect as expecting the driver won't need modifications for any other reason. b) At the time we add driver support for the new chip, we can define a new DT property that represents the previously hard-coded data. If the property is present, its value is used. Otherwise the previous hard-coded default can still be used. This allows us to retrospectively realise that a DT property would have been a good idea, yet not bog the code down with using the DT property until it is definitively known to be useful. Now, my points above all refer to Tegra HW blocks. Hopefully they don't apply to IP vendors who sell their block to various SoC vendors. There, it's much more obviously beneficial to allow DT-based configuration of any IP block parameters, since different SoC vendors will almost certainly pick different configuration values if it's possible to do so. Note that this is more about different instances (parameterizations) of the same version of an IP block though. Equally,, my points apply to on-SoC HW blocks rather than anything board specific. Board designs very obviously vary a lot, and it's obviously beneficial to use DT to represent those differences (for consumption by a generic OS; there's not so much benefit when talking about firmware consuming DT). -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 13/04/16 14:49, Vinod Koul wrote: > On Tue, Apr 12, 2016 at 05:23:32PM +0100, Jon Hunter wrote: >>>>> why should this be hard coded in kernel and not queried from something like >>>>> DT? This case seems to be hardware property >>>> >>>> Originally, I did have this in DT, however, the Tegra maintainers prefer >>>> this and this is consistent with the other Tegra DMA driver (see >>>> driver/dma/tegra20-apb-dma.c) [0]. >>> >>> But this creates a problem when you have next generation of controller with >>> different channel count! >>> How do we solve then? >> >> Same way we solve this for the tegra20-apb-dma driver by having >> different chip data per SoC in the driver ... >> >> 1259 /* Tegra20 specific DMA controller information */ >> 1260 static const struct tegra_dma_chip_data tegra20_dma_chip_data = { >> 1261 .nr_channels = 16, >> 1262 .channel_reg_size = 0x20, >> 1263 .max_dma_count = 1024UL * 64, >> 1264 .support_channel_pause = false, >> 1265 .support_separate_wcount_reg = false, >> 1266 }; >> 1267 >> 1268 /* Tegra30 specific DMA controller information */ >> 1269 static const struct tegra_dma_chip_data tegra30_dma_chip_data = { >> 1270 .nr_channels = 32, >> 1271 .channel_reg_size = 0x20, >> 1272 .max_dma_count = 1024UL * 64, >> 1273 .support_channel_pause = false, >> 1274 .support_separate_wcount_reg = false, >> 1275 }; >> 1276 >> 1277 /* Tegra114 specific DMA controller information */ >> 1278 static const struct tegra_dma_chip_data tegra114_dma_chip_data = { >> 1279 .nr_channels = 32, >> 1280 .channel_reg_size = 0x20, >> 1281 .max_dma_count = 1024UL * 64, >> 1282 .support_channel_pause = true, >> 1283 .support_separate_wcount_reg = false, >> 1284 }; >> 1285 >> 1286 /* Tegra148 specific DMA controller information */ >> 1287 static const struct tegra_dma_chip_data tegra148_dma_chip_data = { >> 1288 .nr_channels = 32, >> 1289 .channel_reg_size = 0x40, >> 1290 .max_dma_count = 1024UL * 64, >> 1291 .support_channel_pause = true, >> 1292 .support_separate_wcount_reg = true, >> 1293 }; >> >> You may still say this should be in the DT, but the Tegra maintainers >> prefer this data in the driver. > > Okay I don't see a a rationale behind this not being in DT, Stephan? Let me know what you think of Stephen's feedback and if you are OK with this. >>>>>> + dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); >>>>>> + dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); >>>>>> + dma_cap_set(DMA_CYCLIC, tdma->dma_dev.cap_mask); >>>>> >>>>> I think you should not set DMA_SLAVE, do you need caps to be exported. I >>>>> think that should be exported for cyclic too, let me know if that was the >>>>> issue? >>>>> >>>> >>>> Why should I not be setting DMA_SLAVE? Should I not be calling >>>> dma_get_any_slave_channel() in the xlate? >>>> >>>> I think that I do want to set DMA_CYCLIC as well to ensure that we check >>>> that the device->device_prep_dma_cyclic() function pointer is populated >>>> when registering the DMA controller. >>> >>> Only setting DMA_CYCLIC should do, if you see any issues around that please >>> get back, we cna fix those :) >> >> It did not work for me because dma_get_any_slave_channel() wants a DMA >> device with the DMA_SLAVE bit capability set. So if I remove this above, >> then requesting the channel fails via dma_get_any_slave_channel() fails. >> Is there something I don't understand here? > > You should use dma_request_channel() we cleaned up the APIs and recommend to > use dma_request_channel() for slave usages. See Documentation update in > a8135d0d79e9: (dmaengine: core: Introduce new, universal API to request a > channel) I had a look at this, but actually, I don't think this is going to work. Looking at dma_request_channel(), it is going to get a DMA channel that matches the mask for any DMA controller. In the xlate I already know which DMA controller I am and I just want one of the channels. The flow here is ... dma_request_chan() --> of_dma_request_slave_channel() --> xlate() --> dma_get_any_slave_channel() There are several other DMA drivers that are calling dma_get_any_slave_channel() from their xlate function which makes sense because they are requesting one of their own channels. I can understand that you wish to consolidate the APIs for requesting a channel, but it seems to me that you still need to have an API that DMA controller drivers can call where they can pass their dma_device structure to ensure you get a channel for the appropriate DMA controller. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Vinod, Any more comments here? Or are you ok? Cheers Jon On 14/04/16 12:04, Jon Hunter wrote: > > On 13/04/16 14:49, Vinod Koul wrote: >> On Tue, Apr 12, 2016 at 05:23:32PM +0100, Jon Hunter wrote: >>>>>> why should this be hard coded in kernel and not queried from something like >>>>>> DT? This case seems to be hardware property >>>>> >>>>> Originally, I did have this in DT, however, the Tegra maintainers prefer >>>>> this and this is consistent with the other Tegra DMA driver (see >>>>> driver/dma/tegra20-apb-dma.c) [0]. >>>> >>>> But this creates a problem when you have next generation of controller with >>>> different channel count! >>>> How do we solve then? >>> >>> Same way we solve this for the tegra20-apb-dma driver by having >>> different chip data per SoC in the driver ... >>> >>> 1259 /* Tegra20 specific DMA controller information */ >>> 1260 static const struct tegra_dma_chip_data tegra20_dma_chip_data = { >>> 1261 .nr_channels = 16, >>> 1262 .channel_reg_size = 0x20, >>> 1263 .max_dma_count = 1024UL * 64, >>> 1264 .support_channel_pause = false, >>> 1265 .support_separate_wcount_reg = false, >>> 1266 }; >>> 1267 >>> 1268 /* Tegra30 specific DMA controller information */ >>> 1269 static const struct tegra_dma_chip_data tegra30_dma_chip_data = { >>> 1270 .nr_channels = 32, >>> 1271 .channel_reg_size = 0x20, >>> 1272 .max_dma_count = 1024UL * 64, >>> 1273 .support_channel_pause = false, >>> 1274 .support_separate_wcount_reg = false, >>> 1275 }; >>> 1276 >>> 1277 /* Tegra114 specific DMA controller information */ >>> 1278 static const struct tegra_dma_chip_data tegra114_dma_chip_data = { >>> 1279 .nr_channels = 32, >>> 1280 .channel_reg_size = 0x20, >>> 1281 .max_dma_count = 1024UL * 64, >>> 1282 .support_channel_pause = true, >>> 1283 .support_separate_wcount_reg = false, >>> 1284 }; >>> 1285 >>> 1286 /* Tegra148 specific DMA controller information */ >>> 1287 static const struct tegra_dma_chip_data tegra148_dma_chip_data = { >>> 1288 .nr_channels = 32, >>> 1289 .channel_reg_size = 0x40, >>> 1290 .max_dma_count = 1024UL * 64, >>> 1291 .support_channel_pause = true, >>> 1292 .support_separate_wcount_reg = true, >>> 1293 }; >>> >>> You may still say this should be in the DT, but the Tegra maintainers >>> prefer this data in the driver. >> >> Okay I don't see a a rationale behind this not being in DT, Stephan? > > Let me know what you think of Stephen's feedback and if you are OK with > this. > >>>>>>> + dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); >>>>>>> + dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); >>>>>>> + dma_cap_set(DMA_CYCLIC, tdma->dma_dev.cap_mask); >>>>>> >>>>>> I think you should not set DMA_SLAVE, do you need caps to be exported. I >>>>>> think that should be exported for cyclic too, let me know if that was the >>>>>> issue? >>>>>> >>>>> >>>>> Why should I not be setting DMA_SLAVE? Should I not be calling >>>>> dma_get_any_slave_channel() in the xlate? >>>>> >>>>> I think that I do want to set DMA_CYCLIC as well to ensure that we check >>>>> that the device->device_prep_dma_cyclic() function pointer is populated >>>>> when registering the DMA controller. >>>> >>>> Only setting DMA_CYCLIC should do, if you see any issues around that please >>>> get back, we cna fix those :) >>> >>> It did not work for me because dma_get_any_slave_channel() wants a DMA >>> device with the DMA_SLAVE bit capability set. So if I remove this above, >>> then requesting the channel fails via dma_get_any_slave_channel() fails. >>> Is there something I don't understand here? >> >> You should use dma_request_channel() we cleaned up the APIs and recommend to >> use dma_request_channel() for slave usages. See Documentation update in >> a8135d0d79e9: (dmaengine: core: Introduce new, universal API to request a >> channel) > > I had a look at this, but actually, I don't think this is going to work. > > Looking at dma_request_channel(), it is going to get a DMA channel that > matches the mask for any DMA controller. In the xlate I already know > which DMA controller I am and I just want one of the channels. The flow > here is ... > > dma_request_chan() > --> of_dma_request_slave_channel() > --> xlate() > --> dma_get_any_slave_channel() > > There are several other DMA drivers that are calling > dma_get_any_slave_channel() from their xlate function which makes sense > because they are requesting one of their own channels. > > I can understand that you wish to consolidate the APIs for requesting a > channel, but it seems to me that you still need to have an API that DMA > controller drivers can call where they can pass their dma_device > structure to ensure you get a channel for the appropriate DMA controller. > > Cheers > Jon > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 18, 2016 at 04:06:23PM +0100, Jon Hunter wrote: > On 14/04/16 12:04, Jon Hunter wrote: > > > > On 13/04/16 14:49, Vinod Koul wrote: > >> On Tue, Apr 12, 2016 at 05:23:32PM +0100, Jon Hunter wrote: > >>>>>> why should this be hard coded in kernel and not queried from something like > >>>>>> DT? This case seems to be hardware property > >>>>> > >>>>> Originally, I did have this in DT, however, the Tegra maintainers prefer > >>>>> this and this is consistent with the other Tegra DMA driver (see > >>>>> driver/dma/tegra20-apb-dma.c) [0]. > >>>> > >>>> But this creates a problem when you have next generation of controller with > >>>> different channel count! > >>>> How do we solve then? > >>> > >>> Same way we solve this for the tegra20-apb-dma driver by having > >>> different chip data per SoC in the driver ... > >>> > >>> 1259 /* Tegra20 specific DMA controller information */ > >>> 1260 static const struct tegra_dma_chip_data tegra20_dma_chip_data = { > >>> 1261 .nr_channels = 16, > >>> 1262 .channel_reg_size = 0x20, > >>> 1263 .max_dma_count = 1024UL * 64, > >>> 1264 .support_channel_pause = false, > >>> 1265 .support_separate_wcount_reg = false, > >>> 1266 }; > >>> 1267 > >>> 1268 /* Tegra30 specific DMA controller information */ > >>> 1269 static const struct tegra_dma_chip_data tegra30_dma_chip_data = { > >>> 1270 .nr_channels = 32, > >>> 1271 .channel_reg_size = 0x20, > >>> 1272 .max_dma_count = 1024UL * 64, > >>> 1273 .support_channel_pause = false, > >>> 1274 .support_separate_wcount_reg = false, > >>> 1275 }; > >>> 1276 > >>> 1277 /* Tegra114 specific DMA controller information */ > >>> 1278 static const struct tegra_dma_chip_data tegra114_dma_chip_data = { > >>> 1279 .nr_channels = 32, > >>> 1280 .channel_reg_size = 0x20, > >>> 1281 .max_dma_count = 1024UL * 64, > >>> 1282 .support_channel_pause = true, > >>> 1283 .support_separate_wcount_reg = false, > >>> 1284 }; > >>> 1285 > >>> 1286 /* Tegra148 specific DMA controller information */ > >>> 1287 static const struct tegra_dma_chip_data tegra148_dma_chip_data = { > >>> 1288 .nr_channels = 32, > >>> 1289 .channel_reg_size = 0x40, > >>> 1290 .max_dma_count = 1024UL * 64, > >>> 1291 .support_channel_pause = true, > >>> 1292 .support_separate_wcount_reg = true, > >>> 1293 }; > >>> > >>> You may still say this should be in the DT, but the Tegra maintainers > >>> prefer this data in the driver. > >> > >> Okay I don't see a a rationale behind this not being in DT, Stephan? > > > > Let me know what you think of Stephen's feedback and if you are OK with > > this. > > > >>>>>>> + dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); > >>>>>>> + dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); > >>>>>>> + dma_cap_set(DMA_CYCLIC, tdma->dma_dev.cap_mask); > >>>>>> > >>>>>> I think you should not set DMA_SLAVE, do you need caps to be exported. I > >>>>>> think that should be exported for cyclic too, let me know if that was the > >>>>>> issue? > >>>>>> > >>>>> > >>>>> Why should I not be setting DMA_SLAVE? Should I not be calling > >>>>> dma_get_any_slave_channel() in the xlate? > >>>>> > >>>>> I think that I do want to set DMA_CYCLIC as well to ensure that we check > >>>>> that the device->device_prep_dma_cyclic() function pointer is populated > >>>>> when registering the DMA controller. > >>>> > >>>> Only setting DMA_CYCLIC should do, if you see any issues around that please > >>>> get back, we cna fix those :) > >>> > >>> It did not work for me because dma_get_any_slave_channel() wants a DMA > >>> device with the DMA_SLAVE bit capability set. So if I remove this above, > >>> then requesting the channel fails via dma_get_any_slave_channel() fails. > >>> Is there something I don't understand here? > >> > >> You should use dma_request_channel() we cleaned up the APIs and recommend to > >> use dma_request_channel() for slave usages. See Documentation update in > >> a8135d0d79e9: (dmaengine: core: Introduce new, universal API to request a > >> channel) > > > > I had a look at this, but actually, I don't think this is going to work. > > > > Looking at dma_request_channel(), it is going to get a DMA channel that > > matches the mask for any DMA controller. In the xlate I already know > > which DMA controller I am and I just want one of the channels. The flow > > here is ... > > > > dma_request_chan() > > --> of_dma_request_slave_channel() > > --> xlate() > > --> dma_get_any_slave_channel() > > > > There are several other DMA drivers that are calling > > dma_get_any_slave_channel() from their xlate function which makes sense > > because they are requesting one of their own channels. > > > > I can understand that you wish to consolidate the APIs for requesting a > > channel, but it seems to me that you still need to have an API that DMA > > controller drivers can call where they can pass their dma_device > > structure to ensure you get a channel for the appropriate DMA controller. Yes but the idea was that xlate will help you to get the right channel. The whole dmaengine property was supposed to help you with that Please check the omap code which gets a specfic channel using this Thanks
On 19/04/16 14:25, Vinod Koul wrote: > On Mon, Apr 18, 2016 at 04:06:23PM +0100, Jon Hunter wrote: >> On 14/04/16 12:04, Jon Hunter wrote: [snip] >>> I had a look at this, but actually, I don't think this is going to work. >>> >>> Looking at dma_request_channel(), it is going to get a DMA channel that >>> matches the mask for any DMA controller. In the xlate I already know >>> which DMA controller I am and I just want one of the channels. The flow >>> here is ... >>> >>> dma_request_chan() >>> --> of_dma_request_slave_channel() >>> --> xlate() >>> --> dma_get_any_slave_channel() >>> >>> There are several other DMA drivers that are calling >>> dma_get_any_slave_channel() from their xlate function which makes sense >>> because they are requesting one of their own channels. >>> >>> I can understand that you wish to consolidate the APIs for requesting a >>> channel, but it seems to me that you still need to have an API that DMA >>> controller drivers can call where they can pass their dma_device >>> structure to ensure you get a channel for the appropriate DMA controller. > > Yes but the idea was that xlate will help you to get the right channel. The > whole dmaengine property was supposed to help you with that Well it depends on the DMA controller. In the case of tegra the xlate helps you extract the slave request ID for a given device. However, because any channel can be used with any slave request ID, we don't care about the exact channel. So we request any available channel for the DMA controller in question and program it with the slave request we got from the xlate. > Please check the omap code which gets a specfic channel using this Yes I have already seen this and it does not help here, because OMAP uses of_dma_simple_xlate() which calls dma_request_channel() and again this will get a channel from any DMA controller that matches the caps flags. For tegra we have two DMA controllers and both support CYCLIC transfers and so I am concerned we would get a channel from the wrong DMA controller. I have seen some DMA drivers (such as imx-dma) use the filter function to match the DMA controller, but it seems daft to use the filter function to match the DMA controller when we already know in the xlate which DMA controller we are. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 19 April 2016 14:59:49 Jon Hunter wrote: > On 19/04/16 14:25, Vinod Koul wrote: > > On Mon, Apr 18, 2016 at 04:06:23PM +0100, Jon Hunter wrote: > >> On 14/04/16 12:04, Jon Hunter wrote: > >>> I had a look at this, but actually, I don't think this is going to work. > >>> > >>> Looking at dma_request_channel(), it is going to get a DMA channel that > >>> matches the mask for any DMA controller. In the xlate I already know > >>> which DMA controller I am and I just want one of the channels. The flow > >>> here is ... > >>> > >>> dma_request_chan() > >>> --> of_dma_request_slave_channel() > >>> --> xlate() > >>> --> dma_get_any_slave_channel() > >>> > >>> There are several other DMA drivers that are calling > >>> dma_get_any_slave_channel() from their xlate function which makes sense > >>> because they are requesting one of their own channels. > >>> > >>> I can understand that you wish to consolidate the APIs for requesting a > >>> channel, but it seems to me that you still need to have an API that DMA > >>> controller drivers can call where they can pass their dma_device > >>> structure to ensure you get a channel for the appropriate DMA controller. > > > > Yes but the idea was that xlate will help you to get the right channel. The > > whole dmaengine property was supposed to help you with that > > Well it depends on the DMA controller. In the case of tegra the xlate > helps you extract the slave request ID for a given device. However, > because any channel can be used with any slave request ID, we don't care > about the exact channel. So we request any available channel for the DMA > controller in question and program it with the slave request we got from > the xlate. Right. the cleanup was supposed to reduce the number of interfaces that a slave driver can call and consolidate them as much as possible into dma_request_chan(), but we still need dma_get_any_slave_channel() as an interface for the dmaengine masters as you said. > I have seen some DMA drivers (such as imx-dma) use the filter function > to match the DMA controller, but it seems daft to use the filter > function to match the DMA controller when we already know in the xlate > which DMA controller we are. The filter functions should ideally all go away after the cleanup is complete. Arnd -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 19/04/16 15:18, Arnd Bergmann wrote: > On Tuesday 19 April 2016 14:59:49 Jon Hunter wrote: >> On 19/04/16 14:25, Vinod Koul wrote: >>> On Mon, Apr 18, 2016 at 04:06:23PM +0100, Jon Hunter wrote: >>>> On 14/04/16 12:04, Jon Hunter wrote: >>>>> I had a look at this, but actually, I don't think this is going to work. >>>>> >>>>> Looking at dma_request_channel(), it is going to get a DMA channel that >>>>> matches the mask for any DMA controller. In the xlate I already know >>>>> which DMA controller I am and I just want one of the channels. The flow >>>>> here is ... >>>>> >>>>> dma_request_chan() >>>>> --> of_dma_request_slave_channel() >>>>> --> xlate() >>>>> --> dma_get_any_slave_channel() >>>>> >>>>> There are several other DMA drivers that are calling >>>>> dma_get_any_slave_channel() from their xlate function which makes sense >>>>> because they are requesting one of their own channels. >>>>> >>>>> I can understand that you wish to consolidate the APIs for requesting a >>>>> channel, but it seems to me that you still need to have an API that DMA >>>>> controller drivers can call where they can pass their dma_device >>>>> structure to ensure you get a channel for the appropriate DMA controller. >>> >>> Yes but the idea was that xlate will help you to get the right channel. The >>> whole dmaengine property was supposed to help you with that >> >> Well it depends on the DMA controller. In the case of tegra the xlate >> helps you extract the slave request ID for a given device. However, >> because any channel can be used with any slave request ID, we don't care >> about the exact channel. So we request any available channel for the DMA >> controller in question and program it with the slave request we got from >> the xlate. > > Right. the cleanup was supposed to reduce the number of interfaces > that a slave driver can call and consolidate them as much as possible > into dma_request_chan(), but we still need dma_get_any_slave_channel() > as an interface for the dmaengine masters as you said. OK, great. Vinod, are you ok with this then? Any other items to fix-up? >> I have seen some DMA drivers (such as imx-dma) use the filter function >> to match the DMA controller, but it seems daft to use the filter >> function to match the DMA controller when we already know in the xlate >> which DMA controller we are. > > The filter functions should ideally all go away after the cleanup > is complete. Wonderful! Thanks for the info. Cheers Jon -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 19, 2016 at 03:54:27PM +0100, Jon Hunter wrote: > >>>>> I can understand that you wish to consolidate the APIs for requesting a > >>>>> channel, but it seems to me that you still need to have an API that DMA > >>>>> controller drivers can call where they can pass their dma_device > >>>>> structure to ensure you get a channel for the appropriate DMA controller. > >>> > >>> Yes but the idea was that xlate will help you to get the right channel. The > >>> whole dmaengine property was supposed to help you with that > >> > >> Well it depends on the DMA controller. In the case of tegra the xlate > >> helps you extract the slave request ID for a given device. However, > >> because any channel can be used with any slave request ID, we don't care > >> about the exact channel. So we request any available channel for the DMA > >> controller in question and program it with the slave request we got from > >> the xlate. > > > > Right. the cleanup was supposed to reduce the number of interfaces > > that a slave driver can call and consolidate them as much as possible > > into dma_request_chan(), but we still need dma_get_any_slave_channel() > > as an interface for the dmaengine masters as you said. > > OK, great. > > Vinod, are you ok with this then? Any other items to fix-up? Okay from me.
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index d96d87c56f2e..f7a413c33c35 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -467,6 +467,20 @@ config TEGRA20_APB_DMA This DMA controller transfers data from memory to peripheral fifo or vice versa. It does not support memory to memory data transfer. +config TEGRA210_ADMA + bool "NVIDIA Tegra210 ADMA support" + depends on ARCH_TEGRA + select DMA_ENGINE + select DMA_VIRTUAL_CHANNELS + select PM_CLK + help + Support for the NVIDIA Tegra210 ADMA controller driver. The + DMA controller has multiple DMA channels and is used to service + various audio clients in the Tegra210 audio processing engine + (APE). This DMA controller transfers data from memory to + peripheral and vice versa. It does not support memory to + memory data transfer. + config TIMB_DMA tristate "Timberdale FPGA DMA support" depends on MFD_TIMBERDALE diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile index 6084127c1486..614f28b0b739 100644 --- a/drivers/dma/Makefile +++ b/drivers/dma/Makefile @@ -59,6 +59,7 @@ obj-$(CONFIG_STM32_DMA) += stm32-dma.o obj-$(CONFIG_S3C24XX_DMAC) += s3c24xx-dma.o obj-$(CONFIG_TXX9_DMAC) += txx9dmac.o obj-$(CONFIG_TEGRA20_APB_DMA) += tegra20-apb-dma.o +obj-$(CONFIG_TEGRA210_ADMA) += tegra210-adma.o obj-$(CONFIG_TIMB_DMA) += timb_dma.o obj-$(CONFIG_TI_CPPI41) += cppi41.o obj-$(CONFIG_TI_DMA_CROSSBAR) += ti-dma-crossbar.o diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c new file mode 100644 index 000000000000..56fefbe6a4c0 --- /dev/null +++ b/drivers/dma/tegra210-adma.c @@ -0,0 +1,869 @@ +/* + * ADMA driver for Nvidia's Tegra210 ADMA controller. + * + * Copyright (c) 2015, NVIDIA CORPORATION. All rights reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/clk.h> +#include <linux/iopoll.h> +#include <linux/module.h> +#include <linux/of_device.h> +#include <linux/of_dma.h> +#include <linux/of_irq.h> +#include <linux/pm_clock.h> +#include <linux/pm_runtime.h> +#include <linux/slab.h> + +#include "virt-dma.h" + +#define ADMA_CH_CMD 0x00 +#define ADMA_CH_STATUS 0x0c +#define ADMA_CH_STATUS_XFER_EN BIT(0) + +#define ADMA_CH_INT_STATUS 0x10 +#define ADMA_CH_INT_STATUS_XFER_DONE BIT(0) + +#define ADMA_CH_INT_CLEAR 0x1c +#define ADMA_CH_CTRL 0x24 +#define ADMA_CH_CTRL_TX_REQ(val) (((val) & 0xf) << 28) +#define ADMA_CH_CTRL_TX_REQ_MAX 10 +#define ADMA_CH_CTRL_RX_REQ(val) (((val) & 0xf) << 24) +#define ADMA_CH_CTRL_RX_REQ_MAX 10 +#define ADMA_CH_CTRL_DIR(val) (((val) & 0xf) << 12) +#define ADMA_CH_CTRL_DIR_AHUB2MEM 2 +#define ADMA_CH_CTRL_DIR_MEM2AHUB 4 +#define ADMA_CH_CTRL_MODE_CONTINUOUS (2 << 8) +#define ADMA_CH_CTRL_FLOWCTRL_EN BIT(1) + +#define ADMA_CH_CONFIG 0x28 +#define ADMA_CH_CONFIG_SRC_BUF(val) (((val) & 0x7) << 28) +#define ADMA_CH_CONFIG_TRG_BUF(val) (((val) & 0x7) << 24) +#define ADMA_CH_CONFIG_BURST_SIZE(val) (((val) & 0x7) << 20) +#define ADMA_CH_CONFIG_BURST_16 5 +#define ADMA_CH_CONFIG_WEIGHT_FOR_WRR(val) ((val) & 0xf) +#define ADMA_CH_CONFIG_MAX_BUFS 8 + +#define ADMA_CH_FIFO_CTRL 0x2c +#define ADMA_CH_FIFO_CTRL_OVRFW_THRES(val) (((val) & 0xf) << 24) +#define ADMA_CH_FIFO_CTRL_STARV_THRES(val) (((val) & 0xf) << 16) +#define ADMA_CH_FIFO_CTRL_TX_SIZE(val) (((val) & 0xf) << 8) +#define ADMA_CH_FIFO_CTRL_RX_SIZE(val) ((val) & 0xf) + +#define ADMA_CH_LOWER_SRC_ADDR 0x34 +#define ADMA_CH_LOWER_TRG_ADDR 0x3c +#define ADMA_CH_TC 0x44 +#define ADMA_CH_TC_COUNT_MASK 0x3ffffffc + +#define ADMA_CH_XFER_STATUS 0x54 +#define ADMA_CH_XFER_STATUS_COUNT_MASK 0xffff + +#define ADMA_GLOBAL_CMD 0xc00 +#define ADMA_GLOBAL_SOFT_RESET 0xc04 +#define ADMA_GLOBAL_INT_CLEAR 0xc20 +#define ADMA_GLOBAL_CTRL 0xc24 + +#define ADMA_CH_REG_OFFSET(a) (a * 0x80) + +#define ADMA_CH_FIFO_CTRL_DEFAULT (ADMA_CH_FIFO_CTRL_OVRFW_THRES(1) | \ + ADMA_CH_FIFO_CTRL_STARV_THRES(1) | \ + ADMA_CH_FIFO_CTRL_TX_SIZE(3) | \ + ADMA_CH_FIFO_CTRL_RX_SIZE(3)) +struct tegra_adma; + +/* + * struct tegra_adma_chip_data - Tegra chip specific data + * @nr_channels: Number of DMA channels available. + */ +struct tegra_adma_chip_data { + int nr_channels; +}; + +/* + * struct tegra_adma_chan_regs - Tegra ADMA channel registers + */ +struct tegra_adma_chan_regs { + unsigned int ctrl; + unsigned int config; + unsigned int src_addr; + unsigned int trg_addr; + unsigned int fifo_ctrl; + unsigned int tc; +}; + +/* + * struct tegra_adma_desc - Tegra ADMA descriptor to manage transfer requests. + */ +struct tegra_adma_desc { + struct virt_dma_desc vd; + struct tegra_adma_chan_regs ch_regs; + size_t buf_len; + size_t period_len; + size_t num_periods; +}; + +/* + * struct tegra_adma_chan - Tegra ADMA channel information + */ +struct tegra_adma_chan { + struct virt_dma_chan vc; + struct tegra_adma_desc *desc; + struct tegra_adma *tdma; + int irq; + void __iomem *chan_addr; + + /* Slave channel configuration info */ + struct dma_slave_config sconfig; + enum dma_transfer_direction sreq_dir; + unsigned int sreq_index; + bool sreq_reserved; + + /* Transfer count and position info */ + unsigned int tx_buf_count; + unsigned int tx_buf_pos; +}; + +/* + * struct tegra_adma - Tegra ADMA controller information + */ +struct tegra_adma { + struct dma_device dma_dev; + struct device *dev; + void __iomem *base_addr; + unsigned int nr_channels; + unsigned long rx_requests_reserved; + unsigned long tx_requests_reserved; + + /* Used to store global command register state when suspending */ + unsigned int global_cmd; + + /* Last member of the structure */ + struct tegra_adma_chan channels[0]; +}; + +static inline void tdma_write(struct tegra_adma *tdma, u32 reg, u32 val) +{ + writel(val, tdma->base_addr + reg); +} + +static inline u32 tdma_read(struct tegra_adma *tdma, u32 reg) +{ + return readl(tdma->base_addr + reg); +} + +static inline void tdma_ch_write(struct tegra_adma_chan *tdc, u32 reg, u32 val) +{ + writel(val, tdc->chan_addr + reg); +} + +static inline u32 tdma_ch_read(struct tegra_adma_chan *tdc, u32 reg) +{ + return readl(tdc->chan_addr + reg); +} + +static inline struct tegra_adma_chan *to_tegra_adma_chan(struct dma_chan *dc) +{ + return container_of(dc, struct tegra_adma_chan, vc.chan); +} + +static inline struct tegra_adma_desc *to_tegra_adma_desc( + struct dma_async_tx_descriptor *td) +{ + return container_of(td, struct tegra_adma_desc, vd.tx); +} + +static inline struct device *tdc2dev(struct tegra_adma_chan *tdc) +{ + return tdc->tdma->dev; +} + +static void tegra_adma_desc_free(struct virt_dma_desc *vd) +{ + kfree(container_of(vd, struct tegra_adma_desc, vd)); +} + +static int tegra_adma_slave_config(struct dma_chan *dc, + struct dma_slave_config *sconfig) +{ + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); + + memcpy(&tdc->sconfig, sconfig, sizeof(*sconfig)); + + return 0; +} + +static int tegra_adma_init(struct tegra_adma *tdma) +{ + u32 status; + int ret; + + /* Clear any interrupts */ + tdma_write(tdma, ADMA_GLOBAL_INT_CLEAR, 0x1); + + /* Assert soft reset */ + tdma_write(tdma, ADMA_GLOBAL_SOFT_RESET, 0x1); + + /* Wait for reset to clear */ + ret = readx_poll_timeout(readl, + tdma->base_addr + ADMA_GLOBAL_SOFT_RESET, + status, status == 0, 20, 10000); + if (ret) + return ret; + + /* Enable global ADMA registers */ + tdma_write(tdma, ADMA_GLOBAL_CMD, 1); + + return 0; +} + +static int tegra_adma_request_alloc(struct tegra_adma_chan *tdc, + enum dma_transfer_direction direction) +{ + struct tegra_adma *tdma = tdc->tdma; + unsigned int sreq_index = tdc->sreq_index; + + if (tdc->sreq_reserved) + return tdc->sreq_dir == direction ? 0 : -EINVAL; + + switch (direction) { + case DMA_MEM_TO_DEV: + if (sreq_index > ADMA_CH_CTRL_TX_REQ_MAX) { + dev_err(tdma->dev, "invalid DMA request\n"); + return -EINVAL; + } + + if (test_and_set_bit(sreq_index, &tdma->tx_requests_reserved)) { + dev_err(tdma->dev, "DMA request reserved\n"); + return -EINVAL; + } + break; + + case DMA_DEV_TO_MEM: + if (sreq_index > ADMA_CH_CTRL_RX_REQ_MAX) { + dev_err(tdma->dev, "invalid DMA request\n"); + return -EINVAL; + } + + if (test_and_set_bit(sreq_index, &tdma->rx_requests_reserved)) { + dev_err(tdma->dev, "DMA request reserved\n"); + return -EINVAL; + } + break; + + default: + dev_WARN(tdma->dev, "channel %s has invalid transfer type\n", + dma_chan_name(&tdc->vc.chan)); + return -EINVAL; + } + + tdc->sreq_dir = direction; + tdc->sreq_reserved = true; + + return 0; +} + +static void tegra_adma_request_free(struct tegra_adma_chan *tdc) +{ + struct tegra_adma *tdma = tdc->tdma; + + if (!tdc->sreq_reserved) + return; + + switch (tdc->sreq_dir) { + case DMA_MEM_TO_DEV: + clear_bit(tdc->sreq_index, &tdma->tx_requests_reserved); + break; + case DMA_DEV_TO_MEM: + clear_bit(tdc->sreq_index, &tdma->rx_requests_reserved); + break; + default: + dev_WARN(tdma->dev, "channel %s has invalid transfer type\n", + dma_chan_name(&tdc->vc.chan)); + return; + } + + tdc->sreq_reserved = false; +} + +static u32 tegra_adma_irq_status(struct tegra_adma_chan *tdc) +{ + u32 status = tdma_ch_read(tdc, ADMA_CH_INT_STATUS); + + return status & ADMA_CH_INT_STATUS_XFER_DONE; +} + +static u32 tegra_adma_irq_clear(struct tegra_adma_chan *tdc) +{ + u32 status = tegra_adma_irq_status(tdc); + + if (status) + tdma_ch_write(tdc, ADMA_CH_INT_CLEAR, status); + + return status; +} + +static void tegra_adma_stop(struct tegra_adma_chan *tdc) +{ + unsigned int status; + + /* Disable ADMA */ + tdma_ch_write(tdc, ADMA_CH_CMD, 0); + + /* Clear interrupt status */ + tegra_adma_irq_clear(tdc); + + if (readx_poll_timeout_atomic(readl, tdc->chan_addr + ADMA_CH_STATUS, + status, !(status & ADMA_CH_STATUS_XFER_EN), + 20, 10000)) { + dev_err(tdc2dev(tdc), "unable to stop DMA channel\n"); + return; + } + + kfree(tdc->desc); + tdc->desc = NULL; +} + +static void tegra_adma_start(struct tegra_adma_chan *tdc) +{ + struct virt_dma_desc *vd = vchan_next_desc(&tdc->vc); + struct tegra_adma_chan_regs *ch_regs; + struct tegra_adma_desc *desc; + + if (!vd) + return; + + list_del(&vd->node); + + desc = to_tegra_adma_desc(&vd->tx); + + if (!desc) { + dev_warn(tdc2dev(tdc), "unable to start DMA, no descriptor\n"); + return; + } + + ch_regs = &desc->ch_regs; + + tdc->tx_buf_pos = 0; + tdc->tx_buf_count = 0; + tdma_ch_write(tdc, ADMA_CH_TC, ch_regs->tc); + tdma_ch_write(tdc, ADMA_CH_CTRL, ch_regs->ctrl); + tdma_ch_write(tdc, ADMA_CH_LOWER_SRC_ADDR, ch_regs->src_addr); + tdma_ch_write(tdc, ADMA_CH_LOWER_TRG_ADDR, ch_regs->trg_addr); + tdma_ch_write(tdc, ADMA_CH_FIFO_CTRL, ch_regs->fifo_ctrl); + tdma_ch_write(tdc, ADMA_CH_CONFIG, ch_regs->config); + + /* Start ADMA */ + tdma_ch_write(tdc, ADMA_CH_CMD, 1); + + tdc->desc = desc; +} + +static unsigned int tegra_adma_get_residue(struct tegra_adma_chan *tdc) +{ + struct tegra_adma_desc *desc = tdc->desc; + unsigned int max = ADMA_CH_XFER_STATUS_COUNT_MASK + 1; + unsigned int pos = tdma_ch_read(tdc, ADMA_CH_XFER_STATUS); + unsigned int periods_remaining; + + /* + * Handle wrap around of buffer count register + */ + if (pos < tdc->tx_buf_pos) + tdc->tx_buf_count += pos + (max - tdc->tx_buf_pos); + else + tdc->tx_buf_count += pos - tdc->tx_buf_pos; + + periods_remaining = tdc->tx_buf_count % desc->num_periods; + tdc->tx_buf_pos = pos; + + return desc->buf_len - (periods_remaining * desc->period_len); +} + +static irqreturn_t tegra_adma_isr(int irq, void *dev_id) +{ + struct tegra_adma_chan *tdc = dev_id; + unsigned long status; + unsigned long flags; + + spin_lock_irqsave(&tdc->vc.lock, flags); + + status = tegra_adma_irq_clear(tdc); + if (status == 0 || !tdc->desc) { + spin_unlock_irqrestore(&tdc->vc.lock, flags); + return IRQ_NONE; + } + + vchan_cyclic_callback(&tdc->desc->vd); + + spin_unlock_irqrestore(&tdc->vc.lock, flags); + + return IRQ_HANDLED; +} + +static void tegra_adma_issue_pending(struct dma_chan *dc) +{ + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); + unsigned long flags; + + spin_lock_irqsave(&tdc->vc.lock, flags); + + if (vchan_issue_pending(&tdc->vc)) { + if (!tdc->desc) + tegra_adma_start(tdc); + } + + spin_unlock_irqrestore(&tdc->vc.lock, flags); +} + +static int tegra_adma_terminate_all(struct dma_chan *dc) +{ + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); + unsigned long flags; + LIST_HEAD(head); + + spin_lock_irqsave(&tdc->vc.lock, flags); + + if (tdc->desc) + tegra_adma_stop(tdc); + + tegra_adma_request_free(tdc); + vchan_get_all_descriptors(&tdc->vc, &head); + spin_unlock_irqrestore(&tdc->vc.lock, flags); + vchan_dma_desc_free_list(&tdc->vc, &head); + + return 0; +} + +static enum dma_status tegra_adma_tx_status(struct dma_chan *dc, + dma_cookie_t cookie, + struct dma_tx_state *txstate) +{ + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); + struct tegra_adma_desc *desc; + struct virt_dma_desc *vd; + enum dma_status ret; + unsigned long flags; + unsigned int residual; + + ret = dma_cookie_status(dc, cookie, txstate); + if (ret == DMA_COMPLETE || !txstate) + return ret; + + spin_lock_irqsave(&tdc->vc.lock, flags); + + vd = vchan_find_desc(&tdc->vc, cookie); + if (vd) { + desc = to_tegra_adma_desc(&vd->tx); + residual = desc->ch_regs.tc; + } else if (tdc->desc && tdc->desc->vd.tx.cookie == cookie) { + residual = tegra_adma_get_residue(tdc); + } else { + residual = 0; + } + + spin_unlock_irqrestore(&tdc->vc.lock, flags); + + dma_set_residue(txstate, residual); + + return ret; +} + +static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc, + struct tegra_adma_desc *desc, + dma_addr_t buf_addr, + enum dma_transfer_direction direction) +{ + struct tegra_adma_chan_regs *ch_regs = &desc->ch_regs; + unsigned int burst_size, adma_dir; + + if (desc->num_periods > ADMA_CH_CONFIG_MAX_BUFS) + return -EINVAL; + + switch (direction) { + case DMA_MEM_TO_DEV: + adma_dir = ADMA_CH_CTRL_DIR_MEM2AHUB; + burst_size = fls(tdc->sconfig.dst_maxburst); + ch_regs->config = ADMA_CH_CONFIG_SRC_BUF(desc->num_periods - 1); + ch_regs->ctrl = ADMA_CH_CTRL_TX_REQ(tdc->sreq_index); + ch_regs->src_addr = buf_addr; + break; + + case DMA_DEV_TO_MEM: + adma_dir = ADMA_CH_CTRL_DIR_AHUB2MEM; + burst_size = fls(tdc->sconfig.src_maxburst); + ch_regs->config = ADMA_CH_CONFIG_TRG_BUF(desc->num_periods - 1); + ch_regs->ctrl = ADMA_CH_CTRL_RX_REQ(tdc->sreq_index); + ch_regs->trg_addr = buf_addr; + break; + + default: + dev_err(tdc2dev(tdc), "DMA direction is not supported\n"); + return -EINVAL; + } + + if (!burst_size || burst_size > ADMA_CH_CONFIG_BURST_16) + burst_size = ADMA_CH_CONFIG_BURST_16; + + ch_regs->ctrl |= ADMA_CH_CTRL_DIR(adma_dir) | + ADMA_CH_CTRL_MODE_CONTINUOUS | + ADMA_CH_CTRL_FLOWCTRL_EN; + ch_regs->config |= ADMA_CH_CONFIG_BURST_SIZE(burst_size); + ch_regs->config |= ADMA_CH_CONFIG_WEIGHT_FOR_WRR(1); + ch_regs->fifo_ctrl = ADMA_CH_FIFO_CTRL_DEFAULT; + ch_regs->tc = desc->period_len & ADMA_CH_TC_COUNT_MASK; + + return tegra_adma_request_alloc(tdc, direction); +} + +static struct dma_async_tx_descriptor *tegra_adma_prep_slave_sg( + struct dma_chan *dc, struct scatterlist *sgl, unsigned int sg_len, + enum dma_transfer_direction direction, unsigned long flags, + void *context) +{ + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); + + dev_warn(tdc2dev(tdc), "scatter-gather transfers are not supported\n"); + + return NULL; +} + +static struct dma_async_tx_descriptor *tegra_adma_prep_dma_cyclic( + struct dma_chan *dc, dma_addr_t buf_addr, size_t buf_len, + size_t period_len, enum dma_transfer_direction direction, + unsigned long flags) +{ + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); + struct tegra_adma_desc *desc = NULL; + + if (!buf_len || !period_len || period_len > ADMA_CH_TC_COUNT_MASK) { + dev_err(tdc2dev(tdc), "invalid buffer/period len\n"); + return NULL; + } + + if (buf_len % period_len) { + dev_err(tdc2dev(tdc), "buf_len not a multiple of period_len\n"); + return NULL; + } + + if (!IS_ALIGNED(buf_addr, 4)) { + dev_err(tdc2dev(tdc), "invalid buffer alignment\n"); + return NULL; + } + + desc = kzalloc(sizeof(*desc), GFP_NOWAIT); + if (!desc) + return NULL; + + desc->buf_len = buf_len; + desc->period_len = period_len; + desc->num_periods = buf_len / period_len; + + if (tegra_adma_set_xfer_params(tdc, desc, buf_addr, direction)) { + kfree(desc); + return NULL; + } + + return vchan_tx_prep(&tdc->vc, &desc->vd, flags); +} + +static int tegra_adma_alloc_chan_resources(struct dma_chan *dc) +{ + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); + int ret; + + ret = request_irq(tdc->irq, tegra_adma_isr, 0, dma_chan_name(dc), tdc); + if (ret) { + dev_err(tdc2dev(tdc), "failed to get interrupt for %s\n", + dma_chan_name(dc)); + return ret; + } + + ret = pm_runtime_get_sync(tdc2dev(tdc)); + if (ret < 0) { + free_irq(tdc->irq, tdc); + return ret; + } + + dma_cookie_init(&tdc->vc.chan); + + return 0; +} + +static void tegra_adma_free_chan_resources(struct dma_chan *dc) +{ + struct tegra_adma_chan *tdc = to_tegra_adma_chan(dc); + + tegra_adma_terminate_all(dc); + vchan_free_chan_resources(&tdc->vc); + tasklet_kill(&tdc->vc.task); + free_irq(tdc->irq, tdc); + pm_runtime_put(tdc2dev(tdc)); + + tdc->sreq_index = 0; + tdc->sreq_dir = DMA_TRANS_NONE; +} + +static struct dma_chan *tegra_dma_of_xlate(struct of_phandle_args *dma_spec, + struct of_dma *ofdma) +{ + struct tegra_adma *tdma = ofdma->of_dma_data; + struct tegra_adma_chan *tdc; + struct dma_chan *chan; + unsigned int sreq_index; + + if (dma_spec->args_count != 1) + return NULL; + + sreq_index = dma_spec->args[0]; + + if (sreq_index == 0) { + dev_err(tdma->dev, "DMA request must not be 0\n"); + return NULL; + } + + chan = dma_get_any_slave_channel(&tdma->dma_dev); + if (!chan) + return NULL; + + tdc = to_tegra_adma_chan(chan); + tdc->sreq_index = sreq_index; + + return chan; +} + +static int tegra_adma_runtime_suspend(struct device *dev) +{ + struct tegra_adma *tdma = dev_get_drvdata(dev); + + tdma->global_cmd = tdma_read(tdma, ADMA_GLOBAL_CMD); + + return pm_clk_suspend(dev); +} + +static int tegra_adma_runtime_resume(struct device *dev) +{ + struct tegra_adma *tdma = dev_get_drvdata(dev); + int ret; + + ret = pm_clk_resume(dev); + if (ret) + return ret; + + tdma_write(tdma, ADMA_GLOBAL_CMD, tdma->global_cmd); + + return 0; +} + +static int tegra_adma_add_clock(struct device *dev, char *name) +{ + struct clk *clk; + int ret; + + clk = clk_get(dev, name); + if (IS_ERR(clk)) { + dev_err(dev, "%s clock not found\n", name); + return PTR_ERR(clk); + } + + ret = pm_clk_add_clk(dev, clk); + if (ret) + clk_put(clk); + + return ret; +} + +static const struct tegra_adma_chip_data tegra210_chip_data = { + .nr_channels = 22, +}; + +static const struct of_device_id tegra_adma_of_match[] = { + { .compatible = "nvidia,tegra210-adma", .data = &tegra210_chip_data }, + { }, +}; +MODULE_DEVICE_TABLE(of, tegra_adma_of_match); + +static int tegra_adma_probe(struct platform_device *pdev) +{ + const struct of_device_id *match; + const struct tegra_adma_chip_data *cdata; + struct tegra_adma *tdma; + struct resource *res; + int ret, i; + + match = of_match_device(tegra_adma_of_match, &pdev->dev); + if (!match) { + dev_err(&pdev->dev, "no device match found\n"); + return -ENODEV; + } + cdata = match->data; + + tdma = devm_kzalloc(&pdev->dev, sizeof(*tdma) + cdata->nr_channels * + sizeof(struct tegra_adma_chan), GFP_KERNEL); + if (!tdma) + return -ENOMEM; + + tdma->dev = &pdev->dev; + tdma->nr_channels = cdata->nr_channels; + platform_set_drvdata(pdev, tdma); + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + tdma->base_addr = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(tdma->base_addr)) + return PTR_ERR(tdma->base_addr); + + ret = pm_clk_create(&pdev->dev); + if (ret) + return ret; + + ret = tegra_adma_add_clock(&pdev->dev, "ape"); + if (ret) + goto clk_destroy; + + ret = tegra_adma_add_clock(&pdev->dev, "apb2ape"); + if (ret) + goto clk_destroy; + + ret = tegra_adma_add_clock(&pdev->dev, "d_audio"); + if (ret) + goto clk_destroy; + + pm_runtime_enable(&pdev->dev); + + ret = pm_runtime_get_sync(&pdev->dev); + if (ret < 0) + goto rpm_disable; + + ret = tegra_adma_init(tdma); + if (ret) + goto rpm_put; + + INIT_LIST_HEAD(&tdma->dma_dev.channels); + for (i = 0; i < tdma->nr_channels; i++) { + struct tegra_adma_chan *tdc = &tdma->channels[i]; + + tdc->chan_addr = tdma->base_addr + ADMA_CH_REG_OFFSET(i); + + tdc->irq = of_irq_get(pdev->dev.of_node, i); + if (tdc->irq < 0) { + ret = tdc->irq; + goto irq_dispose; + } + + vchan_init(&tdc->vc, &tdma->dma_dev); + tdc->vc.desc_free = tegra_adma_desc_free; + tdc->tdma = tdma; + } + + dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); + dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); + dma_cap_set(DMA_CYCLIC, tdma->dma_dev.cap_mask); + + tdma->dma_dev.dev = &pdev->dev; + tdma->dma_dev.device_alloc_chan_resources = + tegra_adma_alloc_chan_resources; + tdma->dma_dev.device_free_chan_resources = + tegra_adma_free_chan_resources; + tdma->dma_dev.device_issue_pending = tegra_adma_issue_pending; + tdma->dma_dev.device_prep_slave_sg = tegra_adma_prep_slave_sg; + tdma->dma_dev.device_prep_dma_cyclic = tegra_adma_prep_dma_cyclic; + tdma->dma_dev.device_config = tegra_adma_slave_config; + tdma->dma_dev.device_tx_status = tegra_adma_tx_status; + tdma->dma_dev.device_terminate_all = tegra_adma_terminate_all; + tdma->dma_dev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); + tdma->dma_dev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_4_BYTES); + tdma->dma_dev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV); + tdma->dma_dev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT; + + ret = dma_async_device_register(&tdma->dma_dev); + if (ret < 0) { + dev_err(&pdev->dev, "ADMA registration failed: %d\n", ret); + goto irq_dispose; + } + + ret = of_dma_controller_register(pdev->dev.of_node, + tegra_dma_of_xlate, tdma); + if (ret < 0) { + dev_err(&pdev->dev, "ADMA OF registration failed %d\n", ret); + goto dma_remove; + } + + pm_runtime_put(&pdev->dev); + + dev_info(&pdev->dev, "Tegra210 ADMA driver registered %d channels\n", + tdma->nr_channels); + + return 0; + +dma_remove: + dma_async_device_unregister(&tdma->dma_dev); +irq_dispose: + while (--i >= 0) + irq_dispose_mapping(tdma->channels[i].irq); +rpm_put: + pm_runtime_put_sync(&pdev->dev); +rpm_disable: + pm_runtime_disable(&pdev->dev); +clk_destroy: + pm_clk_destroy(&pdev->dev); + + return ret; +} + +static int tegra_adma_remove(struct platform_device *pdev) +{ + struct tegra_adma *tdma = platform_get_drvdata(pdev); + int i; + + dma_async_device_unregister(&tdma->dma_dev); + + for (i = 0; i < tdma->nr_channels; ++i) + irq_dispose_mapping(tdma->channels[i].irq); + + pm_runtime_put_sync(&pdev->dev); + pm_runtime_disable(&pdev->dev); + pm_clk_destroy(&pdev->dev); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int tegra_adma_pm_suspend(struct device *dev) +{ + return pm_runtime_suspended(dev) == false; +} +#endif + +static const struct dev_pm_ops tegra_adma_dev_pm_ops = { + SET_RUNTIME_PM_OPS(tegra_adma_runtime_suspend, + tegra_adma_runtime_resume, NULL) + SET_SYSTEM_SLEEP_PM_OPS(tegra_adma_pm_suspend, NULL) +}; + +static struct platform_driver tegra_admac_driver = { + .driver = { + .name = "tegra-adma", + .pm = &tegra_adma_dev_pm_ops, + .of_match_table = tegra_adma_of_match, + }, + .probe = tegra_adma_probe, + .remove = tegra_adma_remove, +}; + +module_platform_driver(tegra_admac_driver); + +MODULE_ALIAS("platform:tegra210-adma"); +MODULE_DESCRIPTION("NVIDIA Tegra ADMA driver"); +MODULE_AUTHOR("Dara Ramesh <dramesh@nvidia.com>"); +MODULE_AUTHOR("Jon Hunter <jonathanh@nvidia.com>"); +MODULE_LICENSE("GPL v2");
Add support for the Tegra210 Audio DMA controller that is used for transferring data between system memory and the Audio sub-system. The driver only supports cyclic transfers because this is being solely used for audio. This driver is based upon the work by Dara Ramesh <dramesh@nvidia.com>. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- drivers/dma/Kconfig | 14 + drivers/dma/Makefile | 1 + drivers/dma/tegra210-adma.c | 869 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 884 insertions(+) create mode 100644 drivers/dma/tegra210-adma.c