[{"id":1759113,"web_url":"http://patchwork.ozlabs.org/comment/1759113/","msgid":"<1503994514.5933.8.camel@aj.id.au>","list_archive_url":null,"date":"2017-08-29T08:15:14","subject":"Re: [PATCH linux dev-4.10 resend] dts: add aspeed-pwm-tacho to msn\n\tdts.","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"Hi Mykola,\n\nSorry for taking so long to review this, thanks for the prompt with the resend.\n\nUnfortunately the patch doesn't apply cleanly to dev-4.10 - can you please\nrebase it and send a v2?\n\nI have some additional queries below.\n\nOn Wed, 2017-08-23 at 10:33 +0300, Mykola Kostenok wrote:\n> Add basic pwm-tacho-controller node to ast-g5 dtsi.\n> \n> Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.\n> \n> > Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>\n> ---\n>  arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 49 +++++++++++++++++++++++++++\n>  arch/arm/boot/dts/aspeed-g5.dtsi              | 17 ++++++++++\n>  2 files changed, 66 insertions(+)\n> \n> diff --git a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\n> index 0774959..d790927 100644\n> --- a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\n> +++ b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\n> @@ -139,3 +139,52 @@\n> >  \tstatus = \"okay\";\n>  };\n>  \n> +&pwm_tacho {\n> > +\tstatus = \"okay\";\n> > +\tpinctrl-names = \"default\";\n> > +\tpinctrl-0 = <&pinctrl_pwm0_default>;\n\nJust to confirm: One PWM pin is driving 8 fans?\n\n> > +\t#address-cells = <1>;\n> > +\t#size-cells = <0>;\n\nWe shouldn't need to re-specify either of these as they're done in the dtsi.\nHowever I have a question on #size-cells in the dtsi (see below).\n\n> > +\t#cooling-cells = <2>;\n> +\n> > +\tfan@0 {\n> > +\t\treg = <0x00>;\n> > +\t\tcooling-levels = /bits/ 8 <125 151 177 203 229 255>;\n\nThe bindings documentation says cooling-levels is a required property for each\nsubnode, yet only fan@0 is defining it.\n\nIs this somehow related to only muxing PWM0 above?\n\nRegardless I think you should meet the expectation of the bindings, even if the\ninformation is redundant.\n\n> > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x00>;\n> > +\t};\n> +\n> > +\tfan@1 {\n> > +\t\treg = <0x00>;\n\nReg should match the unit address in the node name, so what you've done here\nexpectations of the dts. However, it doesn't appear to give a compiler warning.\n\nOverall I think it's worth a comment.\n\nIn fact, I'm going to go out on a limb and claim the bindings are completely\nbackwards. You may use the one PWM to drive multiple fans, but you're never\ngoing to have multiple fans feed one tacho input. As such I'd have reg\nrepresent the tacho value, and aspeed,pwm-ch map the associated PWM channel.\nThis way we'd have:\n\n\t...\n\tfan@1 {\n\t\treg = <1>;\n\t\taspeed,pwm-ch = /bits/ 8 <0>;\n\t};\n\n\tfan@2 {\n\t\treg = <2>;\n\t\taspeed,pwm-ch = /bits/ 8 <0>;\n\t};\n\t...\n\nLooking at the datasheet we find that the hardware defines \"Tach Source\nRegisters\". Putting aside that the behaviour of the registers is pretty much\ninsane, this aligns exactly with the alternative bindings I suggested above:\nPWM inputs are configured for tach channels (tach channels dominate).\nEffectively my reg value would be an index into the fields of those registers.\n\nIt's annoying that the ship has probably sailed. We should start a conversation\nwith upstream, as your devicetree exposes the failure of the bindings.\n\n> > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x01>;\n> > +\t};\n> +\n> > +\tfan@2 {\n> > +\t\treg = <0x00>;\n> > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x02>;\n> > +\t};\n> +\n> > +\tfan@3 {\n> > +\t\treg = <0x00>;\n> > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x03>;\n> > +\t};\n> +\n> > +\tfan@4 {\n> > +\t\treg = <0x00>;\n> > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x04>;\n> > +\t};\n> +\n> > +\tfan@5 {\n> > +\t\treg = <0x00>;\n> > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x05>;\n> > +\t};\n> +\n> > +\tfan@6 {\n> > +\t\treg = <0x00>;\n> > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x06>;\n> > +\t};\n> +\n> > +\tfan@7 {\n> > +\t\treg = <0x00>;\n> > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x07>;\n> > +\t};\n> +};\n> diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi\n> index 72b6638..4c012af 100644\n> --- a/arch/arm/boot/dts/aspeed-g5.dtsi\n> +++ b/arch/arm/boot/dts/aspeed-g5.dtsi\n> @@ -40,6 +40,14 @@\n> >  \t\tserial4 = &uart5;\n> >  \t};\n>  \n> > +\tclocks {\n> > +\t\tpwm_tacho_fixed_clk: fixedclk {\n> > +\t\t\tcompatible = \"fixed-clock\";\n> > +\t\t\t#clock-cells = <0>;\n> > +\t\t\tclock-frequency = <24000000>;\n> > +\t\t};\n> > +\t};\n\nHappy to take this as it is what was done for the ast2400. Joel is getting the\nclock driver sorted out so hopefully we can remove it in the near future.\n\n> +\n> >  \tahb {\n> >  \t\tcompatible = \"simple-bus\";\n> >  \t\t#address-cells = <1>;\n> @@ -372,6 +380,15 @@\n> >  \t\t\t\t#size-cells = <1>;\n> >  \t\t\t\tranges = <0 0x1e78a000 0x1000>;\n> >  \t\t\t};\n> +\n> > > +\t\t\tpwm_tacho: pwm-tacho-controller@1e786000 {\n> > +\t\t\t\tcompatible = \"aspeed,ast2500-pwm-tacho\";\n> > +\t\t\t\t#address-cells = <1>;\n> > +\t\t\t\t#size-cells = <0>;\n\nThe bindings documentation claims #size-cells should be 1, however both the\nqanta dts and your patch set this to zero. The child nodes corroborate the\nvalue of 0 - we're specifying the PWM channel which has no need for a length.\nWe should update the bindings, though I'm not sure how happy upstream will be\nabout that. If I recall correctly Rob already quizzed you about why the\nbindings were being changed not long after they were applied. Still, better to\nbe right. Maybe we can rev the bindings and fix the PWM/tach mapping above?\n\nCheers for an interesting patch.\n\nAndrew\n\n> > +\t\t\t\treg = <0x1e786000 0x1000>;\n> > +\t\t\t\tclocks = <&pwm_tacho_fixed_clk>;\n> > +\t\t\t\tstatus = \"disabled\";\n> > +\t\t\t};\n> >  \t\t};\n> >  \t};\n>  };","headers":{"Return-Path":"<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>","X-Original-To":["incoming@patchwork.ozlabs.org","openbmc@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","openbmc@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xhLzw704Vz9t2x\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 18:15:40 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xhLzw5fbYzDqZq\n\tfor <incoming@patchwork.ozlabs.org>;\n\tTue, 29 Aug 2017 18:15:40 +1000 (AEST)","from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com\n\t[66.111.4.26])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xhLzd2JtbzDqYV\n\tfor <openbmc@lists.ozlabs.org>; Tue, 29 Aug 2017 18:15:25 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id 078F22139A;\n\tTue, 29 Aug 2017 04:15:23 -0400 (EDT)","from frontend1 ([10.202.2.160])\n\tby compute4.internal (MEProxy); Tue, 29 Aug 2017 04:15:23 -0400","from keelia (ppp118-210-176-216.bras2.adl6.internode.on.net\n\t[118.210.176.216])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id A40707F9D2;\n\tTue, 29 Aug 2017 04:15:20 -0400 (EDT)"],"Authentication-Results":["ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"qtPiwxQe\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"mkhZO+Or\"; \n\tdkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"qtPiwxQe\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"mkhZO+Or\"; \n\tdkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"qtPiwxQe\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"mkhZO+Or\"; dkim-atps=neutral"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=aj.id.au; h=cc\n\t:content-type:date:from:in-reply-to:message-id:mime-version\n\t:references:subject:to:x-me-sender:x-me-sender:x-sasl-enc\n\t:x-sasl-enc; s=fm1; bh=Q2d9SoeDRyHAX9gVEh2Gkc/+VuX/9Xo2hfgWTNfE9\n\tAg=; b=qtPiwxQeOkEUedwsw+nPhO/IwHQ84pqbZqFBgsDRRHHhjeDb7jzw4Lz6F\n\tRTnyE/Of2Cxd4N4Zfsl2hTLnFfsk4pB0Dxets6rhTBASSXQq9ZwUyJK/sVsnnC9L\n\tIGq7Gr+p3TvfY3F0bmBHRWUdqoButRf3UpuSgbavEXFd8+cfxGA3kYy8VeqnYSZp\n\tZ38Px4y7Mnz4TjF2DOwzi+EbaAjLv4F2/PXKKVtwbAtxBn3OyZmR/3zAxTg92eMC\n\tV0j1FgvRHCnRkFOyCH6ymme4cWgZOQVtSdLLUATUklaX7F0LwgI3i5RpdNoOl1ZL\n\tuT+6kODqWo+8wADSP9DvaR1BLSndQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to:x-me-sender\n\t:x-me-sender:x-sasl-enc:x-sasl-enc; s=fm1; bh=Q2d9SoeDRyHAX9gVEh\n\t2Gkc/+VuX/9Xo2hfgWTNfE9Ag=; b=mkhZO+OroIBp4GSNx/SWX0xy9VP9I3YKf6\n\tx7ce8Q58zl60zAIpoa8IWa6qVHOw1IrI7q+7wljz8/fLGu+ptAmUH65ER0BdaHO7\n\t32l5uttXxwj5c23shbINf/yod15oHzojldk1V8tvMtSJR0CmqHeNoguNkpodb8l1\n\tzL65FBN+qeyRQ/66ri8C2d5wcLTIMpK6/cu2uZTEvIEQcusjsu766mRoCdJ7bbH0\n\tidoEBsEq2Et3iwQK0RBPrsl3JfPdUfE8HFrYZfUww8ut+8Q6vHMNrqTXd5VR/gYC\n\tB0H0B4uQ6T/RNNDuaeDBp83K4vSh8Y8LETf6/K5tXFq0cDizohbg=="],"X-ME-Sender":"<xms:miKlWbSp14N_BCMgjXZvz8KZxgSaxT-EtgIez1P_DRFGDCp1e_g-ew>","X-Sasl-enc":"ZVzUHXk9MFa23yFCnKqywy4n9+B7y+3DHGa7RQ5nI459 1503994522","Message-ID":"<1503994514.5933.8.camel@aj.id.au>","Subject":"Re: [PATCH linux dev-4.10 resend] dts: add aspeed-pwm-tacho to msn\n\tdts.","From":"Andrew Jeffery <andrew@aj.id.au>","To":"Mykola Kostenok <c_mykolak@mellanox.com>, Joel Stanley <joel@jms.id.au>, \n\topenbmc@lists.ozlabs.org","Date":"Tue, 29 Aug 2017 17:45:14 +0930","In-Reply-To":"<20170823073313.1294-1-c_mykolak@mellanox.com>","References":"<20170823073313.1294-1-c_mykolak@mellanox.com>","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-K8CoYt56yyOw6t8y5ME9\"","X-Mailer":"Evolution 3.22.6-1ubuntu1 ","Mime-Version":"1.0","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC <openbmc.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/openbmc/>","List-Post":"<mailto:openbmc@lists.ozlabs.org>","List-Help":"<mailto:openbmc-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=subscribe>","Cc":"Ohad Oz <ohado@mellanox.com>, Vadim Pasternak <vadimp@mellanox.com>","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>"}},{"id":1759430,"web_url":"http://patchwork.ozlabs.org/comment/1759430/","msgid":"<DB5PR05MB122434BAC10BF91338D87500EF9F0@DB5PR05MB1224.eurprd05.prod.outlook.com>","list_archive_url":null,"date":"2017-08-29T15:05:47","subject":"RE: [PATCH linux dev-4.10 resend] dts: add aspeed-pwm-tacho to msn\n\tdts.","submitter":{"id":71635,"url":"http://patchwork.ozlabs.org/api/people/71635/","name":"Mykola Kostenok","email":"c_mykolak@mellanox.com"},"content":"> -----Original Message-----\r\n> From: Andrew Jeffery [mailto:andrew@aj.id.au]\r\n> Sent: Tuesday, August 29, 2017 11:15 AM\r\n> To: Mykola Kostenok <c_mykolak@mellanox.com>; Joel Stanley\r\n> <joel@jms.id.au>; openbmc@lists.ozlabs.org\r\n> Cc: Ohad Oz <ohado@mellanox.com>; Vadim Pasternak\r\n> <vadimp@mellanox.com>\r\n> Subject: Re: [PATCH linux dev-4.10 resend] dts: add aspeed-pwm-tacho to\r\n> msn dts.\r\n> \r\n> Hi Mykola,\r\n> \r\n> Sorry for taking so long to review this, thanks for the prompt with the\r\n> resend.\r\n> \r\n> Unfortunately the patch doesn't apply cleanly to dev-4.10 - can you please\r\n> rebase it and send a v2?\r\n\r\nHi, Andrew.\r\nYes, sure.\r\n\r\n> \r\n> I have some additional queries below.\r\n> \r\n> On Wed, 2017-08-23 at 10:33 +0300, Mykola Kostenok wrote:\r\n> > Add basic pwm-tacho-controller node to ast-g5 dtsi.\r\n> >\r\n> > Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.\r\n> >\r\n> > > Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>\r\n> > ---\r\n> >  arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 49\r\n> > +++++++++++++++++++++++++++\r\n> >  arch/arm/boot/dts/aspeed-g5.dtsi              | 17 ++++++++++\r\n> >  2 files changed, 66 insertions(+)\r\n> >\r\n> > diff --git a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\r\n> > b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\r\n> > index 0774959..d790927 100644\r\n> > --- a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\r\n> > +++ b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\r\n> > @@ -139,3 +139,52 @@\r\n> > >  \tstatus = \"okay\";\r\n> >  };\r\n> >\r\n> > +&pwm_tacho {\r\n> > > +\tstatus = \"okay\";\r\n> > > +\tpinctrl-names = \"default\";\r\n> > > +\tpinctrl-0 = <&pinctrl_pwm0_default>;\r\n> \r\n> Just to confirm: One PWM pin is driving 8 fans?\r\n> \r\n\r\nYes, in Mellanox msn system open PWM driving 8 fans.\r\n\r\n> > > +\t#address-cells = <1>;\r\n> > > +\t#size-cells = <0>;\r\n> \r\n> We shouldn't need to re-specify either of these as they're done in the dtsi.\r\n> However I have a question on #size-cells in the dtsi (see below).\r\n> \r\n\r\nAcked.\r\n\r\n> > > +\t#cooling-cells = <2>;\r\n> > +\r\n> > > +\tfan@0 {\r\n> > > +\t\treg = <0x00>;\r\n> > > +\t\tcooling-levels = /bits/ 8 <125 151 177 203 229 255>;\r\n> \r\n> The bindings documentation says cooling-levels is a required property for\r\n> each subnode, yet only fan@0 is defining it.\r\n> \r\n> Is this somehow related to only muxing PWM0 above?\r\n> \r\n> Regardless I think you should meet the expectation of the bindings, even if\r\n> the information is redundant.\r\n\r\nOh, it's a mistake in docs. cooling-levels is not required - it's optional. \r\nWe tried to make separated node for pwm cooling-levels, but it was not acked by Rob Herring.\r\nNow it can be set only once for each PWM. As we use only PWM0 we set it once.\r\nAnd nothing bad happens with another systems without cooling-levels.\r\n\r\n> \r\n> > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x00>;\r\n> > > +\t};\r\n> > +\r\n> > > +\tfan@1 {\r\n> > > +\t\treg = <0x00>;\r\n> \r\n> Reg should match the unit address in the node name, so what you've done\r\n> here expectations of the dts. However, it doesn't appear to give a compiler\r\n> warning.\r\n> \r\n> Overall I think it's worth a comment.\r\n> \r\n> In fact, I'm going to go out on a limb and claim the bindings are completely\r\n> backwards. You may use the one PWM to drive multiple fans, but you're\r\n> never going to have multiple fans feed one tacho input. As such I'd have reg\r\n> represent the tacho value, and aspeed,pwm-ch map the associated PWM\r\n> channel.\r\n> This way we'd have:\r\n> \r\n> \t...\r\n> \tfan@1 {\r\n> \t\treg = <1>;\r\n> \t\taspeed,pwm-ch = /bits/ 8 <0>;\r\n> \t};\r\n> \r\n> \tfan@2 {\r\n> \t\treg = <2>;\r\n> \t\taspeed,pwm-ch = /bits/ 8 <0>;\r\n> \t};\r\n> \t...\r\n> \r\n> Looking at the datasheet we find that the hardware defines \"Tach Source\r\n> Registers\". Putting aside that the behaviour of the registers is pretty much\r\n> insane, this aligns exactly with the alternative bindings I suggested above:\r\n> PWM inputs are configured for tach channels (tach channels dominate).\r\n> Effectively my reg value would be an index into the fields of those registers.\r\n> \r\n> It's annoying that the ship has probably sailed. We should start a\r\n> conversation with upstream, as your devicetree exposes the failure of the\r\n> bindings.\r\n> \r\n\r\nWe just take it as it was. \r\n\r\n> > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x01>;\r\n> > > +\t};\r\n> > +\r\n> > > +\tfan@2 {\r\n> > > +\t\treg = <0x00>;\r\n> > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x02>;\r\n> > > +\t};\r\n> > +\r\n> > > +\tfan@3 {\r\n> > > +\t\treg = <0x00>;\r\n> > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x03>;\r\n> > > +\t};\r\n> > +\r\n> > > +\tfan@4 {\r\n> > > +\t\treg = <0x00>;\r\n> > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x04>;\r\n> > > +\t};\r\n> > +\r\n> > > +\tfan@5 {\r\n> > > +\t\treg = <0x00>;\r\n> > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x05>;\r\n> > > +\t};\r\n> > +\r\n> > > +\tfan@6 {\r\n> > > +\t\treg = <0x00>;\r\n> > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x06>;\r\n> > > +\t};\r\n> > +\r\n> > > +\tfan@7 {\r\n> > > +\t\treg = <0x00>;\r\n> > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x07>;\r\n> > > +\t};\r\n> > +};\r\n> > diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi\r\n> > b/arch/arm/boot/dts/aspeed-g5.dtsi\r\n> > index 72b6638..4c012af 100644\r\n> > --- a/arch/arm/boot/dts/aspeed-g5.dtsi\r\n> > +++ b/arch/arm/boot/dts/aspeed-g5.dtsi\r\n> > @@ -40,6 +40,14 @@\r\n> > >  \t\tserial4 = &uart5;\r\n> > >  \t};\r\n> >\r\n> > > +\tclocks {\r\n> > > +\t\tpwm_tacho_fixed_clk: fixedclk {\r\n> > > +\t\t\tcompatible = \"fixed-clock\";\r\n> > > +\t\t\t#clock-cells = <0>;\r\n> > > +\t\t\tclock-frequency = <24000000>;\r\n> > > +\t\t};\r\n> > > +\t};\r\n> \r\n> Happy to take this as it is what was done for the ast2400. Joel is getting the\r\n> clock driver sorted out so hopefully we can remove it in the near future.\r\n> \r\n\r\nOk.\r\n\r\n> > +\r\n> > >  \tahb {\r\n> > >  \t\tcompatible = \"simple-bus\";\r\n> > >  \t\t#address-cells = <1>;\r\n> > @@ -372,6 +380,15 @@\r\n> > >  \t\t\t\t#size-cells = <1>;\r\n> > >  \t\t\t\tranges = <0 0x1e78a000 0x1000>;\r\n> > >  \t\t\t};\r\n> > +\r\n> > > > +\t\t\tpwm_tacho: pwm-tacho-controller@1e786000 {\r\n> > > +\t\t\t\tcompatible = \"aspeed,ast2500-pwm-tacho\";\r\n> > > +\t\t\t\t#address-cells = <1>;\r\n> > > +\t\t\t\t#size-cells = <0>;\r\n> \r\n> The bindings documentation claims #size-cells should be 1, however both the\r\n> qanta dts and your patch set this to zero. The child nodes corroborate the\r\n> value of 0 - we're specifying the PWM channel which has no need for a\r\n> length.\r\n\r\nI think it’s a mistake in documentation.\r\n\r\n> We should update the bindings, though I'm not sure how happy upstream\r\n> will be about that. If I recall correctly Rob already quizzed you about why the\r\n> bindings were being changed not long after they were applied. Still, better to\r\n> be right. Maybe we can rev the bindings and fix the PWM/tach mapping\r\n> above?\r\n>\r\n> Cheers for an interesting patch.\r\n> \r\n> Andrew\r\n> \r\n\r\nIt's ok with us. Maybe it will be next step later.\r\n\r\nBest regards, Mykola Kostenok.\r\n\r\n\r\n> > > +\t\t\t\treg = <0x1e786000 0x1000>;\r\n> > > +\t\t\t\tclocks = <&pwm_tacho_fixed_clk>;\r\n> > > +\t\t\t\tstatus = \"disabled\";\r\n> > > +\t\t\t};\r\n> > >  \t\t};\r\n> > >  \t};\r\n> >  };","headers":{"Return-Path":"<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>","X-Original-To":["incoming@patchwork.ozlabs.org","openbmc@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","openbmc@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xhX5V564Fz9sRV\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 30 Aug 2017 01:06:06 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xhX5V3whZzDqlp\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 30 Aug 2017 01:06:06 +1000 (AEST)","from EUR01-HE1-obe.outbound.protection.outlook.com\n\t(mail-he1eur01on0068.outbound.protection.outlook.com [104.47.0.68])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xhX5H4zk0zDqkb\n\tfor <openbmc@lists.ozlabs.org>; Wed, 30 Aug 2017 01:05:55 +1000 (AEST)","from DB5PR05MB1224.eurprd05.prod.outlook.com (10.161.245.11) by\n\tDB5PR05MB1910.eurprd05.prod.outlook.com (10.166.173.143) with\n\tMicrosoft SMTP Server (version=TLS1_2,\n\tcipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id\n\t15.1.1385.9; Tue, 29 Aug 2017 15:05:47 +0000","from DB5PR05MB1224.eurprd05.prod.outlook.com\n\t([fe80::e96a:76a1:8f05:384e]) by\n\tDB5PR05MB1224.eurprd05.prod.outlook.com\n\t([fe80::e96a:76a1:8f05:384e%18]) with mapi id 15.01.1385.014;\n\tTue, 29 Aug 2017 15:05:47 +0000"],"Authentication-Results":["ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"RnYrR6OF\"; dkim-atps=neutral","lists.ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"RnYrR6OF\"; dkim-atps=neutral","lists.ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"RnYrR6OF\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=c_mykolak@mellanox.com; "],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;\n\ts=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version;\n\tbh=tS5QoWRdjX+uavOww7ZMEYiBBqJS0cn6J4j1nNCImRc=;\n\tb=RnYrR6OFtPHP3aC+GSI0w4BkAcd2XFUcV9mODP0zKvaZVRGA8C8I/+cKfOrV7jLmqa8JHBTg2UnuVh/ENWUMQGUTVr55/h1GXyEj8Q4gNHqY39TV6dtMX/5YkQ7Ns2uF1S+RtRvCeG5gcB3ExYto9RylNkon/elW2UMsnWwvUSg=","From":"Mykola Kostenok <c_mykolak@mellanox.com>","To":"Andrew Jeffery <andrew@aj.id.au>, Joel Stanley <joel@jms.id.au>,\n\t\"openbmc@lists.ozlabs.org\" <openbmc@lists.ozlabs.org>","Subject":"RE: [PATCH linux dev-4.10 resend] dts: add aspeed-pwm-tacho to msn\n\tdts.","Thread-Topic":"[PATCH linux dev-4.10 resend] dts: add aspeed-pwm-tacho to msn\n\tdts.","Thread-Index":"AQHTG+IrhevFDggN5k+iy8kiqnKYj6KbBh0AgABgf+A=","Date":"Tue, 29 Aug 2017 15:05:47 +0000","Message-ID":"<DB5PR05MB122434BAC10BF91338D87500EF9F0@DB5PR05MB1224.eurprd05.prod.outlook.com>","References":"<20170823073313.1294-1-c_mykolak@mellanox.com>\n\t<1503994514.5933.8.camel@aj.id.au>","In-Reply-To":"<1503994514.5933.8.camel@aj.id.au>","Accept-Language":"ru-RU, en-US","Content-Language":"en-US","X-MS-Has-Attach":"","X-MS-TNEF-Correlator":"","authentication-results":["ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"RnYrR6OF\"; dkim-atps=neutral","lists.ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"RnYrR6OF\"; dkim-atps=neutral","lists.ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=Mellanox.com header.i=@Mellanox.com\n\theader.b=\"RnYrR6OF\"; dkim-atps=neutral","spf=none (sender IP is )\n\tsmtp.mailfrom=c_mykolak@mellanox.com; "],"x-originating-ip":"[80.90.224.13]","x-ms-publictraffictype":"Email","x-microsoft-exchange-diagnostics":"1; DB5PR05MB1910;\n\t6:ILc8B4X/6RhyRokKQQIG3nkdWIZHXUYfI3E8FEeUCoxWvAhKHZgIltov0p75dgKpttwWuE6LHt+2wH5P3oXYa49UbqU1UXTc6ZClE9AFue++FFNfABjyP+WJDtW6gNKijCTyU32MFHbrxBiks+1p+HGI6CG1NgR3yWZlvwvh6l5tBpD4ec5oToKfVaMYL7azGhjQHIw/KdHpu53iTc+VUqSvDeHtQvpp0nD811CzXQUoSUi2rbc0+EW1wN2ylio0EpaKBalXBKAq4m7bR8OS3kTHxVcf2GAMGbB6Xz3oWqxQVFOozTpFImMw926LKWznBmGGYWWgvE50tDCD6rBaaA==;\n\t5:vEnQiVihy9H5wbpcnJxySdfyHQU01/FfrbHNDFGYKWnFUR+roUe3RSuLrMjgh9DFLgScy5s/stAiGOWyy/RHJdokzgWrlVv4DefjTxrI5TVXL7aG8jxqiOSZb636QbTRCw6dCsCTXfG5ZMao6NKq6Q==;\n\t24:7TKd438SP33JHDUBlQFCKF3BnsxR9z1kmI7SlFYNyn2jF5M+w170Ny+6ZFGuC47l0dpwNpeEV5Nu7EQhSmVC2GE6Ho9oCV0yGZKjBKmXrUM=;\n\t7:h3yFHkM2jhJE2yFIeC+jI2AD6aRv7IdoMqOsi+iYRwk5CiroO807Xx0WZE+5bypWsSsV+vtvn8sTaLen3ZaWdq4w7t4d584FyL/Robn8VfLTJHLvpYxl/tPDRHKj1nW/gvq2RhVMaEU+VWDAQxGitGIJQq2z3ZWT2UFjo7Ti3GH+fOeq+OTZ7afXmlGtRMSAPkfTcsMIe8+JCU3sIAEzvHhvPu0t+Lqd90uCRc5/vJg=","x-ms-exchange-antispam-srfa-diagnostics":"SSOS;","x-ms-office365-filtering-correlation-id":"38e48466-a6cb-42d5-1ebd-08d4eeef6df9","x-ms-office365-filtering-ht":"Tenant","x-microsoft-antispam":"UriScan:; BCL:0; PCL:0;\n\tRULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(300000503095)(300135400095)(48565401081)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095);\n\tSRVR:DB5PR05MB1910; ","x-ms-traffictypediagnostic":"DB5PR05MB1910:","x-exchange-antispam-report-test":"UriScan:(65623756079841);","x-microsoft-antispam-prvs":"<DB5PR05MB191008DBC901F08282648DC7EF9F0@DB5PR05MB1910.eurprd05.prod.outlook.com>","x-exchange-antispam-report-cfa-test":"BCL:0; PCL:0;\n\tRULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(8121501046)(5005006)(2017060902086)(3002001)(93006095)(93001095)(100000703101)(100105400095)(10201501046)(6055026)(6041248)(20161123558100)(20161123560025)(20161123555025)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095);\n\tSRVR:DB5PR05MB1910; BCL:0; PCL:0;\n\tRULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);\n\tSRVR:DB5PR05MB1910; ","x-forefront-prvs":"0414DF926F","x-forefront-antispam-report":"SFV:NSPM;\n\tSFS:(10009020)(6009001)(39860400002)(47560400004)(13464003)(199003)(24454002)(51914003)(377454003)(189002)(377424004)(66066001)(55016002)(74316002)(81166006)(6246003)(8936002)(99286003)(6436002)(81156014)(4326008)(5250100002)(5660300001)(7696004)(478600001)(229853002)(54356999)(76176999)(25786009)(102836003)(6116002)(3846002)(97736004)(53546010)(50986999)(2501003)(8676002)(230783001)(9686003)(54906002)(3660700001)(189998001)(33656002)(14454004)(3280700002)(2906002)(68736007)(45080400002)(106356001)(2950100002)(101416001)(86362001)(6506006)(53936002)(105586002)(107886003)(2900100001)(305945005)(52230400001)(7736002);\n\tDIR:OUT; SFP:1101; SCL:1; SRVR:DB5PR05MB1910;\n\tH:DB5PR05MB1224.eurprd05.prod.outlook.com; FPR:; SPF:None;\n\tPTR:InfoNoRecords; A:1; MX:1; LANG:en; ","received-spf":"None (protection.outlook.com: mellanox.com does not designate\n\tpermitted sender hosts)","spamdiagnosticoutput":"1:99","spamdiagnosticmetadata":"NSPM","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","MIME-Version":"1.0","X-OriginatorOrg":"Mellanox.com","X-MS-Exchange-CrossTenant-originalarrivaltime":"29 Aug 2017 15:05:47.3688\n\t(UTC)","X-MS-Exchange-CrossTenant-fromentityheader":"Hosted","X-MS-Exchange-CrossTenant-id":"a652971c-7d2e-4d9b-a6a4-d149256f461b","X-MS-Exchange-Transport-CrossTenantHeadersStamped":"DB5PR05MB1910","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC <openbmc.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/openbmc/>","List-Post":"<mailto:openbmc@lists.ozlabs.org>","List-Help":"<mailto:openbmc-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=subscribe>","Cc":"Ohad Oz <ohado@mellanox.com>, Vadim Pasternak <vadimp@mellanox.com>","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>"}},{"id":1759866,"web_url":"http://patchwork.ozlabs.org/comment/1759866/","msgid":"<1504068592.5933.21.camel@aj.id.au>","list_archive_url":null,"date":"2017-08-30T04:49:52","subject":"Re: [PATCH linux dev-4.10 resend] dts: add aspeed-pwm-tacho to msn\n\tdts.","submitter":{"id":68332,"url":"http://patchwork.ozlabs.org/api/people/68332/","name":"Andrew Jeffery","email":"andrew@aj.id.au"},"content":"Hi Mykola,\n\nOn Tue, 2017-08-29 at 15:05 +0000, Mykola Kostenok wrote:\n> > -----Original Message-----\n> > > > From: Andrew Jeffery [mailto:andrew@aj.id.au]\n> > Sent: Tuesday, August 29, 2017 11:15 AM\n> > > > To: Mykola Kostenok <c_mykolak@mellanox.com>; Joel Stanley\n> > > > <joel@jms.id.au>; openbmc@lists.ozlabs.org\n> > > > Cc: Ohad Oz <ohado@mellanox.com>; Vadim Pasternak\n> > > > <vadimp@mellanox.com>\n> > Subject: Re: [PATCH linux dev-4.10 resend] dts: add aspeed-pwm-tacho to\n> > msn dts.\n> > \n> > Hi Mykola,\n> > \n> > Sorry for taking so long to review this, thanks for the prompt with the\n> > resend.\n> > \n> > Unfortunately the patch doesn't apply cleanly to dev-4.10 - can you please\n> > rebase it and send a v2?\n> \n> Hi, Andrew.\n> Yes, sure.\n> \n> > \n> > I have some additional queries below.\n> > \n> > On Wed, 2017-08-23 at 10:33 +0300, Mykola Kostenok wrote:\n> > > Add basic pwm-tacho-controller node to ast-g5 dtsi.\n> > > \n> > > Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.\n> > > \n> > > > Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>\n> > > \n> > > ---\n> > >  arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 49\n> > > +++++++++++++++++++++++++++\n> > >  arch/arm/boot/dts/aspeed-g5.dtsi              | 17 ++++++++++\n> > >  2 files changed, 66 insertions(+)\n> > > \n> > > diff --git a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\n> > > b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\n> > > index 0774959..d790927 100644\n> > > --- a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\n> > > +++ b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts\n> > > @@ -139,3 +139,52 @@\n> > > >  \tstatus = \"okay\";\n> > > \n> > >  };\n> > > \n> > > +&pwm_tacho {\n> > > > > > > > +\tstatus = \"okay\";\n> > > > > > > > +\tpinctrl-names = \"default\";\n> > > > +\tpinctrl-0 = <&pinctrl_pwm0_default>;\n> > \n> > Just to confirm: One PWM pin is driving 8 fans?\n> > \n> \n> Yes, in Mellanox msn system open PWM driving 8 fans.\n> \n> > > > > > > > +\t#address-cells = <1>;\n> > > > +\t#size-cells = <0>;\n> > \n> > We shouldn't need to re-specify either of these as they're done in the dtsi.\n> > However I have a question on #size-cells in the dtsi (see below).\n> > \n> \n> Acked.\n> \n> > > > +\t#cooling-cells = <2>;\n> > > \n> > > +\n> > > > > > > > +\tfan@0 {\n> > > > > > > > +\t\treg = <0x00>;\n> > > > +\t\tcooling-levels = /bits/ 8 <125 151 177 203 229 255>;\n> > \n> > The bindings documentation says cooling-levels is a required property for\n> > > > each subnode, yet only fan@0 is defining it.\n> > \n> > Is this somehow related to only muxing PWM0 above?\n> > \n> > Regardless I think you should meet the expectation of the bindings, even if\n> > the information is redundant.\n> \n> Oh, it's a mistake in docs. cooling-levels is not required - it's optional. \n> We tried to make separated node for pwm cooling-levels, but it was not acked by Rob Herring.\n> Now it can be set only once for each PWM. As we use only PWM0 we set it once.\n> And nothing bad happens with another systems without cooling-levels.\n> \n> > \n> > > > > > > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x00>;\n> > > > +\t};\n> > > \n> > > +\n> > > > > > > > +\tfan@1 {\n> > > > +\t\treg = <0x00>;\n> > \n> > Reg should match the unit address in the node name, so what you've done\n> > here expectations of the dts. However, it doesn't appear to give a compiler\n> > warning.\n> > \n> > Overall I think it's worth a comment.\n> > \n> > In fact, I'm going to go out on a limb and claim the bindings are completely\n> > backwards. You may use the one PWM to drive multiple fans, but you're\n> > never going to have multiple fans feed one tacho input. As such I'd have reg\n> > represent the tacho value, and aspeed,pwm-ch map the associated PWM\n> > channel.\n> > This way we'd have:\n> > \n> > \t...\n> > > > \tfan@1 {\n> > > > \t\treg = <1>;\n> > > > \t\taspeed,pwm-ch = /bits/ 8 <0>;\n> > \t};\n> > \n> > > > \tfan@2 {\n> > > > \t\treg = <2>;\n> > > > \t\taspeed,pwm-ch = /bits/ 8 <0>;\n> > \t};\n> > \t...\n> > \n> > Looking at the datasheet we find that the hardware defines \"Tach Source\n> > Registers\". Putting aside that the behaviour of the registers is pretty much\n> > insane, this aligns exactly with the alternative bindings I suggested above:\n> > PWM inputs are configured for tach channels (tach channels dominate).\n> > Effectively my reg value would be an index into the fields of those registers.\n> > \n> > It's annoying that the ship has probably sailed. We should start a\n> > conversation with upstream, as your devicetree exposes the failure of the\n> > bindings.\n> > \n> \n> We just take it as it was. \n\nI understand. I'm discussing fans with upstream anyway, so I'll bring this\nissue up.\n\n> \n> > > > > > > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x01>;\n> > > > +\t};\n> > > \n> > > +\n> > > > > > > > +\tfan@2 {\n> > > > > > > > +\t\treg = <0x00>;\n> > > > > > > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x02>;\n> > > > +\t};\n> > > \n> > > +\n> > > > > > > > +\tfan@3 {\n> > > > > > > > +\t\treg = <0x00>;\n> > > > > > > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x03>;\n> > > > +\t};\n> > > \n> > > +\n> > > > > > > > +\tfan@4 {\n> > > > > > > > +\t\treg = <0x00>;\n> > > > > > > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x04>;\n> > > > +\t};\n> > > \n> > > +\n> > > > > > > > +\tfan@5 {\n> > > > > > > > +\t\treg = <0x00>;\n> > > > > > > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x05>;\n> > > > +\t};\n> > > \n> > > +\n> > > > > > > > +\tfan@6 {\n> > > > > > > > +\t\treg = <0x00>;\n> > > > > > > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x06>;\n> > > > +\t};\n> > > \n> > > +\n> > > > > > > > +\tfan@7 {\n> > > > > > > > +\t\treg = <0x00>;\n> > > > > > > > +\t\taspeed,fan-tach-ch = /bits/ 8 <0x07>;\n> > > > +\t};\n> > > \n> > > +};\n> > > diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi\n> > > b/arch/arm/boot/dts/aspeed-g5.dtsi\n> > > index 72b6638..4c012af 100644\n> > > --- a/arch/arm/boot/dts/aspeed-g5.dtsi\n> > > +++ b/arch/arm/boot/dts/aspeed-g5.dtsi\n> > > @@ -40,6 +40,14 @@\n> > > > > > > >  \t\tserial4 = &uart5;\n> > > > > > > >  \t};\n> > > > > > > > +\tclocks {\n> > > > > > > > +\t\tpwm_tacho_fixed_clk: fixedclk {\n> > > > > > > > +\t\t\tcompatible = \"fixed-clock\";\n> > > > > > > > +\t\t\t#clock-cells = <0>;\n> > > > > > > > +\t\t\tclock-frequency = <24000000>;\n> > > > > > > > +\t\t};\n> > > > +\t};\n> > \n> > Happy to take this as it is what was done for the ast2400. Joel is getting the\n> > clock driver sorted out so hopefully we can remove it in the near future.\n> > \n> \n> Ok.\n> \n> > > +\n> > > > > > > >  \tahb {\n> > > > > > > >  \t\tcompatible = \"simple-bus\";\n> > > >  \t\t#address-cells = <1>;\n> > > \n> > > @@ -372,6 +380,15 @@\n> > > > > > > >  \t\t\t\t#size-cells = <1>;\n> > > > > > > >  \t\t\t\tranges = <0 0x1e78a000 0x1000>;\n> > > >  \t\t\t};\n> > > \n> > > +\n> > > > > +\t\t\tpwm_tacho: pwm-tacho-controller@1e786000 {\n> > > > \n> > > > > > > > +\t\t\t\tcompatible = \"aspeed,ast2500-pwm-tacho\";\n> > > > > > > > +\t\t\t\t#address-cells = <1>;\n> > > > +\t\t\t\t#size-cells = <0>;\n> > \n> > The bindings documentation claims #size-cells should be 1, however both the\n> > qanta dts and your patch set this to zero. The child nodes corroborate the\n> > value of 0 - we're specifying the PWM channel which has no need for a\n> > length.\n> \n> I think it’s a mistake in documentation.\n\nRight; something we should fix. I'll include this with my upstream discussion\non fans.\n\nCheers,\n\nAndrew","headers":{"Return-Path":"<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>","X-Original-To":["incoming@patchwork.ozlabs.org","openbmc@lists.ozlabs.org"],"Delivered-To":["patchwork-incoming@bilbo.ozlabs.org","openbmc@lists.ozlabs.org"],"Received":["from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68])\n\t(using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xhtNN5mP2z9sN7\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 30 Aug 2017 14:50:12 +1000 (AEST)","from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3])\n\tby lists.ozlabs.org (Postfix) with ESMTP id 3xhtNN4Cc3zDqJ8\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 30 Aug 2017 14:50:12 +1000 (AEST)","from out2-smtp.messagingengine.com (out2-smtp.messagingengine.com\n\t[66.111.4.26])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby lists.ozlabs.org (Postfix) with ESMTPS id 3xhtNC0sYmzDq5b\n\tfor <openbmc@lists.ozlabs.org>; Wed, 30 Aug 2017 14:50:03 +1000 (AEST)","from compute4.internal (compute4.nyi.internal [10.202.2.44])\n\tby mailout.nyi.internal (Postfix) with ESMTP id E4EA62139A;\n\tWed, 30 Aug 2017 00:50:00 -0400 (EDT)","from frontend2 ([10.202.2.161])\n\tby compute4.internal (MEProxy); Wed, 30 Aug 2017 00:50:00 -0400","from keelia (unknown [203.0.153.9])\n\tby mail.messagingengine.com (Postfix) with ESMTPA id 7968B24810;\n\tWed, 30 Aug 2017 00:49:58 -0400 (EDT)"],"Authentication-Results":["ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"SaPwDKiW\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"JSoUq0Ku\"; \n\tdkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"SaPwDKiW\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com header.b=\"JSoUq0Ku\"; \n\tdkim-atps=neutral","lists.ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=aj.id.au header.i=@aj.id.au header.b=\"SaPwDKiW\";\n\tdkim=pass (2048-bit key;\n\tunprotected) header.d=messagingengine.com\n\theader.i=@messagingengine.com\n\theader.b=\"JSoUq0Ku\"; dkim-atps=neutral"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=aj.id.au; h=cc\n\t:content-type:date:from:in-reply-to:message-id:mime-version\n\t:references:subject:to:x-me-sender:x-me-sender:x-sasl-enc\n\t:x-sasl-enc; s=fm1; bh=Z5eL22DLARtgDXB41EvikyVi9/1fSNyPiNNR2kP04\n\tVY=; b=SaPwDKiW1HOVUVe8KBt199Z4nPNGTSpJ5c/K5ej6SHsv4eEVSN9/mb9mY\n\trJlgnoZnKOSPz9M6lh7RAYR8hF+ZdopnE8Ealr1CgzPaTcwEtiXHcbfqFGE6vNCn\n\tPDZzkTToKpWTMAwSsm0t0lZs5/GDMmZHKq5ZzFNBxPHKDXAQqU/hM32zW42fhvbg\n\tDkM9XhelmEH2BwEWOxZ4OL67yDMmallkfT5oawDFqxPliOzrbPeeUiLti9doFkoR\n\tHUM8stVrJDckSd+8iaFuh5g9sx0/2NVnsKHjfrGiiWZbd0Mn097eaqMBC7Dob8sN\n\tcHpbasiEdvUeXeeD5OcPHMm5ZFiGQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=\n\tmessagingengine.com; h=cc:content-type:date:from:in-reply-to\n\t:message-id:mime-version:references:subject:to:x-me-sender\n\t:x-me-sender:x-sasl-enc:x-sasl-enc; s=fm1; bh=Z5eL22DLARtgDXB41E\n\tvikyVi9/1fSNyPiNNR2kP04VY=; b=JSoUq0Ku9GUBal5lDAT9YW5RJM1iIOXiQP\n\tNTLFd6F1SsE/hRUkgON4wRYKsuHtqXgete8qJDze6RcBZ64qVWMIs3sOtwneRMkX\n\taQIfsKNX9mF/MaTTpaaBaqC602TtSkCtYG+rsLEJRfXKvRGhZEtcFPO/1fc63Uqa\n\tWjy46fVlR9NBWY8buvuIkdRBPmCjgUa7l4m/2veuURZzpbWYG44H0X+ZAoAghp4X\n\tBmdnSjmw9e/dowu8BcLE/2oPzCqsfl/a4cGdw1h7iSG1ZM7jxeUGngHoObeqKRP3\n\tPH9B1auDfm9SlnSEtREGSl68uQToas9Osn5hUBkgtAFTsRyk2drQ=="],"X-ME-Sender":"<xms:-EOmWX0G_ToPSjrotlnlr7dHt-LYqn_Ipt7TGDBncQAEONGf-Xmchw>","X-Sasl-enc":"gFv5NDMcm2oEfzZxPQywryWCcsy9G9GxeCI7MXp46SkW 1504068600","Message-ID":"<1504068592.5933.21.camel@aj.id.au>","Subject":"Re: [PATCH linux dev-4.10 resend] dts: add aspeed-pwm-tacho to msn\n\tdts.","From":"Andrew Jeffery <andrew@aj.id.au>","To":"Mykola Kostenok <c_mykolak@mellanox.com>, Joel Stanley <joel@jms.id.au>, \n\t\"openbmc@lists.ozlabs.org\" <openbmc@lists.ozlabs.org>","Date":"Wed, 30 Aug 2017 14:19:52 +0930","In-Reply-To":"<DB5PR05MB122434BAC10BF91338D87500EF9F0@DB5PR05MB1224.eurprd05.prod.outlook.com>","References":"<20170823073313.1294-1-c_mykolak@mellanox.com>\n\t<1503994514.5933.8.camel@aj.id.au>\n\t<DB5PR05MB122434BAC10BF91338D87500EF9F0@DB5PR05MB1224.eurprd05.prod.outlook.com>","Content-Type":"multipart/signed; micalg=\"pgp-sha512\";\n\tprotocol=\"application/pgp-signature\";\n\tboundary=\"=-OD+hO/Jt07/4La8G5L7G\"","X-Mailer":"Evolution 3.22.6-1ubuntu1 ","Mime-Version":"1.0","X-BeenThere":"openbmc@lists.ozlabs.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"Development list for OpenBMC <openbmc.lists.ozlabs.org>","List-Unsubscribe":"<https://lists.ozlabs.org/options/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=unsubscribe>","List-Archive":"<http://lists.ozlabs.org/pipermail/openbmc/>","List-Post":"<mailto:openbmc@lists.ozlabs.org>","List-Help":"<mailto:openbmc-request@lists.ozlabs.org?subject=help>","List-Subscribe":"<https://lists.ozlabs.org/listinfo/openbmc>,\n\t<mailto:openbmc-request@lists.ozlabs.org?subject=subscribe>","Cc":"Ohad Oz <ohado@mellanox.com>, Vadim Pasternak <vadimp@mellanox.com>","Errors-To":"openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org","Sender":"\"openbmc\"\n\t<openbmc-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>"}}]