Re: [PATCH v3 2/2] Staging: octeon-usb: Changes macro CVMX_WAIT_FOR_FIELD32 to function call.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Sun, Jul 29, 2018 at 02:40:35PM +0300, Georgios Tsotsos wrote:
> +/**
> + * Loop through register until txfflsh or rxfflsh become zero.
> + *
> + * @usb:           USB block
> + * @address:       64bit address to read
> + * @timeout_usec:  Timeout
> + * @fflsh_type:    Indicates fflsh type, 0 for txfflsh, 1 for rxfflsh
> + *
> + */
> +static int cvmx_wait_for_field32(struct octeon_hcd *usb, u64 address,
> +				 u64 timeout_usec, int fflsh_type)

You should change the name of the function, as it's no longer generic
"wait for any field" to reach specified condition.

The "address" should be calculated from "usb" inside the function.

The timeout is always 100, you could just omit it.

Nobody is ever checking the result, so it's also redundant...

[...]

> +{
> +	int result;
> +	u64 done = cvmx_get_cycle() + timeout_usec *
> +		   (u64)octeon_get_clock_rate / 1000000;
> +
> +	union cvmx_usbcx_grstctl c;
> +
> +	while (1) {
> +		c.u32 = cvmx_usb_read_csr32(usb, address);
> +		if (fflsh_type == 0 && c.s.txfflsh == 0) {
> +			result = 0;
> +			break;
> +		} else if (fflsh_type == 1 && c.s.rxfflsh == 0) {
> +			result = 0;
> +			break;
> +		} else if (cvmx_get_cycle() > done) {
> +			result = -1;
> +			break;
> +		}
> +
> +		__delay(100);
> +	}
> +	return result;
> +}
> +
>  static void cvmx_fifo_setup(struct octeon_hcd *usb)
>  {
>  	union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3;
> @@ -635,11 +648,9 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb)
>  	/* Flush all FIFOs */
>  	USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfnum, 0x10);
>  	USB_SET_FIELD32(address, cvmx_usbcx_grstctl, txfflsh, 1);
> -	CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
> -			      c.s.txfflsh == 0, 100);
> +	cvmx_wait_for_field32(usb, address, 0, 100);

How did you test this patch? The parameters are in wrong order, you are
specifying 0 timeout.

>  	USB_SET_FIELD32(address, cvmx_usbcx_grstctl, rxfflsh, 1);
> -	CVMX_WAIT_FOR_FIELD32(address, cvmx_usbcx_grstctl,
> -			      c.s.rxfflsh == 0, 100);
> +	cvmx_wait_for_field32(usb, address, 1, 100);

Also wrong order here.

A.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux