lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 7 Oct 2014 17:10:44 -0500
From:	Josh Cartwright <joshc@...eaurora.org>
To:	Stephen Boyd <sboyd@...eaurora.org>
Cc:	Kumar Gala <galak@...eaurora.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	linux-arm-msm@...r.kernel.org,
	Russell King <linux@....linux.org.uk>,
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] ARM: qcom: add description of KPSS WDT for IPQ8064

On Thu, Oct 02, 2014 at 12:08:38PM -0700, Stephen Boyd wrote:
[..]
> On 10/01/14 11:15, Josh Cartwright wrote:
> > The percpu-ness of the two WDTs makes configuration even more
> > interesting, as it's possible you'd want to independently configure
> > timeouts for CPU0_WDT0 and CPU1_WDT0, supporting this with a coalesced
> > timer/wdt would be cumbersome.
> 
> We already do similar things for the timers on each cpu. It doesn't seem
> that bad, but that's a matter of opinion.

I think the difference in this case is that each of the 2 per-cpu WDTs
can conceivably be registered independently, each with their own default
timeout configuration.  If we wanted to allow the most flexibility, it
seemed to me that making use of subnodes would be the best, but maybe
you have some other idea.

> > Something like this perhaps:
> >
> > 	timer@...a000 {
> > 		compatible = "qcom,kpss-timer", "qcom,msm-timer";
> > 		interrupts = <1 1 0x301>,
> > 			     <1 2 0x301>,
> > 			     <1 3 0x301>;
> > 		reg = <0x0200a000 0x100>;
> > 		clock-frequency = <25000000>,
> > 				  <32768>;
> > 		cpu-offset = <0x80000>;
> >
> > 		#address-cells = <1>;
> > 		#size-cells = <1>;
> > 		ranges;
> >
> > 		cpu0_wdt0: watchdog@...a038 {
> > 			compatible = "qcom,kpss-wdt";
> > 			reg = <0x208a038 0x40>;
> > 			interrupts = <1 4 0x301>,
> > 			clocks = <&sleep_clk>;
> > 			timeout-sec = <10>;
> > 			cpu = <&cpu0>;
> > 		};
> >
> > 		cpu0_wdt1: watchdog@...a060 {
> > 			compatible = "qcom,kpss-wdt";
> > 			reg = <0x208a060 0x40>;
> > 			interrupts = <1 5 0x301>,
> > 			clocks = <&sleep_clk>;
> > 			timeout-sec = <20>;
> > 			cpu = <&cpu0>;
> > 		};
> >
> > 		cpu1_wdt0: watchdog@...a038 {
> > 			compatible = "qcom,kpss-wdt";
> > 			reg = <0x209a038 0x40>;
> > 			interrupts = <1 4 0x301>,
> > 			clocks = <&sleep_clk>;
> > 			timeout-sec = <8>;
> > 			cpu = <&cpu1>;
> > 		};
> >
> > 		cpu1_wdt1: watchdog@...a060 {
> > 			compatible = "qcom,kpss-wdt";
> > 			reg = <0x209a060 0x40>;
> > 			interrupts = <1 5 0x301>,
> > 			clocks = <&sleep_clk>;
> > 			timeout-sec = <15>;
> > 			cpu = <&cpu1>;
> > 		};
> > 	};
> >
> >
> 
> I'm thinking:
> 
>                 timer@...a000 {
>                         compatible = "qcom,kpss-timer", "qcom,msm-timer";
>                         interrupts = <1 1 0x301>,
>                                      <1 2 0x301>,
>                                      <1 3 0x301>,
>                                      <1 4 0x301>,
>                                      <1 5 0x301>;
>                         reg = <0x0200a000 0x100>;
>                         clock-frequency = <27000000>,
>                                           <32768>;
>                         clocks = <&cxo>, <&sleep_clk>;
>                         clock-names = "ref", "sleep";
>                         cpu-offset = <0x80000>;
>                 };

Where'd the default timeout configuration go?  Or, should we have one
timeout-sec property and not allow setting the default timeouts per WDT
instance?  Or no configurable timeout at all?

> Can you explain the need for the cpu handle? Luckily this device only
> exists in configurations that have up to 4 CPUs and so mapping the
> logical CPU number to the watchdog for that CPU is "easy" in that we can
> convert the CPU from logical to physical and then do the math taking
> into account the cpu-offset to figure out where the non-aliased
> registers are. Once we get into pairs of watchdogs for different
> clusters this isn't so easy and it's better to have the phandle
> somewhere (either in the watchdog node or the cpu node) and then have
> multiple nodes for the watchdog block per-cpu so that we can map the CPU
> to the device. We realized this when making the saw binding.

Ah, yeah, now that I think about it, it's fairly straightforward to map
backward using the parent cpu-offset to determine the corresponding CPU,
so it wouldn't be necessary.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists