lkml.org 
[lkml]   [2022]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 1/2] platform: make platform_get_irq_optional() optional
    On Sat, Jan 15, 2022 at 9:22 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote:
    > On 1/14/22 11:22 PM, Uwe Kleine-König wrote:
    > > You have to understand that for clk (and regulator and gpiod) NULL is a
    > > valid descriptor that can actually be used, it just has no effect. So
    > > this is a convenience value for the case "If the clk/regulator/gpiod in
    > > question isn't available, there is nothing to do". This is what makes
    > > clk_get_optional() and the others really useful and justifies their
    > > existence. This doesn't apply to platform_get_irq_optional().
    >
    > I do understand that. However, IRQs are a different beast with their
    > own justifications...

    > > clk_get_optional() is sane and sensible for cases where the clk might be
    > > absent and it helps you because you don't have to differentiate between
    > > "not found" and "there is an actual resource".
    > >
    > > The reason for platform_get_irq_optional()'s existence is just that
    > > platform_get_irq() emits an error message which is wrong or suboptimal
    >
    > I think you are very wrong here. The real reason is to simplify the
    > callers.

    Indeed.

    Even for clocks, you cannot assume that you can always blindly use
    the returned dummy (actually a NULL pointer) to call into the clk
    API. While this works fine for simple use cases, where you just
    want to enable/disable an optional clock (clk_prepare_enable() and
    clk_disable_unprepare()), it does not work for more complex use cases.

    Consider a device with multiple clock inputs, some of them optional,
    where the device driver has to find, select and configure a suitable
    clock to operate at a certain clock frequency. The driver can call
    clk_get_rate(NULL) fine, but will always receive a zero rate, so it
    has to check for this (regardless of this being a dummy clock or not,
    because this could be an unpopulated clock crystal, which would be
    described in DT as a (present) fixed-rate clock with clock-frequency
    = <0>).
    For configuring the clock rate, the driver does need to check
    explicitly for the presence of a dummy clock, as clk_set_rate(NULL,
    rate) returns 0 ("success"), while obviously it didn't do anything,
    and thus configuring the device to use that clock would cause breakage.

    You can check if the clock is a real clock or a dummy using
    "if (clk) ...".
    And you'd use the same pattern with platform_irq_get_optional() if it
    would return 0 if the IRQ was not found: "if (irq) ...".

    Gr{oetje,eeting}s,

    Geert

    --
    Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

    In personal conversations with technical people, I call myself a hacker. But
    when I'm talking to journalists I just say "programmer" or something like that.
    -- Linus Torvalds

    \
     
     \ /
      Last update: 2022-01-17 09:43    [W:2.929 / U:0.808 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site