| Message ID | 20180727184527.13287-1-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add Reset Controller support for Actions Semi Owl SoCs | expand |
Hi Mani, Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam: > This patchset adds Reset Controller (RMU) support for Actions Semi > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into > the clock subsystem in hardware. Hence, in software we integrate RMU > support into common clock driver inorder to maintain compatibility. Can this not be placed into drivers/reset/ by using mfd-simple with a sub-node in DT? Regards, Andreas
Hi Andreas, On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote: > Hi Mani, > > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam: > > This patchset adds Reset Controller (RMU) support for Actions Semi > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into > > the clock subsystem in hardware. Hence, in software we integrate RMU > > support into common clock driver inorder to maintain compatibility. > > Can this not be placed into drivers/reset/ by using mfd-simple with a > sub-node in DT? > Actually I was not sure where to place this reset controller driver. When I looked into other similar ones such as sunxi, they just integrated into the clk subsystem. So I just chose that path. But yeah, this is hacky! But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset) are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since RMU has only 2 registers, the HW designers decided to use up the CMU memory map. So, maybe syscon would be best option I think. What is your opinion? Even if we go for syscon, we should place the reset driver within clk framework as I can see other SoCs like Mediatek are doing the same. But again I'm not sure! Thanks, Mani > Regards, > Andreas > > -- > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg)
On Mon, 2018-07-30 at 20:41 +0530, Manivannan Sadhasivam wrote: > Hi Andreas, > > On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote: > > Hi Mani, > > > > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam: > > > This patchset adds Reset Controller (RMU) support for Actions Semi > > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into > > > the clock subsystem in hardware. Hence, in software we integrate RMU > > > support into common clock driver inorder to maintain compatibility. > > > > Can this not be placed into drivers/reset/ by using mfd-simple with a > > sub-node in DT? > > > > Actually I was not sure where to place this reset controller driver. When I > looked into other similar ones such as sunxi, they just integrated into the > clk subsystem. So I just chose that path. But yeah, this is hacky! > > But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset) > are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since > RMU has only 2 registers, the HW designers decided to use up the CMU memory > map. So, maybe syscon would be best option I think. What is your opinion? Using syscon seems cleaner than stuffing the regmap into owl_clk_desc. > Even if we go for syscon, we should place the reset driver within clk > framework as I can see other SoCs like Mediatek are doing the same. But again > I'm not sure! Me neither. If the CMU and RMU are really separate and only share the memory map, a syscon driver could live in drivers/reset without problems. It's only when there are interactions between clocks and resets that you really want to have the reset driver integrated with clk. regards Philipp
Hi Philipp, On Mon, Jul 30, 2018 at 05:38:31PM +0200, Philipp Zabel wrote: > On Mon, 2018-07-30 at 20:41 +0530, Manivannan Sadhasivam wrote: > > Hi Andreas, > > > > On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote: > > > Hi Mani, > > > > > > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam: > > > > This patchset adds Reset Controller (RMU) support for Actions Semi > > > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into > > > > the clock subsystem in hardware. Hence, in software we integrate RMU > > > > support into common clock driver inorder to maintain compatibility. > > > > > > Can this not be placed into drivers/reset/ by using mfd-simple with a > > > sub-node in DT? > > > > > > > Actually I was not sure where to place this reset controller driver. When I > > looked into other similar ones such as sunxi, they just integrated into the > > clk subsystem. So I just chose that path. But yeah, this is hacky! > > > > But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset) > > are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since > > RMU has only 2 registers, the HW designers decided to use up the CMU memory > > map. So, maybe syscon would be best option I think. What is your opinion? > > Using syscon seems cleaner than stuffing the regmap into owl_clk_desc. > Okay. > > Even if we go for syscon, we should place the reset driver within clk > > framework as I can see other SoCs like Mediatek are doing the same. But again > > I'm not sure! > > Me neither. If the CMU and RMU are really separate and only share the > memory map, a syscon driver could live in drivers/reset without > problems. > It's only when there are interactions between clocks and resets that you > really want to have the reset driver integrated with clk. > Okay. Then I will add it as a syscon driver under drivers/reset. I hope that Andreas would also be happy with this. Thanks a lot for your response! Regards, Mani > regards > Philipp
On Mon, Jul 30, 2018 at 08:41:31PM +0530, Manivannan Sadhasivam wrote: > Hi Andreas, > > On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote: > > Hi Mani, > > > > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam: > > > This patchset adds Reset Controller (RMU) support for Actions Semi > > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into > > > the clock subsystem in hardware. Hence, in software we integrate RMU > > > support into common clock driver inorder to maintain compatibility. > > > > Can this not be placed into drivers/reset/ by using mfd-simple with a > > sub-node in DT? That is exactly what I tell folks not to do. Design the DT based on h/w blocks, not current desired driver split for some OS. > Actually I was not sure where to place this reset controller driver. When I > looked into other similar ones such as sunxi, they just integrated into the > clk subsystem. So I just chose that path. But yeah, this is hacky! > > But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset) > are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since > RMU has only 2 registers, the HW designers decided to use up the CMU memory > map. So, maybe syscon would be best option I think. What is your opinion? If there's nothing shared then it is not a syscon. If you can create separate address ranges, then 2 nodes is probably okay. If the registers are all mixed up, then 1 node. Rob
Hi Rob, On Tue, Aug 07, 2018 at 12:47:10PM -0600, Rob Herring wrote: > On Mon, Jul 30, 2018 at 08:41:31PM +0530, Manivannan Sadhasivam wrote: > > Hi Andreas, > > > > On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote: > > > Hi Mani, > > > > > > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam: > > > > This patchset adds Reset Controller (RMU) support for Actions Semi > > > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into > > > > the clock subsystem in hardware. Hence, in software we integrate RMU > > > > support into common clock driver inorder to maintain compatibility. > > > > > > Can this not be placed into drivers/reset/ by using mfd-simple with a > > > sub-node in DT? > > That is exactly what I tell folks not to do. Design the DT based on h/w > blocks, not current desired driver split for some OS. > > > Actually I was not sure where to place this reset controller driver. When I > > looked into other similar ones such as sunxi, they just integrated into the > > clk subsystem. So I just chose that path. But yeah, this is hacky! > > > > But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset) > > are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since > > RMU has only 2 registers, the HW designers decided to use up the CMU memory > > map. So, maybe syscon would be best option I think. What is your opinion? > > If there's nothing shared then it is not a syscon. If you can create > separate address ranges, then 2 nodes is probably okay. If the registers > are all mixed up, then 1 node. > I don't quite understand the reason for not being syscon. The definition of syscon says that, "System controller node represents a register region containing a set of miscellaneous registers. The registers are not cohesive enough to represent as any specific type of device." which exactly fits this case. Only the registers of CMU & RMU are shared and not the HW block! Can you please clarify? Thanks, Mani > Rob
On Wed, Aug 8, 2018 at 11:30 AM Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > > Hi Rob, > > On Tue, Aug 07, 2018 at 12:47:10PM -0600, Rob Herring wrote: > > On Mon, Jul 30, 2018 at 08:41:31PM +0530, Manivannan Sadhasivam wrote: > > > Hi Andreas, > > > > > > On Mon, Jul 30, 2018 at 12:26:07PM +0200, Andreas Färber wrote: > > > > Hi Mani, > > > > > > > > Am 27.07.2018 um 20:45 schrieb Manivannan Sadhasivam: > > > > > This patchset adds Reset Controller (RMU) support for Actions Semi > > > > > Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into > > > > > the clock subsystem in hardware. Hence, in software we integrate RMU > > > > > support into common clock driver inorder to maintain compatibility. > > > > > > > > Can this not be placed into drivers/reset/ by using mfd-simple with a > > > > sub-node in DT? > > > > That is exactly what I tell folks not to do. Design the DT based on h/w > > blocks, not current desired driver split for some OS. > > > > > Actually I was not sure where to place this reset controller driver. When I > > > looked into other similar ones such as sunxi, they just integrated into the > > > clk subsystem. So I just chose that path. But yeah, this is hacky! > > > > > > But this RMU is not MFD by any means. Since the CMU (Clock) and RMU (Reset) > > > are two separate IPs inside SoC, we shouldn't describe it as a MFD driver. Since > > > RMU has only 2 registers, the HW designers decided to use up the CMU memory > > > map. So, maybe syscon would be best option I think. What is your opinion? > > > > If there's nothing shared then it is not a syscon. If you can create > > separate address ranges, then 2 nodes is probably okay. If the registers > > are all mixed up, then 1 node. > > > > I don't quite understand the reason for not being syscon. The definition > of syscon says that, "System controller node represents a register region > containing a set of miscellaneous registers. The registers are not cohesive > enough to represent as any specific type of device." which exactly fits > this case. Only the registers of CMU & RMU are shared and not the HW block! > > Can you please clarify? IIRC, the original intent of 'syscon' was really for things that had no subsystem. Random bits all just dumped together. A block with clock and reset doesn't sounds pretty well defined functions. Plus that criteria doesn't work well because what does and doesn't have a subsystem (and DT binding) evolves. IMO, we should probably get rid of 'syscon'. Let me turn it around. Why do you want it do be a syscon? Simply to create a regmap I suppose because that is all that 'syscon' compatible is. A flag to create a regmap. Why do you need a regmap then? Do you need to protect concurrent accesses (a single register has both clock and reset bits). If so, you can't really call CMU and RMU 2 blocks. If not, you don't really need regmap. Rob
This patchset adds Reset Controller (RMU) support for Actions Semi Owl SoCs, S900 and S700. For the Owl SoCs, RMU has been integrated into the clock subsystem in hardware. Hence, in software we integrate RMU support into common clock driver inorder to maintain compatibility. This patch series depends on the recently posted S700 clk series: "[PATCH v7 0/5] Add clock driver for Actions S700 SoC". For the S700 clk series, driver and bindings patches are applied through the clk tree. But the DTS patches are not yet picked up by the platform maintainer, Andreas. Hence, Andreas is expected to pick the DTS patches in this series once reviewed by the maintainers along with S700 clk DTS patches. Because of the absence of the S500 SoC clk support, the reset controller registration code is added to both S700 and S900 SoC clk drivers for now. But once S500 clk support is added, the reset controller registration part will be moved to Owl SoCs common clk code. Thanks, Mani Manivannan Sadhasivam (9): clk: actions: Cache regmap info in private clock descriptor dt-bindings: clock: Add reset controller bindings for Actions Semi Owl SoCs dt-bindings: reset: Add binding constants for Actions Semi S700 RMU dt-bindings: reset: Add binding constants for Actions Semi S900 RMU arm64: dts: actions: Add Reset Controller support for S700 SoC arm64: dts: actions: Add Reset Controller support for S900 SoC clk: actions: Add Actions Semi Owl SoCs Reset Management Unit support clk: actions: Add Actions Semi S700 SoC Reset Management Unit support clk: actions: Add Actions Semi S900 SoC Reset Management Unit support .../bindings/clock/actions,owl-cmu.txt | 2 + arch/arm64/boot/dts/actions/s700.dtsi | 2 + arch/arm64/boot/dts/actions/s900.dtsi | 2 + drivers/clk/actions/Kconfig | 1 + drivers/clk/actions/Makefile | 1 + drivers/clk/actions/owl-common.c | 3 +- drivers/clk/actions/owl-common.h | 5 +- drivers/clk/actions/owl-reset.c | 72 ++++++++++++++++ drivers/clk/actions/owl-reset.h | 32 +++++++ drivers/clk/actions/owl-s700.c | 55 +++++++++++- drivers/clk/actions/owl-s900.c | 86 ++++++++++++++++++- .../dt-bindings/reset/actions,s700-reset.h | 34 ++++++++ .../dt-bindings/reset/actions,s900-reset.h | 65 ++++++++++++++ 13 files changed, 354 insertions(+), 6 deletions(-) create mode 100644 drivers/clk/actions/owl-reset.c create mode 100644 drivers/clk/actions/owl-reset.h create mode 100644 include/dt-bindings/reset/actions,s700-reset.h create mode 100644 include/dt-bindings/reset/actions,s900-reset.h