From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: use IORESOURCE_REG resource type for non-translatable addresses in DT Date: Tue, 29 Jul 2014 23:06:19 -0700 Message-ID: <20140730060619.GC19933@codeaurora.org> References: <53D788A7.4020303@mm-sol.com> <3794875.CZFbAag5Sv@wuerfel> <53D7AA72.6010309@mm-sol.com> <20140729234522.E9FF1C40738@trevor.secretlab.ca> <53D84558.7060406@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Rob Herring Cc: Grant Likely , Stanimir Varbanov , Arnd Bergmann , Rob Herring , Lee Jones , "linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" , Mark Brown List-Id: linux-arm-msm@vger.kernel.org On 07/29, Rob Herring wrote: > On Tue, Jul 29, 2014 at 8:07 PM, Stephen Boyd wrote: > > On 07/29/14 16:45, Grant Likely wrote: > >> On Tue, 29 Jul 2014 17:06:42 +0300, Stanimir Varbanov wrote: > >>> > >>> This was just an example. Of course it has many issues and probaly it is > >>> wrong:) The main goal was to understand does IORESOURCE_REG resource > >>> type and parsing the *reg* properties for non-translatable addresses are > >>> feasible. And also does it acceptable by community and OF platform > >>> maintainers. > >> The use case is actually very different from of_address_to_resource or > >> of_get_address() because those APIs explicitly return physical memory > >> addresses from the CPU perspective. It makes more sense to create a new > >> API that doesn't attempt to translate the reg address. Alternately, a > >> new API that only translates upto a given parent node. > > > > The most important thing is that platform_get_resource{_by_name}(&pdev, > > IORESOURCE_REG, n) returns the reg property and optional size encoded > > into a struct resource. I think Rob is suggesting we circumvent the > > entire of_address_to_resource() path and do some if > > (IS_ENABLED(CONFIG_OF) && type == IORESOURCE_REG) check in > > platform_get_resource() to package up the reg property into a struct > > resource. That should work. > > No, I'm saying why are you using platform_get_resource at all and > adding a new resource type? I don't see any advantage. First off, the resource type is not new. IORESOURCE_REG has existed for two years (see commit 72dcb1197228 "resources: Add register address resource type, 2012-08-07"). The main advantage is allowing things like platform_get_resource_by_name() and platform_get_resource() to work seamlessly with devicetree. If we don't have this, drivers are going to open code their reg property parsing and possibly reg-names parsing. There are a handful of drivers that would be doing this duplicate work. Sure, we could consolidate them into an OF helper function, but then these are the only platform drivers that are getting their reg properties via a special non-translatable address function. The drivers don't care that they're using non-translateable addresses as long as they can pass the address returned from platform_get_resource() to their regmap and do I/O. The drivers are written under the assumption that they're a sub-device of some parent device (in this case a PMIC) and so they assume that the regmap has already been setup for them. > > You might as well do of_property_read_u32 in the below example. > Fair enough. The example is probably too simple. Things are sometimes slightly more complicated and a simple of_property_read_u32() isn't going to work in the case of multiple reg values or when reg-names is in play. -- 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 devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754914AbaG3GGY (ORCPT ); Wed, 30 Jul 2014 02:06:24 -0400 Received: from smtp.codeaurora.org ([198.145.11.231]:42317 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754759AbaG3GGW (ORCPT ); Wed, 30 Jul 2014 02:06:22 -0400 Date: Tue, 29 Jul 2014 23:06:19 -0700 From: Stephen Boyd To: Rob Herring Cc: Grant Likely , Stanimir Varbanov , Arnd Bergmann , Rob Herring , Lee Jones , "linux-arm-msm@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Mark Brown Subject: Re: use IORESOURCE_REG resource type for non-translatable addresses in DT Message-ID: <20140730060619.GC19933@codeaurora.org> References: <53D788A7.4020303@mm-sol.com> <3794875.CZFbAag5Sv@wuerfel> <53D7AA72.6010309@mm-sol.com> <20140729234522.E9FF1C40738@trevor.secretlab.ca> <53D84558.7060406@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/29, Rob Herring wrote: > On Tue, Jul 29, 2014 at 8:07 PM, Stephen Boyd wrote: > > On 07/29/14 16:45, Grant Likely wrote: > >> On Tue, 29 Jul 2014 17:06:42 +0300, Stanimir Varbanov wrote: > >>> > >>> This was just an example. Of course it has many issues and probaly it is > >>> wrong:) The main goal was to understand does IORESOURCE_REG resource > >>> type and parsing the *reg* properties for non-translatable addresses are > >>> feasible. And also does it acceptable by community and OF platform > >>> maintainers. > >> The use case is actually very different from of_address_to_resource or > >> of_get_address() because those APIs explicitly return physical memory > >> addresses from the CPU perspective. It makes more sense to create a new > >> API that doesn't attempt to translate the reg address. Alternately, a > >> new API that only translates upto a given parent node. > > > > The most important thing is that platform_get_resource{_by_name}(&pdev, > > IORESOURCE_REG, n) returns the reg property and optional size encoded > > into a struct resource. I think Rob is suggesting we circumvent the > > entire of_address_to_resource() path and do some if > > (IS_ENABLED(CONFIG_OF) && type == IORESOURCE_REG) check in > > platform_get_resource() to package up the reg property into a struct > > resource. That should work. > > No, I'm saying why are you using platform_get_resource at all and > adding a new resource type? I don't see any advantage. First off, the resource type is not new. IORESOURCE_REG has existed for two years (see commit 72dcb1197228 "resources: Add register address resource type, 2012-08-07"). The main advantage is allowing things like platform_get_resource_by_name() and platform_get_resource() to work seamlessly with devicetree. If we don't have this, drivers are going to open code their reg property parsing and possibly reg-names parsing. There are a handful of drivers that would be doing this duplicate work. Sure, we could consolidate them into an OF helper function, but then these are the only platform drivers that are getting their reg properties via a special non-translatable address function. The drivers don't care that they're using non-translateable addresses as long as they can pass the address returned from platform_get_resource() to their regmap and do I/O. The drivers are written under the assumption that they're a sub-device of some parent device (in this case a PMIC) and so they assume that the regmap has already been setup for them. > > You might as well do of_property_read_u32 in the below example. > Fair enough. The example is probably too simple. Things are sometimes slightly more complicated and a simple of_property_read_u32() isn't going to work in the case of multiple reg values or when reg-names is in play. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation From mboxrd@z Thu Jan 1 00:00:00 1970 From: sboyd@codeaurora.org (Stephen Boyd) Date: Tue, 29 Jul 2014 23:06:19 -0700 Subject: use IORESOURCE_REG resource type for non-translatable addresses in DT In-Reply-To: References: <53D788A7.4020303@mm-sol.com> <3794875.CZFbAag5Sv@wuerfel> <53D7AA72.6010309@mm-sol.com> <20140729234522.E9FF1C40738@trevor.secretlab.ca> <53D84558.7060406@codeaurora.org> Message-ID: <20140730060619.GC19933@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 07/29, Rob Herring wrote: > On Tue, Jul 29, 2014 at 8:07 PM, Stephen Boyd wrote: > > On 07/29/14 16:45, Grant Likely wrote: > >> On Tue, 29 Jul 2014 17:06:42 +0300, Stanimir Varbanov wrote: > >>> > >>> This was just an example. Of course it has many issues and probaly it is > >>> wrong:) The main goal was to understand does IORESOURCE_REG resource > >>> type and parsing the *reg* properties for non-translatable addresses are > >>> feasible. And also does it acceptable by community and OF platform > >>> maintainers. > >> The use case is actually very different from of_address_to_resource or > >> of_get_address() because those APIs explicitly return physical memory > >> addresses from the CPU perspective. It makes more sense to create a new > >> API that doesn't attempt to translate the reg address. Alternately, a > >> new API that only translates upto a given parent node. > > > > The most important thing is that platform_get_resource{_by_name}(&pdev, > > IORESOURCE_REG, n) returns the reg property and optional size encoded > > into a struct resource. I think Rob is suggesting we circumvent the > > entire of_address_to_resource() path and do some if > > (IS_ENABLED(CONFIG_OF) && type == IORESOURCE_REG) check in > > platform_get_resource() to package up the reg property into a struct > > resource. That should work. > > No, I'm saying why are you using platform_get_resource at all and > adding a new resource type? I don't see any advantage. First off, the resource type is not new. IORESOURCE_REG has existed for two years (see commit 72dcb1197228 "resources: Add register address resource type, 2012-08-07"). The main advantage is allowing things like platform_get_resource_by_name() and platform_get_resource() to work seamlessly with devicetree. If we don't have this, drivers are going to open code their reg property parsing and possibly reg-names parsing. There are a handful of drivers that would be doing this duplicate work. Sure, we could consolidate them into an OF helper function, but then these are the only platform drivers that are getting their reg properties via a special non-translatable address function. The drivers don't care that they're using non-translateable addresses as long as they can pass the address returned from platform_get_resource() to their regmap and do I/O. The drivers are written under the assumption that they're a sub-device of some parent device (in this case a PMIC) and so they assume that the regmap has already been setup for them. > > You might as well do of_property_read_u32 in the below example. > Fair enough. The example is probably too simple. Things are sometimes slightly more complicated and a simple of_property_read_u32() isn't going to work in the case of multiple reg values or when reg-names is in play. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation