<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <p><span style="color: rgb(18, 23, 50); font-family: "Microsoft YaHei", arial, sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: pre-wrap; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(247, 248, 250); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;">Hi  Krzysztof,</span><span style="color: rgb(18, 23, 50); font-family: "Microsoft YaHei", arial, sans-serif; font-size: 14px; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; white-space: pre-wrap; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; background-color: rgb(247, 248, 250); text-decoration-style: initial; text-decoration-color: initial; display: inline !important; float: none;"></span></p>
    <div class="moz-cite-prefix">On 2023/6/30 19:11, Thomas Gleixner
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:87h6qpyzkd.ffs@tglx">
      <pre class="moz-quote-pre" wrap="">On Tue, Jun 27 2023 at 13:00, Uwe Kleine-König wrote:
</pre>
      <blockquote type="cite">
        <pre class="moz-quote-pre" wrap="">On Tue, Jun 27, 2023 at 06:12:01PM +0800, Yangtao Li wrote:

While I assume changing to dev_err_probe is a result of my concern that
no error should be printed when rc=-EPROBEDEFER, my other concern that
adding an error message to a generic allocation function is a bad idea
still stands.
</pre>
      </blockquote>
      <pre class="moz-quote-pre" wrap="">
I agree in general, but if you actually look at the call sites of
devm_request_threaded_irq() then the vast majority of them print more or
less lousy error messages. A quick grep/sed/awk/sort/uniq revealed

     519 messages total (there are probably more)

     352 unique messages

     323 unique messages after lower casing

         Those 323 are mostly just variants of the same patterns with slight
         modifications in formatting and information provided.

     186 of these messages do not deliver any useful information,
         e.g. "no irq", "

     The most useful one of all is: "could request wakeup irq: %d"

So there is certainly an argument to be made that this particular
function should print a well formatted and informative error message.

It's not a general allocator like kmalloc(). It's specialized and in the
vast majority of cases failing to request the interrupt causes the
device probe to fail. So having proper and consistent information why
the device cannot be used _is_ useful.

Yangtao: The way how this is attempted is not useful at all.

    1) The changelog is word salad and provides 0 rationale

       Also such series require a cover letter...

    2) The dev_err() which is added is not informative at all and cannot
       replace actually useful error messages. It's not that hard to
       make it useful.

    2) Adding the printks unconditionally first will emit two messages
       with different content.

       This is not how such changes are done.

       The proper approach is to create a wrapper function which emits
       the error message:

       wrapper(....., const char *info)
       {
            ret = devm_request_threaded_irq(....);
            if (ret < 0) {
               dev_err(dev, "Failed to request %sinterrupt %u %s %s: %d\n,
                       thread_fn ? "threaded " : "",
                       irq, devname, info ? : "", ret);
            }
            return ret;
       }
</pre>
    </blockquote>
    <p><br>
    </p>
    <p>Here.<br>
      <br>
      V3 was modified according to tglx's suggestion, if there is any
      problem, please point out.<br>
      <br>
      <br>
      Thx,<br>
      Yangtao<br>
    </p>
    <p><br>
    </p>
    <blockquote type="cite" cite="mid:87h6qpyzkd.ffs@tglx">
      <pre class="moz-quote-pre" wrap="">
       Then convert the callsites over one by one with proper
       changelogs and justification.

       See?

Thanks,

        tglx
</pre>
    </blockquote>
  </body>
</html>