On 12-07-23, 15:38, Martin Povišer wrote:
> +struct sio_chan {
> + unsigned int no;
> + struct sio_data *host;
> + struct dma_chan chan;
> + struct tasklet_struct tasklet;
> + struct work_struct terminate_wq;
> +
> + spinlock_t lock;
> + struct sio_tx *current_tx;
> + /*
> + * 'tx_cookie' is used for distinguishing between transactions from
> + * within tag ack/nack callbacks. Without it, we would have no way
> + * of knowing if the current transaction is the one the callback handler
> + * was installed for.
not sure what you mean by here.. I dont see why you would need to store
cookie here, care to explain?
> + */
> + unsigned long tx_cookie;
> + int nperiod_acks;
> +
> + /*
> + * We maintain a 'submitted' and 'issued' list mainly for interface
> + * correctness. Typical use of the driver (per channel) will be
> + * prepping, submitting and issuing a single cyclic transaction which
> + * will stay current until terminate_all is called.
> + */
> + struct list_head submitted;
> + struct list_head issued;
> +
> + struct list_head to_free;
can you use virt_dma_chan, that should simplify list handling etc
> +};
> +
> +#define SIO_NTAGS 16
> +
> +typedef void (*sio_ack_callback)(struct sio_chan *, void *, bool);
any reason not to use dmaengine callbacks?
> +static int sio_alloc_tag(struct sio_data *sio)
> +{
> + struct sio_tagdata *tags = &sio->tags;
> + int tag, i;
> +
> + /*
> + * Because tag number 0 is special, the usable tag range
> + * is 1...(SIO_NTAGS - 1). So, to pick the next usable tag,
> + * we do modulo (SIO_NTAGS - 1) *then* plus one.
> + */
> +
> +#define SIO_USABLE_TAGS (SIO_NTAGS - 1)
> + tag = (READ_ONCE(tags->last_tag) % SIO_USABLE_TAGS) + 1;
> +
> + for (i = 0; i < SIO_USABLE_TAGS; i++) {
> + if (!test_and_set_bit(tag, &tags->allocated))
> + break;
> +
> + tag = (tag % SIO_USABLE_TAGS) + 1;
> + }
> +
> + WRITE_ONCE(tags->last_tag, tag);
> +
> + if (i < SIO_USABLE_TAGS)
> + return tag;
> + else
> + return -EBUSY;
> +#undef SIO_USABLE_TAGS
> +}
can you use kernel mechanisms like ida to alloc and free the tags...
> +static struct dma_async_tx_descriptor *sio_prep_dma_cyclic(
> + struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
> + size_t period_len, enum dma_transfer_direction direction,
> + unsigned long flags)
> +{
> + struct sio_chan *siochan = to_sio_chan(chan);
> + struct sio_tx *siotx = NULL;
> + int i, nperiods = buf_len / period_len;
> +
> + if (direction != sio_chan_direction(siochan->no))
> + return NULL;
> +
> + siotx = kzalloc(struct_size(siotx, siodesc, nperiods), GFP_NOWAIT);
> + if (!siotx)
> + return NULL;
> +
> + init_completion(&siotx->done);
> + dma_async_tx_descriptor_init(&siotx->tx, chan);
> + siotx->period_len = period_len;
> + siotx->nperiods = nperiods;
> +
> + for (i = 0; i < nperiods; i++) {
> + struct sio_coproc_desc *d;
> +
> + siotx->siodesc[i] = d = sio_alloc_desc(siochan->host);
> + if (!d) {
> + sio_tx_free(&siotx->tx);
> + return NULL;
> + }
> +
> + d->flag = 1; // not sure what's up with this
> + d->iova = buf_addr + period_len * i;
> + d->size = period_len;
> + }
> + dma_wmb();
why use barrier here? and to what purpose..
--
~Vinod
