[RFC PATCH 3/3] add hvc backend for rtas

[Milton Miller]

On Dec 16, 2005, at 6:10 PM, Arnd Bergmann wrote:

> Current Cell hardware is using the console through a set
> of rtas calls. This driver is needed to get console
> output on those boards.
> +#define RTASCONS_PUT_ATTEMPTS  16
> +
> +static int rtascons_put_char_token = RTAS_UNKNOWN_SERVICE;
> +static int rtascons_get_char_token = RTAS_UNKNOWN_SERVICE;
> +static int rtascons_put_delay = 100;
> +module_param(rtascons_put_delay, int, 0644);
> +
> +static inline int hvc_rtas_write_console(uint32_t vtermno, const char 
> *buf, int count)
> +{
> +	int attempts = RTASCONS_PUT_ATTEMPTS;
> +	int result;
> +	int done;
> +
> +	/* if there is more than one character to be displayed, wait a bit */
> +	for (done = 0; done < count && attempts; udelay(rtascons_put_delay)) 
> {
> +		attempts--;
> +		result = rtas_call(rtascons_put_char_token, 1, 1, NULL, buf[done]);
> +
> +		if (!result) {
> +			attempts = RTASCONS_PUT_ATTEMPTS;
> +			done++;
> +		}
> +       }
> +	/* the calling routine expects to receive the number of bytes sent */
> +	return done ?: 0;

this reduces to return done

> +}

Do we really know that we need a delay after every character written?  
Does firmware not tell us when it fails, and we could skip the delay 
when we make progress?

I would move the delay to the bottom of the loop and continue on 
success after the other processing.


Should we make RTAS_PUT_ATTEMPTS another module paramter?


> +
> +static inline int rtascons_get_char(void)
> +{
> +	int result;
> +
> +	if (rtas_call(rtascons_get_char_token, 0, 2, &result))
> +		result = -1;
> +
> +	return result;
> +}

This function doesn't hide much ...

> +
> +static int hvc_rtas_read_console(uint32_t vtermno, char *buf, int 
> count)
> +{
> +	unsigned long got;
> +	int c;
> +	int i;
> +
> +	for (got = 0, i = 0; i < count; i++) {
> +
> +		if (( c = rtascons_get_char() ) != -1) {

And the assignment in if statement doesn't make this more readable.

I would just expand rtas_get_char here, the if would check the return
of the function and c (already the correct size) wouuld be set as a 
side effect.


> +			buf[i] = c;
> +			++got;
> +		}
> +		else
> +			break;
> +	}

reversing the sense of the if will eliminate the need for braces on the 
if, as the else can just be the remainder of the loop.

> +	return got;
> +}
> +
>