mbox series

[v2,0/2] Apple ADMAC driver

Message ID 20220411222204.96860-1-povik+lin@cutebit.org
Headers show
Series Apple ADMAC driver | expand

Message

Martin Povišer April 11, 2022, 10:22 p.m. UTC 
Hi all,

I am submitting v2 of driver for Audio DMA Controller on recent Apple SoCs.

Changes since v1:
(link: https://lore.kernel.org/dmaengine/20220330164458.93055-1-povik+lin@cutebit.org/ )
 - in binding, drop 'apple,internal-irq-destination' in favor of
   prepending entries to 'interrupts'
 - drop admac_peek/poke helpers
 - use special versions of dev_err/WARN_ON where desirable, fix %pad
   formatter invocation
 - bring implementation of terminate_all up to dmaengine spec, add
   device_synchronize
 - minor fixes (comments, formatting)

Thanks,
Martin

Martin Povišer (2):
  dt-bindings: dma: Add Apple ADMAC
  dmaengine: apple-admac: Add Apple ADMAC driver

 .../devicetree/bindings/dma/apple,admac.yaml  |  68 ++
 MAINTAINERS                                   |   2 +
 drivers/dma/Kconfig                           |   8 +
 drivers/dma/Makefile                          |   1 +
 drivers/dma/apple-admac.c                     | 818 ++++++++++++++++++
 5 files changed, 897 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/dma/apple,admac.yaml
 create mode 100644 drivers/dma/apple-admac.c

Comments

Vinod Koul April 20, 2022, 12:40 p.m. UTC | #1 
On 12-04-22, 00:22, Martin Povišer wrote:

> +struct admac_chan {
> +	int no;

Unsigned int perhaps?

> +static int admac_desc_free(struct dma_async_tx_descriptor *tx)
> +{
> +	struct admac_tx *adtx = to_admac_tx(tx);
> +
> +	devm_kfree(to_admac_chan(tx->chan)->host->dev, adtx);

Why use devm for descriptor memory?

> +static int admac_device_config(struct dma_chan *chan,
> +			       struct dma_slave_config *config)
> +{
> +	struct admac_chan *adchan = to_admac_chan(chan);
> +	struct admac_data *ad = adchan->host;
> +	bool is_tx = admac_chan_direction(adchan->no) == DMA_MEM_TO_DEV;

so are the channel directions hard wired in hardware?
Martin Povišer April 20, 2022, 1:41 p.m. UTC | #2 
> On 20. 4. 2022, at 14:40, Vinod Koul <vkoul@kernel.org> wrote:
> 
> On 12-04-22, 00:22, Martin Povišer wrote:
> 
>> +struct admac_chan {
>> +	int no;
> 
> Unsigned int perhaps?

Sure.

> 
>> +static int admac_desc_free(struct dma_async_tx_descriptor *tx)
>> +{
>> +	struct admac_tx *adtx = to_admac_tx(tx);
>> +
>> +	devm_kfree(to_admac_chan(tx->chan)->host->dev, adtx);
> 
> Why use devm for descriptor memory?

Well, good point. :-)

>> +static int admac_device_config(struct dma_chan *chan,
>> +			       struct dma_slave_config *config)
>> +{
>> +	struct admac_chan *adchan = to_admac_chan(chan);
>> +	struct admac_data *ad = adchan->host;
>> +	bool is_tx = admac_chan_direction(adchan->no) == DMA_MEM_TO_DEV;
> 
> so are the channel directions hard wired in hardware?

Yes they are.

> -- 
> ~Vinod

Martin