Re: [libopenstm32-devel] gpio.c problems

[Uwe H. <uw...@he...>]

Hi,

thanks for the patch!

On Sun, Feb 28, 2010 at 01:35:44PM +0100, Thomas Otto wrote:
> I had 2 problems with the function gpio_set_mode().
> 1. it generally sets the submited config to the mentioned gpio-pins but
> kills configs for other pins on the same gpio port. So if i want to set
> PB6 and PB7 to push-pull and I2C2 SDA and SCL (PB10 and PB11) to open
> drain its simply impossible, because the second config try kills the
> first.

Yep, that was a known issue / TODO indeed.


> 2. the floating-bit thing isnt working correctly. if i enable a config
> for PB6 for instance the same config will also apply to all following
> pins of that port (aka PB7-PB15). Thats because the shifting isnt only
> done if a pin isnt to configure, if you are hitting a matching bit the
> shiftig is missing. I think shifting isnt nessessary for a separate
> variable. We have the counting index from the for statement.

And this one was probably "just" a bug :)


>  	for (i = 0; i < 16; i++) {
> -		if ((movingbit & gpios) != movingbit) {
> -			movingbit <<= 1;
> -			continue;
> +		/* only set the config if the bit is set in gpios */
> +		if ((1 << i) & gpios) {
> +			offset = (i < 8) ? (i * 4) : ((i - 8) * 4);
> +			if (i < 8) {
> +				crl &= ~(0b1111 << offset);  /* cleaning first */
> +				crl |= (mode << offset) | (cnf << (offset + 2));
> +			}
> +			else {
> +				crh &= ~(0b1111 << offset);  /* cleaning first */
> +				crh |= (mode << offset) | (cnf << (offset + 2));
> +			}

I committed a slightly modified version of your patch, which should be
functionally equivalent though. I tried to reduce the nesting level a bit.
The code is admittedly untested, please let me know if I introduced more bugs.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.randomprojects.org
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org