lkml.org 
[lkml]   [2007]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/3] Maple bus support for the Sega Dreamcast
    On 09/09/07, Arjan van de Ven <arjan@infradead.org> wrote:
    > On Sun, 9 Sep 2007 17:46:54 +0100
    > "Adrian McMenamin" <lkmladrian@gmail.com> wrote:
    >
    > > This patch adds support for Sega's proprietary Maple bus - which is
    > > required to support the Dreamcast's peripherals.
    >

    >
    > First of all, I'm a little concerned about the lack of locking that
    > this driver seems to have; what guarantees the integrity of the lists
    > used in the driver?
    >
    Could you explain what you think the issue is? The lists are only
    handled by this driver and not anything else.


    > Second, you have a lot of use of likely() and unlikely(); so much that I think it's waaaay overkill. The code it's in generally isn't hotpath, and gcc also does a pretty decent job without giving these hints in general.
    >
    >
    >

    Fair enough.

    > > +
    > > +void maple_add_packet(struct mapleq *mq)
    > > +{
    > > + list_add((struct list_head *) mq, &maple_waitq);
    > > +}
    >
    > for example this list.. what makes sure that no 2 pieces of code muck with it at the same time?
    >
    >

    Because everything is handled by one kernel thread on a uniprocessor machine.


    > > +static irqreturn_t maplebus_dma_interrupt(int irq, void *dev_id)
    > > +{
    > > + /* Load everything into the bottom half */
    > > + schedule_work(&maple_dma_process);
    > > + return IRQ_HANDLED;
    > > +}
    >
    > I wonder if you want to at least check if the work was really for you before returning IRQ_HANDLED....
    >

    It's *always* for me - this is an end of DMA interrupt for the bus.
    -
    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: 2007-09-09 20:39    [W:0.024 / U:0.616 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site