| Message ID | 79CD15C6BA57404B839C016229A409A83EA8F4CD@DBDE01.ent.ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
* Hiremath, Vaibhav <hvaibhav@ti.com> [120815 02:11]: > On Tue, Aug 14, 2012 at 13:59:12, Tony Lindgren wrote: > > Hi, > > > > * Paul Walmsley <paul@pwsan.com> [120726 13:07]: > > > On Wed, 25 Jul 2012, Paul Walmsley wrote: > > > > > > > These IP blocks and the ECAP IP blocks have periods in their names. > > > > The rest of the IP block named in the file don't use periods -- which > > > > is also the style used by the rest of the OMAP SoCs. Is there > > > > some reason that these have periods in their names? > > > > > > I've changed those to match the rest of the names, and queued the > > > following for 3.7. > > > > > > Thanks again for all the hard work you all put into this. I realize that > > > with all the upstream changes, this was probably a little painful for you. > > > But from my perspective you've been a pleasure to work with through the > > > process. > > > > As am33xx and omap5 are DT only, we should start moving towards getting > > rid of grep -E "irq|pa_start|pa_end|dma" in this patch and get the standard > > data from .dtsi files instead. > > > > It may not be so straight, given the fact that hwmod layer > requires these resources, as hwmod is responsible to configure SYSCONF > register of the device (happens very early at core_init level). > > Somehow we have to have some mechanism to initialize " _mpu_rt_va" before > core_init call, which will take DT resources and initialize accordingly. That's for the device reset/idle? We should not do that in hwmod code except in late_initcall for unclaimed devices as discussed earlier. We discussed setting up the reset function in the device specific headers so both device and hwmod code can call them as needed. > Another dependency we will have is on system timers, Jon has already done > some initial work on this, but I believe we did not concluded on the it. > I will re-start the thread again, since in any case DT support in timer is > required. Right.. The timers are a pain right now as they need to be initialized early. Everything else can and should be initialized at the device probe time, or from a late_initcall for the unclaimed devices. > > Alternatively we could get rid of the names and match the module based on > > pa_start and pa_end. > > > > Just FYI, from resource perspective, I have changed omap_device layer for DT > resources only (still need hwmod resources as explained above) and patch > looks something like (and it works) - Cool yeah few more dependencies remain still. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 17, 2012 at 13:19:34, Tony Lindgren wrote: > * Hiremath, Vaibhav <hvaibhav@ti.com> [120815 02:11]: > > On Tue, Aug 14, 2012 at 13:59:12, Tony Lindgren wrote: > > > Hi, > > > > > > * Paul Walmsley <paul@pwsan.com> [120726 13:07]: > > > > On Wed, 25 Jul 2012, Paul Walmsley wrote: > > > > > > > > > These IP blocks and the ECAP IP blocks have periods in their names. > > > > > The rest of the IP block named in the file don't use periods -- which > > > > > is also the style used by the rest of the OMAP SoCs. Is there > > > > > some reason that these have periods in their names? > > > > > > > > I've changed those to match the rest of the names, and queued the > > > > following for 3.7. > > > > > > > > Thanks again for all the hard work you all put into this. I realize that > > > > with all the upstream changes, this was probably a little painful for you. > > > > But from my perspective you've been a pleasure to work with through the > > > > process. > > > > > > As am33xx and omap5 are DT only, we should start moving towards getting > > > rid of grep -E "irq|pa_start|pa_end|dma" in this patch and get the standard > > > data from .dtsi files instead. > > > > > > > It may not be so straight, given the fact that hwmod layer > > requires these resources, as hwmod is responsible to configure SYSCONF > > register of the device (happens very early at core_init level). > > > > Somehow we have to have some mechanism to initialize " _mpu_rt_va" before > > core_init call, which will take DT resources and initialize accordingly. > > That's for the device reset/idle? We should not do that in hwmod code > except in late_initcall for unclaimed devices as discussed earlier. We > discussed setting up the reset function in the device specific headers > so both device and hwmod code can call them as needed. > Tony, May be I didn't understand your above statement, can you please elaborate more on "device specific header file"? Just to highlight, its not only during late_initcall, the _enable and _idle functions are getting executed as part of every runtime_pm_get\put_sync from the driver. Are you saying as part of late_initcall, we should initialize " _mpu_rt_va" of "struct omap_hwmod"??? Also, as far unclaimed resources is concerned, somewhere you should have base addr of the device maintained, right? Currently, hwmod is maintaining this data and the execution goes something like, Core_initcall() -> _setup() -> _setup_reset() -> _idle() Another biggest worry is, if I play here, it may break existing omap3/4 Functionality, especially may impact from PM perspective. So I believe, we need to have something which adds/cleans-up things in stages. May be get rid of resource overwriting for AM33xx and OMAP5 alone as of now?? Any thoughts?? Thanks, Vaibhav > > Another dependency we will have is on system timers, Jon has already done > > some initial work on this, but I believe we did not concluded on the it. > > I will re-start the thread again, since in any case DT support in timer is > > required. > > Right.. The timers are a pain right now as they need to be initialized > early. Everything else can and should be initialized at the device probe > time, or from a late_initcall for the unclaimed devices. > > > > Alternatively we could get rid of the names and match the module based on > > > pa_start and pa_end. > > > > > > > Just FYI, from resource perspective, I have changed omap_device layer for DT > > resources only (still need hwmod resources as explained above) and patch > > looks something like (and it works) - > > Cool yeah few more dependencies remain still. > > Regards, > > Tony > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Hiremath, Vaibhav <hvaibhav@ti.com> [120817 01:22]: > On Fri, Aug 17, 2012 at 13:19:34, Tony Lindgren wrote: > > * Hiremath, Vaibhav <hvaibhav@ti.com> [120815 02:11]: > > > > > > Somehow we have to have some mechanism to initialize " _mpu_rt_va" before > > > core_init call, which will take DT resources and initialize accordingly. > > > > That's for the device reset/idle? We should not do that in hwmod code > > except in late_initcall for unclaimed devices as discussed earlier. We > > discussed setting up the reset function in the device specific headers > > so both device and hwmod code can call them as needed. > > May be I didn't understand your above statement, can you please elaborate > more on "device specific header file"? We should have the device drivers idle and reset the devices. But also hwmod may need to do that for the unclaimed devices for PM to work. As the driver may not be even compiled in, the driver specific reset and idle functions should be static inline functions in the device specific header file so hwmod code can still call those. > Just to highlight, its not only during late_initcall, the _enable and _idle > functions are getting executed as part of every runtime_pm_get\put_sync from > the driver. Right, so no need to initialize those until at driver probe time. > Are you saying as part of late_initcall, we should initialize " _mpu_rt_va" > of "struct omap_hwmod"??? I guess either at driver probe for the claimed devices, or at late_initcall for the unclaimeded devices. > Also, as far unclaimed resources is concerned, somewhere you should have > base addr of the device maintained, right? Currently, hwmod is maintaining > this data and the execution goes something like, > > Core_initcall() > -> _setup() > -> _setup_reset() > -> _idle() For the DT only case also the unclaimed devices can from DT with status = "disabled". > Another biggest worry is, if I play here, it may break existing omap3/4 > Functionality, especially may impact from PM perspective. So I believe, we > need to have something which adds/cleans-up things in stages. It seems that we need three stages: Something early for the timers only, initialize most things during driver probe, then late_initcall for the unclaimed devices. > May be get rid of resource overwriting for AM33xx and OMAP5 alone as of > now?? Sorry I don't follow this part, care to explain a little more? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 17, 2012 at 14:10:47, Tony Lindgren wrote: > * Hiremath, Vaibhav <hvaibhav@ti.com> [120817 01:22]: > > On Fri, Aug 17, 2012 at 13:19:34, Tony Lindgren wrote: > > > * Hiremath, Vaibhav <hvaibhav@ti.com> [120815 02:11]: > > > > > > > > Somehow we have to have some mechanism to initialize " _mpu_rt_va" before > > > > core_init call, which will take DT resources and initialize accordingly. > > > > > > That's for the device reset/idle? We should not do that in hwmod code > > > except in late_initcall for unclaimed devices as discussed earlier. We > > > discussed setting up the reset function in the device specific headers > > > so both device and hwmod code can call them as needed. > > > > May be I didn't understand your above statement, can you please elaborate > > more on "device specific header file"? > > We should have the device drivers idle and reset the devices. But also > hwmod may need to do that for the unclaimed devices for PM to work. > As the driver may not be even compiled in, the driver specific reset and > idle functions should be static inline functions in the device specific > header file so hwmod code can still call those. > How about mapping the address of the devices? Some more thoughts on the similar line on unclaimed devices - As par of late_initcall(), traverse all the DTB nodes and for each node with "state = disabled", we can do something like, Late_initcall() -> Traverse DTB nodes and check for "state = disabled" -> Read the "ti,hwmod" entry and get offsets like, sys_config and reset. -> Map the base address -> Enable the module/clock -> Reset the device -> Write to sysc register offset -> Disable the module/clock > > Just to highlight, its not only during late_initcall, the _enable and _idle > > functions are getting executed as part of every runtime_pm_get\put_sync from > > the driver. > > Right, so no need to initialize those until at driver probe time. > Agreed. > > Are you saying as part of late_initcall, we should initialize " _mpu_rt_va" > > of "struct omap_hwmod"??? > > I guess either at driver probe for the claimed devices, or at late_initcall > for the unclaimeded devices. > > > Also, as far unclaimed resources is concerned, somewhere you should have > > base addr of the device maintained, right? Currently, hwmod is maintaining > > this data and the execution goes something like, > > > > Core_initcall() > > -> _setup() > > -> _setup_reset() > > -> _idle() > > For the DT only case also the unclaimed devices can from DT with > status = "disabled". > > > Another biggest worry is, if I play here, it may break existing omap3/4 > > Functionality, especially may impact from PM perspective. So I believe, we > > need to have something which adds/cleans-up things in stages. > > It seems that we need three stages: Something early for the timers only, > initialize most things during driver probe, then late_initcall for the > unclaimed devices. > Most likely yes, or it may even get into more stages. At the current stage, I really think it would be really nice and beneficial for driver developers from the community if they get Linux Boot from linux-next/master branch. I have seen lot of community folks are struggling with it and they are blocked because we do not have booting kernel on BeagleBone. They always end up patching kernel with HWMOD patch and submit the driver patches. So request to consider this cleanup as sequential patch on top of original HWMOD patch? > > May be get rid of resource overwriting for AM33xx and OMAP5 alone as of > > now?? > > Sorry I don't follow this part, care to explain a little more? > As per current implementation, we are using HWMOD information to fill the platform_device->resources. Even if you pass "reg" and "interrupt" property from DT, omap_device layer overwrites it with HWMOD data information. So what I am trying to propose here is, avoid this overwrite and resources (address and interrupt) should strictly gets used from DT blob. Since AM33xx and OMAP5 devices are the new devices and supports DT boot only, we can implement it easily for them. Only issue her is, DMA resources, I was going through some of the old email archives today, submitted by Benoit and later Jon, but I am still not sure how to pass dma_channel to driver as a resource. Below logic works for me, I have tested it on both BeagleBone and AM37xEVM - omap_device_alloc () { ... res_count = omap_device_count_resources(od); if (res_count > pdev->num_resources) { res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL); if (!res) goto oda_exit3; if (pdev->num_resources && pdev->resource) { omap_device_fill_dma_resources(od, res); memcpy(&res[pdev->num_resources], pdev->resource, sizeof(struct resource) * pdev->num_resources); } else { omap_device_fill_resources(od, res); } ret = platform_device_add_resources(pdev, res, res_count); kfree(res); } ... } Thanks, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Hiremath, Vaibhav <hvaibhav@ti.com> [120817 02:51]: > On Fri, Aug 17, 2012 at 14:10:47, Tony Lindgren wrote: > > * Hiremath, Vaibhav <hvaibhav@ti.com> [120817 01:22]: > > > On Fri, Aug 17, 2012 at 13:19:34, Tony Lindgren wrote: > > > > * Hiremath, Vaibhav <hvaibhav@ti.com> [120815 02:11]: > > > > > > > > > > Somehow we have to have some mechanism to initialize " _mpu_rt_va" before > > > > > core_init call, which will take DT resources and initialize accordingly. > > > > > > > > That's for the device reset/idle? We should not do that in hwmod code > > > > except in late_initcall for unclaimed devices as discussed earlier. We > > > > discussed setting up the reset function in the device specific headers > > > > so both device and hwmod code can call them as needed. > > > > > > May be I didn't understand your above statement, can you please elaborate > > > more on "device specific header file"? > > > > We should have the device drivers idle and reset the devices. But also > > hwmod may need to do that for the unclaimed devices for PM to work. > > As the driver may not be even compiled in, the driver specific reset and > > idle functions should be static inline functions in the device specific > > header file so hwmod code can still call those. > > > > How about mapping the address of the devices? That should be done based on the iorange coming from DT during the driver probe. The range in hwmod can be populated at that point too if needed. > Some more thoughts on the similar line on unclaimed devices - > As par of late_initcall(), traverse all the DTB nodes and for each node with > "state = disabled", we can do something like, > > Late_initcall() > -> Traverse DTB nodes and check for "state = disabled" > -> Read the "ti,hwmod" entry and get offsets like, sys_config > and reset. > -> Map the base address > -> Enable the module/clock > -> Reset the device > -> Write to sysc register offset > -> Disable the module/clock > > > > > > Just to highlight, its not only during late_initcall, the _enable and _idle > > > functions are getting executed as part of every runtime_pm_get\put_sync from > > > the driver. Heh, but the unclaimed devices do not have a driver :) So there's no runtime_pm calls for the unclaimed devicdes. > At the current stage, I really think it would be really nice and beneficial > for driver developers from the community if they get Linux Boot from > linux-next/master branch. I have seen lot of community folks are struggling > with it and they are blocked because we do not have booting kernel on > BeagleBone. > They always end up patching kernel with HWMOD patch and submit the driver > patches. So request to consider this cleanup as sequential patch on top of > original HWMOD patch? Yes I'm fine merging the data as is, just trying to figure out how to sort out the issues for future. > > > May be get rid of resource overwriting for AM33xx and OMAP5 alone as of > > > now?? > > > > Sorry I don't follow this part, care to explain a little more? > > > > As per current implementation, we are using HWMOD information to fill the > platform_device->resources. Even if you pass "reg" and "interrupt" property > from DT, omap_device layer overwrites it with HWMOD data information. > > So what I am trying to propose here is, avoid this overwrite and resources > (address and interrupt) should strictly gets used from DT blob. Since AM33xx > and OMAP5 devices are the new devices and supports DT boot only, we can > implement it easily for them. Yes I agree. And then we should also drop the data from hwmod when it's no longer needed. > Only issue her is, DMA resources, I was going through some of the old email > archives today, submitted by Benoit and later Jon, but I am still not sure > how to pass dma_channel to driver as a resource. Hmm I thought that got merged already? Anyways, the iorange and interrupts are standard resources that should exist in every device entry in .dts files. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 17, 2012 at 15:28:51, Tony Lindgren wrote: > * Hiremath, Vaibhav <hvaibhav@ti.com> [120817 02:51]: > > On Fri, Aug 17, 2012 at 14:10:47, Tony Lindgren wrote: > > > * Hiremath, Vaibhav <hvaibhav@ti.com> [120817 01:22]: > > > > On Fri, Aug 17, 2012 at 13:19:34, Tony Lindgren wrote: > > > > > * Hiremath, Vaibhav <hvaibhav@ti.com> [120815 02:11]: > > > > > > > > > > > > Somehow we have to have some mechanism to initialize " _mpu_rt_va" before > > > > > > core_init call, which will take DT resources and initialize accordingly. > > > > > > > > > > That's for the device reset/idle? We should not do that in hwmod code > > > > > except in late_initcall for unclaimed devices as discussed earlier. We > > > > > discussed setting up the reset function in the device specific headers > > > > > so both device and hwmod code can call them as needed. > > > > > > > > May be I didn't understand your above statement, can you please elaborate > > > > more on "device specific header file"? > > > > > > We should have the device drivers idle and reset the devices. But also > > > hwmod may need to do that for the unclaimed devices for PM to work. > > > As the driver may not be even compiled in, the driver specific reset and > > > idle functions should be static inline functions in the device specific > > > header file so hwmod code can still call those. > > > > > > > How about mapping the address of the devices? > > That should be done based on the iorange coming from DT during the driver > probe. The range in hwmod can be populated at that point too if needed. > There will not be any driver probe for unclaimed devices, right? > > Some more thoughts on the similar line on unclaimed devices - > > As par of late_initcall(), traverse all the DTB nodes and for each node with > > "state = disabled", we can do something like, > > > > Late_initcall() > > -> Traverse DTB nodes and check for "state = disabled" > > -> Read the "ti,hwmod" entry and get offsets like, sys_config > > and reset. > > -> Map the base address > > -> Enable the module/clock > > -> Reset the device > > -> Write to sysc register offset > > -> Disable the module/clock > > > > > > > > > > Just to highlight, its not only during late_initcall, the _enable and _idle > > > > functions are getting executed as part of every runtime_pm_get\put_sync from > > > > the driver. > > Heh, but the unclaimed devices do not have a driver :) So there's no runtime_pm > calls for the unclaimed devicdes. > I think you mixed two responses here, this was not part of my last response. Just only read above code execution steps, I think they still make sense and we may end up doing something similar. > > At the current stage, I really think it would be really nice and beneficial > > for driver developers from the community if they get Linux Boot from > > linux-next/master branch. I have seen lot of community folks are struggling > > with it and they are blocked because we do not have booting kernel on > > BeagleBone. > > They always end up patching kernel with HWMOD patch and submit the driver > > patches. So request to consider this cleanup as sequential patch on top of > > original HWMOD patch? > > Yes I'm fine merging the data as is, just trying to figure out how to sort > out the issues for future. > Thanks a lot Tony. Paul, Care to add it in your next pull request. > > > > May be get rid of resource overwriting for AM33xx and OMAP5 alone as of > > > > now?? > > > > > > Sorry I don't follow this part, care to explain a little more? > > > > > > > As per current implementation, we are using HWMOD information to fill the > > platform_device->resources. Even if you pass "reg" and "interrupt" property > > from DT, omap_device layer overwrites it with HWMOD data information. > > > > So what I am trying to propose here is, avoid this overwrite and resources > > (address and interrupt) should strictly gets used from DT blob. Since AM33xx > > and OMAP5 devices are the new devices and supports DT boot only, we can > > implement it easily for them. > > Yes I agree. And then we should also drop the data from hwmod when it's no > longer needed. > Exactly, I will submit RFC for this. > > Only issue her is, DMA resources, I was going through some of the old email > > archives today, submitted by Benoit and later Jon, but I am still not sure > > how to pass dma_channel to driver as a resource. > > Hmm I thought that got merged already? No, atleast didn't find in 3.6-rc1. > Anyways, the iorange and interrupts > are standard resources that should exist in every device entry in .dts files. > Agreed, I am submitting patch for this as well. Thanks, Vaibhav -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guys, On 08/17/2012 11:58 AM, Tony Lindgren wrote: > * Hiremath, Vaibhav <hvaibhav@ti.com> [120817 02:51]: >> On Fri, Aug 17, 2012 at 14:10:47, Tony Lindgren wrote: >>> * Hiremath, Vaibhav <hvaibhav@ti.com> [120817 01:22]: >>>> On Fri, Aug 17, 2012 at 13:19:34, Tony Lindgren wrote: >>>>> * Hiremath, Vaibhav <hvaibhav@ti.com> [120815 02:11]: >>>>>> >>>>>> Somehow we have to have some mechanism to initialize " _mpu_rt_va" before >>>>>> core_init call, which will take DT resources and initialize accordingly. >>>>> >>>>> That's for the device reset/idle? We should not do that in hwmod code >>>>> except in late_initcall for unclaimed devices as discussed earlier. We >>>>> discussed setting up the reset function in the device specific headers >>>>> so both device and hwmod code can call them as needed. >>>> >>>> May be I didn't understand your above statement, can you please elaborate >>>> more on "device specific header file"? >>> >>> We should have the device drivers idle and reset the devices. But also >>> hwmod may need to do that for the unclaimed devices for PM to work. >>> As the driver may not be even compiled in, the driver specific reset and >>> idle functions should be static inline functions in the device specific >>> header file so hwmod code can still call those. >>> >> >> How about mapping the address of the devices? > > That should be done based on the iorange coming from DT during the driver > probe. The range in hwmod can be populated at that point too if needed. > >> Some more thoughts on the similar line on unclaimed devices - >> As par of late_initcall(), traverse all the DTB nodes and for each node with >> "state = disabled", we can do something like, >> >> Late_initcall() >> -> Traverse DTB nodes and check for "state = disabled" >> -> Read the "ti,hwmod" entry and get offsets like, sys_config >> and reset. >> -> Map the base address >> -> Enable the module/clock >> -> Reset the device >> -> Write to sysc register offset >> -> Disable the module/clock >> >> >> >>>> Just to highlight, its not only during late_initcall, the _enable and _idle >>>> functions are getting executed as part of every runtime_pm_get\put_sync from >>>> the driver. > > Heh, but the unclaimed devices do not have a driver :) So there's no runtime_pm > calls for the unclaimed devicdes. > >> At the current stage, I really think it would be really nice and beneficial >> for driver developers from the community if they get Linux Boot from >> linux-next/master branch. I have seen lot of community folks are struggling >> with it and they are blocked because we do not have booting kernel on >> BeagleBone. >> They always end up patching kernel with HWMOD patch and submit the driver >> patches. So request to consider this cleanup as sequential patch on top of >> original HWMOD patch? > > Yes I'm fine merging the data as is, just trying to figure out how to sort > out the issues for future. > >>>> May be get rid of resource overwriting for AM33xx and OMAP5 alone as of >>>> now?? >>> >>> Sorry I don't follow this part, care to explain a little more? >>> >> >> As per current implementation, we are using HWMOD information to fill the >> platform_device->resources. Even if you pass "reg" and "interrupt" property >> from DT, omap_device layer overwrites it with HWMOD data information. >> >> So what I am trying to propose here is, avoid this overwrite and resources >> (address and interrupt) should strictly gets used from DT blob. Since AM33xx >> and OMAP5 devices are the new devices and supports DT boot only, we can >> implement it easily for them. > > Yes I agree. And then we should also drop the data from hwmod when it's no > longer needed. > >> Only issue her is, DMA resources, I was going through some of the old email >> archives today, submitted by Benoit and later Jon, but I am still not sure >> how to pass dma_channel to driver as a resource. > > Hmm I thought that got merged already? No, it is still under discussion. I don't even know why they are still arguing, but there are still a lot of discussion on that thread. > Anyways, the iorange and interrupts > are standard resources that should exist in every device entry in .dts files. Yeah, I wanted to avoid that to make things simpler. As soon as we have the DMA we can switch easily to DT resource. My other concern is that so far we still have to duplicate the io space address with the way the hwmod is initializing the syscofig today. So it will require some change in that part to make an effective use of DT. So point is that if we start using that mechanism for DT boot, we will still have to keep the legacy one for non DT boot. Nothing is impossible, but that's still a lot of work before having something useful. The idea was to stop iterating the hwmods early and init the hwmod only during device probe or late_initcall for the one that will not probe. Again, easy with DT, less straighforward for non-DT boot. Regards, Benoit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Vaibhav, On 08/17/2012 11:51 AM, Hiremath, Vaibhav wrote: > On Fri, Aug 17, 2012 at 14:10:47, Tony Lindgren wrote: >> * Hiremath, Vaibhav <hvaibhav@ti.com> [120817 01:22]: >>> On Fri, Aug 17, 2012 at 13:19:34, Tony Lindgren wrote: >>>> * Hiremath, Vaibhav <hvaibhav@ti.com> [120815 02:11]: >>>>> >>>>> Somehow we have to have some mechanism to initialize " _mpu_rt_va" before >>>>> core_init call, which will take DT resources and initialize accordingly. >>>> >>>> That's for the device reset/idle? We should not do that in hwmod code >>>> except in late_initcall for unclaimed devices as discussed earlier. We >>>> discussed setting up the reset function in the device specific headers >>>> so both device and hwmod code can call them as needed. >>> >>> May be I didn't understand your above statement, can you please elaborate >>> more on "device specific header file"? >> >> We should have the device drivers idle and reset the devices. But also >> hwmod may need to do that for the unclaimed devices for PM to work. >> As the driver may not be even compiled in, the driver specific reset and >> idle functions should be static inline functions in the device specific >> header file so hwmod code can still call those. >> > > How about mapping the address of the devices? > > Some more thoughts on the similar line on unclaimed devices - > As par of late_initcall(), traverse all the DTB nodes and for each node with > "state = disabled", we can do something like, Yes, in fact we should duplicate the same mechanism that will be used to init a regular device during probe. > Late_initcall() > -> Traverse DTB nodes and check for "state = disabled" > -> Read the "ti,hwmod" entry and get offsets like, sys_config > and reset. > -> Map the base address > -> Enable the module/clock > -> Reset the device > -> Write to sysc register offset > -> Disable the module/clock > > > >>> Just to highlight, its not only during late_initcall, the _enable and _idle >>> functions are getting executed as part of every runtime_pm_get\put_sync from >>> the driver. >> >> Right, so no need to initialize those until at driver probe time. >> > > Agreed. > >>> Are you saying as part of late_initcall, we should initialize " _mpu_rt_va" >>> of "struct omap_hwmod"??? >> >> I guess either at driver probe for the claimed devices, or at late_initcall >> for the unclaimeded devices. >> >>> Also, as far unclaimed resources is concerned, somewhere you should have >>> base addr of the device maintained, right? Currently, hwmod is maintaining >>> this data and the execution goes something like, >>> >>> Core_initcall() >>> -> _setup() >>> -> _setup_reset() >>> -> _idle() >> >> For the DT only case also the unclaimed devices can from DT with >> status = "disabled". >> >>> Another biggest worry is, if I play here, it may break existing omap3/4 >>> Functionality, especially may impact from PM perspective. So I believe, we >>> need to have something which adds/cleans-up things in stages. >> >> It seems that we need three stages: Something early for the timers only, >> initialize most things during driver probe, then late_initcall for the >> unclaimed devices. >> > > Most likely yes, or it may even get into more stages. > > At the current stage, I really think it would be really nice and beneficial > for driver developers from the community if they get Linux Boot from > linux-next/master branch. I have seen lot of community folks are struggling > with it and they are blocked because we do not have booting kernel on > BeagleBone. > They always end up patching kernel with HWMOD patch and submit the driver > patches. So request to consider this cleanup as sequential patch on top of > original HWMOD patch? > > >>> May be get rid of resource overwriting for AM33xx and OMAP5 alone as of >>> now?? >> >> Sorry I don't follow this part, care to explain a little more? >> > > As per current implementation, we are using HWMOD information to fill the > platform_device->resources. Even if you pass "reg" and "interrupt" property > from DT, omap_device layer overwrites it with HWMOD data information. > > So what I am trying to propose here is, avoid this overwrite and resources > (address and interrupt) should strictly gets used from DT blob. Since AM33xx > and OMAP5 devices are the new devices and supports DT boot only, we can > implement it easily for them. Well, the point is that that mechanism is still common to OMAP2+ devices, so the tricky part is that a legacy mode will still be needed. > Only issue her is, DMA resources, I was going through some of the old email > archives today, submitted by Benoit and later Jon, but I am still not sure > how to pass dma_channel to driver as a resource. There is no way currently :-(. > Below logic works for me, I have tested it on both BeagleBone and AM37xEVM - > > omap_device_alloc () > { > ... > > res_count = omap_device_count_resources(od); > if (res_count > pdev->num_resources) { > res = kzalloc(sizeof(struct resource) * res_count, GFP_KERNEL); > if (!res) > goto oda_exit3; > > if (pdev->num_resources && pdev->resource) { > omap_device_fill_dma_resources(od, res); > memcpy(&res[pdev->num_resources], pdev->resource, > sizeof(struct resource) * pdev->num_resources); > } else { > omap_device_fill_resources(od, res); > } > > ret = platform_device_add_resources(pdev, res, res_count); > kfree(res); > } > > ... > } Yeah, or we can potentially define a custom ti DMA binding for the time being. In fact Tegra and Exynios are currently using some custom DMA binding. I wanted to avoid that with a generic binding RFC, but as already said, that series is not progressing really fast whereas our need for SDMA/EDMA is pretty simple. Well at least for SDMA. Anyway, your approach is the proper one to me and something we've been discussing for a while with Tony. I've just never had the time to progress on that. I'll be more than happy to support you on OMAP4+ devices to validate that code if you can work on it. Thanks, Benoit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c index c490240..cb481fe 100644 --- a/arch/arm/plat-omap/omap_device.c +++ b/arch/arm/plat-omap/omap_device.c @@ -528,10 +528,15 @@ struct omap_device *omap_device_alloc(struct platform_device *pdev, * HACK: Ideally the resources from DT should match, and hwmod * should just add the missing ones. Since the name is not * properly populated by DT, stick to hwmod resources only. + * + * But in case of all new devices, like AM33XX and OMAP5, where + * only DT boot is supported, stick to DT resources only. */ - if (pdev->num_resources && pdev->resource) + if (pdev->num_resources && pdev->resource) { dev_warn(&pdev->dev, "%s(): resources already allocated %d\n", __func__, pdev->num_resources); + goto skip_res_override; + } res_count = omap_device_count_resources(od); if (res_count > 0) { @@ -550,6 +555,7 @@ struct omap_device *omap_device_alloc(struct platform_device *pdev, goto oda_exit3; } +skip_res_override: if (!pm_lats) { pm_lats = omap_default_latency; pm_lats_cnt = ARRAY_SIZE(omap_default_latency);