From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Subject: Re: [PATCH v2 04/13] rc: sunxi-cir: Add support for an optional reset controller Date: Mon, 19 Jan 2015 15:17:03 +0100 Message-ID: <54BD11DF.1020907@redhat.com> References: <1418836704-15689-1-git-send-email-hdegoede@redhat.com> <1418836704-15689-5-git-send-email-hdegoede@redhat.com> <54929552.8090707@redhat.com> <20141219181708.GQ4820@lukather> <54954D5B.2020904@redhat.com> Reply-To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Return-path: In-Reply-To: List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , To: Chen-Yu Tsai , Maxime Ripard Cc: Linus Walleij , Lee Jones , Samuel Ortiz , Mike Turquette , Linux Media Mailing List , linux-arm-kernel , devicetree , linux-sunxi List-Id: devicetree@vger.kernel.org Hi, On 19-01-15 15:10, Chen-Yu Tsai wrote: > Hi, > > On Sat, Dec 20, 2014 at 6:20 PM, Hans de Goede wrote: >> Hi, >> >> >> On 19-12-14 19:17, Maxime Ripard wrote: >>> >>> Hi, >>> >>> On Thu, Dec 18, 2014 at 09:50:26AM +0100, Hans de Goede wrote: >>>> >>>> Hi, >>>> >>>> On 18-12-14 03:48, Chen-Yu Tsai wrote: >>>>> >>>>> Hi, >>>>> >>>>> On Thu, Dec 18, 2014 at 1:18 AM, Hans de Goede >>>>> wrote: >>>>>> >>>>>> On sun6i the cir block is attached to the reset controller, add support >>>>>> for de-asserting the reset if a reset controller is specified in dt. >>>>>> >>>>>> Signed-off-by: Hans de Goede >>>>>> Acked-by: Mauro Carvalho Chehab >>>>>> Acked-by: Maxime Ripard >>>>>> --- >>>>>> .../devicetree/bindings/media/sunxi-ir.txt | 2 ++ >>>>>> drivers/media/rc/sunxi-cir.c | 25 >>>>>> ++++++++++++++++++++-- >>>>>> 2 files changed, 25 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>> b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>> index 23dd5ad..6b70b9b 100644 >>>>>> --- a/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>> +++ b/Documentation/devicetree/bindings/media/sunxi-ir.txt >>>>>> @@ -10,6 +10,7 @@ Required properties: >>>>>> >>>>>> Optional properties: >>>>>> - linux,rc-map-name : Remote control map name. >>>>>> +- resets : phandle + reset specifier pair >>>>> >>>>> >>>>> Should it be optional? Or should we use a sun6i compatible with >>>>> a mandatory reset phandle? I mean, the driver/hardware is not >>>>> going to work with the reset missing on sun6i. >>>>> >>>>> Seems we are doing it one way for some of our drivers, and >>>>> the other (optional) way for more generic ones, like USB. >>>> >>>> >>>> I do not believe that we should add a new compatible just because >>>> the reset line of a block is hooked up differently. It is the >>>> exact same ip-block. Only now the reset is not controlled >>>> through the apb-gate, but controlled separately. >>> >>> >>> He has a point though. Your driver might very well probe nicely and >>> everything, but still wouldn't be functional at all because the reset >>> line wouldn't have been specified in the DT. >> >> >> Right, just like other drivers we've, see e.g.: >> >> Documentation/devicetree/bindings/mmc/sunxi-mmc.txt >> >> Which is dealing with this in the same way. >> >>> The easiest way to deal with that would be in the bindings doc to >>> update it with a compatible for the A31, and mentionning that the >>> reset property is mandatory there. >> >> >> No the easiest way to deal with this is to expect people writing >> the dts to know what they are doing, just like we do for a lot >> of the other blocks in sun6i. >> >> Maybe put a generic note somewhere that sun6i has a reset controller, >> and that for all the blocks with optional resets property it should >> be considered mandatory on sun6i ? >> >> I'm sorry but I'm not going to make this change for the ir bindings >> given that we've the same situation in a lot of other places. >> >> Consistency is important. Moreover I believe that having a sun6i >> specific compatible string is just wrong, since it is the exact >> same hardware block as on sun5i, just with its reset line routed >> differently, just like e.g. the mmc controller, the uarts or the gmac >> all of which also do not have a sun6i specific compatible to enforce >> reset controller usage. >> >> Regards, >> >> Hans >> >> >> >> >>> Note that the code itself might not change at all though. I'd just >>> like to avoid any potential breaking of the DT bindings themselves. If >>> we further want to refine the code, we can do that however we want. >>> >>> I have a slight preference for a clean error if reset is missing, but >>> I won't get in the way just for that. > > Seems this patch and the following patch were overlooked after the > discussion. Any chance we could get this in? I'm a linux/media sub-maintainer, so I've already send a pull-req for these 2 to the linux/media maintainer, iow this is taken care of :) Regards, Hans