Thanks Matthias for the comments and sorry for the holiday delay.
The recommended changes have been posted in patch v7 1/9.
Thanks,
Liming
> -----Original Message-----
> From: Matthias Brugger <matthias.bgg@xxxxxxxxx>
> Sent: Wednesday, December 12, 2018 6:08 PM
> To: Liming Sun <lsun@xxxxxxxxxxxx>; y@xxxxxxxxxxxxxxxxxxxxxxx; Olof Johansson <olof@xxxxxxxxx>; Arnd Bergmann <arnd@xxxxxxxx>;
> David Woods <dwoods@xxxxxxxxxxxx>; Robin Murphy <robin.murphy@xxxxxxx>; arm-soc <arm@xxxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; mbrugger@xxxxxxxx >> Matthias Brugger <mbrugger@xxxxxxxx>
> Subject: Re: [PATCH v6 1/9] soc: Add TmFifo driver for Mellanox BlueField Soc
>
>
>
> On 01/11/2018 17:23, Liming Sun wrote:> This commit adds the TmFifo driver for
> Mellanox BlueField Soc.
> > TmFifo is a shared FIFO which enables external host machine to
> > exchange data with the SoC via USB or PCIe. The driver is based on
> > virtio framework and has console and network access enabled.
> >
> > Reviewed-by: David Woods <dwoods@xxxxxxxxxxxx>
> > Signed-off-by: Liming Sun <lsun@xxxxxxxxxxxx>
> > ---
> > drivers/soc/Kconfig | 1 +
> > drivers/soc/Makefile | 1 +
> > drivers/soc/mellanox/Kconfig | 18 +
> > drivers/soc/mellanox/Makefile | 5 +
> > drivers/soc/mellanox/tmfifo.c | 1236 ++++++++++++++++++++++++++++++++++++
> > drivers/soc/mellanox/tmfifo_regs.h | 76 +++
> > 6 files changed, 1337 insertions(+)
> > create mode 100644 drivers/soc/mellanox/Kconfig
> > create mode 100644 drivers/soc/mellanox/Makefile
> > create mode 100644 drivers/soc/mellanox/tmfifo.c
> > create mode 100644 drivers/soc/mellanox/tmfifo_regs.h
> >
>
> [...]
>
> > +
> > +/* Interrupt handler. */
> > +static irqreturn_t tmfifo_irq_handler(int irq, void *dev_id)
> > +{
> > + int i = (uintptr_t)dev_id % sizeof(void *);
> > + struct tmfifo *fifo = dev_id - i;
> > +
> > + if (i < TM_IRQ_CNT && !test_and_set_bit(i, &fifo->pend_events))
> > + schedule_work(&fifo->work);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> [...]
> > +
> > +/* Probe the TMFIFO. */
> > +static int tmfifo_probe(struct platform_device *pdev)
> > +{
> > + u64 ctl;
> > + struct tmfifo *fifo;
> > + struct resource *rx_res, *tx_res;
> > + struct virtio_net_config net_config;
> > + int i, ret;
> > +
> > + /* Get the resource of the Rx & Tx FIFO. */
> > + rx_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + tx_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > + if (!rx_res || !tx_res) {
> > + ret = -EINVAL;
> > + goto err;
> > + }
> > +
> > + if (request_mem_region(rx_res->start,
> > + resource_size(rx_res), "bf-tmfifo") == NULL) {
> > + ret = -EBUSY;
> > + goto early_err;
> > + }
> > +
> > + if (request_mem_region(tx_res->start,
> > + resource_size(tx_res), "bf-tmfifo") == NULL) {
> > + release_mem_region(rx_res->start, resource_size(rx_res));
> > + ret = -EBUSY;
> > + goto early_err;
> > + }
> > +
> > + ret = -ENOMEM;
> > + fifo = kzalloc(sizeof(struct tmfifo), GFP_KERNEL);
> > + if (!fifo)
> > + goto err;
> > +
> > + fifo->pdev = pdev;
> > + platform_set_drvdata(pdev, fifo);
> > +
> > + spin_lock_init(&fifo->spin_lock);
> > + INIT_WORK(&fifo->work, tmfifo_work_handler);
> > +
> > + timer_setup(&fifo->timer, tmfifo_timer, 0);
> > + fifo->timer.function = tmfifo_timer;
> > +
> > + for (i = 0; i < TM_IRQ_CNT; i++) {
> > + fifo->irq[i] = platform_get_irq(pdev, i);
> > + ret = request_irq(fifo->irq[i], tmfifo_irq_handler, 0,
> > + "tmfifo", (u8 *)fifo + i);
>
> I think it would be better if you create a struct that passes a pointer to fifo
> and the ID instead of "hiding" the ID inside the address.
>
> Regards,
> Matthias
