lkml.org 
[lkml]   [2013]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] Bluetooth: Add hci_h4p driver
    From
    Date
    On Fri, 2013-10-18 at 12:30 +0200, Pali Rohár wrote:

    > I rebased patch on top of https://git.kernel.org/cgit/linux/kernel/git/bluetooth/bluetooth-next.git branch master

    Hi Pali, just some trivial notes:

    []

    +static ssize_t hci_h4p_show_bdaddr(struct device *dev,
    > + struct device_attribute *attr, char *buf)
    > +{
    > + struct hci_h4p_info *info = dev_get_drvdata(dev);
    > +
    > + return sprintf(buf, "%.2x:%.2x:%.2x:%.2x:%.2x:%.2x\n",
    > + info->bd_addr[0], info->bd_addr[1], info->bd_addr[2],
    > + info->bd_addr[3], info->bd_addr[4], info->bd_addr[5]);

    sprintf(buf, "%pM", info->bd_addr)

    and if this is really bluetooth, does the output need to
    be emitted in reverse order? ie: %pMR

    []

    > +#define NBT_DBG(fmt, arg...) \
    > + pr_debug("%s: " fmt "" , __func__ , ## arg)
    > +
    > +#define NBT_DBG_FW(fmt, arg...) \
    > + pr_debug("%s: " fmt "" , __func__ , ## arg)
    > +
    > +#define NBT_DBG_POWER(fmt, arg...) \
    > + pr_debug("%s: " fmt "" , __func__ , ## arg)
    > +
    > +#define NBT_DBG_TRANSFER(fmt, arg...) \
    > + pr_debug("%s: " fmt "" , __func__ , ## arg)
    > +
    > +#define NBT_DBG_TRANSFER_NF(fmt, arg...) \
    > + pr_debug(fmt "" , ## arg)
    > +
    > +#define NBT_DBG_DMA(fmt, arg...) \
    > + pr_debug("%s: " fmt "" , __func__ , ## arg)

    The "" isn't useful.

    dynamic_debugging can add __func__ to each message output
    with +f.

    I think all of these should be converted to pr_debug
    where used or consolidated into a single
    #define nbt_dbg(mask, fmt, ...) \
    do { \
    if (mask & debug) \
    pr_debug(fmt, ##__VA_ARGS__);
    } while (0)

    and used like:
    nbt_dbg(TRANSFER, fmt, etc...);
    where debug is some static.

    Also there are many uses missing "\n" which can
    cause interleaving problems with other printks.

    []

    > +int hci_h4p_wait_for_cts(struct hci_h4p_info *info, int active,
    > + int timeout_ms)
    > +{
    > + unsigned long timeout;
    > + int state;
    > +
    > + timeout = jiffies + msecs_to_jiffies(timeout_ms);
    > + for (;;) {

    while (time_before(jiffies, timeout)) {

    > + state = hci_h4p_inb(info, UART_MSR) & UART_MSR_CTS;
    > + if (active) {
    > + if (state)
    > + return 0;
    > + } else {
    > + if (!state)
    > + return 0;
    > + }
    > + if (time_after(jiffies, timeout))
    > + return -ETIMEDOUT;

    > + msleep(1);
    > + }

    return -ETIMEDOUT;

    > +}



    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2013-10-24 21:01    [W:0.029 / U:93.732 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site