From mboxrd@z Thu Jan 1 00:00:00 1970 From: sboyd@codeaurora.org (Stephen Boyd) Date: Tue, 2 Jan 2018 11:01:59 -0800 Subject: [PATCH 01/33] clk_ops: change round_rate() to return unsigned long In-Reply-To: <6d83a5c3-6589-24bc-4ca5-4d1bbca47432@nexus-software.ie> References: <1514596392-22270-1-git-send-email-pure.logic@nexus-software.ie> <1514596392-22270-2-git-send-email-pure.logic@nexus-software.ie> <9f4bef5a-8a71-6f30-5cfb-5e8fe133e3d3@kapsi.fi> <6d83a5c3-6589-24bc-4ca5-4d1bbca47432@nexus-software.ie> Message-ID: <20180102190159.GH7997@codeaurora.org> To: linus-amlogic@lists.infradead.org List-Id: linus-amlogic.lists.infradead.org On 12/31, Bryan O'Donoghue wrote: > On 30/12/17 16:36, Mikko Perttunen wrote: > >FWIW, we had this problem some years ago with the Tegra CPU clock > >- then it was determined that a simpler solution was to have the > >determine_rate callback support unsigned long rates - so clock > >drivers that need to return rates higher than 2^31 can instead > >implement the determine_rate callback. That is what's currently > >implemented. > > > >Mikko > > Granted we could work around it but, having both zero and less than > zero indicate error means you can't support larger than LONG_MAX > which is I think worth fixing. > Ok. But can you implement the determine_rate op instead of the round_rate op for your clk? It's not a work-around, it's the preferred solution. That would allow rates larger than 2^31 for the clk without pushing through a change to all the drivers to express zero as "error" and non-zero as the rounded rate. I'm not entirely opposed to this approach, because we probably don't care to pass the particular error value from a clk provider to a clk consumer about what the error is. It's actually what we proposed as the solution for clk_round_rate() to return values larger than LONG_MAX to consumers. But doing that consumer API change or this provider side change is going to require us to evaluate all the consumers of these clks to make sure they don't check for some error value that's less than zero. This series does half the work, by changing the provider side, while ignoring the consumer side and any potential fallout of the less than zero to zero return value change. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH 01/33] clk_ops: change round_rate() to return unsigned long Date: Tue, 2 Jan 2018 11:01:59 -0800 Message-ID: <20180102190159.GH7997@codeaurora.org> References: <1514596392-22270-1-git-send-email-pure.logic@nexus-software.ie> <1514596392-22270-2-git-send-email-pure.logic@nexus-software.ie> <9f4bef5a-8a71-6f30-5cfb-5e8fe133e3d3@kapsi.fi> <6d83a5c3-6589-24bc-4ca5-4d1bbca47432@nexus-software.ie> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <6d83a5c3-6589-24bc-4ca5-4d1bbca47432@nexus-software.ie> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Bryan O'Donoghue Cc: linux-mips@linux-mips.org, mturquette@baylibre.com, dri-devel@lists.freedesktop.org, linux-clk@vger.kernel.org, linux-rtc@vger.kernel.org, linux-samsung-soc@vger.kernel.org, Mikko Perttunen , linux-rockchip@lists.infradead.org, linux-media@vger.kernel.org, uclinux-h8-devel@lists.sourceforge.jp, linux-arm-msm@vger.kernel.org, linux-mediatek@lists.infradead.org, linux-rpi-kernel@lists.infradead.org, linux-tegra@vger.kernel.org, linux-amlogic@lists.infradead.org, linux-omap@vger.kernel.org, linux-soc@vger.kernel.org, linux-arm-kernel@lists.infradead.org, patches@opensource.cirrus.com, linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, freedreno@lists.freedesktop.org List-Id: linux-arm-msm@vger.kernel.org T24gMTIvMzEsIEJyeWFuIE8nRG9ub2dodWUgd3JvdGU6Cj4gT24gMzAvMTIvMTcgMTY6MzYsIE1p a2tvIFBlcnR0dW5lbiB3cm90ZToKPiA+RldJVywgd2UgaGFkIHRoaXMgcHJvYmxlbSBzb21lIHll YXJzIGFnbyB3aXRoIHRoZSBUZWdyYSBDUFUgY2xvY2sKPiA+LSB0aGVuIGl0IHdhcyBkZXRlcm1p bmVkIHRoYXQgYSBzaW1wbGVyIHNvbHV0aW9uIHdhcyB0byBoYXZlIHRoZQo+ID5kZXRlcm1pbmVf cmF0ZSBjYWxsYmFjayBzdXBwb3J0IHVuc2lnbmVkIGxvbmcgcmF0ZXMgLSBzbyBjbG9jawo+ID5k cml2ZXJzIHRoYXQgbmVlZCB0byByZXR1cm4gcmF0ZXMgaGlnaGVyIHRoYW4gMl4zMSBjYW4gaW5z dGVhZAo+ID5pbXBsZW1lbnQgdGhlIGRldGVybWluZV9yYXRlIGNhbGxiYWNrLiBUaGF0IGlzIHdo YXQncyBjdXJyZW50bHkKPiA+aW1wbGVtZW50ZWQuCj4gPgo+ID5NaWtrbwo+IAo+IEdyYW50ZWQg d2UgY291bGQgd29yayBhcm91bmQgaXQgYnV0LCBoYXZpbmcgYm90aCB6ZXJvIGFuZCBsZXNzIHRo YW4KPiB6ZXJvIGluZGljYXRlIGVycm9yIG1lYW5zIHlvdSBjYW4ndCBzdXBwb3J0IGxhcmdlciB0 aGFuIExPTkdfTUFYCj4gd2hpY2ggaXMgSSB0aGluayB3b3J0aCBmaXhpbmcuCj4gCgpPay4gQnV0 IGNhbiB5b3UgaW1wbGVtZW50IHRoZSBkZXRlcm1pbmVfcmF0ZSBvcCBpbnN0ZWFkIG9mIHRoZQpy b3VuZF9yYXRlIG9wIGZvciB5b3VyIGNsaz8gSXQncyBub3QgYSB3b3JrLWFyb3VuZCwgaXQncyB0 aGUKcHJlZmVycmVkIHNvbHV0aW9uLiBUaGF0IHdvdWxkIGFsbG93IHJhdGVzIGxhcmdlciB0aGFu IDJeMzEgZm9yCnRoZSBjbGsgd2l0aG91dCBwdXNoaW5nIHRocm91Z2ggYSBjaGFuZ2UgdG8gYWxs IHRoZSBkcml2ZXJzIHRvCmV4cHJlc3MgemVybyBhcyAiZXJyb3IiIGFuZCBub24temVybyBhcyB0 aGUgcm91bmRlZCByYXRlLgoKSSdtIG5vdCBlbnRpcmVseSBvcHBvc2VkIHRvIHRoaXMgYXBwcm9h Y2gsIGJlY2F1c2Ugd2UgcHJvYmFibHkKZG9uJ3QgY2FyZSB0byBwYXNzIHRoZSBwYXJ0aWN1bGFy IGVycm9yIHZhbHVlIGZyb20gYSBjbGsgcHJvdmlkZXIKdG8gYSBjbGsgY29uc3VtZXIgYWJvdXQg d2hhdCB0aGUgZXJyb3IgaXMuIEl0J3MgYWN0dWFsbHkgd2hhdCB3ZQpwcm9wb3NlZCBhcyB0aGUg c29sdXRpb24gZm9yIGNsa19yb3VuZF9yYXRlKCkgdG8gcmV0dXJuIHZhbHVlcwpsYXJnZXIgdGhh biBMT05HX01BWCB0byBjb25zdW1lcnMuIEJ1dCBkb2luZyB0aGF0IGNvbnN1bWVyIEFQSQpjaGFu Z2Ugb3IgdGhpcyBwcm92aWRlciBzaWRlIGNoYW5nZSBpcyBnb2luZyB0byByZXF1aXJlIHVzIHRv CmV2YWx1YXRlIGFsbCB0aGUgY29uc3VtZXJzIG9mIHRoZXNlIGNsa3MgdG8gbWFrZSBzdXJlIHRo ZXkgZG9uJ3QKY2hlY2sgZm9yIHNvbWUgZXJyb3IgdmFsdWUgdGhhdCdzIGxlc3MgdGhhbiB6ZXJv LiBUaGlzIHNlcmllcwpkb2VzIGhhbGYgdGhlIHdvcmssIGJ5IGNoYW5naW5nIHRoZSBwcm92aWRl ciBzaWRlLCB3aGlsZSBpZ25vcmluZwp0aGUgY29uc3VtZXIgc2lkZSBhbmQgYW55IHBvdGVudGlh bCBmYWxsb3V0IG9mIHRoZSBsZXNzIHRoYW4gemVybwp0byB6ZXJvIHJldHVybiB2YWx1ZSBjaGFu Z2UuCgotLSAKUXVhbGNvbW0gSW5ub3ZhdGlvbiBDZW50ZXIsIEluYy4gaXMgYSBtZW1iZXIgb2Yg Q29kZSBBdXJvcmEgRm9ydW0sCmEgTGludXggRm91bmRhdGlvbiBDb2xsYWJvcmF0aXZlIFByb2pl Y3QKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRl dmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8v bGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 2 Jan 2018 11:01:59 -0800 From: Stephen Boyd To: Bryan O'Donoghue Cc: Mikko Perttunen , mturquette@baylibre.com, linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mips@linux-mips.org, linux-rpi-kernel@lists.infradead.org, patches@opensource.cirrus.com, uclinux-h8-devel@lists.sourceforge.jp, linux-amlogic@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-soc@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-tegra@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-mediatek@lists.infradead.org, freedreno@lists.freedesktop.org, linux-media@vger.kernel.org, linux-rtc@vger.kernel.org Subject: Re: [PATCH 01/33] clk_ops: change round_rate() to return unsigned long Message-ID: <20180102190159.GH7997@codeaurora.org> References: <1514596392-22270-1-git-send-email-pure.logic@nexus-software.ie> <1514596392-22270-2-git-send-email-pure.logic@nexus-software.ie> <9f4bef5a-8a71-6f30-5cfb-5e8fe133e3d3@kapsi.fi> <6d83a5c3-6589-24bc-4ca5-4d1bbca47432@nexus-software.ie> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <6d83a5c3-6589-24bc-4ca5-4d1bbca47432@nexus-software.ie> List-ID: On 12/31, Bryan O'Donoghue wrote: > On 30/12/17 16:36, Mikko Perttunen wrote: > >FWIW, we had this problem some years ago with the Tegra CPU clock > >- then it was determined that a simpler solution was to have the > >determine_rate callback support unsigned long rates - so clock > >drivers that need to return rates higher than 2^31 can instead > >implement the determine_rate callback. That is what's currently > >implemented. > > > >Mikko > > Granted we could work around it but, having both zero and less than > zero indicate error means you can't support larger than LONG_MAX > which is I think worth fixing. > Ok. But can you implement the determine_rate op instead of the round_rate op for your clk? It's not a work-around, it's the preferred solution. That would allow rates larger than 2^31 for the clk without pushing through a change to all the drivers to express zero as "error" and non-zero as the rounded rate. I'm not entirely opposed to this approach, because we probably don't care to pass the particular error value from a clk provider to a clk consumer about what the error is. It's actually what we proposed as the solution for clk_round_rate() to return values larger than LONG_MAX to consumers. But doing that consumer API change or this provider side change is going to require us to evaluate all the consumers of these clks to make sure they don't check for some error value that's less than zero. This series does half the work, by changing the provider side, while ignoring the consumer side and any potential fallout of the less than zero to zero return value change. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project From mboxrd@z Thu Jan 1 00:00:00 1970 From: sboyd@codeaurora.org (Stephen Boyd) Date: Tue, 2 Jan 2018 11:01:59 -0800 Subject: [PATCH 01/33] clk_ops: change round_rate() to return unsigned long In-Reply-To: <6d83a5c3-6589-24bc-4ca5-4d1bbca47432@nexus-software.ie> References: <1514596392-22270-1-git-send-email-pure.logic@nexus-software.ie> <1514596392-22270-2-git-send-email-pure.logic@nexus-software.ie> <9f4bef5a-8a71-6f30-5cfb-5e8fe133e3d3@kapsi.fi> <6d83a5c3-6589-24bc-4ca5-4d1bbca47432@nexus-software.ie> Message-ID: <20180102190159.GH7997@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 12/31, Bryan O'Donoghue wrote: > On 30/12/17 16:36, Mikko Perttunen wrote: > >FWIW, we had this problem some years ago with the Tegra CPU clock > >- then it was determined that a simpler solution was to have the > >determine_rate callback support unsigned long rates - so clock > >drivers that need to return rates higher than 2^31 can instead > >implement the determine_rate callback. That is what's currently > >implemented. > > > >Mikko > > Granted we could work around it but, having both zero and less than > zero indicate error means you can't support larger than LONG_MAX > which is I think worth fixing. > Ok. But can you implement the determine_rate op instead of the round_rate op for your clk? It's not a work-around, it's the preferred solution. That would allow rates larger than 2^31 for the clk without pushing through a change to all the drivers to express zero as "error" and non-zero as the rounded rate. I'm not entirely opposed to this approach, because we probably don't care to pass the particular error value from a clk provider to a clk consumer about what the error is. It's actually what we proposed as the solution for clk_round_rate() to return values larger than LONG_MAX to consumers. But doing that consumer API change or this provider side change is going to require us to evaluate all the consumers of these clks to make sure they don't check for some error value that's less than zero. This series does half the work, by changing the provider side, while ignoring the consumer side and any potential fallout of the less than zero to zero return value change. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project