Messages in this thread | | | Date | Mon, 23 Jun 2008 20:32:54 +0300 | From | "Pekka Enberg" <> | Subject | Re: [PATCH][resubmit] HP iLO driver |
| |
Hi David,
Some minor nitpicks follow.
On Mon, Jun 23, 2008 at 7:00 PM, David Altobelli <david.altobelli@hp.com> wrote: > + > +/* > + * FIFO queues, shared with hardware. > + * > + * If a queue has empty slots, an entry is added to the queue tail, > + * and that entry is marked as occupied. > + * Entries can be dequeued from the head of the list, when the device > + * has marked the entry as consumed. > + * > + * Returns true on successful queue/dequeue, false on failure. > + */ > +static int fifo_enqueue(struct ilo_hwinfo *hw, char *fifobar, int entry) > +{ > + struct fifo *Q = FIFOBARTOHANDLE(fifobar);
The upper case Q is not a proper local variable name (appears elsewhere as well).
> diff -urpN linux-2.6.26-rc7.orig/drivers/misc/hpilo.h linux-2.6.26-rc7/drivers/misc/hpilo.h > --- linux-2.6.26-rc7.orig/drivers/misc/hpilo.h 1969-12-31 18:00:00.000000000 -0600 > +++ linux-2.6.26-rc7/drivers/misc/hpilo.h 2008-06-23 08:52:25.000000000 -0500
> +#define IS_DEVICE_RESET(hw) (ioread32(&(hw)->mmio_vaddr[DB_OUT]) & (1 << 26)) > +/* clear the device (reset bits, pending channel entries) */ > +#define CLEAR_DEVICE(hw) (iowrite32(-1, &(hw)->mmio_vaddr[DB_OUT]))
> +#define DESC_MEM_SZ(_descs) ((_descs) << L2_QENTRY_SZ)
> +#define CTRL_SET(_c, _l, _f, _d, _a, _g) \ > + ((_c) = \ > + (((_l) << CTRL_BITPOS_L2SZ) |\ > + ((_f) << CTRL_BITPOS_FIFOINDEXMASK) |\ > + ((_d) << CTRL_BITPOS_DESCLIMIT) |\ > + ((_a) << CTRL_BITPOS_A) |\ > + ((_g) << CTRL_BITPOS_G))) > +
> +#define DOORBELL_SET(_ccb) (iowrite8(1, (_ccb)->ccb_u5.db_base)) > +#define DOORBELL_CLR(_ccb) (iowrite8(2, (_ccb)->ccb_u5.db_base))
> +#define FIFOBARTOHANDLE(_fifo) \ > + ((struct fifo *)(((char *)(_fifo)) - FIFOHANDLESIZE)) > + > +/* set a flag indicating this channel needs a reset */ > +#define SET_CHANNEL_RESET(_ccb) \ > + (FIFOBARTOHANDLE((_ccb)->ccb_u1.send_fifobar)->reset = 1) > + > +/* check for this particular channel needing a reset */ > +#define IS_CHANNEL_RESET(_ccb) \ > + (FIFOBARTOHANDLE((_ccb)->ccb_u1.send_fifobar)->reset) > + > +/* overall size of a fifo is determined by the number of entries it contains */ > +#define FIFO_SZ(_num) (((_num)*sizeof(u64)) + FIFOHANDLESIZE)
> +#define GETQWORDS(_e) (((_e) & ENTRY_MASK_QWORDS) >> ENTRY_BITPOS_QWORDS) > +#define GETDESC(_e) (((_e) & ENTRY_MASK_DESCRIPTOR) >> ENTRY_BITPOS_DESCRIPTOR) > + > +#define QWORDS(_B) (((_B) & 7) ? (((_B) >> 3) + 1) : ((_B) >> 3))
Static inline functions are preferred over function-like macros.
| |