Messages in this thread | | | Date | Wed, 10 Nov 2021 23:06:19 +0100 | From | Andrew Lunn <> | Subject | Re: [PATCH 2/5] net: fec: fec-uio driver |
| |
On Wed, Nov 10, 2021 at 11:18:35AM +0530, Apeksha Gupta wrote: > i.mx: fec-uio driver > > This patch adds the userspace support. In this basic > hardware initialization is performed in kernel via userspace > input/output, while the majority of code is written in the > userspace.
Where do i find this usespace code. Please include a URL to a git repo.
> +static unsigned char macaddr[ETH_ALEN]; > +module_param_array(macaddr, byte, NULL, 0); > +MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
No module parameters please. Use the standard device tree bindings.
> +static int fec_enet_uio_init(struct net_device *ndev) > +{ > + unsigned int total_tx_ring_size = 0, total_rx_ring_size = 0; > + struct fec_enet_private *fep = netdev_priv(ndev); > + unsigned int dsize = sizeof(struct bufdesc); > + unsigned short tx_ring_size, rx_ring_size; > + int ret, i; > + > + /* Check mask of the streaming and coherent API */ > + ret = dma_set_mask_and_coherent(&fep->pdev->dev, DMA_BIT_MASK(32)); > + if (ret < 0) { > + dev_warn(&fep->pdev->dev, "No suitable DMA available\n"); > + return ret; > + } > + > + tx_ring_size = TX_RING_SIZE; > + rx_ring_size = RX_RING_SIZE; > + > + for (i = 0; i < FEC_ENET_MAX_TX_QS; i++) > + total_tx_ring_size += tx_ring_size; > + for (i = 0; i < FEC_ENET_MAX_RX_QS; i++) > + total_rx_ring_size += rx_ring_size; > + > + bd_size = (total_tx_ring_size + total_rx_ring_size) * dsize;
These are the buffer descriptors, not buffers themselves. I assume the user space driver is allocating the buffer? And your userspace then set the descriptor to point to user allocated memory? Or some other memory in the address space, and overwrite whatever you want on the next DMA? Or DMAing kernel memory out as frames?
> +static int > +fec_enet_uio_probe(struct platform_device *pdev) > +{ > + struct fec_uio_devinfo *dev_info; > + const struct of_device_id *of_id; > + struct fec_enet_private *fep; > + struct net_device *ndev; > + u32 ecntl = ETHER_EN; > + static int dev_id; > + bool reset_again; > + int ret = 0; > + > + /* Init network device */ > + ndev = alloc_etherdev_mq(sizeof(struct fec_enet_private) + > + FEC_PRIV_SIZE, FEC_MAX_Q);
Why do you need this. This is not a netdev driver, since it does not connect to the network stack.
> +static int > +fec_enet_uio_remove(struct platform_device *pdev) > +{ > + struct net_device *ndev = platform_get_drvdata(pdev); > + struct fec_enet_private *fep = netdev_priv(ndev); > + > + kfree(fec_dev); > + iounmap(fep->hwp); > + dma_free_coherent(&fep->pdev->dev, bd_size, cbd_base, bd_dma);
Don't you have to assume that the userspace driver has crashed and burned, leaving the hardware in an undefined state. It could still be receiving, into buffers we have no idea about. Don't you need to stop the hardware, and then wait for all DMA activity to stop, and only then can you free the buffer descriptors?
Andrew
| |