From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932367AbbKDIrT (ORCPT ); Wed, 4 Nov 2015 03:47:19 -0500 Received: from mout.kundenserver.de ([212.227.17.10]:57510 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932223AbbKDIrR (ORCPT ); Wed, 4 Nov 2015 03:47:17 -0500 From: Arnd Bergmann To: Peter Ujfalusi Cc: Vinod Koul , dmaengine@vger.kernel.org, Dan Williams , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Sekhar Nori , Kevin Hilman , linux-omap@vger.kernel.org Subject: Re: [PATCH] dmaengine: edma: fix build without CONFIG_OF Date: Wed, 04 Nov 2015 09:46:41 +0100 Message-ID: <16166945.0Jm7Xdjkb9@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <5639B6EB.8030101@ti.com> References: <17811472.bY8CqmdEVy@wuerfel> <5639B6EB.8030101@ti.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:xnxeDs1U2rl7Rm4LAgwhJFMyrHH9/6JY6n20PPs/o03Z0+yS5Pm WFtMJxhEN4tOKAOdcB/eAIxtGAHYmEK83K3PbQExPiBEXnUxtlw4srdA07UQ3BLEfg/95g5 UnbzVsduVFJK0+FfyYnkPJ9YFMzbGVjBwabRLfNaWlBXAxv3d4L+dFZkqjazSIvuNLo2nfr hwZjAMrKKNnuoL7AlTzsQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:sMMwRWzOPag=:0MlfhZ1eFDksEWfVWjUwMf FPf73CkHpO9GQnN2laI6Er0jJe7rO4C9jIiUkMrweMiTHdL7NaLio5Ya52IVScpPd0O1Fu3Fs HYtppmE2O5/asLzxFXFd1G2/kqBiDHG0G0c7eXfczr+LVy/WJCpEyq3ulXhf+S0hy9wqL8x75 uzqb2feDrtG9iOyo+BRMqG8uhnVTbnQ4PHdT2f38mcqMf8HxWAZaAyJxq3so2F9QjezPTred2 VTt5EKKkZ6+YQ2MkEOVbKLnuDdTv1BZgUYv9GVXbUDnzmyA+Lk3o+TDubJXSD62mkPJagSMm4 yyldPFK1vCHKoHRkbuZ4zhvsks3vEq4VhL0d9+WxX+tZ/365pnMwbN7np8VsJmDd1gb8a3jR8 qUMGnMGVhRovAX/bV3xWAc+++8nK2msh456U7DWYHD0g6mYjwqZySBbO1DlQD+sbCTHb7uav8 sPFv1b/oDIkvcT5hYssoRxcsC5KqpGgqrRDTLxAUplXHHs/mifcg3Ob135utEqHVaD8SDjxsD YkYFVqdRmbPGxI1XH20W/xw7sEgC7lqPlPGd1AFCfQ+E+m/Qrd2/P5qEDUTivaX3gMRQ8U4rk Ji5bUTxG2EQau/SOpnZYqL7AqPeEordK4R+nogTq7mE0iBllUV5IgqXCeisUwa7r9l+fp89Nt f85xGxFlN5rA/IOxo2JAK0nGc09G/gvcnw8PHx8Jn7z4lDXy6N6LduPj8FF5I8PKetL5YD7ZJ 4vC140qsCNG3sf3/ Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 04 November 2015 09:42:35 Peter Ujfalusi wrote: > On 11/03/2015 04:00 PM, Arnd Bergmann wrote: > > During the edma rework, a build error was introduced for the > > case that CONFIG_OF is disabled: > > > > drivers/built-in.o: In function `edma_tc_set_pm_state': > > :(.text+0x43bf0): undefined reference to `of_find_device_by_node' > > > > As the edma_tc_set_pm_state() function does nothing in case > > we are running without OF, this adds an IS_ENABLED() check > > that turns the function into an empty stub then and avoids the > > link error. > > > > Signed-off-by: Arnd Bergmann > > Fixes: ca304fa9bb76 ("ARM/dmaengine: edma: Public API to use private struct pointer") > > The actual commit this patch is fixing is: > 1be5336bc7ba dmaengine: edma: New device tree binding That's what I first thought, but it seems to just move around the call to of_find_device_by_node that was first introduced in the commit I mentioned. Did you build-test it successfully with ca304fa9bb76 and CONFIG_OF enabled? I have to admit that I was just guessing from the contents and did not bisect this fully. > > --- > > Found on ARM randconfig builds with today's linux-next > > I have sanity built the kernel with omap2plus_defconfig and > davinci_all_defconfig since eDMA is used by these platforms and did not faced > with this issue, as obviously these defconfigs will result OF to be enabled. Right. The defconfigs were all fine, and this is hard to hit even in the randconfig builds. > > diff --git a/drivers/dma/edma.c b/drivers/dma/edma.c > > index 31722d436a42..16713a93da10 100644 > > --- a/drivers/dma/edma.c > > +++ b/drivers/dma/edma.c > > @@ -1560,7 +1560,7 @@ static void edma_tc_set_pm_state(struct edma_tc *tc, bool enable) > > struct platform_device *tc_pdev; > > int ret; > > > > - if (!tc) > > + if (!IS_ENABLED(CONFIG_OF) || !tc) > > return; > > Should we instead put the function inside of: > #if IS_ENABLED(CONFIG_OF) > static void edma_tc_set_pm_state(struct edma_tc *tc, bool enable) > { > ... > } > #else > static inline void edma_tc_set_pm_state(struct edma_tc *tc, bool enable) > { > } > #endif /* IS_ENABLED(CONFIG_OF) */ I think that would be less readable, and gives no compile-time coverage to the contents of the edma_tc_set_pm_state function. The effect is the same, so I'd rather stay with my version. Arnd