lkml.org 
[lkml]   [2011]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/4] arch/arm/mach-at91/clock.c: Add missing IS_ERR test
    On 01/25/2011 10:31 AM, Julia Lawall wrote:
    > On Tue, 25 Jan 2011, Ryan Mallon wrote:
    >
    >> On 01/25/2011 10:01 AM, Julia Lawall wrote:
    >>> On Tue, 25 Jan 2011, Ryan Mallon wrote:
    >>>
    >>>> On 01/25/2011 09:28 AM, Julia Lawall wrote:
    >>>>>> Julia is correct. Some architectures can return NULL from clk_get, but I
    >>>>>> didn't check the at91 before posting :-/. If we can't return NULL from
    >>>>>> clk_get then we shouldn't bother checking for it. I do think we should
    >>>>>> drop the !IS_ERR(clk_get(dev, func)) check though.
    >>>>>
    >>>>> It seems a bit subtle, because the clk manipulated by clk_get in the call
    >>>>> of clk_get(dev, func) is not necessarily the same as the one in
    >>>>> clock_associate. But perhaps this is the only possibility in practice?
    >>>>
    >>>> Not sure I follow. The at91 clk_get does not modify the clk. In
    >>>> at91_clock_associate we have:
    >>>>
    >>>> clk->function = func;
    >>>> clk->dev = dev;
    >>>>
    >>>> and in clk_get we have:
    >>>>
    >>>> if (clk->function && (dev == clk->dev) &&
    >>>> strcmp(id, clk->function) == 0)
    >>>> return clk;
    >>>>
    >>>> So at91_clock_associate sets the function for a clock, and clk_get
    >>>> returns clocks based on the function association if the name lookup
    >>>> fails. The only caveat to this is that the the clock function name
    >>>> (clk->function) is not the same as any others clock's clk->name.
    >>>
    >>> Right, that was what I was worried about. That one would find the same
    >>> information already present but somewhere else. But perhaps it can't
    >>> happen, or it doesn't matter if it does?
    >>
    >> I think that users are expected to ensure that clock names and clock
    >> function names do not overlap.
    >
    > One can't have a clock with a different name but the same device and
    > function?

    You could, but it would not be helpful. Clock associations are used so
    that _different_ devices can have the same function and map to the
    correct clock. This is used when there are multiple instances of a
    single peripheral. For example, the uart clocks work like this:

    at91_clock_associate("usart1_clk", &pdev->dev, "usart");

    so then you can do this in a driver:

    uart_clk = clk_get(&pdev->dev, "usart");

    Rather than:

    uart_clk = clk_get(NULL, "usart1_clk");

    The former will find the correct uart clock for the device. Because each
    uart is a separate device the correct clock will be selected for each uart.

    My point was that there should be no overlap between clk->name and
    clk->function otherwise clk_get will not be able to return the correct
    clock.

    ~Ryan

    --
    Bluewater Systems Ltd - ARM Technology Solution Centre

    Ryan Mallon 5 Amuri Park, 404 Barbadoes St
    ryan@bluewatersys.com PO Box 13 889, Christchurch 8013
    http://www.bluewatersys.com New Zealand
    Phone: +64 3 3779127 Freecall: Australia 1800 148 751
    Fax: +64 3 3779135 USA 1800 261 2934


    \
     
     \ /
      Last update: 2011-01-24 22:53    [W:0.025 / U:92.148 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site