On Monday, October 19, 2015 at 04:36:21 AM, Thomas Chou wrote:
Hi!
> Convert altera_tse to driver model and phylib.
>
> Signed-off-by: Thomas Chou <tho...@wytron.com.tw>
> ---
> configs/nios2-generic_defconfig | 2 +
> doc/device-tree-bindings/net/altera_tse.txt | 112 ++++
> drivers/net/Kconfig | 9 +
> drivers/net/altera_tse.c | 938
> ++++++++++------------------ drivers/net/altera_tse.h |
> 283 +++------
> include/configs/nios2-generic.h | 8 +
> 6 files changed, 536 insertions(+), 816 deletions(-)
> create mode 100644 doc/device-tree-bindings/net/altera_tse.txt
>
> diff --git a/configs/nios2-generic_defconfig
[...]
> /* sgdma debug - print descriptor */
> static void alt_sgdma_print_desc(volatile struct alt_sgdma_descriptor
> *desc) {
> debug("SGDMA DEBUG :\n");
> - debug("desc->source : 0x%x \n", (unsigned int)desc->source);
> - debug("desc->destination : 0x%x \n", (unsigned int)desc->destination);
> - debug("desc->next : 0x%x \n", (unsigned int)desc->next);
> - debug("desc->source_pad : 0x%x \n", (unsigned int)desc->source_pad);
> - debug("desc->destination_pad : 0x%x \n",
> - (unsigned int)desc->destination_pad);
> - debug("desc->next_pad : 0x%x \n", (unsigned int)desc->next_pad);
> - debug("desc->bytes_to_transfer : 0x%x \n",
> - (unsigned int)desc->bytes_to_transfer);
> - debug("desc->actual_bytes_transferred : 0x%x \n",
> - (unsigned int)desc->actual_bytes_transferred);
> - debug("desc->descriptor_status : 0x%x \n",
> - (unsigned int)desc->descriptor_status);
> - debug("desc->descriptor_control : 0x%x \n",
> - (unsigned int)desc->descriptor_control);
> + debug("desc->source : 0x%x\n", (unsigned int)desc->source);
> + debug("desc->destination : 0x%x\n", (unsigned int)desc->destination);
> + debug("desc->next : 0x%x\n", (unsigned int)desc->next);
> + debug("desc->source_pad : 0x%x\n", desc->source_pad);
> + debug("desc->destination_pad : 0x%x\n", desc->destination_pad);
> + debug("desc->next_pad : 0x%x\n", desc->next_pad);
> + debug("desc->bytes_to_transfer : 0x%x\n", desc->bytes_to_transfer);
> + debug("desc->actual_bytes_transferred : 0x%x\n",
> + desc->actual_bytes_transferred);
> + debug("desc->descriptor_status : 0x%x\n", desc->descriptor_status);
> + debug("desc->descriptor_control : 0x%x\n", desc->descriptor_control);
> }
Looks like a cleanup part of the patch, not a conversion, so you might want
to split this out. Btw. while at it, please drop those (unsigned int) casts.
> -/* This is a generic routine that the SGDMA mode-specific routines
> +/*
> + * This is a generic routine that the SGDMA mode-specific routines
> * call to populate a descriptor.
> * arg1 :pointer to first SGDMA descriptor.
> * arg2 :pointer to next SGDMA descriptor.
> @@ -108,7 +107,6 @@ static void alt_sgdma_construct_descriptor_burst(
> * pointing at this descriptor, it will not run (via the "owned by
> * hardware" bit) until all other descriptor has been set up.
> */
> -
> desc->descriptor_control =
> ((ALT_SGDMA_DESCRIPTOR_CONTROL_OWNED_BY_HW_MSK) |
> (generate_eop ?
> @@ -121,18 +119,19 @@ static void alt_sgdma_construct_descriptor_burst(
> );
> }
>
> -static int alt_sgdma_do_sync_transfer(volatile struct alt_sgdma_registers
> *dev, - volatile struct alt_sgdma_descriptor
*desc)
> +static int alt_sgdma_do_sync_transfer(
> + volatile struct alt_sgdma_registers *regs,
> + volatile struct alt_sgdma_descriptor *desc)
The volatiles are almost certainly wrong, so they should go.
Oh wait, is this driver NOT using any I/O accessors ? :-O
> {
> unsigned int status;
> int counter = 0;
>
> /* Wait for any pending transfers to complete */
> alt_sgdma_print_desc(desc);
> - status = dev->status;
> + status = regs->status;
>
> counter = 0;
> - while (dev->status & ALT_SGDMA_STATUS_BUSY_MSK) {
> + while (regs->status & ALT_SGDMA_STATUS_BUSY_MSK) {
> if (counter++ > ALT_TSE_SGDMA_BUSY_WATCHDOG_CNTR)
> break;
> }
> @@ -144,12 +143,12 @@ static int alt_sgdma_do_sync_transfer(volatile struct
> alt_sgdma_registers *dev, * Clear any (previous) status register
> information
> * that might occlude our error checking later.
> */
> - dev->status = 0xFF;
> + regs->status = 0xFF;
>
> /* Point the controller at the descriptor */
> - dev->next_descriptor_pointer = (unsigned int)desc & 0x1FFFFFFF;
> + regs->next_descriptor_pointer = (unsigned int)desc & 0x1FFFFFFF;
This looks pretty wrong, with the cast and all.
> debug("next desc in sgdma 0x%x\n",
> - (unsigned int)dev->next_descriptor_pointer);
> + (unsigned int)regs->next_descriptor_pointer);
>
> /*
> * Set up SGDMA controller to:
> @@ -157,30 +156,31 @@ static int alt_sgdma_do_sync_transfer(volatile struct
> alt_sgdma_registers *dev, * - Run once a valid descriptor is written to
> controller
> * - Stop on an error with any particular descriptor
> */
> - dev->control = (ALT_SGDMA_CONTROL_RUN_MSK |
> + regs->control = (ALT_SGDMA_CONTROL_RUN_MSK |
> ALT_SGDMA_CONTROL_STOP_DMA_ER_MSK);
>
> /* Wait for the descriptor (chain) to complete */
> - status = dev->status;
> + status = regs->status;
> debug("wait for sgdma....");
> - while (dev->status & ALT_SGDMA_STATUS_BUSY_MSK)
> + while (regs->status & ALT_SGDMA_STATUS_BUSY_MSK)
> ;
Eventually, this unbounded wait should be nuked and replaced with a proper
wait with timeout. In case you're up to it, that'd be real nice.
> debug("done\n");
>
> /* Clear Run */
> - dev->control = (dev->control & (~ALT_SGDMA_CONTROL_RUN_MSK));
> + regs->control = (regs->control & (~ALT_SGDMA_CONTROL_RUN_MSK));
>
> /* Get & clear status register contents */
> - status = dev->status;
> - dev->status = 0xFF;
> + status = regs->status;
> + regs->status = 0xFF;
>
> /* we really should check if the transfer completes properly */
> debug("tx sgdma status = 0x%x", status);
> return 0;
> }
[...]
> -static int tse_eth_rx(struct eth_device *dev)
> +static void tse_eth_rx_setup(struct altera_tse_priv *priv)
> +{
> + uchar *rx_buf = net_rx_packets[priv->rx_cur & 1];
> + volatile struct alt_sgdma_descriptor *rx_desc = priv->rx_desc;
> +
> + invalidate_dcache_range((unsigned long)rx_buf,
> + (unsigned long)rx_buf + PKTSIZE_ALIGN);
> + alt_sgdma_construct_descriptor_burst(
> + &rx_desc[0],
> + &rx_desc[1],
> + (unsigned int)0x0, /* read addr */
> + (unsigned int *)rx_buf,
> + 0x0, /* length or EOP */
> + 0x0, /* gen eop */
> + 0x0, /* read fixed */
> + 0x0, /* write fixed or sop */
> + 0x0, /* read burst */
> + 0x0, /* write burst */
> + 0x0 /* channel */
You might want to pass the args as a structure instead of having a function
with billion arguments. But that's just a suggestion for improvement.
> + );
> +
> + /* setup the sgdma */
> + alt_sgdma_do_async_transfer(priv->sgdma_rx, &rx_desc[0]);
> +}
[...]
> +static int altera_tse_probe(struct udevice *dev)
> +{
> + struct eth_pdata *pdata = dev_get_platdata(dev);
> + struct altera_tse_priv *priv = dev_get_priv(dev);
> + const void *blob = gd->fdt_blob;
> + int node = dev->of_offset;
> + const char *list, *end;
> + const fdt32_t *cell;
> + void *base, *desc_mem = NULL;
> + unsigned long addr, size;
> + int len, idx;
> + int ret;
> +
> + /* decode regs, assume address-cells and size-cells are both one */
> + list = fdt_getprop(blob, node, "reg-names", &len);
> + if (!list)
> + return -ENOENT;
> + end = list + len;
> + cell = fdt_getprop(blob, node, "reg", &len);
> + if (!cell)
> + return -ENOENT;
> + idx = 0;
> + while (list < end) {
> + addr = fdt_translate_address((void *)blob,
> + node, cell + idx);
> + size = fdt_addr_to_cpu(cell[idx + 1]);
> + base = ioremap(addr, size);
> + len = strlen(list);
> + if (strcmp(list, "control_port") == 0)
> + priv->mac_dev = base;
fdtdec_get_addr() won't do here?
> + else if (strcmp(list, "rx_csr") == 0)
> + priv->sgdma_rx = base;
> + else if (strcmp(list, "tx_csr") == 0)
> + priv->sgdma_tx = base;
> + else if (strcmp(list, "s1") == 0)
> + desc_mem = base;
> + idx += 2;
> + list += (len + 1);
> }
[...]
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
