Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
Paul, On 08/20/2013 06:39 PM, Paul Walmsley wrote: Hi a few comments On Wed, 14 Aug 2013, Suman Anna wrote: The remoteproc infrastructure is currently tied closely with the virtio/rpmsg framework, and the boot requires that there are virtio devices present in the resource table from the firmware image. Using static channels is something that can be added to the existing code, if you don't want to use dynamic channels. The resource table is used for both indicating the remoteproc resources as well as allowing the driver to publish the data back so that the remote processor can configure itself. The virtio devices do have other configuration fields and this allows the driver to update the status fields which the remote processor can read. This functionality is definitely lost if we use static virtio devices. Also, the remoteproc boot is a two-step process currently. It reads the virtio device information from the resource table and creates the virtio devices. The virtio device probe then triggers the remoteproc boot, which processes the firmware segments and handles the actual processor boot. The memory for firmware segments are allocated through CMA and the allocated addresses are published through the resource table to the remote processor-side. The core definitely needs to be patched up to support loading internal memory segments. The rpmsg shared memory buffers are currently kinda fixed (512 buffers of 512 bytes each) and requires 3 pages just for the vring structures for this many buffers. So, if there are restrictions on DDR access, then this pretty much rules out remoteproc/rpmsg infrastructure. It should be possible to patch that code to vary the size and count of the memory buffers, based on the remote processor. So no direct DDR access should be required - for that reason, anyway. Yep, I agree that this needs to be programmable. Support needs to be added to both the remoteproc and rpmsg cores to deal with internal memory for the vrings and vring buffers. Currently, it is all allocated dynamically through CMA. That said, the WkupM3 really has a very small memory regions, 16K of UMEM and 8K of DMEM. Everything has to fit within that including the vrings and vring buffers. I have to check if there are alignment restrictions imposed by the virtio code on the vrings and if that overshoots the available internal memory. If the DDR access is ok, then there are other challenges that needs to be met. The current firmware definitely requires the addition of the resource table and the lower level code for handling the virtio_ring transport for receiving messages. It would also need its own remoteproc driver for handling the firmware binary format Hmm, could you explain this further? Are you just referring to the process of parsing out the dynamic channel data during initialization? Yes, for parsing out where the resource table is. The remoteproc core provides the necessary hooks for finding the resource table and loading the firmware segments. If the firmware file is gonna remain a binary blob, then these hooks need to be implemented for that binary format. No issues if it were an ELF file, since we expect the resource table to be present in a special section. and the signalling required to trigger the rpmsg buffer processing. The firmware binary format needs to be adapted to something that this driver would understand. It definitely doesn't look like ELF currently, so something on the lines of ste_modem_rproc needs to be done. Or just use ELF or static channels. I have taken a look at the firmware tree, and the binary is generated from an ELF file, so the format is not an issue as long as the memory footprint is satisfied. static channels is more of an issue as pointed above. Also, the remoteproc/rpmsg infrastructure can support multiple vring transport channels between the processor, and depending on how many are supported, we either need to exchange the vq_id (like OMAP remoteproc), or process the known virtqueues always (like DA8xx remoteproc). The former requires that a message payload is used, and mandates the usage of the IPC data registers in the control module given that WkupM3 on AM335 cannot access any mailbox registers. Any usage of IPC data registers depends on where we do it. If all the accesses were to be done within mach-omap2/control.c, then there is no easy way for using this API from drivers/remoteproc, until we have the control module driver. Yep, real SCM drivers have been needed for some time now, for pretty much all of the OMAP SoCs. It should be pretty easy to prototype for your purposes, though. The current communication uses the IPC data registers, and sometimes uses them as plain status registers. There's certain registers used for sharing status, version etc which are shared by both the processors. Using rpmsg would require communicating every single message, and if there
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
Hi a few comments On Wed, 14 Aug 2013, Suman Anna wrote: The remoteproc infrastructure is currently tied closely with the virtio/rpmsg framework, and the boot requires that there are virtio devices present in the resource table from the firmware image. Using static channels is something that can be added to the existing code, if you don't want to use dynamic channels. The rpmsg shared memory buffers are currently kinda fixed (512 buffers of 512 bytes each) and requires 3 pages just for the vring structures for this many buffers. So, if there are restrictions on DDR access, then this pretty much rules out remoteproc/rpmsg infrastructure. It should be possible to patch that code to vary the size and count of the memory buffers, based on the remote processor. So no direct DDR access should be required - for that reason, anyway. If the DDR access is ok, then there are other challenges that needs to be met. The current firmware definitely requires the addition of the resource table and the lower level code for handling the virtio_ring transport for receiving messages. It would also need its own remoteproc driver for handling the firmware binary format Hmm, could you explain this further? Are you just referring to the process of parsing out the dynamic channel data during initialization? and the signalling required to trigger the rpmsg buffer processing. The firmware binary format needs to be adapted to something that this driver would understand. It definitely doesn't look like ELF currently, so something on the lines of ste_modem_rproc needs to be done. Or just use ELF or static channels. Also, the remoteproc/rpmsg infrastructure can support multiple vring transport channels between the processor, and depending on how many are supported, we either need to exchange the vq_id (like OMAP remoteproc), or process the known virtqueues always (like DA8xx remoteproc). The former requires that a message payload is used, and mandates the usage of the IPC data registers in the control module given that WkupM3 on AM335 cannot access any mailbox registers. Any usage of IPC data registers depends on where we do it. If all the accesses were to be done within mach-omap2/control.c, then there is no easy way for using this API from drivers/remoteproc, until we have the control module driver. Yep, real SCM drivers have been needed for some time now, for pretty much all of the OMAP SoCs. It should be pretty easy to prototype for your purposes, though. The current communication uses the IPC data registers, and sometimes uses them as plain status registers. There's certain registers used for sharing status, version etc which are shared by both the processors. Using rpmsg would require communicating every single message, and if there were to be some shared variables to be used simultaneously, then this has to be exchanged through a new remoteproc resource type. I don't quite understand this last part - shared variables to be used simultaneously. How does the existing code synchronize them? One additional aspect is that the current remoteproc core does not have the necessary runtime pm support, but in general the approach would be to treat the remoteprocs as true slave devices. I would imagine the driver core to put the remoteprocs into reset state, after asking them to save their context during suspend. Why is runtime PM support needed in the remoteproc core? Wouldn't that only be needed in the remote processor's device driver? - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
Kevin, Santosh, Dave, Russ, On 08/13/2013 02:11 PM, Kevin Hilman wrote: Russ Dill russ.d...@gmail.com writes: ARM world is also moving towards that by standardizing some of these through (read PSCI) and thats the way to go in general. Agreed, but I'm not sure (yet) about enforcing PSCI on legacy platforms that don't support it natively. Are you saying that the AM33xx firmware should be converted to be PSCI compliant? Admittedly, I haven't read the PSCI spec closely, but I'm wondering if the current role splitting between MPU and M3 fits well with PSCI. I didn't mean for the AM3XXX specifically because its current job is rather very limited. i.e suspend. My concern is that IPC is not viewed as an option for power management controllers like M3 which can abstract all the hardware gory details and export a simpled interface for OS in form of PSCI/ACPI. The IPC between the M3 and the A8 on the am335x is just a pair of notification mechanisms (one from the A8, mailbox, and one from the M3, sev) and a set of 8 32 bit registers. The 8 32 bit registers are just a scratchpad and have no access rules or functionality beyond being a scratchpad. How complicated do we want to make this? The OMAP IPC between the MPU and DSP (read: any other remote processor) is just a mailbox and some shared memory. No complicated, and works with remoteproc/rpmsg. I had to catch up on all the threads before I responded, but here are my notes regarding using remoteproc/rpmsg infrastructure for WkupM3. It definitely brings in lot more complexity than what's present today. The WkupM3 in AM335x does not have the same feature set as the Cortex-M3s used within the IPU in OMAP4+ SoCs. It does not have an MMU and peripheral accesses seems to be limited to just those in L4_Wkup domain. A lot actually depends on whether the WkupM3 can use DDR. I am not sure if there are any restrictions on WkupM3 from where it could run the code or access data from. From what I can gather from the TRM (there's no big separate chapter on WkupM3), it has just 16K UMEM and a 8K DMEM, and its not much. The current firmware is loaded directly into the WkupM3's internal UMem space and a check is being performed to make sure the firmware is less than 16K, so that it can fit into UMem. The remoteproc infrastructure is currently tied closely with the virtio/rpmsg framework, and the boot requires that there are virtio devices present in the resource table from the firmware image. The rpmsg shared memory buffers are currently kinda fixed (512 buffers of 512 bytes each) and requires 3 pages just for the vring structures for this many buffers. So, if there are restrictions on DDR access, then this pretty much rules out remoteproc/rpmsg infrastructure. If the DDR access is ok, then there are other challenges that needs to be met. The current firmware definitely requires the addition of the resource table and the lower level code for handling the virtio_ring transport for receiving messages. It would also need its own remoteproc driver for handling the firmware binary format and the signalling required to trigger the rpmsg buffer processing. The firmware binary format needs to be adapted to something that this driver would understand. It definitely doesn't look like ELF currently, so something on the lines of ste_modem_rproc needs to be done. Also, the remoteproc/rpmsg infrastructure can support multiple vring transport channels between the processor, and depending on how many are supported, we either need to exchange the vq_id (like OMAP remoteproc), or process the known virtqueues always (like DA8xx remoteproc). The former requires that a message payload is used, and mandates the usage of the IPC data registers in the control module given that WkupM3 on AM335 cannot access any mailbox registers. Any usage of IPC data registers depends on where we do it. If all the accesses were to be done within mach-omap2/control.c, then there is no easy way for using this API from drivers/remoteproc, until we have the control module driver. I do feel that this needs to be done within the wkup_m3 driver currently. This would be no different to the crossbar driver example on DRA7, which also has to deal with registers in Control module. The IPC data registers do not have any associated interrupts by itself, so just making this a standalone driver is also not possible. Also, the dependencies with using the omap_device API for dealing with the hard reset lines needs to be dealt with. The current communication uses the IPC data registers, and sometimes uses them as plain status registers. There's certain registers used for sharing status, version etc which are shared by both the processors. Using rpmsg would require communicating every single message, and if there were to be some shared variables to be used simultaneously, then this has to be exchanged through a new remoteproc resource type. One additional aspect is that the current remoteproc core does
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On Wed, Aug 14, 2013 at 10:27 AM, Suman Anna s-a...@ti.com wrote: Kevin, Santosh, Dave, Russ, On 08/13/2013 02:11 PM, Kevin Hilman wrote: Russ Dill russ.d...@gmail.com writes: ARM world is also moving towards that by standardizing some of these through (read PSCI) and thats the way to go in general. Agreed, but I'm not sure (yet) about enforcing PSCI on legacy platforms that don't support it natively. Are you saying that the AM33xx firmware should be converted to be PSCI compliant? Admittedly, I haven't read the PSCI spec closely, but I'm wondering if the current role splitting between MPU and M3 fits well with PSCI. I didn't mean for the AM3XXX specifically because its current job is rather very limited. i.e suspend. My concern is that IPC is not viewed as an option for power management controllers like M3 which can abstract all the hardware gory details and export a simpled interface for OS in form of PSCI/ACPI. The IPC between the M3 and the A8 on the am335x is just a pair of notification mechanisms (one from the A8, mailbox, and one from the M3, sev) and a set of 8 32 bit registers. The 8 32 bit registers are just a scratchpad and have no access rules or functionality beyond being a scratchpad. How complicated do we want to make this? The OMAP IPC between the MPU and DSP (read: any other remote processor) is just a mailbox and some shared memory. No complicated, and works with remoteproc/rpmsg. I had to catch up on all the threads before I responded, but here are my notes regarding using remoteproc/rpmsg infrastructure for WkupM3. It definitely brings in lot more complexity than what's present today. The WkupM3 in AM335x does not have the same feature set as the Cortex-M3s used within the IPU in OMAP4+ SoCs. It does not have an MMU and peripheral accesses seems to be limited to just those in L4_Wkup domain. A lot actually depends on whether the WkupM3 can use DDR. I am not sure if there are any restrictions on WkupM3 from where it could run the code or access data from. From what I can gather from the TRM (there's no big separate chapter on WkupM3), it has just 16K UMEM and a 8K DMEM, and its not much. The current firmware is loaded directly into the WkupM3's internal UMem space and a check is being performed to make sure the firmware is less than 16K, so that it can fit into UMem. Even if the WkupM3 could access DMEM, it would defeat the whole purpose of using it, as it lives to do things like enable VTP, but the EMIF clocks into bypass, turn off the power domain the EMIF is contained in, etc. The remoteproc infrastructure is currently tied closely with the virtio/rpmsg framework, and the boot requires that there are virtio devices present in the resource table from the firmware image. The rpmsg shared memory buffers are currently kinda fixed (512 buffers of 512 bytes each) and requires 3 pages just for the vring structures for this many buffers. So, if there are restrictions on DDR access, then this pretty much rules out remoteproc/rpmsg infrastructure. If the DDR access is ok, then there are other challenges that needs to be met. The current firmware definitely requires the addition of the resource table and the lower level code for handling the virtio_ring transport for receiving messages. It would also need its own remoteproc driver for handling the firmware binary format and the signalling required to trigger the rpmsg buffer processing. The firmware binary format needs to be adapted to something that this driver would understand. It definitely doesn't look like ELF currently, so something on the lines of ste_modem_rproc needs to be done. Also, the remoteproc/rpmsg infrastructure can support multiple vring transport channels between the processor, and depending on how many are supported, we either need to exchange the vq_id (like OMAP remoteproc), or process the known virtqueues always (like DA8xx remoteproc). The former requires that a message payload is used, and mandates the usage of the IPC data registers in the control module given that WkupM3 on AM335 cannot access any mailbox registers. Any usage of IPC data registers depends on where we do it. If all the accesses were to be done within mach-omap2/control.c, then there is no easy way for using this API from drivers/remoteproc, until we have the control module driver. I do feel that this needs to be done within the wkup_m3 driver currently. This would be no different to the crossbar driver example on DRA7, which also has to deal with registers in Control module. The IPC data registers do not have any associated interrupts by itself, so just making this a standalone driver is also not possible. Also, the dependencies with using the omap_device API for dealing with the hard reset lines needs to be dealt with. The current communication uses the IPC data registers, and sometimes uses them as plain status registers. There's certain registers used for sharing
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
Dave Gerlach d-gerl...@ti.com writes: On 08/12/2013 02:17 PM, Kevin Hilman wrote: Dave Gerlach d-gerl...@ti.com writes: On 08/09/2013 12:11 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [130808 09:23]: On 08/08/2013 08:44 AM, Santosh Shilimkar wrote: Lets address the above better. I don't see a need of 8 functions exported doing one or 2 register writes. Look M3 based handling is going to be there on future SOCs as well and this kind of handling of IPC is very short cited. The idea here was to move all control module register accesses into one file in planning of implementing a driver for the control module itself in the future. Probably we should have a separate driver for M3 in linux which can have all this local code instead of all these exports. The wkup_m3 code has been moved to a small driver found in patch 8 of this series, would it better to move this code there rather than with the rest of the control module code? Please make everything you can into regular device drivers. We still have some dependencies to mach-omap2 code for PRCM for example, but we're trying to get all that to live in drivers. So for new pieces, let's not add further dependencies to complicate moving things to drivers. Regards, Tony Ok I will go ahead and pull the control module code that handles IPC into the wkup_m3 driver. Any control module register access still needs to stay in control.c. The wkup_m3.c file is still present in mach-omap2 as the right location for it wasn't decided in the last RFC. Any thoughts on a good location for it? I raised this also in earlier reviews, but don't remember the if it was answered... Why can't we handle the wkup_m3 using remoteproc/rpmsg instead of creating another little driver that duplicates communication with the M3. Note also that the firmware load part would also be provided by remoteproc/rpmsg. Looping Suman Anna who handled the IPC patches this patch set is based on top of... For the wkup_m3, the mailbox isn't used in a traditional manner. It's only used with a dummy write to trigger an interrupt from the A8 to the M3 and then communication happens in IPC registers within the control module. No messages are actually sent through the mailbox in either direction so that's why it was done this way rather than bring in full support for the mailbox. I don't believe remoteproc/rpmsg forces you to use the mailbox for communication, and can use other IPC mechanisms. This still sounds cleaner than reinventing remoteproc/rpmsg because of slight variations. The linux way is to use and extend what is already there, and if it's extended to the breaking point, then make a case for why something new is needed. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On Tuesday 13 August 2013 10:29 AM, Kevin Hilman wrote: Dave Gerlach d-gerl...@ti.com writes: On 08/12/2013 02:17 PM, Kevin Hilman wrote: Dave Gerlach d-gerl...@ti.com writes: On 08/09/2013 12:11 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [130808 09:23]: On 08/08/2013 08:44 AM, Santosh Shilimkar wrote: Lets address the above better. I don't see a need of 8 functions exported doing one or 2 register writes. Look M3 based handling is going to be there on future SOCs as well and this kind of handling of IPC is very short cited. The idea here was to move all control module register accesses into one file in planning of implementing a driver for the control module itself in the future. Probably we should have a separate driver for M3 in linux which can have all this local code instead of all these exports. The wkup_m3 code has been moved to a small driver found in patch 8 of this series, would it better to move this code there rather than with the rest of the control module code? Please make everything you can into regular device drivers. We still have some dependencies to mach-omap2 code for PRCM for example, but we're trying to get all that to live in drivers. So for new pieces, let's not add further dependencies to complicate moving things to drivers. Regards, Tony Ok I will go ahead and pull the control module code that handles IPC into the wkup_m3 driver. Any control module register access still needs to stay in control.c. The wkup_m3.c file is still present in mach-omap2 as the right location for it wasn't decided in the last RFC. Any thoughts on a good location for it? I raised this also in earlier reviews, but don't remember the if it was answered... Why can't we handle the wkup_m3 using remoteproc/rpmsg instead of creating another little driver that duplicates communication with the M3. Note also that the firmware load part would also be provided by remoteproc/rpmsg. Looping Suman Anna who handled the IPC patches this patch set is based on top of... For the wkup_m3, the mailbox isn't used in a traditional manner. It's only used with a dummy write to trigger an interrupt from the A8 to the M3 and then communication happens in IPC registers within the control module. No messages are actually sent through the mailbox in either direction so that's why it was done this way rather than bring in full support for the mailbox. I don't believe remoteproc/rpmsg forces you to use the mailbox for communication, and can use other IPC mechanisms. This still sounds cleaner than reinventing remoteproc/rpmsg because of slight variations. The linux way is to use and extend what is already there, and if it's extended to the breaking point, then make a case for why something new is needed. While I agree to re-use frameworks, am strongly against use of IPC for the power management which is time sensitive. Imagine a scenario where ACPI is asked to go through remoteproc/ipc for talking to firmware. If it takes 10 to 15 uS just to send a command to firmware to change some control, that just tells something is not right and thats what IPC will do. ARM world is also moving towards that by standardizing some of these through (read PSCI) and thats the way to go in general. Specifically for this series, I am also against having tons of exports and all of that should be extracted properly but remoteproc is not going to be the way. Firmware download has to happen much earlier(ROM path) so thats not the requirement where we could have used the remoteproc firmware download feature. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
+ Ohad Santosh Shilimkar santosh.shilim...@ti.com writes: On Tuesday 13 August 2013 10:29 AM, Kevin Hilman wrote: Dave Gerlach d-gerl...@ti.com writes: On 08/12/2013 02:17 PM, Kevin Hilman wrote: Dave Gerlach d-gerl...@ti.com writes: On 08/09/2013 12:11 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [130808 09:23]: On 08/08/2013 08:44 AM, Santosh Shilimkar wrote: Lets address the above better. I don't see a need of 8 functions exported doing one or 2 register writes. Look M3 based handling is going to be there on future SOCs as well and this kind of handling of IPC is very short cited. The idea here was to move all control module register accesses into one file in planning of implementing a driver for the control module itself in the future. Probably we should have a separate driver for M3 in linux which can have all this local code instead of all these exports. The wkup_m3 code has been moved to a small driver found in patch 8 of this series, would it better to move this code there rather than with the rest of the control module code? Please make everything you can into regular device drivers. We still have some dependencies to mach-omap2 code for PRCM for example, but we're trying to get all that to live in drivers. So for new pieces, let's not add further dependencies to complicate moving things to drivers. Regards, Tony Ok I will go ahead and pull the control module code that handles IPC into the wkup_m3 driver. Any control module register access still needs to stay in control.c. The wkup_m3.c file is still present in mach-omap2 as the right location for it wasn't decided in the last RFC. Any thoughts on a good location for it? I raised this also in earlier reviews, but don't remember the if it was answered... Why can't we handle the wkup_m3 using remoteproc/rpmsg instead of creating another little driver that duplicates communication with the M3. Note also that the firmware load part would also be provided by remoteproc/rpmsg. Looping Suman Anna who handled the IPC patches this patch set is based on top of... For the wkup_m3, the mailbox isn't used in a traditional manner. It's only used with a dummy write to trigger an interrupt from the A8 to the M3 and then communication happens in IPC registers within the control module. No messages are actually sent through the mailbox in either direction so that's why it was done this way rather than bring in full support for the mailbox. I don't believe remoteproc/rpmsg forces you to use the mailbox for communication, and can use other IPC mechanisms. This still sounds cleaner than reinventing remoteproc/rpmsg because of slight variations. The linux way is to use and extend what is already there, and if it's extended to the breaking point, then make a case for why something new is needed. While I agree to re-use frameworks, am strongly against use of IPC for the power management which is time sensitive. Maybe I'm misunderstanding what you mean by IPC, but I'm not sure how you can be against IPC here. Communication with M3 requires it. The only question is what framework to use: create a new wkup_m3 driver which creates yet another set of IPC APIs as well as a seprate firmware load path? Or reuse remoteproc/rpmsg which provides both. Imagine a scenario where ACPI is asked to go through remoteproc/ipc for talking to firmware. If it takes 10 to 15 uS just to send a command to firmware to change some control, that just tells something is not right and thats what IPC will do. Sounds like what you're saying is that remoteproc/rpmsg will be too slow for this? Is there any evidence of that? Even if it is slow, it sounds like a perfect opportuntity to enhance it. Also, if speed is that much of a concern concern here, I wonder why there's a 500msec timeout in in the communication between MPU and M3 in the current code. ARM world is also moving towards that by standardizing some of these through (read PSCI) and thats the way to go in general. Agreed, but I'm not sure (yet) about enforcing PSCI on legacy platforms that don't support it natively. Are you saying that the AM33xx firmware should be converted to be PSCI compliant? Admittedly, I haven't read the PSCI spec closely, but I'm wondering if the current role splitting between MPU and M3 fits well with PSCI. Specifically for this series, I am also against having tons of exports and all of that should be extracted properly but remoteproc is not going to be the way. Firmware download has to happen much earlier(ROM path) so thats not the requirement where we could have used the remoteproc firmware download feature. At least for AM33xx, I don't see any requirement that firmware download happens early (or at all.) PM will simply be disabled until firmware is available (and running.) Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On Tuesday 13 August 2013 12:19 PM, Kevin Hilman wrote: + Ohad Santosh Shilimkar santosh.shilim...@ti.com writes: On Tuesday 13 August 2013 10:29 AM, Kevin Hilman wrote: Dave Gerlach d-gerl...@ti.com writes: On 08/12/2013 02:17 PM, Kevin Hilman wrote: Dave Gerlach d-gerl...@ti.com writes: On 08/09/2013 12:11 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [130808 09:23]: On 08/08/2013 08:44 AM, Santosh Shilimkar wrote: Lets address the above better. I don't see a need of 8 functions exported doing one or 2 register writes. Look M3 based handling is going to be there on future SOCs as well and this kind of handling of IPC is very short cited. The idea here was to move all control module register accesses into one file in planning of implementing a driver for the control module itself in the future. Probably we should have a separate driver for M3 in linux which can have all this local code instead of all these exports. The wkup_m3 code has been moved to a small driver found in patch 8 of this series, would it better to move this code there rather than with the rest of the control module code? Please make everything you can into regular device drivers. We still have some dependencies to mach-omap2 code for PRCM for example, but we're trying to get all that to live in drivers. So for new pieces, let's not add further dependencies to complicate moving things to drivers. Regards, Tony Ok I will go ahead and pull the control module code that handles IPC into the wkup_m3 driver. Any control module register access still needs to stay in control.c. The wkup_m3.c file is still present in mach-omap2 as the right location for it wasn't decided in the last RFC. Any thoughts on a good location for it? I raised this also in earlier reviews, but don't remember the if it was answered... Why can't we handle the wkup_m3 using remoteproc/rpmsg instead of creating another little driver that duplicates communication with the M3. Note also that the firmware load part would also be provided by remoteproc/rpmsg. Looping Suman Anna who handled the IPC patches this patch set is based on top of... For the wkup_m3, the mailbox isn't used in a traditional manner. It's only used with a dummy write to trigger an interrupt from the A8 to the M3 and then communication happens in IPC registers within the control module. No messages are actually sent through the mailbox in either direction so that's why it was done this way rather than bring in full support for the mailbox. I don't believe remoteproc/rpmsg forces you to use the mailbox for communication, and can use other IPC mechanisms. This still sounds cleaner than reinventing remoteproc/rpmsg because of slight variations. The linux way is to use and extend what is already there, and if it's extended to the breaking point, then make a case for why something new is needed. While I agree to re-use frameworks, am strongly against use of IPC for the power management which is time sensitive. Maybe I'm misunderstanding what you mean by IPC, but I'm not sure how you can be against IPC here. Communication with M3 requires it. The only question is what framework to use: create a new wkup_m3 driver which creates yet another set of IPC APIs as well as a seprate firmware load path? Or reuse remoteproc/rpmsg which provides both. I mean Linux IPC layers. Imagine a scenario where ACPI is asked to go through remoteproc/ipc for talking to firmware. If it takes 10 to 15 uS just to send a command to firmware to change some control, that just tells something is not right and thats what IPC will do. Sounds like what you're saying is that remoteproc/rpmsg will be too slow for this? Is there any evidence of that? Even if it is slow, it sounds like a perfect opportuntity to enhance it. Well 10's uS is fine for the pure IPC kind of messaging. The numbers were measured on Linux OMAP IPC stack for different purpose. Also, if speed is that much of a concern concern here, I wonder why there's a 500msec timeout in in the communication between MPU and M3 in the current code. ARM world is also moving towards that by standardizing some of these through (read PSCI) and thats the way to go in general. Agreed, but I'm not sure (yet) about enforcing PSCI on legacy platforms that don't support it natively. Are you saying that the AM33xx firmware should be converted to be PSCI compliant? Admittedly, I haven't read the PSCI spec closely, but I'm wondering if the current role splitting between MPU and M3 fits well with PSCI. I didn't mean for the AM3XXX specifically because its current job is rather very limited. i.e suspend. My concern is that IPC is not viewed as an option for power management controllers like M3 which can abstract all the hardware gory details and export a simpled interface for OS in form of PSCI/ACPI. Specifically for this series, I am also against having tons of
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
ARM world is also moving towards that by standardizing some of these through (read PSCI) and thats the way to go in general. Agreed, but I'm not sure (yet) about enforcing PSCI on legacy platforms that don't support it natively. Are you saying that the AM33xx firmware should be converted to be PSCI compliant? Admittedly, I haven't read the PSCI spec closely, but I'm wondering if the current role splitting between MPU and M3 fits well with PSCI. I didn't mean for the AM3XXX specifically because its current job is rather very limited. i.e suspend. My concern is that IPC is not viewed as an option for power management controllers like M3 which can abstract all the hardware gory details and export a simpled interface for OS in form of PSCI/ACPI. The IPC between the M3 and the A8 on the am335x is just a pair of notification mechanisms (one from the A8, mailbox, and one from the M3, sev) and a set of 8 32 bit registers. The 8 32 bit registers are just a scratchpad and have no access rules or functionality beyond being a scratchpad. How complicated do we want to make this? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On Tuesday 13 August 2013 02:30 PM, Russ Dill wrote: ARM world is also moving towards that by standardizing some of these through (read PSCI) and thats the way to go in general. Agreed, but I'm not sure (yet) about enforcing PSCI on legacy platforms that don't support it natively. Are you saying that the AM33xx firmware should be converted to be PSCI compliant? Admittedly, I haven't read the PSCI spec closely, but I'm wondering if the current role splitting between MPU and M3 fits well with PSCI. I didn't mean for the AM3XXX specifically because its current job is rather very limited. i.e suspend. My concern is that IPC is not viewed as an option for power management controllers like M3 which can abstract all the hardware gory details and export a simpled interface for OS in form of PSCI/ACPI. The IPC between the M3 and the A8 on the am335x is just a pair of notification mechanisms (one from the A8, mailbox, and one from the M3, sev) and a set of 8 32 bit registers. The 8 32 bit registers are just a scratchpad and have no access rules or functionality beyond being a scratchpad. How complicated do we want to make this? Exactly what I mean as well. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
Russ Dill russ.d...@gmail.com writes: ARM world is also moving towards that by standardizing some of these through (read PSCI) and thats the way to go in general. Agreed, but I'm not sure (yet) about enforcing PSCI on legacy platforms that don't support it natively. Are you saying that the AM33xx firmware should be converted to be PSCI compliant? Admittedly, I haven't read the PSCI spec closely, but I'm wondering if the current role splitting between MPU and M3 fits well with PSCI. I didn't mean for the AM3XXX specifically because its current job is rather very limited. i.e suspend. My concern is that IPC is not viewed as an option for power management controllers like M3 which can abstract all the hardware gory details and export a simpled interface for OS in form of PSCI/ACPI. The IPC between the M3 and the A8 on the am335x is just a pair of notification mechanisms (one from the A8, mailbox, and one from the M3, sev) and a set of 8 32 bit registers. The 8 32 bit registers are just a scratchpad and have no access rules or functionality beyond being a scratchpad. How complicated do we want to make this? The OMAP IPC between the MPU and DSP (read: any other remote processor) is just a mailbox and some shared memory. No complicated, and works with remoteproc/rpmsg. What's more complicated? reusing existing frameworks or re-inventing them? Don't forget that simple drivers rarely stay that way. Also, this is not a hard stance on my part. I just need to be convinced. The fact is that there are existing frameworks for doing what is being proposed here. Either use the existing frameworks, or make a technical argument based on why those frameworks are not a good fit (ideally, after having tried to use those frameworks.) Once again, I'm pretty sure I mentioned this in earlier reviews of this series. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
* Dave Gerlach d-gerl...@ti.com [130809 14:02]: Ok I will go ahead and pull the control module code that handles IPC into the wkup_m3 driver. The wkup_m3.c file is still present in mach-omap2 as the right location for it wasn't decided in the last RFC. Any thoughts on a good location for it? Well maybe try to think how to use these features in a Linux generic way without having to export tons of custom functions? Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
Dave Gerlach d-gerl...@ti.com writes: On 08/09/2013 12:11 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [130808 09:23]: On 08/08/2013 08:44 AM, Santosh Shilimkar wrote: Lets address the above better. I don't see a need of 8 functions exported doing one or 2 register writes. Look M3 based handling is going to be there on future SOCs as well and this kind of handling of IPC is very short cited. The idea here was to move all control module register accesses into one file in planning of implementing a driver for the control module itself in the future. Probably we should have a separate driver for M3 in linux which can have all this local code instead of all these exports. The wkup_m3 code has been moved to a small driver found in patch 8 of this series, would it better to move this code there rather than with the rest of the control module code? Please make everything you can into regular device drivers. We still have some dependencies to mach-omap2 code for PRCM for example, but we're trying to get all that to live in drivers. So for new pieces, let's not add further dependencies to complicate moving things to drivers. Regards, Tony Ok I will go ahead and pull the control module code that handles IPC into the wkup_m3 driver. Any control module register access still needs to stay in control.c. The wkup_m3.c file is still present in mach-omap2 as the right location for it wasn't decided in the last RFC. Any thoughts on a good location for it? I raised this also in earlier reviews, but don't remember the if it was answered... Why can't we handle the wkup_m3 using remoteproc/rpmsg instead of creating another little driver that duplicates communication with the M3. Note also that the firmware load part would also be provided by remoteproc/rpmsg. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On 08/12/2013 02:17 PM, Kevin Hilman wrote: Dave Gerlach d-gerl...@ti.com writes: On 08/09/2013 12:11 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [130808 09:23]: On 08/08/2013 08:44 AM, Santosh Shilimkar wrote: Lets address the above better. I don't see a need of 8 functions exported doing one or 2 register writes. Look M3 based handling is going to be there on future SOCs as well and this kind of handling of IPC is very short cited. The idea here was to move all control module register accesses into one file in planning of implementing a driver for the control module itself in the future. Probably we should have a separate driver for M3 in linux which can have all this local code instead of all these exports. The wkup_m3 code has been moved to a small driver found in patch 8 of this series, would it better to move this code there rather than with the rest of the control module code? Please make everything you can into regular device drivers. We still have some dependencies to mach-omap2 code for PRCM for example, but we're trying to get all that to live in drivers. So for new pieces, let's not add further dependencies to complicate moving things to drivers. Regards, Tony Ok I will go ahead and pull the control module code that handles IPC into the wkup_m3 driver. Any control module register access still needs to stay in control.c. The wkup_m3.c file is still present in mach-omap2 as the right location for it wasn't decided in the last RFC. Any thoughts on a good location for it? I raised this also in earlier reviews, but don't remember the if it was answered... Why can't we handle the wkup_m3 using remoteproc/rpmsg instead of creating another little driver that duplicates communication with the M3. Note also that the firmware load part would also be provided by remoteproc/rpmsg. Looping Suman Anna who handled the IPC patches this patch set is based on top of... For the wkup_m3, the mailbox isn't used in a traditional manner. It's only used with a dummy write to trigger an interrupt from the A8 to the M3 and then communication happens in IPC registers within the control module. No messages are actually sent through the mailbox in either direction so that's why it was done this way rather than bring in full support for the mailbox. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On 08/09/2013 12:11 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [130808 09:23]: On 08/08/2013 08:44 AM, Santosh Shilimkar wrote: Lets address the above better. I don't see a need of 8 functions exported doing one or 2 register writes. Look M3 based handling is going to be there on future SOCs as well and this kind of handling of IPC is very short cited. The idea here was to move all control module register accesses into one file in planning of implementing a driver for the control module itself in the future. Probably we should have a separate driver for M3 in linux which can have all this local code instead of all these exports. The wkup_m3 code has been moved to a small driver found in patch 8 of this series, would it better to move this code there rather than with the rest of the control module code? Please make everything you can into regular device drivers. We still have some dependencies to mach-omap2 code for PRCM for example, but we're trying to get all that to live in drivers. So for new pieces, let's not add further dependencies to complicate moving things to drivers. Regards, Tony Ok I will go ahead and pull the control module code that handles IPC into the wkup_m3 driver. The wkup_m3.c file is still present in mach-omap2 as the right location for it wasn't decided in the last RFC. Any thoughts on a good location for it? Regards, Dave -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On Tuesday 06 August 2013 01:49 PM, Dave Gerlach wrote:
From: Vaibhav Bedia vaibhav.be...@ti.com
Interacting with WKUP-M3 requires some more control
module register writes. Add the register offsets and
APIs to write to these.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Signed-off-by: Dave Gerlach d-gerl...@ti.com
---
arch/arm/mach-omap2/control.c | 57
+
arch/arm/mach-omap2/control.h | 54 ++
2 files changed, 111 insertions(+)
diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 31e0dfe..934041a 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -605,3 +605,60 @@ int omap3_ctrl_save_padconf(void)
}
#endif /* CONFIG_ARCH_OMAP3 CONFIG_PM */
+
+#if defined(CONFIG_SOC_AM33XX) defined(CONFIG_PM)
+void am33xx_txev_eoi(void)
+{
+ omap_ctrl_writel(AM33XX_M3_TXEV_ACK, AM33XX_CONTROL_M3_TXEV_EOI);
+}
+
+void am33xx_txev_enable(void)
+{
+ omap_ctrl_writel(AM33XX_M3_TXEV_ENABLE, AM33XX_CONTROL_M3_TXEV_EOI);
+}
+
+/*
+ * Invalidate M3 firmware version before hardreset.
+ * Write invalid version in lower 4 nibbles of parameter
+ * register (ipc_regs + 0x8).
+ */
+void am33xx_pm_version_clear(void)
+{
+ omap_ctrl_writel(0x, AM33XX_CONTROL_IPC_MSG_REG2);
+}
+
+int am33xx_pm_version_get(void)
+{
+ return omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG2) M3_VERSION_MASK;
+}
+
+void am33xx_pm_ipc_cmd(struct am33xx_ipc_data *data)
+{
+ omap_ctrl_writel(data-resume_addr, AM33XX_CONTROL_IPC_MSG_REG0);
+ omap_ctrl_writel(data-sleep_mode, AM33XX_CONTROL_IPC_MSG_REG1);
+ omap_ctrl_writel(data-param1, AM33XX_CONTROL_IPC_MSG_REG2);
+ omap_ctrl_writel(data-param2, AM33XX_CONTROL_IPC_MSG_REG3);
+ omap_ctrl_writel(data-param3, AM33XX_CONTROL_IPC_MSG_REG4);
+}
+
+int am33xx_pm_status(void)
+{
+ int i;
+
+ i = omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG1);
+ i = IPC_RESP_MASK;
+ i = __ffs(IPC_RESP_MASK);
+
+ return i;
+}
+
+int am33xx_pm_wake_src(void)
+{
+ int i;
+
+ i = omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG6);
+ i = 0xff;
+
+ return i;
+}
+#endif /* CONFIG_SOC_AM33XX CONFIG_PM */
diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
index f7d7c2e..9be587c 100644
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -370,6 +370,22 @@
#define AM33XX_DEV_FEATURE 0x604
#define AM33XX_SGX_MASK BIT(29)
+/* AM33XX M3_TXEV_EOI register */
+#define AM33XX_CONTROL_M3_TXEV_EOI 0x1324
+
+#define AM33XX_M3_TXEV_ACK (0x1 0)
+#define AM33XX_M3_TXEV_ENABLE(0x0 0)
+
+/* AM33XX IPC message registers */
+#define AM33XX_CONTROL_IPC_MSG_REG0 0x1328
+#define AM33XX_CONTROL_IPC_MSG_REG1 0x132C
+#define AM33XX_CONTROL_IPC_MSG_REG2 0x1330
+#define AM33XX_CONTROL_IPC_MSG_REG3 0x1334
+#define AM33XX_CONTROL_IPC_MSG_REG4 0x1338
+#define AM33XX_CONTROL_IPC_MSG_REG5 0x133C
+#define AM33XX_CONTROL_IPC_MSG_REG6 0x1340
+#define AM33XX_CONTROL_IPC_MSG_REG7 0x1344
+
/* CONTROL OMAP STATUS register to identify OMAP3 features */
#define OMAP3_CONTROL_OMAP_STATUS0x044c
@@ -429,6 +445,44 @@ extern void omap3630_ctrl_disable_rta(void);
extern int omap3_ctrl_save_padconf(void);
extern void omap2_set_globals_control(void __iomem *ctrl,
void __iomem *ctrl_pad);
+struct am33xx_ipc_data {
+ u32 resume_addr;
+ u32 sleep_mode;
+ u32 param1;
+ u32 param2;
+ u32 param3;
+ u32 param4;
+ u32 param5;
+ u32 param6;
+};
+
+#define IPC_RESP_SHIFT 16
+#define IPC_RESP_MASK(0x 16)
+
+#define M3_VERSION_SHIFT 0
+#define M3_VERSION_MASK (0x 0)
+
+/*
+ * 9-4 = VTT GPIO PIN (6 Bits)
+ * 3 = VTT Status (1 Bit)
+ * 2-0 = Memory Type (2 Bits)
+*/
+#define MEM_TYPE_SHIFT (0x0)
+#define MEM_TYPE_MASK(0x7 0)
+#define VTT_STAT_SHIFT (0x3)
+#define VTT_STAT_MASK(0x1 3)
+#define VTT_GPIO_PIN_SHIFT (0x4)
+#define VTT_GPIO_PIN_MASK(0x2f 4)
+
+extern void am33xx_wkup_m3_ipc_cmd(struct am33xx_ipc_data *data);
+extern void am33xx_txev_eoi(void);
+extern void am33xx_txev_enable(void);
+extern void am33xx_pm_version_clear(void);
+extern int am33xx_pm_version_get(void);
+extern void am33xx_pm_ipc_cmd(struct am33xx_ipc_data *data);
+extern int am33xx_pm_status(void);
+extern int am33xx_pm_wake_src(void);
+
Lets address the above better. I don't see a need of 8 functions
exported doing one or 2 register writes.
Look M3 based handling is going to be there on future SOCs
as well and this kind of handling of IPC is very short cited.
Probably we should have a separate driver for M3 in linux
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On 08/08/2013 08:44 AM, Santosh Shilimkar wrote:
On Tuesday 06 August 2013 01:49 PM, Dave Gerlach wrote:
From: Vaibhav Bedia vaibhav.be...@ti.com
Interacting with WKUP-M3 requires some more control
module register writes. Add the register offsets and
APIs to write to these.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Signed-off-by: Dave Gerlach d-gerl...@ti.com
---
arch/arm/mach-omap2/control.c | 57 +
arch/arm/mach-omap2/control.h | 54 ++
2 files changed, 111 insertions(+)
diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 31e0dfe..934041a 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -605,3 +605,60 @@ int omap3_ctrl_save_padconf(void)
}
#endif /* CONFIG_ARCH_OMAP3 CONFIG_PM */
+
+#if defined(CONFIG_SOC_AM33XX) defined(CONFIG_PM)
+void am33xx_txev_eoi(void)
+{
+ omap_ctrl_writel(AM33XX_M3_TXEV_ACK, AM33XX_CONTROL_M3_TXEV_EOI);
+}
+
+void am33xx_txev_enable(void)
+{
+ omap_ctrl_writel(AM33XX_M3_TXEV_ENABLE, AM33XX_CONTROL_M3_TXEV_EOI);
+}
+
+/*
+ * Invalidate M3 firmware version before hardreset.
+ * Write invalid version in lower 4 nibbles of parameter
+ * register (ipc_regs + 0x8).
+ */
+void am33xx_pm_version_clear(void)
+{
+ omap_ctrl_writel(0x, AM33XX_CONTROL_IPC_MSG_REG2);
+}
+
+int am33xx_pm_version_get(void)
+{
+ return omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG2) M3_VERSION_MASK;
+}
+
+void am33xx_pm_ipc_cmd(struct am33xx_ipc_data *data)
+{
+ omap_ctrl_writel(data-resume_addr, AM33XX_CONTROL_IPC_MSG_REG0);
+ omap_ctrl_writel(data-sleep_mode, AM33XX_CONTROL_IPC_MSG_REG1);
+ omap_ctrl_writel(data-param1, AM33XX_CONTROL_IPC_MSG_REG2);
+ omap_ctrl_writel(data-param2, AM33XX_CONTROL_IPC_MSG_REG3);
+ omap_ctrl_writel(data-param3, AM33XX_CONTROL_IPC_MSG_REG4);
+}
+
+int am33xx_pm_status(void)
+{
+ int i;
+
+ i = omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG1);
+ i = IPC_RESP_MASK;
+ i = __ffs(IPC_RESP_MASK);
+
+ return i;
+}
+
+int am33xx_pm_wake_src(void)
+{
+ int i;
+
+ i = omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG6);
+ i = 0xff;
+
+ return i;
+}
+#endif /* CONFIG_SOC_AM33XX CONFIG_PM */
diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
index f7d7c2e..9be587c 100644
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -370,6 +370,22 @@
#define AM33XX_DEV_FEATURE0x604
#define AM33XX_SGX_MASK BIT(29)
+/* AM33XX M3_TXEV_EOI register */
+#define AM33XX_CONTROL_M3_TXEV_EOI 0x1324
+
+#define AM33XX_M3_TXEV_ACK (0x1 0)
+#define AM33XX_M3_TXEV_ENABLE (0x0 0)
+
+/* AM33XX IPC message registers */
+#define AM33XX_CONTROL_IPC_MSG_REG00x1328
+#define AM33XX_CONTROL_IPC_MSG_REG10x132C
+#define AM33XX_CONTROL_IPC_MSG_REG20x1330
+#define AM33XX_CONTROL_IPC_MSG_REG30x1334
+#define AM33XX_CONTROL_IPC_MSG_REG40x1338
+#define AM33XX_CONTROL_IPC_MSG_REG50x133C
+#define AM33XX_CONTROL_IPC_MSG_REG60x1340
+#define AM33XX_CONTROL_IPC_MSG_REG70x1344
+
/* CONTROL OMAP STATUS register to identify OMAP3 features */
#define OMAP3_CONTROL_OMAP_STATUS 0x044c
@@ -429,6 +445,44 @@ extern void omap3630_ctrl_disable_rta(void);
extern int omap3_ctrl_save_padconf(void);
extern void omap2_set_globals_control(void __iomem *ctrl,
void __iomem *ctrl_pad);
+struct am33xx_ipc_data {
+ u32 resume_addr;
+ u32 sleep_mode;
+ u32 param1;
+ u32 param2;
+ u32 param3;
+ u32 param4;
+ u32 param5;
+ u32 param6;
+};
+
+#define IPC_RESP_SHIFT 16
+#define IPC_RESP_MASK (0x 16)
+
+#define M3_VERSION_SHIFT 0
+#define M3_VERSION_MASK(0x 0)
+
+/*
+ * 9-4 = VTT GPIO PIN (6 Bits)
+ * 3 = VTT Status (1 Bit)
+ * 2-0 = Memory Type (2 Bits)
+*/
+#define MEM_TYPE_SHIFT (0x0)
+#define MEM_TYPE_MASK (0x7 0)
+#define VTT_STAT_SHIFT (0x3)
+#define VTT_STAT_MASK (0x1 3)
+#define VTT_GPIO_PIN_SHIFT (0x4)
+#define VTT_GPIO_PIN_MASK (0x2f 4)
+
+extern void am33xx_wkup_m3_ipc_cmd(struct am33xx_ipc_data *data);
+extern void am33xx_txev_eoi(void);
+extern void am33xx_txev_enable(void);
+extern void am33xx_pm_version_clear(void);
+extern int am33xx_pm_version_get(void);
+extern void am33xx_pm_ipc_cmd(struct am33xx_ipc_data *data);
+extern int am33xx_pm_status(void);
+extern int am33xx_pm_wake_src(void);
+
Lets address the above better. I don't see a need of 8 functions
exported doing one or 2 register writes.
Look M3 based handling is going to be there on future SOCs
as well and this kind of handling of IPC is very short cited.
The idea here was to move all control module register accesses into one
file in planning of
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
* Dave Gerlach d-gerl...@ti.com [130808 09:23]: On 08/08/2013 08:44 AM, Santosh Shilimkar wrote: Lets address the above better. I don't see a need of 8 functions exported doing one or 2 register writes. Look M3 based handling is going to be there on future SOCs as well and this kind of handling of IPC is very short cited. The idea here was to move all control module register accesses into one file in planning of implementing a driver for the control module itself in the future. Probably we should have a separate driver for M3 in linux which can have all this local code instead of all these exports. The wkup_m3 code has been moved to a small driver found in patch 8 of this series, would it better to move this code there rather than with the rest of the control module code? Please make everything you can into regular device drivers. We still have some dependencies to mach-omap2 code for PRCM for example, but we're trying to get all that to live in drivers. So for new pieces, let's not add further dependencies to complicate moving things to drivers. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 2/9] ARM: OMAP2+: AM33XX: control: Add some control module registers and APIs
On Tue, Aug 6, 2013 at 10:49 AM, Dave Gerlach d-gerl...@ti.com wrote:
From: Vaibhav Bedia vaibhav.be...@ti.com
Interacting with WKUP-M3 requires some more control
module register writes. Add the register offsets and
APIs to write to these.
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
Signed-off-by: Dave Gerlach d-gerl...@ti.com
Reviewed-by: Russ Dill russ.d...@ti.com
---
arch/arm/mach-omap2/control.c | 57
+
arch/arm/mach-omap2/control.h | 54 ++
2 files changed, 111 insertions(+)
diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 31e0dfe..934041a 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -605,3 +605,60 @@ int omap3_ctrl_save_padconf(void)
}
#endif /* CONFIG_ARCH_OMAP3 CONFIG_PM */
+
+#if defined(CONFIG_SOC_AM33XX) defined(CONFIG_PM)
+void am33xx_txev_eoi(void)
+{
+ omap_ctrl_writel(AM33XX_M3_TXEV_ACK, AM33XX_CONTROL_M3_TXEV_EOI);
+}
+
+void am33xx_txev_enable(void)
+{
+ omap_ctrl_writel(AM33XX_M3_TXEV_ENABLE, AM33XX_CONTROL_M3_TXEV_EOI);
+}
+
+/*
+ * Invalidate M3 firmware version before hardreset.
+ * Write invalid version in lower 4 nibbles of parameter
+ * register (ipc_regs + 0x8).
+ */
+void am33xx_pm_version_clear(void)
+{
+ omap_ctrl_writel(0x, AM33XX_CONTROL_IPC_MSG_REG2);
+}
+
+int am33xx_pm_version_get(void)
+{
+ return omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG2) M3_VERSION_MASK;
+}
+
+void am33xx_pm_ipc_cmd(struct am33xx_ipc_data *data)
+{
+ omap_ctrl_writel(data-resume_addr, AM33XX_CONTROL_IPC_MSG_REG0);
+ omap_ctrl_writel(data-sleep_mode, AM33XX_CONTROL_IPC_MSG_REG1);
+ omap_ctrl_writel(data-param1, AM33XX_CONTROL_IPC_MSG_REG2);
+ omap_ctrl_writel(data-param2, AM33XX_CONTROL_IPC_MSG_REG3);
+ omap_ctrl_writel(data-param3, AM33XX_CONTROL_IPC_MSG_REG4);
+}
+
+int am33xx_pm_status(void)
+{
+ int i;
+
+ i = omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG1);
+ i = IPC_RESP_MASK;
+ i = __ffs(IPC_RESP_MASK);
+
+ return i;
+}
+
+int am33xx_pm_wake_src(void)
+{
+ int i;
+
+ i = omap_ctrl_readl(AM33XX_CONTROL_IPC_MSG_REG6);
+ i = 0xff;
+
+ return i;
+}
+#endif /* CONFIG_SOC_AM33XX CONFIG_PM */
diff --git a/arch/arm/mach-omap2/control.h b/arch/arm/mach-omap2/control.h
index f7d7c2e..9be587c 100644
--- a/arch/arm/mach-omap2/control.h
+++ b/arch/arm/mach-omap2/control.h
@@ -370,6 +370,22 @@
#define AM33XX_DEV_FEATURE 0x604
#define AM33XX_SGX_MASKBIT(29)
+/* AM33XX M3_TXEV_EOI register */
+#define AM33XX_CONTROL_M3_TXEV_EOI 0x1324
+
+#define AM33XX_M3_TXEV_ACK (0x1 0)
+#define AM33XX_M3_TXEV_ENABLE (0x0 0)
+
+/* AM33XX IPC message registers */
+#define AM33XX_CONTROL_IPC_MSG_REG00x1328
+#define AM33XX_CONTROL_IPC_MSG_REG10x132C
+#define AM33XX_CONTROL_IPC_MSG_REG20x1330
+#define AM33XX_CONTROL_IPC_MSG_REG30x1334
+#define AM33XX_CONTROL_IPC_MSG_REG40x1338
+#define AM33XX_CONTROL_IPC_MSG_REG50x133C
+#define AM33XX_CONTROL_IPC_MSG_REG60x1340
+#define AM33XX_CONTROL_IPC_MSG_REG70x1344
+
/* CONTROL OMAP STATUS register to identify OMAP3 features */
#define OMAP3_CONTROL_OMAP_STATUS 0x044c
@@ -429,6 +445,44 @@ extern void omap3630_ctrl_disable_rta(void);
extern int omap3_ctrl_save_padconf(void);
extern void omap2_set_globals_control(void __iomem *ctrl,
void __iomem *ctrl_pad);
+struct am33xx_ipc_data {
+ u32 resume_addr;
+ u32 sleep_mode;
+ u32 param1;
+ u32 param2;
+ u32 param3;
+ u32 param4;
+ u32 param5;
+ u32 param6;
+};
+
+#define IPC_RESP_SHIFT 16
+#define IPC_RESP_MASK (0x 16)
+
+#define M3_VERSION_SHIFT 0
+#define M3_VERSION_MASK(0x 0)
+
+/*
+ * 9-4 = VTT GPIO PIN (6 Bits)
+ * 3 = VTT Status (1 Bit)
+ * 2-0 = Memory Type (2 Bits)
+*/
+#define MEM_TYPE_SHIFT (0x0)
+#define MEM_TYPE_MASK (0x7 0)
+#define VTT_STAT_SHIFT (0x3)
+#define VTT_STAT_MASK (0x1 3)
+#define VTT_GPIO_PIN_SHIFT (0x4)
+#define VTT_GPIO_PIN_MASK (0x2f 4)
+
+extern void am33xx_wkup_m3_ipc_cmd(struct am33xx_ipc_data *data);
+extern void am33xx_txev_eoi(void);
+extern void am33xx_txev_enable(void);
+extern void am33xx_pm_version_clear(void);
+extern int am33xx_pm_version_get(void);
+extern void am33xx_pm_ipc_cmd(struct am33xx_ipc_data *data);
+extern int am33xx_pm_status(void);
+extern int am33xx_pm_wake_src(void);
+
#else
#define omap_ctrl_base_get() 0
#define omap_ctrl_readb(x) 0
--
1.7.9.5
___
linux-arm-kernel mailing list
