Re: [PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver
Hi Philipp, On Monday 09 September 2013 02:36 PM, Philipp Zabel wrote: > So if I understand correctly, the only problem is that on OMAP the clock > needs to be enabled to deassert the reset, but as long as the clock > domain is in hardware supervised mode, it won't be enabled? Yes, enabling clock with reset deassertion might not reset the module if the clock domain is in hardware supervised mode. > Would it be possible to create an internal API to switch the clock > domain to software supervised mode, which can be used both by the code > behind pm_runtime_get_sync and reset_control_deassert? I will see if that is acceptable. Another option that would have to be explored is invoking device_reset() (taking care of clear, deassert & status checking as you suggested) midway through pm_run_time_get_sync(), when the clockdomain is in software supervised mode with reset driver taking care of any particular sequence in the case of multiple reset signals, instead of the IP driver requiring to take care of it. Regards Afzal -- 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: [PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver
Am Donnerstag, den 05.09.2013, 21:26 +0530 schrieb Afzal Mohammed: > Hi Philipp, > > On Thursday 05 September 2013 03:37 PM, Philipp Zabel wrote: > > Am Montag, den 02.09.2013, 19:41 +0530 schrieb Afzal Mohammed: > > >> Two new reset API's are provided to check whether reset is ready and > >> to clear reset. This would be required in case IP needs to mix reset > >> handling procedure with power/clock managment procedure to achieve > >> proper reset and these procedures are sometimes IP specific that would > >> make it difficult to handle reset fully in pm_runtime platform support. > > >> client IP handling s/w (DT based) should do as follows: > > >> 2. In driver, that require reset to be deasserted, > >> (this is the sequence required for gfx on AM43x/AM335x, this would > >> depend on requirements of the IP) > >> > >>mydriver_probe(struct platform device *pdev) > >>{ > >>: > >>: > >>reset_control_get(&pdev->dev, NULL); > >>reset_control_clear_reset(); > >>reset_control_deassert(); > >>pm_runtime_get_sync(); > >>if (reset_control_is_reset() != true) > >>goto err; > >>reset_control_put(); > >>: > >>: > >>} > > > if possible, I'd like to move this logic into the reset controller > > driver. Can this be reordered to enable power before deasserting the > > reset line (assuming it is initially asserted)? In this case, I'd > > suggest to just call device_reset: > > > > pm_runtime_get_sync(&pdev->dev); > > ret = device_reset(&pdev->dev); > > if (ret) > > goto err; > > > > The ops.reset callback in the prcm driver then can handle clearing > > the reset status bit, deasserting the reset control bit, and waiting for > > the reset status bit to be set (or timing out with an error). > > I too would have loved to have it in such a clean way and was initially > proceeding in that direction, but there is an issue specific to OMAP > family SoC's, which was required to be taken care of (even though > present use case for AM335x/AM43x would work [as it does not have > hardware supervised clockdomain mode] with a small change in platform > level power management support code - a generic one shared with other > OMAP family SoC's) > > For a module to be reset, clock domain to which the module clock belongs > should be set to software supervised mode. During "pm_runtime_get_sync", > in OMAP platform level handling code, it first put clockdomain into > software supervised mode, enables clock to module, and once module is > ready, module will be put to hardware supervised mode. In hardware > supervised mode, reset may not happen. > > So if device_reset() is done after pm_runtime_get_sync(), reset may not > happen as by the time device_reset() is called, clockdomain would be in > hardware supervised mode. But in other case, as reset is already > deasserted when pm_runtime_get_sync() is called, module would be reset > as it first puts to software supervised mode. > > And device reset would happen only upon enabling clock to module (if > reset was deasserted) by pm_runtime_get_sync(), so reset status has to > be checked after pm call, preventing us having device_reset() before > pm_runtime_get_sync(), or else in that case we have to sacrifice on > reset status checking (which may not be reliable) > > Another alternative would have been to integrate this reset handling in > low level power management code thus hiding all reset handling from > driver, but as far as I know the reset sequence to be done is sometimes > IP specific, preventing it. So if I understand correctly, the only problem is that on OMAP the clock needs to be enabled to deassert the reset, but as long as the clock domain is in hardware supervised mode, it won't be enabled? Would it be possible to create an internal API to switch the clock domain to software supervised mode, which can be used both by the code behind pm_runtime_get_sync and reset_control_deassert? regards Philipp -- 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: [PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver
Hi Philipp, On Thursday 05 September 2013 03:37 PM, Philipp Zabel wrote: > Am Montag, den 02.09.2013, 19:41 +0530 schrieb Afzal Mohammed: >> Two new reset API's are provided to check whether reset is ready and >> to clear reset. This would be required in case IP needs to mix reset >> handling procedure with power/clock managment procedure to achieve >> proper reset and these procedures are sometimes IP specific that would >> make it difficult to handle reset fully in pm_runtime platform support. >> client IP handling s/w (DT based) should do as follows: >> 2. In driver, that require reset to be deasserted, >> (this is the sequence required for gfx on AM43x/AM335x, this would >> depend on requirements of the IP) >> >> mydriver_probe(struct platform device *pdev) >> { >> : >> : >> reset_control_get(&pdev->dev, NULL); >> reset_control_clear_reset(); >> reset_control_deassert(); >> pm_runtime_get_sync(); >> if (reset_control_is_reset() != true) >> goto err; >> reset_control_put(); >> : >> : >> } > if possible, I'd like to move this logic into the reset controller > driver. Can this be reordered to enable power before deasserting the > reset line (assuming it is initially asserted)? In this case, I'd > suggest to just call device_reset: > > pm_runtime_get_sync(&pdev->dev); > ret = device_reset(&pdev->dev); > if (ret) > goto err; > > The ops.reset callback in the prcm driver then can handle clearing > the reset status bit, deasserting the reset control bit, and waiting for > the reset status bit to be set (or timing out with an error). I too would have loved to have it in such a clean way and was initially proceeding in that direction, but there is an issue specific to OMAP family SoC's, which was required to be taken care of (even though present use case for AM335x/AM43x would work [as it does not have hardware supervised clockdomain mode] with a small change in platform level power management support code - a generic one shared with other OMAP family SoC's) For a module to be reset, clock domain to which the module clock belongs should be set to software supervised mode. During "pm_runtime_get_sync", in OMAP platform level handling code, it first put clockdomain into software supervised mode, enables clock to module, and once module is ready, module will be put to hardware supervised mode. In hardware supervised mode, reset may not happen. So if device_reset() is done after pm_runtime_get_sync(), reset may not happen as by the time device_reset() is called, clockdomain would be in hardware supervised mode. But in other case, as reset is already deasserted when pm_runtime_get_sync() is called, module would be reset as it first puts to software supervised mode. And device reset would happen only upon enabling clock to module (if reset was deasserted) by pm_runtime_get_sync(), so reset status has to be checked after pm call, preventing us having device_reset() before pm_runtime_get_sync(), or else in that case we have to sacrifice on reset status checking (which may not be reliable) Another alternative would have been to integrate this reset handling in low level power management code thus hiding all reset handling from driver, but as far as I know the reset sequence to be done is sometimes IP specific, preventing it. Regards Afzal -- 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: [PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver
Hi Afzal, Am Montag, den 02.09.2013, 19:41 +0530 schrieb Afzal Mohammed: > Hi, > > This is an attempt to achieve reset on AM43x/AM335x based SoC's with > reset driver making use of the reset framework. > > prcm node is added in device tree, which would hold reset bindings. > Initially node was made as a one that represents reset functionality > of SoC. but ended up with node for prcm (which is felt to be natural > choice) instead. I am a bit doubtful whether placement of prcm node in > root node as in this series is the right thing. > > Reset driver gets probed for specific prcm node, the same defines > register details to be used for a particular SoC (using ".data" field > associated with ".compatible" in driver match table). > > Another option to handle different SoC's would be to add register > details to DT and let the driver extract it from DT. I vaguely > remember seeing a thread mentioning that putting register details in > DT is not preferred. But open to putting register level details > instead in DT if that is being generally preferred. This would have > advantage that adding reset support for a new SoC would be easier, but > would have to put more thought before doing so as DT bindings should > not change. > > With the approach taken here, for supporting a new SoC with new prcm > register details, driver would have to be updated much like the way a > pci based ethernet driver would have to be updated to handle a new IP > version that is not register level compatible with the existing ones. > > In this series out of the three IP's (gfx, m3, pruss) that would need > reset support, here as a proof of concept only gfx is taken care. > Other's can be easily supported by adding new register data array > entries. > > Two new reset API's are provided to check whether reset is ready and > to clear reset. This would be required in case IP needs to mix reset > handling procedure with power/clock managment procedure to achieve > proper reset and these procedures are sometimes IP specific that would > make it difficult to handle reset fully in pm_runtime platform support. > > *--* > client IP handling s/w (DT based) should do as follows: > > 1. Specify reset handle in the relevant DT node, for eg. > > myip@deadbeef { > : > : > /* here prcm is the handle to reset binding node */ > resets = <&prcm 0>; > }; > > 2. In driver, that require reset to be deasserted, > (this is the sequence required for gfx on AM43x/AM335x, this would > depend on requirements of the IP) > > mydriver_probe(struct platform device *pdev) > { > : > : > reset_control_get(&pdev->dev, NULL); > reset_control_clear_reset(); > reset_control_deassert(); > pm_runtime_get_sync(); > if (reset_control_is_reset() != true) > goto err; > reset_control_put(); > : > : > } > > *--* if possible, I'd like to move this logic into the reset controller driver. Can this be reordered to enable power before deasserting the reset line (assuming it is initially asserted)? In this case, I'd suggest to just call device_reset: pm_runtime_get_sync(&pdev->dev); ret = device_reset(&pdev->dev); if (ret) goto err; The ops.reset callback in the prcm driver then can handle clearing the reset status bit, deasserting the reset control bit, and waiting for the reset status bit to be set (or timing out with an error). > May be removing reset handling in hwmod can be considered by making > use of reset driver. > > Or as another extreme, perhaps, other logic's in the prcm can be > handled by a new prcm driver and then this reset driver can be a child > of it. > > Regards > Afzal > > > Afzal Mohammed (6): > reset: is_reset and clear_reset api's > doc: dt: binding: omap: am43x/am335x prcm reset > reset: am43x/am335x support > ARM: OMAP2+: AM43x/AM335x: have reset controller > ARM: dts: AM335x: prcm node (for reset) > ARM: dts: AM4372: prcm node (for reset) > > .../devicetree/bindings/arm/omap/prcm.txt | 13 ++ > arch/arm/boot/dts/am33xx.dtsi | 6 + > arch/arm/boot/dts/am4372.dtsi | 6 + > arch/arm/mach-omap2/Kconfig| 2 + > drivers/reset/Kconfig | 14 ++ > drivers/reset/Makefile | 1 + > drivers/reset/amx3_reset.c | 157 > + > drivers/reset/core.c | 32 + > include/linux/reset-controller.h | 2 + > include/linux/reset.h | 2 + > 10 files changed, 235 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/omap/prcm.txt > create mode 100644 drivers/reset/amx3_reset.c
[PATCH RFC 0/6] ARM: OMAP2+: AM43x/AM335x prcm reset driver
Hi, This is an attempt to achieve reset on AM43x/AM335x based SoC's with reset driver making use of the reset framework. prcm node is added in device tree, which would hold reset bindings. Initially node was made as a one that represents reset functionality of SoC. but ended up with node for prcm (which is felt to be natural choice) instead. I am a bit doubtful whether placement of prcm node in root node as in this series is the right thing. Reset driver gets probed for specific prcm node, the same defines register details to be used for a particular SoC (using ".data" field associated with ".compatible" in driver match table). Another option to handle different SoC's would be to add register details to DT and let the driver extract it from DT. I vaguely remember seeing a thread mentioning that putting register details in DT is not preferred. But open to putting register level details instead in DT if that is being generally preferred. This would have advantage that adding reset support for a new SoC would be easier, but would have to put more thought before doing so as DT bindings should not change. With the approach taken here, for supporting a new SoC with new prcm register details, driver would have to be updated much like the way a pci based ethernet driver would have to be updated to handle a new IP version that is not register level compatible with the existing ones. In this series out of the three IP's (gfx, m3, pruss) that would need reset support, here as a proof of concept only gfx is taken care. Other's can be easily supported by adding new register data array entries. Two new reset API's are provided to check whether reset is ready and to clear reset. This would be required in case IP needs to mix reset handling procedure with power/clock managment procedure to achieve proper reset and these procedures are sometimes IP specific that would make it difficult to handle reset fully in pm_runtime platform support. *--* client IP handling s/w (DT based) should do as follows: 1. Specify reset handle in the relevant DT node, for eg. myip@deadbeef { : : /* here prcm is the handle to reset binding node */ resets = <&prcm 0>; }; 2. In driver, that require reset to be deasserted, (this is the sequence required for gfx on AM43x/AM335x, this would depend on requirements of the IP) mydriver_probe(struct platform device *pdev) { : : reset_control_get(&pdev->dev, NULL); reset_control_clear_reset(); reset_control_deassert(); pm_runtime_get_sync(); if (reset_control_is_reset() != true) goto err; reset_control_put(); : : } *--* May be removing reset handling in hwmod can be considered by making use of reset driver. Or as another extreme, perhaps, other logic's in the prcm can be handled by a new prcm driver and then this reset driver can be a child of it. Regards Afzal Afzal Mohammed (6): reset: is_reset and clear_reset api's doc: dt: binding: omap: am43x/am335x prcm reset reset: am43x/am335x support ARM: OMAP2+: AM43x/AM335x: have reset controller ARM: dts: AM335x: prcm node (for reset) ARM: dts: AM4372: prcm node (for reset) .../devicetree/bindings/arm/omap/prcm.txt | 13 ++ arch/arm/boot/dts/am33xx.dtsi | 6 + arch/arm/boot/dts/am4372.dtsi | 6 + arch/arm/mach-omap2/Kconfig| 2 + drivers/reset/Kconfig | 14 ++ drivers/reset/Makefile | 1 + drivers/reset/amx3_reset.c | 157 + drivers/reset/core.c | 32 + include/linux/reset-controller.h | 2 + include/linux/reset.h | 2 + 10 files changed, 235 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/omap/prcm.txt create mode 100644 drivers/reset/amx3_reset.c -- 1.8.3.4 -- 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