From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bjorn Andersson Subject: Re: [PATCH] remoteproc: qcom: q6v5: shore up resource probe handling Date: Tue, 9 Oct 2018 23:19:20 -0700 Message-ID: <20181010061920.GS12063@builder> References: <20181009222527.74260-1-briannorris@chromium.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: Doug Anderson Cc: Brian Norris , ohad@wizery.com, linux-remoteproc@vger.kernel.org, LKML , linux-arm-msm , sibis@codeaurora.org List-Id: linux-arm-msm@vger.kernel.org On Tue 09 Oct 16:34 PDT 2018, Doug Anderson wrote: > Hi, > > On Tue, Oct 9, 2018 at 3:33 PM Brian Norris wrote: > > + if (q6v5->wdog_irq < 0) { > > + if (q6v5->wdog_irq != -EPROBE_DEFER) > > + dev_err(&pdev->dev, > > + "failed to retrieve wdog IRQ: %d\n", > > + q6v5->wdog_irq); > > + return q6v5->wdog_irq; > > + } > > optional: Since there's the same pattern 5 times here, I wonder if we > should abstract it out to a helper function that would print the > error? > Before breaking this code out to a common function shared between the various q6v5 drivers this was a separate helper function, but that wasn't pretty either :/ > ...in the ideal case it would be somewhere that all Linux drivers > could use since this is a super common pattern, but that might be a > bit too much yak shaving... > Yeah I agree, it would be a nice helper but there's a bunch of variants involved. I've added it on the "someday/maybe" list. > > > @@ -237,8 +260,13 @@ int qcom_q6v5_init(struct qcom_q6v5 *q6v5, struct platform_device *pdev, > > disable_irq(q6v5->handover_irq); > > > > q6v5->stop_irq = platform_get_irq_byname(pdev, "stop-ack"); > > - if (q6v5->stop_irq == -EPROBE_DEFER) > > - return -EPROBE_DEFER; > > + if (q6v5->stop_irq < 0) { > > + if (q6v5->stop_irq != -EPROBE_DEFER) > > + dev_err(&pdev->dev, > > + "failed to retrieve stop IRQ: %d\n", > > + q6v5->stop_irq); > > + return q6v5->stop_irq; > > Nitty nit that it's the "stop-ack" IRQ, not the "stop" IRQ. The error > message below this one calls it stop-ack too so it would be ideal to > keep it consistent between the two error messages. > > Reviewed-by: Douglas Anderson I applied the patch with the nit fixed and your r-b. Thanks, Bjorn