[U-Boot-Users] [PATCH v2] QE IO: Add initial data to pin configuration + read/write functions

[Timur Tabi <timur@freescale.com>]

David Saada wrote:

> @@ -38,6 +38,16 @@ void qe_config_iopin(u8 port, u8 pin, in
>  	volatile par_io_t	*par_io = (volatile par_io_t *)
>  						&(gur->qe_par_io);
>  
> +	/* Calculate pin location for 1bit mask */
> +	pin_1bit_mask = (u32)(1 << (NUM_OF_PINS - (pin+1)));
> +
> +	/* Setup the data */
> +	tmp_val = in_be32(&par_io[port].cpdat);
> +	if (data)
> +		out_be32(&par_io[port].cpdat, pin_1bit_mask | tmp_val);
> +	else
> +		out_be32(&par_io[port].cpdat, ~pin_1bit_mask & tmp_val);
> +

I see that qe_config_iopin() will always write a 0 or 1 now, ignoring the 
current value.  Are you sure this is a good idea?  What if I don't want U-Boot 
to change the current pin value?

Plus, doesn't this assume that the pin is set to output?  Shouldn't you check to 
see if the pin is output or input/output, and only write cpdat if it is?  Also, 
I believe that cpdat is used only if the pin is configured as a GPIO.  If so, I 
don't see you check for that, either.

-- 
Timur Tabi
Linux kernel developer at Freescale