Re: [PATCH v3] Add bt8xxgpio driver

[Domenico Andreoli <cavokz@gmail.com>]

On Mon, Jul 14, 2008 at 07:51:39PM +0200, Michael Buesch wrote:
> On Monday 14 July 2008 17:25:50 Domenico Andreoli wrote:
> > On Mon, Jul 14, 2008 at 03:58:29PM +0200, Michael Buesch wrote:
> > > On Monday 14 July 2008 09:27:33 Domenico Andreoli wrote:
> > > > +static u32 nr_to_mask(struct bttv_gpiolib_device *dev, unsigned nr)
> > > > +{
> > > > +	u32 io_mask = dev->in_mask | dev->out_mask;
> > > > +	int shift = 0;
> > > > +
> > > > +	while(io_mask && nr) {
> > > > +		nr -= io_mask & 1;
> > > > +		io_mask >>= 1;
> > > > +		shift++;
> > > > +	}
> > > > +
> > > > +	return 1 << shift;
> > > > +}
> > > 
> > > This loop is really really weird.
> > > What the hell are you doing here?
> > > You ususally convert GPIO numbers to masks by doing (1 << nr), only.
> > 
> > gpiolib does not allow holes in the number space of gpios. once you
> > set chip.ngpio, you get a contiguous slice.
> > 
> > should the board have some of its gpio connected to something private,
> > they are not to be exported to gpiolib and to the user.
> > 
> > indeed once, as a user, I know to have a board which has n inputs and
> > m output and z in/out, that's all, I do not want to know how many GPIOs
> > actually are on the board and how are connected.
> > 
> > nr_to_mask() hides those holes to the user, it maps a pin iff it is
> > available for gpiolib fiddling.
> 
> Ok, I see. However, I'd suggest to implement this with a lookup table
> rather than this weird loop.

yes, of course. it was never meant to be definitive, all the patch was
a prototype. thanks for the support.

cheers,
Domenico

-----[ Domenico Andreoli, aka cavok
 --[ http://www.dandreoli.com/gpgkey.asc
   ---[ 3A0F 2F80 F79C 678A 8936  4FEE 0677 9033 A20E BC50

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@redhat.com?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list