Re: [PATCH v2 05/13] net: ath79: add ag71xx Ethernet driver

[Lucas Stach <l.stach@pengutronix.de>]

Hi Peter,

Am Montag, den 07.08.2017, 23:45 +0300 schrieb Peter Mamonov:
> On Mon, Aug 07, 2017 at 04:39:19PM +0200, Oleksij Rempel wrote:
> > From: Yegor Yefremov <yegorslists@googlemail.com>
> > 
> > Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
[...]
> > +static int ag71xx_ether_send(struct eth_device *edev, void *packet, int length)
> > +{
> > +	struct ag71xx *priv = edev->priv;
> > +	struct device_d *dev = priv->dev;
> > +	ag7240_desc_t *f = &priv->fifo_tx[priv->next_tx];
> > +	int i;
> > +
> 
> > +	/* flush */
> > +	dma_sync_single_for_device((unsigned long)packet, length, DMA_TO_DEVICE);
> > +
> > +	f->pkt_start_addr = virt_to_phys(packet);
> 
> A couple of remarks regarding this code:
> 
> Despite the fact that this code should work fine, it violates the Linux DMA 
> API, which the Barebox tends to conform to. The problem is that the Barebox is 
> missing dma_map_*() functions, which should be used instead of virt_to_phys() 
> here.
> 
> Another DMA-related Barebox issue is the type of the first argument to 
> dma_sync_*() functions, which should be dma_addr_t (just like the return value 
> of dma_map_*() functions), instead of unsigned long.  This is of no importance 
> for 32 bit  architectures, however it will break for 64 bit targets.

A slight correction: unsigned long will work fine for pure 64bit
targets, as it maps to a 64bit type there. The only way this could break
is on 32bit LPAE targets, where long is a 32bit type, but the DMA
addresses could be above the 4GB mark.

Regards,
Lucas


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox