Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754174AbcJEJnP (ORCPT ); Wed, 5 Oct 2016 05:43:15 -0400 Received: from down.free-electrons.com ([37.187.137.238]:46763 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751097AbcJEJnN (ORCPT ); Wed, 5 Oct 2016 05:43:13 -0400 Subject: Re: [PATCH 03/14] ASoC: sun4i-i2s: Add apb reset To: Maxime Ripard , Thomas Petazzoni References: <2c5abe6578c8e4e841cb59357d88ce397551a593.1475571575.git.mylene.josserand@free-electrons.com> <20161004141516.53c6fa8c@free-electrons.com> <20161004154218.GM5228@lukather> Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, alsa-devel@alsa-project.org, vinod.koul@intel.com, linux-kernel@vger.kernel.org, mturquette@baylibre.com, tiwai@suse.com, sboyd@codeaurora.org, lgirdwood@gmail.com, robh+dt@kernel.org, perex@perex.cz, wens@csie.org, broonie@kernel.org, alexandre.belloni@free-electrons.com, dmaengine@vger.kernel.org, lee.jones@linaro.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org From: Mylene Josserand Message-ID: Date: Wed, 5 Oct 2016 11:43:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.2.0 MIME-Version: 1.0 In-Reply-To: <20161004154218.GM5228@lukather> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1979 Lines: 71 Hello, On 04/10/2016 17:42, Maxime Ripard wrote: > Hi, > > On Tue, Oct 04, 2016 at 02:15:16PM +0200, Thomas Petazzoni wrote: >> Hello, >> >> On Tue, 4 Oct 2016 11:46:16 +0200, Myl?ne Josserand wrote: >> >>> #include >>> #include >>> @@ -589,6 +590,7 @@ static int sun4i_i2s_probe(struct platform_device *pdev) >>> { >>> struct sun4i_i2s *i2s; >>> struct resource *res; >>> + struct reset_control *reset_apb; >>> void __iomem *regs; >>> int irq, ret; >>> >>> @@ -626,7 +628,19 @@ static int sun4i_i2s_probe(struct platform_device *pdev) >>> dev_err(&pdev->dev, "Can't get our mod clock\n"); >>> return PTR_ERR(i2s->mod_clk); >>> } >>> - >>> + >>> + reset_apb = devm_reset_control_get(&pdev->dev, "apb_reset"); >> >> I believe this is a change in the Device Tree binding, since you're >> adding support for a new resource. Perhaps the Device Tree binding >> documentation should be updated accordingly? > > Indeed. > > You have two solutions to do that: > - Either mark it as optional and use reset_control_get_optional > (because here, you broke the other SoCs that have that controller > but no reset line) > - Or introduce a new compatible, and make the reset property > mandatory for that new compatible. > > I prefer the latter, since you get a stricter error check, and you > cannot end up in a situation where your driver probes but is > useless. But you'll find both in our drivers. > Okay, thank you for the hints! >>> + } >>> + >>> + ret = reset_control_deassert(reset_apb); >>> + if (ret < 0) { >>> + dev_err(&pdev->dev, "Can't deassert apb reset (%d)\n", ret); >>> + return ret; >>> + } >> >> Do you need to re-assert the reset line in the ->remove() hook? > > Even better, you can add it to the runtime_pm hooks! :) I will have a look to runtime_pm and update it for a V2. -- Myl?ne Josserand, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com