Hi Uwe,
it seems to work... 8-)
Ok, the nesting level is decreased, but i think its not so good readable
any more. But who cares if it works *hehe*
cu Tommi
Am Freitag, den 05.03.2010, 20:25 +0100 schrieb Uwe Hermann:
> 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.
|
