Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752851AbeACO2I (ORCPT + 1 other); Wed, 3 Jan 2018 09:28:08 -0500 Received: from mail-it0-f68.google.com ([209.85.214.68]:32999 "EHLO mail-it0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750857AbeACO2G (ORCPT ); Wed, 3 Jan 2018 09:28:06 -0500 X-Google-Smtp-Source: ACJfBosA+hHrEsIRYEXuUJ/rcOankPmvV99UH7CkoqX8lm/GVE4M8Ffofgu4v7AdPT30dNZ+vEGYITL+LOUywFednGU= MIME-Version: 1.0 In-Reply-To: <698e7ae5-9f19-1282-7b82-0e2fd2080906@roeck-us.net> References: <20171228162939.3928-2-paul@crapouillou.net> <20171230135108.6834-1-paul@crapouillou.net> <20171230135108.6834-5-paul@crapouillou.net> <1514911698.3623.1@smtp.crapouillou.net> <698e7ae5-9f19-1282-7b82-0e2fd2080906@roeck-us.net> From: PrasannaKumar Muralidharan Date: Wed, 3 Jan 2018 19:58:04 +0530 Message-ID: Subject: Re: [PATCH v2 5/8] MIPS: jz4740: dts: Add bindings for the jz4740-wdt driver To: Guenter Roeck Cc: Paul Cercueil , Ralf Baechle , Rob Herring , Mark Rutland , Wim Van Sebroeck , devicetree@vger.kernel.org, linux-mips@linux-mips.org, open list , linux-watchdog@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: Hi Guenter, On 3 January 2018 at 10:16, Guenter Roeck wrote: > On 01/02/2018 08:48 AM, Paul Cercueil wrote: >> >> Hi PrasannaKumar, >> >> Le mar. 2 janv. 2018 à 17:37, PrasannaKumar Muralidharan >> a écrit : >>> >>> Hi Paul, >>> >>> On 30 December 2017 at 19:21, Paul Cercueil wrote: >>>> >>>> Also remove the watchdog platform_device from platform.c, since it >>>> wasn't used anywhere anyway. >>>> >>>> Signed-off-by: Paul Cercueil >>>> --- >>>> arch/mips/boot/dts/ingenic/jz4740.dtsi | 8 ++++++++ >>>> arch/mips/jz4740/platform.c | 16 ---------------- >>>> 2 files changed, 8 insertions(+), 16 deletions(-) >>>> >>>> v2: No change >>>> >>>> diff --git a/arch/mips/boot/dts/ingenic/jz4740.dtsi >>>> b/arch/mips/boot/dts/ingenic/jz4740.dtsi >>>> index cd5185bb90ae..26c6b561d6f7 100644 >>>> --- a/arch/mips/boot/dts/ingenic/jz4740.dtsi >>>> +++ b/arch/mips/boot/dts/ingenic/jz4740.dtsi >>>> @@ -45,6 +45,14 @@ >>>> #clock-cells = <1>; >>>> }; >>>> >>>> + watchdog: watchdog@10002000 { >>>> + compatible = "ingenic,jz4740-watchdog"; >>>> + reg = <0x10002000 0x10>; >>>> + >>>> + clocks = <&cgu JZ4740_CLK_RTC>; >>>> + clock-names = "rtc"; >>>> + }; >>>> + >>> >>> >>> The watchdog driver calls jz4740_timer_enable_watchdog and >>> jz4740_timer_disable_watchdog which defined in >>> arch/mips/jz4740/timer.c. It accesses registers iomapped by timer >>> code. Declaring register size as 0x10 does not show the real picture. >>> Better use register size as 0x100 and let timer, wdt, pwm drivers to >>> share them. >> >> >> As you said, it accesses registers iomapped by timer code. So the watchdog >> driver doesn't need to iomap them. >> >>> Code from one of your branches >>> >>> (https://github.com/OpenDingux/linux/blob/for-upstream-clocksource/arch/mips/boot/dts/ingenic/jz4740.dtsi) >>> does it. Can you prepare a patch series and send it? >>> I have a patch set that moves timer code out of arch/mips/jz4740/ and >>> does a similar thing for watchdog and pwm. As your new timer driver is >>> better than the existing one I have not sent my patches yet. I would >>> like to see it getting mainlined as it paves way for removing most of >>> code in arch/mips/jz4740. >> >> >> The whole 'for-upstream-clocksource' branch is supposed to go upstream, >> but I can't do it in one big patchset without having lots of breakages >> with >> my other patchsets (jz4770 SoC support, and jz4740 watchdog updates) >> currently under review. That also makes it simpler to upstream than having >> one single patchset that touches 6 different frameworks (MIPS, irq, >> clocks, >> clocksource, watchdog, PWM). >> >> So I will submit it in two steps, first the irq/clocks/clocksource drivers >> (this patchset) hopefully for 4.16, and then the platform/watchdog/PWM >> fixes >> for 4.17. >> > > I kind of lost it in this exchange, sorry. At this point I don't know if > something > is wrong with the watchdog patches, and I have no clue what the upstream > path There is nothing wrong in this watchdog patches. > is supposed to be. My working assumption is that 1) something may be wrong > with > the current version of the patches, and, 2), even if not, none of the > patches > is expected to find its way upstream through the watchdog subsystem. Plus, > 3), > even if some of the patches are supposed to go upstream through the watchdog > subsystem, that won't happen in 4.16, and the patches will be resubmitted > later > when they are ready [and will hopefully marked clearly for submission > through > the watchdog subsystem]. > > With that in mind, I'll mark the series for my reference as "not > applicable". > If this is wrong please let me know. Paul has patches related to timer code. While sending that he would send changes to watchdog dts entry also. I was suggesting him to send timer patches together with watchdog patches as a single patch series but he prefers to send them as separate patch series. I would like to reiterate that there is nothing wrong with this watchdog patches. I should have set the correct context in my previous email itself. I sincerely apologize for creating this confusion. Regards, PrasannaKumar