lkml.org 
[lkml]   [2008]   [Jun]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.


\
 
 \ /
  Last update: 2008-06-23 19:35    [W:0.214 / U:0.568 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site